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: always use paths to identify files #51610

Closed
mcollina opened this issue Jan 30, 2024 · 0 comments
Closed

test_runner: always use paths to identify files #51610

mcollina opened this issue Jan 30, 2024 · 0 comments
Labels
good first issue Issues that are suitable for first-time contributors. test_runner Issues and PRs related to the test runner subsystem.

Comments

@mcollina
Copy link
Member

Currently the file property emitted by different events in the test runner can be file URLs or paths.

{"type":"test:enqueue","data":{"nesting":0,"name":"/Users/matteo/Repositories/borp/fixtures/ts-esm/dist/test/add2.test.js","line":1,"column":1,"file":"/Users/matteo/Repositories/borp/fixtures/ts-esm/dist/test/add2.test.js"}}
{"type":"test:dequeue","data":{"nesting":0,"name":"/Users/matteo/Repositories/borp/fixtures/ts-esm/dist/test/add2.test.js","line":1,"column":1,"file":"/Users/matteo/Repositories/borp/fixtures/ts-esm/dist/test/add2.test.js"}}
{"type":"test:enqueue","data":{"nesting":0,"name":"/Users/matteo/Repositories/borp/fixtures/ts-esm/dist/test/add.test.js","line":1,"column":1,"file":"/Users/matteo/Repositories/borp/fixtures/ts-esm/dist/test/add.test.js"}}
{"type":"test:enqueue","data":{"nesting":0,"name":"add2","line":4,"column":1,"file":"file:///Users/matteo/Repositories/borp/fixtures/ts-esm/dist/test/add2.test.js"}}
{"type":"test:dequeue","data":{"nesting":0,"name":"add2","line":4,"column":1,"file":"file:///Users/matteo/Repositories/borp/fixtures/ts-esm/dist/test/add2.test.js"}}
{"type":"test:start","data":{"nesting":0,"name":"add2","line":4,"column":1,"file":"file:///Users/matteo/Repositories/borp/fixtures/ts-esm/dist/test/add2.test.js"}}
{"type":"test:pass","data":{"name":"add2","nesting":0,"testNumber":1,"details":{"duration_ms":0.477958},"line":4,"column":1,"file":"file:///Users/matteo/Repositories/borp/fixtures/ts-esm/dist/test/add2.test.js"}}
{"type":"test:dequeue","data":{"nesting":0,"name":"/Users/matteo/Repositories/borp/fixtures/ts-esm/dist/test/add.test.js","line":1,"column":1,"file":"/Users/matteo/Repositories/borp/fixtures/ts-esm/dist/test/add.test.js"}}
{"type":"test:enqueue","data":{"nesting":0,"name":"add","line":4,"column":1,"file":"file:///Users/matteo/Repositories/borp/fixtures/ts-esm/dist/test/add.test.js"}}
{"type":"test:dequeue","data":{"nesting":0,"name":"add","line":4,"column":1,"file":"file:///Users/matteo/Repositories/borp/fixtures/ts-esm/dist/test/add.test.js"}}
{"type":"test:start","data":{"nesting":0,"name":"add","line":4,"column":1,"file":"file:///Users/matteo/Repositories/borp/fixtures/ts-esm/dist/test/add.test.js"}}
{"type":"test:pass","data":{"name":"add","nesting":0,"testNumber":2,"details":{"duration_ms":0.424333},"line":4,"column":1,"file":"file:///Users/matteo/Repositories/borp/fixtures/ts-esm/dist/test/add.test.js"}}

My take on those is that they should be paths and not file URLs, or in any case they should be consistent.


On a side note, the file URLs generated by this are unparsable on windows, because there is an additional /, e.g. file:///C:\\.

@MoLow MoLow added good first issue Issues that are suitable for first-time contributors. test_runner Issues and PRs related to the test runner subsystem. labels Jan 31, 2024
cjihrig added a commit to cjihrig/node that referenced this issue Mar 8, 2024
This commit transforms test locations to paths when V8 provides
file URLs (which seems to be for ESM files).

Fixes: nodejs#51610
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 <benjamingr@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
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 <benjamingr@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
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 <benjamingr@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
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 <benjamingr@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
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 <benjamingr@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
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 <benjamingr@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
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 <benjamingr@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
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 <benjamingr@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
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 <benjamingr@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
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 <benjamingr@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issues that are suitable for first-time contributors. test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants