-
-
Notifications
You must be signed in to change notification settings - Fork 359
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
Inconsistent Coverage Report by NYC #537
Comments
Upon further experiment, I noticed from time to time I see messages such as: File [xxx.ts] ignored, nothing could be mapped. Whenever this message appears (sometimes a few lines, each for one module), then I won't get 100% coverage for Branch %. This seems to be random and intermittent, I can only reproduce it by repeatedly running the test and coverage script. I am using npm script as followings: "PORT=8000 nodemon --ext ts --watch ./ --exec 'nyc --reporter=text --reporter=html mocha -r ts-node/register ./test/**/*.spec.ts --recursive --reporter spec --slow 0' --delay 1" Hopes this help. |
I am seeing the same (branches sometimes 60%, sometimes 100%). |
@peterharding @CamelKing do me a favor and try running @peterharding are you also using |
@bcoe Same problem with diff --git a/lcov_30.info b/lcov_30.info
index 95c4dea..d9f64e2 100644
--- a/lcov_30.info
+++ b/lcov_30.info
@@ -250,15 +250,15 @@ DA:130,0
LF:49
LH:28
BRDA:78,0,0,3
-BRDA:78,0,1,NaN
-BRDA:78,0,2,NaN
-BRDA:78,0,3,NaN
+BRDA:78,0,1,1
+BRDA:78,0,2,0
+BRDA:78,0,3,0
BRDA:107,1,0,0
-BRDA:107,1,1,NaN
+BRDA:107,1,1,0
BRDA:112,2,0,0
BRDA:112,2,1,0
BRF:8
-BRH:1
+BRH:2
end_of_record The commands I am running (package.json) are:
|
@peterharding interesting, that are you (or are you @CamelKing) able to share the project having issues? |
@peterharding @CamelKing I find it interesting that typescript node register is used in both cases, this might be a good starting point for the investigation. |
@CamelKing @peterharding I usually avoid using the EDIT: TS tutorial here: https://istanbul.js.org/docs/tutorials/typescript/ |
@bcoe I have tried mocha and _mocha, with or without nodemon, or npm watch. Same. Initially I thought may be I have a cyclical requires amongst my files, spent some time to clear them, still no change. As the project gain more files, it seem the chances of getting the 'nothing to mapped' problem is higher. I now have to run multiple times before I managed to get a clean report. My repo is at https://github.com/CamelKing/alpha. The next thing I am gonna try is to pre-transpile all ts codes into js files and run the test without ts-node. |
@JaKXz thanks for the suggestion, I just tried the guide, but having the same problem. |
@CamelKing in my PR I have gotten coverage working and added some of my opinions about package publishing while trying to maintain compatibility with your workflow. :) let me know what you think you'd like to see added or changed but hopefully this has streamlined and simplified things for you. There are some important things to note:
Couple things I learned:
|
@JaKXz , thanks for the PR, it sure helps clean up the tasks quite a fair bit. I have merged it into the master. However, I have been testing it, and it still does produce the long list of 'nothing could be mapped' messages. Again this is not consistent, as in if I will get different results for every I have turned the npm loglevel to silly. The following is what I see. If I get the 'nothing to be mapped' messages:
On cases where it went thru successfully:
Both the above having same source code, same config. |
FWIW, I would try turning it off and on again:
Also node v7.7.4 is out which may contain fixes. Otherwise, honestly I'm not sure why there are intermittent failures... sorry: it works on my machine so I'm in the dark and can only suggest rudimentary steps now. |
@JaKXz I did two tests today.
For now, option 2 above will work for me, although it is a bit messy. It seems the problem is probably with ts-node, or the code interacting with ts-node? I made a tag in my repo, nyc_issue, in case any of you guys wanna take a look at the situation and may be even figure out a fix :) Btw, I am using an iMac 27" (late 2012 model), macOS Sierra 10.12.3. Not sure how much difference that would make. Once again, thanks for your help. @bcoe @peterharding too. Cheers. |
I am experiencing the same issue with the following: Dependencies:
Env:
npm scripts:
.nycrc
In our case the problem got solved downgrading to nyc v9.1.0, consistent 100% of the times. The project source is a single TypeScript file and the error comes in the branching of an if statement, even though both branches are covered by unit tests. Edit: Added npm scripts and .nycrc config |
Yes, in fact as the number of modules grow in my project (now >100), it is more likely to happen. I typically run the coverage tool a few times until I get one without the error message, and this is taking up a lot of time ... my worst fear is I am not 100% sure the coverage report is accurate ... |
@CamelKing my hunch is that we're bumping into something similar to this issue, basically I think there's a chance that a file might be getting instrumented twice. Can you provide me with a fairly minimal branch to run, that exhibits the flappy behavior? |
@CamelKing I've made some headway, it actually seems to be related to caching, and SourceMaps; will report back soon. |
@CamelKing, do me a favor and try reproducing with --cache=false; I believe the issue is that files fail to map on the second run, when source is loaded from cache. Would love help digging into this. |
@CamelKing could you please try:
I think I've hunted down the root cause of your issues (#556) but it's a somewhat frightening change, so I'd love some help QAing and validating the work. |
Hi, @bcoe sorry for the late response. I have install nyc@next and tried running it for 10 times. While the error message does not occur, I notice the followings two outcomes at random, with no change to source code:
It seems while the error message is suppressed, the impact is still there. The one is lower coverage percentage is the same as the one with error message before. I then tried running npm with silly log level, notices the followings:
running mocha on it's own with the same config would yield the followings:
So the problem only arises when calling mocha from nyc. |
@CamelKing I believe I've solved one other issue, could you please try |
@bcoe test run 10 times and
I will continue to run this for the next week or so and report back if I encountered any issue. I did try to run it with silly npm log level, and observed the followings:-
Not sure if this mean anything, but as far as I can tell, things seems running fine and I am ignoring this error message (by turning it off, LOL). |
@CamelKing I believe the error output is that you're below your coverage thresholds; which causes nyc to exit with a |
@bcoe ah ... got it. Thanks :) |
@CamelKing FYI, make sure you install the latest |
This is still happening for me in Removing |
@SneakyMax you legend, this is what i've been after for a while. |
I am experiencing this still as well with |
Same issue on nyc v12.0.2 and QUnit v2.6.1. The coverage rates and uncovered line #s vary across runs when |
Same thing here. With latest nyc as of today the same behavior is being seen. I've tried cache:false and all:true and other combinations. It's not working. |
@ralphholzmann @niedzielski @mohitkhanna Please open new issues with details including a link to an example repo showing the problem. These details will make it easier to find the specific cause of your issue so it can be dealt with. |
@coreyfarrell thanks -- my issue is here #849 with a repo that reproduces the bug |
Replace Istanbul with nyc, Istanbul's CLI. nyc appears to have some bugs that this patch works around: - When all files in the project are considered, not just those imported via tests, the coverage rates and line numbers vary between runs. This patch disables the `all` option for now and points to the bug: istanbuljs/nyc#537 (comment). - Source map line numbers appear to be incorrect except when `all` is enabled and working correctly (see previous bullet). - `sourceMap` must be disabled to avoid ENAMETOOLONG errors when nyc tries to include them as encoded strings. The patch disables the setting and points to: istanbuljs/nyc#847. Using babel-plugin-istanbul and source-map-support appears to have no effect (the former in tests/node-qunit/run.js and .babelrc). - CI fails with `Error: EACCES: permission denied, mkdir '/nonexistent'`. Specify `SPAWN_WRAP_SHIM_ROOT` instead of constructing a subdirectory from a nonexistent home directory. Bug: T196952 Bug: T193519 Change-Id: Idf2e3accd4a6277cbef91c1156fcd206c9e7d882
Running tests multiple times in a row was producing inconsistent coverage reports. About half of the time the coverage report was 50% coverage and the other half of the time the coverage report was at 75%. In searching the nyc Github repo, I found a Github issue that described this problem. The issue had a comment by one of the maintainers [1] that linked to a blog post he'd written that describes a recommended set of configuration for NYC, Mocha, and TypeScript [2]. There, I noticed that the example nycrc.json file had configuration to require 'ts-node/register', as mocha does in this repo now. He does not explain why it's required, and why it should be done in NYC instead of Mocha, but adding that to our nycrc.json fixed the inconsistent code coverage reports and they now correctly and consistently report 75%. [1]: istanbuljs/nyc#537 (comment) [2]: https://azimi.me/2016/09/30/nyc-mocha-typescript.1.html
Expected Behavior
I am expecting consistent coverage report from test to test, unless there is a code change.
Observed Behavior
Without changing any of the code, source or test, running nyc consecutively gives different coverage % in the report.
Screen shots attached in gist.
Forensic Information
Operating System: Mac OSX Sierra.
Environment Information: NodeJS, Typescript.
** output.txt. @ https://gist.github.com/CamelKing/ca471857202851d81f57e0a9473ec7fd#file-output-txt
** screen shot 1 @ https://gist.github.com/CamelKing/ca471857202851d81f57e0a9473ec7fd#file-first-run
** screen shot 2 @ https://gist.github.com/CamelKing/ca471857202851d81f57e0a9473ec7fd#file-second-run-1sec-later
The text was updated successfully, but these errors were encountered: