test_runner: add 'test:summary' event #54851

cjihrig · 2024-09-08T20:27:10Z

This commit adds a new 'test:summary' event to the test runner's reporting interface. This new event serves two purposes:

In the future, the test runner internals will no longer need to change the process exit code. This may be important to run() users. Unfortunately, this is a breaking change, so it needs to be changed in a major version.
The reporting interface now has a single event that can identify passing or failing test runs.

Refs: #53867
Refs: #54812

~~(Still need to add docs)~~

nodejs-github-bot · 2024-09-08T20:27:15Z

Review requested:

@nodejs/test_runner

codecov · 2024-09-08T21:56:47Z

Codecov Report

Attention: Patch coverage is 88.23529% with 4 lines in your changes missing coverage. Please review.

Project coverage is 88.02%. Comparing base (e35299a) to head (81e000d).
Report is 28 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/test_runner/harness.js	72.72%	3 Missing ⚠️
lib/internal/test_runner/test.js	87.50%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #54851      +/-   ##
==========================================
- Coverage   88.03%   88.02%   -0.01%     
==========================================
  Files         652      652              
  Lines      183797   183822      +25     
  Branches    35863    35868       +5     
==========================================
+ Hits       161804   161813       +9     
- Misses      15242    15259      +17     
+ Partials     6751     6750       -1

Files with missing lines	Coverage Δ
lib/internal/main/test_runner.js	`100.00% <100.00%> (ø)`
lib/internal/test_runner/tests_stream.js	`91.61% <100.00%> (+0.53%)`	⬆️
lib/internal/test_runner/utils.js	`55.88% <100.00%> (+0.14%)`	⬆️
lib/internal/test_runner/test.js	`96.93% <87.50%> (-0.07%)`	⬇️
lib/internal/test_runner/harness.js	`92.65% <72.72%> (-0.72%)`	⬇️

... and 22 files with indirect coverage changes

lib/internal/test_runner/harness.js

lib/internal/test_runner/tests_stream.js

RedYetiDev · 2024-09-14T00:01:42Z

(Still need to add docs)

I've added the wip label, as you stated that you still need to add the documentation for this PR

atlowChemi · 2024-09-14T19:25:49Z

Unfortunately, this is a breaking change, so it needs to be changed in a major version.

Added semver-major PRs that contain breaking changes and should be released in the next major version.

cjihrig · 2024-09-14T19:27:25Z

Unfortunately, this is a breaking change, so it needs to be changed in a major version.

The original implementation that changed the process exit code was semver major. I removed that change, so this is only semver minor now

nodejs-github-bot · 2024-09-14T20:32:25Z

CI: https://ci.nodejs.org/job/node-test-pull-request/62438/

mcollina · 2024-09-15T10:27:21Z

Can we make it non-breaking by detecting if there is a summary listener and if not reverting to the old behavior? I think having this as a major would significantly impact the ability to backport certain fixes to v20 and v22.

cjihrig · 2024-09-15T13:09:41Z

Can we make it non-breaking by detecting if there is a summary listener and if not reverting to the old behavior?

This change is no longer breaking. Do you mean the setting of the exit code? Yes, we could do that - I think I'd prefer to do that in a separate PR though.

mcollina · 2024-09-15T14:14:38Z

For whatever reason I didn't see the last couple of messages, nevermind!

mcollina

lgtm

RedYetiDev · 2024-09-20T16:41:56Z

Can you remove the logic for #53937's handling of the exit code for thresholds, as it's no longer needed with this, right?

Notably the documentation about exit codes here, here, and here.

cjihrig · 2024-09-20T16:48:25Z

No. run() will continue to set the process exit code even though it's not needed with this event. Changing run() to not change the exit code is a breaking change that we shouldn't make until all supported release lines have access to this event.

cjihrig · 2024-09-20T16:49:04Z

This needs a re-approval in order to land. The CI is good.

RedYetiDev · 2024-09-20T16:55:32Z

No. run() will continue to set the process exit code even though it's not needed with this event. Changing run() to not change the exit code is a breaking change that we shouldn't make until all supported release lines have access to this event.

Oh okay, I thought that that didn't happen with this change. My bad. I'll probably open a followup (breaking?) PR to change that after this.

cjihrig · 2024-09-21T16:49:24Z

Ping @MoLow @atlowChemi @mcollina

nodejs-github-bot · 2024-09-21T17:41:54Z

Landed in 059e08b

This commit adds a new 'test:summary' event to the test runner's reporting interface. This new event serves two purposes: - In the future, the test runner internals will no longer need to change the process exit code. This may be important to run() users. Unfortunately, this is a breaking change, so it needs to be changed in a major version. - The reporting interface now has a single event that can identify passing or failing test runs. Refs: #53867 Refs: #54812 PR-URL: #54851 Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Notable changes: crypto: * (SEMVER-MINOR) add `KeyObject.prototype.toCryptoKey` (Filip Skokan) #55262 * (SEMVER-MINOR) add Date fields for `validTo` and `validFrom` (Andrew Moon) #54159 doc: * add abmusse to collaborators (Abdirahim Musse) #55086 http2: * (SEMVER-MINOR) expose `nghttp2_option_set_stream_reset_rate_limit` as an option (Maël Nison) #54875 lib: * (SEMVER-MINOR) propagate aborted state to dependent signals before firing events (jazelly) #54826 module: * (SEMVER-MINOR) support loading entrypoint as url (RedYetiDev) #54933 * (SEMVER-MINOR) implement the `"module-sync"` exports condition (Joyee Cheung) #54648 * (SEMVER-MINOR) implement `flushCompileCache()` (Joyee Cheung) #54971 * (SEMVER-MINOR) throw when invalid argument is passed to `enableCompileCache()` (Joyee Cheung) #54971 * (SEMVER-MINOR) write compile cache to temporary file and then rename it (Joyee Cheung) #54971 src: * mark node `--run` as stable (Yagiz Nizipli) #53763 test_runner: * (SEMVER-MINOR) support custom arguments in `run()` (Aviv Keller) #55126 * (SEMVER-MINOR) add `'test:summary'` event (Colin Ihrig) #54851 * (SEMVER-MINOR) add support for coverage via `run()` (Chemi Atlow) #53937 worker: * (SEMVER-MINOR) add `markAsUncloneable` api (Jason Zhang) #55234 PR-URL: TODO

Notable changes: crypto: * (SEMVER-MINOR) add `KeyObject.prototype.toCryptoKey` (Filip Skokan) #55262 * (SEMVER-MINOR) add Date fields for `validTo` and `validFrom` (Andrew Moon) #54159 doc: * add abmusse to collaborators (Abdirahim Musse) #55086 http2: * (SEMVER-MINOR) expose `nghttp2_option_set_stream_reset_rate_limit` as an option (Maël Nison) #54875 lib: * (SEMVER-MINOR) propagate aborted state to dependent signals before firing events (jazelly) #54826 module: * (SEMVER-MINOR) support loading entrypoint as url (RedYetiDev) #54933 * (SEMVER-MINOR) implement the `"module-sync"` exports condition (Joyee Cheung) #54648 * (SEMVER-MINOR) implement `flushCompileCache()` (Joyee Cheung) #54971 * (SEMVER-MINOR) throw when invalid argument is passed to `enableCompileCache()` (Joyee Cheung) #54971 * (SEMVER-MINOR) write compile cache to temporary file and then rename it (Joyee Cheung) #54971 src: * mark node `--run` as stable (Yagiz Nizipli) #53763 test_runner: * (SEMVER-MINOR) support custom arguments in `run()` (Aviv Keller) #55126 * (SEMVER-MINOR) add `'test:summary'` event (Colin Ihrig) #54851 * (SEMVER-MINOR) add support for coverage via `run()` (Chemi Atlow) #53937 worker: * (SEMVER-MINOR) add `markAsUncloneable` api (Jason Zhang) #55234 PR-URL: #55343

Notable changes: crypto: * (SEMVER-MINOR) add `KeyObject.prototype.toCryptoKey` (Filip Skokan) #55262 * (SEMVER-MINOR) add Date fields for `validTo` and `validFrom` (Andrew Moon) #54159 doc: * add abmusse to collaborators (Abdirahim Musse) #55086 http2: * (SEMVER-MINOR) expose `nghttp2_option_set_stream_reset_rate_limit` as an option (Maël Nison) #54875 lib: * (SEMVER-MINOR) propagate aborted state to dependent signals before firing events (jazelly) #54826 module: * (SEMVER-MINOR) support loading entrypoint as url (RedYetiDev) #54933 * (SEMVER-MINOR) implement the `"module-sync"` exports condition (Joyee Cheung) #54648 * (SEMVER-MINOR) implement `flushCompileCache()` (Joyee Cheung) #54971 * (SEMVER-MINOR) throw when invalid argument is passed to `enableCompileCache()` (Joyee Cheung) #54971 * (SEMVER-MINOR) write compile cache to temporary file and then rename it (Joyee Cheung) #54971 src: * mark `node --run` as stable (Yagiz Nizipli) #53763 test_runner: * (SEMVER-MINOR) support custom arguments in `run()` (Aviv Keller) #55126 * (SEMVER-MINOR) add `'test:summary'` event (Colin Ihrig) #54851 * (SEMVER-MINOR) add support for coverage via `run()` (Chemi Atlow) #53937 worker: * (SEMVER-MINOR) add `markAsUncloneable` api (Jason Zhang) #55234 PR-URL: #55343

Notable changes: crypto: * (SEMVER-MINOR) add `KeyObject.prototype.toCryptoKey` (Filip Skokan) #55262 * (SEMVER-MINOR) add Date fields for `validTo` and `validFrom` (Andrew Moon) #54159 doc: * add abmusse to collaborators (Abdirahim Musse) #55086 http2: * (SEMVER-MINOR) expose `nghttp2_option_set_stream_reset_rate_limit` as an option (Maël Nison) #54875 lib: * (SEMVER-MINOR) propagate aborted state to dependent signals before firing events (jazelly) #54826 module: * (SEMVER-MINOR) support loading entrypoint as url (RedYetiDev) #54933 * (SEMVER-MINOR) implement the `"module-sync"` exports condition (Joyee Cheung) #54648 * (SEMVER-MINOR) implement `flushCompileCache()` (Joyee Cheung) #54971 * (SEMVER-MINOR) throw when invalid argument is passed to `enableCompileCache()` (Joyee Cheung) #54971 * (SEMVER-MINOR) write compile cache to temporary file and then rename it (Joyee Cheung) #54971 process: * (SEMVER-MINOR) add `process.features.require_module` (Joyee Cheung) #55241 src: * mark `node --run` as stable (Yagiz Nizipli) #53763 test_runner: * (SEMVER-MINOR) support custom arguments in `run()` (Aviv Keller) #55126 * (SEMVER-MINOR) add `'test:summary'` event (Colin Ihrig) #54851 * (SEMVER-MINOR) add support for coverage via `run()` (Chemi Atlow) #53937 worker: * (SEMVER-MINOR) add `markAsUncloneable` api (Jason Zhang) #55234 PR-URL: #55343

Notable changes: crypto: * (SEMVER-MINOR) add `KeyObject.prototype.toCryptoKey` (Filip Skokan) #55262 * (SEMVER-MINOR) add Date fields for `validTo` and `validFrom` (Andrew Moon) #54159 doc: * add abmusse to collaborators (Abdirahim Musse) #55086 http2: * (SEMVER-MINOR) expose `nghttp2_option_set_stream_reset_rate_limit` as an option (Maël Nison) #54875 lib: * (SEMVER-MINOR) propagate aborted state to dependent signals before firing events (jazelly) #54826 module: * (SEMVER-MINOR) support loading entrypoint as url (RedYetiDev) #54933 * (SEMVER-MINOR) implement the `"module-sync"` exports condition (Joyee Cheung) #54648 * (SEMVER-MINOR) implement `flushCompileCache()` (Joyee Cheung) #54971 * (SEMVER-MINOR) throw when invalid argument is passed to `enableCompileCache()` (Joyee Cheung) #54971 * (SEMVER-MINOR) write compile cache to temporary file and then rename it (Joyee Cheung) #54971 process: * (SEMVER-MINOR) add `process.features.require_module` (Joyee Cheung) #55241 * (SEMVER-MINOR) add `process.features.typescript` (Aviv Keller) #54295 src: * mark `node --run` as stable (Yagiz Nizipli) #53763 test_runner: * (SEMVER-MINOR) support custom arguments in `run()` (Aviv Keller) #55126 * (SEMVER-MINOR) add `'test:summary'` event (Colin Ihrig) #54851 * (SEMVER-MINOR) add support for coverage via `run()` (Chemi Atlow) #53937 worker: * (SEMVER-MINOR) add `markAsUncloneable` api (Jason Zhang) #55234 PR-URL: #55343

This commit adds a new 'test:summary' event to the test runner's reporting interface. This new event serves two purposes: - In the future, the test runner internals will no longer need to change the process exit code. This may be important to run() users. Unfortunately, this is a breaking change, so it needs to be changed in a major version. - The reporting interface now has a single event that can identify passing or failing test runs. Refs: nodejs#53867 Refs: nodejs#54812 PR-URL: nodejs#54851 Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Notable changes: crypto: * (SEMVER-MINOR) add `KeyObject.prototype.toCryptoKey` (Filip Skokan) nodejs#55262 * (SEMVER-MINOR) add Date fields for `validTo` and `validFrom` (Andrew Moon) nodejs#54159 doc: * add abmusse to collaborators (Abdirahim Musse) nodejs#55086 http2: * (SEMVER-MINOR) expose `nghttp2_option_set_stream_reset_rate_limit` as an option (Maël Nison) nodejs#54875 lib: * (SEMVER-MINOR) propagate aborted state to dependent signals before firing events (jazelly) nodejs#54826 module: * (SEMVER-MINOR) support loading entrypoint as url (RedYetiDev) nodejs#54933 * (SEMVER-MINOR) implement the `"module-sync"` exports condition (Joyee Cheung) nodejs#54648 * (SEMVER-MINOR) implement `flushCompileCache()` (Joyee Cheung) nodejs#54971 * (SEMVER-MINOR) throw when invalid argument is passed to `enableCompileCache()` (Joyee Cheung) nodejs#54971 * (SEMVER-MINOR) write compile cache to temporary file and then rename it (Joyee Cheung) nodejs#54971 process: * (SEMVER-MINOR) add `process.features.require_module` (Joyee Cheung) nodejs#55241 * (SEMVER-MINOR) add `process.features.typescript` (Aviv Keller) nodejs#54295 src: * mark `node --run` as stable (Yagiz Nizipli) nodejs#53763 test_runner: * (SEMVER-MINOR) support custom arguments in `run()` (Aviv Keller) nodejs#55126 * (SEMVER-MINOR) add `'test:summary'` event (Colin Ihrig) nodejs#54851 * (SEMVER-MINOR) add support for coverage via `run()` (Chemi Atlow) nodejs#53937 worker: * (SEMVER-MINOR) add `markAsUncloneable` api (Jason Zhang) nodejs#55234 PR-URL: nodejs#55343

nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Sep 8, 2024

cjihrig mentioned this pull request Sep 8, 2024

test_runner: report coverage thresholds in test:coverage #54813

Merged

RedYetiDev mentioned this pull request Sep 8, 2024

test_runner: add coverage support to run function #53937

Merged

RedYetiDev added test_runner Issues and PRs related to the test runner subsystem. and removed lib / src Issues and PRs related to general changes in the lib or src directory. labels Sep 8, 2024

MoLow reviewed Sep 9, 2024

View reviewed changes

lib/internal/test_runner/harness.js Show resolved Hide resolved

MoLow reviewed Sep 9, 2024

View reviewed changes

lib/internal/test_runner/tests_stream.js Outdated Show resolved Hide resolved

