From 3fe92df7ad4e5c59855c15da1c58cf56eec0129d Mon Sep 17 00:00:00 2001 From: MHSanaei Date: Sat, 26 Sep 2026 21:58:25 +0200 Subject: [PATCH] docs: adopt correct-fix-over-small-fix and TDD policy Replace the "smallest fix" rule with a "correct fix over small fix" policy: fix root causes properly, regardless of size, while still disallowing speculative additions. Add a dedicated TDD section (red-green-refactor, fake-test prohibitions) to CLAUDE.md and CONTRIBUTING.md, consolidating prior scattered testing guidance. Also promote jackc/pgx/v5 from an indirect to a direct go.mod dependency. --- CLAUDE.md | 53 +++++++++++++++++++++++++++++++++++++++---------- CONTRIBUTING.md | 6 ++++++ go.mod | 2 +- 3 files changed, 49 insertions(+), 12 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index f5b32ffa9..56bb624ab 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -75,11 +75,18 @@ file locations when it can answer in one hop. share-link or install-command output changes. ## Hard rules (non-negotiable) -- Fix size must match bug size. Find the root cause, then make the SMALLEST - change that removes it — a one-line guard beats a new subsystem. A small bug - does not earn new columns, jobs, abstractions, config knobs or helper layers. - If a fix genuinely needs new architecture, say so and get agreement first; - never ship it unasked next to the fix. +- Correct fix over small fix. Find the root cause and fix it the right way, however + much code that takes. Size the change by what the correct fix needs, never by + line count: when the right fix spans many files, or needs a migration, a shared + helper or a new abstraction, write it. A guard that hides the symptom while the + cause survives is the wrong fix, however small. Two limits remain: + - Everything added must be something the correct fix needs. No speculative + knobs, unused extension points or "while I was here" rewrites. Unrelated + refactors and cleanups go in their own commit. + - Stop and ask only when the right fix needs a decision the code cannot answer: + a deliberate user-visible behaviour change, or two sound designs with a real + trade-off. Ask with a recommendation. Size alone is never a reason to stop, + defer or ship a smaller patch. - Comments in committed Go/TS: 2 lines MAX per comment block. Make the name carry the meaning first and rename rather than annotate; spend the 2 lines on the *why* a name cannot hold — an invariant, an issue number, a non-obvious @@ -113,6 +120,36 @@ file locations when it can answer in one hop. explaining the why. Types in use: `fix`, `feat`, `chore`, `refactor`, `perf`, `docs`, `style`. +## Tests: TDD, and only tests that can fail (Go and frontend) +- Work red → green → refactor. + - Bug: turn the reproduction into a test first, and watch it fail for the + reported reason. + - Feature: write the test for the first behaviour before writing its code. + - Then write the code that makes it pass, and refactor with the suite green. + + If a test was written after the code, prove it anyway: revert the code, watch + the test go red, then restore. A test that passes either way is worse than no + test. It certifies nothing, and then gets cited as proof the fix works. +- Every test must name the failure it catches. When no test can reach a change + (workflow YAML, pure wiring, layout), say so and name the command that + demonstrates it. Never write a stand-in test. +- Fake tests are forbidden. Delete any you write or meet in the code you touch: + - tests of a getter, a constant, a rename, a pure map lookup, or an input the + function can never receive; + - tests that restate the implementation, such as recomputing the expected + value with the same formula or asserting that a mock was called exactly the + way the code calls it; + - mocking the unit under test, or mocking so much around it that the real + code path never runs; + - assertions too weak to fail: `err != nil`, `len > 0`, `toBeDefined()`, or + `not.toThrow()` alone; + - golden files or snapshots regenerated to match whatever the code now outputs; + - extra cases that exercise no distinct branch, and tests written to raise + coverage. + + One real test that drives the bug through the actual code path beats five + that restate the code. + ## Go conventions - Stdlib `testing` only (no testify). Table-driven, `t.Run` subtests, `t.Helper()` on helpers. Assert the exact value / typed error / emitted @@ -120,12 +157,6 @@ file locations when it can answer in one hop. `database.InitDB(filepath.Join(t.TempDir(), "x-ui.db"))` + `t.Cleanup(func() { _ = database.CloseDB() })`; `httptest` for HTTP. `internal/sub`'s `initSubDB(t)` is the template. -- A test must fail without its fix. Write it, revert the fix, watch it go red, - restore. A test that passes either way is worse than no test: it certifies - nothing and then gets cited as proof the fix works. -- Test what can actually break. No test for a getter, a constant, a rename, a - pure map lookup, or inputs the function can never receive. One real test that - drives the bug through the actual code path beats five that restate the code. - Code must pass `golangci-lint run` (gofumpt + goimports formatting): `make lint`. - Postgres, xray-gRPC-e2e and scale tests `t.Skip` unless `XUI_TEST_PG_DSN`, `XUI_DB_TYPE`+`XUI_DB_DSN`, `XRAY_E2E_BINARY` or `XUI_SCALE_TEST` is set — a diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index aeb84d2d5..3f45bbeb2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -243,6 +243,12 @@ For deeper notes on the frontend toolchain see [`frontend/README.md`](frontend/R Tests live next to the code (`foo.go` ↔ `foo_test.go`); frontend specs and golden fixtures live in `frontend/src/test/`. +### Test first, and only tests that can fail + +- **Red → green → refactor.** Write the test before the code. For a bug, the test reproduces the report; for a feature, it covers the first behaviour. Watch it fail, write the code that makes it pass, then refactor with the suite green. +- **Every test catches a named failure.** Don't test getters, constants or renames. Don't restate the implementation, mock the unit under test, write assertions too weak to fail, or regenerate snapshots to match whatever the code now outputs. +- **Fix the root cause the right way**, even when that takes more code. A small patch that hides the symptom is not a fix. + ### Go conventions - **Stdlib `testing` only** — no testify. Table-driven with `t.Run` subtests and `t.Helper()` on helpers. diff --git a/go.mod b/go.mod index 7d6c2c4b0..55a66ccce 100644 --- a/go.mod +++ b/go.mod @@ -13,6 +13,7 @@ require ( github.com/goccy/go-yaml v1.19.2 github.com/google/uuid v1.6.0 github.com/gorilla/websocket v1.5.3 + github.com/jackc/pgx/v5 v5.11.0 github.com/joho/godotenv v1.5.1 github.com/klauspost/compress v1.20.1 github.com/mattn/go-sqlite3 v1.14.52 @@ -65,7 +66,6 @@ require ( github.com/huin/goupnp v1.3.0 // indirect github.com/jackc/pgpassfile v1.0.0 // indirect github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 // indirect - github.com/jackc/pgx/v5 v5.11.0 // indirect github.com/jackc/puddle/v2 v2.2.2 // indirect github.com/jackpal/go-nat-pmp v1.1.0 // indirect github.com/jinzhu/inflection v1.0.0 // indirect