bun test: fix the reason reported when a snapshot matcher's expect() has no running test

src/runtime/test_runner/expect.rs

@@ -175,6 +175,55 @@ impl Flags {
     }
 }
 
+/// Why a snapshot cannot be named after the test an `expect()` was created in.
+#[derive(Clone, Copy)]
+enum SnapshotContextError {
+    TestFinished,
+    ConcurrentGroup,
+    NoTest,
+}
+
+impl SnapshotContextError {
+    /// For a state captured by `expect()` that no longer resolves to a running entry.
+    fn from_phase(phase: &bun_test::RefDataValue) -> Self {
+        match phase {
+            bun_test::RefDataValue::Execution { entry_data: Some(_), .. } => Self::TestFinished,
+            // `get_current_state_data` records no entry only while a concurrent group runs.
+            bun_test::RefDataValue::Execution { entry_data: None, .. } => Self::ConcurrentGroup,
+            bun_test::RefDataValue::Start
+            | bun_test::RefDataValue::Collection { .. }
+            | bun_test::RefDataValue::Done => Self::NoTest,
+        }
+    }
+
+    fn from_error(err: &crate::Error) -> Option<Self> {
+        match err {
+            crate::Error::TestNotActive => Some(Self::TestFinished),
+            crate::Error::SnapshotInConcurrentGroup => Some(Self::ConcurrentGroup),
+            crate::Error::NoTest => Some(Self::NoTest),
+            _ => None,
+        }
+    }
+
+    fn message(self) -> &'static str {
+        match self {
+            Self::TestFinished => "Snapshot matchers are not supported after the test has finished executing",
+            Self::ConcurrentGroup => "Snapshot matchers are not supported in concurrent tests",
+            Self::NoTest => "Snapshot matchers cannot be used outside of a test",
+        }
+    }
+}
+
+impl From<SnapshotContextError> for crate::Error {
+    fn from(err: SnapshotContextError) -> Self {
+        match err {
+            SnapshotContextError::TestFinished => Self::TestNotActive,
+            SnapshotContextError::ConcurrentGroup => Self::SnapshotInConcurrentGroup,
+            SnapshotContextError::NoTest => Self::NoTest,
+        }
+    }
+}
+
 impl Expect {
     /// R-2 helper: read-modify-write the packed `Cell<Flags>` through `&self`.
     #[inline]
@@ -210,6 +259,34 @@ impl Expect {
         parent.bun_test()
     }
 
+    /// Only valid once `bun_test()` is None; a live file is classified by `get_snapshot_name`.
+    fn snapshot_context_error(&self) -> SnapshotContextError {
+        match self.parent.as_ref() {
+            Some(parent) => SnapshotContextError::from_phase(&parent.phase),
+            None => SnapshotContextError::NoTest,
+        }
+    }
+
+    /// Throws unless the test file this `expect()` was created in is still running.
+    pub(crate) fn check_snapshot_context(
+        &self,
+        global_this: &JSGlobalObject,
+        matcher_name: &'static str,
+    ) -> JsResult<()> {
+        if self.bun_test().is_some() {
+            return Ok(());
+        }
+        let signature = Self::get_signature(matcher_name, "", false);
+        throw!(
+            self,
+            global_this,
+            signature,
+            "\n\n<b>Matcher error<r>: {}\n",
+            self.snapshot_context_error().message(),
+        )
+        .map(drop)
+    }
+
     pub(crate) fn get_signature(
         matcher_name: &'static str,
         args: &'static str,
@@ -621,12 +698,10 @@ impl Expect {
 
     pub(crate) fn get_snapshot_name(&self, hint: &[u8]) -> crate::Result<Vec<u8>> {
         let parent = self.parent.as_ref().ok_or(crate::Error::NoTest)?;
-        let buntest_strong = parent.bun_test().ok_or(crate::Error::TestNotActive)?;
+        let context_error = || crate::Error::from(SnapshotContextError::from_phase(&parent.phase));
+        let buntest_strong = parent.bun_test().ok_or_else(context_error)?;
         let buntest = buntest_strong.get();
-        let execution_entry = parent
-            .phase
-            .entry(buntest)
-            .ok_or(crate::Error::SnapshotInConcurrentGroup)?;
+        let execution_entry = parent.phase.entry(buntest).ok_or_else(context_error)?;
 
         let test_name: &[u8] = execution_entry.base.name.as_deref().unwrap_or(b"(unnamed)");
 
@@ -1212,8 +1287,11 @@ impl Expect {
         let existing_value = match runner.snapshots.get_or_put(this, &pretty_value, hint) {
             Ok(v) => v,
             Err(err) => {
+                if let Some(reason) = SnapshotContextError::from_error(&err) {
+                    return Err(global_this.throw(format_args!("{}", reason.message())));
+                }
                 let Some(buntest_strong) = this.bun_test() else {
-                    return Err(global_this.throw(format_args!("Snapshot matchers cannot be used outside of a test")));
+                    return Err(global_this.throw(format_args!("{}", this.snapshot_context_error().message())));
                 };
                 let buntest = buntest_strong.get();
                 // MultiArrayList::get requires MultiArrayElement (derive pending); use column accessor.
@@ -1247,12 +1325,6 @@ impl Expect {
                             ))
                         }
                     }
-                    crate::Error::SnapshotInConcurrentGroup => {
-                        global_this.throw(format_args!("Snapshot matchers are not supported in concurrent tests"))
-                    }
-                    crate::Error::TestNotActive => {
-                        global_this.throw(format_args!("Snapshot matchers are not supported after the test has finished executing"))
-                    }
                     _ => {
                         let mut formatter = ConsoleObject::Formatter::new(global_this);
                         global_this.throw(format_args!("Failed to snapshot value: {}", value.to_fmt(&mut formatter)))

src/runtime/test_runner/expect/toMatchSnapshot.rs

@@ -31,16 +31,7 @@ pub(crate) fn to_match_snapshot(
         );
     }
 
-    let Some(buntest_strong) = this.bun_test() else {
-        let signature = get_signature("toMatchSnapshot", "", true);
-        return throw!(
-            this,
-            global,
-            signature,
-            "\n\n<b>Matcher error<r>: Snapshot matchers cannot be used outside of a test\n",
-        );
-    };
-    let _ = buntest_strong; // released by Drop at scope exit.
+    this.check_snapshot_context(global, "toMatchSnapshot")?;
 
     let mut hint_string: ZigString = ZigString::EMPTY;
     let mut property_matchers: Option<JSValue> = None;

src/runtime/test_runner/expect/toThrowErrorMatchingSnapshot.rs

@@ -30,16 +30,7 @@ pub(crate) fn to_throw_error_matching_snapshot(
         );
     }
 
-    let Some(bun_test_strong) = this.bun_test() else {
-        let signature = get_signature("toThrowErrorMatchingSnapshot", "", true);
-        return throw!(
-            this,
-            global,
-            signature,
-            "\n\n<b>Matcher error<r>: Snapshot matchers cannot be used outside of a test\n",
-        );
-    };
-    let _ = &bun_test_strong;
+    this.check_snapshot_context(global, "toThrowErrorMatchingSnapshot")?;
 
     let mut hint_string: ZigString = ZigString::EMPTY;
     match arguments.len() {

test/js/bun/test/bun_test.test.ts

@@ -232,7 +232,9 @@ test("cross-file safety", async () => {
   const exitCode = await result.exited;
   const stdout = await result.stdout.text();
   const stderr = await result.stderr.text();
-  expect(stderr).toInclude("Snapshot matchers cannot be used outside of a test");
+  expect(normalizeBunSnapshot(stderr)).toInclude(
+    "error: expect(received).toMatchSnapshot()\n\nMatcher error: Snapshot matchers are not supported after the test has finished executing",
+  );
   expect(exitCode).toBe(1);
 });
 

test/js/bun/test/snapshot-tests/bun-snapshots.test.ts

@@ -1,4 +1,7 @@
-import { describe, expect, it, test } from "bun:test";
+import { beforeAll, describe, expect, it, test } from "bun:test";
+import { bunEnv, bunExe, normalizeBunSnapshot, tempDir } from "harness";
+import { readdirSync } from "node:fs";
+import { join } from "node:path";
 
 test("it will create a snapshot file if it doesn't exist", () => {
   expect({ a: { b: { c: false } }, c: 2, jkfje: 99238 }).toMatchSnapshot({ a: { b: { c: expect.any(Boolean) } } });
@@ -62,3 +65,175 @@ describe("toMatchSnapshot errors", () => {
     }).toThrow();
   });
 });
+
+// A snapshot is named after the test that was running when expect() was called. These cover
+// the error reported when that test cannot name a snapshot by the time the matcher runs.
+describe("snapshot matchers on an expect() that has no running test", () => {
+  const finishedMessage = "Snapshot matchers are not supported after the test has finished executing";
+  const outsideTestMessage = "Snapshot matchers cannot be used outside of a test";
+  const concurrentMessage = "Snapshot matchers are not supported in concurrent tests";
+
+  describe("expect() created in a describe callback", () => {
+    let error: unknown;
+    try {
+      expect("described").toMatchSnapshot();
+    } catch (e) {
+      error = e;
+    }
+
+    it("throws the outside-of-a-test error", () => {
+      expect((error as Error).message).toBe(outsideTestMessage);
+    });
+  });
+
+  describe("expect() created in a beforeAll that has since finished", () => {
+    let value: ReturnType<typeof expect>;
+    let thrower: ReturnType<typeof expect>;
+    let inline: ReturnType<typeof expect>;
+
+    beforeAll(() => {
+      value = expect({ created: "in beforeAll" });
+      thrower = expect(() => {
+        throw new Error("created in beforeAll");
+      });
+      inline = expect("created in beforeAll");
+    });
+
+    it("toMatchSnapshot throws the finished-test error", () => {
+      expect(() => value.toMatchSnapshot()).toThrow(finishedMessage);
+      expect(() => value.toMatchSnapshot("with hint")).toThrow(finishedMessage);
+    });
+
+    it("toThrowErrorMatchingSnapshot throws the finished-test error", () => {
+      expect(() => thrower.toThrowErrorMatchingSnapshot()).toThrow(finishedMessage);
+    });
+
+    it("inline snapshots are keyed by source location and still work", () => {
+      inline.toMatchInlineSnapshot(`"created in beforeAll"`);
+    });
+  });
+
+  describe.concurrent("expect() created in a concurrent group", () => {
+    for (const name of ["first", "second"]) {
+      it(`${name} test throws the concurrent error`, async () => {
+        expect(() => expect(name).toMatchSnapshot()).toThrow(concurrentMessage);
+      });
+    }
+  });
+
+  it.concurrent("expect() from a timed out test throws the finished-test error and writes nothing", async () => {
+    using dir = tempDir("snapshot-after-timeout", {
+      "timeout.test.ts": `
+        import { expect, test } from "bun:test";
+
+        const nextTestStarted = Promise.withResolvers<void>();
+        const lateMatcherRan = Promise.withResolvers<void>();
+
+        test("times out", async () => {
+          const captured = expect({ from: "times out" });
+          // Only "next" resolves this, so this test has always timed out by the time it continues.
+          await nextTestStarted.promise;
+          try {
+            captured.toMatchSnapshot();
+            console.log("late toMatchSnapshot: did not throw");
+          } catch (error) {
+            console.log("late toMatchSnapshot:", (error as Error).message);
+          }
+          lateMatcherRan.resolve();
+        }, 1);
+
+        test("next", async () => {
+          nextTestStarted.resolve();
+          await lateMatcherRan.promise;
+          expect({ from: "next" }).toMatchSnapshot();
+        });
+      `,
+    });
+
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "test", "timeout.test.ts"],
+      cwd: String(dir),
+      env: { ...bunEnv, CI: "false" },
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+
+    expect(normalizeBunSnapshot(stdout)).toMatchInlineSnapshot(`
+      "bun test <version> (<revision>)
+      late toMatchSnapshot: Snapshot matchers are not supported after the test has finished executing"
+    `);
+    expect(stderr).toContain("this test timed out after 1ms");
+    expect(stderr).toContain("(pass) next");
+    expect(stderr).toContain("snapshots: +1 added");
+    expect(await Bun.file(join(String(dir), "__snapshots__", "timeout.test.ts.snap")).text()).toBe(
+      '// Bun Snapshot v1, https://bun.sh/docs/test/snapshots\n\nexports[`next 1`] = `\n{\n  "from": "next",\n}\n`;\n',
+    );
+    expect(exitCode).toBe(1);
+  });
+
+  it.concurrent("expect() created in a test file that has finished throws the finished-test error", async () => {
+    using dir = tempDir("snapshot-after-file", {
+      "shared.ts": `
+        import type { expect } from "bun:test";
+        export const captured: { value?: ReturnType<typeof expect>; thrower?: ReturnType<typeof expect> } = {};
+      `,
+      "1-create.test.ts": `
+        import { expect, test } from "bun:test";
+        import { captured } from "./shared";
+
+        test("creates", () => {
+          captured.value = expect({ from: "1-create" });
+          captured.thrower = expect(() => {
+            throw new Error("from 1-create");
+          });
+        });
+      `,
+      "2-use.test.ts": `
+        import { expect, test } from "bun:test";
+        import { captured } from "./shared";
+
+        test("uses", () => {
+          for (const [name, matcher] of [
+            ["toMatchSnapshot", () => captured.value!.toMatchSnapshot()],
+            ["toThrowErrorMatchingSnapshot", () => captured.thrower!.toThrowErrorMatchingSnapshot()],
+          ] as const) {
+            try {
+              matcher();
+              console.log(name + ": did not throw");
+            } catch (error) {
+              console.log(name + ": " + (error as Error).message);
+            }
+          }
+          expect({ from: "2-use" }).toMatchSnapshot();
+        });
+      `,
+    });
+
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "test", "./1-create.test.ts", "./2-use.test.ts"],
+      cwd: String(dir),
+      env: { ...bunEnv, CI: "false" },
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+
+    expect(normalizeBunSnapshot(stdout)).toMatchInlineSnapshot(`
+      "bun test <version> (<revision>)
+      toMatchSnapshot: expect(received).toMatchSnapshot()
+
+      Matcher error: Snapshot matchers are not supported after the test has finished executing
+
+      toThrowErrorMatchingSnapshot: expect(received).toThrowErrorMatchingSnapshot()
+
+      Matcher error: Snapshot matchers are not supported after the test has finished executing"
+    `);
+    expect(stderr).toContain("snapshots: +1 added");
+    expect(readdirSync(join(String(dir), "__snapshots__"))).toEqual(["2-use.test.ts.snap"]);
+    expect(await Bun.file(join(String(dir), "__snapshots__", "2-use.test.ts.snap")).text()).toBe(
+      '// Bun Snapshot v1, https://bun.sh/docs/test/snapshots\n\nexports[`uses 1`] = `\n{\n  "from": "2-use",\n}\n`;\n',
+    );
+    expect(exitCode).toBe(0);
+  });
+});