GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — test_runner: expose location of tests by cjihrig · Pull Request #48975 · nodejs/node

Conversation

@cjihrig

@cjihrig cjihrig commented Jul 30, 2023

Copy link
Copy Markdown
Contributor

This commit adds each test's line and column number to the reporter output. This will aid in debugging test suite failures when error stacks are not helpful, test suites are large, or tests have the same name. This data is also exposed on the spec reporter.

This commit also replaces the filename that was previously being reported, with the filename where the test actually exists. These are normally correct, but could be wrong if tests were run from a file other than the user's entrypoint.

Fixes: #48457

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

atlowChemi, tonivj5, styfle, and mdrobny reacted with rocket emoji bricss reacted with eyes emoji
All reactions
  • 🚀 4 reactions
  • 👀 1 reaction
@cjihrig
cjihrig requested a review from mcollina July 30, 2023 23:31
@nodejs-github-bot

nodejs-github-bot commented Jul 30, 2023

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/test_runner
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Jul 30, 2023
@cjihrig cjihrig added semver-minor PRs that contain new features and should be released in the next minor version. test_runner Issues and PRs related to the test runner subsystem. labels Jul 30, 2023
anonrig
anonrig approved these changes Jul 31, 2023
View reviewed changes
MoLow
MoLow reviewed Jul 31, 2023
View reviewed changes
Comment thread lib/internal/test_runner/harness.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

MoLow
MoLow reviewed Jul 31, 2023
View reviewed changes
Comment thread lib/internal/test_runner/reporter/spec.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

MoLow
MoLow reviewed Jul 31, 2023
View reviewed changes
Comment thread lib/internal/test_runner/reporter/spec.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

mcollina
mcollina reviewed Jul 31, 2023
View reviewed changes

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing work! Could you add this information to the tap reporter as well? This happens relatively often in CI too.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
atlowChemi
atlowChemi reviewed Jul 31, 2023
View reviewed changes
Comment thread lib/internal/test_runner/test.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

@benjamingr

benjamingr commented Jul 31, 2023

Copy link
Copy Markdown
Member

This probably needs benchmark as taking a trace can be expensive, doesn't it?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@cjihrig

cjihrig commented Jul 31, 2023

Copy link
Copy Markdown
Contributor Author

This probably needs benchmark as taking a trace can be expensive, doesn't it?

I tried really hard to avoid implementing this because I was worried about that. I tried to make it as fast as possible while being accurate. It only collects two stack frames and does not iterate over them.

I think we actually need this from a correctness standpoint. The reporter emits events with filenames attached to it, which might not be correct. The DX is a secondary improvement for things like #48457 (comment).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ljharb

ljharb commented Jul 31, 2023

Copy link
Copy Markdown
Member

Usually perf isn’t that important in failure cases, which any failed tests would be. What’s the actual impact here?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@cjihrig

cjihrig commented Aug 1, 2023

Copy link
Copy Markdown
Contributor Author

The problem is that if you wait until you know that the test failed, you can't get at the original call site, so you have to do it for all tests. If you do know of a way to accurately get the original test() call site at failure time, I would love to use that instead.

There's also the issue of reporting the wrong filename if you run a test from a non-main file via, for example, require('./some-other-file-with-tests-in-it').

ljharb, benjamingr, and MoLow reacted with thumbs up emoji
All reactions
  • 👍 3 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ljharb

ljharb commented Aug 1, 2023

Copy link
Copy Markdown
Member

Presumably the expensive part is stack generation, and that’s what needs to be done eagerly :-/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@benjamingr

benjamingr commented Aug 1, 2023

Copy link
Copy Markdown
Member

I tried really hard to avoid implementing this because I was worried about that. I tried to make it as fast as possible while being accurate. It only collects two stack frames and does not iterate over them.

We would still need a benchmark or alternatively to make this opt in only if you listen to the event + a doc warning?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@cjihrig

cjihrig commented Aug 1, 2023

Copy link
Copy Markdown
Contributor Author

