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

Test runner location output not source-mapped #51392

Closed
malthe opened this issue Jan 6, 2024 · 3 comments
Closed

Test runner location output not source-mapped #51392

malthe opened this issue Jan 6, 2024 · 3 comments
Labels
test_runner Issues and PRs related to the test runner subsystem.

Comments

@malthe
Copy link
Contributor

malthe commented Jan 6, 2024

Version

v21.3.0

Platform

Darwin wavel.local 23.2.0 Darwin Kernel Version 23.2.0: Wed Nov 15 21:53:34 PST 2023; root:xnu-10002.61.3~2/RELEASE_ARM64_T8103 arm64

Subsystem

node:test

What steps will reproduce the bug?

I have checked in a simple example at
https://github.com/malthe/node/tree/source-map-test-issue.

Clone the repo and run the following commands.

$ npm i -D ts-node
$ node --loader ts-node/esm --enable-source-maps test.mts

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

No response

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

In the first line in the output, starting with "test at", notice that the location is being reported as "3:1".

But there is nothing on this line, it's empty. It should be "4:1". The "test at" location is not mapped, even though --enable-source-maps is given.

In the stack trace provided for the assertion failure, locations are correct (mapped).

Note that if this option is not used at all, then both locations are wrong (i.e., unmapped).

What do you see instead?

...
✖ failing tests:

test at file:/.../test.mts:3:1
✖ hello (1.201292ms)
  AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
  
  1 !== 2
  
      at TestContext.<anonymous> (file:///.../test.mts:5:10)
      at Test.runInAsyncScope (node:async_hooks:206:9)
      at Test.run (node:internal/test_runner/test:631:25)
      at Test.start (node:internal/test_runner/test:542:17)
      at startSubtest (node:internal/test_runner/harness:216:17) {
    generatedMessage: true,
    code: 'ERR_ASSERTION',
    actual: 1,
    expected: 2,
    operator: 'strictEqual'
  }

Additional information

No response

@MoLow
Copy link
Member

MoLow commented Jan 6, 2024

My guess is this is not diredtly related to the test runner, do you have a minimal reproduction?

@malthe
Copy link
Contributor Author

malthe commented Jan 6, 2024

@MoLow that repo/branch linked ^^^ is an attempt at a minimal reproduction, but I don't know enough about how it works to take ts-node out of the equation.

Note that it is somewhat related to the test runner at least in the sense that it's the test runner output specifically that lacks correct location information.

@mertcanaltin mertcanaltin added the test_runner Issues and PRs related to the test runner subsystem. label Jan 14, 2024
cjihrig added a commit to cjihrig/node that referenced this issue Mar 8, 2024
This commit adds support for source mapping test locations
when the --enable-source-maps flag is present.

Fixes: nodejs#51392
nodejs-github-bot pushed a commit that referenced this issue Mar 11, 2024
This commit transforms test locations to paths when V8 provides
file URLs (which seems to be for ESM files).

Fixes: #51610
PR-URL: #52010
Fixes: #51392
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
rdw-msft pushed a commit to rdw-msft/node that referenced this issue Mar 26, 2024
This commit adds support for source mapping test locations
when the --enable-source-maps flag is present.

Fixes: nodejs#51392
PR-URL: nodejs#52010
Fixes: nodejs#51610
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
rdw-msft pushed a commit to rdw-msft/node that referenced this issue Mar 26, 2024
This commit transforms test locations to paths when V8 provides
file URLs (which seems to be for ESM files).

Fixes: nodejs#51610
PR-URL: nodejs#52010
Fixes: nodejs#51392
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
marco-ippolito pushed a commit that referenced this issue May 2, 2024
This commit adds support for source mapping test locations
when the --enable-source-maps flag is present.

Fixes: #51392
PR-URL: #52010
Fixes: #51610
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
MoLow pushed a commit to MoLow/node that referenced this issue May 7, 2024
This commit adds support for source mapping test locations
when the --enable-source-maps flag is present.

Fixes: nodejs#51392
PR-URL: nodejs#52010
Fixes: nodejs#51610
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
MoLow pushed a commit to MoLow/node that referenced this issue May 7, 2024
This commit transforms test locations to paths when V8 provides
file URLs (which seems to be for ESM files).

Fixes: nodejs#51610
PR-URL: nodejs#52010
Fixes: nodejs#51392
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
jcbhmr pushed a commit to jcbhmr/node that referenced this issue May 15, 2024
This commit adds support for source mapping test locations
when the --enable-source-maps flag is present.

Fixes: nodejs#51392
PR-URL: nodejs#52010
Fixes: nodejs#51610
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
jcbhmr pushed a commit to jcbhmr/node that referenced this issue May 15, 2024
This commit transforms test locations to paths when V8 provides
file URLs (which seems to be for ESM files).

Fixes: nodejs#51610
PR-URL: nodejs#52010
Fixes: nodejs#51392
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
marco-ippolito pushed a commit that referenced this issue May 17, 2024
This commit adds support for source mapping test locations
when the --enable-source-maps flag is present.

Fixes: #51392
PR-URL: #52010
Backport-PR-URL: #52872
Fixes: #51610
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
marco-ippolito pushed a commit that referenced this issue May 17, 2024
This commit transforms test locations to paths when V8 provides
file URLs (which seems to be for ESM files).

Fixes: #51610
PR-URL: #52010
Backport-PR-URL: #52872
Fixes: #51392
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
@fenying
Copy link

fenying commented Aug 2, 2024

I think the changes in v20.16.0 broke my project, because in v20.15.1 it works perfectly... :(

Now I got Warning: Could not report code coverage. TypeError: Cannot read properties of undefined (reading 'line') with command node --trace-warnings --test --experimental-test-coverage xxx.test.js when source-map is on, and no problem if source-map is off.

However I can't find a minimal reproduction of this problem yet, I will open a new issue if I found it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants