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

sourcesContent not correctly handled in coverage reporter with --enable-source-maps #55106

Closed
avivkeller opened this issue Sep 24, 2024 · 4 comments
Labels
coverage Issues and PRs related to native coverage support. regression Issues related to regressions. source maps Issues and PRs related to source map support. test_runner Issues and PRs related to the test runner subsystem.

Comments

@avivkeller
Copy link
Member

avivkeller commented Sep 24, 2024

Version

node-v23.0.0-nightly202409243c5ceff85f

Platform

N/A

Subsystem

test_runner

What steps will reproduce the bug?

The Node.js coverage reporter does not correctly handle source maps with a custom sourcesContent and --with-source-maps.

This example was taken from the test-runner/coverage fixture:

  1. Generate a sourcemap via:
echo "import { test } from 'node:test';
test('ok', () => {});

function uncovered() {
  return 'uncovered';
}
" | npx esbuild --sourcemap --sourcefile=stdin.test.ts --sources-content=true --bundle --platform=node --outfile="stdin.test.js"
Output
// stdin.test.ts
var import_node_test = require("node:test");
(0, import_node_test.test)("ok", () => {
});
//# sourceMappingURL=stdin.test.js.map
{
  "version": 3,
  "sources": ["stdin.test.ts"],
  "sourcesContent": ["import { test } from 'node:test';\ntest('ok', () => {});\n\nfunction uncovered() {\n  return 'uncovered';\n}\n\n"],
  "mappings": ";AAAA,uBAAqB;AAAA,IACrB,uBAAK,MAAM,MAAM;AAAC,CAAC;",
  "names": []
}
  1. Compare the output of node --test --experimental-test-coverage --enable-source-maps with node --test --experimental-test-coverage

How often does it reproduce? Is there a required condition?

Everytime

What is the expected behavior? Why is that the expected behavior?

Without --enable-source-maps:

ℹ start of coverage report
ℹ --------------------------------------------------------------
ℹ file          | line % | branch % | funcs % | uncovered lines
ℹ --------------------------------------------------------------
ℹ stdin.test.ts |  57.14 |   100.00 |  100.00 | 4-6
ℹ --------------------------------------------------------------
ℹ all files     |  57.14 |   100.00 |  100.00 |
ℹ --------------------------------------------------------------
ℹ end of coverage report

What do you see instead?

ℹ start of coverage report
ℹ ----------------------------------------------------------------
ℹ file            | line % | branch % | funcs % | uncovered lines
ℹ ----------------------------------------------------------------
ℹ stdin.test.ts   |  42.86 |   100.00 |    0.00 | 2 4-6
ℹ ----------------------------------------------------------------
ℹ all files       |  42.86 |   100.00 |    0.00 |
ℹ ----------------------------------------------------------------
ℹ end of coverage report

Additional information

No response

@avivkeller avivkeller added coverage Issues and PRs related to native coverage support. test_runner Issues and PRs related to the test runner subsystem. labels Sep 24, 2024
@avivkeller
Copy link
Member Author

In v22.9.0, there is no difference in the output with/without --enable-source-maps:

ℹ start of coverage report
ℹ --------------------------------------------------------------
ℹ file          | line % | branch % | funcs % | uncovered lines
ℹ --------------------------------------------------------------
ℹ stdin.test.ts |  57.14 |   100.00 |  100.00 | 4-6
ℹ --------------------------------------------------------------
ℹ all files     |  57.14 |   100.00 |  100.00 |
ℹ --------------------------------------------------------------
ℹ end of coverage report

@avivkeller avivkeller added source maps Issues and PRs related to source map support. regression Issues related to regressions. labels Sep 24, 2024
@avivkeller avivkeller changed the title sourcesContent not correctly handled in coverage reporter sourcesContent not correctly handled in coverage reporter with --enable-source-maps Sep 24, 2024
@geeksilva97
Copy link
Contributor

geeksilva97 commented Sep 30, 2024

On node v23.0.0-pre I got an error

When executed

./node --test --experimental-test-coverage --enable-source-maps

Error

