test: make the error snapshots test pass with colors disabled

[robobun]

Problem

• bun bd test test/js/bun/test/snapshot-tests/snapshots/snapshot.test.ts fails on main when run the way CLAUDE.md tells contributors to run it (stdout and stderr piped, no FORCE_COLOR in the environment):

  (fail) error snapshots
  - "\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
  

• Cause: the last assertion of "error snapshots" (snapshot.test.ts:893) inline-snapshots the nested matcher error in its ANSI-colored form, so it only holds when the test process itself has colors enabled.
• CI is green because scripts/runner.node.mjs:1785 spawns every test file with FORCE_COLOR=1; a run with both stdio streams piped has colors off and gets the uncolored message.
• It is the only color dependent assertion in test/js/bun/test/snapshot-tests/, and it keeps getting in the way: test: fix toMatchInlineSnapshot through a same-file tail-call helper #34978, CallFrame: make CallerSrcLoc own the source URL ref it is handed (Bun.cron leak) #37461, Compare functions by identity in toMatchObject and Bun.deepMatch #38339, bun test: fix the reason reported when a snapshot matcher's expect() has no running test #38799 and bun test: name snapshots taken in beforeEach/afterEach after the running test #38820 each had to note in their verification that this one case needs FORCE_COLOR=1, and test: fix toMatchInlineSnapshot through a same-file tail-call helper #34978 carries a Bun.enableANSIColors branch around it in its diff.

Fix

• Test only change. The nested matcher error is caught, Bun.stripANSI is applied to its message, and it is rethrown; the inline snapshot becomes the uncolored text. Same idiom as test/js/bun/test/mock-fn.test.js (toHaveReturned failure message stripped, then inline-snapshotted); rethrowing keeps the outer toThrowErrorMatchingInlineSnapshot in the picture, which is what this test is about.
• Correct because the assertion exists to check that toThrowErrorMatchingInlineSnapshot on a function that does not throw raises Matcher error: Received function did not throw. Whether that message carries escape codes is decided once per process from FORCE_COLOR / NO_COLOR / a TTY on either stdio stream (src/bun_core/output.rs), not by the matcher, so it does not belong in the expectation. toThrowErrorMatchingInlineSnapshot snapshots the thrown error's message property (fn_to_err_string_or_undefined, src/runtime/test_runner/expect.rs), so the stripped message is exactly what the outer snapshot compares, and it is byte for byte the same with colors on and off.
• Nothing is left unpinned by dropping the colored literal: the colored matcher header bytes are already inline-snapshotted by test/js/bun/test/printing/diffexample.test.ts ("color"), which spawns a child with an explicit FORCE_COLOR=1 instead of depending on the test process's own environment; the bold on Matcher error is generic output markup, not behavior of this matcher.
• No runtime change is warranted: matcher output being colored whenever bun test has colors enabled is the intended behavior, and whether snapshots themselves should strip color is a separate open question (NO_COLOR 環境變數不影響 snapshot 輸出 #28439; Strip ANSI escape codes from snapshot content and honor NO_COLOR from env files #28443 attempted it and was not merged). This assertion holds either way that is decided.
• Once this lands, the Bun.enableANSIColors branch in test: fix toMatchInlineSnapshot through a same-file tail-call helper #34978 can be dropped when it rebases.
• Verified with the debug build, all on this file:
  • unfixed test, colors off: 61 pass, 1 fail (error snapshots), exit 1
  • fixed test, colors off: 62 pass, 0 fail
  • fixed test, FORCE_COLOR=1 (what CI does): 62 pass, 0 fail
  • fixed test, NO_COLOR=1 (release binary): 62 pass, 0 fail
  • snapshots: 111 passed, 1 failed in every passing run, the same counts as main under FORCE_COLOR=1 (the one failing snapshot is the intentional one inside toThrow() in "indented inline snapshots")

Background

• bun test decides once at startup whether to emit ANSI colors: FORCE_COLOR wins, then NO_COLOR, then whether either stdout or stderr is a terminal. When colors are on, the matcher errors that expect throws (the expect(received).matcher() header, the Matcher error label) carry the escape codes inside error.message, as Jest's chalk based messages do.
• toThrowErrorMatchingInlineSnapshot(fn) calls fn, takes .message off whatever it threw (undefined for a non-Error), and compares that string against the inline snapshot. Nesting two of them, as this assertion does, snapshots the inner matcher's error text verbatim.
• Bun.stripANSI removes ANSI escape sequences from a string and leaves a string without any unchanged, which is why one expectation serves both color modes.

[robobun]

^{Updated 11:05 PM PT - Aug 14th, 2026}

@robobun, your commit f702991 is building: #97159

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 5 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: d26d4763-521a-4582-875b-7673f86dcf74

📥 Commits

Reviewing files that changed from the base of the PR and between e9b5e63 and b0dbab8.

📒 Files selected for processing (1)

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

---

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

[robobun]

Status: fix pushed (b0dbab8), waiting on CI.

Reproduced on main (2c2ef7c and a5c86ae) with bun bd test test/js/bun/test/snapshot-tests/snapshots/snapshot.test.ts run with stdout and stderr piped and no FORCE_COLOR: 61 pass, 1 fail (error snapshots). With this branch the same command passes with colors off, with FORCE_COLOR=1 (CI's setting), and with NO_COLOR=1: 62 pass, 0 fail in all three.

Self-review found nothing against the code. It corrected two statements in the description (an issue number copied from another PR that turned out to be unrelated, and a claim that #38799/#38820 moved tests because of this case, when they only noted it in their verification) and removed a comment from the diff whose wording about piped runs was imprecise.

[claude]

LGTM — test-only normalization that makes the assertion environment-independent.

What was reviewed:

• The rethrown error's .message is what toThrowErrorMatchingInlineSnapshot snapshots, so stripping there is exactly what the outer matcher sees.
• Bun.stripANSI is idempotent on uncolored input, so one snapshot serves both FORCE_COLOR=1 (CI) and piped/NO_COLOR runs.
• The assertion still checks the full matcher-error text ("Received function did not throw"); only the environment-dependent escape codes are dropped, so no coverage is lost.

Extended reasoning...

Overview

Single-file test-only change to test/js/bun/test/snapshot-tests/snapshots/snapshot.test.ts. The last assertion in the "error snapshots" test previously inline-snapshotted a nested matcher error in its ANSI-colored form, which only matches when the test process has colors enabled (CI sets FORCE_COLOR=1; a local piped bun bd test does not). The fix wraps the inner expect(() => {}).toThrowErrorMatchingInlineSnapshot(...) in a try/catch, applies Bun.stripANSI to the caught error's .message, rethrows, and updates the inline snapshot to the uncolored text.

Security risks

None. Test-only change, no runtime code touched, no external input handling.

Level of scrutiny

Low. This is a targeted de-flaking of one assertion in a test file. The change is mechanical and follows an established pattern — Bun.stripANSI is used the same way in several other tests (inspect-error.test.js, tty.test.ts, etc.) to normalize color-dependent output. The updated snapshot is the exact same text minus escape sequences, so the property under test (that the matcher raises "Received function did not throw") is preserved at full precision. Per REVIEW.md's de-flaking guidance ("keep asserting the property the original assertion protected"), this is the correct shape: normalize the environment-dependent part rather than branching or loosening to toContain.

Other factors

• Bun.stripANSI is a documented public API (packages/bun-types/bun.d.ts) and is the identity on strings without escape codes, so the assertion holds under both color modes without branching.
• The outer toThrowErrorMatchingInlineSnapshot reads the thrown value's .message property, which is exactly what the catch block mutates — the rethrow path is correct.
• The PR description documents verification under plain env, FORCE_COLOR=1, and NO_COLOR=1 on the debug build, all passing with the same snapshot counts as main.
• No prior reviews from me on this PR; no outstanding reviewer comments.

[claude]

LGTM — test-only fix that makes the assertion environment-independent without weakening it.

What was reviewed:

• The stripped snapshot still asserts the exact matcher error text (Matcher error: Received function did not throw); only the environment-dependent ANSI codes are removed, so the property under test is preserved.
• Confirmed this was the only ANSI-escape-bearing assertion in test/js/bun/test/snapshot-tests/; no sibling sites need the same treatment.
• Bun.stripANSI on the caught error's .message before rethrow matches the pattern used elsewhere in the test suite and is idempotent when colors are already off.

Extended reasoning...

Overview

Single-file, test-only change to test/js/bun/test/snapshot-tests/snapshots/snapshot.test.ts. The last assertion in the "error snapshots" test previously inline-snapshotted a nested matcher error in its ANSI-colored form, which only matched when the test process had colors enabled (CI sets FORCE_COLOR=1; a local piped bun bd test does not). The fix wraps the inner failing expect in a try/catch, applies Bun.stripANSI to the thrown error's .message, rethrows, and updates the inline snapshot to the uncolored text.

Security risks

None. Test-only change with no runtime, network, filesystem, or dependency surface.

Level of scrutiny

Low. This is a 7-line de-flaking change to a single test assertion. The key REVIEW.md concern for de-flaking — "keep asserting the property the original assertion protected" — is satisfied: the assertion still checks that toThrowErrorMatchingInlineSnapshot on a non-throwing function raises the exact Matcher error: Received function did not throw message. The ANSI escape codes were never the property under test; they are a function of the process environment (TTY / FORCE_COLOR / NO_COLOR), not of the matcher.

Other factors

• Grepped test/js/bun/test/snapshot-tests/ for remaining \\x1B[ sequences — none, confirming the PR description's claim that this was the only color-dependent assertion in the directory.
• Bun.stripANSI is already used for exactly this purpose in several other tests (inspect-error.test.js, double-connect.test.ts, etc.), so the approach follows established convention.
• The (e as Error).message cast is safe here: the inner expect is guaranteed to throw a matcher Error (that is what the outer assertion is testing), and if that ever changed the test would fail loudly rather than silently pass.
• The PR description documents verification under plain env, FORCE_COLOR=1, and NO_COLOR=1 with identical pass/snapshot counts, and the two commits on this branch (f7029917, b0dbab8e) are already the two most recent commits on the checked-out HEAD.