test_runner: support forced exit by cjihrig · Pull Request #52038 · nodejs/node

cjihrig · 2024-03-10T22:03:35Z

This commit updates the test runner to allow a forced exit once all known tests have finished running.

Fixes: #49925

nodejs-github-bot · 2024-03-10T22:03:40Z

Review requested:

@nodejs/test_runner

test/fixtures/test-runner/output/force_exit.js

test/parallel/test-runner-output.mjs

rluvaton · 2024-03-11T09:17:34Z

lib/internal/test_runner/test.js

+      // possible that a ref'ed handle could asynchronously create more tests,
+      // but the user opted into this behavior.
+      this.reporter.once('close', () => {
+        process.exit();


what if the reporter is already closed, should we check if the reporter is already closed?

I think we should check if there are active handles before exiting so maybe the force exit is not needed

maybe print a warning that some handles are refed and it's best to clean them up

what if the reporter is already closed

in this case, there is no need for forced exit. the reporter closes on a call to root.postRun

MoLow

LGTM. +1 for some @rluvaton comments but not blocking (fire a warning /w count & description of open handles, test exit code not effected when using this)

This commit updates the test runner to allow a forced exit once all known tests have finished running. Fixes: nodejs#49925

cjihrig · 2024-03-11T23:08:20Z

I've made the requested changes. I did not print any information about the handles because:

It can be trivially added in a follow up if someone wants.
Some users may not care to see that output since they don't even want to be bothered to make their tests exit correctly.

atlowChemi · 2024-03-12T18:37:02Z

src/node_options.cc

+    } else if (test_runner_force_exit) {
+      errors->push_back("either --watch or --test-force-exit "
+                        "can be used, not both");


This is becoming a repetitive check (if AAA -> push_back("either --watch or AAA can be used, not both")),
Might be worth extracting to some function 🤔

nodejs-github-bot · 2024-03-12T21:03:11Z

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

nodejs-github-bot · 2024-03-13T00:15:07Z

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

nodejs-github-bot · 2024-03-13T00:59:31Z

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

rluvaton

LGTM

nodejs-github-bot · 2024-03-13T09:00:15Z

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

nodejs-github-bot · 2024-03-13T10:49:17Z

Landed in 84de97a

This commit updates the test runner to allow a forced exit once all known tests have finished running. Fixes: nodejs#49925 PR-URL: nodejs#52038 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Raz Luvaton <rluvaton@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

This commit updates the test runner to allow a forced exit once all known tests have finished running. Fixes: #49925 PR-URL: #52038 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Raz Luvaton <rluvaton@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Notable changes: benchmark: * add AbortSignal.abort benchmarks (Raz Luvaton) #52408 buffer: * improve `base64` and `base64url` performance (Yagiz Nizipli) #52428 crypto: * deprecate implicitly shortened GCM tags (Tobias Nießen) #52345 deps: * (SEMVER-MINOR) update simdutf to 5.0.0 (Daniel Lemire) #52138 * (SEMVER-MINOR) update undici to 6.3.0 (Node.js GitHub Bot) #51462 * (SEMVER-MINOR) update undici to 6.2.1 (Node.js GitHub Bot) #51278 dns: * (SEMVER-MINOR) add order option and support ipv6first (Paolo Insogna) #52492 doc: * update release gpg keyserver (marco-ippolito) #52257 * add release key for marco-ippolito (marco-ippolito) #52257 * add UlisesGascon as a collaborator (Ulises Gascón) #51991 * (SEMVER-MINOR) deprecate fs.Stats public constructor (Marco Ippolito) #51879 events,doc: * mark CustomEvent as stable (Daeyeon Jeong) #52618 fs: * add stacktrace to fs/promises (翠 / green) #49849 lib, url: * (SEMVER-MINOR) add a `windows` option to path parsing (Aviv Keller) #52509 net: * (SEMVER-MINOR) add CLI option for autoSelectFamilyAttemptTimeout (Paolo Insogna) #52474 report: * (SEMVER-MINOR) add `--report-exclude-network` option (Ethan Arrowood) #51645 src: * (SEMVER-MINOR) add `string_view` overload to snapshot FromBlob (Anna Henningsen) #52595 * (SEMVER-MINOR) add C++ ProcessEmitWarningSync() (Joyee Cheung) #51977 * (SEMVER-MINOR) add uv_get_available_memory to report and process (theanarkh) #52023 * (SEMVER-MINOR) preload function for Environment (Cheng Zhao) #51539 stream: * (SEMVER-MINOR) support typed arrays (IlyasShabi) #51866 test_runner: * (SEMVER-MINOR) add suite() (Colin Ihrig) #52127 * (SEMVER-MINOR) support forced exit (Colin Ihrig) #52038 * (SEMVER-MINOR) add `test:complete` event to reflect execution order (Moshe Atlow) #51909 util: * (SEMVER-MINOR) support array of formats in util.styleText (Marco Ippolito) #52040 v8: * (SEMVER-MINOR) implement v8.queryObjects() for memory leak regression testing (Joyee Cheung) #51927 watch: * mark as stable (Moshe Atlow) #52074 PR-URL: TODO

Notable changes: benchmark: * add AbortSignal.abort benchmarks (Raz Luvaton) #52408 buffer: * improve `base64` and `base64url` performance (Yagiz Nizipli) #52428 crypto: * deprecate implicitly shortened GCM tags (Tobias Nießen) #52345 deps: * (SEMVER-MINOR) update simdutf to 5.0.0 (Daniel Lemire) #52138 * (SEMVER-MINOR) update undici to 6.3.0 (Node.js GitHub Bot) #51462 * (SEMVER-MINOR) update undici to 6.2.1 (Node.js GitHub Bot) #51278 dns: * (SEMVER-MINOR) add order option and support ipv6first (Paolo Insogna) #52492 doc: * update release gpg keyserver (marco-ippolito) #52257 * add release key for marco-ippolito (marco-ippolito) #52257 * add UlisesGascon as a collaborator (Ulises Gascón) #51991 * (SEMVER-MINOR) deprecate fs.Stats public constructor (Marco Ippolito) #51879 events,doc: * mark CustomEvent as stable (Daeyeon Jeong) #52618 fs: * add stacktrace to fs/promises (翠 / green) #49849 lib, url: * (SEMVER-MINOR) add a `windows` option to path parsing (Aviv Keller) #52509 net: * (SEMVER-MINOR) add CLI option for autoSelectFamilyAttemptTimeout (Paolo Insogna) #52474 report: * (SEMVER-MINOR) add `--report-exclude-network` option (Ethan Arrowood) #51645 src: * (SEMVER-MINOR) add `string_view` overload to snapshot FromBlob (Anna Henningsen) #52595 * (SEMVER-MINOR) add C++ ProcessEmitWarningSync() (Joyee Cheung) #51977 * (SEMVER-MINOR) add uv_get_available_memory to report and process (theanarkh) #52023 * (SEMVER-MINOR) preload function for Environment (Cheng Zhao) #51539 stream: * (SEMVER-MINOR) support typed arrays (IlyasShabi) #51866 test_runner: * (SEMVER-MINOR) add suite() (Colin Ihrig) #52127 * (SEMVER-MINOR) support forced exit (Colin Ihrig) #52038 * (SEMVER-MINOR) add `test:complete` event to reflect execution order (Moshe Atlow) #51909 util: * (SEMVER-MINOR) support array of formats in util.styleText (Marco Ippolito) #52040 v8: * (SEMVER-MINOR) implement v8.queryObjects() for memory leak regression testing (Joyee Cheung) #51927 watch: * mark as stable (Moshe Atlow) #52074 PR-URL: #52793

This commit updates the test runner to allow a forced exit once all known tests have finished running. Fixes: nodejs#49925 PR-URL: nodejs#52038 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Raz Luvaton <rluvaton@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

This commit updates the test runner to allow a forced exit once all known tests have finished running. Fixes: #49925 PR-URL: #52038 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Raz Luvaton <rluvaton@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Notable changes: src,permission: * throw async errors on async APIs (Rafael Gonzaga) #52730 test_runner: * (SEMVER-MINOR) support forced exit (Colin Ihrig) #52038 PR-URL: TODO

Notable changes: src,permission: * throw async errors on async APIs (Rafael Gonzaga) #52730 test_runner: * (SEMVER-MINOR) support forced exit (Colin Ihrig) #52038 PR-URL: #53120

Notable changes: src,permission: * throw async errors on async APIs (Rafael Gonzaga) nodejs#52730 test_runner: * (SEMVER-MINOR) support forced exit (Colin Ihrig) nodejs#52038 PR-URL: nodejs#53120

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 Mar 10, 2024

cjihrig force-pushed the force-exit branch from 9bab5b7 to a82c365 Compare March 10, 2024 22:04

cjihrig mentioned this pull request Mar 10, 2024

test_runner: allow to force exit after all tests finished #49925

Closed

juliangruber reviewed Mar 10, 2024

View reviewed changes

test/fixtures/test-runner/output/force_exit.js Outdated Show resolved Hide resolved

rluvaton reviewed Mar 11, 2024

View reviewed changes

test/parallel/test-runner-output.mjs Show resolved Hide resolved

rluvaton reviewed Mar 11, 2024

View reviewed changes

MoLow approved these changes Mar 11, 2024

View reviewed changes

test_runner: support forced exit

d898cf8

This commit updates the test runner to allow a forced exit once all known tests have finished running. Fixes: nodejs#49925

cjihrig force-pushed the force-exit branch from a82c365 to d898cf8 Compare March 11, 2024 23:05

MoLow approved these changes Mar 11, 2024

View reviewed changes

juliangruber approved these changes Mar 12, 2024

View reviewed changes

atlowChemi approved these changes Mar 12, 2024

View reviewed changes

rluvaton approved these changes Mar 13, 2024

View reviewed changes

benjamingr approved these changes Mar 13, 2024

View reviewed changes

MoLow added the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 13, 2024

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

nodejs-github-bot merged commit 84de97a into nodejs:main Mar 13, 2024

MoLow linked an issue Mar 13, 2024 that may be closed by this pull request

node:test hangs when BroadcastChannel is left open #52073

Closed

cjihrig deleted the force-exit branch March 13, 2024 13:36

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

RafaelGSS mentioned this pull request Apr 15, 2024

2024-04-24, Version 22.0.0 (Current) #52505

Merged

marco-ippolito mentioned this pull request May 2, 2024

v20.13.0 proposal #52793

Merged

marco-ippolito mentioned this pull request May 23, 2024

v20.14.0 proposal #53120

Merged

samuelmurray mentioned this pull request Aug 12, 2024

Test runner with junit reporter and forceExit flag results in incomplete report #54327

Closed

Uh oh!

Conversation

cjihrig commented Mar 10, 2024

Uh oh!

nodejs-github-bot commented Mar 10, 2024

Uh oh!

Uh oh!

Uh oh!

rluvaton Mar 11, 2024

Choose a reason for hiding this comment

Uh oh!

MoLow Mar 11, 2024

Choose a reason for hiding this comment

Uh oh!

MoLow left a comment

Choose a reason for hiding this comment

Uh oh!

cjihrig commented Mar 11, 2024

Uh oh!

atlowChemi Mar 12, 2024

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Mar 12, 2024

Uh oh!

nodejs-github-bot commented Mar 13, 2024

Uh oh!

nodejs-github-bot commented Mar 13, 2024

Uh oh!

rluvaton left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Mar 13, 2024

Uh oh!

nodejs-github-bot commented Mar 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants