From 435cae93c470c2720aa7be7c602471efe8d979ea Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 10 Jun 2026 22:54:57 -0700 Subject: [PATCH 1/2] test(cmd): make CSV purchase-path tests assert real behavior runToolFromCSV now returns an error instead of calling log.Fatalf, so its error branches are testable; runToolMultiService keeps the fail-loud CLI behavior by turning the returned error fatal. Test changes: - TestRunToolFromCSV asserts observable output (the written purchase report: resource types, counts, dry-run success flags) instead of assert.NotPanics. The old fixture used CSV headers the parser does not recognize (Instance Type, Instance Count), so every row loaded with Count=0, the whole run processed nothing, and NotPanics stayed green; the fixture now uses the real round-trip column names. - Un-skip TestRunToolFromCSV_NonExistentFile and _EmptyFile with real error assertions (previously skipped citing nonexistent integration coverage). - TestRunToolFromCSV_WithMaxInstances/_WithOverrideCount assert the cap and override against the report rows. - Replace tautological, live-AWS TestQueryMajorEngineVersions_Success and _ProfileHandling (ran real API calls under CI go test -short on credentialed machines) with hermetic stubbed-client tests of queryMajorEngineVersionsWithClient and a deterministic profile-precedence test that fails at config load, no network. - isolateAWSEnv pins invalid static credentials so the CSV-path tests behave identically on credentialed and bare machines. Closes #1154 --- cmd/multi_service.go | 15 +- cmd/multi_service_coverage_test.go | 143 ++++++++++----- cmd/multi_service_test.go | 269 +++++++++++++++++++---------- 3 files changed, 283 insertions(+), 144 deletions(-) diff --git a/cmd/multi_service.go b/cmd/multi_service.go index 9c4cf5e94..7bde3caf3 100644 --- a/cmd/multi_service.go +++ b/cmd/multi_service.go @@ -83,7 +83,9 @@ func effectiveDryRun(cfg Config) bool { // (2) score, display, confirm, and purchase. func runToolMultiService(ctx context.Context, cfg Config) { if cfg.CSVInput != "" { - runToolFromCSV(ctx, cfg) + if err := runToolFromCSV(ctx, cfg); err != nil { + log.Fatalf("%v", err) + } return } @@ -273,7 +275,9 @@ func buildServiceStats(recs []common.Recommendation, results []common.PurchaseRe } // runToolFromCSV processes recommendations from a CSV input file. -func runToolFromCSV(ctx context.Context, cfg Config) { +// It returns an error instead of exiting so the orchestration glue is +// unit-testable; the caller (runToolMultiService) turns errors fatal. +func runToolFromCSV(ctx context.Context, cfg Config) error { isDryRun := effectiveDryRun(cfg) printRunMode(isDryRun) @@ -284,7 +288,7 @@ func runToolFromCSV(ctx context.Context, cfg Config) { // Read recommendations from CSV recs, err := loadRecommendationsFromCSV(cfg.CSVInput) if err != nil { - log.Fatalf("Failed to read CSV file: %v", err) + return fmt.Errorf("failed to read CSV file: %w", err) } AppLogger.Printf("✅ Loaded %d recommendations from CSV\n", len(recs)) @@ -294,7 +298,7 @@ func runToolFromCSV(ctx context.Context, cfg Config) { if len(recs) == 0 { AppLogger.Println("⚠️ No recommendations to process after filtering") - return + return nil } // Load AWS configuration @@ -305,7 +309,7 @@ func runToolFromCSV(ctx context.Context, cfg Config) { } awsCfg, err := awsconfig.LoadDefaultConfig(ctx, configOptions...) if err != nil { - log.Fatalf("Failed to load AWS config: %v", err) + return fmt.Errorf("failed to load AWS config: %w", err) } // Create account alias cache for lookup @@ -386,6 +390,7 @@ func runToolFromCSV(ctx context.Context, cfg Config) { // Print final summary using the post-dedup slice so counts match what was // actually processed, not the pre-dedup input passed into the outer loop. printMultiServiceSummary(allAdjustedRecs, allResults, serviceStats, isDryRun) + return nil } // filterAndAdjustRecommendations applies filters, coverage, count override, and instance limits to recommendations. diff --git a/cmd/multi_service_coverage_test.go b/cmd/multi_service_coverage_test.go index 2fa4a676c..88afdc988 100644 --- a/cmd/multi_service_coverage_test.go +++ b/cmd/multi_service_coverage_test.go @@ -9,7 +9,10 @@ import ( "github.com/LeanerCloud/CUDly/pkg/common" "github.com/aws/aws-sdk-go-v2/aws" + awsrds "github.com/aws/aws-sdk-go-v2/service/rds" + rdstypes "github.com/aws/aws-sdk-go-v2/service/rds/types" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // ==================== Tests for coverage improvement ==================== @@ -18,72 +21,124 @@ import ( // ==================== Tests for queryMajorEngineVersions ==================== -func TestQueryMajorEngineVersions_Success(t *testing.T) { - // This test verifies the logic of queryMajorEngineVersions without mocking - // Since it requires AWS credentials, we test the error cases and structure +// engineKeyedRDSMajorVersionsStub implements RDSMajorVersionsClient with +// canned per-engine responses and records which engines were queried, so +// queryMajorEngineVersionsWithClient can be tested hermetically (no AWS +// credentials, no network). +type engineKeyedRDSMajorVersionsStub struct { + versionsByEngine map[string][]rdstypes.DBMajorEngineVersion + errByEngine map[string]error + enginesQueried []string +} - ctx := context.Background() - origCfg := toolCfg - defer func() { toolCfg = origCfg }() +func (s *engineKeyedRDSMajorVersionsStub) DescribeDBMajorEngineVersions( + _ context.Context, + params *awsrds.DescribeDBMajorEngineVersionsInput, + _ ...func(*awsrds.Options), +) (*awsrds.DescribeDBMajorEngineVersionsOutput, error) { + engine := aws.ToString(params.Engine) + s.enginesQueried = append(s.enginesQueried, engine) + if err := s.errByEngine[engine]; err != nil { + return nil, err + } + return &awsrds.DescribeDBMajorEngineVersionsOutput{ + DBMajorEngineVersions: s.versionsByEngine[engine], + }, nil +} - // Test with empty profile (should work if AWS credentials are configured) - toolCfg.Profile = "" - toolCfg.ValidationProfile = "" +// TestQueryMajorEngineVersionsWithClient_Success replaces a former +// live-credential test: it stubs the RDS client and asserts the merged +// engine:major-version map deterministically. +func TestQueryMajorEngineVersionsWithClient_Success(t *testing.T) { + stub := &engineKeyedRDSMajorVersionsStub{ + versionsByEngine: map[string][]rdstypes.DBMajorEngineVersion{ + "mysql": {rdsMajorVersion("mysql", "5.7"), rdsMajorVersion("mysql", "8.0")}, + "postgres": {rdsMajorVersion("postgres", "13")}, + }, + } - // This will attempt to load AWS config - may fail without credentials - result, err := queryMajorEngineVersions(ctx, toolCfg) + result, err := queryMajorEngineVersionsWithClient(context.Background(), stub) + require.NoError(t, err) + + assert.ElementsMatch(t, + []string{"mysql", "postgres", "aurora-mysql", "aurora-postgresql"}, + stub.enginesQueried, + "must query every supported engine exactly once") + + require.Len(t, result, 3) + assert.Equal(t, "mysql", result["mysql:5.7"].Engine) + assert.Equal(t, "5.7", result["mysql:5.7"].MajorEngineVersion) + assert.Equal(t, "8.0", result["mysql:8.0"].MajorEngineVersion) + assert.Equal(t, "13", result["postgres:13"].MajorEngineVersion) + require.Len(t, result["postgres:13"].SupportedEngineLifecycles, 1, + "lifecycle data must be carried through from the API response") +} - // Either succeeds with valid credentials or fails gracefully - if err != nil { - assert.Error(t, err) - assert.Contains(t, err.Error(), "failed to load AWS config") - } else { - // If it succeeds, verify the result structure - assert.NotNil(t, result) - // Result can be empty if no versions are found +// TestQueryMajorEngineVersionsWithClient_EngineErrorContinues asserts the +// warn-and-continue contract: one engine failing must not drop the results +// of the others, and the overall call still succeeds. +func TestQueryMajorEngineVersionsWithClient_EngineErrorContinues(t *testing.T) { + stub := &engineKeyedRDSMajorVersionsStub{ + versionsByEngine: map[string][]rdstypes.DBMajorEngineVersion{ + "postgres": {rdsMajorVersion("postgres", "15")}, + }, + errByEngine: map[string]error{ + "mysql": errors.New("DescribeDBMajorEngineVersions throttled"), + }, } + + result, err := queryMajorEngineVersionsWithClient(context.Background(), stub) + require.NoError(t, err, "per-engine API failures are warn-and-continue") + + assert.ElementsMatch(t, + []string{"mysql", "postgres", "aurora-mysql", "aurora-postgresql"}, + stub.enginesQueried, + "a failing engine must not stop the remaining engine queries") + require.Len(t, result, 1) + assert.Equal(t, "15", result["postgres:15"].MajorEngineVersion) } -func TestQueryMajorEngineVersions_ProfileHandling(t *testing.T) { +// TestQueryMajorEngineVersions_ProfileSelection asserts validation-profile +// precedence deterministically: the configured profiles point at names +// guaranteed not to exist in the (redirected, empty) shared config, so +// config loading fails fast without any live AWS call and the error names +// the profile the function actually selected. +func TestQueryMajorEngineVersions_ProfileSelection(t *testing.T) { ctx := context.Background() - origCfg := toolCfg - defer func() { toolCfg = origCfg }() tests := []struct { name string profile string validationProfile string + wantProfileInErr string }{ { - name: "Uses validation profile if set", - profile: "main-profile", - validationProfile: "validation-profile", + name: "validation profile takes precedence", + profile: "cudly-test-main-profile-does-not-exist", + validationProfile: "cudly-test-validation-profile-does-not-exist", + wantProfileInErr: "cudly-test-validation-profile-does-not-exist", }, { - name: "Falls back to main profile", - profile: "main-profile", - validationProfile: "", - }, - { - name: "Empty profiles use default", - profile: "", - validationProfile: "", + name: "falls back to main profile", + profile: "cudly-test-main-profile-does-not-exist", + wantProfileInErr: "cudly-test-main-profile-does-not-exist", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - toolCfg.Profile = tt.profile - toolCfg.ValidationProfile = tt.validationProfile - - // This will attempt to load config - may fail without valid profiles - _, err := queryMajorEngineVersions(ctx, toolCfg) - - // We just verify it doesn't panic - actual AWS calls may fail - // Error is acceptable for invalid profiles - if err != nil { - assert.Error(t, err) - } + t.Setenv("AWS_CONFIG_FILE", os.DevNull) + t.Setenv("AWS_SHARED_CREDENTIALS_FILE", os.DevNull) + + _, err := queryMajorEngineVersions(ctx, Config{ + Profile: tt.profile, + ValidationProfile: tt.validationProfile, + }) + + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to load AWS config") + assert.Contains(t, err.Error(), tt.wantProfileInErr, + "error must name the selected profile") }) } } diff --git a/cmd/multi_service_test.go b/cmd/multi_service_test.go index 16b52817d..d487ffe8c 100644 --- a/cmd/multi_service_test.go +++ b/cmd/multi_service_test.go @@ -2,8 +2,11 @@ package main import ( "context" + "encoding/csv" "fmt" "os" + "path/filepath" + "strconv" "testing" "time" @@ -11,6 +14,7 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" ) func TestRunToolMultiService_Validation(t *testing.T) { @@ -1591,148 +1595,223 @@ func TestFilterAndAdjustRecommendations(t *testing.T) { } } +// isolateAWSEnv pins the AWS environment to deterministic invalid static +// credentials so the runToolFromCSV tests behave identically on credentialed +// developer machines and bare CI runners: config loading succeeds, every +// outbound AWS API call fails and takes the warn-and-continue path, and no +// real account is ever touched. +func isolateAWSEnv(t *testing.T) { + t.Helper() + t.Setenv("AWS_ACCESS_KEY_ID", "invalid-test-key-id") + t.Setenv("AWS_SECRET_ACCESS_KEY", "invalid-test-secret") + t.Setenv("AWS_SESSION_TOKEN", "") + t.Setenv("AWS_PROFILE", "") + t.Setenv("AWS_CONFIG_FILE", os.DevNull) + t.Setenv("AWS_SHARED_CREDENTIALS_FILE", os.DevNull) + t.Setenv("AWS_EC2_METADATA_DISABLED", "true") +} + +// readPurchaseReport parses the CSV purchase report written by runToolFromCSV +// and returns the header plus per-purchase data rows. The trailing TOTAL +// summary row is skipped, mirroring loadRecommendationsFromCSV. +func readPurchaseReport(t *testing.T, path string) ([]string, [][]string) { + t.Helper() + f, err := os.Open(path) + require.NoError(t, err, "runToolFromCSV must write the purchase report") + defer func() { _ = f.Close() }() + rows, err := csv.NewReader(f).ReadAll() + require.NoError(t, err) + require.NotEmpty(t, rows, "purchase report must contain a header row") + dataRows := make([][]string, 0, len(rows)-1) + for _, row := range rows[1:] { + if len(row) > 0 && row[0] == "TOTAL" { + continue + } + dataRows = append(dataRows, row) + } + return rows[0], dataRows +} + +// reportColumn returns the values of the named column across all data rows. +func reportColumn(t *testing.T, header []string, rows [][]string, name string) []string { + t.Helper() + idx := -1 + for i, col := range header { + if col == name { + idx = i + break + } + } + require.NotEqual(t, -1, idx, "column %s missing from purchase report header", name) + vals := make([]string, 0, len(rows)) + for _, row := range rows { + vals = append(vals, row[idx]) + } + return vals +} + +// writeTestRecommendationsCSV writes a recommendations CSV into a temp dir +// and returns its path. +func writeTestRecommendationsCSV(t *testing.T, csvData string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "recommendations.csv") + require.NoError(t, os.WriteFile(path, []byte(csvData), 0o600)) + return path +} + func TestRunToolFromCSV(t *testing.T) { - // Save original values origCfg := toolCfg + defer func() { toolCfg = origCfg }() + isolateAWSEnv(t) - defer func() { - toolCfg = origCfg - }() - - // Create a temporary CSV file for testing - tmpFile, err := os.CreateTemp("", "test_recommendations_*.csv") - assert.NoError(t, err) - defer func() { _ = os.Remove(tmpFile.Name()) }() - - // Write test CSV data - csvData := `Service,Region,Engine,Instance Type,Payment Option,Term (months),Instance Count,Account ID -rds,us-east-1,postgres,db.t3.small,All Upfront,12,2,123456789012 -elasticache,us-west-2,redis,cache.t3.micro,All Upfront,12,1,123456789012 -` - _, err = tmpFile.WriteString(csvData) - assert.NoError(t, err) - _ = tmpFile.Close() + // Column names match writeMultiServiceCSVReport's header (the real + // round-trip shape). The previous fixture used headers the parser + // silently ignored, so every row loaded with Count=0 and the whole run + // processed nothing while the NotPanics assertion stayed green. + csvPath := writeTestRecommendationsCSV(t, `Service,Region,ResourceType,Engine,Count,Term,PaymentOption,Account +rds,us-east-1,db.t3.small,postgres,2,1yr,All Upfront,123456789012 +elasticache,us-west-2,cache.t3.micro,redis,1,1yr,All Upfront,123456789012 +`) tests := []struct { - setupConfig func() - validateFunc func(t *testing.T) - name string - expectPanic bool + name string + coverage float64 + wantResourceTypes []string + wantCounts []string }{ { - name: "Dry run mode", - setupConfig: func() { - toolCfg.CSVInput = tmpFile.Name() - toolCfg.ActualPurchase = false - toolCfg.Coverage = 100.0 - toolCfg.MaxInstances = 0 - }, - expectPanic: false, + // 100% coverage keeps both recommendations at their CSV counts. + name: "Dry run writes a report row per recommendation", + coverage: 100.0, + wantResourceTypes: []string{"db.t3.small", "cache.t3.micro"}, + wantCounts: []string{"2", "1"}, }, { - name: "With coverage adjustment", - setupConfig: func() { - toolCfg.CSVInput = tmpFile.Name() - toolCfg.ActualPurchase = false - toolCfg.Coverage = 50.0 - toolCfg.MaxInstances = 0 - }, - expectPanic: false, + // 50% coverage halves db.t3.small (2 -> 1) and drops + // cache.t3.micro (1 -> 0). + name: "Coverage sizing is applied before the purchase loop", + coverage: 50.0, + wantResourceTypes: []string{"db.t3.small"}, + wantCounts: []string{"1"}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - tt.setupConfig() - - // Suppress logger - // Logger output disabled for testing - - ctx := context.Background() - - if tt.expectPanic { - assert.Panics(t, func() { - runToolFromCSV(ctx, toolCfg) - }) - } else { - // Just verify it doesn't panic - actual purchase testing requires AWS mocks - assert.NotPanics(t, func() { - runToolFromCSV(ctx, toolCfg) - }) + reportPath := filepath.Join(t.TempDir(), "report.csv") + toolCfg.CSVInput = csvPath + toolCfg.CSVOutput = reportPath + toolCfg.ActualPurchase = false + toolCfg.Coverage = tt.coverage + toolCfg.TargetCoverage = 0 + toolCfg.MaxInstances = 0 + toolCfg.OverrideCount = 0 + + err := runToolFromCSV(context.Background(), toolCfg) + require.NoError(t, err) + + header, rows := readPurchaseReport(t, reportPath) + assert.ElementsMatch(t, tt.wantResourceTypes, reportColumn(t, header, rows, "ResourceType")) + assert.ElementsMatch(t, tt.wantCounts, reportColumn(t, header, rows, "Count")) + for _, success := range reportColumn(t, header, rows, "Success") { + assert.Equal(t, "true", success, "dry-run purchases must be recorded as successful") } }) } } +// TestRunToolFromCSV_NonExistentFile asserts the unreadable-input error path +// surfaces as an error (previously log.Fatalf, which made it untestable). func TestRunToolFromCSV_NonExistentFile(t *testing.T) { - // This test is skipped because runToolFromCSV calls log.Fatalf on file errors, - // which causes os.Exit and cannot be caught in tests. - // The error handling path is exercised in integration tests. - t.Skip("Skipping test that calls log.Fatalf - cannot be tested in unit tests") + origCfg := toolCfg + defer func() { toolCfg = origCfg }() + + toolCfg.CSVInput = filepath.Join(t.TempDir(), "does-not-exist.csv") + toolCfg.ActualPurchase = false + + err := runToolFromCSV(context.Background(), toolCfg) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to read CSV file") + assert.Contains(t, err.Error(), "failed to open CSV file") } +// TestRunToolFromCSV_EmptyFile asserts a CSV without a header row is rejected +// with a parse error rather than being silently treated as zero rows. func TestRunToolFromCSV_EmptyFile(t *testing.T) { - // This test is skipped because runToolFromCSV calls log.Fatalf on CSV parsing errors, - // which causes os.Exit and cannot be caught in tests. - t.Skip("Skipping test that calls log.Fatalf - cannot be tested in unit tests") + origCfg := toolCfg + defer func() { toolCfg = origCfg }() + + toolCfg.CSVInput = writeTestRecommendationsCSV(t, "") + toolCfg.ActualPurchase = false + + err := runToolFromCSV(context.Background(), toolCfg) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to read CSV file") + assert.Contains(t, err.Error(), "failed to read CSV header") } func TestRunToolFromCSV_WithMaxInstances(t *testing.T) { origCfg := toolCfg defer func() { toolCfg = origCfg }() + isolateAWSEnv(t) - // Create a CSV file with multiple recommendations - tmpFile, err := os.CreateTemp("", "test_max_instances_*.csv") - assert.NoError(t, err) - defer os.Remove(tmpFile.Name()) - - csvData := `Service,Region,Engine,Instance Type,Payment Option,Term (months),Instance Count,Account ID -rds,us-east-1,postgres,db.t3.small,All Upfront,12,10,123456789012 -rds,us-east-1,mysql,db.t3.medium,All Upfront,12,10,123456789012 -rds,us-east-1,postgres,db.t3.large,All Upfront,12,10,123456789012 -` - _, err = tmpFile.WriteString(csvData) - assert.NoError(t, err) - tmpFile.Close() + csvPath := writeTestRecommendationsCSV(t, `Service,Region,ResourceType,Engine,Count,Term,PaymentOption,Account +rds,us-east-1,db.t3.small,postgres,10,1yr,All Upfront,123456789012 +rds,us-east-1,db.t3.medium,mysql,10,1yr,All Upfront,123456789012 +rds,us-east-1,db.t3.large,postgres,10,1yr,All Upfront,123456789012 +`) - toolCfg.CSVInput = tmpFile.Name() + reportPath := filepath.Join(t.TempDir(), "report.csv") + toolCfg.CSVInput = csvPath + toolCfg.CSVOutput = reportPath toolCfg.ActualPurchase = false toolCfg.Coverage = 100.0 + toolCfg.TargetCoverage = 0 + toolCfg.OverrideCount = 0 toolCfg.MaxInstances = 15 // Should limit total instances to 15 - ctx := context.Background() + err := runToolFromCSV(context.Background(), toolCfg) + require.NoError(t, err) - assert.NotPanics(t, func() { - runToolFromCSV(ctx, toolCfg) - }) + header, rows := readPurchaseReport(t, reportPath) + total := 0 + for _, raw := range reportColumn(t, header, rows, "Count") { + n, convErr := strconv.Atoi(raw) + require.NoError(t, convErr) + total += n + } + assert.Positive(t, total, "some instances must survive the max-instances cap") + assert.LessOrEqual(t, total, 15, "purchased instance total must respect --max-instances") } func TestRunToolFromCSV_WithOverrideCount(t *testing.T) { origCfg := toolCfg defer func() { toolCfg = origCfg }() + isolateAWSEnv(t) - tmpFile, err := os.CreateTemp("", "test_override_*.csv") - assert.NoError(t, err) - defer os.Remove(tmpFile.Name()) + csvPath := writeTestRecommendationsCSV(t, `Service,Region,ResourceType,Engine,Count,Term,PaymentOption,Account +rds,us-east-1,db.t3.small,postgres,10,1yr,All Upfront,123456789012 +rds,us-east-1,db.t3.medium,mysql,5,1yr,All Upfront,123456789012 +`) - csvData := `Service,Region,Engine,Instance Type,Payment Option,Term (months),Instance Count,Account ID -rds,us-east-1,postgres,db.t3.small,All Upfront,12,10,123456789012 -rds,us-east-1,mysql,db.t3.medium,All Upfront,12,5,123456789012 -` - _, err = tmpFile.WriteString(csvData) - assert.NoError(t, err) - tmpFile.Close() - - toolCfg.CSVInput = tmpFile.Name() + reportPath := filepath.Join(t.TempDir(), "report.csv") + toolCfg.CSVInput = csvPath + toolCfg.CSVOutput = reportPath toolCfg.ActualPurchase = false toolCfg.Coverage = 100.0 + toolCfg.TargetCoverage = 0 + toolCfg.MaxInstances = 0 toolCfg.OverrideCount = 3 // Override each recommendation to count=3 - ctx := context.Background() + err := runToolFromCSV(context.Background(), toolCfg) + require.NoError(t, err) - assert.NotPanics(t, func() { - runToolFromCSV(ctx, toolCfg) - }) + header, rows := readPurchaseReport(t, reportPath) + counts := reportColumn(t, header, rows, "Count") + require.Len(t, counts, 2, "both recommendations must reach the purchase loop") + for _, raw := range counts { + assert.Equal(t, "3", raw, "--override-count must replace every recommendation count") + } } // ==================== Tests for adjustRecommendationForExcludedVersions ==================== From 12e80c5cace591fd9da2a02fc8cbc21a4a655ed4 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 10 Jul 2026 16:05:54 +0200 Subject: [PATCH 2/2] fix(cmd): keep runToolMultiService under the gocyclo budget Making runToolFromCSV return an error (so the CSV purchase path is unit-testable) added an `if err != nil { log.Fatalf }` branch to runToolMultiService, pushing its cyclomatic complexity from 10 to 11 and tripping the gocyclo pre-commit hook (over 10). Extract the error-to-fatal glue into runCSVPathOrFatal so the entrypoint drops back to complexity 10 while runToolFromCSV keeps its testable error return. --- cmd/multi_service.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/cmd/multi_service.go b/cmd/multi_service.go index 7bde3caf3..77c22c0aa 100644 --- a/cmd/multi_service.go +++ b/cmd/multi_service.go @@ -83,9 +83,7 @@ func effectiveDryRun(cfg Config) bool { // (2) score, display, confirm, and purchase. func runToolMultiService(ctx context.Context, cfg Config) { if cfg.CSVInput != "" { - if err := runToolFromCSV(ctx, cfg); err != nil { - log.Fatalf("%v", err) - } + runCSVPathOrFatal(ctx, cfg) return } @@ -274,9 +272,19 @@ func buildServiceStats(recs []common.Recommendation, results []common.PurchaseRe return stats } +// runCSVPathOrFatal runs the CSV purchase path and exits fatally on error. +// It isolates the error-to-fatal glue from runToolMultiService so that path +// stays under the cyclomatic-complexity budget while runToolFromCSV remains +// unit-testable via its returned error. +func runCSVPathOrFatal(ctx context.Context, cfg Config) { + if err := runToolFromCSV(ctx, cfg); err != nil { + log.Fatalf("%v", err) + } +} + // runToolFromCSV processes recommendations from a CSV input file. // It returns an error instead of exiting so the orchestration glue is -// unit-testable; the caller (runToolMultiService) turns errors fatal. +// unit-testable; the caller (runCSVPathOrFatal) turns errors fatal. func runToolFromCSV(ctx context.Context, cfg Config) error { isDryRun := effectiveDryRun(cfg) printRunMode(isDryRun)