Open AndrewFinlay opened 5 years ago
Could you test this with nyc 15 beta? In nyc 15 it should be possible to run nyc --all=true your-test-runner
on instrumented code. nyc 14 did not support loading --all
coverage from already instrumented files but this should be fixed in nyc 15 (among other fixes to the handling of --all
).
I'm using the instrumented source as input to an e2e testing setup so I can't wrap your-test-runner
in nyc unfortunately.
I'm currently working with 15 beta, and nyc --all=true /usr/bin/true
works to generate a baseline coverage file on uninstrumented source.
It's a little flaky running the same command on already instrumented source and seems to not report a few files in the project I'm working with.
For completeness the old istanbul instrument
switch was --save-baseline
.
Thanks for explaining I understand now. I'm open to such a feature as long as it's opt-in, --save-baseline
seems like a fine idea.
It's a little flaky running the same command on already instrumented source and seems to not report a few files in the project I'm working with.
I would be interested in seeing an example of this. Running nyc --all=true /usr/bin/true
should capture 0% coverage for all files.
Cool, I'll try and put a PR together soon to add --save-baseline
.
Regarding the flakiness, I've tested this again this morning and can confirm there is a difference in behaviour between establishing a baseline against raw code and pre-instrumented code. It'll take a little while to put together a demonstration repo as I've only noticed this on our production source so far.
There seems to be two issues here,
nyc --all=true <options> /usr/bin/true
against already instrumented source some files are missed.nyc --all=true <options> /usr/bin/true
there is a loss of fidelity wrt number of statements, branches, functions and lines as everything has been put in its instrumented form.I'm not too sure what I expect nyc to do when running this baseline coverage task against pre-instrumented source though. How do you expect it to behave?
When nyc --all=true
is run I expect it to capture the correct coverage from pre-instrumented sources. This is a new thing for nyc@15 and I'm not really surprised that some edge cases do not work. I'd love to identify and fix those edge cases.
I'll try and put together a demo project but it might take a little time as I'm seeing the issue on internal source that I can't share without some redaction. I'll drop it into a new issue as it's a little off topic in here.
New issue for coverage differences #1208
Hi @coreyfarrell, I've got a couple of questions regarding this feature. I've got a rudimentary version of this going using the flag --save-baseline
. It currently produces a standard coverage file as created by nyc.addAllFiles()
and stores it as <temp-dir>/<uuid>.json
.
The questions I need answered are:
<temp-dir>
a good location for the output?<uuid>.json
a good name for the baseline coverage file?<temp-dir>/processinfo
directory and files?--clean
if we're saving to the <temp-dir>
?My gut feeling is that the above answers should be yes, no, yes, yes. But I'll wait to hear back before I proceed.
<temp-dir>
then it won't be found for reporting.<uuid>.json
or in a subfolder. Creating a file not matching this pattern could conflict with something someone else is doing (they might overwrite it).--save-baseline
process would fit into this tree or if the existence an orphaned processinfo file will break anything.--clean
will be essential if baseline is written directly to .nyc_output/<uuid>.json
.An alternative process I just thought of which might avoid the above issues:
nyc instrument --save-baseline
creates .nyc_output/baseline/coverage.json
nyc --save-baseline npm t
reads .nyc_output/baseline/coverage.json
and records it into <uuid>.json
of the nyc master process (the same one which would store --all
coverage). Any coverage acquired for baseline files would override coverage found by --all
(if both flags are used).This might be the best way to work baseline coverage into the normal process, I think would be easier to document? I wouldn't argue against nyc instrument
unconditionally creating .nyc_output/baseline/coverage.json
, then we could just have nyc --baseline npm t
(maybe also support nyc instrument --no-baseline
)? I think pulling baseline coverage into the active coverage needs to be opt-in to avoid potentially using outdated information (for example if a project stops pre-instrumenting). For this to work I think --clean
would need to ignore the .nyc_output/baseline
folder.
I've been thinking a little bit about this feature and it seems to me that we really want is what nyc --all ...
currently does. We should implement nyc instrument --all
, then it's quite reasonable to expect the two --all
commands to behave in the same way. No need for a separate baseline
directory, the filename can stay <uuid>.json
and the processInfo
files would still be there if needed.
This also simplifies the interface and the documentation.
Problem
Currently when testing with files created by the standalone instrument command, if a file isn't hit it will not produce a coverage output file. This can lead to untouched files being excluded from the coverage report. Istanbul handled this by being able to produce a baseline coverage file with content similar to that produced by running
nyc --all=true /usr/bin/true
. When the baseline coverage file is included with test generated coverage files and fed to the reporter, files that aren't hit during tests are included in the report.In hindsight I think this was the problem being faced in #1044.
Proposal
Add an option to the instrument command, 'baseline', that will produce a baseline coverage output json file similar to that produced by running
nyc --all=true /usr/bin/true
.Workaround
You can generate a report that includes untouched files with:
nyc --all <options> /usr/bin/true
against the uninstrumented code