alternatively to make this opt in only if you listen to the event

I'm not sure how that would work. The filename is already part of the public API. Even if we limited these changes to the 'test:fail' event, every meaningful reporter will listen for that event.

MoLow reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@rluvaton

rluvaton commented Aug 1, 2023

Copy link
Copy Markdown
Member

How would this work with compilers that change the code location (e.g. TypeScript)

meyfa reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@cjihrig

cjihrig commented Aug 1, 2023

Copy link
Copy Markdown
Contributor Author

How would this work with compilers that change the code location (e.g. TypeScript)

It points to the location at runtime (same as the file, which would be the .js file, not the .ts file). Source maps aren't really supported in the test runner yet.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@cjihrig
cjihrig force-pushed the test-location branch from afa290d to c735349 Compare August 8, 2023 12:48
@cjihrig

cjihrig commented Aug 8, 2023

Copy link
Copy Markdown
Contributor Author

I've updated this PR based on feedback, including updating the TAP reporter.

Here are the benchmark numbers:

test_runner/global-concurrent-tests.js type='async' n=100                                                  ***     -2.39 %       ±1.20% ±1.61% ±2.11%
test_runner/global-concurrent-tests.js type='async' n=1000                                                 ***    -15.10 %       ±1.10% ±1.47% ±1.91%
test_runner/global-concurrent-tests.js type='async' n=10000                                                ***    -12.71 %       ±1.95% ±2.61% ±3.45%
test_runner/global-concurrent-tests.js type='sync' n=100                                                   ***     -2.82 %       ±1.26% ±1.68% ±2.18%
test_runner/global-concurrent-tests.js type='sync' n=1000                                                  ***    -14.19 %       ±0.95% ±1.27% ±1.65%
test_runner/global-concurrent-tests.js type='sync' n=10000                                                 ***    -13.48 %       ±1.05% ±1.40% ±1.84%
test_runner/global-sequential-tests.js type='async' n=100                                                   **      1.91 %       ±1.39% ±1.85% ±2.41%
test_runner/global-sequential-tests.js type='async' n=1000                                                          0.91 %       ±2.53% ±3.37% ±4.40%
test_runner/global-sequential-tests.js type='async' n=10000                                                         0.71 %       ±1.22% ±1.62% ±2.11%
test_runner/global-sequential-tests.js type='sync' n=100                                                            0.78 %       ±1.51% ±2.01% ±2.62%
test_runner/global-sequential-tests.js type='sync' n=1000                                                           0.72 %       ±2.06% ±2.74% ±3.57%
test_runner/global-sequential-tests.js type='sync' n=10000                                                         -0.11 %       ±1.20% ±1.59% ±2.07%
test_runner/suite-tests.js concurrency='no' testType='async' testsPerSuite=10 numberOfSuites=10            ***     -2.83 %       ±0.71% ±0.94% ±1.23%
test_runner/suite-tests.js concurrency='no' testType='async' testsPerSuite=10 numberOfSuites=100           ***      3.70 %       ±1.45% ±1.94% ±2.53%
test_runner/suite-tests.js concurrency='no' testType='async' testsPerSuite=100 numberOfSuites=10           ***      4.71 %       ±1.37% ±1.83% ±2.41%
test_runner/suite-tests.js concurrency='no' testType='async' testsPerSuite=100 numberOfSuites=100          ***     -4.82 %       ±0.70% ±0.94% ±1.22%
test_runner/suite-tests.js concurrency='no' testType='async' testsPerSuite=1000 numberOfSuites=10          ***     -4.63 %       ±0.77% ±1.03% ±1.35%
test_runner/suite-tests.js concurrency='no' testType='async' testsPerSuite=1000 numberOfSuites=100                 -0.32 %       ±4.42% ±5.92% ±7.79%
test_runner/suite-tests.js concurrency='no' testType='sync' testsPerSuite=10 numberOfSuites=10             ***     -3.96 %       ±0.96% ±1.28% ±1.69%
test_runner/suite-tests.js concurrency='no' testType='sync' testsPerSuite=10 numberOfSuites=100            ***      5.81 %       ±1.68% ±2.25% ±2.95%
test_runner/suite-tests.js concurrency='no' testType='sync' testsPerSuite=100 numberOfSuites=10            ***      4.96 %       ±0.81% ±1.08% ±1.41%
test_runner/suite-tests.js concurrency='no' testType='sync' testsPerSuite=100 numberOfSuites=100           ***     -5.29 %       ±0.68% ±0.91% ±1.19%
test_runner/suite-tests.js concurrency='no' testType='sync' testsPerSuite=1000 numberOfSuites=10           ***     -4.31 %       ±0.62% ±0.83% ±1.08%
test_runner/suite-tests.js concurrency='no' testType='sync' testsPerSuite=1000 numberOfSuites=100            *     -4.90 %       ±4.29% ±5.75% ±7.56%
test_runner/suite-tests.js concurrency='yes' testType='async' testsPerSuite=10 numberOfSuites=10           ***     -4.88 %       ±0.71% ±0.94% ±1.23%
test_runner/suite-tests.js concurrency='yes' testType='async' testsPerSuite=10 numberOfSuites=100          ***      7.81 %       ±2.02% ±2.69% ±3.50%
test_runner/suite-tests.js concurrency='yes' testType='async' testsPerSuite=100 numberOfSuites=10          ***      4.54 %       ±1.33% ±1.77% ±2.31%
test_runner/suite-tests.js concurrency='yes' testType='async' testsPerSuite=100 numberOfSuites=100         ***     -4.98 %       ±0.63% ±0.84% ±1.09%
test_runner/suite-tests.js concurrency='yes' testType='async' testsPerSuite=1000 numberOfSuites=10         ***     -5.33 %       ±0.68% ±0.91% ±1.18%
test_runner/suite-tests.js concurrency='yes' testType='async' testsPerSuite=1000 numberOfSuites=100        ***     -5.40 %       ±2.69% ±3.62% ±4.78%
test_runner/suite-tests.js concurrency='yes' testType='sync' testsPerSuite=10 numberOfSuites=10            ***     -5.96 %       ±0.73% ±0.98% ±1.27%
test_runner/suite-tests.js concurrency='yes' testType='sync' testsPerSuite=10 numberOfSuites=100           ***      7.47 %       ±1.78% ±2.38% ±3.13%
test_runner/suite-tests.js concurrency='yes' testType='sync' testsPerSuite=100 numberOfSuites=10           ***      5.63 %       ±0.86% ±1.14% ±1.49%
test_runner/suite-tests.js concurrency='yes' testType='sync' testsPerSuite=100 numberOfSuites=100          ***     -5.14 %       ±0.70% ±0.94% ±1.24%
test_runner/suite-tests.js concurrency='yes' testType='sync' testsPerSuite=1000 numberOfSuites=10          ***     -5.06 %       ±0.67% ±0.89% ±1.17%
test_runner/suite-tests.js concurrency='yes' testType='sync' testsPerSuite=1000 numberOfSuites=100         ***     -8.32 %       ±2.39% ±3.19% ±4.18%

I'm on the fence because, as expected, there is a performance hit. On the other hand, as previously mentioned, this improves correctness and provides important debugging information. 🤷

benjamingr and MoLow reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mcollina

mcollina commented Aug 8, 2023

Copy link
Copy Markdown
Member

The alternative solution to this issue is to restore printing the file name when more than one file is being run (wrapping it in a suite automatically). It's a less accurate, but at least we can point out where things are broken.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@cjihrig

cjihrig commented Aug 8, 2023

Copy link
Copy Markdown
Contributor Author

The more I think about this, the more I think we should land this change.

I'm not entirely confident in the benchmarks. For example, why does global-concurrent-tests.js show a bigger perf hit when n=1,000 than when n=10,000? At the same time, running tests sequentially shows a small improvement (although not statistically significant) even though each test is doing more work. Similarly, there are a number of suite-tests.js runs that show over a 5% improvement.

atlowChemi and MoLow reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mcollina
mcollina approved these changes Aug 8, 2023
View reviewed changes

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@rluvaton

rluvaton commented Aug 8, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

The more I think about this, the more I think we should land this change.

I'm not entirely confident in the benchmarks. For example, why does global-concurrent-tests.js show a bigger perf hit when n=1,000 than when n=10,000? At the same time, running tests sequentially shows a small improvement (although not statistically significant) even though each test is doing more work. Similarly, there are a number of suite-tests.js runs that show over a 5% improvement.

