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

Regression: nyc 15 creates output folder in wrong location and doesn't create HTML report at all #1234

Closed
ghost opened this issue Nov 20, 2019 · 17 comments

Comments

@ghost
Copy link

ghost commented Nov 20, 2019

Repo Steps

Execute npm test with the following configuration using nyc 15.0 (beta):

  "nyc": {
    "all": true,
    "exclude": [
      "**/index.ts",
      "**/*.d.ts",
      "**/*.js"
    ],
    "extension": [
      ".ts",
      ".tsx"
    ],
    "reporter": [
      "html"
    ],
    "report-dir": "../TypeScriptUnitTest/coverage",
    "temp-directory": "../TypeScriptUnitTest/.nyc_output"
  },
  "scripts": {
    "test": "nyc --cwd ../Application mocha -r ts-node/register -r source-map-support/register -r jsdom-global/register **/*.spec.ts"
  }

Expected Behavior

Creation of a folder TypeScriptUnitTest\.nyc_output\ with the raw nyc output and a folder TypeScriptUnitTest\coverage\ with the code coverage report.

Observed Behavior

The output folder is in the wrong location (Application\.nyc_output\) and the coverage\ folder isn't created at all (so there is no report).

Notes

This is a regression since this setup has worked for over a year without any issues using nyc versions 12.0.2 and 14.1.1.

@coreyfarrell
Copy link
Member

One thing I'm noticing is that you are using the temp-directory option which is silently deprecated. The preferred option is temp-dir. That said it is a regression that the temp-directory alias is not working so I will work on a fix for that.

If you rename the temp-directory to temp-dir does it fix your issues? If not I'm going to need a link to a repository, preferably a minimal reproduction.

@ghost
Copy link
Author

ghost commented Nov 20, 2019

Hey Corey!

Thanks for getting back to me so quickly. So, unfortunately no, changing temp-directory to temp-dir doesn't make a difference (or maybe fortunately; at least we know the alias isn't to blame). Also, something I failed to notice before, it looks like it's not measuring any coverage and ignoring the HTML reporter too because this is the summary on the console:

  1134 passing (2s)
  3 pending

----------|---------|----------|---------|---------|-------------------
File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
----------|---------|----------|---------|---------|-------------------
All files |       0 |        0 |       0 |       0 |
----------|---------|----------|---------|---------|-------------------

That said, I'm gonna create a minimal repo for you ASAP. Also, this regression aside, I love Istanbul/NYC so keep it up! :)

Thanks again!
D.C.

@coreyfarrell
Copy link
Member

nyc 15 contains breaking changes, once we can see a reproduction we can determine if the breakage is intentional or not. If the breaking change is intentional this will help determine if the changelog needs to be clearer about the change.

ghost pushed a commit to dennisdietrich/NycBug1234Repo that referenced this issue Nov 20, 2019
@ghost
Copy link
Author

ghost commented Nov 20, 2019

Done: https://github.com/dcdietrich/NycBug1234Repo/
There are two commits. The first has the package version set to 14.1.1 and works as expected. The second changed the version to 15.0.0-beta.1 which causes the issue.

@coreyfarrell
Copy link
Member

This repo seems to work for me:

> [email protected] test /usr/src/npm/@istanbuljs/istanbuljs/NycBug1234Repo/TypeScriptUnitTest
> nyc --cwd ../Application mocha -r ts-node/register -r source-map-support/register **/*.spec.ts



  Foo
    sum(number, number)
      ✓ should return the sum of two numbers


  1 passing (3ms)

----------|---------|----------|---------|---------|-------------------
File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
----------|---------|----------|---------|---------|-------------------
All files |     100 |      100 |     100 |     100 |                   
 foo.ts   |     100 |      100 |     100 |     100 |                   
 index.ts |     100 |      100 |     100 |     100 |                   
----------|---------|----------|---------|---------|-------------------

I'm running in Linux (Fedora), maybe this is a platform specific issue?

@ghost
Copy link
Author

ghost commented Nov 20, 2019

Possibly. I'm running this on Windows 10 (1803). I don't have a Linux box to try this on right now but I'll check as soon as I get a chance. However, the coverage report on the console is not supposed to be there when the HTML reporter is configured (at least that was the behavior of versions 12 and 14).

@coreyfarrell
Copy link
Member

Ah, your configuration isn't even loading in nyc 15. Moving the nyc configuration to ../Application/.nycrc.json fixes this. The problem is that --cwd is changing the basis of finding the config so the package.json#nyc is not visible.

@ghost
Copy link
Author

ghost commented Nov 20, 2019

I don't know if this helps, but I just tried this Ubuntu 18.03 and there seems to be a platform-specific aspect to it. As I mentioned, when I run this on Windows I get no coverage at all. When I run this on Ubuntu I do see the report you were seeing on Fedora. But from your last comment it sounds more like the root cause is not platform-specific.

@coreyfarrell
Copy link
Member

@dcdietrich Please do another test on Windows for me. Add "cwd": "../Application" to your package.json#nyc settings. Then remove --cwd ../Application from the command-line.

Once you do this I think your test should work (it did locally for me but again that's on Linux). The problem is that --cwd on the command-line effects where nyc starts looking for configuration, so --cwd ../Application prevents nyc 15 from finding ../TypeScriptUnitTest/package.json.

@ghost
Copy link
Author

ghost commented Nov 22, 2019

@coreyfarrell, rather mixed results, I'm afraid. So the good news is that with the change the configuration gets picked up correctly again and the console output is what I expect:

> [email protected] test C:\Users\[accountname]\repos\NycBug1234Repo\TypeScriptUnitTest
> nyc mocha -r ts-node/register -r source-map-support/register **/*.spec.ts

  Foo
    sum(number, number)
      √ should return the sum of two numbers

  1 passing (7ms)

However, the bad news is that we definitely have a platform-specific regression on Windows because I'm getting an empty coverage report.

image

@coreyfarrell
Copy link
Member

I've found the issue, we need to eagerly resolve the path to cwd. I'm working on a fix for @istanbuljs/load-nyc-config, I'll let you know when I have an update ready for you to test.

@coreyfarrell
Copy link
Member

@dcdietrich Could you please run npm install git://github.com/coreyfarrell/load-nyc-config#resolve-cwd and test with that? After this install npm ls @istanbuljs/load-nyc-config should show only one copy (from git).

Note that the change in nyc --cwd behavior is not being considered a regression, it's a breaking change. So in your setup the cwd option will still need to be in your package.json and not on your command-line.

@ghost
Copy link
Author

ghost commented Nov 23, 2019

Sorry, still broken:

C:\Users\Dennis Dietrich\Repos\NycBug1234Repo\TypeScriptUnitTest>npm install git://github.com/coreyfarrell/load-nyc-config#resolve-cwd
+ @istanbuljs/[email protected]
updated 1 package in 8.552s

C:\Users\Dennis Dietrich\Repos\NycBug1234Repo\TypeScriptUnitTest>npm test

> [email protected] test C:\Users\Dennis Dietrich\Repos\NycBug1234Repo\TypeScriptUnitTest
> nyc mocha -r ts-node/register -r source-map-support/register **/*.spec.ts



  Foo
    sum(number, number)
      √ should return the sum of two numbers


  1 passing (19ms)

image

@coreyfarrell
Copy link
Member

coreyfarrell commented Nov 23, 2019

Can you commit / push your updated package.json / package-lock.json to github? I have a MiniPC that runs Windows I'm going to give this a try but it's very cumbersome for me to work from Windows so I'd appreciate your help.

Edit: please comment here when you've pushed to your repo otherwise I don't see it.

@ghost
Copy link
Author

ghost commented Nov 23, 2019

Done. Happy to help in any way I can so please let me know if there's anything else I can do to make this easier for you to debug.

@coreyfarrell
Copy link
Member

I've published @istanbuljs/load-nyc-config 1.0.0-alpha.2 with the fix. You should be able to revert the last commit on your demo repository, delete package-lock.json and node_modules then reinstall.

@ghost
Copy link
Author

ghost commented Nov 24, 2019

That looks much better! :)

image

image

This issue was closed.
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

No branches or pull requests

1 participant