From 2fd67d42f9d2d28b8bc76e782e639183e7591350 Mon Sep 17 00:00:00 2001 From: PJ Date: Tue, 18 Aug 2026 00:17:10 +0530 Subject: [PATCH] fix(campaign): name every missing required flag, in flag order Five required flags ranged as a map, so omitting three told the operator about one, chosen at random. --- cmd/internal-tools/campaign/main.go | 25 +++++++++++++------- cmd/internal-tools/campaign/main_test.go | 29 ++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 8 deletions(-) diff --git a/cmd/internal-tools/campaign/main.go b/cmd/internal-tools/campaign/main.go index c545c8b..b04908f 100644 --- a/cmd/internal-tools/campaign/main.go +++ b/cmd/internal-tools/campaign/main.go @@ -74,17 +74,26 @@ func parseArguments(arguments []string, stderr io.Writer) (config, error) { } configuration.extraArguments = flagSet.Args() - for name, value := range map[string]string{ - "--spec": configuration.specPath, - "--bundle-id": configuration.bundleID, - "--arm": configuration.arm, - "--seeds": seedSpecification, - "--output": configuration.outputDirectory, + // Every missing flag is named together, in flag order: stopping at the + // first turns one rerun into one rerun per missing flag. + var missing []error + for _, required := range []struct { + name string + value string + }{ + {"--spec", configuration.specPath}, + {"--bundle-id", configuration.bundleID}, + {"--arm", configuration.arm}, + {"--seeds", seedSpecification}, + {"--output", configuration.outputDirectory}, } { - if value == "" { - return config{}, fmt.Errorf("%s is required", name) + if required.value == "" { + missing = append(missing, fmt.Errorf("%s is required", required.name)) } } + if err := errors.Join(missing...); err != nil { + return config{}, err + } switch configuration.platform { case "android", "ios", "web": default: diff --git a/cmd/internal-tools/campaign/main_test.go b/cmd/internal-tools/campaign/main_test.go index 7ba8798..1fa3bc0 100644 --- a/cmd/internal-tools/campaign/main_test.go +++ b/cmd/internal-tools/campaign/main_test.go @@ -71,6 +71,35 @@ func TestParseArguments_Rejections(t *testing.T) { } } +// Three flags missing is one rerun, not three: the operator is told about all +// of them at once, in flag order, whatever order the check happened to walk. +func TestParseArguments_NamesEveryMissingRequiredFlagInFlagOrder(t *testing.T) { + _, err := parseArguments( + []string{"--bundle-id", "a", "--seeds", "1", "--max-steps", "10"}, + io.Discard, + ) + if err == nil { + t.Fatal("got no error, want every missing flag named") + } + message := err.Error() + previous := -1 + for _, name := range []string{"--spec", "--arm", "--output"} { + at := strings.Index(message, name) + if at < 0 { + t.Fatalf("got %q, want %s named", message, name) + } + if at < previous { + t.Errorf("got %q, want the flags named in flag order", message) + } + previous = at + } + for _, supplied := range []string{"--bundle-id", "--seeds"} { + if strings.Contains(message, supplied) { + t.Errorf("got %q, want the supplied %s left out", message, supplied) + } + } +} + func TestRunArguments_PlatformDeviceFlagAndPassthrough(t *testing.T) { cases := []struct { platform string