Repository navigation
ci: run the Go race detector on concurrency-sensitive packages (APA-83 P8) - #156
Merged
Merged
Conversation
…3 P8) No workflow invoked `go test -race`, so data races in shared-state packages would not fail CI. Race coverage was manual-only. Scope is derived from actual concurrency surface, not a blanket `-race ./...`: the only production goroutine spawn is cmd/api/main.go:159, and the packages carrying sync.Mutex / sync/atomic state are investigate, investigate/tools, ingest, workflow, worker, ports (event bus), observability, metrics, and adapters/memblob. `internal/investigate/orchestrate` is reached via ./internal/investigate/.... Scoped deliberately: -race adds substantial runtime, so the step covers the packages that actually share mutable state rather than the whole module. Evidence: - green baseline: all 10 packages pass under -race with provider keys and TEST_POSTGRES_DSN unset (exit 0). - detector proven active (negative control, since reverted): a deliberate unsynchronized `shared++` across 8 goroutines in internal/metrics produced WARNING: DATA RACE and FAIL under -race, and passed without -race — proving the detector, not the test, is what caught it. 3/3 deterministic repeats. - live-provider suites are not compiled into this scope: `go test -race -list` reports 0 live-named tests on a clean checkout (APA-82/APA-89 build tags). Existing gates unchanged: gofmt clean, go vet clean, offline `go test ./...` exit 0, pytest 44 passed, ruff clean, mypy strict clean over 9 source files, isolation guard PASS. No live-provider test bodies executed. Release remains NOT QUALIFIED.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
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.
Why
No workflow invoked
go test -race. Data races in the packages that share mutable state would not fail CI — race coverage was manual-only, so a regression could ship green.Scope — derived from actual concurrency surface
Not a blanket
-race ./.... Verified surface:cmd/api/main.go:159sync.Mutex/sync/atomicstate lives ininternal/investigate,internal/investigate/orchestrate,internal/ingest,internal/workflow,internal/worker,internal/ports(event bus),internal/observability,internal/metrics,internal/adapters/memblob-raceadds substantial runtime, so the step covers the packages that actually share mutable state.orchestrateis reached via./internal/investigate/....1 file, +22. No test, assertion, or gate modified — this only adds a step.
Evidence
Green baseline — all 10 packages under
-race, provider keys andTEST_POSTGRES_DSNunset:Negative control — the detector is proven active, not merely configured. A deliberate unsynchronized
shared++across 8 goroutines ininternal/metrics(since reverted):-raceWARNING: DATA RACE→FAIL, exit 1-raceokexit 0 — passes, race is timing-dependent-race, 3 repeatsThe second row is the important one: the test passes on its own, so the detector, not the test, is what caught it. The control file was removed and is not part of this diff.
Live-provider safety
Live suites are not compiled into this scope.
go test -race -listreports 0 live-named tests on a clean checkout — APA-82/APA-89 build tags hold. No live-provider test bodies executed.Existing gates unchanged
gofmtclean ·go vetclean · offlinego test ./...exit 0 ·pytest tests/unit44 passed ·ruff check .clean · mypy strict clean (9 source files) · isolation guard PASS.Scope guard
Isolated from the other APA-83 gaps (P10/P12, P7, P11/P13–P15) and from P9. Release remains NOT QUALIFIED.
Refs APA-83 (P8). Next: P10/P12 PostgreSQL and Mockoon.