No make target runs the tests under -race, so no gate ever exercises the detector #23

Open
opened 2026-08-10 14:50:53 +02:00 by clawbot · 0 comments
Collaborator

Found during the adversarial review of
#21
(#21 (comment)).

The reviewer needed to check that handlers derived with WithAttrs/WithGroup
are safe when shared across goroutines, and found no make target or script/
entrypoint that runs the tests with -race. They had to invoke the toolchain
directly to do it, and disclosed the deviation.

That is a gate gap, not a reviewer problem. This library's handlers are shared
across goroutines by construction — that is what a process-wide slog default
is — so data races are exactly the defect class most worth catching here, and
nothing in the repo catches them. Raw toolchain invocation is disallowed across
this account precisely because the targets are supposed to carry the flags.

Definition of done

  • The tests run under -race as part of the repo's standard check, not as an
    optional extra target a person has to remember.
  • The race build works in whatever container the checks run in — -race needs
    cgo and a C toolchain, so an Alpine or CGO_ENABLED=0 stage will need
    attention rather than a flag.
  • A deliberately racy test is used as a negative control: confirm the detector
    actually fires and the check goes red, then remove it and confirm green. A
    -race flag that is present but not exercising anything is worse than none,
    because it reads as coverage.
  • The full check stays green and does not become materially slower than the
    suite justifies; if it does, say so rather than quietly accepting it.

Implementation requirements

  • Coordinate with #16 (STRTA scaffold
    divergence) and #20 (linting in
    Docker) — all three touch the entrypoints, and whichever lands later must not
    revert the others.
  • Landing commit title must end with (closes #<this issue>).
Found during the adversarial review of https://git.eeqj.de/sneak/simplelog/pulls/21 (https://git.eeqj.de/sneak/simplelog/pulls/21#issuecomment-53518). The reviewer needed to check that handlers derived with `WithAttrs`/`WithGroup` are safe when shared across goroutines, and found no `make` target or `script/` entrypoint that runs the tests with `-race`. They had to invoke the toolchain directly to do it, and disclosed the deviation. That is a gate gap, not a reviewer problem. This library&#39;s handlers are shared across goroutines by construction — that is what a process-wide `slog` default is — so data races are exactly the defect class most worth catching here, and nothing in the repo catches them. Raw toolchain invocation is disallowed across this account precisely because the targets are supposed to carry the flags. ## Definition of done - The tests run under `-race` as part of the repo&#39;s standard check, not as an optional extra target a person has to remember. - The race build works in whatever container the checks run in — `-race` needs cgo and a C toolchain, so an Alpine or `CGO_ENABLED=0` stage will need attention rather than a flag. - A deliberately racy test is used as a negative control: confirm the detector actually fires and the check goes red, then remove it and confirm green. A `-race` flag that is present but not exercising anything is worse than none, because it reads as coverage. - The full check stays green and does not become materially slower than the suite justifies; if it does, say so rather than quietly accepting it. ## Implementation requirements - Coordinate with https://git.eeqj.de/sneak/simplelog/issues/16 (STRTA scaffold divergence) and https://git.eeqj.de/sneak/simplelog/issues/20 (linting in Docker) — all three touch the entrypoints, and whichever lands later must not revert the others. - Landing commit title must end with ` (closes #<this issue>)`.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/simplelog#23