Skip to content

fix(node/test): route nested top-level test() to a subtest#35406

Merged
bartlomieju merged 3 commits into
mainfrom
fix/node-test-nested-top-level
Jun 22, 2026
Merged

fix(node/test): route nested top-level test() to a subtest#35406
bartlomieju merged 3 commits into
mainfrom
fix/node-test-nested-top-level

Conversation

@bartlomieju

Copy link
Copy Markdown
Member

Calling the top-level test() (or it()) from node:test inside another
test's body threw "Nested Deno.test() calls are not supported. Use t.step()
for nested tests." In Node this pattern registers a subtest of the running
test, and real-world suites rely on it; the incompatibility was found while
trying to run fastify's test suite.

This tracks the test whose body is currently executing (via an
AsyncLocalStorage so the association survives await boundaries) and routes a
nested top-level test() to it as a subtest through t.step(), instead of
falling through to Deno.test(). Such subtests are commonly not awaited by
the body, so the running test now waits for them to finish before it settles,
matching Node. Only these routed calls are drained; an unawaited t.test()
keeps its existing behavior so subtests holding the loop open with unref'd
timers are not forced to resolve.

Closes #35391

A top-level `test()`/`it()` from `node:test` called from inside another
test's body now registers as a subtest of the running test (via
`t.step()`), matching Node, instead of throwing "Nested Deno.test() calls
are not supported". Discovered running fastify's test suite.

Closes #35391
Wrapping the test body in `als.run(...)` to expose the running context
inserted extra frames between the test runner and the user body, which pushed
`ext:cli/40_test.js` out of a failed test's captured stack and dropped the
source location reported for failures (broke the LSP testing API). Track the
active context with a plain saved/restored variable instead; Deno runs test
bodies sequentially, so it tracks the body across awaits and nested subtests
while leaving the body's call stack unchanged.

@bartlomieju bartlomieju left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Nice fix — the routing + drain mechanics are clean, the comments are excellent, and the node_test_module level 0a case (concurrency + mixed t.test()/routed test()) is good coverage. overrides pass-through and the defensive rejection handler on PromisePrototypeThen both look right. CI is green so far.

One concern I want on the record before this is relied on for messy suites: the module-global currentTestContext with synchronous save/restore is only correct for stack-disciplined (LIFO) execution, but the unawaited-routed-subtest path this PR enables is exactly where execution stops being LIFO. A routed subtest with an await in its body can leave currentTestContext pointing at a sibling/child when the parent resumes:

test("parent", async (t) => {                       // current = parent
  test("childA", async () => { await delay(50); });  // routed; childA suspends with current = childA
  await delay(10);                                   // parent suspends
  // childA's body resumes first: current = childA, then awaits delay(50)
  test("childB", () => {});                          // parent resumes while current === childA -> mis-routed under childA
});

childB registers as a subtest of childA instead of parent — silent mis-nesting (and if the wrong context's step has already finished, this.#denoContext.step(...) throws instead). It needs a routed subtest whose body awaits plus a parent that awaits and then issues another routed test(), which is plausible in fastify-style suites.

The 8330212 commit message explains the als.run(...) revert: it added stack frames that pushed ext:cli/40_test.js out of the failure stack and broke the LSP source location. That is a real constraint — but AsyncLocalStorage.enterWith(nodeTestContext) avoids it: it's a synchronous setter that adds no stack frame, yet still propagates across await and into child async contexts, so each interleaved body sees its own context without manual save/restore. Each Deno test/step fn is its own async root, so the store stays scoped to that body's subtree.

Not blocking from my side (your PR, your call) — but worth either confirming the interleaving case behaves or switching to enterWith.

Comment thread ext/node/polyfills/testing.ts Outdated
The single module global with synchronous save/restore was only correct for
stack-disciplined (LIFO) execution, but the unawaited routed-subtest path this
PR enables makes execution non-LIFO: a routed subtest that awaits in its body
can leave the global pointing at a sibling when the parent resumes, silently
mis-nesting the next routed top-level test() (or throwing if the wrong
context's step has already finished).

Use an AsyncLocalStorage set via enterWith() instead. enterWith is a
synchronous setter that adds no stack frame (unlike als.run(), which pushed
ext:cli/40_test.js out of the failure stack and broke the LSP source location),
yet propagates across await and into child async contexts. Each Deno test/step
fn is its own async root, so every interleaved body sees its own context with
no manual save/restore.
@bartlomieju
bartlomieju merged commit a6f2917 into main Jun 22, 2026
136 checks passed
@bartlomieju
bartlomieju deleted the fix/node-test-nested-top-level branch June 22, 2026 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

node:test: calling top-level test() inside another test throws "Nested Deno.test() calls are not supported"

1 participant