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

coverage results for typescript are non-deterministic #1148

Open
Kosta-Github opened this issue Jul 17, 2019 · 4 comments

Comments

@Kosta-Github
Copy link

commented Jul 17, 2019

Link to bug demonstration repository

demo repo: https://github.com/Kosta-Github/ts-nyc-issue

the typescript setup described in nyc-config-typescript has been followed

Expected Behavior

running one of the 2 provided test scripts several times should always result in the same (correct) output/result

Observed Behavior

Sometimes the result looks like this and the generated HTML coverage report looks OK:

-------------|----------|----------|----------|----------|-------------------|
File         |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
-------------|----------|----------|----------|----------|-------------------|
All files    |       80 |       50 |    66.67 |       80 |                   |
 func.ts     |    83.33 |       50 |      100 |    83.33 |                 8 |
 nested_1.ts |      100 |      100 |      100 |      100 |                   |
 nested_2.ts |       50 |      100 |        0 |       50 |                 2 |
-------------|----------|----------|----------|----------|-------------------|

=============================== Coverage summary ===============================
Statements   : 80% ( 8/10 )
Branches     : 50% ( 1/2 )
Functions    : 66.67% ( 2/3 )
Lines        : 80% ( 8/10 )
================================================================================

And sometimes the result looks like this:

-------------|----------|----------|----------|----------|-------------------|
File         |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
-------------|----------|----------|----------|----------|-------------------|
All files    |       40 |       50 |    66.67 |       80 |                   |
 func.ts     |       50 |       50 |      100 |      100 |                 5 |
 nested_1.ts |       50 |      100 |      100 |      100 |                   |
 nested_2.ts |        0 |      100 |        0 |        0 |                 2 |
-------------|----------|----------|----------|----------|-------------------|

=============================== Coverage summary ===============================
Statements   : 40% ( 4/10 )
Branches     : 50% ( 1/2 )
Functions    : 66.67% ( 2/3 )
Lines        : 80% ( 4/5 )
================================================================================

And in this case the generated HTML files contain this error message:

Cannot read property 'start' of undefined
TypeError: Cannot read property 'start' of undefined
    at Object.keys.forEach.stName (/Users/kosta/devel/misc/ts-nyc-issue/node_modules/istanbul-reports/lib/html/annotator.js:53:31)
    at Array.forEach ()
    at annotateStatements (/Users/kosta/devel/misc/ts-nyc-issue/node_modules/istanbul-reports/lib/html/annotator.js:49:33)
    at Object.annotateSourceCode (/Users/kosta/devel/misc/ts-nyc-issue/node_modules/istanbul-reports/lib/html/annotator.js:239:9)
    at HtmlReport.onDetail (/Users/kosta/devel/misc/ts-nyc-issue/node_modules/istanbul-reports/lib/html/index.js:265:27)
    at LcovReport.(anonymous function) [as onDetail] (/Users/kosta/devel/misc/ts-nyc-issue/node_modules/istanbul-reports/lib/lcov/index.js:23:23)
    at Visitor.(anonymous function) [as onDetail] (/Users/kosta/devel/misc/ts-nyc-issue/node_modules/istanbul-lib-report/lib/tree.js:34:30)
    at ReportNode.Node.visit (/Users/kosta/devel/misc/ts-nyc-issue/node_modules/istanbul-lib-report/lib/tree.js:114:17)
    at getChildren.forEach.child (/Users/kosta/devel/misc/ts-nyc-issue/node_modules/istanbul-lib-report/lib/tree.js:118:15)
    at Array.forEach ()

Troubleshooting steps

  • still occurring when I put cache: false in my nyc config

Environment Information

$ npx envinfo@latest --preset nyc

  System:
    OS: macOS 10.14.5
    CPU: (4) x64 Intel(R) Core(TM) i7-5557U CPU @ 3.10GHz
    Memory: 1.20 GB / 16.00 GB
  Binaries:
    Node: 10.16.0 - /usr/local/opt/node@10/bin/node
    Yarn: 1.17.3 - /usr/local/bin/yarn
    npm: 6.10.1 - /usr/local/opt/node@10/bin/npm
  npmPackages:
    nyc: ^14.1.1 => 14.1.1 
    ts-node: ^8.3.0 => 8.3.0 
    typescript: ^3.5.3 => 3.5.3 
