Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alii merged 2 commits into from
Apr 21, 2026
+6 −6
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fac15d9
docs(CLAUDE.md): only use test/regression/ for numbered regressions
robobun Apr 21, 2026
deef269
docs: go firmer on reusing existing test files; sync test/CLAUDE.md a…
robobun Apr 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,7 @@ This is an explicit exception to the "never use `bun test` directly" rule. There

### Test Organization

If a test is for a specific numbered GitHub Issue, it should be placed in `test/regression/issue/${issueNumber}.test.ts`. Ensure the issue number is **REAL** and not a placeholder!

If no valid issue number is provided, find the best existing file to modify instead, such as;
**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`, and so on. Keeping tests next to related coverage is what makes them discoverable and prevents duplicated setup.

- `test/js/bun/` - Bun-specific API tests (http, crypto, ffi, shell, etc.)
- `test/js/node/` - Node.js compatibility tests
Expand All @@ -65,6 +63,8 @@ If no valid issue number is provided, find the best existing file to modify inst
- `test/napi/` - N-API compatibility tests
- `test/v8/` - V8 C++ API compatibility tests

**Exception:** `test/regression/issue/${issueNumber}.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 — if the behavior was never correct, it's not a regression and the test belongs in the existing file for that module. The issue number must be **REAL**, not a placeholder.

### Writing Tests

Tests use Bun's Jest-compatible test runner with proper test fixtures.
Expand Down
4 changes: 2 additions & 2 deletions test/CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,8 @@ To create a repetitive string, use `Buffer.alloc(count, fill).toString()` instea
### Test Organization

- Use `describe` blocks for grouping related tests
- Regression tests for specific issues go in `/test/regression/issue/${issueNumber}.test.ts`. If there's no issue number, do not put them in the regression directory.
- Unit tests for specific features are organized by module (e.g., `/test/js/bun/`, `/test/js/node/`)
- **Add tests to the existing test file for the code you're changing** — do not create a new file. Tests are organized by module (e.g., `/test/js/bun/`, `/test/js/node/`, `/test/js/web/`).
- `/test/regression/issue/${issueNumber}.test.ts` is **only** for bugs that have a GitHub issue number **and** are true regressions (worked in a previous release, then broke). An issue number alone does not qualify — if it was never correct, put the test in the module's existing test file instead.
- Integration tests are in `/test/integration/`

### Nested/complex object equality
Expand Down
2 changes: 1 addition & 1 deletion test/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
});
```

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.

Check warning on line 53 in test/README.md

View check run for this annotation

Claude / Claude Code Review

test/README.md directory listing still uses old 'specific issue' framing 

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."

@claude claude Bot Apr 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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

  1. A contributor opens test/README.md to figure out where their test goes.
  2. The first content section is "Finding tests", which lists regression/ - tests that reproduce a specific issue.
  3. Their bug is a specific (numbered) issue, so this blurb suggests regression/ is the right home — the old mental model.
  4. 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").
  5. 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.


```ts
// test/regression/issue/02005.test.ts
Expand Down
Loading