docs(CLAUDE.md): only use test/regression/ for numbered regressions by robobun · Pull Request #29560 · oven-sh/bun

robobun · 2026-04-21T21:44:00Z

Clarifies test placement guidance across CLAUDE.md, test/CLAUDE.md, and test/README.md:

Default: add your test to the existing test file for the code you're changing. Do not create a new file. A fetch bug goes in test/js/web/fetch/fetch.test.ts, a Bun.serve bug goes in test/js/bun/http/serve.test.ts, etc.
Exception: test/regression/issue/${N}.test.ts is reserved for bugs that have a GitHub issue number and are true regressions (worked in a previous release, then broke). An issue number alone is not enough.

Previously the wording implied any numbered issue → regression dir, which scatters tests away from the code they cover (e.g. #29198 added test/regression/issue/29195.test.ts for a fetch bug that belonged in fetch.test.ts). All three docs now agree.

test/regression/issue/ is for bugs that have both a GitHub issue number and are actual regressions (worked before, broke). Everything else should be added to the existing test file for the code being changed.

robobun · 2026-04-21T21:44:12Z

^{Updated 8:40 PM PT - Apr 21st, 2026}

❌ @robobun, your commit deef269 has 4 failures in Build #46934 (All Failures):

test/js/bun/webview/webview-chrome.test.ts - code 1 on 🍎 14 aarch64
test/js/bun/webview/webview-chrome.test.ts - code 1 on 🍎 13 aarch64
test/js/bun/util/v8-heap-snapshot.test.ts - SIGKILL on 🐧 25.04 x64-baseline
test/js/bun/net/socket.test.ts - code 1 on 🪟 11 aarch64
test/integration/bun-types/bun-types.test.ts - unchecked exception at constructEmptyArray @ cache/webkit-4d5e75ebd84a14ed-asan/include/JavaScriptCore/JSGlobalObjectInlines.h:286�[0m (thrown from cache/webkit-4d5e75ebd84a14ed-asan/include/JavaScriptCore/JSGlobalObjectInlines.h:306�[0m) +1 more on 🐧 13 x64-asan

🧪 To try this PR locally:

bunx bun-pr 29560

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

bun-29560 --bun

coderabbitai · 2026-04-21T21:46:05Z

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 62df4ff3-0c19-445c-b974-d0ee6c7b68d2

📥 Commits

Reviewing files that changed from the base of the PR and between fac15d9 and deef269.

📒 Files selected for processing (3)

CLAUDE.md
test/CLAUDE.md
test/README.md

Disabled knowledge base sources:

Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

Walkthrough

Refined test organization rules: tests tied to a GitHub issue are placed in test/regression/issue/... only if the issue has a number and the bug is a true regression; otherwise add tests to the existing module-specific test file. Guidance now maps common bug types to their module test locations.

Changes

Cohort / File(s)	Summary
Test documentation `CLAUDE.md`, `test/CLAUDE.md`, `test/README.md`	Rewrote test placement rules: require both a GitHub issue number and confirmed regression to use `test/regression/issue/${issueNumber}.test.ts`. Otherwise, instruct contributors to add tests to the existing module-specific test file (with clearer mappings of bug types to test locations).

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	The description covers the key changes but does not follow the required template structure with explicit 'What does this PR do?' and 'How did you verify your code works?' sections.	Restructure the description to match the repository template with clear sections for what the PR does and verification steps, even if verification is 'documentation review only'.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: restricting the test/regression/ directory to only numbered regressions, which aligns with the file modifications in CLAUDE.md.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

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

…nd test/README.md Lead with the default (add to the existing test file for the code) and frame test/regression/ as the exception. Add concrete examples (fetch bug -> fetch.test.ts). Update test/CLAUDE.md and test/README.md to match so the guidance is consistent across all docs.

claude

Thanks for the follow-up — all three docs are now consistent. LGTM; the inline nit about test/README.md:11 is optional polish.

Extended reasoning...

Overview

Docs-only change to three files: root CLAUDE.md, test/CLAUDE.md, and test/README.md. Tightens the test-placement rule so test/regression/issue/${N}.test.ts is reserved for true regressions (worked before, then broke) with a GitHub issue number, and makes "add to the existing test file for the code you're changing" the explicit default. My earlier review flagged that test/CLAUDE.md and test/README.md still carried the old wording; deef269 addressed that and all three docs now state the same rule.

Security risks

None. Pure documentation/guidance change — no executable code, config, build scripts, or CI workflows touched.

Level of scrutiny

Low. These are contributor-guidance docs (one human-facing README, two agent-facing CLAUDE.md files). The change is a wording clarification with clear intent, no behavioral side effects, and no CODEOWNERS coverage on the affected paths.

Other factors

The one remaining finding is a nit: the brief directory blurb at test/README.md:11 still says "tests that reproduce a specific issue." That phrasing isn't strictly wrong (a regression is a specific issue) and the detailed rule at line 53 of the same file is correct, so it doesn't reintroduce the contradiction this PR set out to fix. It's a one-line polish item the author can take or leave — not a reason to block.

claude · 2026-04-21T21:54:30Z

 ```

-If you are fixing a bug that was reported from a GitHub issue, remember to add a test in the `test/regression/` directory.
+When fixing a bug, add the test to the existing test file for that code (e.g. a fetch bug → `test/js/web/fetch/fetch.test.ts`). Only use `test/regression/` when the bug has a GitHub issue number **and** is a true regression — it worked in a previous release and then broke.


🟡 nit: test/README.md:11 (the directory listing in "Finding tests") still describes regression/ as "tests that reproduce a specific issue" — the loose framing this PR is retiring. Since deef269 already updated line 53 of this file for the same reason, consider tweaking the one-line blurb too, e.g. "regression/ - tests for true regressions (worked in a prior release, then broke) tied to a GitHub issue."

Extended reasoning...

What the issue is

The follow-up commit deef269 synced test/CLAUDE.md and test/README.md with the new placement rule, updating test/README.md:53 to say "only use test/regression/ when the bug has a GitHub issue number and is a true regression". However, the directory-structure listing earlier in the same file was missed:

* `test/` * `js/` - tests for JavaScript APIs. * `cli/` - tests for commands, configs, and stdout. * `bundler/` - tests for the transpiler/bundler. * `regression/` - tests that reproduce a specific issue. ← line 11

"tests that reproduce a specific issue" is precisely the broad framing the PR description identifies as the problem ("the wording implied any numbered issue → regression dir").

Why this is in scope

This isn't a pre-existing nit being opportunistically flagged — the PR already touched this exact file for this exact reason in response to the earlier review comment. The stated purpose of deef269 was to "sync test/CLAUDE.md and test/README.md", and line 11 is part of test/README.md's description of the regression/ directory. It's a straightforward miss in that sync pass.

Step-by-step: how it conflicts

A contributor opens test/README.md to figure out where their test goes.

The first content section is "Finding tests", which lists regression/ - tests that reproduce a specific issue.

Their bug is a specific (numbered) issue, so this blurb suggests regression/ is the right home — the old mental model.

Only if they keep reading to line 53 do they see the narrower rule ("and is a true regression — it worked in a previous release and then broke").

The two descriptions of the same directory now disagree on the qualifying criterion (issue-reproduction vs. true-regression).

It's not a hard contradiction — a true regression is also "a specific issue" — but the whole point of this PR is that the loose phrasing was actively misdirecting people, and this is one remaining instance of it.

Why nothing prevents it

There's no automated consistency check across doc files; line 11 is simply outside the diff hunks and was overlooked.

Suggested fix

Change line 11 to something like:

* `regression/` - tests for true regressions (worked in a prior release, then broke) tied to a GitHub issue.

One-line edit, no other impact.

…ven-sh#29560) Clarifies test placement guidance in CLAUDE.md: - `test/regression/issue/${N}.test.ts` is **only** for bugs that have a GitHub issue number **and** are actual regressions (worked in a previous release, then broke). - Everything else should be added to the **existing test file** for the code being changed, not a new standalone file. Previously the wording implied any numbered issue → regression dir, which led to tests being scattered away from the code they cover. --------- Co-authored-by: Alistair Smith <hi@alistair.sh> Co-authored-by: Dylan Conway <dylan.conway567@gmail.com>

docs(CLAUDE.md): only use test/regression/ for numbered regressions

fac15d9

test/regression/issue/ is for bugs that have both a GitHub issue number and are actual regressions (worked before, broke). Everything else should be added to the existing test file for the code being changed.

github-actions Bot added the claude label Apr 21, 2026

claude Bot reviewed Apr 21, 2026

View reviewed changes

Comment thread CLAUDE.md Outdated

alii merged commit 4311f5b into main Apr 21, 2026
5 of 13 checks passed

alii deleted the farm/36dfda72/claude-md-regression-test-placement branch April 21, 2026 21:50

claude Bot reviewed Apr 21, 2026

View reviewed changes

robobun mentioned this pull request Sep 11, 2026

test(29513): AsyncDisposableStack.disposeAsync awaits each resource #29564

Open

Navigation Menu

docs(CLAUDE.md): only use test/regression/ for numbered regressions - #29560

docs(CLAUDE.md): only use test/regression/ for numbered regressions#29560
alii merged 2 commits into
mainfrom
farm/36dfda72/claude-md-regression-test-placement

robobun commented Apr 21, 2026 •

edited

Loading

Uh oh!

robobun commented Apr 21, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Apr 21, 2026 •

edited

Loading

Review failed

❌ Failed checks (1 inconclusive)

Uh oh!

Uh oh!

Uh oh!

claude Bot left a comment

Uh oh!

claude Bot Apr 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

robobun commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robobun commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Walkthrough

Changes

❌ Failed checks (1 inconclusive)

Uh oh!

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

claude Bot Apr 21, 2026

Choose a reason for hiding this comment

What the issue is

Why this is in scope

Step-by-step: how it conflicts

Why nothing prevents it

Suggested fix

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

robobun commented Apr 21, 2026 •

edited

Loading

robobun commented Apr 21, 2026 •

edited

Loading

coderabbitai Bot commented Apr 21, 2026 •

edited

Loading