diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 95a7d36..06bd125 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -575,9 +575,20 @@ jobs: - name: did anything concurrent change id: look # The three files concurrency is allowed to live in are not a guess. - # They are the map in internal/guard/concurrency_test.go, and a fourth - # file growing a goroutine turns that guard red before it gets here - so - # this list cannot quietly fall behind the tree. + # They are the map in internal/guard/concurrency_test.go. + # + # This used to say the map guard was enough to keep the two in step: "a + # fourth file growing a goroutine turns that guard red before it gets + # here, so this list cannot quietly fall behind the tree". That was not + # true, and adding the fourth file on 2026-09-05 is what showed it. The + # map guard is red only while the new file is NOT in the map. The moment + # somebody adds it there - which is what the guard's own message tells + # them to do - it goes green, and nothing at all asks whether this line + # was updated too. The list could then fall behind exactly when it + # mattered: concurrency living in a file the detector is not run for. + # + # TestTheRaceDetectorIsRunForEveryFileThatDeclaresConcurrency now reads + # this line and compares it against the map, so the two cannot drift. # # go.mod is watched as well. A toolchain or dependency change can alter # what the detector sees even when none of our own lines moved. @@ -589,7 +600,7 @@ jobs: # in somebody else's file. run: | set -euo pipefail - watched='internal/format/registry.go cmd/tfg/main.go internal/gui/window/run.go go.mod' + watched='internal/format/registry.go cmd/tfg/main.go internal/gui/window/run.go internal/audit/parallel.go go.mod' # On a pull request there is no "before" - the field belongs to a push # - so this asked for something empty and every pull request answered # "touched". That quietly undid the decision of 2026-08-20, because diff --git a/CHANGELOG.md b/CHANGELOG.md index 87cd619..72b7aeb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,21 @@ because it turns other people's test suites red. ## [Unreleased] +### Changed + +- **`verify` and `cleanup` read the files over several threads, so checking a + large run is several times faster.** Nothing about what they report changes - + the same differences, in the same order, with the same exit codes. + + Measured on 6.1 GB in 96 files: `tfg verify` went from 4.28-4.30 seconds to + 0.54-0.56 seconds on an eight core machine. The gain follows the number of + cores you have, up to the point where the disk becomes the limit. On a set of + files too large to sit in memory, where every byte has to come off the disk, + it is about 1.6 times faster rather than nine. + + Small runs are unaffected either way. A handful of files was already + instant and still is. + ## [0.3.0-rc1] - 2026-09-03 ### Breaking diff --git a/internal/audit/audit.go b/internal/audit/audit.go index 8642a08..78a36cd 100644 --- a/internal/audit/audit.go +++ b/internal/audit/audit.go @@ -202,52 +202,37 @@ func Verify(ctx context.Context, dir string, m *manifest.Manifest, skip string) folded := make(map[string]string, len(claimed)) for _, f := range claimed { folded[core.FoldName(comparablePath(f.Path))] = f.Path - } - - for _, f := range claimed { - if err := ctx.Err(); err != nil { - return diffs, err - } seen[comparablePath(f.Path)] = true + } - // The raw path, never the compared one. Cleaning resolves a parent step - // against the text of the path rather than against the disk, and the - // two differ exactly where a link sits in the middle - which is the - // case core.Boundary exists for. - full, err := resolved(boundary, f) - if err != nil { - return nil, err - } - info, statErr := os.Stat(full) - if statErr != nil { - diffs = append(diffs, Difference{Kind: Missing, Path: f.Path}) - continue - } - - // Size first. It is free, it catches the common failure, and it names - // the cause more precisely than a hash mismatch would. - if info.Size() != f.Bytes { - diffs = append(diffs, Difference{ - Kind: WrongSize, Path: f.Path, - Want: fmt.Sprintf("%d B", f.Bytes), - Got: fmt.Sprintf("%d B", info.Size()), - }) - continue - } + // Every path is resolved here, on one goroutine, in order, before a single + // byte is read - and that is a correctness requirement rather than a step + // that happens to come first. + // + // A path that leaves the directory refuses the WHOLE pass. If that refusal + // could arrive from a worker, stopping the others would mean a lower index + // never got asked, so the same manifest would name a different file on + // different days. Settled in order, the first one that refuses is the first + // one there is. + full, err := claimedPaths(boundary, claimed) + if err != nil { + return nil, err + } - sum, hashErr := hashFile(full) - if hashErr != nil { - diffs = append(diffs, Difference{Kind: Unreadable, Path: f.Path, Got: hashErr.Error()}) - continue - } - if sum != f.Hashes.SHA256 { - diffs = append(diffs, Difference{ - Kind: WrongHash, Path: f.Path, - Want: f.Hashes.SHA256, - Got: sum, - }) + found, stopped := inOrder(ctx, len(claimed), func(i int) Difference { + return compare(claimed[i], full[i]) + }) + for _, d := range found { + if d.Kind != "" { + diffs = append(diffs, d) } } + if stopped != nil { + // Left unsorted and the walk below left undone, which is what the + // sequential version did. A cancelled pass reports what it compared, + // and the files it never reached are not absent - nobody looked. + return diffs, stopped + } // Not asked again here. This loop walks a slice already in memory, the walk // above asks before it, and Verify ends on ctx.Err() - so a check would be a @@ -378,6 +363,65 @@ func walk(ctx context.Context, dir string) ([]string, error) { return out, nil } +// claimedPaths is where each claimed file sits on the disk, in the order the +// manifest gives them, refusing on the first one that leaves the directory. +// +// Shared by Verify and Inspect because both have to refuse the same manifest +// for the same reason, and because both then hand the answers to workers that +// must not be able to refuse anything themselves. See parallel.go. +func claimedPaths(b core.Boundary, files []manifest.File) ([]string, error) { + full := make([]string, len(files)) + for i, f := range files { + // The raw path, never the compared one. Cleaning resolves a parent step + // against the text of the path rather than against the disk, and the + // two differ exactly where a link sits in the middle - which is the + // case core.Boundary exists for. + p, err := resolved(b, f) + if err != nil { + return nil, err + } + full[i] = p + } + return full, nil +} + +// compare is what one claimed file comes to: the difference it shows, or the +// zero Difference when what is on the disk is what the manifest describes. +// +// Kind is a string, so its zero value is not one of the kinds and can mean +// agreement on its own. No sentinel anybody has to remember, and no pointer +// per file - a run of ten thousand would allocate ten thousand of them to say +// "nothing to report" ten thousand times. +func compare(f manifest.File, full string) Difference { + info, statErr := os.Stat(full) + if statErr != nil { + return Difference{Kind: Missing, Path: f.Path} + } + + // Size first. It is free, it catches the common failure, and it names the + // cause more precisely than a hash mismatch would. + if info.Size() != f.Bytes { + return Difference{ + Kind: WrongSize, Path: f.Path, + Want: fmt.Sprintf("%d B", f.Bytes), + Got: fmt.Sprintf("%d B", info.Size()), + } + } + + sum, hashErr := hashFile(full) + if hashErr != nil { + return Difference{Kind: Unreadable, Path: f.Path, Got: hashErr.Error()} + } + if sum != f.Hashes.SHA256 { + return Difference{ + Kind: WrongHash, Path: f.Path, + Want: f.Hashes.SHA256, + Got: sum, + } + } + return Difference{} +} + // hashFile streams the file rather than reading it in. A run of this tool // produces files measured in gigabytes, and verify has to survive its own // output. diff --git a/internal/audit/cleanup.go b/internal/audit/cleanup.go index 88760d7..84249e6 100644 --- a/internal/audit/cleanup.go +++ b/internal/audit/cleanup.go @@ -62,51 +62,54 @@ func (c Candidate) Removable(force bool) bool { // does, and CLI.md section 9 rules out asking them interactively. func Inspect(ctx context.Context, dir string, m *manifest.Manifest) ([]Candidate, error) { boundary := core.NewBoundary(dir) + claimed := Claimed(m) - var out []Candidate - for _, f := range Claimed(m) { - if err := ctx.Err(); err != nil { - return out, err - } - full, err := resolved(boundary, f) - if err != nil { - // Nothing is inspected and nothing is offered. A list that points - // outside the directory is not a list this tool acts on, and the - // preview is where somebody decides - so it must not show the entry - // as something it would remove. - return nil, err - } + // Nothing is inspected and nothing is offered when one of these points + // outside the directory. A list that points outside is not a list this tool + // acts on, and the preview is where somebody decides - so it must not show + // the entry as something it would remove. + // + // Resolved before anything is read and on one goroutine, for the reason + // written out at claimedPaths and at parallel.go: a refusal that could + // arrive from a worker would name whichever file lost the race. + full, err := claimedPaths(boundary, claimed) + if err != nil { + return nil, err + } - info, err := os.Stat(full) - if errors.Is(err, fs.ErrNotExist) { - out = append(out, Candidate{Path: f.Path, Disposition: Absent}) - continue - } - if err != nil { - out = append(out, Candidate{Path: f.Path, Disposition: Unreachable, Detail: err.Error()}) - continue - } + // In order, because this list is what cleanup removes from and what it + // printed to a person beforehand. + return inOrder(ctx, len(claimed), func(i int) Candidate { + return look(claimed[i], full[i]) + }) +} - // Size before hash, so a file of obviously the wrong length does not - // cost a read of however many gigabytes it is. - if info.Size() != f.Bytes { - out = append(out, Candidate{Path: f.Path, Disposition: Changed, - Detail: fmt.Sprintf("it is %d B and the manifest recorded %d B", info.Size(), f.Bytes)}) - continue - } - sum, err := hashFile(full) - if err != nil { - out = append(out, Candidate{Path: f.Path, Disposition: Unreachable, Detail: err.Error()}) - continue - } - if sum != f.Hashes.SHA256 { - out = append(out, Candidate{Path: f.Path, Disposition: Changed, - Detail: "its content is not the content this run wrote"}) - continue - } - out = append(out, Candidate{Path: f.Path, Disposition: Ready}) +// look is what one claimed file comes to for cleanup: whether it may be +// removed, and if not, why not in the words a person needs. +func look(f manifest.File, full string) Candidate { + info, err := os.Stat(full) + if errors.Is(err, fs.ErrNotExist) { + return Candidate{Path: f.Path, Disposition: Absent} } - return out, ctx.Err() + if err != nil { + return Candidate{Path: f.Path, Disposition: Unreachable, Detail: err.Error()} + } + + // Size before hash, so a file of obviously the wrong length does not cost + // a read of however many gigabytes it is. + if info.Size() != f.Bytes { + return Candidate{Path: f.Path, Disposition: Changed, + Detail: fmt.Sprintf("it is %d B and the manifest recorded %d B", info.Size(), f.Bytes)} + } + sum, err := hashFile(full) + if err != nil { + return Candidate{Path: f.Path, Disposition: Unreachable, Detail: err.Error()} + } + if sum != f.Hashes.SHA256 { + return Candidate{Path: f.Path, Disposition: Changed, + Detail: "its content is not the content this run wrote"} + } + return Candidate{Path: f.Path, Disposition: Ready} } // Outcome is what happened to one file. diff --git a/internal/audit/parallel.go b/internal/audit/parallel.go new file mode 100644 index 0000000..e0c8726 --- /dev/null +++ b/internal/audit/parallel.go @@ -0,0 +1,137 @@ +package audit + +import ( + "context" + "runtime" + "sync" + "sync/atomic" +) + +// This file is the only place in internal/audit that runs anything beside +// anything else, and it is listed in internal/guard/concurrency_test.go with +// that reason. Keeping it to one file is the point: everything else in this +// package stays a plain loop, and a reader looking for the goroutines finds +// them here rather than spread through two passes. +// +// Why it exists. Verify and Inspect both walk the files a manifest claims and +// hash each one, and hashing is the work that dominates once O117 took the +// path resolution out of the loop. Measured 2026-09-05 with +// tools/probes/hashparallel on 6.1 GB in 96 files of 64 MB, three interleaved +// repetitions, drift canary held: +// +// workers 1 2 4 8 16 32 +// median 5074ms 2645ms 1424ms 766ms 544ms 571ms +// speedup 1.00x 1.92x 3.56x 6.63x 9.33x 8.88x +// +// O116 turned this down on 2026-08-20 and the number it turned down was real: +// on 3000 files of 1 kB the whole of verify is about a second and hashing is +// half of it, so the most that could be won was half a second. That reading +// describes small files. The measurement above describes the corpora this tool +// exists to produce, and the owner reopened the decision on 2026-09-05 with it +// in hand. +// +// The plateau at sixteen is this machine's hardware thread count, not a +// constant worth writing down. O117 recorded a plateau at eight from the +// 1 kB corpus and said the number would not need measuring again - it did, +// because a performance number is a property of the SHAPE OF THE WORK as well +// as of the machine. + +// widthFor is how many goroutines a pass of n files gets. +// +// GOMAXPROCS rather than a number, because the plateau measured above sat +// exactly at this machine's thread count and a constant would describe this +// machine rather than the one the tool is run on. Never more than there are +// files, so a manifest of three entries does not start sixteen goroutines to +// have thirteen of them find nothing to do. +func widthFor(n int) int { + w := runtime.GOMAXPROCS(0) + if w > n { + w = n + } + if w < 1 { + w = 1 + } + return w +} + +// inOrder answers one question for each of n items, over several goroutines, +// and hands the answers back in the order the items were given. +// +// Three properties, and each one is depended on by something else in this +// package rather than being tidiness: +// +// - Answers come back IN ORDER. Verify sorts its differences afterwards so +// it would not notice, but Inspect's list is what cleanup deletes from and +// what it printed to a person beforehand, so a reordered list would remove +// things in an order nobody was shown. +// - A cancelled pass returns the CONTIGUOUS PREFIX that finished, not +// whatever happened to be done. The sequential loop this replaces returned +// a prefix, and "verify was interrupted after N differences" is a sentence +// about a prefix. A set with holes in it would make that sentence describe +// a directory nobody checked in that shape. +// - one is never given a reason to fail. Everything that can refuse a whole +// pass - a path that leaves the directory - is settled before this is +// called, on one goroutine, in order. That is not a simplification for its +// own sake: if a refusal could arrive here, stopping the other goroutines +// would mean a LOWER index never got asked, and the same manifest would +// name a different file on different days. +func inOrder[T any](ctx context.Context, n int, one func(i int) T) ([]T, error) { + out := make([]T, n) + done := make([]bool, n) + + // next is the only thing every goroutine touches, and it is an atomic + // counter. Everything else they write - out and done - is written at + // indices no other one is given. See drain. + var next atomic.Int64 + var wg sync.WaitGroup + + work := func() { + defer wg.Done() + drain(ctx, &next, out, done, one) + } + for w := widthFor(n); w > 0; w-- { + wg.Add(1) + go work() + } + wg.Wait() + + return out[:finishedPrefix(done)], ctx.Err() +} + +// drain takes items off the counter until there are none left, answering each +// one and marking it done. +// +// A function of its own rather than the body of the goroutine above, and the +// reason is the same one written beside depthChange in internal/recipe: a +// literal inside a loop counts one level deeper than it reads, so loop plus +// literal plus loop plus branch is four, and the shape guard counts how many +// functions sit three deep as well as how deep the deepest one is. Splitting +// is what that guard asks for and it costs nothing here. +// +// out and done are written at indices no other goroutine will take, because +// the counter hands each index out exactly once. They are the only things +// written at all. +func drain[T any](ctx context.Context, next *atomic.Int64, out []T, done []bool, one func(i int) T) { + for { + i := int(next.Add(1)) - 1 + // Cancellation is asked per item rather than per pass, so a Ctrl+C + // during a long hash is noticed at the next file rather than at the + // end of the run. + if i >= len(out) || ctx.Err() != nil { + return + } + out[i] = one(i) + done[i] = true + } +} + +// finishedPrefix is how many items were answered before the first one that was +// not - which on a cancelled pass is everything the caller may speak about. +func finishedPrefix(done []bool) int { + for i, ok := range done { + if !ok { + return i + } + } + return len(done) +} diff --git a/internal/guard/auditorder_test.go b/internal/guard/auditorder_test.go new file mode 100644 index 0000000..7bbf589 --- /dev/null +++ b/internal/guard/auditorder_test.go @@ -0,0 +1,126 @@ +package guard + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/donislawdev/TestingFilesGenerator/internal/cli" +) + +// Since 2026-09-05 verify and cleanup hash the files a manifest claims over as +// many goroutines as the machine has hardware threads, because that is where +// the time went once O117 took the path resolution out of the loop. Measured +// on the corpora this tool exists to produce: tfg verify on 6.1 GB went from +// 5.6-7.8 s to 0.68-0.76 s. +// +// The order of the answers is the property that had nothing watching it, and +// it is not decoration. cleanup PRINTS this list to a person and then deletes +// from it. A list assembled by appending whatever goroutine finished first +// still holds the same files, so every other cleanup guard here stays green - +// they ask what was removed, not in what order it was offered. What changes is +// that somebody reads one order and the tool acts in another, on the one +// command in this project that destroys data. +// +// Asked through the command line rather than by calling audit directly, +// because that is where the order is observable and because every other guard +// in this package drives the tool the way a person does. +func TestCleanupOffersTheFilesInTheOrderTheManifestListsThem(t *testing.T) { + dir := t.TempDir() + out := filepath.Join(dir, "out") + + // Enough files that a shuffle cannot pass by luck. With sixteen workers and + // a hundred and twenty files, a list built out of completion order comes + // back wrong every time rather than one run in a hundred. + const count = 120 + if code, _, errOut := run(t, "generate", "--format", "txt", + "--count", "120", "--size", "4kb", "--out", out); code != cli.ExitOK { + t.Fatalf("generate gave %d, expected %d: %s", code, cli.ExitOK, errOut) + } + mf := filepath.Join(out, "manifest.json") + + listed := manifestPathsInOrder(t, mf) + // Asserted rather than assumed. A guard comparing two lists of nought + // agrees with itself and proves nothing, and this suite has been bitten by + // exactly that (O118). + if len(listed) != count { + t.Fatalf("the manifest lists %d files and the run asked for %d - this guard would be about the wrong thing", + len(listed), count) + } + + code, stdout, errOut := run(t, "cleanup", mf, "--json") + if code != cli.ExitOK { + t.Fatalf("the cleanup preview gave %d, expected %d: %s", code, cli.ExitOK, errOut) + } + + var report struct { + Files []struct { + Path string `json:"path"` + } `json:"files"` + } + if err := json.Unmarshal([]byte(stdout), &report); err != nil { + t.Fatalf("parsing the cleanup report: %v\n%s", err, stdout) + } + if len(report.Files) == 0 { + t.Fatalf("the cleanup preview offered nothing, so there is no order to check:\n%s", stdout) + } + + // A subsequence rather than an equal list, so this asks about ORDER and + // nothing else. Which entries reach the offer is audit.Claimed's rule and + // it has its own guards - repeating it here would mean two places to change + // when it moves, and a red guard naming the wrong thing. + at := 0 + for _, f := range report.Files { + found := false + for at < len(listed) { + if listed[at] == f.Path { + found, at = true, at+1 + break + } + at++ + } + if !found { + t.Fatalf("cleanup offered %q after the entries before it, and the manifest does not list it there.\n\n"+ + "The offer is built over several goroutines since 2026-09-05. In manifest order it is a list somebody\n"+ + "can read before the files go. In completion order it holds the same files and says them in an order\n"+ + "nobody was shown - on the one command here that deletes.\n\ncleanup offered:\n %v\n\nthe manifest lists:\n %v", + f.Path, pathsOf(report.Files), listed) + } + } +} + +// pathsOf is the offered list as plain strings, for a failure message that can +// be read next to the manifest. +func pathsOf(files []struct { + Path string `json:"path"` +}) []string { + out := make([]string, 0, len(files)) + for _, f := range files { + out = append(out, f.Path) + } + return out +} + +// manifestPathsInOrder is every path a manifest names, in the order it names +// them - the order audit.Claimed walks and the order the offer has to keep. +func manifestPathsInOrder(t *testing.T, path string) []string { + t.Helper() + b, err := os.ReadFile(path) + if err != nil { + t.Fatalf("reading the manifest: %v", err) + } + var m struct { + Files []struct { + Path string `json:"path"` + } `json:"files"` + } + if err := json.Unmarshal(b, &m); err != nil { + t.Fatalf("parsing the manifest: %v", err) + } + out := make([]string, 0, len(m.Files)) + for _, f := range m.Files { + out = append(out, f.Path) + } + return out +} diff --git a/internal/guard/branching_test.go b/internal/guard/branching_test.go index 745870c..7735c16 100644 --- a/internal/guard/branching_test.go +++ b/internal/guard/branching_test.go @@ -34,7 +34,9 @@ const ( crowdingArguments = 7 crowdingDepth = 3 - crowdedComplexity = 5 + // Lowered from 5 on 2026-09-05: splitting the verify loop into compare and + // claimedPaths took one function out of the band. The ratchet only tightens. + crowdedComplexity = 4 crowdedArguments = 5 // 53 until 2026-08-29, when TIFF arrived. The function that took it to 54 // is tiff.chooseSize, and it is the same shape as bmp.chooseSize because diff --git a/internal/guard/concurrency_test.go b/internal/guard/concurrency_test.go index ec3ac5c..7070893 100644 --- a/internal/guard/concurrency_test.go +++ b/internal/guard/concurrency_test.go @@ -5,6 +5,7 @@ import ( "go/ast" "go/parser" "go/token" + "os" "path/filepath" "sort" "strings" @@ -37,6 +38,22 @@ var mayBeConcurrent = map[string]string{ // invariant G7 exists to hold. Added 2026-08-05 with the first generate // window, and the owner was told. "internal/gui/window/run.go": "the run happens beside the window, and closing the window waits for it", + // Hashing the files a manifest claims is the work verify and cleanup are + // made of, and it is embarrassingly parallel. Added 2026-09-05 and the + // owner decided it: O116 turned the same idea down on 2026-08-20 on a + // measurement of 3000 files of 1 kB, where the whole of verify is about a + // second. Measured again on the corpora this tool exists to produce - + // 6.1 GB in files of 64 MB - tfg verify goes from 4.28-4.30 s to + // 0.54-0.56 s, the hashing itself is 9.33x at sixteen workers, and 1.58x + // even when the corpus is larger than memory and the disk is the limit. + // Numbers and the two instrument mistakes made getting them: + // docs/PERFORMANCE-REVIEW-2026-09-05.md. + // + // Kept to one file on purpose. Everything that could refuse a whole pass + // is settled before the goroutines start, so a worker answers about one + // file and cannot fail - which is what makes the order of the answers, and + // the file a refusal names, the same on every run. + "internal/audit/parallel.go": "hashing the claimed files runs beside itself, and nothing else in the package does", } // Waiting on cancellation is not the same thing as running in parallel. Every @@ -139,3 +156,74 @@ func onlyCancellation(s *ast.SelectStmt) bool { } return true } + +// TestTheRaceDetectorIsRunForEveryFileThatDeclaresConcurrency ties the map +// above to the list in .github/workflows/ci.yml that decides whether the race +// detector job runs at all. +// +// Why there are two lists. The detector does not run on every push - it was +// measured at 10m31s on 2026-08-20 and given its own job with its own trigger. +// That trigger is a literal list of file names inside the workflow, and it is a +// second copy of the map above. +// +// The workflow used to claim the two could not drift, on the reasoning that a +// file growing a goroutine reddens the map guard before it gets that far. That +// is true only while the file is MISSING from the map. Adding it - which is +// exactly what the map guard's own message tells somebody to do - turns that +// guard green and leaves this question to nobody. So the list could fall behind +// precisely when it mattered: concurrency living in a file the detector is +// never run for, with the job reporting "skipped" and looking like a decision. +// +// Found on 2026-09-05 while adding internal/audit/parallel.go, by walking into +// it. This is the mechanism rather than the warning. +// +// A file watched but not declared is fine and is not reported. go.mod is +// exactly that: a toolchain or dependency change can alter what the detector +// sees without one of our own lines moving. +func TestTheRaceDetectorIsRunForEveryFileThatDeclaresConcurrency(t *testing.T) { + path := filepath.Join(repoRoot(t), ".github", "workflows", "ci.yml") + body, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", path, err) + } + + // Asserted rather than assumed. A guard that quietly finds nothing to + // compare is green about a question it never asked - and this whole test + // exists because a claim about drift went unchecked. + const marker = "watched='" + start := strings.Index(string(body), marker) + if start < 0 { + t.Fatalf("%s no longer sets %s, so nothing states which files the race detector runs for", + path, strings.TrimSuffix(marker, "='")) + } + rest := string(body)[start+len(marker):] + end := strings.Index(rest, "'") + if end < 0 { + t.Fatalf("the %s list in %s is never closed", strings.TrimSuffix(marker, "='"), path) + } + watched := strings.Fields(rest[:end]) + if len(watched) == 0 { + t.Fatalf("the race detector trigger in %s watches nothing at all", path) + } + + listed := make(map[string]bool, len(watched)) + for _, f := range watched { + listed[f] = true + } + + var missing []string + for file := range mayBeConcurrent { + if !listed[file] { + missing = append(missing, file) + } + } + sort.Strings(missing) + + if len(missing) > 0 { + t.Errorf("%d file(s) declare concurrency and the race detector is not run for them:\n %s\n\n"+ + "The detector is the only thing in this project that sees a data race. A file that may run\n"+ + "beside itself and is not on the trigger list gets a job that reports \"skipped\", which reads\n"+ + "like a decision rather than a gap. Add it to the watched list in %s.", + len(missing), strings.Join(missing, "\n "), path) + } +} diff --git a/internal/guard/crowding_test.go b/internal/guard/crowding_test.go index 23b983a..00d0bde 100644 --- a/internal/guard/crowding_test.go +++ b/internal/guard/crowding_test.go @@ -43,7 +43,9 @@ const ( // becomes a rubber band. Like the ceilings // themselves these only go down. Raising one to turn a run green is the // same act as editing a golden value for the same reason. - crowdedFunctions = 10 + // Lowered from 10 on 2026-09-05: audit.Verify dropped under sixty lines + // when the per file work moved into compare. The ratchet only tightens. + crowdedFunctions = 9 crowdedFiles = 2 )