From 176b49524576881becdc470c4d4ea93f40c7a4d5 Mon Sep 17 00:00:00 2001 From: PJ Date: Mon, 17 Aug 2026 23:57:54 +0530 Subject: [PATCH] fix(implementation-sweep): name every missing binary, in flag order Ranging a map returned at the first failure, so an operator missing three binaries was told about one, fixed it, reran, and was told about the next. The function exists to stop the sweep once rather than fail per implementation and seed. Two identical runs also printed different errors, which is why this reached master as a flake instead of a clean red. --- .../implementation-sweep/sweep.go | 28 +++++++++++------ .../implementation-sweep/sweep_test.go | 31 +++++++++++++++++++ 2 files changed, 49 insertions(+), 10 deletions(-) diff --git a/cmd/internal-tools/implementation-sweep/sweep.go b/cmd/internal-tools/implementation-sweep/sweep.go index defba0e..3df5ead 100644 --- a/cmd/internal-tools/implementation-sweep/sweep.go +++ b/cmd/internal-tools/implementation-sweep/sweep.go @@ -75,24 +75,32 @@ func discoverImplementations( // anything is installed. Each campaign runs from the sweep's own directory // rather than the implementation's, so a relative --sanderling would otherwise // resolve against the wrong one, and a binary that is missing altogether has to -// stop the sweep here rather than fail once per implementation and seed. +// stop the sweep here rather than fail once per implementation and seed. Every +// one that is missing is named together, in flag order: stopping at the first +// turns that single stop into one rerun per missing binary. func resolveBinaries(configuration *config) error { - for name, value := range map[string]*string{ - "--bun": &configuration.bunPath, - "--campaign": &configuration.campaignPath, - "--sanderling": &configuration.sanderlingPath, + var missing []error + for _, binary := range []struct { + name string + value *string + }{ + {"--bun", &configuration.bunPath}, + {"--campaign", &configuration.campaignPath}, + {"--sanderling", &configuration.sanderlingPath}, } { - resolved, err := exec.LookPath(*value) + resolved, err := exec.LookPath(*binary.value) if err != nil { - return fmt.Errorf("%s: %w", name, err) + missing = append(missing, fmt.Errorf("%s: %w", binary.name, err)) + continue } absolute, err := filepath.Abs(resolved) if err != nil { - return fmt.Errorf("%s: %w", name, err) + missing = append(missing, fmt.Errorf("%s: %w", binary.name, err)) + continue } - *value = absolute + *binary.value = absolute } - return nil + return errors.Join(missing...) } type sweep struct { diff --git a/cmd/internal-tools/implementation-sweep/sweep_test.go b/cmd/internal-tools/implementation-sweep/sweep_test.go index 8665716..872b312 100644 --- a/cmd/internal-tools/implementation-sweep/sweep_test.go +++ b/cmd/internal-tools/implementation-sweep/sweep_test.go @@ -101,6 +101,37 @@ func TestRunSweep_StopsBeforeItInstallsAnythingWhenABinaryIsMissing( } } +// Two binaries missing is one rerun, not two: the operator is told about both +// at once, in flag order, whatever order the check happened to walk. +func TestResolveBinaries_NamesEveryMissingBinaryInFlagOrder(t *testing.T) { + configuration := config{ + bunPath: writeScript( + t, + filepath.Join(t.TempDir(), "stub-bun"), + "#!/bin/sh\nexit 0\n", + ), + campaignPath: "campaign-that-is-not-installed", + sanderlingPath: "sanderling-that-is-not-installed", + } + + err := resolveBinaries(&configuration) + if err == nil { + t.Fatal("got no error, want both missing binaries named") + } + message := err.Error() + campaign := strings.Index(message, "--campaign") + sanderling := strings.Index(message, "--sanderling") + if campaign < 0 || sanderling < 0 { + t.Fatalf("got %q, want both --campaign and --sanderling named", message) + } + if campaign > sanderling { + t.Errorf("got %q, want --campaign named before --sanderling", message) + } + if strings.Contains(message, "--bun") { + t.Errorf("got %q, want the bun that resolved left out", message) + } +} + func TestRunSweep_RefusesADirectoryThatAlreadyHoldsASweep(t *testing.T) { implementations := t.TempDir() if err := os.MkdirAll(filepath.Join(implementations, "impl-01"), 0o755); err != nil {