bun test: flush .snap file before afterAll runs

Closed
RO
10d ago
+153-8

Fixes #12114.

Repro

const fs = require("node:fs");
test("make a dummy snapshot", () => {
  expect({ dummy: "snapshot" }).toMatchSnapshot();
});
afterAll(() => {
  console.log("SNAPSHOT-LEN:", fs.readFileSync("./__snapshots__/dummy.test.js.snap").toString().length);
});
$ bun test --update-snapshots dummy.test.js
(pass) make a dummy snapshot
SNAPSHOT-LEN: 0          # file populated only after process exit

Cause

Snapshots::get_snapshot_file opened the .snap file with O_TRUNC under --update-snapshots, and write_snapshot_file only ran when the next test file opened its own snapshot or at process exit. Between those two points (which includes every afterAll in the file) the on-disk file was zero bytes. The first-create path had the same shape: O_CREAT made an empty file that wasn't written until later.

Fix

src/runtime/test_runner/snapshot.rs

  • Drop O_TRUNC from the open flags; the old bytes stay on disk until we overwrite them.
  • Track needs_write (set when get_or_put appends a new entry, or under -u when the buffer is rebuilt from a clean header) and add flush_current_file, which does seek(0) + write_all + ftruncate(len) and leaves the fd open so a later toMatchSnapshot can keep appending.
  • write_snapshot_file now calls flush_current_file and then closes/clears. A passing run with no added snapshots no longer rewrites an unchanged file.

src/runtime/test_runner/Execution.rs

  • step_group flushes the snapshot file whenever a hook-only group (beforeAll/afterAll, any describe depth) is about to start. needs_write makes the beforeAll case and repeated flushes no-ops. The flush is best-effort here; a persistent write error still surfaces via the end-of-run write_snapshot_file()?.

Verification

$ bun test --update-snapshots dummy.test.js
(pass) make a dummy snapshot
SNAPSHOT-LEN: 125

New tests in test/js/bun/test/snapshot-tests/new-snapshot.test.ts: the #12114 repro on first create, -u with a shorter value than the existing snapshot (exercises the ftruncate path and checks the exact body seen by afterAll against the final file), a two-file -u run where each file's afterAll reads its own .snap, and a nested describe whose inner and outer afterAll both read the flushed file. All fail on main and pass with this change. bun_test.test.ts, ci-restrictions.test.ts, test-filter-lifecycle-snapshot.test.ts, and the rest of snapshot-tests/ pass unchanged.

#34042 also touches this area as part of obsolete-snapshot detection; this PR is the narrow fix for the afterAll visibility bug and should rebase cleanly onto it in either direction.


[review] gate passed · iteration 0 · 4 files touched

fails on main (without fix)
ASAN without fix: 3 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/bun/test/snapshot-tests/new-snapshot.test.ts
bun test v1.4.0 (13576e4ee)

test/js/bun/test/snapshot-tests/new-snapshot.test.ts:
(pass) it will create a snapshot file and directory if they don't exist [1155.92ms]
67 |   {
68 |     const { stderr, exitCode, seen } = await run(longValue);
69 |     expect(stderr).toContain("1 pass");
70 |     const onDisk = fs.readFileSync(snapPath, "utf8");
71 |     expect(onDisk).toContain(longValue);
72 |     expect(seen).toEqual({ len: onDisk.length, body: onDisk });
                      ^
error: expect(received).toEqual(expected)

  {
-   "body": 
- "// Bun Snapshot v1, https://bun.sh/docs/test/snapshots
- 
- exports[`make a snapshot 1`] = `"long-initial-value-xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"`;
- "
- ,
-   "len": 172,
+   "body": "",
+   "len": 0,
  }

- Expected  - 7
+ Received  + 2

      at <anonymous> (/workspace/bun/test/js/bun/test/snapshot-tests/new-snapshot.test.ts:72:18)
(fail) afterAll sees the written .snap file under --update-snapshots [1377.10ms]
142 
... (truncated)

release without fix: 3 FAILED
bun test v1.4.0-canary.1 (1498d7b77)

test/js/bun/test/snapshot-tests/new-snapshot.test.ts:
(pass) it will create a snapshot file and directory if they don't exist [366.57ms]
67 |   {
68 |     const { stderr, exitCode, seen } = await run(longValue);
69 |     expect(stderr).toContain("1 pass");
70 |     const onDisk = fs.readFileSync(snapPath, "utf8");
71 |     expect(onDisk).toContain(longValue);
72 |     expect(seen).toEqual({ len: onDisk.length, body: onDisk });
                      ^
error: expect(received).toEqual(expected)

  {
-   "body": 
- "// Bun Snapshot v1, https://bun.sh/docs/test/snapshots
- 
- exports[`make a snapshot 1`] = `"long-initial-value-xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"`;
- "
- ,
-   "len": 172,
+   "body": "",
+   "len": 0,
  }

- Expected  - 7
+ Received  + 2

      at <anonymous> (/workspace/bun/test/js/bun/test/snapshot-tests/new-snapshot.test.ts:72:18)
(fail) afterAll sees the written .snap file under --update-snapshots [283.90ms]
110 |     env: { ...bunEnv, CI: "false" },
111 |     cwd: String(dir),
112 |     stdout: "pipe",
113 |     stderr: "pipe",
114 |   });
115 |   const [stdout, stderr, exitCode] = await
... (truncated)
passes on PR (with fix)
ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/bun/test/snapshot-tests/new-snapshot.test.ts
bun test v1.4.0 (13576e4ee)

test/js/bun/test/snapshot-tests/new-snapshot.test.ts:
(pass) it will create a snapshot file and directory if they don't exist [1385.09ms]
(pass) describe-level afterAll sees the written .snap file [1550.67ms]
(pass) afterAll sees the written .snap file across multiple test files [1940.46ms]
(pass) afterAll sees the written .snap file under --update-snapshots [2977.51ms]

 4 pass
 0 fail
 22 expect() calls
Ran 4 tests across 1 file. [10.67s]
__F:0:S:0

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped)
  target       linux-x64-gnu
  build type   Release
  build dir    ./build/release
  revision     13576e4eea
  features     baseline

22 deps, 108 codegen, 1171 objects in 4046ms

ninja: Entering directory `/workspace/bun/build/release'
[1/1234] gen ProcessBindingConstants.lut.h
Generating /workspace/bun/build/release/codegen/ProcessBindingConstants.lut.h from /workspace/bun/src/jsc/bindings/ProcessBindingConstants.cpp
[2/1234] fetch libjpeg-turbo
[libjpeg-turbo] up to date
[3/1234] fetch picohttpparser
[picohttpparser] up to date
[4/1234] gen JSBuffer.lut.h
Generating /workspace/bun/build/release/codegen/JSBuffer.lut.h from /workspace/bun/src/jsc/bindings/JSBuffer.cpp
[5/1234] gen ProcessBindingBuffer.lut.h
Generating /workspace/bun/build/release/codegen/ProcessBindingBuffer.lut.h from /workspace/bun/src/jsc/bindings/ProcessBindingBuffer.cpp
[6/1234] fetch zlib
[zlib] up to date
[7/1234] gen .bind.ts → GeneratedBindings.cpp
[8/1234] subst deps/libjpeg-turbo/jconfigint.h
[9/1234] fetch tinycc
[tinycc] up to date
[10/1234] gen ErrorCode+*.h
[11/1234] fetch nodejs (prebuilt)
[nodejs
... (truncated)
diff hotspot
src/runtime/cli/test_command.rs                    |   1 +
 src/runtime/test_runner/Execution.rs               |   7 ++
 src/runtime/test_runner/snapshot.rs                |  31 ++++--
 .../bun/test/snapshot-tests/new-snapshot.test.ts   | 122 ++++++++++++++++++++-
 4 files changed, 153 insertions(+), 8 deletions(-)

gate history · 1 passed · 0 rejected · iteration 0

evidence per changed file
file                                                  reads  edits  tests
src/runtime/cli/test_command.rs                           3      1      0
src/runtime/test_runner/Execution.rs                      8     12      0
src/runtime/test_runner/snapshot.rs                       5      7      0
test/js/bun/test/snapshot-tests/new-snapshot.test.ts      4      4      0