node:internal/test_runner/test:569
      if (StringPrototypeStartsWith(this.loc.file, 'file://')) {
          ^

This entry returns an empty object instead of undefined which makes it to get into the condition at https://github.com/nodejs/node/blob/main/lib/internal/test_runner/test.js#L563.

The coverage report is displayed anyways

Full output

node:internal/test_runner/test:570
      if (StringPrototypeStartsWith(this.loc.file, 'file://')) {
          ^

TypeError: String.prototype.startsWith called on null or undefined
    at startsWith (<anonymous>)
    at new Test (node:internal/test_runner/test:570:11)
    at createTestTree (node:internal/test_runner/harness:82:16)
    at lazyBootstrapRoot (node:internal/test_runner/harness:263:5)
    at run (node:internal/test_runner/harness:301:61)
    at test (node:internal/test_runner/harness:316:12)
    at Object.<anonymous> (/Users/edysilva/projects/contributions/test-node/repro-55106/stdin.test.ts:2:1)
    at Module._compile (node:internal/modules/cjs/loader:1557:14)
    at Object..js (node:internal/modules/cjs/loader:1700:10)
    at Module.load (node:internal/modules/cjs/loader:1328:32)

Node.js v23.0.0-pre
✖ stdin.test.js (39.7005ms)
  'test failed'

ℹ tests 1
ℹ suites 0
ℹ pass 0
ℹ fail 1
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 45.501625
ℹ start of coverage report
ℹ ----------------------------------------------------------------
ℹ file            | line % | branch % | funcs % | uncovered lines
ℹ ----------------------------------------------------------------
ℹ stdin.test.ts   |  85.71 |   100.00 |    0.00 | 2
ℹ ----------------------------------------------------------------
ℹ all files       |  85.71 |   100.00 |    0.00 |
ℹ ----------------------------------------------------------------
ℹ end of coverage report

✖ failing tests:

test at stdin.test.js:1:1
✖ stdin.test.js (39.7005ms)
  'test failed'

@geeksilva97
Copy link
Contributor

Okay, I reached the expected behavior. Let me confirm.

@geeksilva97
Copy link
Contributor

I Just opened a still-drafted pr.

With source maps:

/.node --test --experimental-test-coverage --enable-source-maps

✔ ok (0.397625ms)
ℹ tests 1
ℹ suites 0
ℹ pass 1
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 53.309625
ℹ start of coverage report
ℹ ----------------------------------------------------------------
ℹ file            | line % | branch % | funcs % | uncovered lines
ℹ ----------------------------------------------------------------
ℹ stdin.test.ts   | 100.00 |   100.00 |  100.00 |
ℹ ----------------------------------------------------------------
ℹ all files       | 100.00 |   100.00 |  100.00 |
ℹ ----------------------------------------------------------------
ℹ end of coverage report

Without source maps:

./node/node --test --experimental-test-coverage
✔ ok (0.377625ms)
ℹ tests 1
ℹ suites 0
ℹ pass 1
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 54.716125
ℹ start of coverage report
ℹ ----------------------------------------------------------------
ℹ file            | line % | branch % | funcs % | uncovered lines
ℹ ----------------------------------------------------------------
ℹ stdin.test.ts   |  57.14 |   100.00 |  100.00 | 4-6
ℹ ----------------------------------------------------------------
ℹ all files       |  57.14 |   100.00 |  100.00 |
ℹ ----------------------------------------------------------------
ℹ end of coverage report

nodejs-github-bot pushed a commit that referenced this issue Oct 8, 2024
PR-URL: #55228
Refs: #55106
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
@avivkeller avivkeller reopened this Oct 8, 2024
aduh95 pushed a commit that referenced this issue Oct 9, 2024
PR-URL: #55228
Refs: #55106
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
louwers pushed a commit to louwers/node that referenced this issue Nov 2, 2024
PR-URL: nodejs#55228
Refs: nodejs#55106
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
tpoisseau pushed a commit to tpoisseau/node that referenced this issue Nov 21, 2024
PR-URL: nodejs#55228
Refs: nodejs#55106
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
coverage Issues and PRs related to native coverage support. regression Issues related to regressions. source maps Issues and PRs related to source map support. test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Development

No branches or pull requests

2 participants