Add property/fuzz/invariant tests; fix 2 fuzz-found bugs; restore Makefile (#152)#152
Merged
Add property/fuzz/invariant tests; fix 2 fuzz-found bugs; restore Makefile (#152)#152
Conversation
…efile ## Test infrastructure - circuit_property_test.go: invariant-asserting stress tests + deterministic state-machine property test (testing/quick + MockClock + reference model). Verified catches the TOCTOU bugs from PR #151 by temporarily reverting CAS. - TestCircuit_RunMetricsExactlyOnce_Concurrent: verifies the exactly-once RunMetrics contract under 100K concurrent Execute calls - TestCircuit_TransitionAlternation_Concurrent: verifies Opened/Closed alternate correctly (closed ≤ opened ≤ closed+1) under contention - TestCircuit_FallbackMetricsExactlyOnce_Concurrent: same for fallback path - TestCircuit_StateMachineProperty: 500 random op sequences replayed against real circuit + trivial reference model, 6 invariants checked per step, fully deterministic via MockClock - faststats/fuzz_test.go: 5 Go-native fuzz targets + naiveRollingCounter differential oracle. Seeds run as unit tests in ~5ms. - FuzzRollingCounterOps: Inc/RollingSumAt/GetBuckets vs naive oracle - FuzzRollingCounterJSON: round-trip + hostile-input safety - FuzzSortedDurationsPercentile: bounds + monotonicity - FuzzRollingBucketAdvance: index bounds + LastAbsIndex monotonicity - FuzzTimedCheckJSON: round-trip + hostile-input safety - Tightened existing stress tests (were assertion-free, just t.Logf): - circuit_stress_test.go: TestCircuitStateTransitionRace now verifies Opened/Closed alternation + ConcurrentCommands balance - faststats/rolling_stress_test.go: deleted 2 permanent-skip tests, added real TestTimedCheckConcurrency, added RollingSum≤TotalSum check ## Bug fixes (found by fuzzers in ~10s) - faststats/rolling_percentile.go: Percentile(NaN) panicked — NaN falls through both p<=0 and p>=100 guards, int(math.Floor(NaN)) is INT64_MIN on amd64, index-out-of-range. Now returns -1. - faststats/rolling_counter.go: UnmarshalJSON accepted hostile JSON with NumBuckets > len(Buckets), then GetBuckets panicked. Now rejected at unmarshal time with a descriptive error. ## Build tooling - Makefile: restored after Jan 2024 deletion. make (fast dev loop), make ci (pre-PR gate mirroring GitHub Actions), make fuzz, make bench, make cover, make fix, make help. - .github/workflows/build.yml: Build/Test steps now call make build / make test-race (same commands, now DRY). - .github/workflows/fuzz.yml: daily scheduled fuzzing, 5min per target, uploads crash inputs as artifacts on failure. Manual dispatch enabled. - README.md: Development section points to make targets. Fixed dead Makefile link. - .gitignore: added /coverage.html (from make cover).
TestRollingCounterBucketRolloverRace had a rollingSum < 0 check inside the hot concurrent loop since PR #139/#150. This check was always wrong for the lock-free RollingCounter design (documented at rolling_bucket.go): Thread A (Inc): buckets[idx].Add(1) Thread B (rollover): Swap(0) returns the incremented value, rollingSum.Add(-toDec) — briefly over-decrements Thread A (Inc cont): rollingSum.Add(1) — corrects A reader between B and A's rollingSum write observes a transient negative. The value is correct once writers quiesce. Moved the < 0 check to after wg.Wait(). The rollingSum <= TotalSum check stays in the hot loop (safe: reads are correctly ordered).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three layers of new test coverage, two bug fixes they surfaced, and build tooling.
Test infrastructure
circuit_property_test.go— invariant-asserting stress tests + deterministic state-machine property test (testing/quick+MockClock+ trivial reference model). Verified to catch the PR Fix 8 bugs: TOCTOU races, panics, and missed cases from PR #150 #151 TOCTOU bugs by temporarily reverting the CAS fix.faststats/fuzz_test.go— 5 Go-native fuzz targets +naiveRollingCounterdifferential oracle. Seed corpora run as unit tests in ~5ms; active fuzzing viamake fuzz.TestCircuitStateTransitionRaceand friends weret.Logf-only; now assert real invariants (closed ≤ opened ≤ closed+1, counter balance). Deleted 2 permanently-skipped tests.Bug fixes (found by fuzzers in ~10 seconds)
Percentile(NaN)panicked — NaN falls through bothp<=0andp>=100guards (NaN comparisons are always false), thenint(math.Floor(NaN))= INT64_MIN on amd64 → index-out-of-range. Now returns-1.RollingCounter.UnmarshalJSONaccepted hostile JSON withNumBuckets > len(Buckets), thenGetBucketspanicked. Now rejected at unmarshal time with a descriptive error.Build tooling
Makefilerestored (deleted Jan 2024).make= fast dev loop;make ci= what GitHub Actions runs (build +-race -count 10+ lint);make fuzz FUZZTIME=30s;make helpfor the rest..github/workflows/fuzz.yml— daily scheduled fuzzing, 5min/target, uploads crash inputs as artifacts on failure. Manual dispatch enabled.build.ymlnow callsmake build/make test-race(same commands, now DRY).Test plan
make ci→ all pass, ~61s (CI was ~60s before — negligible overhead)golangci-lint run→ 0 issuesTestCircuit_TransitionAlternation_Concurrentcatches the pre-PR#151 CAS/TOCTOU bug by reverting the fix locallyTestRollingCounter_UnmarshalJSON_RoundTripstill passes (new validation doesn't break valid Marshal output)