RedYetiDev added the wip Issues and PRs that are still a work in progress. label Sep 14, 2024

cjihrig force-pushed the summary branch from b5254f9 to dfcbb86 Compare September 14, 2024 19:17

cjihrig removed the wip Issues and PRs that are still a work in progress. label Sep 14, 2024

cjihrig requested a review from MoLow September 14, 2024 19:18

cjihrig force-pushed the summary branch from dfcbb86 to 808d7ad Compare September 14, 2024 19:22

atlowChemi approved these changes Sep 14, 2024

View reviewed changes

atlowChemi added the semver-major PRs that contain breaking changes and should be released in the next major version. label Sep 14, 2024

atlowChemi removed the semver-major PRs that contain breaking changes and should be released in the next major version. label Sep 14, 2024

RedYetiDev added the semver-minor PRs that contain new features and should be released in the next minor version. label Sep 14, 2024

cjihrig added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 14, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 14, 2024

MoLow approved these changes Sep 15, 2024

View reviewed changes

mcollina approved these changes Sep 15, 2024

View reviewed changes

cjihrig removed the needs-ci PRs that need a full CI run. label Sep 20, 2024

RedYetiDev added the review wanted PRs that need reviews. label Sep 20, 2024

atlowChemi approved these changes Sep 21, 2024

View reviewed changes

cjihrig added commit-queue Add this label to land a pull request using GitHub Actions. and removed review wanted PRs that need reviews. labels Sep 21, 2024

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 21, 2024

nodejs-github-bot merged commit 059e08b into nodejs:main Sep 21, 2024
58 checks passed

cjihrig deleted the summary branch September 21, 2024 17:48

aduh95 mentioned this pull request Oct 9, 2024

V22.10.0 proposal #55343

Merged

RafaelGSS mentioned this pull request Oct 11, 2024

2024-10-16, Version 23.0.0 (Current) #55338

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test_runner: add 'test:summary' event #54851

test_runner: add 'test:summary' event #54851

cjihrig commented Sep 8, 2024 •

edited

Loading

nodejs-github-bot commented Sep 8, 2024

codecov bot commented Sep 8, 2024 •

edited

Loading

RedYetiDev commented Sep 14, 2024

atlowChemi commented Sep 14, 2024

cjihrig commented Sep 14, 2024

nodejs-github-bot commented Sep 14, 2024

mcollina commented Sep 15, 2024

cjihrig commented Sep 15, 2024

mcollina commented Sep 15, 2024

mcollina left a comment

RedYetiDev commented Sep 20, 2024 •

edited

Loading

cjihrig commented Sep 20, 2024

cjihrig commented Sep 20, 2024

RedYetiDev commented Sep 20, 2024 •

edited

Loading

cjihrig commented Sep 21, 2024

nodejs-github-bot commented Sep 21, 2024

test_runner: add 'test:summary' event #54851

test_runner: add 'test:summary' event #54851

Conversation

cjihrig commented Sep 8, 2024 • edited Loading

nodejs-github-bot commented Sep 8, 2024

codecov bot commented Sep 8, 2024 • edited Loading

Codecov Report

RedYetiDev commented Sep 14, 2024

atlowChemi commented Sep 14, 2024

cjihrig commented Sep 14, 2024

nodejs-github-bot commented Sep 14, 2024

mcollina commented Sep 15, 2024

cjihrig commented Sep 15, 2024

mcollina commented Sep 15, 2024

mcollina left a comment

Choose a reason for hiding this comment

RedYetiDev commented Sep 20, 2024 • edited Loading

cjihrig commented Sep 20, 2024

cjihrig commented Sep 20, 2024

RedYetiDev commented Sep 20, 2024 • edited Loading

cjihrig commented Sep 21, 2024

nodejs-github-bot commented Sep 21, 2024

cjihrig commented Sep 8, 2024 •

edited

Loading

codecov bot commented Sep 8, 2024 •

edited

Loading

RedYetiDev commented Sep 20, 2024 •

edited

Loading

RedYetiDev commented Sep 20, 2024 •

edited

Loading