diff --git a/.github/osv-scanner.toml b/.github/osv-scanner.toml new file mode 100644 index 00000000..293960de --- /dev/null +++ b/.github/osv-scanner.toml @@ -0,0 +1,27 @@ +# Exceptions for the OSV scan of the files in this directory. OSV-Scanner reads +# this file for the lockfiles beside it and for no other directory. One advisory +# per entry, and every entry has a date it stops working on. +# +# The rule is to raise what can be raised - pull request #181 fixed thirty +# findings rather than listing them here. An entry belongs here only when no +# version anybody can install has the fix. + +# mcp 1.29.0, in requirements-semgrep.txt for the scanner and nothing else. +# +# Every semgrep release, 1.181.0 of 2026-10-14 included, asks for mcp==1.29.0 +# exactly, so no set pip can install holds the fixed 1.30.0. Dependabot's own +# security update said the same on 2026-10-15: latest resolvable 1.29.0, lowest +# fixed 1.30.0. +# +# The advisory is about an MCP server over Streamable HTTP keeping its sessions +# without bound. The job runs `semgrep scan` and nothing else, which starts no +# server and listens nowhere, on a runner that lives for one scan. +# +# When semgrep moves to a fixed mcp, Dependabot brings that release as a pull +# request against requirements-semgrep.in, and this entry has nothing left to +# hide - remove it then. If that has not happened by the date below, the weekly +# scan goes red again, which is the reminder. +[[IgnoredVulns]] +id = "GHSA-84m7-p3x7-pcfv" +ignoreUntil = 2026-11-15 +reason = "semgrep pins mcp==1.29.0 in every release, and the advisory is about an MCP HTTP server, which semgrep scan never starts" diff --git a/internal/guard/boxwidth_test.go b/internal/guard/boxwidth_test.go index 541a5f1d..a6368de0 100644 --- a/internal/guard/boxwidth_test.go +++ b/internal/guard/boxwidth_test.go @@ -78,10 +78,9 @@ func TestABoxIsWideEnoughForItsOwnPlaceholder(t *testing.T) { continue } hint := placeholderShownFor(p) - words := fyne.MeasureText(hint, theme.TextSize(), fyne.TextStyle{}) - if got := typedInWidth(control); got < words.Width { - t.Errorf("%s.%s shows %q, which needs %.0f px, in a box %.0f px wide - so it is cut off", - d.ID, p.Name, hint, words.Width, got) + if needs, got := boxNeededFor(hint), typedInWidth(control); got < needs { + t.Errorf("%s.%s shows %q, which needs a box of %.0f px, in a box %.0f px wide - so it is cut off", + d.ID, p.Name, hint, needs, got) } checked++ } @@ -106,6 +105,20 @@ func placeholderShownFor(p format.Property) string { return text.PlaceholderLeftEmpty(p.Default) } +// boxNeededFor is how wide a box has to be to show words without cutting +// them: the words, and the two paddings on each side the toolkit lays an +// entry's text out inside. +// +// Written out here rather than asked of parts.roomFor, which says the same +// thing, because a guard that called it would agree with it whatever it said. +// One copy for both screens since a review of #195: the single batch screen +// compared the words with the whole box, the batch screen with the room inside +// it, and "worked out from the size" - 165 px of words - fits the first in a +// 179 px box and not the second. +func boxNeededFor(words string) float32 { + return fyne.MeasureText(words, theme.TextSize(), fyne.TextStyle{}).Width + theme.InnerPadding()*2 + theme.Padding()*2 +} + // formatsLaidOut opens the window, puts it on a canvas and hands back a way to // change format that leaves the screen laid out afterwards. // @@ -259,10 +272,22 @@ func TestABoxForANumberIsNotAsWideAsTheFormOnTheBatchScreen(t *testing.T) { t.Errorf("%s declares %q and the batch screen draws no field for it", d.ID, p.Name) continue } - if width := typedInWidth(control); width > float32(parts.ColumnWidth)/2 { + width := typedInWidth(control) + if width > float32(parts.ColumnWidth)/2 { t.Errorf("on the batch screen %s.%s holds a %s and its box is %.0f px of a %.0f px column", d.ID, p.Name, p.Kind, width, float32(parts.ColumnWidth)) } + // And no narrower than what it shows when left alone, which is the + // half the grid made the one that matters: since 2026-09-23 a box + // left unshaped takes one column, 179 px here, and "worked out from + // the size" needs 189 px of box (boxNeededFor). Measured on + // 2026-10-15, shaped 374 px and unshaped 179, and until then this + // guard asked only for the ceiling, which both pass. + hint := placeholderShownFor(p) + if needs := boxNeededFor(hint); width < needs { + t.Errorf("on the batch screen %s.%s shows %q, which needs %.0f px, in a box %.0f px wide - so it is cut off", + d.ID, p.Name, hint, needs, width) + } checked++ } } diff --git a/internal/guard/environment_test.go b/internal/guard/environment_test.go index 0c78d23a..6f5a908a 100644 --- a/internal/guard/environment_test.go +++ b/internal/guard/environment_test.go @@ -1,8 +1,10 @@ package guard import ( + "context" "encoding/json" "os" + "os/exec" "path/filepath" "runtime" "strings" @@ -47,6 +49,14 @@ func TestADirectoryWeCannotWriteInIsAPermissionProblem(t *testing.T) { if strings.Contains(errOut, "already exists") { t.Errorf("a permission problem is reported as a name that is taken:\n%s", errOut) } + // And as what it is. The first thing a run creates in the directory is the + // lock that keeps a second run out, so this is where the refusal comes from, + // and a failure there that is not "something is already there" is reported + // as one. Without this the guard passed on a refusal calling the directory + // busy with another run - which says nothing about a taken name either. + if !strings.Contains(errOut, "cannot start a run in") { + t.Errorf("a directory nobody can write in is not reported as a run that could not start there:\n%s", errOut) + } if n := strings.Count(errOut, "tfg: "); n != 1 { t.Errorf("one fault produced %d messages:\n%s", n, errOut) } @@ -61,7 +71,50 @@ func TestADirectoryWeCannotWriteInIsAPermissionProblem(t *testing.T) { // California describe the same instant the same way. A local time would make // two manifests of the same run look like different runs, and a reader // comparing them has no way to tell which. +// +// Asked in a zone that is not UTC, because in UTC the two are the same text. +// The runners keep their clocks in UTC, so a manifest that recorded local time +// passed here on every one of them - the mutation run of 2026-10-15 found it, +// and only the owner's machine, east of UTC, could tell. The zone is set in a +// child process of its own rather than here, because time.Local is a variable +// every goroutine of this binary reads and the race detector runs these guards. +// Not through TZ, which Go on Windows does not read: measured on 2026-10-16, +// TZ=Asia/Kathmandu and TZ=UTC both left a program on this machine in CEST. func TestTheRecordedTimeDoesNotDependOnTheLocalZone(t *testing.T) { + if os.Getenv(awayFromUTCChild) == "1" { + recordedTimeAwayFromUTC(t) + return + } + // A ceiling on the child, so one that hangs ends this guard rather than + // the whole run. And the child's own PASS line rather than the word: a + // pattern that matched nothing ends with "no tests to run" and PASS too + // (a review of #195). + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cancel() + run := exec.CommandContext(ctx, os.Args[0], "-test.run=^TestTheRecordedTimeDoesNotDependOnTheLocalZone$", "-test.v") + run.Env = append(os.Environ(), awayFromUTCChild+"=1") + out, err := run.CombinedOutput() + if err != nil { + t.Fatalf("the manifest's time moved with the zone:\n%s", out) + } + if !strings.Contains(string(out), "--- PASS: TestTheRecordedTimeDoesNotDependOnTheLocalZone") { + t.Fatalf("the child said nothing about passing, which means it never ran:\n%s", out) + } +} + +const awayFromUTCChild = "TFG_AWAY_FROM_UTC_CHILD" + +// recordedTimeAwayFromUTC is the guard above, in a process whose local zone is +// five hours and forty five minutes east of UTC - a zone no runner and no +// developer's machine is likely to be in already. +func recordedTimeAwayFromUTC(t *testing.T) { + t.Helper() + time.Local = time.FixedZone("UTC+05:45", 5*60*60+45*60) + // In the state this is about, or it proves nothing. + if now := time.Now().Format(time.RFC3339); strings.HasSuffix(now, "Z") { + t.Fatalf("the local time reads %s, which is UTC, so this cannot tell a local time from a universal one", now) + } + dir := t.TempDir() code, stdout, errOut := run(t, "generate", "--format", "txt", "--size", "1kb", "--json", "--out", dir) diff --git a/internal/guard/formwidth_test.go b/internal/guard/formwidth_test.go index fcd88ef5..39422a94 100644 --- a/internal/guard/formwidth_test.go +++ b/internal/guard/formwidth_test.go @@ -115,6 +115,29 @@ func TestABoxForANumberIsTheWidthOfANumber(t *testing.T) { screenName, label, got, parts.NumericWidth*2) } } + + // And the other bound, in the narrowest window the form allows. A + // number's box asks for NumericWidth, and that is what makes the grid + // lay a narrow window out in fewer, wider columns. Measured on + // 2026-10-15 in a 580 px window: 262 px boxes, and 123 px - narrower than a + // number - with parts.Numeric handing the control over unsized. At the + // width above the two look the same, which is how the mutation run + // scored that change a HOLE. + w.Resize(fyne.NewSize(content.MinSize().Width, 760)) + content.Refresh() + // In the state this is about, or it proves nothing (a review of #195): + // a resize the window ignored would measure the wide layout again. + if got, least := content.Size().Width, content.MinSize().Width; got > least+1 { + t.Fatalf("%s: the form is %.0f px wide after a resize to its least width of %.0f px, so this did not measure the narrowest layout", + screenName, got, least) + } + for _, label := range numeric { + box := entryUnder(t, content, label) + if got := box.Size().Width; got < parts.NumericWidth-1 { + t.Errorf("%s: in the narrowest window the box for %q is %.0f px wide, under the %d px of a number", + screenName, label, got, parts.NumericWidth) + } + } w.Close() } } diff --git a/internal/guard/gogl_test.go b/internal/guard/gogl_test.go index 354721e2..d65f1a3a 100644 --- a/internal/guard/gogl_test.go +++ b/internal/guard/gogl_test.go @@ -2,6 +2,7 @@ package guard import ( "encoding/json" + "go/build" "io/fs" "os" "os/exec" @@ -312,18 +313,32 @@ func firstDifferingLine(a, b string) int { // generated file with C in its comments, and no guard reading the shape of // this module's code sees it. A walk that stopped skipping would hand that // file to fourteen guards at once, and they would all be right to refuse it. +// +// Whether the walk lists a package of that module is asked only where the +// module holds a package this build sees. Its files use cgo, so with cgo off - +// the mutation runners build that way - packages() would find nothing in it +// even after walking in: the run of 2026-10-15 scored the walk that stopped +// skipping as a HOLE on those runners and caught it where cgo is on. That one +// question skips there, as a subtest, and the list of skipped modules is +// asked everywhere, because it does not depend on cgo (a review of #195). func TestANestedModuleIsNotAPackageOfThisModule(t *testing.T) { root := repoRoot(t) generated := filepath.Join(root, filepath.FromSlash(bindingCopy), "v2.1", "gl", "package.go") if _, err := os.Stat(generated); err != nil { t.Fatalf("the nested module this guard expects to be skipped is not there: %v", err) } - for _, p := range packages(t) { - if strings.HasPrefix(p.rel, "third_party/") { - t.Errorf("packages() lists %s, which is inside another module.\n"+ - "A directory with its own go.mod is outside \"./...\" and has to be outside this walk.", p.rel) + listed := packages(t) + t.Run("no package of it is listed", func(t *testing.T) { + if _, err := build.ImportDir(filepath.Dir(generated), 0); err != nil { + t.Skipf("this build sees no package in %s (%v), so a walk into it would list nothing and this cannot tell", bindingCopy, err) } - } + for _, p := range listed { + if strings.HasPrefix(p.rel, "third_party/") { + t.Errorf("packages() lists %s, which is inside another module.\n"+ + "A directory with its own go.mod is outside \"./...\" and has to be outside this walk.", p.rel) + } + } + }) // And the walk skipped exactly that module. A go.mod dropped into a // first party directory would take it out of every guard that reads // packages(), and nothing but this line would say so. diff --git a/internal/guard/manifestpaths_test.go b/internal/guard/manifestpaths_test.go index a9670839..093998d2 100644 --- a/internal/guard/manifestpaths_test.go +++ b/internal/guard/manifestpaths_test.go @@ -168,21 +168,39 @@ func TestVerifyNeverReachesOutsideTheDirectory(t *testing.T) { // a name no disk has, the file is reported unreachable, cleanup leaves it and // ends with 5 anyway. The code was right by accident while the manifest was // being resolved exactly as before. +// +// Two spellings, because each system reaches a different check. A path from +// this system's own temporary directory starts with a separator on Linux and +// macOS, so there the check for a leading separator refused it and the check +// for a volume letter was never reached - the mutation run of 2026-10-15 +// scored that check a HOLE on the runners and caught it on Windows. A volume +// written out reaches it everywhere, which is the point of reading a manifest +// with the same rules on every system. func TestAManifestPathIsNeverAbsolute(t *testing.T) { - out, victim := outsideVictim(t) - mf := escapingManifest(t, out, filepath.ToSlash(victim), 21) + for _, c := range []struct { + what string + path func(victim string) string + }{ + {"this system's absolute path", func(victim string) string { return filepath.ToSlash(victim) }}, + {"a path naming a disk volume", func(string) string { return "C:/somewhere/else/victim.txt" }}, + } { + t.Run(c.what, func(t *testing.T) { + out, victim := outsideVictim(t) + mf := escapingManifest(t, out, c.path(victim), 21) - code, _, errOut := run(t, "cleanup", mf, "--yes", "--force") - victimSurvives(t, victim) + code, _, errOut := run(t, "cleanup", mf, "--yes", "--force") + victimSurvives(t, victim) - if code != cli.ExitIO { - t.Errorf("exit %d, expected %d - an absolute path in a manifest is refused, not resolved:\n%s", code, cli.ExitIO, errOut) - } - if !strings.Contains(errOut, "cannot be read as a manifest") { - t.Errorf("the manifest was acted on rather than turned away - an absolute path has to be refused at the door, not discovered to be unreachable later:\n%s", errOut) - } - if !strings.Contains(errOut, "outside") { - t.Errorf("the refusal does not say what is wrong with the path:\n%s", errOut) + if code != cli.ExitIO { + t.Errorf("exit %d, expected %d - an absolute path in a manifest is refused, not resolved:\n%s", code, cli.ExitIO, errOut) + } + if !strings.Contains(errOut, "cannot be read as a manifest") { + t.Errorf("the manifest was acted on rather than turned away - an absolute path has to be refused at the door, not discovered to be unreachable later:\n%s", errOut) + } + if !strings.Contains(errOut, "outside") { + t.Errorf("the refusal does not say what is wrong with the path:\n%s", errOut) + } + }) } } diff --git a/internal/guard/mutation/entries.py b/internal/guard/mutation/entries.py index a49819d8..225e4c38 100644 --- a/internal/guard/mutation/entries.py +++ b/internal/guard/mutation/entries.py @@ -166,11 +166,11 @@ ), # --- a preset renders a recipe that still means what it says (2026-09-06) --- ( - "the whitelist on a spread value goes, so a carriage return reaches the document raw", + "the whitelist on a spread value goes, so a space reaches a file name", "internal/preset/sizeboundaries.go", 'if bad := firstUnusable(piece); bad != "" {', 'if bad := firstUnusable(piece); false && bad != "" {', - "TestEveryPresetRendersARecipeThatParsesBack", + "TestEveryPresetEndingHasTheCodeItShould", ), # --- two one word findings, and a third the guard for them found (2026-09-06) --- ( @@ -347,7 +347,7 @@ ), # --- the write path claims every name it writes under (2026-09-06) --- ( - "the claim believes os.Stat, so a link pointing at nothing reads as a free name", + "the claim believes os.Stat, so a link pointing at nothing is refused in the system's words rather than as a name in use", "internal/core/createnew.go", "_, lookErr := os.Lstat(path)", "_, lookErr := os.Stat(path)", @@ -1914,8 +1914,8 @@ ( "the window stops saying where the files will go, so the only field that decides it is off the screen", "internal/gui/window/runline.go", - " return text.WritingTo(l.where(dir))", - " return \"\"", + "\treturn text.RunLine(int(sum.files), sum.totalText(), sum.formats, l.where(dir))", + "\treturn text.RunLine(int(sum.files), sum.totalText(), sum.formats, \"\")", "TestWhereTheFilesGoIsSaidOutsideTheScrollingPart", ), ( @@ -2293,7 +2293,7 @@ ( # Two screens draw a field from the same declaration through different # code, and only one of them got the width on the first try. - "the batch screen draws a number in a box the width of the form", + "the batch screen draws a number in one column of the grid, too narrow for the words it shows when left alone", "internal/gui/window/recipefolds.go", " parts.ShapedFor(b.declared[i], f.Control))", " f.Control)", @@ -3070,8 +3070,8 @@ # pdf has one case, measured by counting Format: fields in the file. "a format quietly stops having its bytes pinned anywhere", "internal/guard/generatorbytes_test.go", - '"pdf_16kib": {ID: "g", Format: "pdf", Sizes: engine.Uniform(1, 16384), Label: true},', - '"pdf_16kib": {ID: "g", Format: "txt", Sizes: engine.Uniform(1, 16384), Label: true},', + "\"png_64kib\": {ID: \"g\", Format: \"png\",", + "\"png_64kib\": {ID: \"g\", Format: \"txt\",", "TestEveryFormatHasAGoldenValue", ), ( @@ -4123,7 +4123,7 @@ # The real defect, put back. Capping the column stopped a field at # 820 px, which is still a paragraph's width offered for the digit 0. # Measured before the fix: the seed was 397 px and the count 399. - "a box for a number goes back to taking the whole column", + "a box for a number loses its floor, so in the narrowest window it is drawn narrower than a number", "internal/gui/parts/parts.go", "\treturn Sized(NumericWidth, control)", "\treturn control", @@ -4511,11 +4511,11 @@ # The colour the open list had until 2026-08-12: the input colour to the # byte, over a panel 3.8 L* away, with no border. Reported from a # screenshot as a list where everything runs together. - "an open list goes back to the colour of an input box", + "the menu's surface drops to the colour of an input box, so the rung of the ladder above a button's face is missing", "internal/gui/parts/theme.go", "theme.ColorNameMenuBackground: hex(0x57, 0x57, 0x5F),", "theme.ColorNameMenuBackground: hex(0x39, 0x39, 0x3F),", - "TestAnOpenListIsToldFromTheFormBehindIt", + "TestTheLadderClimbsInEvenSteps", ), ( # The toolkit's own behaviour, put back: a list of thirteen formats @@ -4751,9 +4751,9 @@ # half that asks two canvases, and by the taller one leaving more than # a row of its half unused. "the open list's ceiling is the same in every window again", - "internal/gui/parts/openlist.go", - "rows := float32(math.Floor(float64(canvasHeight * listShare / row)))", - "rows := float32(math.Floor(float64(800 * listShare / row)))", + "internal/gui/parts/ring.go", + "ceiling := wholeRows(canvasHeight*listShare - head)", + "ceiling := wholeRows(800*listShare - head)", "TestTheOpenListCoversHalfTheWindowAndGrowsWithIt", ), ( @@ -5249,15 +5249,6 @@ "\t_ = opt.OutDir", "TestTheFolderOfferGoesAwayWhenTheNextRunStarts", ), -( - # A shortcut that worked on a hidden button would reach a state the screen - # does not offer - Escape cancelling a run that is not going. - "a shortcut presses a button the screen has taken out of use", - "internal/gui/window/runactions.go", - "if b == nil || b.Disabled() || !b.Visible() || b.OnTapped == nil {", - "if b == nil || b.OnTapped == nil {", - "TestAShortcutCannotStartASecondRunDuringTheFirst", -), ( # Escape has to be harmless while nothing is running. Broken the other way # round - by making it start a run instead of stopping one - because a @@ -5583,13 +5574,6 @@ "if limit == 0 {", "TestABoundaryLimitIsRefusedAtBothEnds", ), - ( - "the screen wires its fields for typing a second time", - "internal/gui/parts/fields.go", - "\ts.tell = tell\n}", - "\ts.tell = tell\n\tfor _, f := range s.list {\n\t\ts.listen(f.Setting, f.Control)\n\t}\n}", - "TestAFieldWiredBeforeTheScreenListensReportsOnce", - ), ( "the walk of the directory stops asking whether it was cancelled", "internal/audit/walk.go", @@ -6276,13 +6260,6 @@ "\tif err := preflight(context.Background(), files, opt); err != nil {", "TestWorkingOutTheCostCanBeStopped", ), - ( - "planning stops noticing that it was asked to stop", - "internal/engine/plantarget.go", - "\t\tif err := ctx.Err(); err != nil {", - "\t\tif false {", - "TestWorkingOutTheCostCanBeStopped", - ), ( "a cleanup report goes back to counting only blocked files as kept", "internal/cli/cleanup.go", @@ -6428,8 +6405,8 @@ # its words. Four values come out cut off and the guard names each. "a row gives its words less room than it promised, so values are cut off in the open list", "internal/gui/parts/listrow.go", - "r.label.Resize(fyne.NewSize(size.Width-left-right, text.Height))", - "r.label.Resize(fyne.NewSize(size.Width-left-right-rowGutter*3, text.Height))", + "room := size.Width - left - right\n", + "room := size.Width - left - right - rowGutter*3\n", "TestEveryValueInAMenuFitsInTheListItOpens", ), ( @@ -11492,14 +11469,14 @@ "internal/engine/names.go", "which differ only in case. Most filesystems treat those as one file, so one would be written over the other and the manifest would describe both\",\n\t\t\tcore.A(\"First\", owner.id), core.A(\"Second\", id), core.A(\"FirstName\", core.Shown(owner.name)), core.A(\"SecondName\", core.Shown(name)))", "which differ only in case. Most filesystems treat those as one file, so one would be written over the other and the manifest would describe both\",\n\t\t\tcore.A(\"First\", owner.id), core.A(\"Second\", id), core.A(\"FirstName\", owner.name), core.A(\"SecondName\", name))", - "TestARefusalShowsANameNobodyCanReadAsAnEscape", + "TestTheWindowsWordsForARefusalShowANameNobodyCanReadAsAnEscape", ), ( "a refusal of a taken name prints the path raw", "internal/engine/errors.go", "remove the file first\",\n\t\tcore.A(\"Path\", core.Shown(e.Path)))", "remove the file first\",\n\t\tcore.A(\"Path\", e.Path))", - "TestARefusalShowsANameNobodyCanReadAsAnEscape", + "TestTheWindowsWordsForARefusalShowANameNobodyCanReadAsAnEscape", ), ( "the files of a boundary set are listed raw", @@ -11534,14 +11511,14 @@ "internal/engine/preflight.go", "\"the output directory %s is a file, not a directory\", core.A(\"Dir\", core.Shown(opt.OutDir)))", "\"the output directory %s is a file, not a directory\", core.A(\"Dir\", opt.OutDir))", - "TestTheLinesAboutARunShowANameNobodyCanReadAsAnEscape", + "TestTheWindowsWordsForARefusalShowANameNobodyCanReadAsAnEscape", ), ( "a refusal of a held directory prints it raw", "internal/engine/errors.go", "core.A(\"Dir\", core.Shown(e.Dir)), core.A(\"Lock\", core.Shown(e.Path)))", "core.A(\"Dir\", e.Dir), core.A(\"Lock\", e.Path))", - "TestTheLinesAboutARunShowANameNobodyCanReadAsAnEscape", + "TestTheWindowsWordsForARefusalShowANameNobodyCanReadAsAnEscape", ), ( # A right to left override typed into a comment of shipped code - the @@ -11792,13 +11769,6 @@ "text.InstructionsNotSaved(saved.Missed.Path, core.ShownText(text.Refusal(saved.Missed.Err, \"\")))", "TestTheWindowSaysInstructionsItCouldNotWriteWithTheEscape", ), - ( - "the window says why the instructions were not written in the system's raw words", - "internal/gui/window/run.go", - "text.InstructionsNotSaved(core.Shown(saved.Missed.Path), core.ShownText(text.Refusal(saved.Missed.Err, \"\")))", - "text.InstructionsNotSaved(core.Shown(saved.Missed.Path), text.Refusal(saved.Missed.Err, \"\"))", - "TestTheWindowSaysInstructionsItCouldNotWriteWithTheEscape", - ), # --- the WinGet and Chocolatey packages and their renderer (2026-09-25) --- ( "the Chocolatey nuspec is written under the template's name instead of the package's", @@ -11957,8 +11927,8 @@ ( "a checksum file saved with a byte order mark is refused", ".github/scripts/build_packages.py", - ' with open(path, encoding="utf-8-sig") as handle:', - ' with open(path, encoding="utf-8") as handle:', + "text = data.decode(\"utf-8-sig\").replace(\"\\r\\n\", \"\\n\")", + "text = data.decode(\"utf-8\").replace(\"\\r\\n\", \"\\n\")", "TestEverySpellingOfAChecksumFileRendersTheSamePackages", ), ( @@ -12775,10 +12745,10 @@ 'TestSavingAManifestTellsAnEmptySlotFromAnUnreadableOne', ), ( - 'every failure to take the manifest name is called a collision', + "every failure to take the run's lock is called another run writing there", 'internal/engine/engine.go', - 'if errors.Is(err, fs.ErrExist) {\n\t\treturn nil, &CollisionError{Path: manifestPath, Manifest: true}', - 'if !errors.Is(err, fs.ErrExist) {\n\t\treturn nil, &CollisionError{Path: manifestPath, Manifest: true}', + "\t\tif errors.Is(err, fs.ErrExist) {\n\t\t\treturn res, &RunInProgressError{Path: lockPath, Dir: opt.OutDir}", + "\t\tif !errors.Is(err, fs.ErrExist) {\n\t\t\treturn res, &RunInProgressError{Path: lockPath, Dir: opt.OutDir}", 'TestADirectoryWeCannotWriteInIsAPermissionProblem', ), ( diff --git a/internal/guard/mutation/mutate.py b/internal/guard/mutation/mutate.py index 75ec4c91..01975855 100644 --- a/internal/guard/mutation/mutate.py +++ b/internal/guard/mutation/mutate.py @@ -94,12 +94,25 @@ def one(tree) -> None: return "\n".join(faults) +def first_complaint(output: str) -> str: + """The compiler's first complaint, the line a BROKEN verdict carries. + + On CI the verdict is all there is - fifteen BROKEN entries of the first + run had to be built again by hand to learn why (2026-10-15). Go prints the + path relative to the directory it ran in, so a worker tree's own path does + not reach the report. Empty when no line names a Go file. + """ + return next((line.strip() for line in output.splitlines() if ".go:" in line), "") + + def check_canary_in(tree) -> bool: """True when a non compiling mutation is correctly reported as BROKEN. Runs in a worker tree like every other entry, so the real repository is never mutated - not even by the one entry that is meant to leave the tree - unable to compile. + unable to compile. The complaint has to come through as well, naming the + file and the variable the canary added, or a BROKEN below says nothing + about why. """ label, rel, old, new, test = CANARY path = tree.file(rel) @@ -109,13 +122,19 @@ def check_canary_in(tree) -> bool: return False with mutation_applied(path, original, original.replace(old.encode(), new.encode(), 1)): - compiled = run_test(test, tree.path).compiled + result = run_test(test, tree.path) - if compiled: + if result.compiled: print(f"HOLE {label}\n a tree that does not compile was reported as compiling, " f"so every BROKEN below is unreliable") return False - print(f"ok {label}\n the broken tree was detected") + complaint = first_complaint(result.output) + if Path(rel).name not in complaint or "orphan" not in complaint: + print(f"HOLE {label}\n the compiler's complaint about {rel} did not reach the verdict, " + f"so every BROKEN below says nothing about why\n" + f" {complaint or 'no output line names a Go file'}") + return False + print(f"ok {label}\n the broken tree was detected\n {complaint}") return True @@ -257,11 +276,7 @@ def judge(index: int, entry, tree, budget, deadline: float, if result.capped: return Outcome(index, "CAPPED", label, rel, test, secs, result.capped) if not result.compiled: - # The compiler's first complaint goes with the verdict. On CI the - # verdict is all there is - fifteen BROKEN entries of the first run - # had to be built again by hand to learn why (2026-10-15). - complaint = next((line.strip() for line in result.output.splitlines() if ".go:" in line), "") - return Outcome(index, "BROKEN", label, rel, test, secs, complaint) + return Outcome(index, "BROKEN", label, rel, test, secs, first_complaint(result.output)) if result.passed: reason = unjudged_reason(test, tree.path) if reason: diff --git a/internal/guard/mutationcoverage_test.go b/internal/guard/mutationcoverage_test.go index 03a7eb5d..e2afa787 100644 --- a/internal/guard/mutationcoverage_test.go +++ b/internal/guard/mutationcoverage_test.go @@ -125,6 +125,20 @@ var notProvenByMutation = map[string]bool{ // "proven another way" are different states and lumping them together would // send a later session to re-prove what is already proven. var provenByProbe = map[string]string{ + // Three guards each held by two layers, so no single substitution can redden + // them and each mutation of one layer scored a HOLE in the CI run of + // 2026-10-15. Broken that day in a worker tree, one layer at a time and then + // both: green, green, red, for all three. + "TestAShortcutCannotStartASecondRunDuringTheFirst": "broken on 2026-10-15 in two places at once: the check on the button in pressIfLive (runactions.go) and the refusal of onGenerate while the state is occupied (run.go, since 2026-09-21). " + + "With either one taken away alone the guard stayed green, because the other refused the second run. With both gone it went red saying the screen reports a run that wrote nothing, which is the second one.", + "TestAFieldWiredBeforeTheScreenListensReportsOnce": "broken on 2026-10-15 in two places at once: WhenTypedIn walking the fields it already has, and chainOnce chaining again rather than pointing the one handler it put in (both in parts/fields.go). " + + "With either alone the guard stayed green - a second wiring is pointed rather than chained, and without the walk nothing wires twice. With both it went red: typing into one box told the screen 2 times.", + "TestEveryPresetRendersARecipeThatParsesBack": "broken on 2026-10-15 in two places at once: the whitelist in checkDistance (preset/sizeboundaries.go) and the refusal in recipe.Compose of a character a recipe cannot hold (refuseUnwritable). " + + "With either alone the guard stayed green, 154 values tried. With both it went red: size-boundaries accepted spread=\"1\\rB\" and produced source the parser refuses. " + + "The whitelist's other job, a space reaching a file name, is the mutation on TestEveryPresetEndingHasTheCodeItShould.", + "TestARefusalShowsANameNobodyCanReadAsAnEscape": "broken on 2026-10-15 in three places at once: the escape of the two names in the refusal of a case collision (engine/names.go), the escape of the path in the refusal of a taken name (engine/errors.go), and the escape of the whole message on its way out of the command line (describeError in cli/errors.go). " + + "With any one of them gone the guard stayed green, because the line is escaped either way. With all three it went red: the refusal of two names that collide printed U+200B raw. " + + "The two escapes in the sentences are the mutations on TestTheWindowsWordsForARefusalShowANameNobodyCanReadAsAnEscape, where the window has no funnel to cover them.", "TestAFolderThatCannotBeListedIsRefusedWholeAndVerifyStillStopsAtIt": "broken on 2026-09-30 in a Linux container (golang:1.27-alpine, the repository read only, an ordinary user), through the command line rather than through this test, which cannot build there without the window's libraries. " + "With internal/audit/walk.go as it is, verify of a run with a folder nobody may list ended with 5 naming it. With the three lines that hand verify the first unreadable folder removed - the overlay put the mutated file in place - the same verify ended with 0 and said the directory matches. " + "A probe rather than a mutation entry because this guard skips on Windows, where the runner runs, and would score NOT CAUGHT about a healthy guard.", diff --git a/internal/guard/oracleminimum_test.go b/internal/guard/oracleminimum_test.go index 5514ef96..74d0fe02 100644 --- a/internal/guard/oracleminimum_test.go +++ b/internal/guard/oracleminimum_test.go @@ -31,6 +31,13 @@ import ( // Both ends are checked here rather than only the small one. A format that // refuses everything except its one tested size would satisfy a guard that // only knew about the minimum. +// +// One subtest per format, and a format whose tool is missing SKIPS rather than +// counting itself into a number in the log. Until 2026-10-15 a missing tool was +// that number and nothing else, so the guard was green whether it had put SVG +// past Inkscape or not - and the mutation run read green as caught: it scored +// the empty drawing at 193 B as a HOLE on the runner with no Inkscape and on a +// machine whose python has no Pillow. A skip says where nothing was asked. func TestEveryFormatSurvivesItsReferenceToolAtItsSmallest(t *testing.T) { dir := t.TempDir() checked, skipped := 0, 0 @@ -45,46 +52,51 @@ func TestEveryFormatSurvivesItsReferenceToolAtItsSmallest(t *testing.T) { continue } - // The label costs bytes, so the smallest file with one is not the - // smallest file. Both are produced, because a reader meets both. - for _, label := range []bool{true, false} { - for _, size := range []int64{d.MinBytes, d.MinBytes + 1, d.MinBytes + 2} { - plan, err := d.Generator.Plan(format.Request{Bytes: size, Seed: 7741, Label: label}) - if err != nil { - // Genuinely unreachable sizes are a legitimate answer and - // the size guard already holds them to being typed - // refusals. Nothing to check when no file exists. - continue - } + t.Run(d.ID, func(t *testing.T) { + // The label costs bytes, so the smallest file with one is not the + // smallest file. Both are produced, because a reader meets both. + for _, label := range []bool{true, false} { + for _, size := range []int64{d.MinBytes, d.MinBytes + 1, d.MinBytes + 2} { + plan, err := d.Generator.Plan(format.Request{Bytes: size, Seed: 7741, Label: label}) + if err != nil { + // Genuinely unreachable sizes are a legitimate answer and + // the size guard already holds them to being typed + // refusals. Nothing to check when no file exists. + continue + } - path := filepath.Join(dir, "small"+d.Extension) - f, createErr := os.Create(path) - if createErr != nil { - t.Fatalf("creating %s: %v", path, createErr) - } - writeErr := d.Generator.Write(context.Background(), f, plan) - closeErr := f.Close() - if writeErr != nil || closeErr != nil { - t.Fatalf("%s at %d B: writing failed: %v %v", d.ID, size, writeErr, closeErr) - } + path := filepath.Join(dir, "small"+d.Extension) + f, createErr := os.Create(path) + if createErr != nil { + t.Fatalf("creating %s: %v", path, createErr) + } + writeErr := d.Generator.Write(context.Background(), f, plan) + closeErr := f.Close() + if writeErr != nil || closeErr != nil { + t.Fatalf("%s at %d B: writing failed: %v %v", d.ID, size, writeErr, closeErr) + } - res := checker.Check(path) - if !res.Available { - skipped++ - continue - } - checked++ - if res.Err != nil { - t.Errorf("%s at %d B with label=%v: %v\n the file is the right size and repeatable, and %s still rejects it", - d.ID, size, label, res.Err, checker.Name) + res := checker.Check(path) + if !res.Available { + skipped++ + t.Skipf("%s is not on this machine, so no %s was put past it", checker.Name, d.ID) + } + checked++ + if res.Err != nil { + t.Errorf("%s at %d B with label=%v: %v\n the file is the right size and repeatable, and %s still rejects it", + d.ID, size, label, res.Err, checker.Name) + } + os.Remove(path) } - os.Remove(path) } - } + }) } - t.Logf("smallest files put past a reference tool: %d checked, %d skipped for a missing tool", checked, skipped) + t.Logf("smallest files put past a reference tool: %d checked, %d format(s) skipped for a missing tool", checked, skipped) + // A skip of its own when every format skipped, so the guard as a whole + // does not read as passed having put nothing past anything (a review of + // #195). if checked == 0 && skipped > 0 { - t.Log("NOTHING was verified against an external tool on this machine") + t.Skip("NOTHING was verified against an external tool on this machine") } } diff --git a/internal/guard/presetcommands_test.go b/internal/guard/presetcommands_test.go index c679441d..aaa04d16 100644 --- a/internal/guard/presetcommands_test.go +++ b/internal/guard/presetcommands_test.go @@ -406,6 +406,12 @@ func TestEveryPresetEndingHasTheCodeItShould(t *testing.T) { {"the same distance twice", []string{"preset", "show", "size-boundaries", "--spread", "1B,1B"}, cli.ExitFormat}, {"the same distance spelled two ways", []string{"preset", "show", "size-boundaries", "--spread", "1024,1kb"}, cli.ExitFormat}, {"a distance carrying a character a name cannot", []string{"preset", "show", "size-boundaries", "--spread", "1\rB"}, cli.ExitFormat}, + // A space reads as a size and parses as one, and nothing after the + // whitelist in checkDistance refuses it: without that check it became + // part of a file name, 10mb_over_1 kb.pdf, measured 2026-10-15. The + // carriage return above is refused by the recipe's own check of an id + // as well, so it cannot tell whether the whitelist is there. + {"a distance with a space inside it", []string{"preset", "show", "size-boundaries", "--spread", "1 kb"}, cli.ExitFormat}, // A set no format here can build. FORMAT rather than RECIPE, because // the request is well formed and it is the floor of a format that puts // the smallest file of the set out of reach. diff --git a/internal/guard/presetsource_test.go b/internal/guard/presetsource_test.go index a5ed0660..c3699754 100644 --- a/internal/guard/presetsource_test.go +++ b/internal/guard/presetsource_test.go @@ -39,20 +39,25 @@ var hostileParameterValues = []string{ // Every preset renders a recipe that means what it says, whatever it is given. // -// internal/preset builds its source by writing strings into a template, which an +// internal/preset built its source by writing strings into a template, which an // outside review on 2026-09-05 raised as finding S8: nothing in the shape of -// that code stops a value from carrying a newline or a colon into the document. -// What keeps it safe today is firstUnusable, a whitelist of the characters a -// size is written with, and the review's point was that the whitelist has to be -// remembered by whoever adds the next preset. +// that code stopped a value from carrying a newline or a colon into the +// document. What kept it safe then was firstUnusable, a whitelist of the +// characters a size is written with, and the review's point was that the +// whitelist has to be remembered by whoever adds the next preset. // // The review's remedy - composing through recipe.Compose, which lets the -// encoder quote - was measured and turned down. It is in -// docs/SECURITY-REVIEW-2026-09-06.md section 13 with the numbers: the source -// comes out 627 B rather than 965 B, and internal/cli/preset.go records -// recipe.Hash of exactly those bytes as recipe_hash in the manifest. Changing -// the renderer changes that hash for everybody who runs a preset, which is a -// promise about other people's records rather than a tidy. +// encoder quote - was turned down that week for what it does to recipe_hash +// (docs/SECURITY-REVIEW-2026-09-06.md section 13), and arrived anyway with the +// presets of 2026-09-22 (#120): plan.source composes through it today. +// +// So two things hold this now, and neither is the whitelist. Measured on +// 2026-10-15 with firstUnusable switched off: the encoder quoted every value, +// and "1\rB" was refused by the recipe's own check of a target id. Which is why +// no single change breaks this guard, and why it is proven by hand rather than +// by a mutation (provenByProbe). The whitelist still does a job of its own - a +// distance's text becomes a file name - and TestEveryPresetEndingHasTheCodeItShould +// asks that with a space in it. // // So the guarantee is held by asking rather than by rewriting: whatever any // preset is given, the source it produces either is refused with a sentence or diff --git a/internal/guard/registry_test.go b/internal/guard/registry_test.go index f9cc371d..ffa5096c 100644 --- a/internal/guard/registry_test.go +++ b/internal/guard/registry_test.go @@ -217,9 +217,12 @@ func TestEveryFormatHasARowInTheFidelityChecklist(t *testing.T) { body, err := os.ReadFile(path) if err != nil { - t.Logf("SKIPPED: docs/MVP-FORMATS.md is not here, so nothing was compared. "+ + // A skip rather than a line in the log and a pass, since 2026-10-15: + // the mutation run takes a pass for a verdict, and on a checkout + // without docs/ it scored the two mutations of this guard as HOLES + // when nothing had been compared at all. + t.Skipf("docs/MVP-FORMATS.md is not here, so nothing was compared. "+ "The internal documents are excluded from the repository, so this check only runs on a machine that has them. (%v)", err) - return } section, ok := fidelityChecklist(string(body)) diff --git a/internal/guard/smallfixes_test.go b/internal/guard/smallfixes_test.go index de2bec0b..d39b57f4 100644 --- a/internal/guard/smallfixes_test.go +++ b/internal/guard/smallfixes_test.go @@ -6,10 +6,11 @@ import ( "fmt" "os" "path/filepath" - "regexp" "strings" "testing" + "github.com/goccy/go-yaml" + "github.com/donislawdev/TestingFilesGenerator/internal/cli" "github.com/donislawdev/TestingFilesGenerator/internal/engine" ) @@ -167,65 +168,71 @@ func TestAManifestThatCannotReachStandardOutputIsNotASuccess(t *testing.T) { } } -// Every job in the workflow has a ceiling on how long it may run. Without one +// Every job in every workflow has a ceiling on how long it may run. Without one // a hung job holds a runner until the GitHub default of six hours, and the // fuzzing job is the one most able to hang. -// topLevelKey matches a key of the workflow itself, such as "on:" or "env:". -// Anything else in column one is content somebody wrote inside a step. -var topLevelKey = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_-]*:`) - +// +// Read as YAML and asked of each job by name since 2026-10-15. The version +// before counted the lines that start with timeout-minutes against the lines +// that look like a job, and a step's own ceiling counts as one of those lines: +// ci.yml has one, so a job that lost its ceiling still left as many ceilings as +// jobs, and the mutation run scored it a HOLE. It also read ci.yml alone, and +// the workflows written since had no guard at all. func TestEveryCIJobHasACeilingOnItsTime(t *testing.T) { - raw, err := os.ReadFile(filepath.Join("..", "..", ".github", "workflows", "ci.yml")) - if err != nil { - t.Skipf("the workflow is not here: %v", err) + files, err := filepath.Glob(filepath.Join("..", "..", ".github", "workflows", "*.yml")) + if err != nil || len(files) == 0 { + t.Skipf("the workflows are not here: %v", err) } - text := string(raw) - // Counted rather than listed, so a job added later is covered without - // anybody remembering to come back here. - // - // Only the keys under "jobs:", which is the correction the first version of - // this needed: every key two spaces in also matched the triggers under - // "on:", so it demanded four ceilings that have nothing to run. - jobs, ceilings := 0, 0 - inJobs := false - for _, line := range strings.Split(text, "\n") { - if strings.HasPrefix(line, "jobs:") { - inJobs = true - continue + jobs := 0 + for _, path := range files { + raw, err := os.ReadFile(path) + if err != nil { + t.Fatalf("reading %s: %v", path, err) } - // Leaving the block takes a top level key, not merely a line starting - // in column one. A shell string inside a "run:" step can carry those, - // and one did: the dependency gate listed its expected modules that - // way, this scan called the jobs block finished at the first of them, - // and from then on it counted one job instead of seven. Seven ceilings - // against one job is not fewer, so the guard was green whatever it was - // given - found by mutation on 2026-08-04, not by reading. - if inJobs && len(line) > 0 && !strings.HasPrefix(line, " ") && topLevelKey.MatchString(line) { - inJobs = false + var workflow struct { + Jobs map[string]map[string]any `yaml:"jobs"` } - trimmed := strings.TrimSpace(line) - // Commented out does not count, and that was mutation's correction: - // counting the text anywhere in the file left a commented ceiling - // counting as a real one, so the guard stayed green on a job that had - // lost its own. - if strings.HasPrefix(trimmed, "#") { + if err := yaml.Unmarshal(raw, &workflow); err != nil { + t.Errorf("%s does not read as YAML: %v", filepath.Base(path), err) continue } - if strings.HasPrefix(trimmed, "timeout-minutes:") { - ceilings++ - } - if inJobs && strings.HasPrefix(line, " ") && !strings.HasPrefix(line, " ") && - strings.HasSuffix(trimmed, ":") { + for name, job := range workflow.Jobs { jobs++ + minutes, has := job["timeout-minutes"] + if !has { + t.Errorf("%s: the job %s has no timeout-minutes of its own, so it holds a runner for six hours when it hangs.\n"+ + "A step's timeout-minutes does not count - it ends that step, and the job waits on whatever comes next.", + filepath.Base(path), name) + continue + } + // A number of minutes, and more than none. The key with nothing + // after it reads as null and passed the version that asked only + // whether it was there (a review of #195). + if !someMinutes(minutes) { + t.Errorf("%s: the job %s has timeout-minutes: %v, which is not a number of minutes above none", + filepath.Base(path), name, minutes) + } } } if jobs == 0 { - t.Fatal("no job was found, so this guard would pass without checking anything") + t.Fatal("no job was found in any workflow, so this guard would pass without checking anything") } - if ceilings < jobs { - t.Errorf("%d job(s) and %d ceiling(s) - a job with no timeout-minutes holds a runner for six hours when it hangs", jobs, ceilings) +} + +// someMinutes is whether a decoded timeout-minutes is a number above nought. +func someMinutes(v any) bool { + switch n := v.(type) { + case uint64: + return n > 0 + case int64: + return n > 0 + case int: + return n > 0 + case float64: + return n > 0 } + return false } // The window binary is built. Nothing built it, so it could stop compiling and diff --git a/internal/guard/spacingscale_test.go b/internal/guard/spacingscale_test.go index 2feec5fb..013f397c 100644 --- a/internal/guard/spacingscale_test.go +++ b/internal/guard/spacingscale_test.go @@ -176,6 +176,15 @@ func TestTwoSectionsAreTheSameDistanceApartOnEveryScreen(t *testing.T) { // sections 7 px apart were uniform on the generate screen and still wrong, // because a boundary drawn more weakly than the padding inside a panel groups // the wrong things. +// +// The gap inside a section is the room between two FIELDS, each the whole of +// what the grid places - its name, its box and the line kept under the box for +// a refusal. Until 2026-10-15 this measured from the format box to the name of +// the size, which stand side by side since the grid of 2026-09-23: -55 px, so +// every section gap was wider than it and the guard was green whatever the +// sections were given. Found by the mutation run, which drew the sections 16 px +// apart and saw nothing. Measured that day: 24 px between two sections, 15.9 +// between the format and the batch name under it. func TestTheGapBetweenSectionsIsWiderThanTheGapBetweenFields(t *testing.T) { ourTheme(t) content, _ := laidOutWindow(t) @@ -185,13 +194,87 @@ func TestTheGapBetweenSectionsIsWiderThanTheGapBetweenFields(t *testing.T) { if len(gaps) == 0 { t.Fatal("the generate screen shows fewer than two sections, so there is no gap to measure") } - between := gapBelowField(t, generate, text.FieldFormat(), text.FieldSize()) + // The pair is found down a column rather than named, so it follows the + // layout (a review of #195): the first field with another under it in + // its own column, the way the guard above walks the form. + names := []string{text.FieldFormat(), text.FieldSize(), text.FieldCount(), text.FieldTargetID(), + text.FieldNameTemplate(), text.FieldOutputDir(), text.FieldSeed()} + var boxes []band + for _, label := range names { + _, box := nameAndBox(t, generate, label) + boxes = append(boxes, box) + } + above, below := -1, -1 + for i := range names { + if j := nextInColumn(boxes, i); j >= 0 { + above, below = i, j + break + } + } + // In the state this is about, or it proves nothing. + if above < 0 { + t.Fatal("no field on the generate screen stands under another in its column, so there is no gap between two fields to compare with - the layout changed, not the spacing") + } + upper, lower := fieldCell(t, generate, names[above], names), fieldCell(t, generate, names[below], names) + between := lower.Y - (upper.Y + upper.Height) + if between <= 0 { + t.Fatalf("%q ends at y=%.1f and %q under it starts at y=%.1f, so there is no room between two fields to compare with", + names[above], upper.Y+upper.Height, names[below], lower.Y) + } for _, gap := range gaps { - if gap <= between { - t.Errorf("two sections are %.1f px apart and two fields inside one are %.1f px apart."+ - " A section boundary has to be the wider of the two or it groups nothing", gap, between) + // By more than a pixel, because a layout lands on fractions and two + // gaps drawn from one token can differ by one. + if gap <= between+1 { + t.Errorf("two sections are %.1f px apart and two fields inside one (%q over %q) are %.1f px apart."+ + " A section boundary has to be the wider of the two or it groups nothing", gap, names[above], names[below], between) + } + } +} + +// fieldCell is the whole of one field as the grid placed it: the tallest thing +// in the field's column that holds its name and its box and the name of no +// other field. The last condition is what keeps it to one field when the grid +// itself is one column wide and as wide as the box (a review of #195). +func fieldCell(t *testing.T, screen fyne.CanvasObject, label string, others []string) band { + t.Helper() + name, box := nameAndBox(t, screen, label) + var foreign []band + for _, other := range others { + if other == label { + continue + } + if words, ok := labelBox(screen, other); ok { + foreign = append(foreign, words) + } + } + var cell band + atAbsolute(screen, func(o fyne.CanvasObject, at fyne.Position) { + if _, is := o.(*fyne.Container); !is { + return + } + size := o.Size() + if off := size.Width - box.Width; off > 1 || off < -1 { + return } + if at.X > box.X+1 || at.X+size.Width < box.X+box.Width-1 { + return + } + if at.Y > name.Y || at.Y+size.Height < box.Y+box.Height { + return + } + for _, words := range foreign { + if words.X >= at.X-1 && words.X <= at.X+size.Width && words.Y >= at.Y-1 && words.Y+words.Height <= at.Y+size.Height+1 { + return + } + } + if size.Height > cell.Height { + cell = band{X: at.X, Y: at.Y, Width: size.Width, Height: size.Height} + } + }) + if cell.Height == 0 { + t.Fatalf("nothing as wide as the box of %q holds its name and its box and no other field", label) } + return cell } // tabContent is one screen of the window, laid out, by the name on its tab. diff --git a/internal/guard/unseenoutput_test.go b/internal/guard/unseenoutput_test.go index 8885c503..1466f528 100644 --- a/internal/guard/unseenoutput_test.go +++ b/internal/guard/unseenoutput_test.go @@ -12,6 +12,7 @@ import ( "testing" "github.com/donislawdev/TestingFilesGenerator/internal/cli" + "github.com/donislawdev/TestingFilesGenerator/internal/engine" _ "github.com/donislawdev/TestingFilesGenerator/internal/format/all" "github.com/donislawdev/TestingFilesGenerator/internal/gui/text" "github.com/donislawdev/TestingFilesGenerator/internal/recipe" @@ -334,3 +335,70 @@ func TestTheWindowShowsADirectoryNobodyCanReadAsAnEscape(t *testing.T) { saysNothingUnseen(t, "the window's refusal of "+out, said) } } + +// The window's own words for the same four refusals the command line is asked +// about above: two names that collide, a name already taken, an output +// directory that is a file, and one another run is holding. +// +// The command line escapes a whole message on its way out (describeError, +// O241), which covers the name each of these sentences escapes as well - so +// the guards above stay green with either of the two taken away, and the CI +// run of 2026-10-15 scored all four of these escapes as HOLES. The window has +// no such funnel for a refusal it puts under a box: parts.Fields hands +// text.Refusal of the error to the box as it is. A sentence the engine wrote +// reaches the screen with only the escape it made itself, so that escape is +// asked for here, through the words the window would say, and nowhere is it +// covered twice. +func TestTheWindowsWordsForARefusalShowANameNobodyCanReadAsAnEscape(t *testing.T) { + txt := func(id, name string) engine.Target { + return engine.Target{ID: id, Format: "txt", Sizes: engine.Uniform(1, 1024), NameTmpl: name} + } + runTargets := func(out string, targets ...engine.Target) error { + opt := engine.Options{OutDir: out, ManifestName: "manifest.json"} + planned, err := engine.Plan(targets, opt) + if err != nil { + return err + } + _, err = engine.Run(context.Background(), planned, opt) + return err + } + + _, collide := engine.Plan([]engine.Target{txt("lower", "a"+zeroWidth+".txt"), txt("upper", "A"+zeroWidth+".txt")}, + engine.Options{OutDir: t.TempDir(), ManifestName: "manifest.json"}) + windowSaysEscaped(t, "a refusal of two names that collide", collide, "A", zeroWidth, ".txt") + + dir := t.TempDir() + name := "photo" + rightToLeft + "gpj.txt" + if err := os.WriteFile(filepath.Join(dir, name), []byte("mine"), 0o600); err != nil { + t.Fatal(err) + } + windowSaysEscaped(t, "a refusal of a taken name", runTargets(dir, txt("g", name)), "photo", rightToLeft, "gpj.txt") + + parent := t.TempDir() + file := filepath.Join(parent, "out"+rightToLeft+"file") + if err := os.WriteFile(file, []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + windowSaysEscaped(t, "a refusal of an output directory that is a file", runTargets(file, txt("g", "g.txt")), "out", rightToLeft, "file") + + held := filepath.Join(parent, "held"+zeroWidth+"dir") + if err := os.MkdirAll(held, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(engine.RunLockPath(held), nil, 0o600); err != nil { + t.Fatal(err) + } + windowSaysEscaped(t, "a refusal of a held directory", runTargets(held, txt("g", "g.txt")), "held", zeroWidth, "dir") +} + +// windowSaysEscaped is saysEscaped of a refusal in the words the window puts +// under a box, after asserting there was a refusal to read. +func windowSaysEscaped(t *testing.T, what string, err error, stem, unseen, rest string) { + t.Helper() + if err == nil { + t.Fatalf("%s did not happen, so this guard checked nothing", what) + } + said := text.Refusal(err, "") + saysNothingUnseen(t, what+", in the window's words", said) + saysEscaped(t, what+", in the window's words", said, stem, unseen, rest) +} diff --git a/internal/guard/writeescape_test.go b/internal/guard/writeescape_test.go index f984e0c2..887ec86c 100644 --- a/internal/guard/writeescape_test.go +++ b/internal/guard/writeescape_test.go @@ -253,6 +253,18 @@ func TestCreateNewCreatesOnlyWhenTheNameIsFree(t *testing.T) { "Callers tell this apart from a disk failure with errors.Is(err, fs.ErrExist), "+ "and the engine turns it into its own wording that way.", c.what, err) } + // And in this tool's words, which is what os.Lstat in CreateNew is + // for since the second create went (2026-09-29): it only chooses the + // sentence. With os.Stat there, a link pointing at nothing reads as + // no name at all, and the refusal is the system's "file exists" + // rather than the sentence about a name in use. It is the only shape + // here where the two differ, and it needs a symbolic link, so this + // half is proven where plantLink can make one. + var taken *core.NameTakenError + if !errors.As(err, &taken) { + t.Errorf("the refusal for %s is the system's words rather than the sentence about a name in use: %v", + c.what, err) + } if got, rerr := os.ReadFile(victim); rerr != nil || string(got) != "ORIGINAL" { t.Errorf("the file outside the directory changed: %q, %v", got, rerr) }