If you think the benchmark would need an improvement I would love to see that... I added those as we did not have anything so we had no data to make a decision

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@cjihrig

cjihrig commented Aug 8, 2023

Copy link
Copy Markdown
Contributor Author

Having the benchmarks is a great improvement. Thank you for that. Unfortunately, I don't have any suggestions for improving them (other than I'm not sure global-concurrent-tests.js makes a ton of sense because the test runner has internal concurrency machinery that I don't think that benchmark accounts for). It's good to see numbers and how changes impact them... I'm just having trouble reasoning about some of the results in the context of this change.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@rluvaton

rluvaton commented Aug 8, 2023

Copy link
Copy Markdown
Member

Yagiz suggested to improve the benchmark reliability we should extract to test creation an test running but I don't think that's possible but it might give you some idea 😄

cjihrig reacted with rocket emoji
All reactions
  • 🚀 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

atlowChemi
atlowChemi approved these changes Aug 9, 2023
View reviewed changes
UlisesGascon added a commit that referenced this pull request Aug 22, 2023
@UlisesGascon
2023-08-23, Version 20.6.0 (Current)
4148741
Notable changes:

deps:
  * V8: cherry-pick 93275031284c (Joyee Cheung) #48660
doc:
  * add new TSC members (Michael Dawson) #48841
  * add rluvaton to collaborators (Raz Luvaton) #49215
esm:
  * unflag import.meta.resolve (Guy Bedford) #49028
  * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842
  * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559
inspector:
  * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765
module:
  * implement `register` utility (João Lenon) #46826
  * make CJS load from ESM loader (Antoine du Hamel) #47999
src:
  * add built-in `.env` file support (Yagiz Nizipli) #48890
  * initialize cppgc (Daryl Haresign and Joyee Cheung) #48660 and #45704
test_runner:
  * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975

PR-URL: #49185
UlisesGascon added a commit that referenced this pull request Aug 24, 2023
@UlisesGascon
2023-08-28, Version 20.6.0 (Current)
df92c38
Notable changes:

deps:
  * V8: cherry-pick 93275031284c (Joyee Cheung) #48660
doc:
  * add new TSC members (Michael Dawson) #48841
  * add rluvaton to collaborators (Raz Luvaton) #49215
esm:
  * unflag import.meta.resolve (Guy Bedford) #49028
  * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842
  * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559
inspector:
  * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765
module:
  * implement `register` utility (João Lenon) #46826
  * make CJS load from ESM loader (Antoine du Hamel) #47999
src:
  * add built-in `.env` file support (Yagiz Nizipli) #48890
  * initialize cppgc (Daryl Haresign and Joyee Cheung) #48660 and #45704
test_runner:
  * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975

PR-URL: #49185
UlisesGascon added a commit that referenced this pull request Aug 29, 2023
@UlisesGascon
2023-08-30, Version 20.6.0 (Current)
4e4bb9f
Notable changes:

deps:
  * V8: cherry-pick 93275031284c (Joyee Cheung) #48660
doc:
  * add new TSC members (Michael Dawson) #48841
  * add rluvaton to collaborators (Raz Luvaton) #49215
esm:
  * unflag import.meta.resolve (Guy Bedford) #49028
  * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842
  * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559
inspector:
  * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765
module:
  * implement `register` utility (João Lenon) #46826
  * make CJS load from ESM loader (Antoine du Hamel) #47999
