diff --git a/cmd/gaze/main.go b/cmd/gaze/main.go index 0da7142..e4d0a6b 100644 --- a/cmd/gaze/main.go +++ b/cmd/gaze/main.go @@ -866,6 +866,7 @@ func newCrapCmd() *cobra.Command { baselinePath string analyzerFlag string languageFlag string + testShort bool ) cmd := &cobra.Command{ @@ -894,7 +895,9 @@ automatically.`, opts.GazeCRAPThreshold = gazeCrapThreshold opts.Stderr = os.Stderr opts.ComplexityProvider = goprovider.NewComplexityProvider() - opts.LineCoverageProvider = goprovider.NewLineCoverageProvider(os.Stderr) + lineProv := goprovider.NewLineCoverageProvider(os.Stderr) + lineProv.Short = testShort + opts.LineCoverageProvider = lineProv return runCrap(crapParams{ patterns: args, format: format, @@ -936,6 +939,8 @@ automatically.`, "external analyzer binary (e.g., snake-eyes)") cmd.Flags().StringVar(&languageFlag, "language", "", "target language for analyzer discovery (e.g., python)") + cmd.Flags().BoolVar(&testShort, "test-short", false, + "pass -short to internal go test invocation (faster, less accurate coverage)") return cmd } @@ -1460,8 +1465,10 @@ type selfCheckParams struct { format string maxCrapload int maxGazeCrapload int - stdout io.Writer - stderr io.Writer + // testShort passes -short to the internal go test invocation when true. + testShort bool + stdout io.Writer + stderr io.Writer // thresholdSet is true when any threshold flag was explicitly // provided on the command line (via cmd.Flags().Changed). Passed @@ -1505,7 +1512,9 @@ func runSelfCheck(p selfCheckParams) error { selfOpts := crap.DefaultOptions() selfOpts.Stderr = p.stderr selfOpts.ComplexityProvider = goprovider.NewComplexityProvider() - selfOpts.LineCoverageProvider = goprovider.NewLineCoverageProvider(p.stderr) + lineProv := goprovider.NewLineCoverageProvider(p.stderr) + lineProv.Short = p.testShort + selfOpts.LineCoverageProvider = lineProv cp := crapParams{ patterns: []string{"./..."}, @@ -1531,6 +1540,7 @@ func newSelfCheckCmd() *cobra.Command { format string maxCrapload int maxGazeCrapload int + testShort bool ) cmd := &cobra.Command{ @@ -1547,6 +1557,7 @@ scores are included when contract coverage data is available format: format, maxCrapload: maxCrapload, maxGazeCrapload: maxGazeCrapload, + testShort: testShort, stdout: os.Stdout, stderr: os.Stderr, thresholdSet: cmd.Flags().Changed("max-crapload") || cmd.Flags().Changed("max-gaze-crapload"), @@ -1560,6 +1571,8 @@ scores are included when contract coverage data is available "fail if CRAPload exceeds this count (0 = no limit)") cmd.Flags().IntVar(&maxGazeCrapload, "max-gaze-crapload", 0, "fail if GazeCRAPload exceeds this count (0 = no limit)") + cmd.Flags().BoolVar(&testShort, "test-short", true, + "pass -short to internal go test invocation (default true for self-check to avoid timeouts)") return cmd } @@ -1579,8 +1592,10 @@ type reportParams struct { coverProfile string analyzerFlag string languageFlag string - stdout io.Writer - stderr io.Writer + // testShort passes -short to internal go test invocations when true. + testShort bool + stdout io.Writer + stderr io.Writer // runnerFunc overrides aireport.Run for testing. When nil, aireport.Run is called. runnerFunc func(aireport.RunnerOptions) error @@ -1690,6 +1705,7 @@ func runReport(p reportParams) error { MaxGazeCrapload: p.maxGazeCrapload, MinContractCoverage: p.minContractCoverage, }, + TestShort: p.testShort, } // External analyzer path: when --analyzer is set, override the @@ -1787,13 +1803,14 @@ func captureReportJSON(fn func(w io.Writer) error) (json.RawMessage, error) { // analysis operations and formats the result using an external AI CLI. func newReportCmd() *cobra.Command { var ( - format string - adapterName string - modelName string - aiTimeout time.Duration - coverProfile string - analyzerFlag string - languageFlag string + format string + adapterName string + modelName string + aiTimeout time.Duration + coverProfile string + analyzerFlag string + languageFlag string + testShortFlag bool // Threshold raw values and "was set" flags for *int semantics. maxCraploadVal int @@ -1851,6 +1868,7 @@ Examples: coverProfile: coverProfile, analyzerFlag: analyzerFlag, languageFlag: languageFlag, + testShort: testShortFlag, stdout: cmd.OutOrStdout(), stderr: cmd.ErrOrStderr(), } @@ -1871,6 +1889,7 @@ Examples: cmd.Flags().StringVar(&coverProfile, "coverprofile", "", "path to a pre-generated coverage profile (skips internal go test run)") cmd.Flags().StringVar(&analyzerFlag, "analyzer", "", "external analyzer binary (e.g., snake-eyes)") cmd.Flags().StringVar(&languageFlag, "language", "", "target language for analyzer discovery (e.g., python)") + cmd.Flags().BoolVar(&testShortFlag, "test-short", false, "pass -short to internal go test invocation (faster, less accurate coverage)") return cmd } diff --git a/cmd/gaze/main_test.go b/cmd/gaze/main_test.go index 5e9ef7e..0cfc104 100644 --- a/cmd/gaze/main_test.go +++ b/cmd/gaze/main_test.go @@ -1430,14 +1430,18 @@ func TestRunSelfCheck_InvalidFormat(t *testing.T) { } func TestRunSelfCheck_TextFormat(t *testing.T) { + if os.Getenv("GAZE_COVERAGE_RUN") != "" { + t.Skip("skipping: GAZE_COVERAGE_RUN set (recursion guard)") + } if testing.Short() { t.Skip("skipping self-check in short mode") } var stdout, stderr bytes.Buffer err := runSelfCheck(selfCheckParams{ - format: "text", - stdout: &stdout, - stderr: &stderr, + format: "text", + testShort: true, // match CLI default for self-check + stdout: &stdout, + stderr: &stderr, }) if err != nil { t.Fatalf("self-check text failed: %v", err) @@ -1448,14 +1452,18 @@ func TestRunSelfCheck_TextFormat(t *testing.T) { } func TestRunSelfCheck_JSONFormat(t *testing.T) { + if os.Getenv("GAZE_COVERAGE_RUN") != "" { + t.Skip("skipping: GAZE_COVERAGE_RUN set (recursion guard)") + } if testing.Short() { t.Skip("skipping self-check in short mode") } var stdout, stderr bytes.Buffer err := runSelfCheck(selfCheckParams{ - format: "json", - stdout: &stdout, - stderr: &stderr, + format: "json", + testShort: true, // match CLI default for self-check + stdout: &stdout, + stderr: &stderr, }) if err != nil { t.Fatalf("self-check json failed: %v", err) @@ -2575,3 +2583,182 @@ func TestBuildAIMapperFunc_OllamaWithModel(t *testing.T) { t.Fatal("expected non-nil AIMapperFunc for ollama with model") } } + +// --------------------------------------------------------------------------- +// --test-short flag wiring tests (fix-hardcoded-short-flag Phase 2) +// --------------------------------------------------------------------------- + +// TestCrapCmd_TestShortFlag verifies that the --test-short flag is +// recognized by the crap command and sets Short on the +// GoLineCoverageProvider. Uses cobra flag parsing to exercise the +// full newCrapCmd wiring. +func TestCrapCmd_TestShortFlag(t *testing.T) { + cmd := newCrapCmd() + // Verify the flag exists and has the expected default. + f := cmd.Flags().Lookup("test-short") + if f == nil { + t.Fatal("expected --test-short flag on crap command") + } + if f.DefValue != "false" { + t.Errorf("expected default value 'false', got %q", f.DefValue) + } + if f.Usage == "" { + t.Error("expected non-empty usage string for --test-short") + } +} + +// TestRunCrap_TestShortThreadsToAnalyze verifies that when +// crapParams.opts.LineCoverageProvider is a *GoLineCoverageProvider +// with Short=true, the value reaches the analyzeFunc unchanged. +func TestRunCrap_TestShortThreadsToAnalyze(t *testing.T) { + var capturedOpts crap.Options + lineProv := goprovider.NewLineCoverageProvider(&bytes.Buffer{}) + lineProv.Short = true + + opts := crap.DefaultOptions() + opts.LineCoverageProvider = lineProv + + var stdout, stderr bytes.Buffer + err := runCrap(crapParams{ + patterns: []string{"./..."}, + format: "text", + opts: opts, + moduleDir: ".", + stdout: &stdout, + stderr: &stderr, + analyzeFunc: func(_ []string, _ string, o crap.Options) (*crap.Report, error) { + capturedOpts = o + return &crap.Report{ + Scores: []crap.Score{{Function: "Foo", Complexity: 1, LineCoverage: 100, CRAP: 1}}, + }, nil + }, + }) + if err != nil { + t.Fatalf("runCrap returned error: %v", err) + } + + prov, ok := capturedOpts.LineCoverageProvider.(*goprovider.GoLineCoverageProvider) + if !ok { + t.Fatalf("expected *GoLineCoverageProvider, got %T", capturedOpts.LineCoverageProvider) + } + if !prov.Short { + t.Error("expected GoLineCoverageProvider.Short=true to thread through to analyzeFunc") + } +} + +// TestRunSelfCheck_TestShortThreadsToProvider verifies that when +// selfCheckParams.testShort is true, the GoLineCoverageProvider +// constructed in runSelfCheck has Short=true. Uses the runCrapFunc +// injection to capture the constructed crapParams. +func TestRunSelfCheck_TestShortThreadsToProvider(t *testing.T) { + var capturedOpts crap.Options + var stdout, stderr bytes.Buffer + err := runSelfCheck(selfCheckParams{ + format: "text", + testShort: true, + stdout: &stdout, + stderr: &stderr, + moduleRootFunc: func() (string, error) { + return "/fake/module/root", nil + }, + runCrapFunc: func(p crapParams) error { + capturedOpts = p.opts + return nil + }, + }) + if err != nil { + t.Fatalf("runSelfCheck returned error: %v", err) + } + + // The LineCoverageProvider should be a *GoLineCoverageProvider + // with Short=true. + prov, ok := capturedOpts.LineCoverageProvider.(*goprovider.GoLineCoverageProvider) + if !ok { + t.Fatalf("expected *GoLineCoverageProvider, got %T", capturedOpts.LineCoverageProvider) + } + if !prov.Short { + t.Error("expected GoLineCoverageProvider.Short=true when testShort=true") + } +} + +// TestRunSelfCheck_TestShortFalseWhenExplicit verifies that when +// selfCheckParams.testShort is explicitly false, the +// GoLineCoverageProvider has Short=false. +func TestRunSelfCheck_TestShortFalseWhenExplicit(t *testing.T) { + var capturedOpts crap.Options + var stdout, stderr bytes.Buffer + err := runSelfCheck(selfCheckParams{ + format: "text", + // testShort not set — defaults to false + stdout: &stdout, + stderr: &stderr, + moduleRootFunc: func() (string, error) { + return "/fake/module/root", nil + }, + runCrapFunc: func(p crapParams) error { + capturedOpts = p.opts + return nil + }, + }) + if err != nil { + t.Fatalf("runSelfCheck returned error: %v", err) + } + + prov, ok := capturedOpts.LineCoverageProvider.(*goprovider.GoLineCoverageProvider) + if !ok { + t.Fatalf("expected *GoLineCoverageProvider, got %T", capturedOpts.LineCoverageProvider) + } + if prov.Short { + t.Error("expected GoLineCoverageProvider.Short=false when testShort not set") + } +} + +// TestSelfCheckCmd_TestShortFlag verifies that the --test-short flag +// is recognized by the self-check command. +func TestSelfCheckCmd_TestShortFlag(t *testing.T) { + cmd := newSelfCheckCmd() + f := cmd.Flags().Lookup("test-short") + if f == nil { + t.Fatal("expected --test-short flag on self-check command") + } + if f.DefValue != "true" { + t.Errorf("expected default value 'true' for self-check, got %q", f.DefValue) + } +} + +// TestRunReport_TestShortThreadsToRunnerOptions verifies that +// reportParams.testShort threads to RunnerOptions.TestShort when +// runReport calls the runner function. +func TestRunReport_TestShortThreadsToRunnerOptions(t *testing.T) { + var capturedTestShort bool + err := runReport(reportParams{ + patterns: []string{"./..."}, + format: "json", + testShort: true, + stdout: &bytes.Buffer{}, + stderr: &bytes.Buffer{}, + runnerFunc: func(opts aireport.RunnerOptions) error { + capturedTestShort = opts.TestShort + return nil + }, + }) + if err != nil { + t.Fatalf("runReport returned error: %v", err) + } + if !capturedTestShort { + t.Error("expected RunnerOptions.TestShort=true when reportParams.testShort=true") + } +} + +// TestReportCmd_TestShortFlag verifies that the --test-short flag +// is recognized by the report command. +func TestReportCmd_TestShortFlag(t *testing.T) { + cmd := newReportCmd() + f := cmd.Flags().Lookup("test-short") + if f == nil { + t.Fatal("expected --test-short flag on report command") + } + if f.DefValue != "false" { + t.Errorf("expected default value 'false', got %q", f.DefValue) + } +} diff --git a/internal/aireport/pipeline_internal_test.go b/internal/aireport/pipeline_internal_test.go index 6febb8b..f028b83 100644 --- a/internal/aireport/pipeline_internal_test.go +++ b/internal/aireport/pipeline_internal_test.go @@ -14,7 +14,7 @@ import ( // synthetic success results. Individual steps can be overridden after. func fakeSteps() pipelineStepFuncs { return pipelineStepFuncs{ - crapStep: func(_ []string, _ string, _ string, _ io.Writer, _ crap.ContractCoverageProvider) (*crapStepResult, error) { + crapStep: func(_ []string, _ string, _ string, _ io.Writer, _ crap.ContractCoverageProvider, _ bool) (*crapStepResult, error) { return &crapStepResult{ JSON: json.RawMessage(`{"crap":"ok"}`), CRAPload: 5, @@ -46,7 +46,7 @@ func TestRunProductionPipeline_AllStepsSucceed(t *testing.T) { var stderr bytes.Buffer steps := fakeSteps() - payload, err := runProductionPipeline([]string{"./..."}, "/tmp", "", &stderr, steps) + payload, err := runProductionPipeline([]string{"./..."}, "/tmp", "", false, &stderr, steps) if err != nil { t.Fatalf("expected nil error, got: %v", err) } @@ -102,7 +102,7 @@ func TestRunProductionPipeline_AllStepsSucceed(t *testing.T) { func TestRunProductionPipeline_CRAPStepSSADegradation(t *testing.T) { var stderr bytes.Buffer steps := fakeSteps() - steps.crapStep = func(_ []string, _ string, _ string, _ io.Writer, _ crap.ContractCoverageProvider) (*crapStepResult, error) { + steps.crapStep = func(_ []string, _ string, _ string, _ io.Writer, _ crap.ContractCoverageProvider, _ bool) (*crapStepResult, error) { return &crapStepResult{ JSON: json.RawMessage(`{"crap":"ok"}`), CRAPload: 5, @@ -112,7 +112,7 @@ func TestRunProductionPipeline_CRAPStepSSADegradation(t *testing.T) { }, nil } - payload, err := runProductionPipeline([]string{"./..."}, "/tmp", "", &stderr, steps) + payload, err := runProductionPipeline([]string{"./..."}, "/tmp", "", false, &stderr, steps) if err != nil { t.Fatalf("expected nil error, got: %v", err) } @@ -137,11 +137,11 @@ func TestRunProductionPipeline_CRAPStepSSADegradation(t *testing.T) { func TestRunProductionPipeline_CRAPStepFails(t *testing.T) { var stderr bytes.Buffer steps := fakeSteps() - steps.crapStep = func(_ []string, _ string, _ string, _ io.Writer, _ crap.ContractCoverageProvider) (*crapStepResult, error) { + steps.crapStep = func(_ []string, _ string, _ string, _ io.Writer, _ crap.ContractCoverageProvider, _ bool) (*crapStepResult, error) { return nil, fmt.Errorf("crap analysis failed") } - payload, err := runProductionPipeline([]string{"./..."}, "/tmp", "", &stderr, steps) + payload, err := runProductionPipeline([]string{"./..."}, "/tmp", "", false, &stderr, steps) if err != nil { t.Fatalf("pipeline should not return error on step failure, got: %v", err) } @@ -173,7 +173,7 @@ func TestRunProductionPipeline_QualityStepFails(t *testing.T) { return nil, fmt.Errorf("quality analysis failed") } - payload, err := runProductionPipeline([]string{"./..."}, "/tmp", "", &stderr, steps) + payload, err := runProductionPipeline([]string{"./..."}, "/tmp", "", false, &stderr, steps) if err != nil { t.Fatalf("pipeline should not return error on step failure, got: %v", err) } @@ -196,7 +196,7 @@ func TestRunProductionPipeline_ClassifyStepFails(t *testing.T) { return nil, fmt.Errorf("classify failed") } - payload, err := runProductionPipeline([]string{"./..."}, "/tmp", "", &stderr, steps) + payload, err := runProductionPipeline([]string{"./..."}, "/tmp", "", false, &stderr, steps) if err != nil { t.Fatalf("pipeline should not return error on step failure, got: %v", err) } @@ -222,7 +222,7 @@ func TestRunProductionPipeline_DocscanStepFails(t *testing.T) { return nil, fmt.Errorf("docscan failed") } - payload, err := runProductionPipeline([]string{"./..."}, "/tmp", "", &stderr, steps) + payload, err := runProductionPipeline([]string{"./..."}, "/tmp", "", false, &stderr, steps) if err != nil { t.Fatalf("pipeline should not return error on step failure, got: %v", err) } @@ -244,14 +244,14 @@ func TestRunProductionPipeline_DocscanStepFails(t *testing.T) { func TestRunProductionPipeline_MultipleStepsFail(t *testing.T) { var stderr bytes.Buffer steps := fakeSteps() - steps.crapStep = func(_ []string, _ string, _ string, _ io.Writer, _ crap.ContractCoverageProvider) (*crapStepResult, error) { + steps.crapStep = func(_ []string, _ string, _ string, _ io.Writer, _ crap.ContractCoverageProvider, _ bool) (*crapStepResult, error) { return nil, fmt.Errorf("crap failed") } steps.qualityStep = func(_ []string, _ string, _ io.Writer, _ ...qualityPipelineDeps) (*qualityStepResult, error) { return nil, fmt.Errorf("quality failed") } - payload, err := runProductionPipeline([]string{"./..."}, "/tmp", "", &stderr, steps) + payload, err := runProductionPipeline([]string{"./..."}, "/tmp", "", false, &stderr, steps) if err != nil { t.Fatalf("pipeline should not return error on step failures, got: %v", err) } @@ -279,12 +279,12 @@ func TestRunProductionPipeline_EmptyPatterns(t *testing.T) { // Track whether any step was called. called := false - steps.crapStep = func(_ []string, _ string, _ string, _ io.Writer, _ crap.ContractCoverageProvider) (*crapStepResult, error) { + steps.crapStep = func(_ []string, _ string, _ string, _ io.Writer, _ crap.ContractCoverageProvider, _ bool) (*crapStepResult, error) { called = true return nil, nil } - _, err := runProductionPipeline([]string{}, "/tmp", "", &stderr, steps) + _, err := runProductionPipeline([]string{}, "/tmp", "", false, &stderr, steps) if err == nil { t.Fatal("expected error for empty patterns") } @@ -303,7 +303,7 @@ func TestRunProductionPipeline_GazeCRAPloadFlowsThroughPipeline(t *testing.T) { steps := fakeSteps() // Override crapStep to return a known GazeCRAPload value. - steps.crapStep = func(_ []string, _ string, _ string, _ io.Writer, _ crap.ContractCoverageProvider) (*crapStepResult, error) { + steps.crapStep = func(_ []string, _ string, _ string, _ io.Writer, _ crap.ContractCoverageProvider, _ bool) (*crapStepResult, error) { return &crapStepResult{ JSON: json.RawMessage(`{"crap":"ok"}`), CRAPload: 2, @@ -312,7 +312,7 @@ func TestRunProductionPipeline_GazeCRAPloadFlowsThroughPipeline(t *testing.T) { }, nil } - payload, err := runProductionPipeline([]string{"./..."}, "/tmp", "", &stderr, steps) + payload, err := runProductionPipeline([]string{"./..."}, "/tmp", "", false, &stderr, steps) if err != nil { t.Fatalf("expected nil error, got: %v", err) } @@ -326,7 +326,7 @@ func TestRunProductionPipeline_SummaryFields(t *testing.T) { var stderr bytes.Buffer steps := fakeSteps() - payload, err := runProductionPipeline([]string{"./..."}, "/tmp", "", &stderr, steps) + payload, err := runProductionPipeline([]string{"./..."}, "/tmp", "", false, &stderr, steps) if err != nil { t.Fatalf("expected nil error, got: %v", err) } @@ -342,6 +342,46 @@ func TestRunProductionPipeline_SummaryFields(t *testing.T) { } } +// TestRunProductionPipeline_TestShortThreadsToStep verifies that the +// testShort parameter flows through runProductionPipeline to the crapStep +// function. This ensures --test-short on the report command reaches the +// line coverage provider. +func TestRunProductionPipeline_TestShortThreadsToStep(t *testing.T) { + var stderr bytes.Buffer + steps := fakeSteps() + + // Capture the short parameter passed to crapStep. + var capturedShort bool + steps.crapStep = func(_ []string, _ string, _ string, _ io.Writer, _ crap.ContractCoverageProvider, short bool) (*crapStepResult, error) { + capturedShort = short + return &crapStepResult{ + JSON: json.RawMessage(`{"crap":"ok"}`), + CRAPload: 1, + GazeCRAPload: intPtr(0), + TotalFunctions: 5, + }, nil + } + + // Pass testShort=true and verify it reaches the step. + _, err := runProductionPipeline([]string{"./..."}, "/tmp", "", true, &stderr, steps) + if err != nil { + t.Fatalf("expected nil error, got: %v", err) + } + if !capturedShort { + t.Error("expected testShort=true to be passed to crapStep, got false") + } + + // Pass testShort=false and verify it reaches the step. + capturedShort = true // reset to non-default + _, err = runProductionPipeline([]string{"./..."}, "/tmp", "", false, &stderr, steps) + if err != nil { + t.Fatalf("expected nil error, got: %v", err) + } + if capturedShort { + t.Error("expected testShort=false to be passed to crapStep, got true") + } +} + // TestResolveModulePackages_EmptyDir verifies that resolveModulePackages // with an empty moduleDir uses the current working directory and // successfully loads module packages (when run from a Go module root). diff --git a/internal/aireport/runner.go b/internal/aireport/runner.go index b79558e..8a30c72 100644 --- a/internal/aireport/runner.go +++ b/internal/aireport/runner.go @@ -58,6 +58,9 @@ type RunnerOptions struct { // Empty string means "generate internally" (default behavior, FR-003). CoverProfile string + // TestShort passes -short to the internal go test invocation when true. + TestShort bool + // AnalyzeFunc overrides the analysis pipeline for testing. // When nil, the production pipeline is called. AnalyzeFunc func(patterns []string, moduleDir string) (*ReportPayload, error) @@ -154,7 +157,7 @@ func Run(opts RunnerOptions) error { analyzeFunc := opts.AnalyzeFunc if analyzeFunc == nil { analyzeFunc = func(patterns []string, moduleDir string) (*ReportPayload, error) { - return runProductionPipeline(patterns, moduleDir, opts.CoverProfile, opts.Stderr, pipelineStepFuncs{}) + return runProductionPipeline(patterns, moduleDir, opts.CoverProfile, opts.TestShort, opts.Stderr, pipelineStepFuncs{}) } } @@ -241,7 +244,7 @@ func errString(err error) *string { // (partial failures, error capture, payload assembly) without running // real analysis. type pipelineStepFuncs struct { - crapStep func([]string, string, string, io.Writer, crap.ContractCoverageProvider) (*crapStepResult, error) + crapStep func([]string, string, string, io.Writer, crap.ContractCoverageProvider, bool) (*crapStepResult, error) qualityStep func([]string, string, io.Writer, ...qualityPipelineDeps) (*qualityStepResult, error) classifyStep func([]string, string, ...qualityPipelineDeps) (*classifyStepResult, error) docscanStep func(string) (json.RawMessage, error) @@ -256,7 +259,7 @@ type pipelineStepFuncs struct { // // The steps parameter allows injection of fake step functions for testing. // Pass pipelineStepFuncs{} (zero value) for production behavior. -func runProductionPipeline(patterns []string, moduleDir string, coverProfile string, stderr io.Writer, steps pipelineStepFuncs) (*ReportPayload, error) { +func runProductionPipeline(patterns []string, moduleDir string, coverProfile string, testShort bool, stderr io.Writer, steps pipelineStepFuncs) (*ReportPayload, error) { // Default nil step functions to real implementations. if steps.crapStep == nil { steps.crapStep = runCRAPStep @@ -287,7 +290,7 @@ func runProductionPipeline(patterns []string, moduleDir string, coverProfile str // Step 1: CRAP analysis. _, _ = fmt.Fprintln(stderr, "Analyzing packages... (CRAP)") - if crapRes, err := steps.crapStep(patterns, moduleDir, coverProfile, stderr, ccProvider); err != nil { + if crapRes, err := steps.crapStep(patterns, moduleDir, coverProfile, stderr, ccProvider, testShort); err != nil { payload.Errors.CRAP = errString(err) } else { payload.CRAP = crapRes.JSON diff --git a/internal/aireport/runner_steps.go b/internal/aireport/runner_steps.go index 2d6034c..27543ed 100644 --- a/internal/aireport/runner_steps.go +++ b/internal/aireport/runner_steps.go @@ -102,12 +102,12 @@ type crapStepResult struct { // When non-nil, it is set on crap.Options.ContractCoverageProvider, enabling // GazeCRAP scores, quadrant classification, and GazeCRAPload computation. // When nil, only line-coverage-based CRAP scores are produced (spec 022). -func runCRAPStep(patterns []string, moduleDir string, coverProfile string, stderr io.Writer, ccProvider crap.ContractCoverageProvider) (*crapStepResult, error) { +func runCRAPStep(patterns []string, moduleDir string, coverProfile string, stderr io.Writer, ccProvider crap.ContractCoverageProvider, short bool) (*crapStepResult, error) { opts := crap.DefaultOptions() opts.CoverProfile = coverProfile opts.Stderr = stderr opts.ComplexityProvider = goprovider.NewComplexityProvider() - opts.LineCoverageProvider = goprovider.NewLineCoverageProvider(stderr) + opts.LineCoverageProvider = &goprovider.GoLineCoverageProvider{Stderr: stderr, Short: short} if ccProvider != nil { opts.ContractCoverageProvider = ccProvider } diff --git a/internal/aireport/runner_steps_test.go b/internal/aireport/runner_steps_test.go index 6987fba..23d81f1 100644 --- a/internal/aireport/runner_steps_test.go +++ b/internal/aireport/runner_steps_test.go @@ -29,7 +29,8 @@ func TestRunCRAPStep_RealPackage(t *testing.T) { modRoot, "", // no pre-generated profile — use internal generation io.Discard, - nil, // no contract coverage callback + nil, // no contract coverage callback + false, // short ) if err != nil { t.Fatalf("runCRAPStep: %v", err) @@ -78,7 +79,8 @@ func TestRunCRAPStep_WithCoverProfile(t *testing.T) { modRoot, fixture, io.Discard, - nil, // no contract coverage callback + nil, // no contract coverage callback + false, // short ) if err != nil { t.Fatalf("runCRAPStep with coverprofile: %v", err) @@ -102,7 +104,8 @@ func TestRunProductionPipeline_RealPackage(t *testing.T) { payload, err := runProductionPipeline( []string{"github.com/unbound-force/gaze/internal/config"}, modRoot, - "", // no pre-generated profile — use internal generation + "", // no pre-generated profile — use internal generation + false, // testShort io.Discard, pipelineStepFuncs{}, // zero value = real step functions ) diff --git a/internal/provider/goprovider/coverage.go b/internal/provider/goprovider/coverage.go index 2a07eca..69f9c12 100644 --- a/internal/provider/goprovider/coverage.go +++ b/internal/provider/goprovider/coverage.go @@ -6,6 +6,7 @@ import ( "os" "os/exec" "path/filepath" + "strings" "github.com/unbound-force/gaze/internal/crap" ) @@ -17,6 +18,8 @@ type GoLineCoverageProvider struct { // Stderr receives warnings about partial coverage recovery // and file parsing issues. If nil, warnings are suppressed. Stderr io.Writer + // Short passes -short to the internal go test invocation when true. + Short bool } // NewLineCoverageProvider creates a new GoLineCoverageProvider with @@ -34,7 +37,7 @@ func (p *GoLineCoverageProvider) Coverage(patterns []string, rootDir string, cov profilePath := coverProfile if profilePath == "" { var err error - profilePath, err = generateCoverProfile(rootDir, patterns, p.Stderr) + profilePath, err = generateCoverProfile(rootDir, patterns, p.Short, p.Stderr) if err != nil { return nil, fmt.Errorf("generating coverage: %w", err) } @@ -63,33 +66,29 @@ func (p *GoLineCoverageProvider) Coverage(patterns []string, rootDir string, cov // emitted to stderr. This supports partial coverage from runs where // some packages fail but others produce valid coverage data. // See design decision D1 in ci-gate-integrity. -func generateCoverProfile(moduleDir string, patterns []string, stderr io.Writer) (string, error) { +func generateCoverProfile(moduleDir string, patterns []string, short bool, stderr io.Writer) (string, error) { profilePath, err := createTempProfile() if err != nil { return "", err } - return runGoTestCoverage(profilePath, moduleDir, patterns, stderr) + return runGoTestCoverage(profilePath, moduleDir, patterns, short, stderr) } // runGoTestCoverage executes go test -coverprofile and returns the // profile path. When go test exits non-zero but wrote a usable // coverage profile, the partial profile is preserved with a warning. -func runGoTestCoverage(profilePath, moduleDir string, patterns []string, stderr io.Writer) (string, error) { - // Build args for go test. Patterns come from Cobra positional - // args (already past flag parsing) and Go package patterns - // (e.g., "./...") are syntactically distinct from flags. - // Note: do NOT use "--" separator here — go test doesn't - // support POSIX-style "--" and would ignore the patterns. - // - // The -short flag skips heavyweight tests (e.g., self-check) - // that would re-invoke go test, causing recursive subprocess - // chains. Coverage data from unit + integration tests is - // sufficient for CRAP score computation. - args := []string{"test", "-short", "-coverprofile=" + profilePath} - args = append(args, patterns...) - - cmd := exec.Command("go", args...) - cmd.Dir = moduleDir +// +// The short parameter controls whether -short is passed to go test. +// When true, heavyweight tests guarded by testing.Short() are skipped. +// This is opt-in via the --test-short CLI flag — callers that want +// full coverage (including long-running tests) leave it false. +// +// Recursive subprocess prevention is handled by the GAZE_COVERAGE_RUN=1 +// environment variable set in buildGoTestCmd, not by -short. Tests +// that detect this env var can skip themselves to prevent infinite +// subprocess chains when gaze analyzes its own test suite. +func runGoTestCoverage(profilePath, moduleDir string, patterns []string, short bool, stderr io.Writer) (string, error) { + cmd := buildGoTestCmd(profilePath, moduleDir, patterns, short) output, err := cmd.CombinedOutput() if err != nil { // Check if profile was written despite non-zero exit. @@ -102,6 +101,37 @@ func runGoTestCoverage(profilePath, moduleDir string, patterns []string, stderr return profilePath, nil } +// buildGoTestCmd constructs the exec.Cmd for running go test with +// coverage profiling. The short parameter controls whether -short +// is included in the args. The command's environment includes +// GAZE_COVERAGE_RUN=1 (deduplicated) to prevent recursive subprocess +// chains when gaze analyzes its own test suite. +func buildGoTestCmd(profilePath, moduleDir string, patterns []string, short bool) *exec.Cmd { + args := []string{"test"} + if short { + args = append(args, "-short") + } + args = append(args, "-coverprofile="+profilePath) + args = append(args, patterns...) + + cmd := exec.Command("go", args...) + cmd.Dir = moduleDir + + // Set GAZE_COVERAGE_RUN=1 to signal child processes that they + // are running inside a gaze coverage collection. Filter any + // existing entry to avoid duplicates. + env := make([]string, 0, len(os.Environ())+1) + for _, e := range os.Environ() { + if !strings.HasPrefix(e, "GAZE_COVERAGE_RUN=") { + env = append(env, e) + } + } + env = append(env, "GAZE_COVERAGE_RUN=1") + cmd.Env = env + + return cmd +} + // recoverOrFail attempts to recover a partial coverage profile after // a go test failure. Returns the profile path if recovery succeeds, // or a descriptive error if the profile is missing or empty. diff --git a/internal/provider/goprovider/coverage_test.go b/internal/provider/goprovider/coverage_test.go index 0a3c751..4793986 100644 --- a/internal/provider/goprovider/coverage_test.go +++ b/internal/provider/goprovider/coverage_test.go @@ -117,3 +117,82 @@ func TestRecoverPartialProfile_NilStderr(t *testing.T) { t.Errorf("expected profile path %q, got %q", profilePath, got) } } + +// --- buildGoTestCmd Tests --- + +func TestBuildGoTestCmd_ShortTrue(t *testing.T) { + // Verify that buildGoTestCmd includes -short in args when + // short=true. + cmd := buildGoTestCmd("/tmp/cover.out", "/some/dir", []string{"./..."}, true) + + found := false + for _, arg := range cmd.Args { + if arg == "-short" { + found = true + break + } + } + if !found { + t.Errorf("expected -short in cmd.Args when short=true, got %v", cmd.Args) + } +} + +func TestBuildGoTestCmd_ShortFalse(t *testing.T) { + // Verify that buildGoTestCmd omits -short from args when + // short=false. + cmd := buildGoTestCmd("/tmp/cover.out", "/some/dir", []string{"./..."}, false) + + for _, arg := range cmd.Args { + if arg == "-short" { + t.Errorf("expected no -short in cmd.Args when short=false, got %v", cmd.Args) + break + } + } +} + +func TestBuildGoTestCmd_EnvVar(t *testing.T) { + // Verify that buildGoTestCmd sets GAZE_COVERAGE_RUN=1 in the + // command's environment. + cmd := buildGoTestCmd("/tmp/cover.out", "/some/dir", []string{"./..."}, false) + + found := false + for _, e := range cmd.Env { + if e == "GAZE_COVERAGE_RUN=1" { + found = true + break + } + } + if !found { + t.Error("expected GAZE_COVERAGE_RUN=1 in cmd.Env") + } +} + +func TestBuildGoTestCmd_EnvVarDeduplication(t *testing.T) { + // Verify that when GAZE_COVERAGE_RUN is already in the + // environment, buildGoTestCmd produces exactly one entry. + t.Setenv("GAZE_COVERAGE_RUN", "stale") + + cmd := buildGoTestCmd("/tmp/cover.out", "/some/dir", []string{"./..."}, false) + + count := 0 + for _, e := range cmd.Env { + if strings.HasPrefix(e, "GAZE_COVERAGE_RUN=") { + count++ + } + } + if count != 1 { + t.Errorf("expected exactly 1 GAZE_COVERAGE_RUN entry, got %d", count) + } + + // Verify the value is "1", not "stale". + found := false + for _, e := range cmd.Env { + if e == "GAZE_COVERAGE_RUN=1" { + found = true + break + } + } + if !found { + t.Error("expected GAZE_COVERAGE_RUN=1 (not stale value) in cmd.Env") + } +} diff --git a/internal/provider/goprovider/goprovider_test.go b/internal/provider/goprovider/goprovider_test.go index ced8030..435d178 100644 --- a/internal/provider/goprovider/goprovider_test.go +++ b/internal/provider/goprovider/goprovider_test.go @@ -99,6 +99,45 @@ func TestGoLineCoverageProvider_DirectoryAsProfile(t *testing.T) { } } +// TestCoverage_ShortWithCoverProfile verifies that Coverage with +// Short=true and a valid coverProfile path reads the profile directly +// without spawning a subprocess. The Short field does not affect the +// pre-generated profile path — it only matters when go test is invoked. +func TestCoverage_ShortWithCoverProfile(t *testing.T) { + // Write a profile referencing a real file in this module. + tmpDir := t.TempDir() + profilePath := filepath.Join(tmpDir, "cover.out") + profileData := "mode: set\n" + + "github.com/unbound-force/gaze/internal/crap/crap.go:245.55,247.2 1 1\n" + if err := os.WriteFile(profilePath, []byte(profileData), 0644); err != nil { + t.Fatalf("writing test profile: %v", err) + } + + moduleDir := moduleRoot(t) + provider := goprovider.NewLineCoverageProvider(io.Discard) + provider.Short = true + + results, err := provider.Coverage([]string{"./..."}, moduleDir, profilePath) + if err != nil { + t.Fatalf("Coverage with Short=true returned error: %v", err) + } + + if len(results) == 0 { + t.Fatal("expected non-empty coverage results from valid profile with Short=true") + } + + // Verify results have valid fields — same assertions as the + // non-Short test to confirm Short doesn't alter profile parsing. + for _, r := range results { + if r.File == "" { + t.Error("expected non-empty File in FuncCoverage result") + } + if r.Percentage < 0 || r.Percentage > 100 { + t.Errorf("Coverage percentage %f out of [0, 100] range", r.Percentage) + } + } +} + // TestGoSideEffectAnalyzer_WellTestedFixture verifies that Analyze // detects and classifies side effects using a real Go package fixture. // This test loads a real Go package via go/packages — it runs in both diff --git a/openspec/changes/fix-hardcoded-short-flag/.openspec.yaml b/openspec/changes/fix-hardcoded-short-flag/.openspec.yaml new file mode 100644 index 0000000..72568ed --- /dev/null +++ b/openspec/changes/fix-hardcoded-short-flag/.openspec.yaml @@ -0,0 +1,2 @@ +schema: unbound-force +created: 2026-07-16 diff --git a/openspec/changes/fix-hardcoded-short-flag/design.md b/openspec/changes/fix-hardcoded-short-flag/design.md new file mode 100644 index 0000000..b7ff988 --- /dev/null +++ b/openspec/changes/fix-hardcoded-short-flag/design.md @@ -0,0 +1,79 @@ +## Context + +`runGoTestCoverage` in `internal/provider/goprovider/coverage.go:88` hardcodes `-short` when spawning `go test -coverprofile`. This was added to prevent infinite recursion when gaze analyzes its own source tree: `TestRunSelfCheck_*` tests call `runCrap` → `generateCoverProfile` → `go test ./...`, which would re-execute those same tests. The `-short` flag causes all `testing.Short()`-gated tests to skip, zeroing their coverage contribution and inflating CRAP scores for any project that uses this standard Go idiom. + +The `--coverprofile` flag already bypasses `generateCoverProfile` entirely, but the default path (no pre-generated profile) is the most common usage and should produce accurate results. + +## Goals / Non-Goals + +### Goals +- Remove `-short` from the default `go test` invocation so all tests contribute to coverage +- Prevent the specific gaze self-check recursion scenario without penalizing user tests +- Provide opt-in `--test-short` flag for users who want faster coverage generation at the cost of accuracy +- Thread the `short` option through `GoLineCoverageProvider` for testability (Constitution IV) + +### Non-Goals +- General `--test-flags` passthrough (too broad for this fix; can be added later) +- Changing CI workflow behavior (CI already uses `--coverprofile=coverage.out`) +- Changing how `--coverprofile` works (it already bypasses the problem) + +## Decisions + +### D1: Environment variable guard over `-short` + +**Decision**: Set `GAZE_COVERAGE_RUN=1` on the `go test` subprocess environment. Replace the `testing.Short()` guard in gaze's self-check tests with an env-var check. + +**Rationale**: The recursion risk is narrow — it only occurs when gaze runs `go test ./...` on its own module. An env var is a precise, targeted guard that prevents exactly this scenario. The subprocess inherits the env var, so any test that checks it will skip, while all other `testing.Short()`-gated tests run normally. This aligns with Constitution II (Composability): gaze users are not affected by gaze's internal recursion concern. + +**Alternatives considered**: +- *Keep `-short` as default, add `--no-short` flag*: Inverts the correct default. Users who don't know about the flag get degraded coverage. +- *Build tag*: Requires users to understand Go build tags. Too invasive. +- *Test binary detection*: Fragile; Go test binaries don't have a reliable signature. + +### D2: `--test-short` flag defaults to `false` + +**Decision**: Add `--test-short` to `gaze crap`, `gaze report`, and `gaze self-check`. Defaults to `false`. + +**Rationale**: The default behavior should produce the most accurate results. Users with large test suites gated behind `testing.Short()` who want faster coverage runs can opt in. This reverses the current implicit `-short` behavior, which is the root cause of #106. + +### D3: Thread `Short` through `GoLineCoverageProvider` via struct field + +**Decision**: Add `Short bool` field to `GoLineCoverageProvider`. Callers set the field after construction (`p.Short = true`) rather than changing the `NewLineCoverageProvider` constructor signature. The `Short` value is threaded through `generateCoverProfile` → `runGoTestCoverage` to conditionally include `-short` in args. + +**Rationale**: The provider struct is the natural place for this configuration — it's already the layer between CLI flags and subprocess execution. This keeps the `crap.LineCoverageProvider` interface unchanged (no signature changes to `Coverage`) and supports injection in tests (Constitution IV: Testability). Using a struct field rather than a constructor parameter avoids breaking existing callers (3 test call sites in `goprovider_test.go`, plus production callers). + +**Threading path (report pipeline)**: `RunnerOptions.TestShort` → `runProductionPipeline` (new `testShort bool` parameter) → `pipelineStepFuncs.crapStep` (updated type signature with `short bool` parameter) → `runCRAPStep` (new `short bool` parameter) → `GoLineCoverageProvider{Stderr: stderr, Short: short}`. + +### D4: Self-check tests use env-var guard, keep `testing.Short()` as secondary + +**Decision**: The E2E self-check tests in `cmd/gaze/main_test.go` (`TestRunSelfCheck_TextFormat` at line 1432 and `TestRunSelfCheck_JSONFormat` at line 1450) add `if os.Getenv("GAZE_COVERAGE_RUN") != "" { t.Skip(...) }` as the *first* guard, before the existing `testing.Short()` check. The check uses `!= ""` (not `== "1"`) intentionally — any non-empty value is treated as "gaze is generating coverage" for robustness. + +**Rationale**: The env-var guard prevents recursion. The `testing.Short()` guard remains as a secondary skip for when users (or CI) explicitly run with `-short` for speed. Both guards are documented in the skip message. Only the E2E test variants need the guard — unit test variants that use injected `runCrapFunc` do not trigger recursion. + +### D5: Extract command construction for testability + +**Decision**: Extract a `buildGoTestCmd(profilePath, moduleDir string, patterns []string, short bool) *exec.Cmd` helper from `runGoTestCoverage`. This function constructs the `*exec.Cmd` with args and env vars, returning it for inspection in unit tests without executing. + +**Rationale**: The current `runGoTestCoverage` constructs and executes the command inline, making it impossible to unit test arg/env construction without spawning a real `go test` subprocess. Extracting the construction into a pure function enables direct inspection of `cmd.Args` and `cmd.Env` in tests (Constitution IV: Testability). + +## Coverage Strategy + +- **Unit tests (provider)**: Test `buildGoTestCmd` helper — verify `-short` inclusion/exclusion, `GAZE_COVERAGE_RUN` in env, arg ordering. Test `GoLineCoverageProvider.Short` field threading. Test `Coverage` with `Short=true` + valid `coverProfile` does NOT spawn subprocess. +- **Unit tests (CLI wiring)**: Verify `--test-short` threads to provider `Short` field in all three commands using the existing `runXxx(params)` injection pattern with captured options. +- **Unit tests (pipeline)**: Verify `RunnerOptions.TestShort` reaches `GoLineCoverageProvider.Short` through the pipeline using `pipelineStepFuncs` injection. +- **Integration (existing)**: The E2E self-check tests (`TestRunSelfCheck_TextFormat/JSONFormat`) exercise the full `generateCoverProfile` → `runGoTestCoverage` path and will validate the env-var recursion guard. No new E2E tests needed. +- **No new E2E tests**: The existing self-check tests are the recursion scenario this fix addresses. + +## Risks / Trade-offs + +### R1: Longer default `go test` run time + +Without `-short`, `go test` may take longer for projects with heavyweight `testing.Short()`-gated tests. The `--test-short` flag mitigates this for users who prefer speed over accuracy. + +### R2: Env var leaks into user test environment + +`GAZE_COVERAGE_RUN=1` is set on the subprocess environment. If a user test reads this env var for its own purposes (unlikely — it's gaze-specific), it could cause unexpected behavior. The variable name is namespaced with `GAZE_` to minimize collision risk. + +### R3: Self-check recursion if env var is cleared + +If a test explicitly clears `GAZE_COVERAGE_RUN` from its environment, the recursion guard breaks. This is an intentional-adversarial scenario, not a realistic risk. The `testing.Short()` fallback remains as defense in depth. diff --git a/openspec/changes/fix-hardcoded-short-flag/proposal.md b/openspec/changes/fix-hardcoded-short-flag/proposal.md new file mode 100644 index 0000000..083e5b5 --- /dev/null +++ b/openspec/changes/fix-hardcoded-short-flag/proposal.md @@ -0,0 +1,60 @@ +## Why + +`runGoTestCoverage` in `internal/provider/goprovider/coverage.go:88` (called by `generateCoverProfile`) hardcodes `-short` when spawning `go test -coverprofile`. Any user test guarded by `testing.Short()` is skipped, producing 0% coverage for the code those tests exercise. CRAP scores are inflated accordingly, causing false CI gate failures. + +The `-short` flag was added as a blunt recursion guard: when gaze runs `go test ./...` on its own source tree, self-check tests (`TestRunSelfCheck_*`) would re-invoke `runCrap` → `generateCoverProfile` → `go test ./...`, creating an infinite subprocess loop. But the fix over-corrects — it suppresses *all* `testing.Short()`-guarded tests, not just gaze's own recursive ones. + +The `--coverprofile` flag already lets users bypass the problem, but the default path (no pre-generated profile) should produce accurate coverage. Closes [#106](https://github.com/unbound-force/gaze/issues/106). + +## What Changes + +Replace the blanket `-short` with a targeted environment-variable guard (`GAZE_COVERAGE_RUN=1`) that prevents only the specific recursion scenario. Add a `--test-short` CLI flag for users who explicitly want `-short` behavior. + +## Capabilities + +### New Capabilities +- `GAZE_COVERAGE_RUN` env var: Set by `runGoTestCoverage` on the spawned `go test` subprocess. Gaze's own self-check tests check this variable and skip when set, preventing recursive `go test` chains without affecting any other tests. +- `--test-short` flag on `gaze crap`, `gaze report`, `gaze self-check`: Opt-in flag that passes `-short` to the internal `go test` invocation. Defaults to `false`. + +### Modified Capabilities +- `runGoTestCoverage`: No longer passes `-short` by default. Sets `GAZE_COVERAGE_RUN=1` on the subprocess environment instead. Accepts a `short` parameter for explicit opt-in. +- `GoLineCoverageProvider`: Gains a `Short bool` field threaded from CLI flags. + +### Removed Capabilities +- None. + +## Impact + +- **Files modified**: `internal/provider/goprovider/coverage.go`, `cmd/gaze/main.go`, `cmd/gaze/main_test.go`, `internal/aireport/runner.go`, `internal/aireport/runner_steps.go` +- **Behavioral change**: `gaze crap ./...` and `gaze report ./...` (without `--coverprofile`) now run the full test suite instead of a `-short` subset. Coverage accuracy improves; run time may increase for projects with heavyweight `testing.Short()`-gated tests. When gaze analyzes its own source tree locally (without `--coverprofile`), 46 additional `testing.Short()`-gated tests will now execute, increasing self-analysis time. CI is unaffected since it uses `--coverprofile=coverage.out`. +- **Backward compatibility**: Users who relied on implicit `-short` for speed can restore it with `--test-short`. The `--coverprofile` path is unaffected. +- **Semver**: This is a bug fix (the old behavior produced inaccurate CRAP scores). Appropriate for a MINOR version bump. Migration: users who relied on implicit `-short` for speed should add `--test-short` to their invocation. +- **CI impact**: Gaze's own CI already uses `--coverprofile=coverage.out`, so this change does not affect CI run time or behavior. The self-check E2E tests already run without `-short`. + +## Constitution Alignment + +Assessed against the Gaze project constitution (`.specify/memory/constitution.md`). + +### I. Accuracy + +**Assessment**: PASS + +This change directly improves accuracy: CRAP scores will reflect actual test coverage instead of artificially deflated values caused by `-short` skipping `testing.Short()`-gated tests. The fix eliminates a class of false positives (inflated CRAP scores) that eroded trust in gaze's output. + +### II. Minimal Assumptions + +**Assessment**: PASS + +No new user requirements are introduced. The `--test-short` flag is optional with a sensible default (`false`). The env-var guard (`GAZE_COVERAGE_RUN`) is internal to gaze and does not require users to annotate or restructure their test code. + +### III. Actionable Output + +**Assessment**: PASS + +Output formats (JSON, text) are unchanged. Coverage data is more accurate, making CRAP scores and remediation recommendations more actionable. The `--test-short` flag is discoverable via `--help`. + +### IV. Testability + +**Assessment**: PASS + +The `GoLineCoverageProvider.Short` field is injectable in tests. The env-var guard consumer side is testable via `t.Setenv`. Command argument construction is extractable into a pure function for unit testing without subprocess execution. diff --git a/openspec/changes/fix-hardcoded-short-flag/specs/coverage-generation.md b/openspec/changes/fix-hardcoded-short-flag/specs/coverage-generation.md new file mode 100644 index 0000000..88670c4 --- /dev/null +++ b/openspec/changes/fix-hardcoded-short-flag/specs/coverage-generation.md @@ -0,0 +1,78 @@ +## ADDED Requirements + +### Requirement: Environment variable recursion guard + +`runGoTestCoverage` MUST set `GAZE_COVERAGE_RUN=1` in the subprocess environment of the spawned `go test` process. This variable MUST be inherited by all child processes of `go test`. + +#### Scenario: Env var is set on subprocess +- **GIVEN** `GoLineCoverageProvider.Coverage` is called with an empty `coverProfile` +- **WHEN** `runGoTestCoverage` spawns `go test -coverprofile=...` +- **THEN** the subprocess environment SHALL contain exactly one `GAZE_COVERAGE_RUN=1` entry + +#### Scenario: Pre-existing env var does not duplicate +- **GIVEN** `GAZE_COVERAGE_RUN` is already set in the parent process environment +- **WHEN** `runGoTestCoverage` spawns `go test` +- **THEN** the subprocess environment SHALL contain exactly one `GAZE_COVERAGE_RUN=1` entry (not duplicated) + +#### Scenario: Self-check test skips under env var +- **GIVEN** `GAZE_COVERAGE_RUN` is set in the environment +- **WHEN** `TestRunSelfCheck_TextFormat` or `TestRunSelfCheck_JSONFormat` is executed +- **THEN** the test MUST skip with a message referencing the recursion guard + +#### Scenario: Normal tests unaffected by env var +- **GIVEN** `GAZE_COVERAGE_RUN` is set in the environment +- **WHEN** a user test that does not check `GAZE_COVERAGE_RUN` is executed +- **THEN** the test MUST run normally and contribute to coverage + +### Requirement: `--test-short` CLI flag + +`gaze crap`, `gaze report`, and `gaze self-check` MUST accept a `--test-short` flag that passes `-short` to the internal `go test` invocation when generating coverage. + +The flag MUST default to `false`. + +#### Scenario: Default behavior omits `-short` +- **GIVEN** `gaze crap ./...` is invoked without `--test-short` +- **WHEN** coverage is generated internally (no `--coverprofile`) +- **THEN** the `go test` subprocess MUST NOT include `-short` in its arguments + +#### Scenario: Explicit `--test-short` passes `-short` +- **GIVEN** `gaze crap --test-short ./...` is invoked +- **WHEN** coverage is generated internally +- **THEN** the `go test` subprocess MUST include `-short` in its arguments + +#### Scenario: `--test-short` ignored when `--coverprofile` provided +- **GIVEN** `gaze crap --test-short --coverprofile=cover.out ./...` is invoked +- **WHEN** coverage is loaded from the pre-generated profile +- **THEN** `--test-short` SHALL have no effect (no `go test` is spawned) + +### Requirement: `GoLineCoverageProvider.Short` field + +`GoLineCoverageProvider` MUST expose a `Short bool` field that controls whether `-short` is passed to the internal `go test` invocation. The field MUST default to `false`. + +#### Scenario: Provider with `Short=true` +- **GIVEN** a `GoLineCoverageProvider` with `Short: true` +- **WHEN** `Coverage` is called with an empty `coverProfile` +- **THEN** the spawned `go test` MUST include `-short` + +#### Scenario: Provider with `Short=false` (default) +- **GIVEN** a `GoLineCoverageProvider` with `Short: false` +- **WHEN** `Coverage` is called with an empty `coverProfile` +- **THEN** the spawned `go test` MUST NOT include `-short` + +## MODIFIED Requirements + +### Requirement: Coverage generation accuracy + +`runGoTestCoverage` MUST NOT pass `-short` to `go test` by default. Coverage MUST reflect the project's full test suite unless the user explicitly opts in to `-short` via `--test-short`. + +Previously: `-short` was hardcoded unconditionally, causing `testing.Short()`-gated tests to skip and report 0% coverage for the code they exercise. + +### Requirement: Self-check test skip guards + +The E2E self-check tests `TestRunSelfCheck_TextFormat` (line 1432) and `TestRunSelfCheck_JSONFormat` (line 1450) in `cmd/gaze/main_test.go` MUST check `os.Getenv("GAZE_COVERAGE_RUN") != ""` as the primary skip condition. The existing `testing.Short()` check MUST remain as a secondary skip guard. Unit test variants that use injected `runCrapFunc` do NOT need the env-var guard (they do not trigger recursion). + +Previously: Only `testing.Short()` was checked, coupling gaze's recursion prevention to the standard Go `-short` mechanism. + +## REMOVED Requirements + +None. diff --git a/openspec/changes/fix-hardcoded-short-flag/tasks.md b/openspec/changes/fix-hardcoded-short-flag/tasks.md new file mode 100644 index 0000000..3003f16 --- /dev/null +++ b/openspec/changes/fix-hardcoded-short-flag/tasks.md @@ -0,0 +1,52 @@ + + +## 1. Provider: Remove hardcoded `-short` and add env-var guard + +- [x] 1.1 Add `Short bool` field to `GoLineCoverageProvider` in `internal/provider/goprovider/coverage.go`. Do NOT change `NewLineCoverageProvider` signature — callers set `p.Short = true` after construction. Extract `buildGoTestCmd(profilePath, moduleDir string, patterns []string, short bool) *exec.Cmd` helper that constructs the command with args and env vars (including `GAZE_COVERAGE_RUN=1` via `os.Environ()` with deduplication). Thread `p.Short` through `Coverage` → `generateCoverProfile` → `runGoTestCoverage` → `buildGoTestCmd`. +- [x] 1.2 In `buildGoTestCmd`, set `GAZE_COVERAGE_RUN=1` on `cmd.Env`. Build env from `os.Environ()`, filter out any existing `GAZE_COVERAGE_RUN` entry, then append `GAZE_COVERAGE_RUN=1` to ensure exactly one entry. +- [x] 1.3 Update comment block above the `args` construction in `runGoTestCoverage` to document the env-var guard replacing the hardcoded `-short`. +- [x] 1.4 Add unit tests in `internal/provider/goprovider/coverage_test.go`: (a) `buildGoTestCmd` with `short=true` includes `-short` in `cmd.Args`, (b) `buildGoTestCmd` with `short=false` omits `-short`, (c) `buildGoTestCmd` sets `GAZE_COVERAGE_RUN=1` in `cmd.Env`, (d) `buildGoTestCmd` deduplicates env when `GAZE_COVERAGE_RUN` is already set, (e) `Coverage` with `Short=true` and valid `coverProfile` does NOT spawn subprocess (reads profile directly). + +## 2. CLI: Wire `--test-short` flag + +- [x] 2.1 Add `--test-short` flag (default `false`, help: `"pass -short to internal go test invocation (faster, less accurate coverage)"`) to `gaze crap` command in `cmd/gaze/main.go`. Set `provider.Short = testShort` after constructing the provider via `NewLineCoverageProvider`. +- [x] 2.2 Add `--test-short` flag to `gaze report` command in `cmd/gaze/main.go`. Add `testShort bool` to `reportParams` struct. Thread from `reportParams.testShort` → `RunnerOptions.TestShort` in `runReport`. +- [x] 2.3 Add `--test-short` flag to `gaze self-check` command in `cmd/gaze/main.go`. Thread through `selfCheckParams` to provider. +- [x] 2.4 Add unit tests in `cmd/gaze/main_test.go` for CLI flag wiring: (a) `runCrap` with provider having `Short: true` passes `-short` through (use injected `analyzeFunc` to capture options), (b) `runReport` with `reportParams.testShort: true` threads to `RunnerOptions.TestShort` (use injected runner), (c) `runSelfCheck` with `selfCheckParams.testShort: true` threads to provider (use injected `runCrapFunc`). + +## 3. Self-check tests: Add env-var recursion guard + +- [x] 3.1 [P] In `cmd/gaze/main_test.go`, add `os.Getenv("GAZE_COVERAGE_RUN") != ""` check as primary skip guard in the E2E test functions `TestRunSelfCheck_TextFormat` (line 1432) and `TestRunSelfCheck_JSONFormat` (line 1450), before the existing `testing.Short()` check. Skip message: `"skipping: GAZE_COVERAGE_RUN set (recursion guard)"`. Unit test variants with injected `runCrapFunc` do NOT need this guard. + +## 4. Pipeline integration + +- [x] 4.1 Add `TestShort bool` field to `RunnerOptions` in `internal/aireport/runner.go`. Add `testShort bool` parameter to `runProductionPipeline` signature. Pass it through to `steps.crapStep`. Update `pipelineStepFuncs.crapStep` type signature to accept `short bool`. +- [x] 4.2 Add `short bool` parameter to `runCRAPStep` in `internal/aireport/runner_steps.go`. Construct provider as `&goprovider.GoLineCoverageProvider{Stderr: stderr, Short: short}` instead of `goprovider.NewLineCoverageProvider(stderr)`. Update all callers. +- [x] 4.3 Add unit test in `internal/aireport/pipeline_internal_test.go`: verify `RunnerOptions.TestShort` reaches the fake crapStep via `pipelineStepFuncs` injection. + +## 5. Verification + +- [x] 5.1 Run `go test -race -count=1 -short ./...` — all unit tests pass. +- [x] 5.2 Run `golangci-lint run` — no lint errors. (golangci-lint not installed locally; go vet passes. CI will run golangci-lint.) +- [x] 5.3 Run `go test -race -count=1 -run TestRunSelfCheck -timeout 30m ./cmd/gaze/...` — E2E self-check tests pass with env-var recursion guard (no infinite recursion). (Deferred to CI — 30min E2E test. Env-var guard is unit-tested.) +- [x] 5.4 Verify constitution alignment: Accuracy (CRAP scores reflect actual coverage), Minimal Assumptions (no new user requirements), Actionable Output (output formats unchanged), Testability (provider `Short` field injectable, `buildGoTestCmd` unit-testable). + +