From 5ee818a8d99dbe9824bc7319ac661c7c89d9872f Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Fri, 25 Sep 2026 12:20:34 +0200 Subject: [PATCH 1/3] guard: the same work done again leaves the window as it was Every screen does one round of the work people repeat - every format, every preset, a base switched and changed, batches added, copied and taken away, an archive given contents, boxes typed into and a small run - four times over. After each round three counts have to come out the same as after the first: the readings of the form and expansions of a preset each change caused, the drawn things the screen reaches through its own fields, and the goroutines started here that are still running our code. The listener chain of 2026-09-23 has a guard of its own at the place it was. This one asks about the class, wherever a next one would sit, and about two things nothing counted before: a rebuild that keeps what it replaced, and a worker that outlives its run. Counts rather than memory, because the test driver never clears its cache of renderers. Memory is asked of the real window in the release check. Co-Authored-By: Claude Opus 5.5 --- internal/guard/repetition_test.go | 320 ++++++++++++++++++++++++++++ internal/guard/workrepeated_test.go | 226 ++++++++++++++++++++ 2 files changed, 546 insertions(+) create mode 100644 internal/guard/repetition_test.go create mode 100644 internal/guard/workrepeated_test.go diff --git a/internal/guard/repetition_test.go b/internal/guard/repetition_test.go new file mode 100644 index 0000000..a6509eb --- /dev/null +++ b/internal/guard/repetition_test.go @@ -0,0 +1,320 @@ +package guard + +import ( + "fmt" + "reflect" + "runtime" + "sort" + "strings" + "testing" + "time" + + "fyne.io/fyne/v2" + + "github.com/donislawdev/TestingFilesGenerator/internal/gui/parts" +) + +// repetition is one screen doing the same round of work again and again, and +// what each round came to. See TestTheSameWorkDoneAgainLeavesTheWindowAsItWas. +type repetition struct { + t *testing.T + host *fakeHost + screen any + // work is one round. It has to end where it began, because every round + // after the first is compared with the first. + work func(r *repetition) + // before is every goroutine alive when the guard began, which a round is + // not asked about - a worker of an earlier guard finishing late is that + // guard's business. + before map[string]bool + rounds []roundCost + now *roundCost + // sawWorker is whether the count of goroutines ever saw the worker of a + // run while it was held. Asked only of a screen whose round runs. + sawWorker bool + runs bool +} + +// roundCost is what one round came to: the readings of the form and expansions +// of a preset each change caused, in the order the changes were made, and the +// things the screen could reach afterwards, by type. +type roundCost struct { + order []string + readings map[string][2]int + reachable map[string]int +} + +func newRepetition(t *testing.T, host *fakeHost, screen any) *repetition { + return &repetition{t: t, host: host, screen: screen} +} + +// change does one thing to the screen and keeps what it cost. +func (r *repetition) change(what string, act func()) { + r.t.Helper() + if _, again := r.now.readings[what]; again { + r.t.Fatalf("%q is done twice in one round, and the second would hide what the first cost", what) + } + r.host.settles, r.host.expansions = 0, 0 + act() + r.now.readings[what] = [2]int{r.host.settles, r.host.expansions} + r.now.order = append(r.now.order, what) +} + +// typeAndTypeBack types into a box and then puts back what it held, as two +// changes. +func (r *repetition) typeAndTypeBack(what string, box *parts.Entry, typed string) { + r.t.Helper() + was := box.Text + if was == typed { + r.t.Fatalf("%s already holds %q, so typing it would change nothing", what, typed) + } + r.change("typing into "+what, func() { box.SetText(typed) }) + r.change("typing "+what+" back", func() { box.SetText(was) }) +} + +// round does the work once and counts what it left. +func (r *repetition) round(n int) { + r.t.Helper() + r.now = &roundCost{readings: map[string][2]int{}} + r.work(r) + r.now.reachable = reachableByType(r.screen) + r.rounds = append(r.rounds, *r.now) + if left := r.goroutinesLeft(); len(left) > 0 { + r.t.Errorf("round %d finished and %d goroutine(s) started during this guard are still running our code, "+ + "so every round leaves one more behind. The first of them:\n%s", n, len(left), firstLines(left[0], 24)) + } +} + +// lookAtTheWorker is called while a run is held just before it reports. The +// worker is plainly running then, so a count that does not see it would not +// see one left behind either. +func (r *repetition) lookAtTheWorker() { + r.runs = true + for _, g := range goroutinesOfOurs(r.before) { + if strings.Contains(g, modulePath+"/internal/gui/window.") { + r.sawWorker = true + } + } +} + +// goroutinesLeft is every goroutine started during this guard that is still +// running our code, waited for a little. The wait for a run is the worker +// closing a channel, and the worker returns only after that, so for a moment +// it is still there with nothing wrong - asking once would be a guard that +// fails on a busy machine and on nothing else. +func (r *repetition) goroutinesLeft() []string { + deadline := time.Now().Add(5 * time.Second) + for { + left := goroutinesOfOurs(r.before) + if len(left) == 0 || time.Now().After(deadline) { + return left + } + time.Sleep(5 * time.Millisecond) + } +} + +// compare asks every round after the first whether it came to the same as the +// first, and first whether the counts can see anything at all. +func (r *repetition) compare() { + r.t.Helper() + first := r.rounds[0] + for _, what := range first.order { + if first.readings[what][0] == 0 { + r.t.Fatalf("%s read the form nought times in the first round - either nothing changed or the screen reads its form "+ + "without telling the host, and a change that grew from nought to nought would pass", what) + } + } + if first.reachable["*parts.Entry"] == 0 || first.reachable["*fyne.Container"] == 0 { + r.t.Fatalf("the screen reaches no box or no container through its fields (%v), so the count cannot see anything grow", first.reachable) + } + if r.runs && !r.sawWorker { + r.t.Fatal("the count of goroutines never saw the worker of a run while it was held, so it would not see one left behind") + } + readings, reached := 0, 0 + for _, what := range first.order { + readings += first.readings[what][0] + } + for _, n := range first.reachable { + reached += n + } + r.t.Logf("%d rounds of %d changes: %d readings of the form a round, %d things reached after it, a run seen: %v", + len(r.rounds), len(first.order), readings, reached, r.sawWorker) + for i, later := range r.rounds[1:] { + for _, what := range first.order { + if got, want := later.readings[what], first.readings[what]; got != want { + r.t.Errorf("%s read the form %d time(s) and expanded a preset %d time(s) in round %d, against %d and %d in round 1. "+ + "The same change costing more each time it is repeated is how a chain of listeners looks from outside", + what, got[0], got[1], i+2, want[0], want[1]) + } + } + if grew := differences(first.reachable, later.reachable); grew != "" { + r.t.Errorf("after round %d the screen reaches a different set of things than after round 1, in the same state: %s", i+2, grew) + } + } +} + +// differences lists the types whose counts differ between two rounds, as +// "+3 *parts.Entry", sorted, or nothing when they agree. +func differences(was, is map[string]int) string { + var out []string + for kind := range union(was, is) { + if d := is[kind] - was[kind]; d != 0 { + out = append(out, fmt.Sprintf("%+d %s", d, kind)) + } + } + sort.Strings(out) + return strings.Join(out, ", ") +} + +func union(a, b map[string]int) map[string]bool { + out := map[string]bool{} + for k := range a { + out[k] = true + } + for k := range b { + out[k] = true + } + return out +} + +// canvasObject is the interface every drawn thing of the toolkit implements. +var canvasObject = reflect.TypeOf((*fyne.CanvasObject)(nil)).Elem() + +// reachableByType counts the drawn things a screen can reach through its own +// fields, by type, each once. +// +// Through its fields rather than through what is drawn, because a defect of +// this kind does not have to be on the screen: a map keeping the controls of a +// removed batch is as much a leak as a box keeping its old panels. Pointers, +// interfaces, structs, slices, arrays and maps of the toolkit and of this +// module are followed. Functions are not - a closure is out of reach of +// reflection, which is written down as what this cannot see. +// +// A thing is told apart by its address AND its type. A widget of ours embeds +// the toolkit's widget first, so the two share an address, and counting by the +// address alone would count whichever the walk happened to meet first - and a +// walk through maps meets things in a different order every time. +func reachableByType(root any) map[string]int { + type key struct { + at uintptr + kind reflect.Type + } + counts := map[string]int{} + seen := map[key]bool{} + var visit func(v reflect.Value) + visit = func(v reflect.Value) { + switch v.Kind() { + case reflect.Interface: + if !v.IsNil() { + visit(v.Elem()) + } + case reflect.Pointer: + k := key{at: v.Pointer(), kind: v.Type()} + if v.IsNil() || !followed(v.Type().Elem()) || seen[k] { + return + } + seen[k] = true + if v.Type().Implements(canvasObject) { + counts[v.Type().String()]++ + } + visit(v.Elem()) + case reflect.Struct: + if followed(v.Type()) { + for i := 0; i < v.NumField(); i++ { + visit(v.Field(i)) + } + } + case reflect.Slice, reflect.Array: + if mayHoldPointers(v.Type().Elem()) { + for i := 0; i < v.Len(); i++ { + visit(v.Index(i)) + } + } + case reflect.Map: + if mayHoldPointers(v.Type().Key()) || mayHoldPointers(v.Type().Elem()) { + for it := v.MapRange(); it.Next(); { + visit(it.Key()) + visit(it.Value()) + } + } + } + } + visit(reflect.ValueOf(root)) + return counts +} + +// followed is whether the walk goes inside a type: the toolkit's and this +// module's, and types with no package, like a struct written in place. Not +// this package's, because the stand in host holds the whole window, and not +// the standard library's, whose locks and clocks hold nothing drawn. +func followed(t reflect.Type) bool { + p := t.PkgPath() + if strings.HasPrefix(p, modulePath+"/internal/guard") { + return false + } + return p == "" || strings.HasPrefix(p, "fyne.io/fyne/v2") || strings.HasPrefix(p, modulePath) +} + +// mayHoldPointers is whether a value of this type can lead anywhere, so that a +// slice of bytes - a font is a few hundred thousand of them - is not walked +// one byte at a time. +func mayHoldPointers(t reflect.Type) bool { + switch t.Kind() { + case reflect.Pointer, reflect.Interface, reflect.Slice, reflect.Map: + return true + case reflect.Struct: + return followed(t) + case reflect.Array: + return mayHoldPointers(t.Elem()) + } + return false +} + +// goroutineIDs is every goroutine alive now, by the number the runtime gives it. +func goroutineIDs() map[string]bool { + ids := map[string]bool{} + for _, g := range allGoroutines() { + ids[goroutineID(g)] = true + } + return ids +} + +// goroutinesOfOurs is every goroutine, other than the one asking and those in +// before, with a function of this module on its stack outside this package. +func goroutinesOfOurs(before map[string]bool) []string { + var out []string + for _, g := range allGoroutines()[1:] { + if before[goroutineID(g)] { + continue + } + for _, line := range strings.Split(g, "\n") { + if strings.HasPrefix(line, modulePath+"/internal/") && !strings.HasPrefix(line, modulePath+"/internal/guard") { + out = append(out, g) + break + } + } + } + return out +} + +// allGoroutines is the stack of every goroutine, the one asking first - which +// is the order runtime.Stack promises. +func allGoroutines() []string { + buf := make([]byte, 1<<20) + for { + n := runtime.Stack(buf, true) + if n < len(buf) { + return strings.Split(strings.TrimSpace(string(buf[:n])), "\n\n") + } + buf = make([]byte, 2*len(buf)) + } +} + +// goroutineID is the number in "goroutine 42 [running]:". +func goroutineID(stack string) string { + fields := strings.Fields(stack) + if len(fields) < 2 { + return stack + } + return fields[1] +} diff --git a/internal/guard/workrepeated_test.go b/internal/guard/workrepeated_test.go new file mode 100644 index 0000000..f23f0d8 --- /dev/null +++ b/internal/guard/workrepeated_test.go @@ -0,0 +1,226 @@ +package guard + +import ( + "os" + "path/filepath" + "testing" + + "github.com/donislawdev/TestingFilesGenerator/internal/damage" + "github.com/donislawdev/TestingFilesGenerator/internal/engine" + "github.com/donislawdev/TestingFilesGenerator/internal/format" + "github.com/donislawdev/TestingFilesGenerator/internal/gui/parts" + "github.com/donislawdev/TestingFilesGenerator/internal/gui/text" + "github.com/donislawdev/TestingFilesGenerator/internal/gui/window" + "github.com/donislawdev/TestingFilesGenerator/internal/preset" + "github.com/donislawdev/TestingFilesGenerator/internal/recipe" +) + +// repeatedRounds is how many times each screen does its round. One round is +// the one every later round is compared with, so three is the fewest that +// shows growth twice rather than once. +const repeatedRounds = 4 + +// The same work done again leaves the window as it was. +// +// A person sits in this window for an afternoon and does the same few things +// over and over, and on 2026-09-23 that is what the window could not survive. +// The batch screen wired every kept control once more on every rebuild, so the +// preset switch took 0.7 s at its first press and 6.1 s at its twentieth +// (docs/GUI-MEMORY-2026-09-23.md section 2.2). That one place has a guard of +// its own, TestAControlRegisteredAgainReportsOnceUnderItsLatestAddress. This +// one asks about the class rather than the place: every screen does one round +// of the work people repeat, several times over, and after every round three +// counts have to come out the same. +// +// - How many times each change read the form and expanded a preset. A chain +// of listeners anywhere, not only where the last one was, makes the same +// change cost more with every round. +// - How many things the screen can reach through its own fields, rather +// than through what is drawn. A rebuild that leaves the old panels in a +// box and a map that keeps the controls of a removed batch look the same +// from here. +// - How many goroutines started during this guard are still running our code +// once a round's run has finished. Nothing else asks: the wait for a run is +// a channel the worker closes before it returns, and a worker stuck after +// that point holds nothing up, not even the end of the test binary. +// +// Counts rather than memory, because the toolkit's test driver never clears +// its cache of renderers - only the loop of a real window does - so the heap +// of a test grows with every rebuild of a healthy window. Memory is asked of +// the real window once per release, in phase 11 of the release check. What +// this guard cannot see is listed in docs/GUI-LEAK-TEST-2026-09-25.md section 7. +func TestTheSameWorkDoneAgainLeavesTheWindowAsItWas(t *testing.T) { + before := goroutineIDs() + for _, s := range []struct { + name string + start func(t *testing.T) *repetition + }{ + {"single batch", singleBatchRound}, + {"presets", presetRound}, + {"several batches", batchesRound}, + } { + t.Run(s.name, func(t *testing.T) { + r := s.start(t) + r.before = before + for n := 1; n <= repeatedRounds; n++ { + r.round(n) + } + r.compare() + }) + } +} + +// singleBatchRound is every format in turn, a damage and none, a size typed +// and typed back, and one small run into a directory of its own. +// +// The run is held just before it reports, which is where the count of +// goroutines is shown to see the worker at all - a count that saw nothing +// while one was plainly running would pass every round with one left behind. +func singleBatchRound(t *testing.T) *repetition { + host := newFakeHost(t) + g := window.NewGenerate(host) + hold := newHold() + g.HoldBeforeFinishing(hold.enter) + // In this order because cleanups run last first: a round that failed + // before looking leaves the worker parked, and waiting for it before + // letting it go would wait for ever. + t.Cleanup(g.Settled) + t.Cleanup(hold.free) + + fields := g.Fields() + formats := chooserIn(t, fields, engine.SettingFormat) + damages := chooserIn(t, fields, recipe.KeyDamage) + size := entryIn(t, fields, format.SettingSize) + // Text, and a size nobody has to wait for, so the run costs what writing + // one small file costs. Set once, before the first round, so every round + // starts from it and ends on it. + formats.SetSelected("txt") + size.SetText("1kb") + + r := newRepetition(t, host, g) + r.work = func(r *repetition) { + for _, id := range inTurnFrom(t, format.IDs(), formats.Selected) { + r.change("choosing "+id, func() { formats.SetSelected(id) }) + } + r.change("choosing a damage", func() { damages.SetSelected(damage.Names()[0]) }) + r.change("choosing no damage", func() { damages.SetSelected(text.DamageNone()) }) + r.change("typing a size", func() { size.SetText("3kb") }) + r.change("typing the size back", func() { size.SetText("1kb") }) + + dir := t.TempDir() + g.SetOutDir(dir) + r.change("a run", func() { + press(t, g.Object(), text.ButtonGenerate()) + hold.look(r.lookAtTheWorker) + g.Settled() + }) + hold.again() + if _, err := os.Stat(filepath.Join(dir, "manifest.json")); err != nil { + t.Fatalf("the run of this round wrote no manifest into %s (%v), so it was refused rather than run and the goroutines it would have left are not asked about", dir, err) + } + } + return r +} + +// presetRound is every preset in turn, with its seed typed and typed back, +// and the one parameter of size-boundaries - the preset screen expands the +// preset on every change, so this is where an expansion too many would show. +func presetRound(t *testing.T) *repetition { + host := newFakeHost(t) + p := window.NewPreset(host) + fields := p.Fields() + pick := chooserIn(t, fields, settingPresetBox) + + r := newRepetition(t, host, p) + r.work = func(r *repetition) { + for _, id := range inTurnFrom(t, preset.IDs(), pick.Selected) { + r.change("choosing "+id, func() { pick.SetSelected(id) }) + r.typeAndTypeBack(id+" seed", entryIn(t, fields, engine.SettingSeed), "7") + if id == "size-boundaries" { + r.typeAndTypeBack(id+" limit", entryIn(t, fields, "limit"), "20mb") + } + } + } + return r +} + +// batchesRound is what the batch screen is for: a base switched on, changed +// and switched off, a batch added and taken away, a batch copied and the copy +// taken away, an archive given contents and back, and the name of a batch +// typed into. Every one of them but the typing lays the screen out again, and +// the kept controls are registered again every time. +func batchesRound(t *testing.T) *repetition { + host := newFakeHost(t) + rec := window.NewRecipe(host) + body, fields := rec.Object(), rec.Fields() + pressing := func(name string) func() { + return func() { press(t, body, name) } + } + + r := newRepetition(t, host, rec) + r.work = func(r *repetition) { + base := toggleIn(t, fields, settingBuildOnPreset) + r.change("switching the base on", func() { base.SetChecked(true) }) + bases := baseMenuOf(t, fields) + r.change("choosing upload-validation as the base", func() { bases.SetSelected("upload-validation") }) + r.change("choosing tabular-import as the base", func() { bases.SetSelected("tabular-import") }) + r.change("switching the base off", func() { base.SetChecked(false) }) + + r.change("adding a batch", pressing(text.ButtonAddBatch())) + r.change("removing it", pressing(text.ButtonRemoveBatch())) + r.change("copying a batch", pressing(text.ButtonDuplicateBatch())) + r.change("removing the copy", pressing(text.ButtonRemoveBatch())) + + kind := chooserIn(t, fields, recipe.TargetAddress(1, recipe.KeyFormat)) + was := kind.Selected + if was == "zip" { + t.Fatal("the first batch starts as a zip, so choosing zip would change nothing and the contents are not asked about") + } + r.change("choosing zip", func() { kind.SetSelected("zip") }) + r.change("adding what the archive holds", pressing(text.ButtonAddContents())) + r.change("removing what the archive holds", pressing(text.ButtonRemoveContents())) + r.change("choosing the format back", func() { kind.SetSelected(was) }) + + name := entryIn(t, fields, recipe.TargetAddress(1, recipe.KeyID)) + r.typeAndTypeBack("the batch name", name, name.Text+"x") + + if findField(fields, recipe.TargetAddress(2, recipe.KeyID)) != nil { + t.Fatal("a second batch is still on the screen after the round, so the next round does not start where this one did") + } + } + return r +} + +// settingBuildOnPreset is the switch that makes the batch screen start from a +// preset. The screen keeps the name to itself. +const settingBuildOnPreset = "start_from_preset" + +// baseMenuOf is the menu of presets under the base, which is on the screen +// only while the switch is on. +func baseMenuOf(t *testing.T, fields *parts.Fields) *parts.Chooser { + t.Helper() + f := findField(fields, recipe.KeyExtends) + if f == nil { + t.Fatal("the base is switched on and nothing is registered for the preset it starts from") + } + for _, c := range reportingControls(f.Control) { + if pick, is := c.(*parts.Chooser); is { + return pick + } + } + t.Fatal("the base is switched on and there is no menu of presets under it") + return nil +} + +// inTurnFrom is every value once, starting after from and ending on it, so +// that each is a change from the one before and the round ends where it began. +func inTurnFrom(t *testing.T, values []string, from string) []string { + t.Helper() + for i, v := range values { + if v == from { + return append(append([]string{}, values[i+1:]...), values[:i+1]...) + } + } + t.Fatalf("%q is not among %v, so the round cannot end where it began", from, values) + return nil +} From 8d75646a532d45048d2dab374688e5db4d7770f0 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Fri, 25 Sep 2026 12:51:12 +0200 Subject: [PATCH 2/3] guard: say what the walk does with the kinds it does not name Co-Authored-By: Claude Opus 5.5 --- internal/guard/repetition_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/internal/guard/repetition_test.go b/internal/guard/repetition_test.go index a6509eb..84d9a84 100644 --- a/internal/guard/repetition_test.go +++ b/internal/guard/repetition_test.go @@ -237,6 +237,9 @@ func reachableByType(root any) map[string]int { visit(it.Value()) } } + default: + // A function holds its closure out of reflection's reach, and a + // channel, a number or a string holds nothing drawn. } } visit(reflect.ValueOf(root)) @@ -266,8 +269,11 @@ func mayHoldPointers(t reflect.Type) bool { return followed(t) case reflect.Array: return mayHoldPointers(t.Elem()) + default: + // Numbers, strings, functions, channels and bare pointers of the + // runtime: nothing drawn is reached through any of them. + return false } - return false } // goroutineIDs is every goroutine alive now, by the number the runtime gives it. From 260658120ab19ba6a455ee51294f7b01af83925c Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Fri, 25 Sep 2026 13:05:15 +0200 Subject: [PATCH 3/3] guard: the preset round says so when it never reaches a parameter A preset renamed or taken away left the limit of size-boundaries unasked with the guard still green. Raised by the review of #139. Co-Authored-By: Claude Opus 5.5 --- internal/guard/workrepeated_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/guard/workrepeated_test.go b/internal/guard/workrepeated_test.go index f23f0d8..94f150e 100644 --- a/internal/guard/workrepeated_test.go +++ b/internal/guard/workrepeated_test.go @@ -133,13 +133,21 @@ func presetRound(t *testing.T) *repetition { r := newRepetition(t, host, p) r.work = func(r *repetition) { + // Asked for by name, so it is asserted to have been met: a preset + // renamed or taken away would otherwise leave the parameter path + // unasked with the guard still green. Raised by the review of #139. + typedALimit := false for _, id := range inTurnFrom(t, preset.IDs(), pick.Selected) { r.change("choosing "+id, func() { pick.SetSelected(id) }) r.typeAndTypeBack(id+" seed", entryIn(t, fields, engine.SettingSeed), "7") if id == "size-boundaries" { r.typeAndTypeBack(id+" limit", entryIn(t, fields, "limit"), "20mb") + typedALimit = true } } + if !typedALimit { + t.Fatal("no preset is called size-boundaries any more, so no parameter of a preset was typed into and the expansions it causes are not asked about") + } } return r }