From 6540723320ee7fdda228e21dd8b306c96aa906d3 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Mon, 7 Sep 2026 13:53:14 +0200 Subject: [PATCH] fix: verify says whose the other files in the directory are A directory is allowed to hold more than one run. output.manifest exists so that a second run records itself beside the first rather than being refused, and verify called every file the neighbour had written "extra" - the word for a file nobody asked for - and then called the directory a mismatch. Measured on 2026-09-07, two runs one after another into one directory with name templates that do not collide, both ending 0: verify manifest-alpha.json 3 differences, exit 7 verify manifest-beta.json 4 differences, exit 7 Every one of those was the other run's work, and which run had written it was recorded in the same directory the whole time. The report described a directory somebody had polluted. They get their own kind now, another-run, carrying the name of the record that lists them, and they do not count towards the verdict. A shared directory ends 0 and matched is true. A real disagreement is untouched: a missing or changed file is still a mismatch, still exit 7, and the heading counts only the disagreements so the list under it does not mix the two. This is the third time a category has been cut out of Extra for this reason, after Leftover and Respelled, so the shape is the one already in the file. The rule it follows is attribution rather than suppression, and that is the half worth stating. Every file stays in the report: one entry each in --json, and in the prose one line per neighbouring record rather than one per file. Untouchable rule 6 is kept literally rather than on trust, and a manifest somebody leaves in a directory can claim a file out loud and cannot hide one. Measured: a neighbour's ten thousand files used to print 10 001 lines and exit 7, and now print one line and exit 0. The grouping is the lesson manifest.Notes() already carries. cleanup needed nothing. It walks the manifest's entries rather than the directory, so it has never known the word. Two limits, both deliberate and both pinned by a guard rather than left to be discovered: - a neighbour's record is only opened when its name ends in .json. Reading the first bytes of every unlisted file was the first design and was measured out the same day - opening a file on Windows is not free. The factor is REFUSED rather than reported, because the canary runs of the unchanged binary disagreed with each other sevenfold while it was taken. What is not in doubt is that it was large enough to change the design. A record under another name is reported exactly as it was before. - a file no manifest in the directory lists is still extra, so leaving a manifest somewhere does not account for everything beside it. A second sieve was written and taken out again the same day. It read the first half kilobyte of a candidate looking for the first key of a manifest, to avoid reading a large JSON document in full. Nothing could redden it: a document that got past it was refused by the schema check behind it anyway, so removing it changed no answer and no test could tell. Cost, interleaved with canaries on a directory of ten thousand unlisted files: 228 ms before against 258 ms after, canaries 188 and 194 ms. The canaries agree to 3% and the two variants OVERLAP - 133-362 against 190-266 - so no factor is claimed. Seven guards, seven mutations. Two of the mutations did not compile at first and had to be rewritten to consume what they replaced, and staleness.py then found five EXISTING mutations that lifting the classification out of the loop had invalidated. Also in here, because the crowding ratchet caught it: - three functions split out so that the count of functions nesting three deep stays at 51 rather than being moved up to 53. The ratchet only tightens. - the comment on worstComplexity named rawTarget.resolveSize, and the guard's own log has named engine.Run since the run lock went in. The number has not moved. Who holds it has, and a comment naming the wrong function sends the next reader to flatten something that is not at the ceiling. O185 records that Run now sits at exactly 22 of 22. Co-authored-by: Claude Opus 5 --- CHANGELOG.md | 25 +++ internal/audit/audit.go | 114 +++++++++--- internal/audit/neighbours.go | 140 +++++++++++++++ internal/cli/verify.go | 142 ++++++++++++++- internal/guard/anotherrun_test.go | 283 ++++++++++++++++++++++++++++++ internal/guard/branching_test.go | 7 +- 6 files changed, 677 insertions(+), 34 deletions(-) create mode 100644 internal/audit/neighbours.go create mode 100644 internal/guard/anotherrun_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index c98fe13..e2de7fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -299,6 +299,31 @@ because it turns other people's test suites red. ### Fixed +- **`tfg verify` no longer calls another run's files "extra".** A directory is + allowed to hold more than one run - that is what `output.manifest` is for - + and verifying one of them reported every file the other had written as a file + nobody asked for, then called the directory a mismatch. Measured with two runs + into one directory whose file names do not collide, both ending `0`: three + differences against one manifest and four against the other, every one of them + the neighbour's work. + + They are reported as `another-run` now, each one naming the record that lists + it, and they no longer make the directory a mismatch. `tfg verify` on a shared + directory ends `0`, and `matched` in `--json` is `true`. A real disagreement + is unaffected - a missing or changed file is still a mismatch and still exits + `7`. + + **Nothing is hidden.** Every file is still in the report: one entry each in + `--json`, and in the prose one line per neighbouring record rather than one + per file. A directory holding a neighbour's ten thousand files used to print + ten thousand and one lines and exit `7`. It now prints one line and exits `0`. + + Two limits worth knowing. A neighbour's record is recognised only when its + name ends in `.json`, because opening every unlisted file was measured and was + too expensive - a record under another name is reported the way it was before. + And a file that no manifest in the directory lists is still `extra`, so + leaving a manifest in a directory does not account for everything in it. + - **Two runs writing into one directory can no longer write over each other's files.** A run holds the directory it is writing into for as long as it is writing. A second run that starts meanwhile is refused before it writes a diff --git a/internal/audit/audit.go b/internal/audit/audit.go index 831bc53..7591f1d 100644 --- a/internal/audit/audit.go +++ b/internal/audit/audit.go @@ -53,6 +53,29 @@ const ( // authority over what may be removed and a file that never finished never // reached it. So the only useful thing is to say plainly what it is. Leftover Kind = "leftover" + // AnotherRun is a file in the directory that a DIFFERENT run's manifest, + // sitting in the same directory, claims - or that manifest itself. + // + // Reported apart from Extra for the reason Leftover and Respelled are, and + // it is the third time that reason has come up. Extra means somebody else + // put it here and the question is whose. This one has an answer to that + // question, written down in the same directory: the neighbouring record + // names it, and Want carries the name of that record. + // + // A directory is allowed to hold more than one run. output.manifest exists + // so that a second run can record itself beside the first rather than being + // refused, and calling the result a mismatch made that unusable in the + // place it is for - a CI job cannot have a check that is red whenever it + // worked. Measured on 2026-09-07: two runs into one directory with names + // that do not collide, both ending 0, and verify then reported three + // differences against one manifest and four against the other, every one of + // them the other run's work. + // + // It does not make the directory a mismatch, and it is still printed. That + // pair is the whole design: attribution rather than suppression, so a + // manifest somebody drops into a directory can claim a file out loud and + // cannot hide one. + AnotherRun Kind = "another-run" // Respelled is the file this manifest describes, stored under a spelling // the filesystem treats as the same name. // @@ -124,6 +147,21 @@ func (d Difference) String() string { "Nothing described by this manifest is missing because of it. "+ "cleanup will not remove it, because it removes only what the manifest lists - delete it by hand", d.Path) + case AnotherRun: + // Two sentences, because the file is either the neighbour's record or + // one of the files it lists, and what a reader does about them differs. + // Naming the record in the second is the point: "somebody else's" is + // only useful when it says which somebody. + if d.Want == "" { + return fmt.Sprintf( + "another-run %s\n the record of another run that wrote into this directory. Nothing "+ + "described by this manifest is affected by it. Verify it on its own to check the files it lists", + d.Path) + } + return fmt.Sprintf( + "another-run %s\n written by the run that %s records, not by this one. Nothing described by "+ + "this manifest is missing because of it, and cleanup will not remove it - run cleanup on %s to remove it", + d.Path, d.Want, d.Want) case Respelled: return fmt.Sprintf( "respelled %s\n the manifest calls this file %s. The letters differ only in a way this "+ @@ -262,6 +300,7 @@ func Verify(ctx context.Context, dir string, m *manifest.Manifest, skip string) // above asks before it, and Verify ends on ctx.Err() - so a check would be a // branch no test could ever redden, which this project removes rather than // keeps. + var unclaimed []string for _, p := range present { // Not normalised on this side, and that was measured rather than // decided. walk builds these with filepath.Rel, which returns a clean @@ -270,32 +309,13 @@ func Verify(ctx context.Context, dir string, m *manifest.Manifest, skip string) if seen[p] || filepath.Base(p) == skip { continue } - // Ours or somebody else's, and the reader needs to be told which. - kind := Extra - want := "" - switch { - case core.IsPartialName(filepath.Base(p)), - core.IsWritingName(filepath.Base(p)), - core.IsRunLockName(filepath.Base(p)): - // All three markers, because each names something this tool put - // here and did not take away. Only the first was recognised until - // 2026-09-06, so a half written manifest was reported as "extra" - - // the word that means somebody else put it here - and the third - // arrived with the run lock on 2026-09-07. They get different - // sentences in String, because what a reader should do about them - // differs, and about the lock it differs most: it is the only one - // that may belong to a run that is still going. - kind = Leftover - default: - // One file under two spellings reads as a polluted directory - // otherwise, and on a filesystem that ignores the difference it - // arrives on its own, without the "missing" that would give it - // away - os.Stat found the entry under the name the manifest - // gives. Measured on 2026-08-27. - if claimedAs, ok := folded[core.FoldName(p)]; ok { - kind, want = Respelled, claimedAs - } - } + unclaimed = append(unclaimed, p) + } + // Read before the loop rather than inside it, because a file the FIRST + // neighbour lists may sit before that neighbour's own record in the walk. + neighbours := findNeighbours(ctx, dir, unclaimed) + for _, p := range unclaimed { + kind, want := nameFor(p, folded, neighbours) diffs = append(diffs, Difference{Kind: kind, Path: p, Want: want}) } @@ -308,6 +328,48 @@ func Verify(ctx context.Context, dir string, m *manifest.Manifest, skip string) return diffs, ctx.Err() } +// nameFor says what a file the manifest does not claim actually is. +// +// Ours or somebody else's, and the reader needs to be told which. Extra is the +// last answer rather than the first, and three of the four ahead of it were put +// there by a report that had used the word about a file that was not anybody +// else's. +// +// Lifted out of the walk on 2026-09-07 when the fourth answer arrived. The loop +// it came from had a switch inside it and a chain inside that, and one more +// branch would have made the question harder to read than the answer. +func nameFor(p string, folded map[string]string, neighbours neighbourClaims) (Kind, string) { + base := filepath.Base(p) + // All three markers, because each names something this tool put here and + // did not take away. Only the first was recognised until 2026-09-06, so a + // half written manifest was reported as "extra" - the word that means + // somebody else put it here - and the third arrived with the run lock on + // 2026-09-07. They get different sentences in String, because what a reader + // should do about them differs, and about the lock it differs most: it is + // the only one that may belong to a run that is still going. + if core.IsPartialName(base) || core.IsWritingName(base) || core.IsRunLockName(base) { + return Leftover, "" + } + // One file under two spellings reads as a polluted directory otherwise, and + // on a filesystem that ignores the difference it arrives on its own, + // without the "missing" that would give it away - os.Stat found the entry + // under the name the manifest gives. Measured on 2026-08-27. + // + // Asked before the neighbours are, because this one is about a file THIS + // manifest describes. A neighbour that happens to list the same name does + // not make the spelling somebody else's problem. + if claimedAs, ok := folded[core.FoldName(p)]; ok { + return Respelled, claimedAs + } + if neighbours.records[p] { + return AnotherRun, "" + } + if by, ok := neighbours.claimedBy[p]; ok { + return AnotherRun, by + } + return Extra, "" +} + // comparablePath is the spelling two paths are matched under when one comes // from a manifest and the other from the disk. // diff --git a/internal/audit/neighbours.go b/internal/audit/neighbours.go new file mode 100644 index 0000000..550cc77 --- /dev/null +++ b/internal/audit/neighbours.go @@ -0,0 +1,140 @@ +// Part of package audit. See audit.go. +package audit + +import ( + "context" + "path/filepath" + "strings" + + "github.com/donislawdev/TestingFilesGenerator/internal/manifest" +) + +// What the OTHER runs recorded in a directory say they wrote. +// +// A directory is allowed to hold more than one run. output.manifest exists for +// exactly that, so that a second run records itself beside the first instead of +// being refused, and people use it - a set of fixtures per test suite, one +// directory. What verify did with it was call every one of the neighbour's +// files "extra", which is the word for a file nobody asked for, and the report +// then read as a directory somebody had polluted. +// +// Measured on 2026-09-07, two runs one after another into one directory with +// name templates that do not collide, both ending 0: +// +// verify manifest-alpha.json 3 differences, exit 7 +// verify manifest-beta.json 4 differences, exit 7 +// +// Every one of those differences was the other run's work, and which run had +// written it was recorded in the same directory the whole time. +// +// The rule this follows is ATTRIBUTION, NOT SUPPRESSION, and the difference +// matters more than the repair. Every file stays in the report. What changes is +// the word it is given and whether it makes the directory a mismatch. So +// untouchable rule 6 is kept literally rather than on trust, and a manifest +// somebody drops into a directory cannot hide a file - at most it can claim +// one, out loud, with its own name printed beside it. + +// neighbourClaims is what the other runs in this directory account for. +type neighbourClaims struct { + // records is the path of every file that is itself another run's manifest. + records map[string]bool + // claimedBy maps a file to the base name of the record listing it, which + // is what a reader needs in order to know which run to ask. + claimedBy map[string]string +} + +// findNeighbours reads the manifests of other runs sitting in this directory. +// +// candidates are the files this manifest does not claim, which is the only set +// worth looking at: anything our own manifest lists has already been compared +// against it, and a neighbour listing it too cannot change that answer. +// +// A candidate that cannot be read, or that is not a manifest, or that is a +// manifest this build refuses, is simply not a neighbour. Nothing is reported +// about it here and it keeps whatever verify would have called it. Guessing on +// a file we could not read is how a tool starts accounting for files nobody +// wrote. +func findNeighbours(ctx context.Context, dir string, candidates []string) neighbourClaims { + found := neighbourClaims{ + records: map[string]bool{}, + claimedBy: map[string]string{}, + } + for _, rel := range candidates { + if ctx.Err() != nil { + // A cancelled pass stops looking. The caller reports what it + // compared and calls nothing sound. + return found + } + if !couldBeNamedLikeARecord(rel) { + continue + } + full := filepath.Join(dir, filepath.FromSlash(rel)) + m, err := manifest.Load(full) + if err != nil { + continue + } + found.records[rel] = true + base := filepath.Base(rel) + found.take(m, base) + } + return found +} + +// take records what one neighbour says it wrote. +// +// Its own function rather than a loop inside a loop, which is the ceiling on +// how deeply this project nests talking. It is also the better shape to read: +// the caller decides WHICH files are records, and this decides what a record +// accounts for. +func (n neighbourClaims) take(m *manifest.Manifest, base string) { + for _, f := range Claimed(m) { + // First record wins, and the walk that produced the candidates is in a + // fixed order, so two neighbours claiming one file name the same one of + // themselves every time rather than whichever was read first on the day. + // + // The path is put through the same spelling both sides of the + // comparison use. Without it a neighbour writing "./a.txt" would claim + // nothing, which is the fault comparablePath was written for. + key := comparablePath(f.Path) + if _, taken := n.claimedBy[key]; !taken { + n.claimedBy[key] = base + } + } +} + +// couldBeNamedLikeARecord is the sieve that costs nothing, and it had to exist. +// +// Reading the first bytes of every file the manifest does not claim was the +// first design and it was measured out of existence on 2026-09-07: on a +// directory holding ten thousand unclaimed files it made verify several times +// slower, because opening a file on Windows is not free - a scanner sees every +// one of them. The exact factor is refused, because the canary runs of the +// unchanged binary disagreed with each other by a factor of seven while it was +// taken. What is not in doubt is that it was large enough to change the design. +// +// So a file is only opened when its name could be a record at all. This is a +// NARROWING rather than a guess about the world, and the difference is what +// makes it safe: a neighbour's manifest under some other extension is reported +// exactly as it was reported before any of this existed, as extra. The cost of +// the sieve being wrong is yesterday's answer rather than a wrong one. +// +// The extension is the one this tool writes and the one it documents. +// DefaultManifestName is manifest.json, the help for verify says +// "tfg verify ", and every preset and every example writes one. +// Written down in docs/SHARED-DIRECTORY-2026-09-07.md section 2.3 as a limit +// rather than left to be discovered. +// +// It is the only sieve, and a second one was written and taken out again the +// same day. That one read the first half kilobyte of every candidate and looked +// for the first key of a manifest, to keep a large JSON document that is not one +// from being read in full. Nothing could make it fail: a document that got past +// it was refused by manifest.Load's schema check anyway, so removing it changed +// no answer and no test could tell. A defence nothing can redden is not a +// defence, and this project takes those out rather than keeping them. +// +// What is left is the name, and then manifest.Load, which has a ceiling of its +// own - so the worst an unclaimed JSON document can cost is one read of at most +// that ceiling. +func couldBeNamedLikeARecord(rel string) bool { + return strings.EqualFold(filepath.Ext(rel), ".json") +} diff --git a/internal/cli/verify.go b/internal/cli/verify.go index 3f1effb..aff76db 100644 --- a/internal/cli/verify.go +++ b/internal/cli/verify.go @@ -9,6 +9,8 @@ import ( "io" "os" "path/filepath" + "sort" + "strings" "github.com/donislawdev/TestingFilesGenerator/internal/audit" "github.com/donislawdev/TestingFilesGenerator/internal/core" @@ -95,22 +97,51 @@ Flags: return reportVerify(diffs, claimed, path, dir, *asJSON, out, errOut) } +// mismatches is how many of these differences say the directory disagrees with +// this manifest. +// +// Not the same as how many differences there are, and the gap is the point. A +// file another run's manifest lists is a difference worth printing and it is +// not a disagreement with THIS one - the directory holds exactly what this +// manifest says it holds. Counting it as a mismatch made a shared directory +// permanently red, which is the same as having no check at all in the place +// output.manifest exists for. +// +// Every other kind still counts, the leftovers included. A leftover is ours and +// nothing is lost by it, and that is an argument for a different exit code that +// nobody has made or measured - so it keeps the one it has. +func mismatches(diffs []audit.Difference) int { + n := 0 + for _, d := range diffs { + if d.Kind != audit.AnotherRun { + n++ + } + } + return n +} + // reportVerify says what the comparison found, as JSON or as prose. func reportVerify(diffs []audit.Difference, claimed int, path, dir string, asJSON bool, out, errOut io.Writer) int { + wrong := mismatches(diffs) if asJSON { report := verifyReport{ Manifest: path, Directory: dir, Checked: claimed, - Matched: len(diffs) == 0, + Matched: wrong == 0, Difference: []verifyDifference{}, } + // Every difference is carried, including the ones that do not make the + // directory a mismatch. A reader that wants only the disagreements + // filters on kind, and a reader that wants to know what else is in the + // directory has it. Dropping them would be the suppression this repair + // is written to avoid. for _, d := range diffs { report.Difference = append(report.Difference, verifyDifference{ Kind: string(d.Kind), Path: d.Path, Expected: d.Want, Found: d.Got, }) } - if len(diffs) > 0 { + if wrong > 0 { // A failed run puts nothing on stdout, so the machine readable // report of a mismatch goes to stderr with the rest of the news. return writeJSON(errOut, errOut, report, ExitVerify) @@ -118,11 +149,10 @@ func reportVerify(diffs []audit.Difference, claimed int, path, dir string, asJSO return writeJSON(out, errOut, report, ExitOK) } - if len(diffs) > 0 { - fmt.Fprintf(errOut, "tfg: %s does not match %s - %s:\n", dir, path, core.Count(len(diffs), "difference", "differences")) - for _, d := range diffs { - fmt.Fprintln(errOut, " "+d.String()) - } + if wrong > 0 { + fmt.Fprintf(errOut, "tfg: %s does not match %s - %s:\n", dir, path, core.Count(wrong, "difference", "differences")) + echoMismatches(diffs, errOut) + echoOtherRuns(diffs, errOut) return ExitVerify } @@ -131,12 +161,110 @@ func reportVerify(diffs []audit.Difference, claimed int, path, dir string, asJSO // that never happened. if claimed == 0 { fmt.Fprintf(errOut, "%s claims no files, so there was nothing to check.\n", path) + echoOtherRuns(diffs, errOut) return ExitOK } fmt.Fprintf(out, "%s matches %s: %s checked\n", dir, path, core.Count(claimed, "file", "files")) + echoOtherRuns(diffs, errOut) return ExitOK } +// echoMismatches lists the differences that are disagreements with THIS +// manifest. +// +// The neighbour's files are counted out of the heading, so they are listed out +// of the list too - they come back below, grouped, which is the only shape that +// survives a neighbour who wrote ten thousand files. A heading that says one +// number over a list of another is a report somebody stops reading. +func echoMismatches(diffs []audit.Difference, errOut io.Writer) { + for _, d := range diffs { + if d.Kind == audit.AnotherRun { + continue + } + fmt.Fprintln(errOut, " "+d.String()) + } +} + +// otherRunExamples is how many file names one of these lines shows before it +// stops listing and starts counting. +// +// The same three the manifest's notes use, for the same measured reason: a run +// of 25 000 files once put 25 001 note lines on stderr, one per file, and the +// one line that mattered was buried under them. A neighbour can be that big. +const otherRunExamples = 3 + +// echoOtherRuns says what else is in the directory, and whose it is. +// +// One line per neighbouring record rather than one per file, which is what +// makes it safe to print at all. The number of records in a directory is small +// by construction - each one is a run that recorded itself - and the number of +// files is not. +// +// On the news channel rather than on stdout, beside the other things this tool +// says about a run that worked. The line before it is the answer somebody asked +// for and a script may be reading it. +func echoOtherRuns(diffs []audit.Difference, errOut io.Writer) { + byRecord := groupedByRecord(diffs) + + names := make([]string, 0, len(byRecord)) + for name := range byRecord { + names = append(names, name) + } + sort.Strings(names) + + for _, name := range names { + files := byRecord[name] + if len(files) == 0 { + fmt.Fprintf(errOut, "note: %s is another run's record, and nothing else here belongs to it.\n", name) + continue + } + fmt.Fprintf(errOut, "note: %s is another run's record. %s here %s to it: %s.\n", + name, core.Count(len(files), "file", "files"), belongs(len(files)), someOf(files)) + } +} + +// groupedByRecord collects the neighbours' files under the record that lists +// them. +// +// Its own function rather than a loop inside echoOtherRuns, which is the +// ceiling on nesting. A record gets an entry even when nothing else here is +// its, or a directory holding only somebody's manifest would say nothing about +// the one file in it that is not ours. +func groupedByRecord(diffs []audit.Difference) map[string][]string { + byRecord := map[string][]string{} + for _, d := range diffs { + switch { + case d.Kind != audit.AnotherRun: + case d.Want != "": + byRecord[d.Want] = append(byRecord[d.Want], d.Path) + default: + name := filepath.Base(d.Path) + if _, seen := byRecord[name]; !seen { + byRecord[name] = nil + } + } + } + return byRecord +} + +// someOf names the first few and counts the rest. +func someOf(names []string) string { + if len(names) <= otherRunExamples { + return strings.Join(names, ", ") + } + return strings.Join(names[:otherRunExamples], ", ") + ", and " + + core.Count(len(names)-otherRunExamples, "file", "files") + " not named here" +} + +// belongs is the verb for that sentence. A number carries a noun in the right +// number on both surfaces, and it carries a verb too. +func belongs(n int) string { + if n == 1 { + return "belongs" + } + return "belong" +} + // cleanup removes the files a manifest lists, and nothing else. type verifyReport struct { diff --git a/internal/guard/anotherrun_test.go b/internal/guard/anotherrun_test.go new file mode 100644 index 0000000..f8c927b --- /dev/null +++ b/internal/guard/anotherrun_test.go @@ -0,0 +1,283 @@ +package guard + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/donislawdev/TestingFilesGenerator/internal/audit" + "github.com/donislawdev/TestingFilesGenerator/internal/cli" + "github.com/donislawdev/TestingFilesGenerator/internal/engine" + "github.com/donislawdev/TestingFilesGenerator/internal/manifest" +) + +// Two runs are allowed to share a directory, and verify used to call one of +// them a mismatch caused by the other. +// +// output.manifest exists so that a second run can record itself beside the +// first instead of being refused. Measured on 2026-09-07 with two runs one +// after another into one directory, name templates that do not collide, both +// ending 0: +// +// verify manifest-alpha.json 3 differences, exit 7 +// extra b_0001.txt extra b_0002.txt extra manifest-beta.json +// verify manifest-beta.json 4 differences, exit 7 +// extra a_0001.txt extra a_0002.txt extra a_0003.txt extra manifest-alpha.json +// +// "extra" is the word for a file nobody asked for, so the report read as a +// directory somebody had polluted. Every one of those files was the +// neighbour's, and which neighbour was written down in the same directory the +// whole time. +// +// The rule the repair follows is ATTRIBUTION, NOT SUPPRESSION, and the guards +// below are split along that line. Every file stays in the report. What changes +// is the word it is given and whether it makes the directory a mismatch - so +// untouchable rule 6 is kept literally rather than on trust, and a manifest +// somebody drops into a directory can claim a file out loud and cannot hide one. + +// twoRunsSharing writes two runs into one directory, whose files do not +// collide, and gives back the directory and the first run's manifest. +func twoRunsSharing(t *testing.T) (dir string, alpha *manifest.Manifest, alphaPath string) { + t.Helper() + dir = t.TempDir() + + run := func(id, name, manifestName string, count int) (*manifest.Manifest, string) { + opt := engine.Options{OutDir: dir, ManifestName: manifestName, Seed: 11, Command: "test"} + target := txtTarget(id, count, 2048) + target.NameTmpl = name + planned, err := engine.Plan([]engine.Target{target}, opt) + if err != nil { + t.Fatalf("planning %s: %v", id, err) + } + res, err := engine.Run(context.Background(), planned, opt) + if err != nil { + t.Fatalf("running %s: %v", id, err) + } + path := engine.ManifestPath(opt) + if err := res.Manifest.Save(path); err != nil { + t.Fatalf("saving the manifest of %s: %v", id, err) + } + return res.Manifest, path + } + + alpha, alphaPath = run("alpha", "a_{index:04}.txt", "manifest-alpha.json", 3) + run("beta", "b_{index:04}.txt", "manifest-beta.json", 2) + return dir, alpha, alphaPath +} + +func TestVerifyNamesTheFilesOfAnotherRunRatherThanCallingThemExtra(t *testing.T) { + dir, alpha, _ := twoRunsSharing(t) + + diffs, err := audit.Verify(context.Background(), dir, alpha, "manifest-alpha.json") + if err != nil { + t.Fatalf("verifying: %v", err) + } + + // Attribution, not suppression: the neighbour's two files AND its record + // are all still here. Naming them correctly must not mean dropping them. + want := map[string]string{ + "b_0001.txt": "manifest-beta.json", + "b_0002.txt": "manifest-beta.json", + "manifest-beta.json": "", + } + if len(diffs) != len(want) { + t.Fatalf("expected the neighbour's two files and its record, got %v.\n"+ + "Silence is banned, so naming them correctly cannot mean leaving them out", diffs) + } + for _, d := range diffs { + claimedBy, known := want[d.Path] + if !known { + t.Errorf("unexpected difference %v", d) + continue + } + if d.Kind != audit.AnotherRun { + t.Errorf("verify called %s %q. It belongs to the run manifest-beta.json records, and %q is the word for a file nobody asked for", + d.Path, d.Kind, audit.Extra) + } + if d.Want != claimedBy { + t.Errorf("%s says it is claimed by %q and it is claimed by %q - a reader has to be told WHICH run to ask", + d.Path, d.Want, claimedBy) + } + } +} + +// A file no manifest in the directory lists is still extra. +// +// This is the half that keeps the repair from being a way of accepting +// anything. Without it, dropping a manifest into a directory would account for +// every file in it. +// +// Two shapes, because they fail differently. A file nothing claims is the +// ordinary one. A JSON document that is not a manifest is the one somebody +// would reach for to test the sieve, and it has to come back extra as well - it +// gets past the name and is refused by the schema check behind it. +func TestAStrayFileBesideAnotherRunsRecordIsStillExtra(t *testing.T) { + dir, alpha, _ := twoRunsSharing(t) + strays := map[string]string{ + "somebody-elses.txt": "not ours", + "settings.json": `{"files":[{"path":"b_0001.txt"}]}`, + } + for name, body := range strays { + if err := os.WriteFile(filepath.Join(dir, name), []byte(body), 0o644); err != nil { + t.Fatalf("writing %s: %v", name, err) + } + } + + diffs, err := audit.Verify(context.Background(), dir, alpha, "manifest-alpha.json") + if err != nil { + t.Fatalf("verifying: %v", err) + } + seen := map[string]bool{} + for _, d := range diffs { + if _, isStray := strays[d.Path]; !isStray { + continue + } + seen[d.Path] = true + if d.Kind != audit.Extra { + t.Errorf("%s was called %q. Nothing in this directory claims it, and the presence of somebody's manifest cannot make every file in the directory accounted for", + d.Path, d.Kind) + } + } + for name := range strays { + if !seen[name] { + t.Errorf("%s is not in the report at all", name) + } + } +} + +// A record under another extension is reported the way it was before any of +// this existed, and that limit is pinned rather than left to be discovered. +// +// Only files named like a record are opened, because reading the first bytes of +// every unclaimed file was measured and was too expensive - see +// couldBeNamedLikeARecord. The cost of that narrowing is exactly this: a +// neighbour who called their manifest something else is still "extra". +// +// Pinned so that widening it later is a decision somebody makes on purpose, +// with this guard in front of them, rather than a line quietly deleted. +func TestARecordUnderAnotherExtensionIsStillExtra(t *testing.T) { + dir, alpha, _ := twoRunsSharing(t) + renamed := filepath.Join(dir, "manifest-beta.record") + if err := os.Rename(filepath.Join(dir, "manifest-beta.json"), renamed); err != nil { + t.Fatalf("renaming the neighbour's record: %v", err) + } + + diffs, err := audit.Verify(context.Background(), dir, alpha, "manifest-alpha.json") + if err != nil { + t.Fatalf("verifying: %v", err) + } + for _, d := range diffs { + if d.Kind == audit.AnotherRun { + t.Errorf("%s was attributed to a record this build does not open, so the sieve is not the one that was measured", d.Path) + } + } +} + +// The sentence tells a reader which run to ask. +func TestTheSentenceAboutAnotherRunNamesTheRecordThatClaimsTheFile(t *testing.T) { + dir, alpha, _ := twoRunsSharing(t) + diffs, err := audit.Verify(context.Background(), dir, alpha, "manifest-alpha.json") + if err != nil { + t.Fatalf("verifying: %v", err) + } + for _, d := range diffs { + if d.Path != "b_0001.txt" { + continue + } + if said := d.String(); !strings.Contains(said, "manifest-beta.json") { + t.Errorf("the sentence does not say which run wrote this file, so a reader still has to guess:\n %s", said) + } + return + } + t.Fatal("the neighbour's file is not in the report, so this guard checked nothing") +} + +// A shared directory is not a mismatch, and that is what makes output.manifest +// usable in the place it exists for. +// +// A check that is red whenever it worked is the same as no check at all, and +// this one sits in CI by design. +func TestADirectorySharedWithAnotherRunStillMatchesThisManifest(t *testing.T) { + _, _, alphaPath := twoRunsSharing(t) + + code, stdout, errOut := run(t, "verify", alphaPath) + if code != cli.ExitOK { + t.Errorf("verify gave %d on a directory that holds exactly what this manifest says it holds.\n%s", code, errOut) + } + if !strings.Contains(stdout, "matches") { + t.Errorf("the answer somebody asked for is not on stdout:\n%s", stdout) + } + // Said, not hidden. One line per neighbouring record rather than one per + // file, because a neighbour can have written twenty five thousand of them. + if !strings.Contains(errOut, "manifest-beta.json") { + t.Errorf("nothing was said about the other run in the directory, so its files are now invisible:\n%s", errOut) + } + if strings.Count(errOut, "\n") > 2 { + t.Errorf("the note is one line per file rather than one per record:\n%s", errOut) + } +} + +// A real disagreement is still a mismatch, neighbour or no neighbour. +func TestARealDifferenceIsStillAMismatchInASharedDirectory(t *testing.T) { + dir, _, alphaPath := twoRunsSharing(t) + if err := os.Remove(filepath.Join(dir, "a_0001.txt")); err != nil { + t.Fatalf("removing one of our own files: %v", err) + } + + code, _, errOut := run(t, "verify", alphaPath) + if code != cli.ExitVerify { + t.Fatalf("a missing file gave %d rather than %d - the neighbour cannot make our own loss acceptable", code, cli.ExitVerify) + } + if !strings.Contains(errOut, "1 difference") { + t.Errorf("the heading counts the neighbour's files as disagreements:\n%s", errOut) + } + if !strings.Contains(errOut, "missing") { + t.Errorf("the missing file is not named:\n%s", errOut) + } +} + +// The machine readable report carries every file, whatever it is called. +// +// This is the guard for attribution against suppression, in the form a script +// reads. A reader that wants only the disagreements filters on kind, and one +// that wants to know what else is in the directory has it. +func TestTheJSONReportCarriesTheOtherRunsFilesAndStillSaysItMatched(t *testing.T) { + _, _, alphaPath := twoRunsSharing(t) + + code, stdout, _ := run(t, "verify", alphaPath, "--json") + if code != cli.ExitOK { + t.Fatalf("verify --json gave %d on a shared directory", code) + } + var report struct { + Matched bool `json:"matched"` + Differences []struct { + Kind string `json:"kind"` + Path string `json:"path"` + Want string `json:"expected"` + } `json:"differences"` + } + if err := json.Unmarshal([]byte(stdout), &report); err != nil { + t.Fatalf("stdout is not JSON: %v\n%s", err, stdout) + } + if !report.Matched { + t.Error("matched is false about a directory that holds exactly what this manifest says it holds") + } + got := map[string]string{} + for _, d := range report.Differences { + if d.Kind != string(audit.AnotherRun) { + t.Errorf("%s is reported as %q", d.Path, d.Kind) + } + got[d.Path] = d.Want + } + for _, name := range []string{"b_0001.txt", "b_0002.txt", "manifest-beta.json"} { + if _, ok := got[name]; !ok { + t.Errorf("%s is not in the report at all. Calling a file the neighbour's cannot mean not mentioning it", name) + } + } + if got["b_0001.txt"] != "manifest-beta.json" { + t.Errorf("the report does not say which run claims b_0001.txt, it says %q", got["b_0001.txt"]) + } +} diff --git a/internal/guard/branching_test.go b/internal/guard/branching_test.go index 9a9fcd7..9f486fd 100644 --- a/internal/guard/branching_test.go +++ b/internal/guard/branching_test.go @@ -18,7 +18,12 @@ import ( // signatures the day it went in - and a ratchet that arrives red gets raised // to make it pass, which is how a ratchet becomes a rubber band. const ( - worstComplexity = 22 // internal/recipe/target.go rawTarget.resolveSize + // The number has not moved. Who holds it has: the guard's own log named + // internal/recipe/target.go rawTarget.resolveSize until 2026-09-07, and + // names internal/engine/engine.go Run since the run lock went in there. + // Written down because a comment naming the wrong function sends the next + // reader to flatten something that is not the one at the ceiling. + worstComplexity = 22 // internal/engine/engine.go Run mostArguments = 9 // internal/cli/cleanup.go applyCleanup // Where each axis counts as on its way to the ceiling. Three quarters for