From 4c3e33181fc315715ad9097b76559d8db3a268dc Mon Sep 17 00:00:00 2001 From: Abdel Fane <32418586+thebenignhacker@users.noreply.github.com> Date: Tue, 20 Oct 2026 13:17:17 -0600 Subject: [PATCH] Start each cobra run in the cli tests with the usage block on, and put the pipeline flags back when a test ends (#43) scanPipeline turns the usage block off on the command it runs, and in the test binary scanCmd kept that setting after executeScan, runScanWith and the tests that call scanTarget(scanCmd, ...) directly. When one of them ran before TestUsageErrorsStillPrintUsage with no executeRoot test in between, cobra printed only "Error: accepts 1 arg(s), received 0" and the test failed. executeRoot and executeScan now turn the usage block back on before they run the command, and executeScan also turns it back on when its test ends, as executeRoot already did. resetPipelineFlags put only outputFormat and outputFile back when a test ended. The two skipped-file tests set failOn to "low" after calling it, and TestRunScanExcludesFromFlagConfigAndDefaults, run directly after either of them, ended the test binary with exit code 1 inside runScan. resetPipelineFlags now puts back every variable it sets when the test ends, as well as when it starts. Two tests cover this: a usage error through executeRoot or executeScan prints the usage block after a scan through executeScan or runScanWith, and resetPipelineFlags puts failOn and four other variables back when a test ends. Both failed before this change. go test -count=1 -shuffle=1792442745693168000 ./internal/cli, which failed before, passes. Shuffle seeds 1 to 40 failed 27 times on the previous commit; seeds 1 to 300 pass, and each of the 102 top-level tests passes when it runs directly after any other one. Fixes #43 --- internal/cli/root_test.go | 62 ++++++++++++++++++++++++++++++++++++ internal/cli/scan_test.go | 66 +++++++++++++++++++++++++++++---------- 2 files changed, 111 insertions(+), 17 deletions(-) diff --git a/internal/cli/root_test.go b/internal/cli/root_test.go index fcd1522..e773d08 100644 --- a/internal/cli/root_test.go +++ b/internal/cli/root_test.go @@ -26,6 +26,10 @@ func executeRoot(t *testing.T, args ...string) (string, error) { rootCmd.SetOut(&buf) rootCmd.SetErr(&buf) rootCmd.SetArgs(args) + // Tests that run the scan with scanTarget(scanCmd, ...) instead of through + // cobra leave this set, so start where the binary starts: with the usage + // block on for a usage error. + scanCmd.SilenceUsage = false t.Cleanup(func() { rootCmd.SetOut(nil) rootCmd.SetErr(nil) @@ -207,3 +211,61 @@ func TestUsageErrorsStillPrintUsage(t *testing.T) { t.Errorf("output = %q, want the usage block for a missing argument", output) } } + +// A scan turns the usage block off on scanCmd, and in a test binary it stays +// off after the test that ran the scan unless something turns it back on. A +// usage error in a later test still has to print the usage block, whichever +// helper ran the earlier scan and whichever helper runs the usage error. +func TestUsageErrorsPrintUsageAfterAnEarlierScan(t *testing.T) { + earlierScans := []struct { + name string + scan func(t *testing.T, dir string) + }{ + {"executeScan", func(t *testing.T, dir string) { + resetScanFlags(t) + if _, _, err := executeScan(t, dir); err != nil { + t.Fatalf("scan %s: %v", dir, err) + } + }}, + {"runScanWith", func(t *testing.T, dir string) { + if out := runScanWith(t, dir); out.err != "" { + t.Fatalf("scan %s: %s", dir, out.err) + } + }}, + } + usageErrors := []struct { + name string + run func(t *testing.T) (string, error) + }{ + {"executeRoot", func(t *testing.T) (string, error) { + return executeRoot(t, "scan") + }}, + {"executeScan", func(t *testing.T) (string, error) { + _, cobraOutput, err := executeScan(t) + return cobraOutput, err + }}, + } + + for _, earlier := range earlierScans { + for _, usage := range usageErrors { + t.Run(earlier.name+" then "+usage.name, func(t *testing.T) { + ranScan := t.Run("earlier test", func(t *testing.T) { + earlier.scan(t, t.TempDir()) + if !scanCmd.SilenceUsage { + t.Fatal("the scan left the usage block on, so this test no longer checks anything") + } + }) + if !ranScan { + return + } + output, err := usage.run(t) + if err == nil { + t.Fatal("expected an error for a missing argument") + } + if !strings.Contains(output, "Usage:") { + t.Errorf("output = %q, want the usage block for a missing argument", output) + } + }) + } + } +} diff --git a/internal/cli/scan_test.go b/internal/cli/scan_test.go index 693372c..53c1dbc 100644 --- a/internal/cli/scan_test.go +++ b/internal/cli/scan_test.go @@ -629,27 +629,55 @@ func TestRunScanEmptyDir(t *testing.T) { } } -// resetPipelineFlags puts the flag variables the scan pipeline reads back to -// their defaults, so a test does not inherit another test's settings. +// resetPipelineFlags sets the flag variables below to their defaults, with +// colour off, now and when the test ends, so a test neither inherits another +// test's values for them nor leaves its own to the next one. func resetPipelineFlags(t *testing.T) { t.Helper() - outputFormat = "text" - outputFile = "" - includeGlobs = "" - excludeGlobs = "" - minSeverity = "info" - noColor = true - showProgress = false - ignorePatterns = "" - ignoreCategories = "" - failOn = "" - baselineFile = "" - configFile = "" - gateFailOn = "high" - t.Cleanup(func() { + reset := func() { outputFormat = "text" outputFile = "" + includeGlobs = "" + excludeGlobs = "" + minSeverity = "info" + noColor = true + showProgress = false + ignorePatterns = "" + ignoreCategories = "" + failOn = "" + baselineFile = "" + configFile = "" + gateFailOn = "high" + } + reset() + t.Cleanup(reset) +} + +// A flag variable a test changes after calling resetPipelineFlags is back +// where resetPipelineFlags put it when that test ends. The skipped-file tests +// set failOn to "low" this way; TestRunScanExcludesFromFlagConfigAndDefaults, +// which does not set failOn, then ran with that threshold, and its runScan +// call ended the test binary with exit code 1. +func TestResetPipelineFlagsPutsTheDefaultsBackWhenTheTestEnds(t *testing.T) { + t.Run("earlier test", func(t *testing.T) { + resetPipelineFlags(t) + failOn = "low" + minSeverity = "high" + excludeGlobs = "test/*" + configFile = "other.yaml" + gateFailOn = "critical" }) + for _, v := range []struct{ name, got, want string }{ + {"failOn", failOn, ""}, + {"minSeverity", minSeverity, "info"}, + {"excludeGlobs", excludeGlobs, ""}, + {"configFile", configFile, ""}, + {"gateFailOn", gateFailOn, "high"}, + } { + if v.got != v.want { + t.Errorf("%s = %q after the earlier test ended, want the default %q", v.name, v.got, v.want) + } + } } func TestExitCode(t *testing.T) { @@ -1047,7 +1075,8 @@ func TestStreamFooterMatchesReportTotal(t *testing.T) { // executeScan runs "cryptoscan scan " through the root command, the way // the binary does. It returns what the scan wrote to stdout, what cobra printed // (the error line and usage), and the error. The flag variables it can change -// are restored afterwards so later tests start from the same state. +// are restored afterwards, and the usage block the scan turns off is turned +// back on, so later tests start from the same state. func executeScan(t *testing.T, args ...string) (stdout, cobraOutput string, err error) { t.Helper() savedFormat, savedMinSeverity, savedFailOn, savedConfig := outputFormat, minSeverity, failOn, configFile @@ -1056,7 +1085,10 @@ func executeScan(t *testing.T, args ...string) (stdout, cobraOutput string, err rootCmd.SetArgs(nil) rootCmd.SetOut(nil) rootCmd.SetErr(nil) + scanCmd.SilenceUsage = false }) + // Start where the binary starts, with the usage block on; see executeRoot. + scanCmd.SilenceUsage = false var cobraBuf bytes.Buffer rootCmd.SetOut(&cobraBuf)