diff --git a/src/runtime/test_runner/ScopeFunctions.rs b/src/runtime/test_runner/ScopeFunctions.rs index 0d387d0722cf..34c1f0ef7b42 100644 --- a/src/runtime/test_runner/ScopeFunctions.rs +++ b/src/runtime/test_runner/ScopeFunctions.rs @@ -330,6 +330,12 @@ impl ScopeFunctions { ) -> JsResult<()> { let _g = group_log::begin(); + // Snapshot any active AsyncLocalStorage context now that the caller has + // finished reading `.length` / `.bind()` from the bare function. This + // runs synchronously inside the user's `als.run(...)` body, so the + // captured frame is the same one `parse_arguments` would have seen. + let callback = callback.map(|cb| cb.with_async_context_if_needed(global)); + // only allow in collection phase match bun_test.phase { bun_test::Phase::Collection => {} // ok @@ -520,6 +526,9 @@ fn error_in_ci(global: &JSGlobalObject, signature: &[u8]) -> JsResult<()> { pub struct ParseArgumentsResult { pub description: Option>, + /// The user's callback as passed. Callers read `.length` (done-callback + /// detection) and `.bind()` (test.each) from it, then wrap via + /// `with_async_context_if_needed` at the storage point. pub callback: Option, pub options: ParseArgumentsOptions, } @@ -668,7 +677,7 @@ pub fn parse_arguments( let result_callback: Option = if cfg.callback != CallbackMode::Require && callback.is_undefined_or_null() { None } else if callback.is_function() { - Some(callback.with_async_context_if_needed(global)) + Some(callback) } else { let ordinal = if cfg.kind == FunctionKind::Hook { "first" } else { "second" }; return Err(global.throw(format_args!("{} expects a function as the {} argument", signature, ordinal))); diff --git a/src/runtime/test_runner/bun_test.rs b/src/runtime/test_runner/bun_test.rs index 5c6b92704cb0..f708a0d3507b 100644 --- a/src/runtime/test_runner/bun_test.rs +++ b/src/runtime/test_runner/bun_test.rs @@ -186,6 +186,14 @@ pub mod js_fns { false }; + // Snapshot any active AsyncLocalStorage context now that `.length` + // has been read from the bare function (an AsyncContextFrame has no + // `.length`). Still synchronous inside the user's `als.run(...)` + // body, so the captured frame is the one the caller expects. + let callback = args + .callback + .map(|cb| cb.with_async_context_if_needed(global_this)); + let bun_test_root = get_active_test_root( global_this, &GetActiveCfg { signature: Signature::Str(sig_bytes), allow_in_preload: true }, @@ -208,7 +216,7 @@ pub mod js_fns { let _ = bun_test_root.hook_scope.append_hook( tag.as_hook_tag().unwrap(), - args.callback, + callback, cfg, BaseScopeCfg::default(), AddedInPhase::Preload, @@ -226,7 +234,7 @@ pub mod js_fns { } let _ = bun_test.collection.active_scope_mut().append_hook( tag.as_hook_tag().unwrap(), - args.callback, + callback, cfg, BaseScopeCfg::default(), AddedInPhase::Collection, @@ -299,7 +307,7 @@ pub mod js_fns { let new_item = ExecutionEntry::create( None, - args.callback, + callback, cfg, None, BaseScopeCfg::default(), diff --git a/test/js/bun/test/als-hook-arity.fixture.ts b/test/js/bun/test/als-hook-arity.fixture.ts new file mode 100644 index 000000000000..3320b56e01de --- /dev/null +++ b/test/js/bun/test/als-hook-arity.fixture.ts @@ -0,0 +1,83 @@ +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, test } from "bun:test"; +import { AsyncLocalStorage } from "node:async_hooks"; + +// When a test or hook is registered while an AsyncLocalStorage context is +// active, bun:test wraps the callback in an AsyncContextFrame so the context +// is restored at call time. The wrapper has no `.length` and is not itself +// callable via JSC::getCallData, so `.length` (done-param detection) and +// `.bind()` (test.each) must be read from the user's function before wrapping. + +const als = new AsyncLocalStorage<{ tag: string }>(); +const order: string[] = []; +const mark = (label: string) => order.push(`${label}:${als.getStore()?.tag ?? "none"}`); + +describe("registered inside an active ALS context", () => { + als.run({ tag: "ctx" }, () => { + beforeAll(function zeroArg() { + mark("beforeAll"); + }); + beforeEach(function zeroArg() { + mark("beforeEach"); + }); + afterEach(function zeroArg() { + mark("afterEach"); + }); + afterAll(function zeroArg() { + mark("afterAll"); + }); + afterAll(function withDone(done) { + mark("afterAll-done"); + setImmediate(done); + }); + + test("zero-arg test", function zeroArg() { + mark("zero-arg test"); + }); + + test("one-arg test still receives done", function withDone(done) { + mark("done test"); + expect(typeof done).toBe("function"); + setImmediate(done); + }); + + test.each([[1], [2]])("each %p", function withArg(n) { + mark(`each ${n}`); + }); + + describe("nested describe", function zeroArg() { + mark("nested.body"); + afterAll(function zeroArg() { + mark("nested.afterAll"); + }); + test("passes", function zeroArg() { + mark("nested.test"); + }); + }); + }); +}); + +test("hooks and tests registered inside an ALS context use the callback's real arity and restore the context", () => { + // Every entry ran inside the restored `{ tag: "ctx" }` store, in order. + expect(order).toEqual([ + "nested.body:ctx", + "beforeAll:ctx", + "beforeEach:ctx", + "zero-arg test:ctx", + "afterEach:ctx", + "beforeEach:ctx", + "done test:ctx", + "afterEach:ctx", + "beforeEach:ctx", + "each 1:ctx", + "afterEach:ctx", + "beforeEach:ctx", + "each 2:ctx", + "afterEach:ctx", + "beforeEach:ctx", + "nested.test:ctx", + "afterEach:ctx", + "nested.afterAll:ctx", + "afterAll:ctx", + "afterAll-done:ctx", + ]); +}); diff --git a/test/js/bun/test/als-hook-arity.test.ts b/test/js/bun/test/als-hook-arity.test.ts new file mode 100644 index 000000000000..2e5180a76048 --- /dev/null +++ b/test/js/bun/test/als-hook-arity.test.ts @@ -0,0 +1,37 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, normalizeBunSnapshot } from "harness"; +import path from "node:path"; + +// Registering a zero-arg test() or hook inside an active AsyncLocalStorage +// context used to be treated as taking a done callback (the AsyncContextFrame +// wrapper has no `.length`), so the callback would wait for done() and time +// out; test.each() threw "bind() called on non-callable" on the same wrapper. +// Run the fixture with a short per-test timeout so the unfixed build fails +// fast rather than sitting on the 5s default for each entry. +test("tests and hooks registered inside an AsyncLocalStorage context detect done-callback arity correctly", async () => { + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "--timeout=500", path.join(import.meta.dir, "als-hook-arity.fixture.ts")], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(normalizeBunSnapshot(stderr)).toMatchInlineSnapshot(` + "test/js/bun/test/als-hook-arity.fixture.ts: + (pass) registered inside an active ALS context > zero-arg test + (pass) registered inside an active ALS context > one-arg test still receives done + (pass) registered inside an active ALS context > each 1 + (pass) registered inside an active ALS context > each 2 + (pass) registered inside an active ALS context > nested describe > passes + (pass) hooks and tests registered inside an ALS context use the callback's real arity and restore the context + + 6 pass + 0 fail + 2 expect() calls + Ran 6 tests across 1 file." + `); + expect(stdout).toStartWith("bun test "); + expect(exitCode).toBe(0); +});