Browse Source

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.
MHSanaei 7 hours ago
parent
commit
3fe92df7ad
3 changed files with 49 additions and 12 deletions
  1. 42 11
      CLAUDE.md
  2. 6 0
      CONTRIBUTING.md
  3. 1 1
      go.mod

+ 42 - 11
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

+ 6 - 0
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.

+ 1 - 1
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