From abc99d36408941662a80391f5e0bd9928b2472ac Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Fri, 16 Oct 2026 18:42:25 +0100 Subject: [PATCH 1/2] fix(cmd): show the RI propagation hint only after an RI purchase succeeds A run where only Savings Plans succeeded still printed "Allow up to 15 minutes for RIs to appear in your account". printFinalMessage now takes the RI success count and prints the hint only when an RI was bought. Co-Authored-By: Claude Opus 5.5 --- cmd/multi_service_stats.go | 11 +++++--- cmd/multi_service_stats_test.go | 2 +- cmd/purchase_failure_exit_2157_test.go | 39 ++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 5 deletions(-) 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") + } + }) + } +} From c36842afd180988f9262bc2ca082680a98ccdaa3 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Fri, 16 Oct 2026 18:47:47 +0100 Subject: [PATCH 2/2] test(csv): expect errPurchasesFailed from a real run with a failed purchase #2238 made a real purchase run with any failure return errPurchasesFailed so the CLI exits non-zero; #2230's TestCSVRealPurchase_FailedPurchaseCarriesError (merged alongside it) still required runCSVPurchases to return nil, so main fails. The shared env helper now requires the sentinel when the test expects a failed purchase. Co-Authored-By: Claude Opus 5.5 --- cmd/csv_real_purchase_test.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) 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")