diff --git a/CHANGELOG.md b/CHANGELOG.md index e2de7fd..7a4e4e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -299,6 +299,22 @@ because it turns other people's test suites red. ### Fixed +- **The window now warns when a run's record will be too big for this build to + read back.** The command line has said this since the ceiling was measured. + The window said nothing at all, so somebody who generated 25 000 files from it + was left with a directory that `tfg verify` and `tfg cleanup` both refuse - + and the manifest is the only authority over what may be removed, so those + files could never be cleaned up by this tool again. + + It appears in two places, because the window cannot speak in the middle of a + run: under Preview, which is the window's answer to `--dry-run`, and again + when a run finishes. The second is the one that matters, since Preview is a + button somebody may never press. It comes straight after the line saying what + the run did, ahead of any other note. + + The run itself still works and is still not refused. What was missing was that + nobody was told. + - **`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 diff --git a/internal/guard/windowmanifestreach_test.go b/internal/guard/windowmanifestreach_test.go new file mode 100644 index 0000000..cbecd4b --- /dev/null +++ b/internal/guard/windowmanifestreach_test.go @@ -0,0 +1,222 @@ +package guard + +import ( + "strings" + "testing" + + "github.com/donislawdev/TestingFilesGenerator/internal/gui/parts" + "github.com/donislawdev/TestingFilesGenerator/internal/gui/text" + "github.com/donislawdev/TestingFilesGenerator/internal/manifest" +) + +// The window says the same thing the command line says about a record too big +// to read back. +// +// Observation O184, measured on 2026-09-06: TooLargeToReadBack had two callers, +// both in internal/cli and internal/manifest, and NOT ONE in internal/gui. A +// person who generated 25 000 files from the window was told nothing at all, +// and was left with a directory that tfg verify and tfg cleanup both refuse - +// the manifest is the only authority over what may be removed, so a manifest +// that cannot be read is a set of files with no owner. +// +// It is the kind of parity gap D1 loses most easily. Not something the engine +// can do from one surface and not the other, which is what the parity guard +// looks for, but something one surface SAYS and the other does not. +// +// The note was written down as a question about the manifest schema, on the +// grounds that notes are per file and this one is per run. It is not. The +// command line does not read this off the manifest either - it works it out +// from the plan and prints it before the first byte - and manifest.TooLarge- +// ToReadBack was put where it is exactly so the two surfaces could not come to +// different conclusions about one run. What was missing was a caller. + +// overTheCeiling is a file count whose manifest this build would refuse. +// +// Worked out from the estimate rather than written here, for the reason the +// command line guard beside it gives: a guard carrying its own copy of a limit +// goes stale the day somebody changes the real one, and says nothing while it +// does. +func overTheCeiling(t *testing.T) int { + t.Helper() + over := int(manifest.MaxBytes/manifest.BytesPerEntry) + 1000 + if _, tooBig := manifest.TooLargeToReadBack(over, 0); !tooBig { + t.Fatalf("%d entries was not judged too large, so this guard would prove nothing", over) + } + return over +} + +// previewOf presses Preview for a run of count files and gives back what the +// screen said. +// +// It REFUSES to return a refusal, and that is the whole reason it exists. The +// first version of the pair below set a size the default format will not take - +// the window opens on the first format in the registry, which is avif, and +// 200 B is far under what a picture needs. Both previews were turned down, so +// the negative half passed while proving nothing: a screen that says "check the +// settings marked above" says nothing about a manifest ceiling either. +func previewOf(t *testing.T, count string) string { + t.Helper() + content, w, host := screenInAWindowWithHost(t, text.TabOneTarget()) + + // txt rather than whatever the window opens on, for two reasons. The size + // below has to be one the format takes, and planning twenty two thousand + // pictures would encode twenty two thousand pictures - png, jpg, gif and + // avif all do that while planning. + picker, ok := controlUnder(content, text.FieldFormat()).(*parts.Chooser) + if !ok { + t.Fatal("the format field is not a list to choose from, so this guard read the wrong tree") + } + picker.SetSelected("txt") + + // Small files, because what is being asked about is the number of ENTRIES + // rather than the number of bytes. A preview writes nothing either way. + fill(t, content, text.FieldSize(), "200b") + fill(t, content, text.FieldCount(), count) + + press(t, content, text.ButtonPreview()) + // This preview is accepted, so it answers from a worker. Joined before the + // status line is read - see join. + join(host) + settle(content, w) + + _, status := runMessages(content) + if status == nil { + t.Fatal("the screen has no status line, so this guard read the wrong tree") + } + // Matched on the tail of the preview's own sentence, the way the action bar + // guard does it, so this cannot be satisfied by a refusal. + marker := text.PreviewCost(1, nil, "1 B") + tail := marker[strings.LastIndex(marker, " ")+1:] + if !strings.Contains(status.Text, tail) { + t.Fatalf("the preview of %s files was not accepted, so nothing here was asked about the manifest.\nIt said:\n%s", + count, status.Text) + } + return status.Text +} + +// runOf presses Generate rather than Preview, and gives back what the screen +// said when it finished. +// +// It exists because the preview is OPTIONAL. Somebody who presses Generate +// straight away never sees the preview's answer, and that person is exactly the +// one observation O184 is about - they end up with a directory nothing in this +// toolset can read or clean. The window cannot say anything in the middle of a +// run, so the end of the run is the only place left. +// +// It writes files, which is why this is the one guard here that does. Twenty +// two thousand of them at 200 B, into a directory that goes away with the test. +func runOf(t *testing.T, count string) string { + t.Helper() + content, w, host := screenInAWindowWithHost(t, text.TabOneTarget()) + + picker, ok := controlUnder(content, text.FieldFormat()).(*parts.Chooser) + if !ok { + t.Fatal("the format field is not a list to choose from, so this guard read the wrong tree") + } + picker.SetSelected("txt") + fill(t, content, text.FieldSize(), "200b") + fill(t, content, text.FieldCount(), count) + fill(t, content, text.FieldOutputDir(), t.TempDir()) + + press(t, content, text.ButtonGenerate()) + join(host) + settle(content, w) + + _, status := runMessages(content) + if status == nil { + t.Fatal("the screen has no status line, so this guard read the wrong tree") + } + // The run has to have HAPPENED. A refused run says nothing about a + // manifest either, and a guard that cannot tell those apart is the shape + // this project has recorded as passing without reaching the code. + if !strings.Contains(status.Text, text.Written(0)[strings.LastIndex(text.Written(0), " ")+1:]) { + t.Fatalf("the run of %s files did not finish, so nothing here was asked about the manifest.\nIt said:\n%s", + count, status.Text) + } + return status.Text +} + +// warningAbout is the fixed half of the sentence, without the two numbers. +// +// Taken from the text package rather than typed here, so a reworded warning +// does not quietly stop being checked. +func warningAbout(t *testing.T) string { + t.Helper() + marker := text.ManifestTooLargeToRead("SIZE", "LIMIT") + at := strings.Index(marker, "SIZE") + if at < 0 { + t.Fatal("the warning does not carry the size it was given, so this guard cannot find its fixed half") + } + return marker[:at] +} + +func TestTheWindowSaysWhenItsManifestWillBeTooBigToReadBack(t *testing.T) { + said := previewOf(t, itoa(overTheCeiling(t))) + if !strings.Contains(said, warningAbout(t)) { + t.Errorf("a preview of %d files said nothing about the record being too big to read back.\n"+ + "The command line has said this since 2026-08-26. Somebody who does the same from the window "+ + "gets a directory that neither Verify nor Clean up can read, and no warning.\nIt said:\n%s", + overTheCeiling(t), said) + } +} + +// And a run that was never previewed says it too, which is the case that +// matters most. +// +// The preview is a button somebody may not press. The warning has to reach the +// person who pressed Generate and nothing else, because they are the one left +// with the directory. +func TestAFinishedRunInTheWindowSaysItsManifestIsTooBigToReadBack(t *testing.T) { + said := runOf(t, itoa(overTheCeiling(t))) + if !strings.Contains(said, warningAbout(t)) { + t.Errorf("a finished run of %d files said nothing about the record being too big to read back.\n"+ + "That directory now has a manifest neither Verify nor Clean up can read, and nobody was told.\nIt said:\n%s", + overTheCeiling(t), said) + } + // The line somebody pressed the button for stays first. The room for these + // messages is a ceiling and the message scrolls inside it. + if first := strings.SplitN(said, "\n", 2)[0]; strings.Contains(first, warningAbout(t)) { + t.Errorf("the warning took the first line from the outcome:\n%s", said) + } +} + +// A run this build CAN read back stays quiet. +// +// Without this the guard above passes on a window that warns about every run, +// which teaches somebody to stop reading the line - the same reason the command +// line has this pair rather than only the first half. +func TestAnOrdinaryPreviewSaysNothingAboutTheManifestCeiling(t *testing.T) { + said := previewOf(t, "100") + if strings.Contains(said, warningAbout(t)) { + t.Errorf("a hundred files drew the warning about the record being too big:\n%s", said) + } +} + +// Both surfaces judge the same run the same way. +// +// The window reads the answer off the document a dry run builds, and the +// command line works it out from the plan before anything is written. Two paths +// to one number, and what makes two paths acceptable is that they go through +// one predicate. Asked at the boundary, which is the only place a disagreement +// would show. +func TestBothSurfacesJudgeTheSameRunTheSameWay(t *testing.T) { + for _, entries := range []int{ + int(manifest.MaxBytes / manifest.BytesPerEntry), + int(manifest.MaxBytes/manifest.BytesPerEntry) + 1, + } { + _, fromThePlan := manifest.TooLargeToReadBack(entries, 0) + + m := manifest.New("testing-files-generator", "0.0.0-dev", "run_x", "tfg generate", 1, "linux", "amd64") + for i := 0; i < entries; i++ { + m.Add(manifest.File{Path: "f.txt", Materialized: true}) + } + _, fromTheDocument := m.ReadBackReach() + + if fromThePlan != fromTheDocument { + t.Errorf("at %d entries the plan says %v and the document says %v.\n"+ + "The command line answers from the first and the window from the second, so one run "+ + "would be warned about on one surface and not on the other", + entries, fromThePlan, fromTheDocument) + } + } +} diff --git a/internal/gui/text/locale/en.json b/internal/gui/text/locale/en.json index 3b8d654..c154a75 100644 --- a/internal/gui/text/locale/en.json +++ b/internal/gui/text/locale/en.json @@ -248,6 +248,10 @@ "description": "Shown in the window. Carries one value, {{.Path}}, which has to stay spelled exactly that way.", "other": "the files were written and the manifest could not be saved to {{.Path}}" }, + "ManifestTooLargeToRead": { + "description": "Shown in the window. Carries these values, each of which has to stay spelled exactly that way: {{.Size}}, {{.Limit}}.", + "other": "this run's record is about {{.Size}} and this build reads at most {{.Limit}}, so Verify and Clean up will not be able to read it. Split the run to keep each record readable." + }, "NotAWholeNumber": { "description": "Shown in the window. Carries these values, each of which has to stay spelled exactly that way: {{.Field}}, {{.Value}}.", "other": "{{.Field}} is {{.Value}}, which is not a whole number. Write the digits out, such as 1 or 500" diff --git a/internal/gui/text/text.go b/internal/gui/text/text.go index 597f319..88db185 100644 --- a/internal/gui/text/text.go +++ b/internal/gui/text/text.go @@ -96,6 +96,23 @@ func PreviewCost(count int, formats []string, total string) string { say("PreviewNothingWritten", "nothing written yet") } +// ManifestTooLargeToRead is said when a run will write a record this build +// cannot read back. +// +// The command line has printed this since 2026-08-26 and the window said +// nothing at all, which is the parity gap observation O184 names. A person who +// generates 25 000 files from a window gets a directory that tfg verify and +// tfg cleanup both refuse - and the manifest is the only authority over what +// may be deleted, so nothing in this toolset can ever remove those files. +// +// A note rather than a refusal, which is the owner's decision from that day and +// is unchanged here. The run works. What was missing was that nobody was told. +func ManifestTooLargeToRead(size, limit string) string { + return sayf("ManifestTooLargeToRead", + "this run's record is about {{.Size}} and this build reads at most {{.Limit}}, so Verify and Clean up will not be able to read it. Split the run to keep each record readable.", + map[string]any{"Size": size, "Limit": limit}) +} + // PreviewFreeSpace follows PreviewCost when the disk could be measured. It is // a separate fact because a disk we cannot read has to say nothing at all // rather than invent a number. diff --git a/internal/gui/window/run.go b/internal/gui/window/run.go index aeade27..f66f6c6 100644 --- a/internal/gui/window/run.go +++ b/internal/gui/window/run.go @@ -664,25 +664,30 @@ func (r *runner) onPreview() { // Do rather than DoAndWait, for the same reason startRun gives: the // interface thread must never be left waiting on a worker. r.holdBeforeFinishing() - fyne.Do(func() { r.previewFinished(nil, opt, planErr) }) + fyne.Do(func() { r.previewFinished(nil, nil, opt, planErr) }) close(done) return } - _, runErr := engine.Run(ctx, planned, opt) + res, runErr := engine.Run(ctx, planned, opt) r.holdBeforeFinishing() - fyne.Do(func() { r.previewFinished(planned, opt, runErr) }) + fyne.Do(func() { r.previewFinished(res, planned, opt, runErr) }) close(done) }() } // previewFinished is the end of a preview, back on the interface thread. -func (r *runner) previewFinished(planned []engine.PlannedFile, opt engine.Options, runErr error) { +// +// The result is carried across as well as the plan, and that is what lets a +// preview warn about a record too big to read back. A dry run builds the whole +// document - see manifestReachNote - so the answer is there for the asking +// rather than something the window would have to work out for itself. +func (r *runner) previewFinished(res *engine.Result, planned []engine.PlannedFile, opt engine.Options, runErr error) { r.setBusy(false, false) if runErr != nil { r.refuse(runErr) return } - r.say(previewText(planned, opt.OutDir)) + r.say(append([]string{previewText(planned, opt.OutDir)}, manifestReachNote(res)...)...) } // formatsOf is what kinds of file the run would produce, each named once. @@ -837,7 +842,14 @@ func (r *runner) runFinished(res *engine.Result, runErr, saveErr error) { // and not only in the manifest - "the manifest says which ones" is an // answer in a terminal and an instruction to open a file with ten thousand // entries in a window. - r.say(append([]string{outcomeText(res, runErr)}, notesOf(res)...)...) + // + // The warning about a record too big to read back comes SECOND, ahead of + // the per file notes, and that order is the same lesson the command line + // learned on 2026-09-06: it is the one line standing between somebody and a + // directory nothing in this toolset can ever clean up, and it was being + // buried under notes about a label that did not fit. + said := append([]string{outcomeText(res, runErr)}, manifestReachNote(res)...) + r.say(append(said, notesOf(res)...)...) r.toneOfOutcome(res, runErr) r.offerTheFolder(res) } diff --git a/internal/gui/window/runreport.go b/internal/gui/window/runreport.go index 5b1a947..baecbd4 100644 --- a/internal/gui/window/runreport.go +++ b/internal/gui/window/runreport.go @@ -7,6 +7,7 @@ import ( "github.com/donislawdev/TestingFilesGenerator/internal/core" "github.com/donislawdev/TestingFilesGenerator/internal/engine" "github.com/donislawdev/TestingFilesGenerator/internal/gui/text" + "github.com/donislawdev/TestingFilesGenerator/internal/manifest" ) // What a run tells the person while it goes and when it ends. @@ -31,6 +32,37 @@ func previewText(planned []engine.PlannedFile, outDir string) string { return line } +// manifestReachNote is the window's half of the warning the command line prints +// before the first byte. +// +// Observation O184: the command line has said this since 2026-08-26 and the +// window said nothing at all, so a person generating 25 000 files from a window +// was left with a directory that neither Verify nor Clean up can read - and the +// manifest is the only authority over what may be removed. A parity gap in +// quality rather than in what the engine can do, which is the kind D1 is +// easiest to lose. +// +// Read off the document rather than worked out here, and off the SAME predicate +// the command line uses, which is what manifest.TooLargeToReadBack exists for. +// The two surfaces cannot come to different conclusions about one run. +// +// A preview reaches this too. engine.Run with DryRun adds an entry for every +// planned file, so the document a preview produces is the document the run +// would produce, minus the bytes on the disk. That is why one shape serves +// both, and why the window can answer before anything is written even though it +// cannot say a word in the middle of a run. +func manifestReachNote(res *engine.Result) []string { + if res == nil || res.Manifest == nil { + return nil + } + size, over := res.Manifest.ReadBackReach() + if !over { + return nil + } + return []string{text.ManifestTooLargeToRead( + core.HumanBytes(size), core.HumanBytes(manifest.MaxBytes))} +} + // progressText is the line under the bar. Bytes rather than files, because one // large file is a run where the file count says nothing for minutes. func progressText(p engine.Progress, elapsed time.Duration) string { diff --git a/internal/manifest/manifest.go b/internal/manifest/manifest.go index a2b5744..370ff70 100644 --- a/internal/manifest/manifest.go +++ b/internal/manifest/manifest.go @@ -527,6 +527,26 @@ func TooLargeToReadBack(entries, withNotes int) (int64, bool) { return n, n > MaxBytes } +// ReadBackReach is what this document will weigh and whether this build would +// refuse to read it. +// +// The same question TooLargeToReadBack answers, asked of a manifest that +// exists rather than of a plan that has not run. It is here because the two +// callers had nowhere else to get the count of entries carrying a note from - +// that is kept while entries are added, so working it out again would mean +// walking every file a second time and getting it subtly wrong when a failed +// entry gains a note of its own. +// +// The window needs this and the command line does not. The command line says +// its piece BEFORE the first byte, from the plan, where no manifest exists yet. +// A window cannot say anything in the middle of a run - a widget touched from a +// worker is a race, and two of those were found on CI - so it says it when the +// run ends, off the document the run actually produced. Both go through +// TooLargeToReadBack, which is where that answer is settled for both surfaces. +func (m *Manifest) ReadBackReach() (int64, bool) { + return TooLargeToReadBack(len(m.Files), m.notedFiles) +} + // TooLargeError is returned for a manifest past MaxBytes. type TooLargeError struct { Path string