From 80ed9479fe150cac9fdcfbc3c63acd55c4e17a1b Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Sun, 6 Sep 2026 21:48:03 +0200 Subject: [PATCH] fix: a note is reported once per thing it says, not once per file A run of 25 000 one byte text files emitted 25 001 "note:" lines on stderr, one per file, every one of them the same sentence about the label not fitting. The advice was per file for a decision that is per target. That is a stability question rather than a cosmetic one because of what it buried. A run whose manifest will be too big for this build to read back warns about it first, in a "note:" line typographically identical to the 25 000 that follow it - and that warning is the only thing standing between somebody and a directory that neither verify nor cleanup can ever read. TestARunSaysWhenItsManifestWillBeTooBigToReadBack proves the sentence is printed. It could not prove anybody would see it. Measured: 25 003 stderr lines become 4, and 25 001 notes become 2. The same reasoning was already applied to the progress bar, which is throttled and silent when stderr is not a terminal because "thousands of redrawn lines in a CI log are worse than no bar". Notes had not had it applied to them. A note about a single file still leads with that file's name. That is the sharper of the three guards: grouping that dropped the name would pass every count and example check while making the common case worse than it was. The manifest does not change, and that was measured rather than reasoned about - the binary before and after, same recipe and seed and the same relative output directory: manifest identical apart from generated_at and run.id, 40 notes over 40 files still stored, all 40 files byte for byte the same. The first comparison reported a difference and that was my instrument: the two runs wrote to different directories, so run.command differed. Split into noteGroups.add and noteGroup.line because the ratchet on nesting went from 51 to 52. Flattened rather than raised, and it reads better for it. Four mutations, all caught, three of them repointed after the flattening. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 21 +++++ internal/guard/notesgrouping_test.go | 111 +++++++++++++++++++++++++++ internal/manifest/manifest.go | 96 ++++++++++++++++++++++- 3 files changed, 225 insertions(+), 3 deletions(-) create mode 100644 internal/guard/notesgrouping_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ce7936..88474a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -149,6 +149,27 @@ because it turns other people's test suites red. ### Changed +- **Notes are reported once per thing they say, not once per file.** A run of + 25 000 one-byte text files used to print 25 001 `note:` lines, every one of + them the same sentence about the label not fitting. It now prints one, with + the count and the first three names: + + ``` + note: 25000 files: The label needs 32 B and the file is 1 B, so this file + carries no label. Its name and the manifest still identify it. Named: + files_0001.txt, files_0002.txt, files_0003.txt. 24997 files not named here. + ``` + + A note about a single file still leads with that file's name, unchanged. + + The reason this matters beyond tidiness: a run whose manifest will be too big + for this build to read back warns you first, in a line that looked exactly + like the 25 000 that followed it. That warning is the only thing standing + between you and a directory that `verify` and `cleanup` can never read. + + **Nothing in the manifest changes.** Every entry still carries its own note, + where a machine reads it and nothing scrolls. + - **Building from source now needs Go 1.27.0.** It used to say 1.26.5, and that sentence was true of compiling and false of the product. Go 1.27 changed `compress/flate`, so a copy built on 1.26 answers the same version number, diff --git a/internal/guard/notesgrouping_test.go b/internal/guard/notesgrouping_test.go new file mode 100644 index 0000000..82fdc41 --- /dev/null +++ b/internal/guard/notesgrouping_test.go @@ -0,0 +1,111 @@ +package guard + +import ( + "strings" + "testing" + + _ "github.com/donislawdev/TestingFilesGenerator/internal/format/all" +) + +// A note is reported once per thing it says, not once per file. +// +// Measured on 2026-09-06: a run of 25 000 one byte text files emitted 25 001 +// "note:" lines on stderr, one per file, every one of them the same sentence +// about the label not fitting. Two consequences, and the second is why this is +// a stability question rather than a cosmetic one. +// +// The advice was per file for a decision that is per target. And the one line +// that matters was buried under them: a run whose manifest will be too big for +// this build to read back says so first, in a "note:" line typographically +// identical to the 25 000 that follow it, and that line is the only thing +// standing between somebody and a directory neither verify nor cleanup can +// ever read. TestARunSaysWhenItsManifestWillBeTooBigToReadBack proves the +// sentence is printed. It cannot prove anybody can see it. +// +// The same reasoning was already applied to the progress bar - throttled and +// silent when stderr is not a terminal, because "thousands of redrawn lines in +// a CI log are worse than no bar". Notes had not had it applied to them. +func TestNotesAreGroupedByWhatTheySayRatherThanOneLinePerFile(t *testing.T) { + const files = 400 + + said := runCLI(t, "generate", "--format", "txt", "--size", "1b", + "--count", itoa(files), "--dry-run", "--out", t.TempDir()) + + notes := 0 + for _, line := range strings.Split(said, "\n") { + if strings.HasPrefix(strings.TrimSpace(line), "note:") { + notes++ + } + } + + // Asserted rather than assumed. Nought notes would pass every check below + // by saying nothing at all, which is the shape this project keeps meeting: + // a guard that stopped reaching the state it guards. + if notes == 0 { + t.Fatal("the run printed no notes at all, so this guard checked nothing. A one byte " + + "text file cannot carry a label, and saying so is the note this is about.") + } + if notes >= files { + t.Errorf("%d files produced %d note lines, which is one per file:\n%s\n"+ + "What to do: group them by what the note says. A reader cannot find the line "+ + "that matters in a list this long.", files, notes, said) + } + + // The count has to be there, or grouping has thrown away how many files + // this is about and the reader is worse off than with one line each. + if !strings.Contains(said, itoa(files)+" files:") { + t.Errorf("the grouped note does not say how many files it covers:\n%s", said) + } + // And some names, because "400 files carry no label" with no name at all + // gives nobody a place to start looking. + if !strings.Contains(said, "files_0001.txt") { + t.Errorf("the grouped note names no file at all:\n%s", said) + } + // And what it is not showing. + if !strings.Contains(said, "not named here") { + t.Errorf("the grouped note does not say that most of the files are unnamed, so the "+ + "three it lists read as the only ones:\n%s", said) + } +} + +// One file keeps its name in front, the way it always had it. +// +// The sharp half of the pair. Grouping that dropped the file name would pass +// every check above - the counts and the examples would all be there - while +// making the common case, a single file with something to say about it, worse +// than it was before. +func TestANoteAboutOneFileStillNamesThatFileFirst(t *testing.T) { + said := runCLI(t, "generate", "--format", "txt", "--size", "1b", + "--count", "1", "--dry-run", "--out", t.TempDir()) + + if !strings.Contains(said, "note:") { + t.Fatal("the run printed no note, so this guard checked nothing") + } + if !strings.Contains(said, "note: files_0001.txt: ") { + t.Errorf("a note about one file no longer leads with that file's name:\n%s", said) + } + if strings.Contains(said, "1 files") { + t.Errorf("the note says \"1 files\":\n%s", said) + } + if strings.Contains(said, "not named here") { + t.Errorf("a note about one file claims to be hiding others:\n%s", said) + } +} + +// The number carries the right noun at the boundary where it changes. +// +// Four files with three named leaves exactly one unnamed, which is the only +// count at which the plural is wrong in a way a reader notices. core.Count +// exists for this and the surrounding sentence is a participle, so nothing in +// it agrees with the number. +func TestTheUnnamedRemainderCarriesTheRightNoun(t *testing.T) { + said := runCLI(t, "generate", "--format", "txt", "--size", "1b", + "--count", "4", "--dry-run", "--out", t.TempDir()) + + if !strings.Contains(said, "1 file not named here") { + t.Errorf("four files with three named should leave \"1 file not named here\":\n%s", said) + } + if strings.Contains(said, "1 files not named") { + t.Errorf("the remainder says \"1 files\":\n%s", said) + } +} diff --git a/internal/manifest/manifest.go b/internal/manifest/manifest.go index b7dc856..9e534bc 100644 --- a/internal/manifest/manifest.go +++ b/internal/manifest/manifest.go @@ -300,19 +300,109 @@ func (m *Manifest) Add(f File) { } } +// noteExamples is how many file names a grouped note shows before it stops +// listing and starts counting. +// +// Three rather than one, because one name reads as if one file were special +// when the note is about a whole target, and three shows the shape of the +// naming without becoming a list. +const noteExamples = 3 + // Notes gathers every note in the run, so a caller can report them without // walking the entries itself. +// +// GROUPED BY WHAT THE NOTE SAYS, not one line per file, and that is the whole +// point of it. Measured on 2026-09-06: a run of 25 000 one byte text files +// emitted 25 001 "note:" lines on stderr, one per file, every one of them the +// same sentence about the label not fitting. The advice was per file for a +// decision that is per target. +// +// The line that matters was buried under them. A run whose manifest will be +// too big for this build to read back says so first, in a "note:" line +// typographically identical to the 25 000 that follow it - and that one is the +// only thing standing between somebody and a directory neither verify nor +// cleanup can ever read. See echoManifestReach in cli/generate.go. +// +// The same reasoning was already applied to the progress bar, which is +// throttled to 10 Hz and silent when stderr is not a terminal, on the grounds +// that "thousands of redrawn lines in a CI log are worse than no bar" +// (cli/progress.go). Notes had not had it applied to them. +// +// The stored notes do not change. This is a view for a reader - every entry +// keeps its own note in the manifest, where a machine reads it and nothing +// scrolls. +// +// Names are not sorted here and the count is kept rather than the names, so a +// million entry run no longer sorts a million strings to print sixteen lines. func (m *Manifest) Notes() []string { - var out []string + groups := noteGroups{byDetail: map[string]*noteGroup{}} for _, f := range m.Files { for _, n := range f.Notes { - out = append(out, fmt.Sprintf("%s: %s", f.Name, n.Detail)) + groups.add(n.Detail, f.Name) } } - sort.Strings(out) + sort.Strings(groups.order) + + out := make([]string, 0, len(groups.order)) + for _, detail := range groups.order { + out = append(out, groups.byDetail[detail].line(detail)) + } return out } +// noteGroup is one sentence and the files that carry it. +// +// The names are kept only up to noteExamples and the rest is a count, so a +// million entry run holds a handful of strings rather than a million. +type noteGroup struct { + count int + first []string +} + +// noteGroups collects them, keeping the order the details were first seen in. +type noteGroups struct { + byDetail map[string]*noteGroup + order []string +} + +// add records one note against the file that carries it. +// +// The first few names are taken in the order the manifest lists them rather +// than sorted, which is what makes this cheap AND deterministic: the manifest's +// own order is fixed, guarded by auditorder_test.go, so the same recipe names +// the same few files every time. +func (n *noteGroups) add(detail, name string) { + g := n.byDetail[detail] + if g == nil { + g = ¬eGroup{} + n.byDetail[detail] = g + n.order = append(n.order, detail) + } + g.count++ + if len(g.first) < noteExamples { + g.first = append(g.first, name) + } +} + +// line renders one group for a person to read. +// +// A group of one keeps the shape it always had - the file name in front - so +// the common case of one file with something to say about it does not get worse +// to make the large case better. +func (g *noteGroup) line(detail string) string { + if g.count == 1 { + return fmt.Sprintf("%s: %s", g.first[0], detail) + } + named := strings.Join(g.first, ", ") + if hidden := g.count - len(g.first); hidden > 0 { + return fmt.Sprintf("%s: %s Named: %s. %s not named here.", + core.Count(g.count, "file", "files"), detail, named, + core.Count(hidden, "file", "files")) + } + return fmt.Sprintf("%s: %s Named: %s.", + core.Count(g.count, "file", "files"), detail, named) +} + // Encode renders the manifest as JSON. func (m *Manifest) Encode(w io.Writer) error { enc := json.NewEncoder(w)