bun test: stop truncating .snap files on open, write them per file and on --bail

src/runtime/cli/test/parallel/runner.rs

@@ -701,8 +701,7 @@ fn worker_flush_aggregates(
     ctx: &Command::ContextData,
     cmds: &mut WorkerCommands,
 ) {
-    // Snapshots flush lazily when the next file opens its snapshot file; the
-    // last file each worker ran has no successor to trigger that.
+    // Inline snapshots are written once per process; each `.snap` was written as its file finished.
     if let Some(runner) = crate::test_runner::jest::Jest::runner() {
         let _ = runner.snapshots.write_inline_snapshots().unwrap_or(false);
         let _ = runner.snapshots.write_snapshot_file();

src/runtime/cli/test_command.rs

@@ -1524,6 +1524,7 @@ impl CommandLineReporter {
                         if this.jest.bail == 1 { "" } else { "s" }
                     );
                     Output::flush();
+                    this.write_snapshots_before_bail();
                     this.write_junit_report_if_needed();
                     this.write_timings_if_needed();
                     Global::exit(1);
@@ -1552,6 +1553,15 @@ impl CommandLineReporter {
         Output::print_start_end(bun::start_time(), bun::time::nano_timestamp());
     }
 
+    pub(crate) fn write_snapshots_before_bail(&mut self) {
+        if let Err(err) = self.jest.snapshots.write_inline_snapshots() {
+            Output::err(err, "Failed to write inline snapshots", ());
+        }
+        if let Err(err) = self.jest.snapshots.write_snapshot_file() {
+            Output::err(err, "Failed to write snapshot file", ());
+        }
+    }
+
     /// Like the JUnit report, called before every exit path (including bail) so measured durations aren't lost.
     pub(crate) fn write_timings_if_needed(&mut self) {
         if self.jest.test_options.update_timings
@@ -3300,6 +3310,7 @@ impl TestCommand {
                             reporter.jest.bail,
                             if reporter.jest.bail == 1 { "" } else { "s" }
                         );
+                        reporter.write_snapshots_before_bail();
                         reporter.write_junit_report_if_needed();
                         reporter.write_timings_if_needed();
 
@@ -3406,6 +3417,11 @@ impl TestCommand {
             }
             junit.current_file = Box::default();
         }
+        // Per file, so this file's snapshots are on disk however a later file ends.
+        if let Err(err) = reporter.jest.snapshots.write_snapshot_file() {
+            Output::err(err.name(), "Failed to write snapshot file", ());
+            return Err(err);
+        }
         Ok(())
     }
 }

src/runtime/test_runner/snapshot.rs

@@ -346,8 +346,10 @@ impl Snapshots {
 
     pub(crate) fn write_snapshot_file(&mut self) -> Result<(), Error> {
         if let Some(file) = self._current_file.take() {
+            // The open in `get_snapshot_file` does not truncate, so drop the old tail here.
             file.file
-                .write_all(&self.file_buf)
+                .pwrite_all(&self.file_buf, 0)
+                .and_then(|()| bun_sys::ftruncate(file.file.handle, self.file_buf.len() as i64))
                 .map_err(|_| crate::Error::FailedToWriteSnapshotFile)?;
             let _ = file.file.close();
             self.file_buf.clear();
@@ -886,10 +888,8 @@ impl Snapshots {
             // SAFETY: buf[pos] == 0 written above
             let snapshot_file_path = ZStr::from_buf(&buf[..], pos);
 
-            let mut flags: i32 = bun_sys::O::CREAT | bun_sys::O::RDWR;
-            if self.update_snapshots {
-                flags |= bun_sys::O::TRUNC;
-            }
+            // Never O_TRUNC: the old contents stay on disk until `write_snapshot_file` replaces them.
+            let flags: i32 = bun_sys::O::CREAT | bun_sys::O::RDWR;
             let fd = match bun_sys::open(snapshot_file_path, flags, 0o644) {
                 bun_sys::Result::Ok(fd) => fd,
                 bun_sys::Result::Err(err) => return Ok(bun_sys::Result::Err(err)),
@@ -909,10 +909,6 @@ impl Snapshots {
                 } else {
                     let mut tmp = vec![0u8; length];
                     let _ = file.file.pread_all(&mut tmp, 0).map_err(Error::from)?;
-                    #[cfg(windows)]
-                    {
-                        file.file.seek_to(0).map_err(Error::from)?;
-                    }
                     self.file_buf.extend_from_slice(&tmp);
                 }
             }

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

@@ -1,6 +1,6 @@
-import { expect, test } from "bun:test";
+import { describe, expect, test } from "bun:test";
 import fs from "fs";
-import { bunEnv, bunExe, tmpdirSync } from "harness";
+import { bunEnv, bunExe, tempDir, tmpdirSync } from "harness";
 
 test("it will create a snapshot file and directory if they don't exist", () => {
   const tempDir = tmpdirSync();
@@ -28,3 +28,142 @@ test("it will create a snapshot file and directory if they don't exist", () => {
   expect(exitCode2).toBe(0);
   expect(fs.existsSync(tempDir + "/__snapshots__/new-snapshot.test.ts.snap")).toBe(true);
 });
+
+const header = "// Bun Snapshot v1, https://bun.sh/docs/test/snapshots\n";
+const snapEntry = (value: string) => "\nexports[`snap 1`] = `" + value + "`;\n";
+const writtenSnap = header + snapEntry('"value"');
+// Longer than what a run of the fixtures below writes, so a rewrite has to shrink the file.
+const staleSnap = header + snapEntry('"stale"') + '\nexports[`gone 1`] = `"stale"`;\n';
+
+const snapTest = /*js*/ `
+  import { test, expect } from "bun:test";
+  test("snap", () => { expect("value").toMatchSnapshot(); });
+`;
+const snapThenFailingTest = /*js*/ `
+  import { test, expect } from "bun:test";
+  test("snap", () => { expect("value").toMatchSnapshot(); });
+  test("fails", () => { expect(1).toBe(2); });
+`;
+const snapThenExitTest = /*js*/ `
+  import { test, expect } from "bun:test";
+  test("snap", () => { expect("value").toMatchSnapshot(); process.exit(0); });
+`;
+const failingTest = /*js*/ `
+  import { test, expect } from "bun:test";
+  test("fails", () => { expect(1).toBe(2); });
+`;
+const inlineTest = (snapshot: string) => /*js*/ `
+  import { test, expect } from "bun:test";
+  test("inline", () => { expect("value").toMatchInlineSnapshot(${snapshot}); });
+`;
+const inlineThenFailingTest = (snapshot: string) => /*js*/ `
+  import { test, expect } from "bun:test";
+  test("inline", () => { expect("value").toMatchInlineSnapshot(${snapshot}); });
+  test("fails", () => { expect(1).toBe(2); });
+`;
+
+async function runBunTest(dir: string, args: string[], env: Record<string, string> = {}) {
+  await using proc = Bun.spawn({
+    cmd: [bunExe(), "test", ...args],
+    cwd: String(dir),
+    env: { ...bunEnv, CI: "false", ...env },
+    stdout: "pipe",
+    stderr: "pipe",
+  });
+  const [, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+  return { stderr, exitCode };
+}
+
+describe("writing the .snap file", () => {
+  test.concurrent("--update-snapshots keeps the existing file until the new contents are written", async () => {
+    using dir = tempDir("snapshot-exit-before-write", {
+      "a.test.ts": snapThenExitTest,
+      "__snapshots__": { "a.test.ts.snap": staleSnap },
+    });
+    const { exitCode } = await runBunTest(dir, ["--update-snapshots", "./a.test.ts"]);
+    expect(fs.readFileSync(`${dir}/__snapshots__/a.test.ts.snap`, "utf8")).toBe(staleSnap);
+    expect(exitCode).toBe(0);
+  });
+
+  test.concurrent("--update-snapshots removes what the existing file had beyond the new contents", async () => {
+    using dir = tempDir("snapshot-update-shrinks", {
+      "a.test.ts": snapTest,
+      "__snapshots__": { "a.test.ts.snap": staleSnap },
+    });
+    const { stderr, exitCode } = await runBunTest(dir, ["--update-snapshots", "./a.test.ts"]);
+    expect(fs.readFileSync(`${dir}/__snapshots__/a.test.ts.snap`, "utf8")).toBe(writtenSnap);
+    expect(stderr).toContain("1 pass");
+    expect(exitCode).toBe(0);
+  });
+
+  test.concurrent("a file's snapshots are written when it finishes, not when the run ends", async () => {
+    using dir = tempDir("snapshot-written-per-file", {
+      "a.test.ts": snapTest,
+      "b.test.ts": `process.exit(1);`,
+    });
+    const { exitCode } = await runBunTest(dir, ["./a.test.ts", "./b.test.ts"]);
+    expect(fs.readFileSync(`${dir}/__snapshots__/a.test.ts.snap`, "utf8")).toBe(writtenSnap);
+    expect(exitCode).toBe(1);
+  });
+});
+
+describe("--bail", () => {
+  async function runUntilBail(dir: string, ...args: string[]) {
+    const { stderr, exitCode } = await runBunTest(dir, ["--bail", ...args], {
+      // Bailing on a failed test exits the process without tearing the VM down, so the
+      // test runner's JSC-owned objects show up as leaks. These tests cover what reaches
+      // disk before that exit, so keep ASAN on but leave LSAN off for the child.
+      ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "detect_leaks=0"].filter(Boolean).join(":"),
+    });
+    expect(stderr).toContain("Bailed out after 1 failure");
+    expect(exitCode).toBe(1);
+  }
+
+  test.concurrent("writes the snapshots recorded in the file that bailed", async () => {
+    using dir = tempDir("snapshot-bail", { "a.test.ts": snapThenFailingTest });
+    await runUntilBail(dir, "./a.test.ts");
+    expect(fs.readFileSync(`${dir}/__snapshots__/a.test.ts.snap`, "utf8")).toBe(writtenSnap);
+  });
+
+  test.concurrent("--update-snapshots replaces the existing file with the snapshots recorded so far", async () => {
+    using dir = tempDir("snapshot-bail-update", {
+      "a.test.ts": snapThenFailingTest,
+      "__snapshots__": { "a.test.ts.snap": staleSnap },
+    });
+    await runUntilBail(dir, "--update-snapshots", "./a.test.ts");
+    expect(fs.readFileSync(`${dir}/__snapshots__/a.test.ts.snap`, "utf8")).toBe(writtenSnap);
+  });
+
+  test.concurrent("writes the pending inline snapshots of the file that bailed", async () => {
+    using dir = tempDir("snapshot-bail-inline", { "a.test.ts": inlineThenFailingTest("") });
+    await runUntilBail(dir, "./a.test.ts");
+    expect(fs.readFileSync(`${dir}/a.test.ts`, "utf8")).toBe(inlineThenFailingTest('`"value"`'));
+  });
+
+  test.concurrent("keeps the snapshots of an earlier file when a later file's test fails", async () => {
+    using dir = tempDir("snapshot-bail-earlier-file", { "a.test.ts": snapTest, "b.test.ts": failingTest });
+    await runUntilBail(dir, "./a.test.ts", "./b.test.ts");
+    expect(fs.readFileSync(`${dir}/__snapshots__/a.test.ts.snap`, "utf8")).toBe(writtenSnap);
+  });
+
+  test.concurrent("keeps the snapshots of an earlier file when a later file fails to load", async () => {
+    using dir = tempDir("snapshot-bail-load-error", {
+      "a.test.ts": snapTest,
+      "b.test.ts": `throw new Error("failed to load");`,
+    });
+    await runUntilBail(dir, "./a.test.ts", "./b.test.ts");
+    expect(fs.readFileSync(`${dir}/__snapshots__/a.test.ts.snap`, "utf8")).toBe(writtenSnap);
+  });
+
+  test.concurrent(
+    "writes the pending inline snapshots of an earlier file when a later file fails to load",
+    async () => {
+      using dir = tempDir("snapshot-bail-load-error-inline", {
+        "a.test.ts": inlineTest(""),
+        "b.test.ts": `throw new Error("failed to load");`,
+      });
+      await runUntilBail(dir, "./a.test.ts", "./b.test.ts");
+      expect(fs.readFileSync(`${dir}/a.test.ts`, "utf8")).toBe(inlineTest('`"value"`'));
+    },
+  );
+});