Skip to content

allow coverage instrumentation of multiple files - #125

Merged
kof merged 2 commits into
qunitjs:masterfrom
Sage:directory-coverage
Feb 2, 2016
Merged

kof merged 2 commits into
qunitjs:masterfrom
Sage:directory-coverage

Conversation

@bjouhier

@bjouhier bjouhier commented Feb 1, 2016

Copy link
Copy Markdown
Contributor

This PR is a quick hack to enable code coverage on multiple files. It allows you to set the coverage option to:

  • a string, in which case all source files with an absolute path matching the string will be included in the coverage report.
  • a regexp, in which case all source files with a path that matches the regexp will be included.
  • true, in which case only the file specified with the code option will be included in the coverage report.

@fyockm

fyockm commented Feb 2, 2016

Copy link
Copy Markdown
Contributor

Hi @bjouhier!

Instead of overloading the coverage option as a string or Regex, do you think it would make more sense to add a new key for files?
While your proposed change provides flexiblility for specifying the coverage files, it wouldn't allow setting the other coverage options, like dir or reporters.

lib/coverage.js

options = {
    dir: 'coverage',
    reporters: ['lcov', 'json'],
    files: null
};

Not quite as clean, but then the matcher function could be something like this:

matcher = function(file) {
    if (options.coverage.files) {
        if (typeof options.coverage.files === 'string') return file.indexOf(options.coverage) === 0;
        else if (options.coverage.files instanceof RegExp) return options.coverage.test(file);
    }
    return file === options.code.path;
}

An even better solution might utilize pattern globbing, but perhaps as a future enhancement.

Comment thread lib/coverage.js Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think instead of string, we should use an array here, otherwise its fine ...

@bjouhier

bjouhier commented Feb 2, 2016

Copy link
Copy Markdown
Contributor Author

@fyockm Hi Drew!!!

That's a good point. I've followed your suggestion to use options.coverage.files and I've also enabled arrays. So you can specify an array of files or regexps.

@kof

kof commented Feb 2, 2016

Copy link
Copy Markdown
Contributor

merged

@bjouhier

bjouhier commented Feb 2, 2016

Copy link
Copy Markdown
Contributor Author

Thanks @kof

kof added a commit that referenced this pull request Feb 2, 2016
allow coverage instrumentation of multiple files
@kof
kof merged commit b0d6832 into qunitjs:master Feb 2, 2016
@bjouhier

bjouhier commented Feb 2, 2016

Copy link
Copy Markdown
Contributor Author

@kof @fyockm I just tried the regexp feature and found out that it is completely broken. Problem is that options are serialized between testrunner and child and JSON does not like regexps!

@kof

kof commented Feb 2, 2016

Copy link
Copy Markdown
Contributor

thats true, do we really need regexp there?

@kof

kof commented Feb 2, 2016

Copy link
Copy Markdown
Contributor

Can you add some tests?

@bjouhier

bjouhier commented Feb 2, 2016

Copy link
Copy Markdown
Contributor Author

I can manage without regexps. Looked like a cool and easy-to-implement feature but it's only cool.

I'll remove regexps and add tests but I'll do it a bit later.

@kof

kof commented Feb 2, 2016

Copy link
Copy Markdown
Contributor

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants