fix(ext/node): fix node:test hook ordering and error handling#35393
Merged
Conversation
A test nested inside multiple `describe()` blocks only ran its immediate suite's `beforeEach()`/`afterEach()` hooks; an enclosing suite's hooks were skipped (issue #35404). Node runs every enclosing suite's per-test hooks, plus the file-scope ones: `beforeEach` outermost-first and `afterEach` innermost-first. TestSuite now keeps a `parent` link, threaded through `addSuite()` / `wrapSuiteFn()`. When a suite test runs, it walks that chain (and the root hooks) at run time so hooks registered late in a suite body are still observed, invoking beforeEach outermost-in and afterEach innermost-out. Extended the `node_test_context` spec with a nested describe that asserts both ancestor `beforeEach` hooks run before the test, outermost-first, and both `afterEach` hooks run after it, innermost-first.
node:test hook ordering and error handling
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #35390. Fixes #35404.
Under
node:test, anafter()hook registered on a test's context viat.after(...)is meant to run once, after that test's body and all of itssubtests have completed. Deno's compatibility layer was instead running the
parent's
before()/after()hooks inside every subtest's step, so anafter()that tore down a shared resource (a server, a database handle) firedright after the first subtest and broke every subtest after it.
The fix separates the once-per-test hooks from the per-subtest ones. A
context's
before()hooks now run a single time, lazily, just before its firstsubtest, and its
after()hooks run a single time after its body and all ofits subtests finish. Both are guarded by idempotency flags so the success and
failure paths can invoke them without double-running, and so concurrent
subtests cannot re-trigger them. A subtest's own
after()hooks now run at theend of that subtest (previously a leaf subtest's
after()hooks never ran atall), and the top-level test's
after()hooks run in the test wrapper once itsbody returns.
beforeEach()/afterEach()continue to run per subtest asbefore.
This was found running the fastify test suite under
deno test, where itaccounted for the bulk of the failures: the premature close cascaded into
server.address()returningnullin later subtests.Added coverage to the
node_test_contextspec asserting thatt.before()runsexactly once before the first subtest and that
t.after()does not fire untilthe parent test completes.
beforeEach/afterEach cascade through ancestor suites (#35404)
While fixing the above, a closely related hook bug was addressed in the same
area. A test nested inside multiple
describe()blocks only ran its immediatesuite's
beforeEach()/afterEach()hooks; the hooks of any enclosing suitewere skipped. Node instead runs every enclosing suite's per-test hooks, plus
the file-scope ones, with
beforeEachgoing outermost-first andafterEachinnermost-first.
TestSuitenow carries aparentlink, threaded throughaddSuite()andwrapSuiteFn(). When a suite test runs it walks that chain (and the roothooks) at run time, so hooks registered late in a suite body are still seen,
and invokes
beforeEachoutermost-in andafterEachinnermost-out. Thenode_test_contextspec gained a nesteddescribe()asserting both thebeforeEach order before the test and the afterEach order after it.