Fix snapshot keys being inconsistent with --rerun-each by robobun · Pull Request #23707 · oven-sh/bun

robobun · 2025-10-15T21:35:09Z

Summary

Fixes snapshot keys being incremented incorrectly when using --rerun-each
Adds regression test in test/regression/issue/23705

Problem

When using --rerun-each, snapshot counters were not being reset between test reruns. This caused:

Run 1: Snapshot key = "test name 1" ✅
Run 2: Snapshot key = "test name 2" ❌ (should be "test name 1")

This triggered CI errors because Bun tried to create a new snapshot on the second run, which is blocked in CI environments.

Solution

Added resetCountsForRerun() method to clear snapshot counts when starting a rerun (repeat_index > 0). This ensures snapshot keys remain consistent across all runs.

Changes:

src/bun.js/test/snapshot.zig: Added resetCountsForRerun() method
src/cli/test_command.zig: Call resetCountsForRerun() before each rerun
test/regression/issue/23705/23705.test.ts: Regression tests for both inline and file snapshots

Test plan

✅ New regression tests pass with the fix
✅ Existing rerun-each.test.ts tests still pass
✅ Existing snapshot tests still pass

Fixes #23705

🤖 Generated with Claude Code

robobun · 2025-10-15T21:35:19Z

^{Updated 3:57 PM PT - Oct 30th, 2025}

❌ @pfgithub, your commit b3a85ce has 3 failures in Build #30566 (All Failures):

test/v8/v8.test.ts - 1 failing on 🐧 12 x64-asan
test/bake/dev/stress.test.ts - 1 failing on 🐧 12 x64-baseline
test/integration/vite-build/vite-build.test.ts - core dumped on 🐧 12 x64-asan

🧪 To try this PR locally:

bunx bun-pr 23707

That installs a local version of the PR into your bun-23707 executable, so you can run:

bun-23707 --bun

coderabbitai · 2025-10-15T21:35:27Z

Walkthrough

Sets the snapshot writer's current file to null after freeing snapshot structures, removes a preemptive write when switching snapshot files, calls the snapshot writer during test reruns to reset state, and adds end-to-end tests validating snapshot-key consistency with --rerun-each.

Changes

Cohort / File(s)	Summary of changes
Snapshot writer lifecycle `src/bun.js/test/snapshot.zig`	`writeSnapshotFile` now sets `this._current_file` to `null` after freeing structures. `getSnapshotFile` guard simplified: it no longer preemptively calls `writeSnapshotFile` when the current file id differs.
Test rerun snapshot flush `src/cli/test_command.zig`	Adds a call to `reporter.jest.snapshots.writeSnapshotFile()` in the test rerun path (after `vm.auto_killer.disable()`), ensuring snapshot state is flushed between reruns.
Regression tests: rerun snapshot keys `test/regression/issue/23705/23705.test.ts`	Adds end-to-end regression tests that run an initial `--update-snapshots` pass then a CI-mode `--rerun-each=2` pass for inline and file snapshots, asserting consistent snapshot keys and no snapshot creation on rerun; verifies snapshot file creation after the update run.

Possibly related PRs

Enable breaking_changes_1_3 #23308: Modifies src/bun.js/test/snapshot.zig and snapshot creation/control-flow logic (strong overlap).
bun test dots reporter #22919: Changes reporter wiring in src/cli/test_command.zig (overlaps test-command/reporter integration).
Prevent test.only and snapshot updates in CI #21811: Touches snapshot write/control-flow code in src/bun.js/test/snapshot.zig (related lifecycle paths).

Suggested reviewers

nektro

Pre-merge checks

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The pull request title "Fix snapshot keys being inconsistent with --rerun-each" is clear, concise, and directly addresses the primary change in this PR. It accurately summarizes the main objective of fixing snapshot key inconsistency during test reruns, avoiding vague terminology and clearly communicating the scope and nature of the fix to someone reviewing the repository history.
Linked Issues Check	✅ Passed	The pull request directly addresses all coding requirements from the linked issue #23705. The implementation clears snapshot counts between test reruns to ensure consistent snapshot keys across multiple runs, adds regression tests validating both inline and file snapshots with the --rerun-each flag, and modifies the appropriate files (snapshot.zig and test_command.zig) to implement the fix. The changes ensure that snapshot keys remain unchanged across reruns instead of being incremented incorrectly, which was the core problem described in the issue.
Out of Scope Changes Check	✅ Passed	All code changes in this pull request are directly within scope and focused on resolving the --rerun-each snapshot inconsistency issue. The modifications to src/bun.js/test/snapshot.zig handle snapshot state reset, changes to src/cli/test_command.zig trigger that reset during reruns, and the new regression test file validates the fix for both inline and file snapshots. No extraneous changes or unrelated features have been introduced.
Description Check	✅ Passed	The pull request description comprehensively covers the required template sections despite not using the exact template headings. It clearly explains what the PR does through the Summary, Problem, and Solution sections, and demonstrates verification through the detailed Test plan section that references both new regression tests and existing tests that continue to pass. The description includes a problem statement explaining the bug, the implemented solution with specific code locations, and comprehensive test coverage, making it substantially complete and informative for review.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between bacc77c and b3a85ce.

📒 Files selected for processing (2)

src/bun.js/test/snapshot.zig (2 hunks)
src/cli/test_command.zig (1 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)

**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue

Files:

src/bun.js/test/snapshot.zig
src/cli/test_command.zig

src/bun.js/**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)

src/bun.js/**/*.zig: In Zig binding structs, expose generated bindings via pub const js = JSC.Codegen.JS and re-export toJS/fromJS/fromJSDirect
Constructors and prototype methods should return bun.JSError!JSC.JSValue to integrate Zig error handling with JS exceptions
Use parameter name globalObject (not ctx) and accept (*JSC.JSGlobalObject, *JSC.CallFrame) in binding methods/constructors
Implement getters as get(this, globalObject) returning JSC.JSValue and matching the .classes.ts interface
Provide deinit() for resource cleanup and finalize() that calls deinit(); use bun.destroy(this) or appropriate destroy pattern
Access JS call data via CallFrame (argument(i), argumentCount(), thisValue()) and throw errors with globalObject.throw(...)
For properties marked cache: true, use the generated Zig accessors (NameSetCached/GetCached) to work with GC-owned values
In finalize() for objects holding JS references, release them using .deref() before destroy

Files:

src/bun.js/test/snapshot.zig

src/**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)

When adding debug logs in Zig, create a scoped logger and log via Bun APIs: const log = bun.Output.scoped(.${SCOPE}, .hidden); then log("...", .{})

src/**/*.zig: Use Zig private fields with the # prefix for encapsulation (e.g., struct { #foo: u32 })
Prefer Decl literals for initialization (e.g., const decl: Decl = .{ .binding = 0, .value = 0 };)
Place @import statements at the bottom of the file (formatter will handle ordering)

src/**/*.zig: In Zig code, manage memory carefully: use appropriate allocators and defer for cleanup
Cache JavaScriptCore class structures in ZigGlobalObject when adding new classes

Files:

src/bun.js/test/snapshot.zig
src/cli/test_command.zig

🧠 Learnings (15)

📓 Common learnings

Learnt from: pfgithub
Repo: oven-sh/bun PR: 23117
File: src/bun.js/test/snapshot.zig:265-276
Timestamp: 2025-09-30T22:53:19.887Z
Learning: In Bun's snapshot testing (src/bun.js/test/snapshot.zig), multiple inline snapshots at the same line and column (same call position) must have identical values. However, multiple inline snapshots on the same line at different columns are allowed to have different values. The check is position-specific (line+col), not line-wide.

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-26T05:04:50.692Z
Learning: Applies to test/**/*.test.{ts,tsx} : Prefer snapshot assertions and use normalizeBunSnapshot for snapshot output in tests

📚 Learning: 2025-09-30T22:53:19.887Z

Learnt from: pfgithub
Repo: oven-sh/bun PR: 23117
File: src/bun.js/test/snapshot.zig:265-276
Timestamp: 2025-09-30T22:53:19.887Z
Learning: In Bun's snapshot testing (src/bun.js/test/snapshot.zig), multiple inline snapshots at the same line and column (same call position) must have identical values. However, multiple inline snapshots on the same line at different columns are allowed to have different values. The check is position-specific (line+col), not line-wide.

Applied to files:

src/bun.js/test/snapshot.zig
src/cli/test_command.zig

📚 Learning: 2025-10-26T05:04:50.692Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-26T05:04:50.692Z
Learning: Applies to test/**/*.test.{ts,tsx} : Prefer snapshot assertions and use normalizeBunSnapshot for snapshot output in tests

Applied to files:

src/bun.js/test/snapshot.zig
src/cli/test_command.zig

📚 Learning: 2025-08-30T00:13:36.815Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : For properties marked cache: true, use the generated Zig accessors (<Prop>NameSetCached/GetCached) to work with GC-owned values

Applied to files:

src/bun.js/test/snapshot.zig

📚 Learning: 2025-08-30T00:13:36.815Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Provide deinit() for resource cleanup and finalize() that calls deinit(); use bun.destroy(this) or appropriate destroy pattern

Applied to files:

src/bun.js/test/snapshot.zig

📚 Learning: 2025-08-30T00:13:36.815Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/bindings/generated_classes_list.zig : Update src/bun.js/bindings/generated_classes_list.zig to include new classes

Applied to files:

src/bun.js/test/snapshot.zig

📚 Learning: 2025-08-30T00:09:39.100Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/{dev/*.test.ts,dev-and-prod.ts} : Do not use node:fs APIs in tests; mutate files via dev.write, dev.patch, and dev.delete

Applied to files:

src/bun.js/test/snapshot.zig

📚 Learning: 2025-08-30T00:13:36.815Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : In Zig binding structs, expose generated bindings via pub const js = JSC.Codegen.JS<ClassName> and re-export toJS/fromJS/fromJSDirect

Applied to files:

src/bun.js/test/snapshot.zig

📚 Learning: 2025-08-30T00:13:36.815Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : In finalize() for objects holding JS references, release them using .deref() before destroy

Applied to files:

src/bun.js/test/snapshot.zig

📚 Learning: 2025-08-30T00:13:36.815Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Use parameter name globalObject (not ctx) and accept (*JSC.JSGlobalObject, *JSC.CallFrame) in binding methods/constructors

Applied to files:

src/bun.js/test/snapshot.zig

📚 Learning: 2025-10-19T03:01:29.084Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:286-289
Timestamp: 2025-10-19T03:01:29.084Z
Learning: In src/bun.js/telemetry.zig, the guard preventing double configuration (lines 213-216) is intentional. The telemetry API uses a single-shot configuration model where configure() is called once during application startup. Users must call Bun.telemetry.configure(null) to reset before reconfiguring. This design ensures: (1) predictable state—callbacks don't change mid-request, avoiding race conditions; (2) zero overhead when disabled—no checking for callback changes on every request; (3) clear ownership—one adapter (e.g., bun-otel) owns the telemetry config. Merge-style reconfiguration would require atomic updates during active requests, adding overhead to the hot path.
<!-- [/add_learning]

Applied to files:

src/bun.js/test/snapshot.zig

📚 Learning: 2025-10-12T02:22:34.373Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use utilities like `describe.each`, `toMatchSnapshot`, and lifecycle hooks (`beforeAll`, `beforeEach`, `afterEach`) and track resources for cleanup

Applied to files:

src/cli/test_command.zig

📚 Learning: 2025-10-12T02:22:34.373Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : Always check exit codes and error scenarios in tests (e.g., spawned processes should assert non-zero on failure)

Applied to files:

src/cli/test_command.zig

📚 Learning: 2025-08-30T00:12:56.803Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/**/*.{js,ts} : Prefer data-driven tests (e.g., test.each) to reduce boilerplate

Applied to files:

src/cli/test_command.zig

📚 Learning: 2025-10-12T02:22:34.373Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`

Applied to files:

src/cli/test_command.zig

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: Format

🔇 Additional comments (3)

src/cli/test_command.zig (1)

1972-1974: LGTM! Correctly resets snapshot state between reruns.

This call ensures snapshot counters are cleared between iterations when using --rerun-each, fixing the issue where snapshot keys would increment (e.g., "test name 1" → "test name 2"). Calling writeSnapshotFile() at the end of each repeat iteration clears counts, writes accumulated snapshots to disk, and nulls _current_file, allowing the next iteration to start with a clean counter state.

src/bun.js/test/snapshot.zig (2)

210-211: LGTM! Good defensive programming.

Setting _current_file to null after cleanup prevents use-after-close bugs and makes the state explicit. This pairs well with the simplified initialization check in getSnapshotFile() (line 487), where null now clearly signals that initialization is needed.

487-487: LGTM! Simplified initialization guard.

The simplified null-check replaces implicit file-switching logic with explicit control via writeSnapshotFile() calls. This works correctly because:

writeSnapshotFile() is called after each repeat iteration (test_command.zig:1974), including the last one

After the last repeat, _current_file is null when moving to the next test file

Explicit reset is clearer than automatic switching based on file ID

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

When using --rerun-each, snapshot counters were not being reset between test reruns. This caused the second run to generate snapshot keys like "test name 2" instead of reusing "test name 1", triggering CI errors when snapshot creation is blocked. The fix unconditionally calls writeSnapshotFile() after each test file execution, which writes the snapshot file, closes it, clears all state (including counts), and sets _current_file to null. This ensures the next rerun starts fresh and generates consistent snapshot keys. This also simplifies getSnapshotFile() by removing the file_id check (no longer needed since _current_file is always null when entering that branch) and the redundant writeSnapshotFile() call (which would be a no-op since _current_file is null). Fixes #23705 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

github-actions · 2026-02-19T08:23:53Z

Closing this PR because it has been inactive for more than 90 days.

github-actions Bot added the claude label Oct 15, 2025

robobun force-pushed the claude/fix-snapshot-rerun-each-23705 branch from 093bdd2 to e98fa47 Compare October 15, 2025 22:01

pfgithub mentioned this pull request Oct 15, 2025

Fix issue #23705 #23711

Closed

robobun force-pushed the claude/fix-snapshot-rerun-each-23705 branch 2 times, most recently from f2ffa0a to c3d32f9 Compare October 15, 2025 23:47

robobun force-pushed the claude/fix-snapshot-rerun-each-23705 branch from c3d32f9 to bacc77c Compare October 16, 2025 00:01

pfgithub self-assigned this Oct 28, 2025

Merge branch 'main' into claude/fix-snapshot-rerun-each-23705

b3a85ce

github-actions Bot closed this Feb 19, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix snapshot keys being inconsistent with --rerun-each#23707

Fix snapshot keys being inconsistent with --rerun-each#23707
robobun wants to merge 2 commits into
mainfrom
claude/fix-snapshot-rerun-each-23705

robobun commented Oct 15, 2025

Uh oh!

robobun commented Oct 15, 2025 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 15, 2025 •

edited

Loading

Uh oh!

github-actions Bot commented Feb 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

robobun commented Oct 15, 2025

Summary

Problem

Solution

Test plan

Uh oh!

robobun commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Possibly related PRs

Suggested reviewers

Pre-merge checks

Uh oh!

github-actions Bot commented Feb 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

robobun commented Oct 15, 2025 •

edited

Loading

coderabbitai Bot commented Oct 15, 2025 •

edited

Loading