@coreyfarrell

This comment has been minimized.

Copy link
Member

commented Jul 17, 2019

I believe to use "all": true you need to configure the require options on nyc instead of mocha. TBH I have not downloaded your repository I'm busy currently but this looks like something I've seen with babel transpilation.

CC @JaKXz my knowledge of TS is very limited but I think this might be an issue where the instrumentation for nyc --all is not being processed by ts-node/register where the instrumentation for files required by actual tests is being processed by ts-node/register, causing a mismatch.

@Kosta-Github

This comment has been minimized.

Copy link
Author

commented Jul 17, 2019

mh, actually looking into the config of nyc-config-typescript I think this is close to useless:

  • it doesn't do the require for ts-node and source-map-support
  • actually, all it effectively does is setting cache to false

IMHO, it would be better and way easier to just replace the documentation about adding this

{
    "extends": "@istanbuljs/nyc-config-typescript",
    ...
}

by describing to inject the following into the .nycrc.json instead:

{
    "require": [
        "ts-node/register",
        "source-map-support/register"
    ],
    "cache": false,
    "extension": [
        ".js",
        ".ts"
    ],
    ...
}

Adding the require option as described above seems to solve the issue about giving non-deterministic results.
However, it is still not clear to me, why it sometimes works and sometimes not without the require option defined in the nyc config?

@coreyfarrell

This comment has been minimized.

Copy link
Member

commented Jul 17, 2019

Each process records coverage to .nyc_output/${uuid}.json. I think what's happening is sometimes the coverage data generated by "all": true fails to merge properly with the coverage data generated by the tests. I suspect the merge fails when depending on the order of the all vs test coverage data is found.

Regarding the usefulness of @istanbuljs/nyc-config-typescript the current releases of nyc and test-exclude do not exclude Typescript test files by default. nyc@15 will ignore TypeScript tests and the extension list will include '.ts' by default, so for nyc@15 that preset will likely be unnecessary. It does seem like the documentation for this needs to be corrected to suggest configuring requires to nyc instead of mocha.

coreyfarrell added a commit to coreyfarrell/nyc that referenced this issue Aug 1, 2019

fix: Drop coverage produced by `nyc --all` for files that were tested
Sometimes the coverage data produced by `nyc --all` is incompatible with
the coverage data produced by actual test runs.  This is generally due
to configuration error but results in inconsistent coverage reports or
in some cases causes `nyc report` to crash.  The workaround is
implemented in istanbul-lib-coverage to drop coverage data associated
with `nyc --all` when coverage data from a test run is found.  This
commit tags the coverage data when appropriate so the coverage merge
logic knows what to do.

Fixes istanbuljs#1113, istanbuljs#1124, istanbuljs#1148

coreyfarrell added a commit that referenced this issue Aug 2, 2019

fix: Drop coverage produced by `nyc --all` for files that were tested (
…#1155)

* fix: Drop coverage produced by `nyc --all` for files that were tested

Sometimes the coverage data produced by `nyc --all` is incompatible with
the coverage data produced by actual test runs.  This is generally due
to configuration error but results in inconsistent coverage reports or
in some cases causes `nyc report` to crash.  The workaround is
implemented in istanbul-lib-coverage to drop coverage data associated
with `nyc --all` when coverage data from a test run is found.  This
commit tags the coverage data when appropriate so the coverage merge
logic knows what to do.

Fixes #1113, #1124, #1148
@Kosta-Github

This comment has been minimized.

Copy link
Author

commented Aug 5, 2019

Actually, the issues are coming back for more complex projects. Sadly, I was not able to create a small reproducible sample yet.

But as you can see in the attached screenshot below I am getting weird reports even in case of requiring ts-node/register and source-map-support/register from the nyc config...

image

And this is also independent from all: true vs. all: false setting...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.