Repository navigation
feat(just): run the Go unit tests under -race in just check #52
Description
The problem (CMO)
Agent role: COD in s3, owner of the M01+M02 stack
Area: Automated checks
Problem Statement: As an agent working in pascalandy/nicely, I want just check to run the Go unit tests under the race detector, so that a data race between goroutines fails a check before a reviewer has to hunt for it.
REF: #47
Analogy
A smoke alarm that beeps only when you open the oven door: smoke can fill the kitchen while the door stays shut.
- What should happen: the alarm watches the whole room, so smoke sets it off whether or not someone opens the oven
- What happened in ✨ feat: sdk: run a Go extension from one value #47: a reviewer wrote a separate probe and ran it with
-raceto see the problem.just checkstayed green before and after the fix
What we saw
- A review of ✨ feat: sdk: run a Go extension from one value #47 found a data race: after a handler outlived cleanup and the drain limit,
Output.Writecould set the stored error whileOutput.Errread it. The reviewer proved it with a custom probe undergo run -race just checkpassed with the race in place, on each layer that held it (✨ feat: sdk: run a Go extension from one value #47, ✨ feat: extensions: run a bundled extension as a child of ncly #48, and the docs PR 📚 docs: sdk: match the docs to the packages that M02 moved #50), on Linux and on macOS- The fix, a mutex around the stored error, got no regression test. Without
-race, such a test passes with or without the fix. The agent proved the fix by rerunning the reviewer's probe by hand, and nothing guards it now - Cost: a review finding, then a fix, restack, push, and checks across 10 layers on two machines, for a fix that has no lasting test
What exists today
just test runs go test ./... ./testdata/extensions/... without -race. On main (c6ccccb), go test -race -count=1 ./sdk/... ./internal/... passes in about 3 seconds, so the unit tests already pass under the race detector, for a small cost. The SDK and the runner use goroutines for handlers, pipe drains, and signals, and later work adds more, such as #49 in M05.
Start, Stop, Continue (FMO)
- Start:
just testalso runsgo test -race ./sdk/... ./internal/.... A unit test covers a handler that keeps writing after the timeout, so the race of ✨ feat: sdk: run a Go extension from one value #47 fails it - Stop: relying on a reviewer's one-off
-raceprobe to find a goroutine race and to prove its fix - Continue: the testscript scenarios run without
-race, so the timing-sensitive cancellation cases, such as the 25 ms settle and the 15 seconds of cleanup, keep their speed and behavior
How we'll know it works
- Today: add the unit test, revert the mutex in
sdk/command/answer.golocally, and runjust check: it passes - After the change: the same revert makes
just checkfail withWARNING: DATA RACE. With the mutex, it passes - Nothing else changes: the scenarios keep their run time and results, and
just checkgrows by a few seconds
👨🏻🍳 For the agent
👨🏻🍳 Details
- Race site before the fix:
Output.Writesetserr, andOutput.Errreads it, insdk/command/answer.go.handleinsdk/extension/extension.goreaches the read aftertime.After(runner.Cleanup + runner.DrainLimit), while the handler goroutine still runs. ✨ feat: sdk: run a Go extension from one value #47 fixed it with async.Mutexthat guardserralone - The reviewer's probe: a handler that waits for SIGTERM, then writes to a stdout pipe whose reader is closed and never returns, with
runner.Cleanupshortened - Two test shapes: one goroutine writes to a closed pipe through
Outputwhile another callsErr, which is small but proves nothing abouthandle; or a seam insdk/extensionthat runsMainwith given files, which proves the timeout path - Keep
-raceoff the testscript binary at first. The scenarios re-exec the test binary, so-racethere would slow every scenario and stress the timing-sensitive cancellation cases -raceneeds cgo and a C toolchain. It ran on Linux in this session. Check macOS before wiring it intojust check. Release builds are unaffected, since only the tests use-race- Blast radius: the
testrecipe of thejustfile, and one new test file
by Opus-5.5 via Claude Code
Activity