fix(cli): use purchaseSucceeded in the interrupt summary and run stats

CHANGELOG.md

@@ -13,6 +13,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
 
 ### Fixed
 
+- A purchase result with `Success=true` and a non-nil `Error` is now treated as
+  a failure in the Ctrl-C summary, the audit-stop message, the per-service
+  success/failed counts and the non-zero exit. The CSV `Success` column keeps the
+  raw provider flag; read the `Error` column alongside it (#2205).
 - `configure gcp`: pressing Enter at a `[r]un, [S]kip (default)?` step now skips
   instead of creating the service account, granting roles, minting a key or
   running `gcloud auth login`; type `r` to run a step (#1613).

cmd/interrupt.go

@@ -40,7 +40,7 @@ func printInterruptedSummary(attempted, total int, results []common.PurchaseResu
 	}
 	for i := range results {
 		r := results[i]
-		if r.Success {
+		if purchaseSucceeded(r) {
 			continue
 		}
 		AppLogger.Printf("   %s\n", unverifiedPurchaseLine(r))
@@ -62,7 +62,7 @@ func auditStopMessage(attempted, total int, results []common.PurchaseResult, cau
 	if n := len(results); n > 0 {
 		r := results[n-1]
 		last = fmt.Sprintf("last attempted: %s %s", r.Recommendation.Service, r.Recommendation.ResourceType)
-		if r.Success {
+		if purchaseSucceeded(r) {
 			last += " (commitment " + r.CommitmentID + ")"
 		} else {
 			last += " (errored: " + fmt.Sprint(r.Error) + ")"

cmd/multi_service_stats.go

@@ -20,7 +20,7 @@ func purchaseFailureError(results []common.PurchaseResult, isDryRun bool) error
 	}
 	failed := 0
 	for i := range results {
-		if !results[i].Success {
+		if !purchaseSucceeded(results[i]) {
 			failed++
 		}
 	}
@@ -61,7 +61,7 @@ func calculateServiceStats(service common.ServiceType, recs []common.Recommendat
 
 	for _rvc := range results {
 		result := results[_rvc]
-		if result.Success {
+		if purchaseSucceeded(result) {
 			stats.SuccessfulPurchases++
 		} else {
 			stats.FailedPurchases++

cmd/purchase_succeeded_summaries_2205_test.go

@@ -0,0 +1,76 @@
+package main
+
+import (
+	"errors"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
+)
+
+func succeededWithError() common.PurchaseResult {
+	return common.PurchaseResult{
+		Success:      true,
+		Error:        errors.New("x"),
+		CommitmentID: "id-1",
+		Recommendation: common.Recommendation{
+			Service:      common.ServiceEC2,
+			ResourceType: "m5.large",
+		},
+	}
+}
+
+func TestPrintInterruptedSummary_SuccessWithErrorIsListed(t *testing.T) {
+	ok := common.PurchaseResult{Success: true}
+	bad := common.PurchaseResult{Success: false, Error: errors.New("boom")}
+	tests := []struct {
+		name    string
+		results []common.PurchaseResult
+		want    bool
+	}{
+		{"success true with error", []common.PurchaseResult{succeededWithError()}, true},
+		{"ordinary success", []common.PurchaseResult{ok}, false},
+		{"success false with error", []common.PurchaseResult{bad}, true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			out := captureAppOutput(t, func() { printInterruptedSummary(1, 2, tt.results, false) })
+			assert.Equal(t, tt.want, containsVerify(out), out)
+		})
+	}
+}
+
+func containsVerify(s string) bool {
+	return strings.Contains(s, "verify in AWS before re-running")
+}
+
+func TestAuditStopMessage_SuccessWithErrorReportsError(t *testing.T) {
+	msg := auditStopMessage(1, 2, []common.PurchaseResult{succeededWithError()}, errors.New("cause"))
+	assert.Contains(t, msg, "(errored: x)")
+	assert.NotContains(t, msg, "(commitment")
+
+	ok := common.PurchaseResult{Success: true, CommitmentID: "id-2"}
+	assert.Contains(t, auditStopMessage(1, 2, []common.PurchaseResult{ok}, errors.New("c")), "(commitment id-2)")
+	bad := common.PurchaseResult{Success: false, Error: errors.New("y")}
+	assert.Contains(t, auditStopMessage(1, 2, []common.PurchaseResult{bad}, errors.New("c")), "(errored: y)")
+}
+
+func TestCalculateServiceStats_SuccessWithErrorCountsFailed(t *testing.T) {
+	results := []common.PurchaseResult{
+		{Success: true},
+		succeededWithError(),
+		{Success: false},
+		{Success: false, Error: errors.New("z")},
+	}
+	stats := calculateServiceStats(common.ServiceEC2, nil, results)
+	assert.Equal(t, 1, stats.SuccessfulPurchases)
+	assert.Equal(t, 3, stats.FailedPurchases)
+}
+
+func TestPurchaseFailureError_SuccessWithErrorIsFailure(t *testing.T) {
+	err := purchaseFailureError([]common.PurchaseResult{{Success: true}, succeededWithError()}, false)
+	assert.ErrorIs(t, err, errPurchasesFailed)
+	assert.Contains(t, err.Error(), "1 of 2")
+}

docs/cli/README.md

@@ -87,7 +87,7 @@ All flags belong to the root command unless noted otherwise.
 
 | Flag | Short | Default | Description |
 |------|-------|---------|-------------|
-| `--output` | `-o` | auto-generated | Output CSV file path. Auto-name format: `ri-helper-dryrun-YYYYMMDD-HHMMSS.csv` or `ri-helper-purchase-YYYYMMDD-HHMMSS.csv`. An explicit path that already exists (file, directory or symlink) is refused instead of overwritten; choose a new path or remove the old file. |
+| `--output` | `-o` | auto-generated | Output CSV file path. Auto-name format: `ri-helper-dryrun-YYYYMMDD-HHMMSS.csv` or `ri-helper-purchase-YYYYMMDD-HHMMSS.csv`. An explicit path that already exists (file, directory or symlink) is refused instead of overwritten; choose a new path or remove the old file. The `Success` column is the raw flag the provider returned; a provider can report `true` together with an error, so read the `Error` column too (a row with an error counts as failed in the run summary). |
 | `--input-csv` | `-i` | (none) | Read recommendations from an existing CSV instead of fetching from cloud APIs. The file must have a `.csv` extension and exist at the path given. The header must contain `Service`, `Region`, `ResourceType`, `Count`, `Term` and `PaymentOption`; a missing column is a load error that names it. `Term` must be `1yr` or `3yr` and `PaymentOption` must be `all-upfront`, `partial-upfront` or `no-upfront` on every row; a blank or unknown value fails the load with the row number. CSVs with only the older four columns are rejected. |
 
 ### Authentication