fix(cmd): repair red main after #2238 and drop the RI hint on SP-only runs by IAmJSD · Pull Request #2243 · LeanerCloud/cloud-commitments-cli · GitHub
Skip to content

fix(cmd): repair red main after #2238 and drop the RI hint on SP-only runs - #2243

Merged
cristim merged 2 commits into
LeanerCloud:mainfrom
IAmJSD:fix/2238-followup
Oct 16, 2026
Merged

cristim merged 2 commits into
LeanerCloud:mainfrom
IAmJSD:fix/2238-followup

Conversation

@IAmJSD

@IAmJSD IAmJSD commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #2242. Follow-up to #2238 (issue #2157). There are two commits on main (4ed7248).

1. Fix red main (test(csv))

#2238 made a real run with any failed purchase return errPurchasesFailed, so the CLI exits non-zero. #2230 merged at about the same time, and its TestCSVRealPurchase_FailedPurchaseCarriesError still did require.NoError(runCSVPurchases(...)) while forcing a failed ElastiCache purchase. So go test ./cmd/ fails on main.

The shared csvPurchaseEnv gets an expectPurchaseFailure flag. When it's set, run requires errors.Is(err, errPurchasesFailed); otherwise it still requires no error. The failed-purchase test sets it. This is test-only, and the per-row report and audit assertions are unchanged.

2. CodeRabbit finding on #2238 (fix(cmd))

This commit is 9c24083, cherry-picked unchanged. It was pushed to the #2238 branch after that PR merged.

printFinalMessage(isDryRun, riSuccess, success, failed) prints "Allow up to 15 minutes for RIs to appear" only when an RI purchase succeeded. The completion line still prints for any success.

TestPrintMultiServiceSummary_RIHintOnlyAfterRISuccess covers five cases:

  • Savings Plans only: no hint.
  • A Savings Plan success plus a failed RI: no hint.
  • RIs only: hint.
  • RIs and Savings Plans: hint.
  • A dry run: unchanged.

Before the fix, the two Savings Plan cases failed. Three mutation probes were all caught.

Verification

Tests ran on macOS with AWS config set to /dev/null, using mocks and the offline HTTP fake. Nothing called real AWS, and nothing ran with --yes.

  • Before: on main (4ed7248), go test ./cmd/ -count=1 fails with TestCSVRealPurchase_FailedPurchaseCarriesError: "Received unexpected error".
  • After: on this branch, go test ./cmd/ -count=1 passes.
  • go build ./cmd passes, go vet ./... is clean, and golangci-lint v2.10.1 reports 0 issues.

Maintainer: please copy the labels from #2157.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Reserved Instance guidance now appears only after a successful Reserved Instance purchase. Savings Plan-only results and failed Reserved Instance purchases no longer display the hint.
    • Purchase summaries continue to reflect combined Reserved Instance and Savings Plan results. Dry-run summaries show purchase guidance instead of a purchase-completion message.
  • Tests
    • Added coverage for purchase-failure handling and the conditions that control Reserved Instance guidance.

IAmJSD and others added 2 commits October 16, 2026 18:43
…eeds

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 <noreply@anthropic.com>
…rchase

LeanerCloud#2238 made a real purchase run with any failure return errPurchasesFailed
so the CLI exits non-zero; LeanerCloud#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 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

@cristim cristim added type/bug Defect priority/p1 Next up; this sprint severity/medium Moderate harm urgency/now Drop other things impact/internal Team-internal only effort/xs Trivial / one-liner triaged Item has been triaged labels Oct 16, 2026
@cristim

cristim commented Oct 16, 2026

Copy link
Copy Markdown
Member

@cristim
cristim merged commit 96ecab1 into LeanerCloud:main Oct 16, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/xs Trivial / one-liner impact/internal Team-internal only priority/p1 Next up; this sprint severity/medium Moderate harm triaged Item has been triaged type/bug Defect urgency/now Drop other things

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test: main fails TestCSVRealPurchase_FailedPurchaseCarriesError after #2238 + #2230; and SP-only runs print the RI hint

2 participants