src:
  * add built-in `.env` file support (Yagiz Nizipli) #48890
  * initialize cppgc (Daryl Haresign and Joyee Cheung) #48660 and #45704
test_runner:
  * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975

PR-URL: #49185
UlisesGascon added a commit that referenced this pull request Aug 31, 2023
@UlisesGascon
2023-09-04, Version 20.6.0 (Current)
da933e9
Notable changes:

deps:
  * V8: cherry-pick 93275031284c (Joyee Cheung) #48660
doc:
  * add new TSC members (Michael Dawson) #48841
  * add rluvaton to collaborators (Raz Luvaton) #49215
esm:
  * unflag import.meta.resolve (Guy Bedford) #49028
  * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842
  * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559
inspector:
  * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765
module:
  * implement `register` utility (João Lenon) #46826
  * make CJS load from ESM loader (Antoine du Hamel) #47999
src:
  * add built-in `.env` file support (Yagiz Nizipli) #48890
  * initialize cppgc (Daryl Haresign and Joyee Cheung) #48660 and #45704
test_runner:
  * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975

PR-URL: #49185
UlisesGascon added a commit that referenced this pull request Sep 1, 2023
@UlisesGascon
2023-09-04, Version 20.6.0 (Current)
9700344
Notable changes:

deps:
  * V8: cherry-pick 93275031284c (Joyee Cheung) #48660
doc:
  * add new TSC members (Michael Dawson) #48841
  * add rluvaton to collaborators (Raz Luvaton) #49215
esm:
  * unflag import.meta.resolve (Guy Bedford) #49028
  * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842
  * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559
inspector:
  * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765
module:
  * implement `register` utility (João Lenon) #46826
  * make CJS load from ESM loader (Antoine du Hamel) #47999
src:
  * add built-in `.env` file support (Yagiz Nizipli) #48890
  * initialize cppgc (Daryl Haresign and Joyee Cheung) #48660 and #45704
test_runner:
  * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975

PR-URL: #49185
rluvaton pushed a commit to rluvaton/node that referenced this pull request Sep 4, 2023
@cjihrig @rluvaton
test_runner: expose location of tests
7c011b6
This commit adds each test's line and column number to the reporter
output. This will aid in debugging test suite failures when error
stacks are not helpful, test suites are large, or tests have the
same name. This data is also exposed on the spec reporter.

This commit also replaces the filename that was previously being
reported, with the filename where the test actually exists. These
are normally correct, but could be wrong if tests were run from
a file other than the user's entrypoint.

PR-URL: nodejs#48975
Fixes: nodejs#48457
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
juanarbol pushed a commit that referenced this pull request Sep 4, 2023
@UlisesGascon @juanarbol
2023-09-04, Version 20.6.0 (Current)
1235426
Notable changes:

deps:
  * V8: cherry-pick 93275031284c (Joyee Cheung) #48660
doc:
  * add new TSC members (Michael Dawson) #48841
  * add rluvaton to collaborators (Raz Luvaton) #49215
esm:
  * unflag import.meta.resolve (Guy Bedford) #49028
  * add `initialize` hook, integrate with `register` (Izaak Schroeder) #48842
  * unflag `Module.register` and allow nested loader `import()` (Izaak Schroeder) #48559
inspector:
  * (SEMVER-MINOR) open add `SymbolDispose` (Chemi Atlow) #48765
module:
  * implement `register` utility (João Lenon) #46826
  * make CJS load from ESM loader (Antoine du Hamel) #47999
src:
  * add built-in `.env` file support (Yagiz Nizipli) #48890
  * initialize cppgc (Daryl Haresign and Joyee Cheung) #48660 and #45704
test_runner:
  * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975

PR-URL: #49185
UlisesGascon pushed a commit that referenced this pull request Sep 10, 2023
@cjihrig @UlisesGascon
test_runner: expose location of tests
efdc95f
This commit adds each test's line and column number to the reporter
output. This will aid in debugging test suite failures when error
stacks are not helpful, test suites are large, or tests have the
same name. This data is also exposed on the spec reporter.

