diff --git a/src/js_parser/parse/parse_entry.rs b/src/js_parser/parse/parse_entry.rs index 4bb47e5e0c0a..f4b7ae6ce90d 100644 --- a/src/js_parser/parse/parse_entry.rs +++ b/src/js_parser/parse/parse_entry.rs @@ -2014,6 +2014,14 @@ impl<'a> Parser<'a> { } } + // `keep_matcher_call_frame` (visit_stmt.rs) rewrote a returned matcher call. Like the + // globals above, this makes the output specific to `bun test`, so it must not be cached. + if p.jest.rewrote_matcher_tail_call { + if let Some(cache) = p.options.features.runtime_transpiler_cache_mut() { + cache.input_hash = None; + } + } + if p.has_called_runtime { let mut runtime_imports: [u8; RuntimeImports::ALL.len()] = [0; RuntimeImports::ALL.len()]; diff --git a/src/js_parser/parser.rs b/src/js_parser/parser.rs index f6680a8d25c4..3228b00df4be 100644 --- a/src/js_parser/parser.rs +++ b/src/js_parser/parser.rs @@ -362,7 +362,7 @@ pub mod Runtime { pub(crate) fn hash_for_runtime_transpiler(&self, hasher: &mut Wyhash) { debug_assert!(self.runtime_transpiler_cache.is_some()); - let bools: [bool; 17] = [ + let bools: [bool; 18] = [ self.top_level_await, self.auto_import_jsx, self.allow_runtime, @@ -380,7 +380,11 @@ pub mod Runtime { self.standard_decorators, self.lower_using, self.repl_mode, - // note that we do not include .inject_jest_globals, as we bail out of the cache entirely if this is true + // `keep_matcher_call_frame` changes the output of a file with an explicit + // `bun:test` import, which stays cacheable. The `input_hash = None` bails in + // `_parse` only stop the write: the read has already served `Result::Cached`, + // so the hash has to separate the two modes. + self.inject_jest_globals, ]; // `[bool; N]` is N bytes of 0x00/0x01. @@ -1600,6 +1604,9 @@ pub struct Jest { pub(crate) xit: Ref, pub(crate) xtest: Ref, pub(crate) xdescribe: Ref, + /// `P::keep_matcher_call_frame` rewrote a returned matcher call, so the output is specific + /// to `bun test` and must not enter the runtime transpiler cache. + pub(crate) rewrote_matcher_tail_call: bool, } impl Jest { @@ -1642,6 +1649,7 @@ impl Default for Jest { xit: Ref::NONE, xtest: Ref::NONE, xdescribe: Ref::NONE, + rewrote_matcher_tail_call: false, } } } diff --git a/src/js_parser/visit/visit_stmt.rs b/src/js_parser/visit/visit_stmt.rs index dfce8bec1923..086131f0a1d2 100644 --- a/src/js_parser/visit/visit_stmt.rs +++ b/src/js_parser/visit/visit_stmt.rs @@ -1552,6 +1552,10 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O if let Some(val) = data.value.as_mut() { p.visit_expr(val); + if p.options.features.inject_jest_globals && !p.is_control_flow_dead { + p.keep_matcher_call_frame(val); + } + // "return undefined;" can safely just always be "return;" if let Some(v) = data.value { if matches!(v.data, js_ast::ExprData::EUndefined(_)) { @@ -1565,6 +1569,94 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O Ok(()) } + /// `bun test` only. A returned matcher call, which is also what `() => expect(x).toBe(1)` is, + /// is a proper tail call (test files are modules, so strict mode): JSC replaces the frame that + /// contains the call with the matcher's own, so a failure has no location to report and an + /// inline snapshot has nowhere to be written. `return [expect(x).toBe(1)][0]` keeps the + /// frame. Tail position continues into the branches of `?:` and the right operand of `,`, + /// `&&`, `||` and `??`. + fn keep_matcher_call_frame(&mut self, value: &mut Expr) { + match value.data { + js_ast::ExprData::ECall(call) => { + if self.is_matcher_call(call.target) { + *value = self.take_out_of_tail_position(*value); + } + } + js_ast::ExprData::EIf(mut e) => { + self.keep_matcher_call_frame(&mut e.yes); + self.keep_matcher_call_frame(&mut e.no); + } + js_ast::ExprData::EBinary(mut e) + if matches!( + e.op, + js_ast::OpCode::BinComma + | js_ast::OpCode::BinLogicalAnd + | js_ast::OpCode::BinLogicalOr + | js_ast::OpCode::BinNullishCoalescing + ) => + { + self.keep_matcher_call_frame(&mut e.right); + } + _ => {} + } + } + + /// A method call on `expect(...)`, through any chain such as `.not` or `.resolves`, or a call + /// to one of the inline snapshot matchers on any receiver. + fn is_matcher_call(&self, callee: Expr) -> bool { + let Some(dot) = callee.data.e_dot() else { + return false; + }; + if dot.name == b"toMatchInlineSnapshot" || dot.name == b"toThrowErrorMatchingInlineSnapshot" + { + return true; + } + let mut receiver = dot.target; + loop { + match receiver.data { + js_ast::ExprData::EDot(inner) => receiver = inner.target, + js_ast::ExprData::ECall(call) => { + let ref_ = match call.target.data { + js_ast::ExprData::EIdentifier(id) => id.ref_, + js_ast::ExprData::EImportIdentifier(id) => id.ref_, + _ => return false, + }; + return self.symbols[ref_.inner_index() as usize] + .original_name + .slice() + == b"expect"; + } + _ => return false, + } + } + } + + /// `[call][0]`. Self-contained on purpose: a generated temporary would turn + /// `Function.prototype.toString` output into code with a free identifier, and test code does + /// round-trip such text into other modules. + fn take_out_of_tail_position(&mut self, call: Expr) -> Expr { + self.jest.rewrote_matcher_tail_call = true; + let loc = call.loc; + let items = js_ast::ExprNodeList::from_arena_slice(self.arena.alloc_slice_copy(&[call])); + let array = self.new_expr( + E::Array { + items, + is_single_line: true, + ..Default::default() + }, + loc, + ); + let index = self.new_expr(E::Number::new(0.0), loc); + self.new_expr( + E::Index { + target: array, + index, + optional_chain: None, + }, + loc, + ) + } + fn s_block( p: &mut Self, stmts: &mut StmtList<'a>, diff --git a/src/jsc/RuntimeTranspilerCache.rs b/src/jsc/RuntimeTranspilerCache.rs index 30345f1578a6..1fbe31ab36e6 100644 --- a/src/jsc/RuntimeTranspilerCache.rs +++ b/src/jsc/RuntimeTranspilerCache.rs @@ -51,7 +51,11 @@ bun_core::declare_scope!(cache, visible); /// Version 25: Every ModuleInfo record carries a trailing FetchParameters slot /// so ImportEntry/ExportEntry/StarExportEntry moduleRequestType matches JSC's /// after WebKit 90b2ecf79ae3 keyed m_loadedModules on (specifier, type). -const EXPECTED_VERSION: u32 = 25; +/// Version 26: `inject_jest_globals` participates in the features hash. The +/// matcher tail-call rewrite changes the output of a file with an explicit +/// `bun:test` import, which stays cacheable, so `bun run` and `bun test` must +/// not share entries for it. +const EXPECTED_VERSION: u32 = 26; /// Source files smaller than this are not written to / read from the on-disk /// transpiler cache. Originally 50 KiB, which excluded almost every file in a diff --git a/test/cli/test/bun-test.test.ts b/test/cli/test/bun-test.test.ts index 566279d3e877..f6b5ad676263 100644 --- a/test/cli/test/bun-test.test.ts +++ b/test/cli/test/bun-test.test.ts @@ -1343,6 +1343,105 @@ describe("bun test", () => { expect(stderr).toContain("1 pass"); }); + // A returned matcher call is a proper tail call, which would otherwise leave the failure with no + // frame in the file that contains the call. + describe("matcher call in tail position", () => { + test("a failure reports the line of the matcher call", () => { + const stderr = runTest({ + input: [ + { + filename: "tail.test.ts", + contents: [ + `import { test, expect } from "bun:test";`, + `test("expression-bodied callback", () => expect(1).toBe(2));`, + `function check(value: number) { return expect(value).toBe(2); }`, + `test("helper", () => { check(1); });`, + `test("nullish", () => globalThis.nothing ?? expect(1).toBe(2));`, + ].join("\n"), + }, + ], + expectExitCode: 1, + }); + expect(stderr).toMatch(/at \(.*tail\.test\.ts:2:\d+\)/); + expect(stderr).toMatch(/at check \(.*tail\.test\.ts:3:\d+\)/); + expect(stderr).toMatch(/at \(.*tail\.test\.ts:5:\d+\)/); + expect(stderr).toContain("3 fail"); + }); + + test("the matcher's return value still reaches the runner", () => { + const stderr = runTest({ + input: ` + import { test, expect } from "bun:test"; + expect.extend({ + async toFailLater() { + return { pass: false, message: () => "failed later" }; + }, + }); + test("async matcher", () => expect(1).toFailLater()); + `, + expectExitCode: 1, + }); + expect(stderr).toContain("failed later"); + expect(stderr).toContain("1 fail"); + }); + + // `bun run` and `bun test` transpile the same file to different output now, so they must not + // share a transpiler cache entry. The file needs an explicit `bun:test` import (injected + // globals already bail out of the cache) and 4KB of source (the cache floor). + test("the rewrite is not lost to a transpiler cache entry from bun run", async () => { + const contents = [ + `import { test, expect } from "bun:test";`, + `test("tail", () => expect(1).toBe(2));`, + `// ${Buffer.alloc(5000, "x").toString()}`, + ].join("\n"); + using dir = tempDir("tail-cache", { "cached.test.ts": contents }); + const cacheDir = join(String(dir), "cache"); + mkdirSync(cacheDir); + const env = { + ...bunEnv, + BUN_RUNTIME_TRANSPILER_CACHE_PATH: cacheDir, + // Debug builds read the cache but serve from it only with this set. + BUN_DEBUG_ENABLE_RESTORE_FROM_TRANSPILER_CACHE: "1", + }; + await using warm = Bun.spawn({ + cmd: [bunExe(), "run", "cached.test.ts"], + env, + cwd: String(dir), + stdout: "ignore", + stderr: "ignore", + }); + await warm.exited; + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "cached.test.ts"], + env, + cwd: String(dir), + stderr: "pipe", + }); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect(stderr).toMatch(/at \(.*cached\.test\.ts:2:\d+\)/); + expect(stderr).toContain("1 fail"); + expect(exitCode).toBe(1); + }); + + // This arrow is rewritten in this very file, and its toString() text lands in a different + // module, so the rewrite must not reference anything from the module it was made in. + test("the rewrite survives a Function.toString round-trip into another module", async () => { + const fn = () => expect(1).toBe(1); + using dir = tempDir("tail-tostring", { + "roundtrip.test.ts": `import { test, expect } from "bun:test";\ntest("round-trip", ${fn.toString()});`, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "roundtrip.test.ts"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect(stderr).toContain("1 pass"); + expect(exitCode).toBe(0); + }); + }); + test("path to a non-test.ts file will work", () => { const stderr = runTest({ args: ["./index.ts"], diff --git a/test/js/bun/test/snapshot-tests/snapshots/snapshot.test.ts b/test/js/bun/test/snapshot-tests/snapshots/snapshot.test.ts index bef562452a65..bc07e6b51fb8 100644 --- a/test/js/bun/test/snapshot-tests/snapshots/snapshot.test.ts +++ b/test/js/bun/test/snapshot-tests/snapshots/snapshot.test.ts @@ -486,6 +486,9 @@ describe("inline snapshots", () => { export function wrongFile(value) { expect(value).toMatchInlineSnapshot(); } + export function wrongFileTailCall(value) { + return expect(value).toMatchInlineSnapshot(); + } `; const tester = new InlineSnapshotTester({ "helper.js": helper_js, @@ -794,6 +797,145 @@ Date) `, ); }); + // In the tests below the matcher call is a proper tail call (test files are modules, so strict + // mode), and the function containing it was invoked by the test runner itself. When the matcher + // runs, the frame that held its location is gone and no JS frame is left below it at all. + describe("matcher call in tail position", () => { + it("expression-bodied test callback", async () => { + await tester.test( + v => /*js*/ ` + test("tail call", () => expect("v").toMatchInlineSnapshot(${v("", bad, '`"v"`')})); + `, + ); + }); + it("expression-bodied test callback (toThrowErrorMatchingInlineSnapshot)", async () => { + await tester.test( + v => /*js*/ ` + test("tail call", () => expect(() => { throw new Error("boom") }).toThrowErrorMatchingInlineSnapshot(${v("", bad, '`"boom"`')})); + `, + ); + }); + it("inline snapshots inside the function the matcher runs", async () => { + // The inner matcher must not disturb the location of the outer, tail-called one. + await tester.test( + v => /*js*/ ` + test("nested", () => expect(() => { + expect("inner").toMatchInlineSnapshot(${v("", bad, '`"inner"`')}); + throw new Error("outer"); + }).toThrowErrorMatchingInlineSnapshot(${v("", bad, '`"outer"`')})); + `, + ); + }); + it("other shapes", async () => { + await tester.test( + // prettier-ignore + v => /*js*/ ` + test("resolves", () => expect(Promise.resolve("r")).resolves.toMatchInlineSnapshot(${v("", bad, '`"r"`')})); + test("return statement", () => { return expect("s").toMatchInlineSnapshot(${v("", bad, '`"s"`')}); }); + test("only the branch that ran", () => Date.now() ? expect("t").toMatchInlineSnapshot(${v("", bad, '`"t"`')}) : expect("never").toMatchInlineSnapshot()); + test("last operand of a comma expression", () => (expect("c1").toMatchInlineSnapshot(${v("", bad, '`"c1"`')}), expect("c2").toMatchInlineSnapshot(${v("", bad, '`"c2"`')}))); + test("right operand of &&", () => Date.now() && expect("and").toMatchInlineSnapshot(${v("", bad, '`"and"`')})); + test("right operand of ||", () => globalThis.nothing || expect("or").toMatchInlineSnapshot(${v("", bad, '`"or"`')})); + test("right operand of ??", () => globalThis.nothing ?? expect("nullish").toMatchInlineSnapshot(${v("", bad, '`"nullish"`')})); + test.each(["e"])("test.each %s", value => expect(value).toMatchInlineSnapshot(${v("", bad, '`"e"`')})); + test("name on its own line", () => + expect("n") + .toMatchInlineSnapshot(${v("", bad, '`"n"`')})); + test("dot on the line before", () => expect("d"). + toMatchInlineSnapshot(${v("", bad, '`"d"`')})); + test("optional chaining", () => expect("o")?.toMatchInlineSnapshot(${v("", bad, '`"o"`')})); + `, + ); + }); + it("helper function in the test file", async () => { + await tester.test( + v => /*js*/ ` + function snap(value) { + return expect(value).toMatchInlineSnapshot(${v("", bad, '`"h"`')}); + } + function snapThrow(fn) { + return expect(fn).toThrowErrorMatchingInlineSnapshot(${v("", bad, '`"thrown"`')}); + } + test("helpers", () => { + snap("h"); + snapThrow(() => { throw new Error("thrown") }); + }); + `, + ); + }); + it("helper function reached through another tail call", async () => { + await tester.test( + v => /*js*/ ` + function inner(value) { + return expect(value).toMatchInlineSnapshot(${v("", bad, '`"nested"`')}); + } + function outer(value) { + return inner(value); + } + test("nested helpers", () => { + outer("nested"); + }); + `, + ); + }); + it("helper function mixed with direct calls", async () => { + await tester.test( + v => /*js*/ ` + function snap(value) { + return expect(value).toMatchInlineSnapshot(${v("", bad, '`"helper"`')}); + } + test("mixed", () => { + expect("before").toMatchInlineSnapshot(${v("", bad, '`"before"`')}); + snap("helper"); + expect("after").toMatchInlineSnapshot(${v("", bad, '`"after"`')}); + }); + `, + ); + }); + it("matcher name also appears in the argument", async () => { + await tester.test( + v => /*js*/ ` + function snap() { + return expect(".toMatchInlineSnapshot(" /* .toMatchInlineSnapshot(\`\`) */).toMatchInlineSnapshot(${v("", bad, '`".toMatchInlineSnapshot("`')}); + } + test("decoy", () => { + snap(); + }); + `, + ); + }); + it("helper function called with different values", async () => { + // Both calls resolve to the helper's line: the tail-call twin of "should error trying to update the same line twice". + await tester.testError( + { + msg: "error: Failed to update inline snapshot: Multiple inline snapshots on the same line must all have the same value", + }, + /*js*/ ` + function snap(value) { + return expect(value).toMatchInlineSnapshot(); + } + test("conflict", () => { + snap("a"); + snap("b"); + }); + `, + ); + }); + it("helper function in another file is still rejected", async () => { + await tester.testError( + { + msg: "Inline snapshot matchers must be called from the test file", + }, + /*js*/ ` + import {wrongFileTailCall} from "./helper"; + test("cases", () => { + wrongFileTailCall("interesting"); + }); + `, + ); + expect(readFileSync(tester.tmpdir + "/helper.js", "utf-8")).toBe(helper_js); + }); + }); it("indentation", async () => { await tester.test( // prettier-ignore @@ -891,11 +1033,16 @@ test("error snapshots", () => { throw undefined; // this one doesn't work in jest because it doesn't think the function threw }).toThrowErrorMatchingInlineSnapshot(`undefined`); expect(() => { - expect(() => {}).toThrowErrorMatchingInlineSnapshot(`undefined`); + try { + expect(() => {}).toThrowErrorMatchingInlineSnapshot(`undefined`); + } catch (e) { + (e as Error).message = Bun.stripANSI((e as Error).message); + throw e; + } }).toThrowErrorMatchingInlineSnapshot(` -"\x1B[2mexpect(\x1B[0m\x1B[31mreceived\x1B[0m\x1B[2m).\x1B[0mtoThrowErrorMatchingInlineSnapshot\x1B[2m(\x1B[0m\x1B[2m)\x1B[0m +"expect(received).toThrowErrorMatchingInlineSnapshot() -\x1B[1mMatcher error\x1B[0m: Received function did not throw +Matcher error: Received function did not throw " `); });