Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/runtime/test_runner/ScopeFunctions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -520,6 +526,9 @@ fn error_in_ci(global: &JSGlobalObject, signature: &[u8]) -> JsResult<()> {

pub struct ParseArgumentsResult {
pub description: Option<Vec<u8>>,
/// 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<JSValue>,
pub options: ParseArgumentsOptions,
}
Expand Down Expand Up @@ -668,7 +677,7 @@ pub fn parse_arguments(
let result_callback: Option<JSValue> = 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)));
Expand Down
14 changes: 11 additions & 3 deletions src/runtime/test_runner/bun_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -299,7 +307,7 @@ pub mod js_fns {

let new_item = ExecutionEntry::create(
None,
args.callback,
callback,
cfg,
None,
BaseScopeCfg::default(),
Expand Down
82 changes: 82 additions & 0 deletions test/js/bun/test/als-hook-arity.fixture.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
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.
Comment thread
coderabbitai[bot] marked this conversation as resolved.

const als = new AsyncLocalStorage<{ tag: string }>();
const order: string[] = [];

describe("registered inside an active ALS context", () => {
als.run({ tag: "collection" }, () => {
beforeAll(function zeroArg() {
order.push("beforeAll");
});
beforeEach(function zeroArg() {
order.push("beforeEach");
});
afterEach(function zeroArg() {
order.push("afterEach");
});
afterAll(function zeroArg() {
order.push("afterAll");
});
afterAll(function withDone(done) {
order.push("afterAll-done");
setImmediate(done);
});

test("zero-arg test", function zeroArg() {
order.push("zero-arg test");
expect(als.getStore()?.tag).toBe("collection");
});

test("one-arg test still receives done", function withDone(done) {
order.push("done test");
expect(typeof done).toBe("function");
expect(als.getStore()?.tag).toBe("collection");
setImmediate(done);
});

test.each([[1], [2]])("each %p", function withArg(n) {
order.push(`each ${n}`);
expect(als.getStore()?.tag).toBe("collection");
});

describe("nested describe", function zeroArg() {
afterAll(function zeroArg() {
order.push("nested.afterAll");
});
test("passes", function zeroArg() {
order.push("nested.test");
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});
});
});

test("hooks and tests registered inside an ALS context use the callback's real arity", () => {
expect(order).toEqual([
"beforeAll",
"beforeEach",
"zero-arg test",
"afterEach",
"beforeEach",
"done test",
"afterEach",
"beforeEach",
"each 1",
"afterEach",
"beforeEach",
"each 2",
"afterEach",
"beforeEach",
"nested.test",
"afterEach",
"nested.afterAll",
"afterAll",
"afterAll-done",
]);
});
37 changes: 37 additions & 0 deletions test/js/bun/test/als-hook-arity.test.ts
Original file line number Diff line number Diff line change
@@ -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

6 pass
0 fail
6 expect() calls
Ran 6 tests across 1 file."
`);
expect(stdout).toStartWith("bun test ");
expect(exitCode).toBe(0);
});
Loading