This commit also replaces the filename that was previously being
reported, with the filename where the test actually exists. These
are normally correct, but could be wrong if tests were run from
a file other than the user's entrypoint.

PR-URL: #48975
Backport-PR-URL: #49225
Fixes: #48457
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
@UlisesGascon UlisesGascon mentioned this pull request Sep 10, 2023
v20.7.0 proposal #49592
Merged
UlisesGascon added a commit that referenced this pull request Sep 10, 2023
@UlisesGascon
2023-09-13, Version 20.7.0 (Current)
9fdf657
Notable changes:

crypto:
  * update root certificates to NSS 3.93 (Node.js GitHub Bot) #49341
doc:
  * move and rename loaders section (Geoffrey Booth) #49261
lib:
  * (SEMVER-MINOR) add api to detect whether source-maps are enabled (翠 / green) #46391
src,permission:
  * add multiple allow-fs-* flags (Carlos Espa) #49047
test_runner:
  * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975

PR-URL: #49592
UlisesGascon added a commit that referenced this pull request Sep 11, 2023
@UlisesGascon
2023-09-13, Version 20.7.0 (Current)
837d013
Notable changes:

crypto:
  * update root certificates to NSS 3.93 (Node.js GitHub Bot) #49341
doc:
  * move and rename loaders section (Geoffrey Booth) #49261
lib:
  * (SEMVER-MINOR) add api to detect whether source-maps are enabled (翠 / green) #46391
src,permission:
  * add multiple allow-fs-* flags (Carlos Espa) #49047
test_runner:
  * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975

PR-URL: #49592
UlisesGascon added a commit that referenced this pull request Sep 11, 2023
@UlisesGascon
2023-09-13, Version 20.7.0 (Current)
dcc3c41
Notable changes:

crypto:
  * update root certificates to NSS 3.93 (Node.js GitHub Bot) #49341
doc:
  * move and rename loaders section (Geoffrey Booth) #49261
lib:
  * (SEMVER-MINOR) add api to detect whether source-maps are enabled (翠 / green) #46391
src:
  * support multiple `--env-file` declarations (Yagiz Nizipli) #49542
src,permission:
  * add multiple allow-fs-* flags (Carlos Espa) #49047
test_runner:
  * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975

PR-URL: #49592
UlisesGascon added a commit that referenced this pull request Sep 12, 2023
@UlisesGascon
2023-09-13, Version 20.7.0 (Current)
9f950df
Notable changes:

crypto:
  * update root certificates to NSS 3.93 (Node.js GitHub Bot) #49341
doc:
  * move and rename loaders section (Geoffrey Booth) #49261
  * add release key for Ulises Gascon (Ulises Gascón) #49196
lib:
  * (SEMVER-MINOR) add api to detect whether source-maps are enabled (翠 / green) #46391
src:
  * support multiple `--env-file` declarations (Yagiz Nizipli) #49542
src,permission:
  * add multiple allow-fs-* flags (Carlos Espa) #49047
test_runner:
  * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975

PR-URL: #49592
UlisesGascon added a commit that referenced this pull request Sep 13, 2023
@UlisesGascon
2023-09-18, Version 20.7.0 (Current)
d9e2456
Notable changes:

crypto:
  * update root certificates to NSS 3.93 (Node.js GitHub Bot) #49341
doc:
  * move and rename loaders section (Geoffrey Booth) #49261
  * add release key for Ulises Gascon (Ulises Gascón) #49196
lib:
  * (SEMVER-MINOR) add api to detect whether source-maps are enabled (翠 / green) #46391
src:
  * support multiple `--env-file` declarations (Yagiz Nizipli) #49542
src,permission:
  * add multiple allow-fs-* flags (Carlos Espa) #49047
test_runner:
  * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975

PR-URL: #49592
UlisesGascon added a commit that referenced this pull request Sep 16, 2023
@UlisesGascon
2023-09-18, Version 20.7.0 (Current)
3557c43
Notable changes:

