Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add possibility to filter coverage maps on absolute filename #24

Merged
merged 2 commits into from
Apr 8, 2017
Merged

Add possibility to filter coverage maps on absolute filename #24

merged 2 commits into from
Apr 8, 2017

Conversation

schutm
Copy link
Contributor

@schutm schutm commented Mar 29, 2017

As discussed, previous changes redone in the mono-repo and introduced a parameter/flag in stead of a separate module.

The problem I try to resolve:
I've a 'non-standard' setup, where I build my distribution-packages outside of the source folder. I do this because I write mainly ES6 code, and distribution-packages need to be plain old JavaScript. In addition because AVA is perfectly able to run ES6 tests, but not able to test ES6 modules I've to transpile my ES6 code (using rollup and babel/bublé) to plain JavaScript. The resulting transpiled file will also include all libraries being imported.

The indication source files and line missing coverage works perfectly, using sourcemaps. However in the sourcemaps the original full paths are included. So if I run nyc with an include or exclude filter the I only have 2 options: show the coverage of all modules (including external modules, which I didn't intend to be instrumented to be covered), or no modules at all, because all of the sourcemaps absolute paths are outside of the current directory.

Therefore I introduced these methods, which I intend to use from nyc (either by having the --report flag taking a different or by introducing a flag (--no-relativepath, --absolutepath or something similar).

Now writing this all down, a solution might be to first have nyc do its work, including generating the report-dir, but no output. Changing to the original source directory again and invoking nyc --report --report-dir=<report-dir>. I've to validate this works.

@coveralls
Copy link

coveralls commented Mar 29, 2017

Coverage Status

Coverage decreased (-0.1%) to 91.199% when pulling 27acf57 on schutm:master into d334095 on istanbuljs:master.

@@ -58,6 +58,19 @@ CoverageMap.prototype.merge = function (obj) {
});
};
/**
* filter the coveragemap based on the callback provided
* @param {Function (filename)} callback - Returns true if the should be
Copy link
Member

Choose a reason for hiding this comment

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

typo, should probably read: "Returns true if the path should be"?

Copy link
Member

Choose a reason for hiding this comment

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

I think I'm blanking on how this additional filter option relates to the change in test-exclude, mind providing an example of the final functionality you're envisioning in the pull request description.

This is looking great btw, thanks for the contribution 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed the typo. And added additional information in the pull request description.

@coveralls
Copy link

coveralls commented Mar 30, 2017

Coverage Status

Coverage decreased (-0.1%) to 91.199% when pulling 337ded4 on schutm:master into d334095 on istanbuljs:master.

@schutm
Copy link
Contributor Author

schutm commented Mar 31, 2017

I checked whether I could work around my issue in another way (using --report-dir) which didn't work. I misread the help-explanation of this flag, it is no directory containing the raw report data, but the report output itself. In addition I tried to copy the .nyc-report directory to the original source directory, but running nyc --report in that case still shows all instrumented files. It seems the filtering (to only include files in the current directory) is only performed during instrumentation and not during reporting. Something already indicated by the method name: shouldInstrument

@bcoe bcoe merged commit e1c99d6 into istanbuljs:master Apr 8, 2017
vivek-freshworks pushed a commit to freshworks/istanbuljs that referenced this pull request Oct 17, 2023
arnabsen pushed a commit to arnabsen/istanbuljs that referenced this pull request Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants