diff --git a/cmd/csv_real_purchase_test.go b/cmd/csv_real_purchase_test.go index 3211c4f5f..a3deb8aae 100644 --- a/cmd/csv_real_purchase_test.go +++ b/cmd/csv_real_purchase_test.go @@ -100,6 +100,9 @@ type csvPurchaseEnv struct { denyHits *atomic.Int64 dir string run func(report string) // buys the CSV rows against the stub, report written to the given name + // expectPurchaseFailure makes run require errPurchasesFailed (#2157): + // a real run with any failed purchase now returns it so the CLI exits non-zero. + expectPurchaseFailure bool } // newCSVPurchaseEnv wires the stub with the RDS and ElastiCache offering and @@ -132,7 +135,12 @@ func newCSVPurchaseEnvFor(t *testing.T, csv string, fixtureSets ...map[string]aw recs, err := loadRecommendationsFromCSV(csvPath) require.NoError(t, err) cfg := Config{CallerAccountID: defaultTestAccount, CSVOutput: filepath.Join(dir, report), AuditLog: filepath.Join(dir, "audit.jsonl")} - require.NoError(t, runCSVPurchases(context.Background(), awsCfg, recs, false, cfg, "run-"+report)) + err = runCSVPurchases(context.Background(), awsCfg, recs, false, cfg, "run-"+report) + if env.expectPurchaseFailure { + require.ErrorIs(t, err, errPurchasesFailed) + return + } + require.NoError(t, err) } return env } @@ -218,6 +226,7 @@ func TestCSVRealPurchase_RDSAndElastiCache(t *testing.T) { func TestCSVRealPurchase_FailedPurchaseCarriesError(t *testing.T) { env := newCSVPurchaseEnv(t) env.stub.fixtures["elasticache "+cachePurch] = queryErr("ReservedCacheNodeQuotaExceeded", "cache quota exceeded by fixture") + env.expectPurchaseFailure = true env.run("report1.csv") rep := env.report(t, "report1.csv") diff --git a/cmd/multi_service_stats.go b/cmd/multi_service_stats.go index 33bad3330..c53d8ced4 100644 --- a/cmd/multi_service_stats.go +++ b/cmd/multi_service_stats.go @@ -103,7 +103,7 @@ func printMultiServiceSummary(allRecommendations []common.Recommendation, allRes success := riAggregates.success + spStats.SuccessfulPurchases failed := riAggregates.failed + spStats.FailedPurchases printSuccessRate(success, failed) - printFinalMessage(isDryRun, success, failed) + printFinalMessage(isDryRun, riAggregates.success, success, failed) } // riAggregateStats holds aggregated RI statistics. @@ -208,8 +208,9 @@ func printSuccessRate(success, failed int) { const archeraSignupURL = common.ArcheraSignupURL // printFinalMessage prints the final message based on mode and results. -// success and failed cover both Reserved Instances and Savings Plans. -func printFinalMessage(isDryRun bool, success, failed int) { +// success and failed cover both Reserved Instances and Savings Plans; +// riSuccess (the RI share of success) gates the RI-propagation hint. +func printFinalMessage(isDryRun bool, riSuccess, success, failed int) { if isDryRun { AppLogger.Println("\nšŸ’” To actually purchase these commitments, run with --purchase flag") return @@ -219,7 +220,9 @@ func printFinalMessage(isDryRun bool, success, failed int) { } if success > 0 { AppLogger.Println("\nšŸŽ‰ Purchase operations completed!") - AppLogger.Println("ā° Allow up to 15 minutes for RIs to appear in your account") + if riSuccess > 0 { + AppLogger.Println("ā° Allow up to 15 minutes for RIs to appear in your account") + } printArcheraPitch() } } diff --git a/cmd/multi_service_stats_test.go b/cmd/multi_service_stats_test.go index 38f735243..004f192da 100644 --- a/cmd/multi_service_stats_test.go +++ b/cmd/multi_service_stats_test.go @@ -559,7 +559,7 @@ func TestPrintFinalMessage(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { output := captureAppOutput(t, func() { - printFinalMessage(tt.isDryRun, tt.riSuccess, 0) + printFinalMessage(tt.isDryRun, tt.riSuccess, tt.riSuccess, 0) }) for _, want := range tt.wantOutput { diff --git a/cmd/purchase_failure_exit_2157_test.go b/cmd/purchase_failure_exit_2157_test.go index ab39cf37b..54c147a55 100644 --- a/cmd/purchase_failure_exit_2157_test.go +++ b/cmd/purchase_failure_exit_2157_test.go @@ -174,3 +174,42 @@ func TestPurchaseFailureError(t *testing.T) { require.ErrorIs(t, err, errPurchasesFailed) assert.Contains(t, err.Error(), "2 of 3") } + +// riHint is the RI-propagation hint; it applies only when an RI was bought. +const riHint = "Allow up to 15 minutes for RIs to appear" + +func TestPrintMultiServiceSummary_RIHintOnlyAfterRISuccess(t *testing.T) { + ri := ServiceProcessingStats{Service: common.ServiceEC2, RecommendationsSelected: 1, SuccessfulPurchases: 1} + riFailed := ServiceProcessingStats{Service: common.ServiceEC2, RecommendationsSelected: 1, FailedPurchases: 1} + sp := ServiceProcessingStats{Service: common.ServiceSavingsPlansCompute, RecommendationsSelected: 1, SuccessfulPurchases: 1} + tests := []struct { + name string + stats map[common.ServiceType]ServiceProcessingStats + isDryRun bool + wantHint bool + }{ + {"savings plan only", map[common.ServiceType]ServiceProcessingStats{common.ServiceSavingsPlansCompute: sp}, false, false}, + {"savings plan success with failed RI", map[common.ServiceType]ServiceProcessingStats{common.ServiceSavingsPlansCompute: sp, common.ServiceEC2: riFailed}, false, false}, + {"RI only", map[common.ServiceType]ServiceProcessingStats{common.ServiceEC2: ri}, false, true}, + {"RI and savings plan", map[common.ServiceType]ServiceProcessingStats{common.ServiceSavingsPlansCompute: sp, common.ServiceEC2: ri}, false, true}, + {"dry run", map[common.ServiceType]ServiceProcessingStats{common.ServiceEC2: ri}, true, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + output := captureAppOutput(t, func() { + printMultiServiceSummary(nil, nil, tt.stats, tt.isDryRun) + }) + if tt.wantHint { + assert.Contains(t, output, riHint) + } else { + assert.NotContains(t, output, riHint) + } + if tt.isDryRun { + assert.Contains(t, output, "run with --purchase flag") + assert.NotContains(t, output, "Purchase operations completed") + } else { + assert.Contains(t, output, "Purchase operations completed") + } + }) + } +}