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)