crypto:
  * update root certificates to NSS 3.93 (Node.js GitHub Bot) #49341
doc:
  * move and rename loaders section (Geoffrey Booth) #49261
  * add release key for Ulises Gascon (Ulises Gascón) #49196
lib:
  * (SEMVER-MINOR) add api to detect whether source-maps are enabled (翠 / green) #46391
src:
  * support multiple `--env-file` declarations (Yagiz Nizipli) #49542
src,permission:
  * add multiple allow-fs-* flags (Carlos Espa) #49047
test_runner:
  * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975

PR-URL: #49592
UlisesGascon added a commit that referenced this pull request Sep 18, 2023
@UlisesGascon
2023-09-18, Version 20.7.0 (Current)
a71037e
Notable changes:

crypto:
  * update root certificates to NSS 3.93 (Node.js GitHub Bot) #49341
deps:
  * upgrade npm to 10.1.0 (npm team) #49570
  * upgrade npm to 10.0.0 (npm team) #49423
doc:
  * move and rename loaders section (Geoffrey Booth) #49261
  * add release key for Ulises Gascon (Ulises Gascón) #49196
lib:
  * (SEMVER-MINOR) add api to detect whether source-maps are enabled (翠 / green) #46391
src:
  * support multiple `--env-file` declarations (Yagiz Nizipli) #49542
src,permission:
  * add multiple allow-fs-* flags (Carlos Espa) #49047
test_runner:
  * (SEMVER-MINOR) expose location of tests (Colin Ihrig) #48975

PR-URL: #49592
UlisesGascon added a commit to UlisesGascon/node that referenced this pull request Sep 18, 2023
@UlisesGascon
2023-09-18, Version 20.7.0 (Current)
b651e37
Notable changes:

crypto:
  * update root certificates to NSS 3.93 (Node.js GitHub Bot) nodejs#49341
deps:
  * upgrade npm to 10.1.0 (npm team) nodejs#49570
  * upgrade npm to 10.0.0 (npm team) nodejs#49423
doc:
  * move and rename loaders section (Geoffrey Booth) nodejs#49261
  * add release key for Ulises Gascon (Ulises Gascón) nodejs#49196
lib:
  * (SEMVER-MINOR) add api to detect whether source-maps are enabled (翠 / green) nodejs#46391
src:
  * support multiple `--env-file` declarations (Yagiz Nizipli) nodejs#49542
src,permission:
  * add multiple allow-fs-* flags (Carlos Espa) nodejs#49047
test_runner:
  * (SEMVER-MINOR) expose location of tests (Colin Ihrig) nodejs#48975

PR-URL: nodejs#49592
@targos targos added backported-to-v20.x and removed backport-open-v20.x labels Nov 12, 2023
targos pushed a commit that referenced this pull request Nov 27, 2023
@cjihrig @targos
test_runner: expose location of tests
d61a505
This commit adds each test's line and column number to the reporter
output. This will aid in debugging test suite failures when error
stacks are not helpful, test suites are large, or tests have the
same name. This data is also exposed on the spec reporter.

This commit also replaces the filename that was previously being
reported, with the filename where the test actually exists. These
are normally correct, but could be wrong if tests were run from
a file other than the user's entrypoint.

PR-URL: #48975
Fixes: #48457
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
@targos targos mentioned this pull request Nov 28, 2023
v18.19.0 release proposal #50953
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ljharb ljharb ljharb left review comments
@MoLow MoLow MoLow approved these changes
@mcollina mcollina mcollina approved these changes
@anonrig anonrig anonrig approved these changes
@atlowChemi atlowChemi atlowChemi approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. semver-minor PRs that contain new features and should be released in the next minor version. test_runner Issues and PRs related to the test runner subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

test runner to include the name of the file being run

Uh oh!

There was an error while loading. Please reload this page.

11 participants

@cjihrig @nodejs-github-bot @benjamingr @ljharb @rluvaton @mcollina @RafaelGSS @anonrig @MoLow @atlowChemi @targos
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.