fix(node/test): route nested top-level test() to a subtest#35406
Conversation
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
left a comment
There was a problem hiding this comment.
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.
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.
Calling the top-level
test()(orit()) fromnode:testinside anothertest'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 throught.step(), instead offalling through to
Deno.test(). Such subtests are commonly not awaited bythe 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