72 lines
5.1 KiB
Markdown
72 lines
5.1 KiB
Markdown
# Proof tests from the 2026-10-14 code review
|
|
|
|
These are the tests the two reviewers wrote to demonstrate their findings. They live under a
|
|
directory whose name starts with `_`, so `go build ./...`, `go test ./...` and `go vet ./...`
|
|
ignore them. They are evidence, not part of the suite. When a finding is fixed, turn its proof
|
|
test into a real regression test in the package it belongs to.
|
|
|
|
| Dir | Reviewer | Style |
|
|
|---|---|---|
|
|
| `a/` | A — correctness and runtime | Demonstrations: the tests **pass** and print the evidence with `t.Log` (run with `-v`). |
|
|
| `b/` | B — security, contracts, ops | Assertions: the tests **fail** while the defect exists (a few `PASS`es only log evidence). |
|
|
|
|
## Running
|
|
|
|
Copy the files into a scratch copy of the repo, so the real packages stay clean:
|
|
|
|
```sh
|
|
rm -rf /tmp/mc_proof && cp -R /Users/dice/Work/migadu_counters /tmp/mc_proof && cd /tmp/mc_proof
|
|
cp -R review/_proof_tests/a/internal/. internal/ # reviewer A
|
|
cp -R review/_proof_tests/b/internal/. internal/ # reviewer B
|
|
cp -R review/_proof_tests/b/cmd/. cmd/
|
|
go test -count=1 -v -run 'TestReview' ./internal/... ./cmd/...
|
|
go test -run '^$' -bench Review -benchmem ./internal/txlog/ # A's CDB parsing benchmarks
|
|
```
|
|
|
|
Requirements and notes:
|
|
|
|
- `a/internal/store/review_test.go` (`TestReview_TimeoutAfterWriteStillCounts`) needs a real Redis on
|
|
port 16401: `redis-server --port 16401 --save '' --appendonly no`. The test uses `DEBUG SLEEP` to make the
|
|
reply arrive after the client timeout. It fails to connect if no Redis is running.
|
|
- `b/bad.cdb` is a three-row fixture with one malformed row (`bad.com x12|A/i=5`). It is used by
|
|
`b/internal/server/review_b4_test.go`, whose path was updated to `../../review/_proof_tests/b/bad.cdb`.
|
|
- `a/run/` holds the config and the app log from reviewer A's manual reproduction of "SIGHUP during startup
|
|
kills the daemon" (merged item #10). The paths in `config.toml` point at the reviewer's former scratch copy,
|
|
so edit them before reuse. The log stops after `domains db loaded`, which is the evidence.
|
|
|
|
## Map to the merged report (`../README.md`)
|
|
|
|
| Merged # | Proof |
|
|
|---|---|
|
|
| 1 | `a/internal/server/review_test.go` `TestReview_ClientDisconnectLosesCount`, `TestReview_ClientDisconnectNoDelay` |
|
|
| 2 | `b/internal/server/review_b_test.go` `TestReviewB_InMailFromDropsRecipientCount`, `TestReviewB_OutRecipientDropsSenderCount`; `a/…` `TestReview_MasterLoginAndOddSenders` |
|
|
| 3 | `a/internal/server/review_test.go` `TestReview_MasterLoginAndOddSenders` (first case) |
|
|
| 4 | `b/internal/server/review_b2_test.go` `TestReviewB_LostCrossingOnReplyTimeout`; `a/internal/server/review_test.go` `TestReview_PartialExecFailure`; `a/internal/store/review_test.go` (real Redis) |
|
|
| 5 | `a/internal/txlog/review_bench_test.go` |
|
|
| 6 | `b/internal/config/review_b_test.go` `TestReviewB_MisconfigStarts`; `b/…/review_b_test.go` `TestReviewB_EmptyTokenOpen` |
|
|
| 7 | `b/internal/server/review_b_test.go` `TestReviewB_PlusTagSeparateCounters` |
|
|
| 8 | `b/internal/server/review_b4_test.go`; `a/internal/counting/review_test.go` |
|
|
| 10 | `a/run/` (manual reproduction) |
|
|
| 11 | `b/internal/server/review_b_test.go` `TestReviewB_RejectLogHasNoTraceID` |
|
|
| 13 | `b/internal/server/review_b3_test.go` |
|
|
| 14 | `b/internal/server/review_b_test.go` `TestReviewB_ErrorLeak` |
|
|
| 15 | `b/internal/config/review_b_test.go` (case "sentinel without password") |
|
|
| 16 | `b/cmd/counterctl/review_b_test.go` `TestReviewB_31DayWarningOffByOne` |
|
|
| 17 | `b/internal/server/review_b_test.go` `TestReviewB_PanicNotInMetrics` |
|
|
|
|
## Fixed in round 1 (2026-10-14) — regression tests now in the suite
|
|
|
|
| Merged # | Regression test |
|
|
|---|---|
|
|
| 1 | `internal/server` `TestClientDisconnectStillCounts`, `TestAcceptedBeforeCounting`, `TestQueueFull`, `TestDrainCountsEverything`; `internal/counting/queue_test.go` |
|
|
| 2 | `internal/server` `TestIn_OddMailFromStillCountsRecipient`, `TestOut_OddRecipientStillCountsSender`; `internal/api` `TestValidateOut`, `TestValidateIn`, `TestNormalizeEmail` |
|
|
| 3 | `internal/server` `TestOut_LoginSuffixCountsBaseAddress`; `internal/api` `TestNormalizeAuthUser` |
|
|
| 4 | `internal/server` `TestPartialExecFailureStillNotifies`, `TestLostReplyStillNotifies`; `internal/counting` `TestCounter_PartialExecFailure`, `TestCounter_LostReplyReReadsMaster`; `internal/store` `TestIncr_PartialFailure`, `TestOutcomeUnknown` |
|
|
| 5 | `internal/domains` `TestLookup_CachesParsedRecord`, `TestLookup_CacheBounded`, `BenchmarkLookupSmall`/`BenchmarkLookupHuge` |
|
|
| 7 | `internal/server` `TestIn_PlusTagCountsMailbox`; `internal/counting` `TestPlan_PlusTagCountsMailbox`; `internal/domains` `TestMailbox` (decision 2026-10-15: strip the tag, as rcptd does) |
|
|
| 8 | `internal/server` `TestMalformedRowCountsOtherSide`; `internal/counting` `TestPlan_MalformedRowIsNotHosted`; `internal/domains` `TestLookup_MalformedRow`; `internal/txlog` `TestTrail_MalformedRow` |
|
|
| 6 | `internal/config` `TestValidate_Errors` (auth_token / unknown keys), `TestLoad_TokenFromEnvOnly`, `TestLoad_Defaults` (`:5555`); `cmd/migadu_counters` `TestRun_FlagsAndExitCodes` |
|
|
|
|
The proof files stay here as the review record; they no longer compile against the current API
|
|
(`CountResponse`, `store.Incr`) and are not meant to.
|