search

istanbuljs / nyc

the Istanbul command line interface
https://istanbul.js.org/
ISC License
5.54k stars 353 forks source link

`nyc` + `esm` is broken in latest NodeJS versions #1530

Open jeremymeng opened 9 months ago

jeremymeng commented 9 months ago

nyc is not reporting code coverage results after possibly related nodejs change https://github.com/nodejs/node/commit/15bced0bde

Link to bug demonstration repository

https://github.com/jeremymeng/nyc-repro

install package then run npm run cc

Expected Behavior

(same as in nodejs 18.17.0)

  basic test
    ✓ add correctly

  1 passing (4ms)

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

Observed Behavior

a warning followed by passing test but 0% coverage

Transformation error for /home/meng/git/nyc-repro/src/index.js ; return original code
The "mod" argument must be an instance of Module. Received an instance of Module

  basic test
    ✓ add correctly

  1 passing (3ms)

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

Troubleshooting steps

I added a rimraf command to remve the cache directory

Environment Information

  System:
    OS: Linux 6.4 Arch Linux
    CPU: (8) x64 Intel(R) Core(TM) i7-4790 CPU @ 3.60GHz
    Memory: 23.40 GB / 31.11 GB
  Binaries:
    Node: 18.17.1 - ~/.nvm/versions/node/v18.17.1/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v18.17.1/bin/yarn
    npm: 9.6.7 - ~/.nvm/versions/node/v18.17.1/bin/npm
  npmPackages:
    nyc: ^15.1.0 => 15.1.0
jeremymeng commented 9 months ago

I wonder whether it's related to #1528

jeremymeng commented 9 months ago

/cc @RafaelGSS who signed off on https://github.com/nodejs/node/commit/15bced0bde. Any insights?

RafaelGSS commented 9 months ago

This change should only have effect when --experimental-policy is enabled. Are you sure reverting this commit nyc works?

/cc @bmeck

jeremymeng commented 9 months ago

@RafaelGSS nyc works fine in previous NodeJS build 18.17.0. The error is thrown by the new code added in the commit.

jeremymeng commented 9 months ago

15bced0bde seems the only related commit in 18.17.1 https://nodejs.org/en/blog/release/v18.17.1#commits

Commits [fe3abdf82e] - deps: update archs files for openssl-3.0.10+quic1 (Node.js GitHub Bot) #49036 [2c5a522d9c] - deps: upgrade openssl sources to quictls/openssl-3.0.10+quic1 (Node.js GitHub Bot) #49036 [15bced0bde] - policy: handle Module.constructor and main.extensions bypass (RafaelGSS) nodejs-private/node-private#417 [d4570fae35] - policy: disable process.binding() when enabled (Tobias Nießen) nodejs-private/node-private#460

RafaelGSS commented 9 months ago

This was a security release. I'm travelling at the moment but looks like nyc is creating Module in a non-conventional way. I will need to check.

RafaelGSS commented 9 months ago

Can you create a reproducible example? I can provide a fix very quickly.

jeremymeng commented 9 months ago

@RafaelGSS yeah I have it in the issue description https://github.com/jeremymeng/nyc-repro

jeremymeng commented 9 months ago

@RafaelGSS Any updates?

RafaelGSS commented 9 months ago

@RafaelGSS yeah I have it in the issue description https://github.com/jeremymeng/nyc-repro

Private repository

jeremymeng commented 8 months ago

Private repository

Oops sorry. never meant to make it private. Fixed now.

RafaelGSS commented 8 months ago

I looked at that now and it's not a nyc bug, but a esm one. Diff https://gist.github.com/RafaelGSS/e08193f1328bdbd18ee4d340b6fb9041

➜  nyc-repro git:(main) ✗ git remote -v
origin  git@github.com:jeremymeng/nyc-repro.git (fetch)
origin  git@github.com:jeremymeng/nyc-repro.git (push)
➜  nyc-repro git:(main) ✗ git rev-parse HEAD
48d25f8b353e483833fbf37489dcc4fae14651ef
➜  nyc-repro git:(main) ✗ curl https://gist.githubusercontent.com/RafaelGSS/e08193f1328bdbd18ee4d340b6fb9041/raw/0f00994a06d3204e229797ecb8d565a7215ab4b5/gistfile0.txt | git apply
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  1009  100  1009    0     0   5347      0 --:--:-- --:--:-- --:--:--  5513
➜  nyc-repro git:(main) ✗ node -v
v20.8.1
➜  nyc-repro git:(main) ✗ npm run cc

> nyc-repro@1.0.0 cc
> rimraf ./node_modules/.cache && nyc mocha test/**/*.spec.js

  basic test
    ✓ add correctly

  1 passing (2ms)

----------|---------|----------|---------|---------|-------------------
File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
----------|---------|----------|---------|---------|-------------------
All files |     100 |      100 |     100 |     100 |
 index.js |     100 |      100 |     100 |     100 |
----------|---------|----------|---------|---------|-------------------
➜  nyc-repro git:(main) ✗ git reset --hard
HEAD is now at 48d25f8 add readme
➜  nyc-repro git:(main) ✗ npm run cc

> nyc-repro@1.0.0 cc
> rimraf ./node_modules/.cache && nyc mocha -r esm test/**/*.spec.js

Transformation error for /Users/rafaelgss/repos/os/nyc-repro/src/index.js ; return original code
The "mod" argument must be an instance of Module. Received an instance of Module

  basic test
    ✓ add correctly

  1 passing (1ms)

----------|---------|----------|---------|---------|-------------------
File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
----------|---------|----------|---------|---------|-------------------
All files |       0 |        0 |       0 |       0 |
----------|---------|----------|---------|---------|-------------------
jeremymeng commented 8 months ago

@RafaelGSS It must have something to do with nyc and esm together, because running the test directly there's no error

I added the following command to the repro

    "test": "rimraf ./node_modules/.cache && mocha -r esm test/**/*.spec.js",
Sandy8i commented 4 months ago

I am still facing the issue with node 20. Do you have any updates, please? Or any workarounds?

jeremymeng commented 4 months ago

@Sandy8i we switched to c8 for now. It still supports .nycrc. Ideally we'd like to continue using nyc + esm

Sandy8i commented 4 months ago

@Sandy8i we switched to c8 for now. It still supports .nycrc. Ideally we'd like to continue using nyc + esm

Thank you for the quick response. I am actually struggling a little bit with c8. I am on node 20, the latest esm, and the latest c8. When I run my unit tests, I get the below error. Can you please help me identify the correct workspace to discuss the issue?

/webui/interface.js:55 response?.status === 401 && ^

SyntaxError: Invalid or unexpected token at Module._extensions..js (node:internal/modules/cjs/loader:1435:10)

Thanks again.

jeremymeng commented 4 months ago

@Sandy8i that's a known esm issue. Some people switched to use a fork that supports newer EcmaScript syntax https://github.com/standard-things/esm/issues/866#issuecomment-1683623541