test_runner: wait for filtered suite build by semimikoh · Pull Request #64208 · nodejs/node · GitHub
Skip to content

test_runner: wait for filtered suite build#64208

Open
semimikoh wants to merge 1 commit into
nodejs:mainfrom
semimikoh:test_runner/await-filtered-suite-build
Open

test_runner: wait for filtered suite build#64208
semimikoh wants to merge 1 commit into
nodejs:mainfrom
semimikoh:test_runner/await-filtered-suite-build

Conversation

@semimikoh

Copy link
Copy Markdown
Contributor

Problem

When a filtered Suite has an async body, it can still be building its
children when the filtered run path calls postRun(). If the suite body
uses await test(...), the filtered child can be registered after the
suite is already being cleaned up, causing the child to be cancelled.

That leaves the run with no reported test failures, but a non-zero exit
code due to the cancelled test.

Fix

Make Suite#filteredRun() wait for buildSuite before delegating to the
base filtered run implementation. This keeps filtered suites from being
cleaned up while their async body is still registering children.

Test

Adds a regression fixture with awaited suites and awaited tests, then runs
it with --test --test-isolation=none --test-name-pattern=C. The test
asserts the run exits successfully and does not report any cancelled tests.

Verification

$ git diff --cached --check

The full regression test was not completed locally because the existing
out/Release/node binary in this checkout is v22.22.2-pre and is not
compatible with the current main test suite; the existing
test-runner-no-isolation-filtering cases fail with status 9 before this
change can be validated.

Fixes: #64203

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels Jun 30, 2026
Signed-off-by: semimikoh <ejffjeosms@gmail.com>
@semimikoh semimikoh force-pushed the test_runner/await-filtered-suite-build branch from 3db8275 to 55f1419 Compare June 30, 2026 04:09
@atlowChemi

atlowChemi commented Jun 30, 2026

Copy link
Copy Markdown
Member

The full regression test was not completed locally because the existing
out/Release/node binary in this checkout is v22.22.2-pre and is not
compatible with the current main test suite; the existing
test-runner-no-isolation-filtering cases fail with status 9 before this
change can be validated.

This part sounds to me like something an LLM would write, and it doesn't really make a lot of sense to me. if the binary is old, make it again?
Anyway, the change very clearly causes the tests to timeout and fail

@semimikoh

Copy link
Copy Markdown
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test runner file exits with code 1 when awaiting tests + filtering

4 participants