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

nyc does not work with symlinks #1301

Closed
1 task done
aalexgabi opened this issue Apr 6, 2020 · 9 comments
Closed
1 task done

nyc does not work with symlinks #1301

aalexgabi opened this issue Apr 6, 2020 · 9 comments

Comments

@aalexgabi
Copy link

aalexgabi commented Apr 6, 2020

I reproduced the issue in this repo: aalexgabi/nyc-symlink-problem@3ce1fe8
When I don't use a symlink it does work: aalexgabi/nyc-symlink-problem@9da9355

This was reported many times (not sure all issues here are due to symlinks) but not fixed:

Expected Behavior

I should have symlinked files from node_modules included:

running
working
ran
-----------------------|---------|----------|---------|---------|-------------------
File                   | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
-----------------------|---------|----------|---------|---------|-------------------
All files              |     100 |      100 |     100 |     100 |                   
 src                   |     100 |      100 |     100 |     100 |                   
  test.js              |     100 |      100 |     100 |     100 |                   
 src/node_modules/work |     100 |      100 |     100 |     100 |                   
  index.js             |     100 |      100 |     100 |     100 |                   
-----------------------|---------|----------|---------|---------|-------------------

Observed Behavior

I don't have symlinked files from node_modules included:

running
working
ran
----------|---------|----------|---------|---------|-------------------
File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
----------|---------|----------|---------|---------|-------------------
All files |     100 |      100 |     100 |     100 |                   
 test.js  |     100 |      100 |     100 |     100 |                   
----------|---------|----------|---------|---------|-------------------

Troubleshooting steps

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

Environment Information

% npx envinfo@latest --preset nyc
npx: installed 1 in 0.741s

  System:
    OS: Linux 5.5 Manjaro Linux
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
    Memory: 887.34 MB / 15.52 GB
  Binaries:
    Node: 13.7.0 - ~/.fnm/current/bin/node
    Yarn: 1.22.0 - /usr/bin/yarn
    npm: 6.13.6 - ~/.fnm/current/bin/npm
  npmPackages:
    nyc: ^15.0.1 => 15.0.1 
@coreyfarrell
Copy link
Member

Please alter your include option, instead of node_modules/work/** you need work/**. For this you don't need excludeNodeModules: true because the realpath is not within in node_modules.

@aalexgabi
Copy link
Author

aalexgabi commented Apr 7, 2020

@coreyfarrell In my project the work folder is a library located outside the project directory as a sibling. The library is linked using npm link inside my project folder node_modules.

So this solution won't work because nyc is ran from the project folder.

So to better illustrate my structure I have updated the repo: aalexgabi/nyc-symlink-problem@5eba65d

It tried with these include patterns but it still does not work:

    "include": [
        "src/**/**",
        "../lib/**/**"
    ],

@coreyfarrell
Copy link
Member

Coverage of files outside the project isn't supported. It might work for you to tweak your settings as follows:

"cwd": "..",
"include": [
  "project/src/**",
  "lib/**"
]

The cwd says that the project root is the directory above process.cwd(), the include options are modified to be relative to the explicitly set cwd. Note that unlike other options the cwd option has slightly different meaning when provided to CLI vs when provided in config. If you ran nyc --cwd .. then this would prevent nyc from finding project/.nycrc. Thus you have to put the cwd option into project/.nycrc.

@aalexgabi
Copy link
Author

@coreyfarrell Thank you for the cwd option (I found it in the meanwhile here: https://stackoverflow.com/questions/53399098/enabling-nyc-istanbul-code-coverage-for-files-outside-the-package-directory).

It would be nice if the cwd option would be documented.

The files are inside the project folder in node_modules through a symlink. From a functional perspective the symlink should not change anything for nyc in my opinion. I get that the node uses realpath to determine if two modules are the same file but I think that nyc should abstract that away because it adds a lot of complexity in the configuration and also it forces the user to mix paths between projects otherwise.

nyc should work the same if a package is downloaded from npm or is linked locally in development. The current behavior requires manually changing the .nycrc file each time you switch from using links to downloading a module.

@coreyfarrell
Copy link
Member

nyc works with realpaths. Changing this would be a major breaking change (IE for monorepos). I'd think you can configure nyc to include both paths (the locally linked any the npm install), but be aware that instrumenting packages downloaded from npm is a non-goal of this project (thus why node_modules is ignored by default).

I appreciate that nyc is not working exactly as you would like but it is working the way it needs to work to support normal project configurations. What you are asking for may simplify what you are trying to do but for others (again monorepo projects is the major issue), including packages/** is much easier than disabling the excludeNodeModules and explicitly enabling specific node_modules/package-name paths.

@AndrewFinlay any thoughts on clarifications to the README for the cwd option? If you don't have time to look don't worry I'm tagging you as you've provided high quality documentation improvements.

@aalexgabi
Copy link
Author

Changing this would be a major breaking change (IE for monorepos)

Not necessarily. It all depends on how the change is implemented.

What you are asking for may simplify what you are trying to do but for others (again monorepo projects is the major issue), including packages/** is much easier than disabling the excludeNodeModules and explicitly enabling specific node_modules/package-name paths.

I am not asking to make it more difficult at all. I am asking that:

From a functional perspective the symlink should not change anything for nyc

It would be very useful if nyc would have support for both symlinks and realpaths because they are both valid from a functional perspective. There would be no breaking change because adding support for symlinks does not mean removing support for realpaths.

@coreyfarrell
Copy link
Member

If someone wants to create a patch for this I would review it for potential merge as long as it does not introduce any changes to existing projects (current realpath functionality must be default). I do worry about the potential for errors that using this option might cause.

I believe the process for enabling what you've requested would be:

  1. Patch test-exclude to support a useRealpath option (must be enabled by default unless explicitly set to false).
  2. Patch nyc to pass the useRealpath option from nyc to test-exclude.
  3. Patch @istanbuljs/schema to publish the useRealpath option with the other test-exclude options.

@rwalle61
Copy link

rwalle61 commented Feb 27, 2021

@coreyfarrell thanks for the cwd suggestion - it worked like a charm! Now I can test coverage across multiple packages in my monorepo (a yarn workspace):

"cwd": "..",
"include": [
  "packageA",
  "packageBRequiresPackageA"
]

Think it would be worth documenting this for people using monorepos

@stale
Copy link

stale bot commented Apr 16, 2022

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants