From 93ad1be25e277fd555ccb57cebe4e88777278dd2 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Tue, 27 Oct 2026 23:54:42 +0000 Subject: [PATCH 1/2] test(purchase): assert mock expectations across the purchase tests Every internal/purchase test that registers a required .On(...) now fails when that call never happens. A shared assertMocksOnCleanup(t, mocks...) helper registers AssertExpectations as a t.Cleanup at mock construction (or in the shared harness: newApproveManager, armedHarness, newDetachFixture, attachManager callers, tokenCapture, runFanOutCapturingOpts and others). Expectations that turned out never to fire were stubs for a path the test does not take, not missed production calls. They are now .Maybe() with a comment, or made conditional on the case that takes the path: - newPinManager: per-path entry reads (cron lists vs SQS GetExecutionByID) - gate tests: the kill-switch read for a missing plan (refused first) - traps the test asserts NotCalled (plan lookup for "", the running CAS, the failed-email for non-failed statuses, non-alerting writers) - shared stubs on paths that never reach them (optionalCalls helper) No production code changes. Refs #35 Co-Authored-By: Claude Opus 5.5 --- internal/purchase/approvals_test.go | 3 ++ internal/purchase/armed_redrive_test.go | 1 + .../purchase/aws_no_token_redrive_test.go | 1 + .../azure_savings_plans_alias_proof_test.go | 1 + internal/purchase/claim_guard_test.go | 8 +++++ internal/purchase/claim_pin_test.go | 18 +++++++++-- internal/purchase/claimed_row_check_test.go | 2 ++ internal/purchase/coverage_extra_test.go | 16 ++++++++++ .../purchase/empty_selection_notify_test.go | 1 + internal/purchase/execute_detached_test.go | 4 +++ internal/purchase/execution_test.go | 12 +++++++ .../purchase/failure_notify_paths_test.go | 11 +++++-- internal/purchase/failure_notify_test.go | 14 ++++++-- .../purchase/failure_notify_writers_test.go | 6 +++- .../purchase/finalize_revocations_test.go | 3 ++ internal/purchase/gcp_history_cost_test.go | 1 + .../purchase/ladder_execution_recheck_test.go | 1 + internal/purchase/manager_test.go | 2 ++ internal/purchase/mocks_test.go | 32 +++++++++++++++++++ .../notified_claim_pin_integration_test.go | 1 + internal/purchase/notified_run_now_test.go | 5 +++ internal/purchase/partition_guard_test.go | 1 + internal/purchase/plan_step_atomic_test.go | 7 ++++ .../plan_step_attach_integration_test.go | 1 + internal/purchase/plan_step_attach_test.go | 15 +++++++-- internal/purchase/plan_step_proofs_test.go | 1 + internal/purchase/reaper_ordering_test.go | 6 +++- internal/purchase/reaper_test.go | 1 + internal/purchase/redrive_claimed_row_test.go | 1 + internal/purchase/requested_at_test.go | 2 ++ internal/purchase/scheduled_fire_test.go | 2 ++ internal/purchase/step_cap_test.go | 26 +++++++++++++-- internal/purchase/zonal_az_test.go | 8 +++-- 33 files changed, 196 insertions(+), 18 deletions(-) diff --git a/internal/purchase/approvals_test.go b/internal/purchase/approvals_test.go index c59f45359..968f62cb3 100644 --- a/internal/purchase/approvals_test.go +++ b/internal/purchase/approvals_test.go @@ -33,6 +33,7 @@ func newApproveManager(t *testing.T) (*Manager, *MockConfigStore, *MockEmailSend prov := new(MockProvider) svc := new(MockServiceClient) factory := new(MockProviderFactory) + assertMocksOnCleanup(t, store, sender, prov, svc, factory) factory.On("CreateAndValidateProvider", mock.Anything, "aws", mock.Anything).Return(prov, nil).Maybe() prov.On("GetServiceClient", mock.Anything, common.ServiceEC2, "us-east-1").Return(svc, nil).Maybe() svc.On("PurchaseCommitment", mock.Anything, mock.Anything, mock.Anything). @@ -454,6 +455,7 @@ func TestManager_ApproveAndExecute_ReturnsRowOnExecutionFailure(t *testing.T) { ctx := context.Background() store := new(MockConfigStore) factory := new(MockProviderFactory) + assertMocksOnCleanup(t, store, factory) factory.On("CreateAndValidateProvider", mock.Anything, "aws", mock.Anything).Return(nil, errors.New("credentials rejected")) manager := &Manager{config: store, email: allowFailedNotification(new(MockEmailSender)), providerFactory: factory} @@ -479,6 +481,7 @@ func TestManager_ApproveExecution_ReturnsRowOnExecutionFailure(t *testing.T) { ctx := context.Background() store := new(MockConfigStore) factory := new(MockProviderFactory) + assertMocksOnCleanup(t, store, factory) factory.On("CreateAndValidateProvider", mock.Anything, "aws", mock.Anything).Return(nil, errors.New("credentials rejected")) manager := &Manager{config: store, email: allowFailedNotification(new(MockEmailSender)), providerFactory: factory} diff --git a/internal/purchase/armed_redrive_test.go b/internal/purchase/armed_redrive_test.go index 9d88c8e4e..fe123f936 100644 --- a/internal/purchase/armed_redrive_test.go +++ b/internal/purchase/armed_redrive_test.go @@ -94,6 +94,7 @@ func armedHarness(t *testing.T, serviceType common.ServiceType) (*Manager, *Mock factory := new(MockProviderFactory) prov := new(MockProvider) svc := new(MockServiceClient) + assertMocksOnCleanup(t, store, email, factory, prov, svc) // Fn overrides rather than testify expectations: these must not double as // assertions, because whether the execution path reaches them at all is diff --git a/internal/purchase/aws_no_token_redrive_test.go b/internal/purchase/aws_no_token_redrive_test.go index 3b754a113..f988db20d 100644 --- a/internal/purchase/aws_no_token_redrive_test.go +++ b/internal/purchase/aws_no_token_redrive_test.go @@ -130,6 +130,7 @@ func TestAWSNoTokenRetrySuccessorIsRefusedAndFirstBuyStillExecutes(t *testing.T) func TestRecoverStrandedApprovals_EC2RowIsFailedNotRedriven(t *testing.T) { ctx := context.Background() mockStore := new(MockConfigStore) + assertMocksOnCleanup(t, mockStore) mockFactory := new(MockProviderFactory) // any CreateAndValidateProvider call would panic: no expectation set stranded := config.PurchaseExecution{ diff --git a/internal/purchase/azure_savings_plans_alias_proof_test.go b/internal/purchase/azure_savings_plans_alias_proof_test.go index 68150e229..41fb45609 100644 --- a/internal/purchase/azure_savings_plans_alias_proof_test.go +++ b/internal/purchase/azure_savings_plans_alias_proof_test.go @@ -97,6 +97,7 @@ func purchaseThroughPinnedAzureClient(t *testing.T, exec *config.PurchaseExecuti client.SetOrderAliasClient(recorder) store, sender, factory, prov := new(MockConfigStore), new(MockEmailSender), new(MockProviderFactory), new(MockProvider) + assertMocksOnCleanup(t, store, sender, factory, prov) store.SavePurchaseExecutionFn = func(context.Context, *config.PurchaseExecution) error { return nil } store.On("SavePurchaseHistory", ctx, mock.Anything).Return(nil) factory.On("CreateAndValidateProvider", mock.Anything, "azure", mock.Anything).Return(prov, nil) diff --git a/internal/purchase/claim_guard_test.go b/internal/purchase/claim_guard_test.go index 27354b261..42d272089 100644 --- a/internal/purchase/claim_guard_test.go +++ b/internal/purchase/claim_guard_test.go @@ -30,6 +30,7 @@ func purchasedExec(status string) *config.PurchaseExecution { func TestSaveOrRecoverTerminal_GuardsOnRunning(t *testing.T) { ctx := context.Background() store := new(MockConfigStore) + assertMocksOnCleanup(t, store) m := &Manager{config: store} exec := purchasedExec("completed") store.On("SaveExecutionIfStatus", ctx, exec, []string{"running"}).Return(nil).Once() @@ -43,6 +44,7 @@ func TestSaveOrRecoverTerminal_GuardsOnRunning(t *testing.T) { func TestSaveOrRecoverTerminal_ReapedButPurchasedIsRecovered(t *testing.T) { ctx := context.Background() store := new(MockConfigStore) + assertMocksOnCleanup(t, store) m := &Manager{config: store} exec := purchasedExec("partially_completed") store.On("SaveExecutionIfStatus", ctx, exec, []string{"running"}).Return(lostGuard()) @@ -56,6 +58,7 @@ func TestSaveOrRecoverTerminal_ReapedButPurchasedIsRecovered(t *testing.T) { func TestSaveOrRecoverTerminal_RetryLinkedFailsLoudWithDoubleBuyFlag(t *testing.T) { ctx := context.Background() store := new(MockConfigStore) + assertMocksOnCleanup(t, store) m := &Manager{config: store} exec := purchasedExec("completed") store.On("SaveExecutionIfStatus", ctx, exec, []string{"running"}).Return(lostGuard()) @@ -69,6 +72,7 @@ func TestSaveOrRecoverTerminal_RetryLinkedFailsLoudWithDoubleBuyFlag(t *testing. func TestSaveOrRecoverTerminal_NothingPurchasedIsNeverOverwritten(t *testing.T) { ctx := context.Background() store := new(MockConfigStore) + assertMocksOnCleanup(t, store) m := &Manager{config: store} exec := &config.PurchaseExecution{ExecutionID: "exec-x", Status: "failed"} store.On("SaveExecutionIfStatus", ctx, exec, []string{"running"}).Return(lostGuard()) @@ -83,6 +87,7 @@ func TestSaveOrRecoverTerminal_NothingPurchasedIsNeverOverwritten(t *testing.T) func TestReapOne_AnnotationLostToRecoveryIsBenign(t *testing.T) { ctx := context.Background() store := new(MockConfigStore) + assertMocksOnCleanup(t, store) row := stuckExec("exec-R", "running") failed := row failed.Status = failedStatus @@ -105,6 +110,9 @@ func TestApprove_ClaimsRunningAndStampsGuarded(t *testing.T) { store.On("GetExecutionByID", mock.Anything, "exec-g").Return(&config.PurchaseExecution{ExecutionID: "exec-g", PlanID: "plan-g", Status: "pending"}, nil).Maybe() store.On("TransitionExecutionStatus", ctx, "exec-g", approveFromStatuses, "running", (*string)(nil)).Return(updated, nil) stubExecuteChain(t, store, sender, "plan-g") + // This path makes no unguarded full-row write (AssertNotCalled below), so the + // shared chain's SavePurchaseExecution stub is optional here. + optionalCalls(&store.Mock, "SavePurchaseExecution") store.On("SaveExecutionIfStatus", ctx, mock.Anything, []string{"running"}).Return(nil).Once() store.On("SaveExecutionIfStatus", mock.Anything, mock.Anything, []string{"running"}).Return(nil).Once() store.On("SaveExecutionIfStatus", mock.Anything, mock.Anything, []string{"completed"}).Return(nil).Once() diff --git a/internal/purchase/claim_pin_test.go b/internal/purchase/claim_pin_test.go index 294820ecd..5175a619f 100644 --- a/internal/purchase/claim_pin_test.go +++ b/internal/purchase/claim_pin_test.go @@ -23,9 +23,13 @@ var errPinLost = errors.New("pin lost") // plan the gate reads is gatePlan; every later GetPurchasePlan read (the // post-claim re-check) returns claimPlan. func newPinManager(ctx context.Context, store *MockConfigStore, factory *MockProviderFactory, exec config.PurchaseExecution, gatePlan, claimPlan *config.PurchasePlan) *Manager { - store.On("GetStaleApprovedExecutions", ctx, mock.Anything).Return([]config.PurchaseExecution{}, nil) - store.On("GetPendingExecutions", ctx).Return([]config.PurchaseExecution{exec}, nil) - store.On("GetExecutionByID", ctx, exec.ExecutionID).Return(&exec, nil) + // Entry reads are per path: the cron path lists the stale and pending rows, + // the SQS path loads the row by ID. A test drives one path, so the other + // path's reads stay unused (.Maybe). The claim and save calls each test adds + // are the required ones. + store.On("GetStaleApprovedExecutions", ctx, mock.Anything).Return([]config.PurchaseExecution{}, nil).Maybe() + store.On("GetPendingExecutions", ctx).Return([]config.PurchaseExecution{exec}, nil).Maybe() + store.On("GetExecutionByID", ctx, exec.ExecutionID).Return(&exec, nil).Maybe() store.On("GetPurchasePlan", ctx, exec.PlanID).Return(gatePlan, nil).Once() store.On("GetPurchasePlan", ctx, exec.PlanID).Return(claimPlan, nil) store.On("GetGlobalConfig", ctx).Return(autoPurchaseOnCfg(), nil) @@ -45,6 +49,7 @@ func TestClaimPin_AttachBetweenGateAndClaimIsRefused(t *testing.T) { for _, path := range []string{"cron", "sqs"} { t.Run(path, func(t *testing.T) { store, factory := new(MockConfigStore), new(MockProviderFactory) + assertMocksOnCleanup(t, store, factory) manager := newPinManager(ctx, store, factory, dueRecExec("notified"), autoPlan(f64(1000)), autoPlan(f64(1000))) store.On("TransitionExecutionStatusPinned", ctx, "exec-gate", []string{"approved", "pending", "notified"}, "running", (*string)(nil), true). Return(nil, config.ErrExecutionNotInExpectedStatus) @@ -70,6 +75,7 @@ func TestClaimPin_AttachBetweenGateAndClaimIsRefused(t *testing.T) { func TestClaimPin_RecommendationBearingRowPinsNonEmpty(t *testing.T) { ctx := context.Background() store, factory := new(MockConfigStore), new(MockProviderFactory) + assertMocksOnCleanup(t, store, factory) manager := newPinManager(ctx, store, factory, dueRecExec("notified", upfrontRec(10)), autoPlan(f64(1000)), autoPlan(f64(1000))) store.On("TransitionExecutionStatusPinned", ctx, "exec-gate", mock.Anything, "running", (*string)(nil), false). Return(nil, config.ErrExecutionNotInExpectedStatus) @@ -84,6 +90,7 @@ func TestClaimPin_RecommendationBearingRowPinsNonEmpty(t *testing.T) { func TestClaimPin_ClaimErrorIsFailure(t *testing.T) { ctx := context.Background() store, factory := new(MockConfigStore), new(MockProviderFactory) + assertMocksOnCleanup(t, store, factory) manager := newPinManager(ctx, store, factory, dueRecExec("notified"), autoPlan(f64(1000)), autoPlan(f64(1000))) store.On("TransitionExecutionStatusPinned", ctx, "exec-gate", mock.Anything, "running", (*string)(nil), true).Return(nil, errPinLost) result, err := manager.ProcessScheduledPurchases(ctx) @@ -102,6 +109,7 @@ func TestClaimPin_PostClaimCapRecheckFailsBeforeSubmission(t *testing.T) { exec := dueRecExec("notified", upfrontRec(10)) exec.Recommendations[0].ID = "rec-1" store, factory := new(MockConfigStore), new(MockProviderFactory) + assertMocksOnCleanup(t, store, factory) manager := newPinManager(ctx, store, factory, exec, autoPlan(f64(1000)), autoPlan(f64(5))) claimed := exec claimed.Status = "running" @@ -144,6 +152,7 @@ func TestClaimPin_PostClaimPlanReadErrorFailsClosed(t *testing.T) { store.On("SavePurchaseExecutionTx", ctx, mock.Anything, mock.Anything). Run(func(args mock.Arguments) { saved = args.Get(2).(*config.PurchaseExecution) }).Return(nil) sender := allowFailedNotification(new(MockEmailSender)) + assertMocksOnCleanup(t, store, sender) manager := &Manager{config: store, email: sender, providerFactory: new(MockProviderFactory)} didClaim, err := manager.claimAndExecute(ctx, &exec) @@ -163,6 +172,7 @@ func TestClaimPin_ApprovedRowSkipsCapRecheck(t *testing.T) { ctx := context.Background() exec := dueRecExec("approved", upfrontRec(1e9)) store := new(MockConfigStore) + assertMocksOnCleanup(t, store) store.On("GetPurchasePlan", mock.Anything, exec.PlanID).Return(autoPlan(nil), nil) store.On("GetPlanAccounts", mock.Anything, exec.PlanID).Return(nil, errors.New("accounts unavailable")) claimed := exec @@ -184,6 +194,7 @@ func TestNotification_EmailFailureLeavesRowPendingAndSchedulerRefusesIt(t *testi ctx := context.Background() store := new(MockConfigStore) sender := new(MockEmailSender) + assertMocksOnCleanup(t, store, sender) plan, existing := pendingExecForNotification(store) notify := "notify@example.com" store.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{NotificationEmail: ¬ify, AutoPurchaseEnabled: true}, nil) @@ -211,6 +222,7 @@ func TestNotification_EmailFailureLeavesRowPendingAndSchedulerRefusesIt(t *testi func TestClaimPin_RecLessRowStillFailsInsteadOfBuying(t *testing.T) { ctx := context.Background() store, factory := new(MockConfigStore), new(MockProviderFactory) + assertMocksOnCleanup(t, store, factory) exec := dueRecExec("pending") manager := newPinManager(ctx, store, factory, exec, autoPlan(nil), autoPlan(nil)) store.On("GetPurchasePlan", mock.Anything, exec.PlanID).Return(autoPlan(nil), nil) diff --git a/internal/purchase/claimed_row_check_test.go b/internal/purchase/claimed_row_check_test.go index 6080712c1..060c45b47 100644 --- a/internal/purchase/claimed_row_check_test.go +++ b/internal/purchase/claimed_row_check_test.go @@ -194,6 +194,7 @@ func TestClaimedRowCheck_CheckErrorFailsClosed(t *testing.T) { func TestClaimedRowCheck_TerminalSaveFailureIsAuditLoss(t *testing.T) { ep := claimEntryPoints()[0] mgr, store, rec, snapshot, _ := claimCheckHarness(t, ep, claimCheckRepriced) + assertMocksOnCleanup(t, store) store.On("SaveExecutionIfStatus", mock.Anything, mock.Anything, executorOwnedStatuses).Return(errors.New("db down")) cc := &capCheck{max: claimCheckCap} @@ -209,6 +210,7 @@ func TestClaimedRowCheck_TerminalSaveFailureIsAuditLoss(t *testing.T) { func TestClaimedRowCheck_PlanRowReleasesSuppressions(t *testing.T) { ep := claimEntryPoints()[0] mgr, store, rec, snapshot, saved := claimCheckHarness(t, ep, claimCheckRepriced) + assertMocksOnCleanup(t, store) // Re-register the claim for a plan row: same row, PlanID set. planRow := *snapshot planRow.PlanID = armedPlanID diff --git a/internal/purchase/coverage_extra_test.go b/internal/purchase/coverage_extra_test.go index 50283fa9c..245e03177 100644 --- a/internal/purchase/coverage_extra_test.go +++ b/internal/purchase/coverage_extra_test.go @@ -143,6 +143,7 @@ func TestResolveAWSProvider_NoSTS(t *testing.T) { func TestResolveAWSProvider_RoleARNWithoutARNOnNonHostFailsClosed(t *testing.T) { ctx := context.Background() hostSTS := new(MockSTSClient) + assertMocksOnCleanup(t, hostSTS) hostSTS.On("GetCallerIdentity", mock.Anything, mock.Anything).Return(&sts.GetCallerIdentityOutput{Account: aws.String("111111111111")}, nil) m := &Manager{ assumeRoleSTS: new(MockAssumeRoleSTS), @@ -250,6 +251,7 @@ func TestHandleExecutePurchase_ApprovedStatus(t *testing.T) { mockSTS.On("GetCallerIdentity", mock.Anything, mock.Anything).Return(nil, errors.New("sts error")) mockProv := new(MockProvider) mockSvc := new(MockServiceClient) + assertMocksOnCleanup(t, mockEmail, mockFactory, mockSTS, mockProv, mockSvc) mockFactory.On("CreateAndValidateProvider", mock.Anything, "aws", mock.Anything).Return(mockProv, nil) mockProv.On("GetServiceClient", mock.Anything, common.ServiceEC2, "us-east-1").Return(mockSvc, nil) mockSvc.On("PurchaseCommitment", mock.Anything, mock.Anything, mock.Anything). @@ -277,6 +279,7 @@ func TestHandleExecutePurchase_ApprovedStatus(t *testing.T) { func TestHandleExecutePurchase_GetError(t *testing.T) { ctx := context.Background() mockStore := new(MockConfigStore) + assertMocksOnCleanup(t, mockStore) mockEmail := new(MockEmailSender) mockStore.On("GetExecutionByID", ctx, "exec-err").Return(nil, errors.New("db error")) @@ -303,6 +306,7 @@ func TestHandleExecutePurchase_SaveError(t *testing.T) { mockEmail := allowFailedNotification(new(MockEmailSender)) mockFactory := new(MockProviderFactory) mockSTS := new(MockSTSClient) + assertMocksOnCleanup(t, mockStore, mockFactory, mockSTS) plan := &config.PurchasePlan{ ID: "plan-save-err", @@ -699,6 +703,7 @@ func TestProcessMessage_CancelRejectsMissingActor(t *testing.T) { func TestProcessMessage_ApproveRejectsNonMatchingActor(t *testing.T) { ctx := context.Background() mockStore := new(MockConfigStore) + assertMocksOnCleanup(t, mockStore) mockEmail := new(MockEmailSender) accountID := "acct-1" @@ -735,6 +740,7 @@ func TestProcessMessage_ApproveRejectsNonMatchingActor(t *testing.T) { func TestProcessMessage_ApproveRejectsTokenMismatch(t *testing.T) { ctx := context.Background() mockStore := new(MockConfigStore) + assertMocksOnCleanup(t, mockStore) mockEmail := new(MockEmailSender) // The recommendation carries a CloudAccountID so that approver resolution, @@ -775,6 +781,7 @@ func TestProcessMessage_ApproveRejectsTokenMismatch(t *testing.T) { func TestProcessMessage_ApproveRejectsNoApprovers(t *testing.T) { ctx := context.Background() mockStore := new(MockConfigStore) + assertMocksOnCleanup(t, mockStore) mockEmail := new(MockEmailSender) exec := &config.PurchaseExecution{ @@ -806,6 +813,7 @@ func TestManager_ExecuteSinglePurchase_ProviderError(t *testing.T) { mockEmail := new(MockEmailSender) mockFactory := new(MockProviderFactory) mockSTS := new(MockSTSClient) + assertMocksOnCleanup(t, mockStore, mockFactory, mockSTS) plan := &config.PurchasePlan{ ID: "plan-prov-err", @@ -854,6 +862,7 @@ func TestManager_ExecuteSinglePurchase_ServiceClientError(t *testing.T) { mockFactory := new(MockProviderFactory) mockProvider := new(MockProvider) mockSTS := new(MockSTSClient) + assertMocksOnCleanup(t, mockStore, mockFactory, mockProvider, mockSTS) plan := &config.PurchasePlan{ ID: "plan-svc-err", @@ -904,6 +913,7 @@ func TestManager_ExecuteSinglePurchase_PurchaseNotSuccessful(t *testing.T) { mockProvider := new(MockProvider) mockServiceClient := new(MockServiceClient) mockSTS := new(MockSTSClient) + assertMocksOnCleanup(t, mockStore, mockFactory, mockProvider, mockServiceClient, mockSTS) plan := &config.PurchasePlan{ ID: "plan-not-success", @@ -956,6 +966,7 @@ func TestManager_ExecuteSinglePurchase_PurchaseNotSuccessful_WithError(t *testin mockProvider := new(MockProvider) mockServiceClient := new(MockServiceClient) mockSTS := new(MockSTSClient) + assertMocksOnCleanup(t, mockStore, mockFactory, mockProvider, mockServiceClient, mockSTS) plan := &config.PurchasePlan{ ID: "plan-err-result", @@ -1009,6 +1020,7 @@ func TestManager_ExecuteSinglePurchase_WithEngine(t *testing.T) { mockProvider := new(MockProvider) mockServiceClient := new(MockServiceClient) mockSTS := new(MockSTSClient) + assertMocksOnCleanup(t, mockStore, mockEmail, mockFactory, mockProvider, mockServiceClient, mockSTS) plan := &config.PurchasePlan{ ID: "plan-engine", @@ -1066,6 +1078,7 @@ func TestManager_SavePurchaseHistory_Error(t *testing.T) { mockProvider := new(MockProvider) mockServiceClient := new(MockServiceClient) mockSTS := new(MockSTSClient) + assertMocksOnCleanup(t, mockStore, mockEmail, mockFactory, mockProvider, mockServiceClient, mockSTS) plan := &config.PurchasePlan{ ID: "plan-hist-err", @@ -1354,6 +1367,7 @@ func TestManager_ExecuteSinglePurchase_DetailsByService(t *testing.T) { mockProvider := new(MockProvider) mockServiceClient := new(MockServiceClient) mockSTS := new(MockSTSClient) + assertMocksOnCleanup(t, mockStore, mockEmail, mockFactory, mockProvider, mockServiceClient, mockSTS) // Marshal the canonical Details into the RecommendationRecord // — this is exactly what the scheduler does at collection @@ -1501,6 +1515,7 @@ func TestManager_ExecuteSinglePurchase_LegacyEmptyDetails(t *testing.T) { mockProvider := new(MockProvider) mockServiceClient := new(MockServiceClient) mockSTS := new(MockSTSClient) + assertMocksOnCleanup(t, mockStore, mockEmail, mockFactory, mockProvider, mockServiceClient, mockSTS) plan := &config.PurchasePlan{ID: "plan-" + tc.name, Name: "Plan"} exec := &config.PurchaseExecution{ @@ -1598,6 +1613,7 @@ func TestExecuteForAccount_NonHostRoleARN_StoredErrorOmitsHostIdentity(t *testin ctx := context.Background() const hostID = "111111111111" hostSTS := new(MockSTSClient) + assertMocksOnCleanup(t, hostSTS) hostSTS.On("GetCallerIdentity", mock.Anything, mock.Anything).Return(&sts.GetCallerIdentityOutput{Account: aws.String(hostID)}, nil) var saved []*config.PurchaseExecution diff --git a/internal/purchase/empty_selection_notify_test.go b/internal/purchase/empty_selection_notify_test.go index b91e25bb6..8e181e8c2 100644 --- a/internal/purchase/empty_selection_notify_test.go +++ b/internal/purchase/empty_selection_notify_test.go @@ -27,6 +27,7 @@ func unselectedRecs() []config.RecommendationRecord { func TestExecutePurchase_SingleAccount_NothingSelected_SendsNoConfirmation(t *testing.T) { ctx := context.Background() mockStore := new(MockConfigStore) + assertMocksOnCleanup(t, mockStore) mockEmail := new(MockEmailSender) mockFactory := new(MockProviderFactory) diff --git a/internal/purchase/execute_detached_test.go b/internal/purchase/execute_detached_test.go index 28f3ecfed..7634a72ac 100644 --- a/internal/purchase/execute_detached_test.go +++ b/internal/purchase/execute_detached_test.go @@ -70,6 +70,7 @@ func newDetachFixture(t *testing.T) *detachFixture { } }).Return(nil) email := allowFailedNotification(new(MockEmailSender)) + assertMocksOnCleanup(t, f.svc, store, factory, prov, stsMock, email) email.On("SendPurchaseConfirmation", mock.Anything, mock.Anything).Return(nil) stsMock.On("GetCallerIdentity", mock.Anything, mock.Anything).Return(&stsOut, nil) factory.On("CreateAndValidateProvider", mock.Anything, "aws", mock.Anything).Return(prov, nil) @@ -185,6 +186,9 @@ func TestExecuteAndFinalize_ProviderErrorWithCanceledCaller_PersistsFailed(t *te f.svc.On("PurchaseCommitment", mock.Anything, mock.Anything, mock.Anything). Run(func(mock.Arguments) { cancel() }). Return(common.PurchaseResult{}, errors.New("provider rejected")).Once() + // Nothing bought: the fixture's history and confirmation stubs stay unused. + optionalCalls(&f.manager.config.(*MockConfigStore).Mock, "SavePurchaseHistory") + optionalCalls(&f.manager.email.(*MockEmailSender).Mock, "SendPurchaseConfirmation") err := f.manager.executeAndFinalize(ctx, f.exec) diff --git a/internal/purchase/execution_test.go b/internal/purchase/execution_test.go index 1728fcacf..eed8fc74a 100644 --- a/internal/purchase/execution_test.go +++ b/internal/purchase/execution_test.go @@ -60,6 +60,7 @@ func TestManager_ExecutePurchase(t *testing.T) { mockFactory := new(MockProviderFactory) mockProviderInst := new(MockProvider) mockServiceClient := new(MockServiceClient) + assertMocksOnCleanup(t, mockProviderInst, mockServiceClient) plan := &config.PurchasePlan{ ID: "plan-123", @@ -150,6 +151,7 @@ func TestManager_ExecutePurchase_WebSourcePropagates(t *testing.T) { mockSTS := new(MockSTSClient) mockFactory := new(MockProviderFactory) mockProviderInst := new(MockProvider) + assertMocksOnCleanup(t, mockStore, mockEmail, mockSTS, mockFactory, mockProviderInst) mockServiceClient := new(MockServiceClient) plan := &config.PurchasePlan{ID: "plan-web", Name: "Web Plan"} @@ -206,6 +208,7 @@ func TestManager_ExecutePurchase_InvalidSourceFallsBackUntagged(t *testing.T) { mockSTS := new(MockSTSClient) mockFactory := new(MockProviderFactory) mockProviderInst := new(MockProvider) + assertMocksOnCleanup(t, mockStore, mockEmail, mockSTS, mockFactory, mockProviderInst) mockServiceClient := new(MockServiceClient) plan := &config.PurchasePlan{ID: "plan-bad", Name: "Bad Plan"} @@ -717,6 +720,7 @@ func TestManager_ExecutePurchase_MultiAccount(t *testing.T) { mockFactory := new(MockProviderFactory) mockProvider := new(MockProvider) mockServiceClient := new(MockServiceClient) + assertMocksOnCleanup(t, mockProvider, mockServiceClient) credStore := &MockCredentialStore{ LoadRawFn: func(_ context.Context, _, _ string) ([]byte, error) { return []byte(`{"access_key_id":"AKIAIOSFODNN7EXAMPLE","secret_access_key":"wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY"}`), nil @@ -856,6 +860,7 @@ func TestExecuteForAccount_CredentialFailure_MarksFailed(t *testing.T) { t.Run(tt.name, func(t *testing.T) { ctx := context.Background() mockStore := new(MockConfigStore) + assertMocksOnCleanup(t, mockStore) mockEmail := allowFailedNotification(new(MockEmailSender)) mockFactory := new(MockProviderFactory) // Credential store returns nothing for every load — this triggers @@ -1332,6 +1337,7 @@ func TestExecutePurchase_SingleAccount_AzureUsesResolvedCreds(t *testing.T) { mockFactory := new(MockProviderFactory) mockProviderInst := new(MockProvider) mockServiceClient := new(MockServiceClient) + assertMocksOnCleanup(t, mockEmail, mockSTS, mockProviderInst, mockServiceClient) acctID := "azure-acct-1" // Direct-execute: no PlanID, no exec-level CloudAccountID. @@ -1467,6 +1473,7 @@ func TestExecutePurchase_AzureCanonicalServiceTypes(t *testing.T) { mockFactory := new(MockProviderFactory) mockProviderInst := new(MockProvider) mockServiceClient := new(MockServiceClient) + assertMocksOnCleanup(t, mockEmail, mockSTS, mockServiceClient) mockCredStore := &MockCredentialStore{} acctID := "azure-acct-" + tc.name @@ -1798,6 +1805,7 @@ func TestManager_ExecuteAndFinalize_HistorySaveFailure_StaysVisible(t *testing.T mockFactory := new(MockProviderFactory) mockProviderInst := new(MockProvider) mockServiceClient := new(MockServiceClient) + assertMocksOnCleanup(t, mockEmail, mockSTS, mockProviderInst, mockServiceClient) // Direct purchase (no plan) → single-account path, no updatePlanProgress. exec := &config.PurchaseExecution{ @@ -1874,6 +1882,7 @@ func TestManager_ExecuteAndFinalize_EmptyCommitmentID_CompletesWithAuditGap(t *t mockFactory := new(MockProviderFactory) mockProviderInst := new(MockProvider) mockServiceClient := new(MockServiceClient) + assertMocksOnCleanup(t, mockStore, mockEmail, mockSTS, mockFactory, mockProviderInst, mockServiceClient) exec := &config.PurchaseExecution{ ExecutionID: "exec-empty-id", @@ -1927,6 +1936,7 @@ func TestManager_ExecuteAndFinalize_SingleAccount_PartialSuccess(t *testing.T) { mockFactory := new(MockProviderFactory) mockProviderInst := new(MockProvider) mockServiceClient := new(MockServiceClient) + assertMocksOnCleanup(t, mockFactory, mockProviderInst, mockServiceClient) exec := &config.PurchaseExecution{ ExecutionID: "exec-partial-single", @@ -2001,6 +2011,7 @@ func TestExecuteForAccount_PartialSuccess(t *testing.T) { mockFactory := new(MockProviderFactory) mockProviderInst := new(MockProvider) mockServiceClient := new(MockServiceClient) + assertMocksOnCleanup(t, mockFactory, mockProviderInst, mockServiceClient) const acctID = "aaaaaaaa-0000-0000-0000-000000000001" account := config.CloudAccount{ID: acctID, Name: "Prod", Provider: "aws", ExternalID: "111111111111", AWSAuthMode: "access_keys"} @@ -2114,6 +2125,7 @@ func TestManager_ExecutePurchase_SingleAccount_StampsTargetAccount(t *testing.T) mockFactory := new(MockProviderFactory) mockProviderInst := new(MockProvider) mockServiceClient := new(MockServiceClient) + assertMocksOnCleanup(t, mockEmail, mockFactory, mockProviderInst, mockServiceClient) mockCredStore := &MockCredentialStore{ LoadRawFn: func(_ context.Context, _, _ string) ([]byte, error) { // AWS access_keys path reads stored key JSON; Azure diff --git a/internal/purchase/failure_notify_paths_test.go b/internal/purchase/failure_notify_paths_test.go index a3b5dbb01..b16324121 100644 --- a/internal/purchase/failure_notify_paths_test.go +++ b/internal/purchase/failure_notify_paths_test.go @@ -170,10 +170,17 @@ func TestTerminalFailureWritersAlertOncePerWonRow(t *testing.T) { t.Run(tc.name, func(t *testing.T) { logs := captureMarkers(t) store := new(MockConfigStore) - store.On("GetGlobalConfig", mock.Anything).Return(failureNotifyCfg(), nil) + cfgRead := store.On("GetGlobalConfig", mock.Anything).Return(failureNotifyCfg(), nil) sender := new(MockEmailSender) + assertMocksOnCleanup(t, store, sender) rec := &failedEmailRecorder{} - rec.expect(sender) + send := rec.expect(sender) + if !tc.alerts { + // A writer that lost the row must not alert (Len 0 below); the + // stubs only keep such a regression from panicking the mock. + cfgRead.Maybe() + send.Maybe() + } m := &Manager{config: store, email: sender} tc.run(t, m, store) diff --git a/internal/purchase/failure_notify_test.go b/internal/purchase/failure_notify_test.go index a95e64653..bfa6b9b75 100644 --- a/internal/purchase/failure_notify_test.go +++ b/internal/purchase/failure_notify_test.go @@ -32,8 +32,8 @@ type failedEmailRecorder struct { sent []email.NotificationData } -func (r *failedEmailRecorder) expect(sender *MockEmailSender) { - sender.On("SendPurchaseFailedNotification", mock.Anything, mock.Anything). +func (r *failedEmailRecorder) expect(sender *MockEmailSender) *mock.Call { + return sender.On("SendPurchaseFailedNotification", mock.Anything, mock.Anything). Run(func(args mock.Arguments) { r.mu.Lock() defer r.mu.Unlock() @@ -51,6 +51,7 @@ func TestNotifyExecutionFailure_FailedRowEmailsUnpurchasedSelectedRecs(t *testin store := new(MockConfigStore) store.On("GetGlobalConfig", mock.Anything).Return(failureNotifyCfg(), nil) sender := new(MockEmailSender) + assertMocksOnCleanup(t, store, sender) rec := &failedEmailRecorder{} rec.expect(sender) m := &Manager{config: store, email: sender, dashboardURL: "https://dashboard.example.com/"} @@ -84,7 +85,10 @@ func TestNotifyExecutionFailure_OnlyFailedOrPartialRowsNotify(t *testing.T) { for _, status := range []string{"completed", "running", "approved", "pending", ""} { t.Run(fmt.Sprintf("status=%q", status), func(t *testing.T) { sender := new(MockEmailSender) - sender.On("SendPurchaseFailedNotification", mock.Anything, mock.Anything).Return(nil) + assertMocksOnCleanup(t, sender) + // Must NOT fire (AssertNotCalled below); .Maybe() keeps a regression + // an assertion failure instead of a mock panic. + sender.On("SendPurchaseFailedNotification", mock.Anything, mock.Anything).Return(nil).Maybe() m := &Manager{config: new(MockConfigStore), email: sender} m.notifyExecutionFailure(context.Background(), &config.PurchaseExecution{ExecutionID: "e", Status: status}) sender.AssertNotCalled(t, "SendPurchaseFailedNotification", mock.Anything, mock.Anything) @@ -93,6 +97,7 @@ func TestNotifyExecutionFailure_OnlyFailedOrPartialRowsNotify(t *testing.T) { for _, status := range []string{"failed", "partially_completed"} { t.Run(fmt.Sprintf("status=%q", status), func(t *testing.T) { sender := new(MockEmailSender) + assertMocksOnCleanup(t, sender) sender.On("SendPurchaseFailedNotification", mock.Anything, mock.Anything).Return(nil) m := &Manager{config: new(MockConfigStore), email: sender} m.notifyExecutionFailure(context.Background(), &config.PurchaseExecution{ExecutionID: "e", Status: status}) @@ -104,6 +109,7 @@ func TestNotifyExecutionFailure_OnlyFailedOrPartialRowsNotify(t *testing.T) { // A send failure is logged, never propagated: the purchase outcome stands. func TestNotifyExecutionFailure_SendErrorAndNilSenderAreBestEffort(t *testing.T) { sender := new(MockEmailSender) + assertMocksOnCleanup(t, sender) sender.On("SendPurchaseFailedNotification", mock.Anything, mock.Anything).Return(errors.New("ses down")) m := &Manager{config: new(MockConfigStore), email: sender} exec := &config.PurchaseExecution{ExecutionID: "e", Status: "failed", Error: "boom"} @@ -120,6 +126,7 @@ func TestNotifyExecutionFailure_SendErrorAndNilSenderAreBestEffort(t *testing.T) // is already canceled (a disconnected approver or an exhausted budget). func TestNotifyExecutionFailure_SurvivesCanceledCallerContext(t *testing.T) { sender := new(MockEmailSender) + assertMocksOnCleanup(t, sender) var sendCtxErr error sender.On("SendPurchaseFailedNotification", mock.Anything, mock.Anything). Run(func(args mock.Arguments) { sendCtxErr = args.Get(0).(context.Context).Err() }).Return(nil) @@ -151,6 +158,7 @@ func TestMultiAccountFailureNotifiesOncePerFailedAccountNotRoot(t *testing.T) { rec := &failedEmailRecorder{} rec.expect(sender) factory, prov, svc := new(MockProviderFactory), new(MockProvider), new(MockServiceClient) + assertMocksOnCleanup(t, store, sender, factory, prov, svc) accounts := []config.CloudAccount{ {ID: "acct-ok", Name: "OK", Provider: "aws", ExternalID: "111111111111", AWSAuthMode: "access_keys"}, diff --git a/internal/purchase/failure_notify_writers_test.go b/internal/purchase/failure_notify_writers_test.go index e12379b16..7f0d4c102 100644 --- a/internal/purchase/failure_notify_writers_test.go +++ b/internal/purchase/failure_notify_writers_test.go @@ -97,6 +97,7 @@ func TestNotifyRootFailure_FanOutRootLogsMarkerWithoutEmail(t *testing.T) { t.Run("non fan-out root emails too", func(t *testing.T) { logs := captureMarkers(t) sender := allowFailedNotification(new(MockEmailSender)) + assertMocksOnCleanup(t, sender) m := &Manager{config: new(MockConfigStore), email: sender} root := &config.PurchaseExecution{ExecutionID: "root", Status: "failed"} m.notifyRootFailure(context.Background(), root, errors.New("provider down")) @@ -115,8 +116,11 @@ func fanOutRun(t *testing.T, other config.CloudAccount, loadRaw func(string) ([] store := new(MockConfigStore) sender := new(MockEmailSender) rec := &failedEmailRecorder{} - rec.expect(sender) + // Optional: callers assert the exact set of emailed rows, which is empty + // when the failing child never reached its own notification. + rec.expect(sender).Maybe() factory, prov, svc := new(MockProviderFactory), new(MockProvider), new(MockServiceClient) + assertMocksOnCleanup(t, store, sender, factory, prov, svc) accounts := []config.CloudAccount{ {ID: "acct-ok", Name: "OK", Provider: "aws", ExternalID: "111111111111", AWSAuthMode: "access_keys"}, diff --git a/internal/purchase/finalize_revocations_test.go b/internal/purchase/finalize_revocations_test.go index 63e379a25..76b1fa87b 100644 --- a/internal/purchase/finalize_revocations_test.go +++ b/internal/purchase/finalize_revocations_test.go @@ -69,6 +69,7 @@ func TestFinalizeInFlightRevocations_Accounting(t *testing.T) { func TestFinalizeInFlightRevocations_ListErrorIsReturned(t *testing.T) { ctx := context.Background() store := new(MockConfigStore) + assertMocksOnCleanup(t, store) store.On("GetPurchaseHistoryInFlight", ctx).Return(nil, errors.New("list failed")) m := &Manager{config: store} @@ -86,6 +87,7 @@ func TestFinalizeInFlightRevocations_CanceledDuringBackoffStopsSweep(t *testing. ctx, cancel := context.WithCancel(context.Background()) defer cancel() store := new(MockConfigStore) + assertMocksOnCleanup(t, store) store.On("GetPurchaseHistoryInFlight", ctx).Return(inFlightRows("ok", "stuck", "later"), nil) expectMark(store, "ok", nil) expectMark(store, "stuck", errors.New("db down")).Run(func(mock.Arguments) { cancel() }) @@ -143,6 +145,7 @@ func TestFinalizeInFlightRevocations_ReleaseErrorStillFinalizes(t *testing.T) { func TestFinalizeInFlightRevocations_ReleaseAndListErrorsJoined(t *testing.T) { ctx := context.Background() store := new(MockConfigStore) + assertMocksOnCleanup(t, store) store.On("ReleaseStaleRevocationsInFlight", ctx, staleRevocationInFlightAfter). Return(nil, errors.New("release failed")).Once() store.On("GetPurchaseHistoryInFlight", ctx).Return(nil, errors.New("list failed")).Once() diff --git a/internal/purchase/gcp_history_cost_test.go b/internal/purchase/gcp_history_cost_test.go index 85a80bd98..b1bc38f35 100644 --- a/internal/purchase/gcp_history_cost_test.go +++ b/internal/purchase/gcp_history_cost_test.go @@ -60,6 +60,7 @@ func TestSavePurchaseHistory_GCPRecordsNoUpfront(t *testing.T) { require.NoError(t, err) store := new(MockConfigStore) + assertMocksOnCleanup(t, store) var saved *config.PurchaseHistoryRecord store.On("SavePurchaseHistory", ctx, mock.AnythingOfType("*config.PurchaseHistoryRecord")). Run(func(args mock.Arguments) { saved = args.Get(1).(*config.PurchaseHistoryRecord) }). diff --git a/internal/purchase/ladder_execution_recheck_test.go b/internal/purchase/ladder_execution_recheck_test.go index 5d3683ad7..c523cfe7c 100644 --- a/internal/purchase/ladder_execution_recheck_test.go +++ b/internal/purchase/ladder_execution_recheck_test.go @@ -150,6 +150,7 @@ func TestLadderRecheck_EveryExecutorEntryRefuses(t *testing.T) { off := ladderGlobal(true, false) t.Run("approve", func(t *testing.T) { mgr, store, rec, exec := ladderHarness(t, ladderTrancheKey, openGate(), nil, off, nil) + assertMocksOnCleanup(t, store) exec.Status = "pending" expectClaim(store, exec, []string{"pending", "notified"}) _, _, err := mgr.ApproveAndExecute(context.Background(), exec.ExecutionID, "operator@example.com", nil, nil) diff --git a/internal/purchase/manager_test.go b/internal/purchase/manager_test.go index cef3465b8..ae9f95d9b 100644 --- a/internal/purchase/manager_test.go +++ b/internal/purchase/manager_test.go @@ -232,6 +232,7 @@ func TestManager_ProcessScheduledPurchases_DuePurchase(t *testing.T) { mockFactory := new(MockProviderFactory) mockProvider := new(MockProvider) mockServiceClient := new(MockServiceClient) + assertMocksOnCleanup(t, mockFactory, mockProvider, mockServiceClient) mockFactory.On("CreateAndValidateProvider", mock.Anything, "", mock.Anything).Return(mockProvider, nil) mockProvider.On("GetServiceClient", mock.MatchedBy(hasPerRecDeadline(30*time.Second)), common.ServiceEC2, "us-east-1").Return(mockServiceClient, nil) @@ -501,6 +502,7 @@ func TestManager_RecoverStrandedApprovals_FailsStrandedRow(t *testing.T) { ctx := context.Background() mockStore := new(MockConfigStore) mockEmail := new(MockEmailSender) + assertMocksOnCleanup(t, mockEmail) mockFactory := new(MockProviderFactory) stranded := config.PurchaseExecution{ diff --git a/internal/purchase/mocks_test.go b/internal/purchase/mocks_test.go index 8d6b92631..121f77276 100644 --- a/internal/purchase/mocks_test.go +++ b/internal/purchase/mocks_test.go @@ -2,6 +2,8 @@ package purchase import ( "context" + "slices" + "testing" "github.com/LeanerCloud/cloud-commitments-go/pkg/common" "github.com/LeanerCloud/cloud-commitments-go/pkg/provider" @@ -17,6 +19,36 @@ import ( // and default behaviors live in internal/mocks. type MockConfigStore = mocks.MockConfigStore +// expectationAsserter is any testify mock (*mock.Mock or a type embedding it). +type expectationAsserter interface { + AssertExpectations(t mock.TestingT) bool +} + +// assertMocksOnCleanup makes t fail at cleanup when any required (non-.Maybe()) +// expectation registered on ms was never consumed, so an .On(...) a test sets up +// is also a check that the call happened (issue #35). Call it right after +// constructing the mocks; registering it before the expectations is fine. +func assertMocksOnCleanup(t *testing.T, ms ...expectationAsserter) { + t.Helper() + t.Cleanup(func() { + for _, m := range ms { + m.AssertExpectations(t) + } + }) +} + +// optionalCalls marks the expectations a shared helper registered for methods +// as optional, for a test whose path never makes those calls. Times(0) clears +// a .Once() count too: testify reports an unconsumed .Once() even when it is +// .Maybe(). +func optionalCalls(m *mock.Mock, methods ...string) { + for _, c := range m.ExpectedCalls { + if slices.Contains(methods, c.Method) { + c.Maybe().Times(0) + } + } +} + // MockProviderFactory is a mock implementation of ProviderFactoryInterface. type MockProviderFactory struct { mock.Mock diff --git a/internal/purchase/notified_claim_pin_integration_test.go b/internal/purchase/notified_claim_pin_integration_test.go index d6653e196..db3d2a9d9 100644 --- a/internal/purchase/notified_claim_pin_integration_test.go +++ b/internal/purchase/notified_claim_pin_integration_test.go @@ -61,6 +61,7 @@ func TestNotifiedTransition_OnlyAfterEmailSucceeds(t *testing.T) { factory := new(MockProviderFactory) sender := new(MockEmailSender) + assertMocksOnCleanup(t, sender) sender.On("SendScheduledPurchaseNotification", ctx, mock.Anything).Return(errors.New("ses down")).Once() manager := &Manager{config: store, email: sender, providerFactory: factory} require.False(t, manager.sendPlanNotification(ctx, plan)) diff --git a/internal/purchase/notified_run_now_test.go b/internal/purchase/notified_run_now_test.go index 9d724fe55..c92c21cea 100644 --- a/internal/purchase/notified_run_now_test.go +++ b/internal/purchase/notified_run_now_test.go @@ -65,11 +65,16 @@ func TestScheduler_NotifiedRowRespectsPushedScheduledDate(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { store, factory := new(MockConfigStore), new(MockProviderFactory) + assertMocksOnCleanup(t, store, factory) exec := dueRecExec("notified", upfrontRec(10)) exec.ScheduledDate = tc.date manager := newPinManager(ctx, store, factory, exec, autoPlan(f64(1000)), autoPlan(f64(1000))) store.On("TransitionExecutionStatusPinned", ctx, "exec-gate", mock.Anything, "running", (*string)(nil), false). Return(nil, config.ErrExecutionNotInExpectedStatus).Maybe() + if !tc.wantClaim { + // A row not yet due never reaches the gate's plan and kill-switch reads. + optionalCalls(&store.Mock, "GetPurchasePlan", "GetGlobalConfig") + } _, err := manager.ProcessScheduledPurchases(ctx) require.NoError(t, err) diff --git a/internal/purchase/partition_guard_test.go b/internal/purchase/partition_guard_test.go index 1cdeb4444..d3825d43a 100644 --- a/internal/purchase/partition_guard_test.go +++ b/internal/purchase/partition_guard_test.go @@ -142,6 +142,7 @@ func TestExecutePurchase_SingleAccount_AWSChinaSavingsPlanRefused(t *testing.T) mock.AnythingOfType("common.PurchaseOptions"), ).Return(common.PurchaseResult{Success: true, CommitmentID: "ri-cn"}, nil).Once() mockEmail := new(MockEmailSender) + assertMocksOnCleanup(t, mockStore, mockFactory, mockProviderInst, mockEmail) mockEmail.On("SendPurchaseConfirmation", ctx, mock.AnythingOfType("email.NotificationData")).Return(nil) m := newAWSCNManager(mockStore, mockFactory, mockEmail) diff --git a/internal/purchase/plan_step_atomic_test.go b/internal/purchase/plan_step_atomic_test.go index 3074e1840..d6bc9f7aa 100644 --- a/internal/purchase/plan_step_atomic_test.go +++ b/internal/purchase/plan_step_atomic_test.go @@ -49,6 +49,7 @@ func TestGetOrCreateExecution_ExistingNonAdoptableRowBlocks(t *testing.T) { for name, row := range cases { t.Run(name, func(t *testing.T) { store := new(MockConfigStore) + assertMocksOnCleanup(t, store) plan := stepTestPlan() expectStepRows(store, plan, row) m := &Manager{config: store} @@ -69,6 +70,7 @@ func TestGetOrCreateExecution_ExistingNonAdoptableRowBlocks(t *testing.T) { func TestGetOrCreateExecution_AdoptsPendingRootOnly(t *testing.T) { acct := "acct-A" store := new(MockConfigStore) + assertMocksOnCleanup(t, store) plan := stepTestPlan() child := stepRow("pending", func(e *config.PurchaseExecution) { e.ExecutionID = "child"; e.CloudAccountID = &acct }) root := stepRow("notified", func(e *config.PurchaseExecution) { e.ExecutionID = "root" }) @@ -86,6 +88,7 @@ func TestGetOrCreateExecution_AdoptsPendingRootOnly(t *testing.T) { // The step is keyed on the locked plan's CurrentStep, not on the date. func TestGetOrCreateExecution_KeysOnStepNotDate(t *testing.T) { store := new(MockConfigStore) + assertMocksOnCleanup(t, store) plan := stepTestPlan() row := stepRow("pending", nil) row.ScheduledDate = time.Now().AddDate(0, 0, -30) // differs from NextExecutionDate @@ -102,6 +105,7 @@ func TestAttachResolvedRecommendations(t *testing.T) { recs := []config.RecommendationRecord{scopedTestRec("acct-A"), scopedTestRec("acct-B")} t.Run("attaches recs and writes one suppression per tuple in the same tx", func(t *testing.T) { store := new(MockConfigStore) + assertMocksOnCleanup(t, store) store.On("GetGlobalConfig", mock.Anything).Return(&config.GlobalConfig{}, nil) store.On("SetExecutionRecommendationsIfEmptyTx", mock.Anything, mock.Anything, "row-1", recs, 600.0, 150.0).Return(true, nil) store.On("CreateSuppressionTx", mock.Anything, mock.Anything, mock.Anything).Return(nil).Twice() @@ -201,6 +205,7 @@ func TestSaveTerminalExecution_ReleasesSuppressionsOnlyBeforeSubmission(t *testi for _, c := range cases { t.Run(c.name, func(t *testing.T) { store := new(MockConfigStore) + assertMocksOnCleanup(t, store) store.On("SavePurchaseExecution", mock.Anything, mock.Anything).Return(nil) store.On("DeleteSuppressionsByExecutionTx", mock.Anything, mock.Anything, "e").Return(nil).Maybe() exec := c.exec @@ -228,6 +233,7 @@ func TestExecuteAndFinalize_SuppressionReleaseFollowsSubmission(t *testing.T) { acct := "acct-A" run := func(rec config.RecommendationRecord) (*MockConfigStore, error) { store := new(MockConfigStore) + assertMocksOnCleanup(t, store) store.On("GetPurchasePlan", mock.Anything, "p").Return(&config.PurchasePlan{ID: "p"}, nil) store.On("GetPlanAccounts", mock.Anything, "p").Return([]config.CloudAccount{}, nil).Maybe() store.On("GetCloudAccount", mock.Anything, acct).Return(nil, errors.New("account lookup failed")).Maybe() @@ -275,6 +281,7 @@ func TestExecuteAndFinalize_PostSubmissionFailureKeepsSuppressions(t *testing.T) store := new(MockConfigStore) factory := new(MockProviderFactory) prov := new(MockProvider) + assertMocksOnCleanup(t, store, factory, prov) svc := new(MockServiceClient) stsMock := new(MockSTSClient) store.On("GetPurchasePlan", mock.Anything, "p").Return(&config.PurchasePlan{ID: "p"}, nil) diff --git a/internal/purchase/plan_step_attach_integration_test.go b/internal/purchase/plan_step_attach_integration_test.go index 5fc65b30c..1caf111ef 100644 --- a/internal/purchase/plan_step_attach_integration_test.go +++ b/internal/purchase/plan_step_attach_integration_test.go @@ -106,6 +106,7 @@ func TestAttachIntegration_EmailFailureLeavesPendingAndNextTickDoesNotResolveAga plan := attachPlanInDB(ctx, t, store, "attach-crash", time.Now().Add(24*time.Hour)) res := &fakeResolver{recs: []config.RecommendationRecord{attachRec("rec-a", 2)}} sender := new(MockEmailSender) + assertMocksOnCleanup(t, sender) sender.On("SendScheduledPurchaseNotification", ctx, mock.Anything).Return(fmt.Errorf("ses down")).Once() sender.On("SendScheduledPurchaseNotification", ctx, mock.Anything).Return(nil).Once() m := &Manager{config: store, email: sender, resolver: res} diff --git a/internal/purchase/plan_step_attach_test.go b/internal/purchase/plan_step_attach_test.go index 60513aa56..cfbc55e3c 100644 --- a/internal/purchase/plan_step_attach_test.go +++ b/internal/purchase/plan_step_attach_test.go @@ -37,7 +37,9 @@ func attachManager(t *testing.T, listed, locked *config.PurchasePlan, res *fakeR store := new(MockConfigStore) store.On("LockPurchasePlanTx", mock.Anything, mock.Anything, listed.ID).Return(locked, nil) store.On("ListExecutionsForPlanStepTx", mock.Anything, mock.Anything, listed.ID, 1).Return(rows, nil) - store.On("GetGlobalConfig", mock.Anything).Return(&config.GlobalConfig{}, nil) + // Read only for the grace periods (no-match and attach paths); a step + // refused before that, e.g. over the cap, never reads it. + store.On("GetGlobalConfig", mock.Anything).Return(&config.GlobalConfig{}, nil).Maybe() return &Manager{config: store, resolver: res}, store } @@ -55,6 +57,7 @@ func TestAttach_NewRowIsPendingWithoutTokenAndRotatesIn(t *testing.T) { plan := attachPlan(time.Now().Add(48*time.Hour), 1e6) res := &fakeResolver{recs: attachRecs()} m, store := attachManager(t, plan, plan, res) + assertMocksOnCleanup(t, store) store.On("SavePurchaseExecution", mock.Anything, mock.Anything).Return(nil) store.On("CreateSuppressionTx", mock.Anything, mock.Anything, mock.Anything).Return(nil) @@ -84,7 +87,11 @@ func TestAttach_NoMatchWaitsForGraceThenFailsOnce(t *testing.T) { t.Run(tc.name, func(t *testing.T) { plan := attachPlan(tc.next, 1e6) m, store := attachManager(t, plan, plan, &fakeResolver{err: planstep.ErrNoMatch}) - store.On("SavePurchaseExecution", mock.Anything, mock.Anything).Return(nil) + assertMocksOnCleanup(t, store) + save := store.On("SavePurchaseExecution", mock.Anything, mock.Anything).Return(nil) + if !tc.wantFail { + save.Maybe() // no row inside the grace window (asserted below) + } _, _, _, err := m.getOrCreateExecution(context.Background(), plan) require.ErrorIs(t, err, errExecutionNotNotifiable) @@ -104,6 +111,7 @@ func TestAttach_CapIsCheckedOnTheLockedPlan(t *testing.T) { listed := attachPlan(time.Now().Add(48*time.Hour), 1e6) locked := attachPlan(*listed.NextExecutionDate, 5) // lowered between list and lock m, store := attachManager(t, listed, locked, &fakeResolver{recs: attachRecs()}) + assertMocksOnCleanup(t, store) store.On("SavePurchaseExecution", mock.Anything, mock.Anything).Return(nil) _, _, _, err := m.getOrCreateExecution(context.Background(), listed) @@ -157,6 +165,7 @@ func TestAttach_PreCreatedEmptyRootRow(t *testing.T) { t.Run("attaches in place without a second row", func(t *testing.T) { m, store := attachManager(t, plan, plan, &fakeResolver{recs: attachRecs()}, root) + assertMocksOnCleanup(t, store) store.On("SetExecutionRecommendationsIfEmptyTx", mock.Anything, mock.Anything, "root", mock.Anything, mock.Anything, mock.Anything).Return(true, nil) exec, _, rotate, err := m.getOrCreateExecution(context.Background(), plan) require.NoError(t, err) @@ -191,6 +200,7 @@ func TestAttachScheduledDate_NeverInsideTheVetoWindow(t *testing.T) { func TestAttach_NewRowGetsTheVetoWindowWhenThePlanDateIsClose(t *testing.T) { plan := attachPlan(time.Now().Add(10*time.Minute), 1e6) // NotificationDaysBefore = 3 m, store := attachManager(t, plan, plan, &fakeResolver{recs: attachRecs()}) + assertMocksOnCleanup(t, store) store.On("SavePurchaseExecution", mock.Anything, mock.Anything).Return(nil) store.On("CreateSuppressionTx", mock.Anything, mock.Anything, mock.Anything).Return(nil) @@ -250,6 +260,7 @@ func TestAttach_UnsupportedPlanFailsTheStepVisibly(t *testing.T) { res := &fakeResolver{err: planstep.Precheck(locked)} require.ErrorIs(t, res.err, planstep.ErrUnsupportedPlan) m, store := attachManager(t, listed, locked, res) + assertMocksOnCleanup(t, store) store.On("SavePurchaseExecution", mock.Anything, mock.Anything).Return(nil) _, _, _, err := m.getOrCreateExecution(context.Background(), listed) diff --git a/internal/purchase/plan_step_proofs_test.go b/internal/purchase/plan_step_proofs_test.go index 1c9c20336..80479ed7e 100644 --- a/internal/purchase/plan_step_proofs_test.go +++ b/internal/purchase/plan_step_proofs_test.go @@ -91,6 +91,7 @@ func tokenCapture(t *testing.T, providerName string, exec *config.PurchaseExecut ctx := context.Background() store, sender, factory := new(MockConfigStore), new(MockEmailSender), new(MockProviderFactory) prov, svc := new(MockProvider), new(MockServiceClient) + assertMocksOnCleanup(t, store, sender, factory, prov, svc) store.SavePurchaseExecutionFn = func(context.Context, *config.PurchaseExecution) error { return nil } store.On("SavePurchaseHistory", ctx, mock.Anything).Return(nil) factory.On("CreateAndValidateProvider", mock.Anything, providerName, mock.Anything).Return(prov, nil) diff --git a/internal/purchase/reaper_ordering_test.go b/internal/purchase/reaper_ordering_test.go index b1531c4de..596e31c46 100644 --- a/internal/purchase/reaper_ordering_test.go +++ b/internal/purchase/reaper_ordering_test.go @@ -26,6 +26,7 @@ func listedThresholds(t *testing.T, reapAfter time.Duration) map[string]time.Dur t.Helper() ctx := context.Background() store := new(MockConfigStore) + assertMocksOnCleanup(t, store) got := map[string]time.Duration{} store.On("ListStuckExecutions", ctx, mock.Anything, mock.Anything). Run(func(args mock.Arguments) { @@ -129,13 +130,16 @@ func expectStuckLists(ctx context.Context, store *MockConfigStore, reapAfter tim func TestReapStuckExecutions_ApprovedRowClaimedBeforeCASIsNotFailed(t *testing.T) { ctx := context.Background() store := new(MockConfigStore) + assertMocksOnCleanup(t, store) row := stuckExec("exec-claimed", "approved") flipped := row flipped.Status = failedStatus expectStuckLists(ctx, store, DefaultReapAfter, row) + // The bug path: a CAS that also matches "running" fails the in-flight row. + // Must not fire (Reaped 0 below), hence .Maybe(). store.On("TransitionExecutionStatus", ctx, row.ExecutionID, mock.MatchedBy(func(from []string) bool { return slices.Contains(from, "running") }), - failedStatus, (*string)(nil)).Return(&flipped, nil) + failedStatus, (*string)(nil)).Return(&flipped, nil).Maybe() store.On("TransitionExecutionStatus", ctx, row.ExecutionID, mock.MatchedBy(func(from []string) bool { return !slices.Contains(from, "running") }), failedStatus, (*string)(nil)). diff --git a/internal/purchase/reaper_test.go b/internal/purchase/reaper_test.go index a9d95aea6..97da30787 100644 --- a/internal/purchase/reaper_test.go +++ b/internal/purchase/reaper_test.go @@ -98,6 +98,7 @@ func TestReapStuckExecutions_StaleApprovedFlippedToFailed(t *testing.T) { func TestReapStuckExecutions_EC2RowIsNotAdvertisedAsSafeToRetry(t *testing.T) { ctx := context.Background() store := new(MockConfigStore) + assertMocksOnCleanup(t, store) reapAfter := 10 * time.Minute row := stuckExecWithAWSEC2Rec("exec-ec2", "running") diff --git a/internal/purchase/redrive_claimed_row_test.go b/internal/purchase/redrive_claimed_row_test.go index 9ce40c3c3..aac77b02c 100644 --- a/internal/purchase/redrive_claimed_row_test.go +++ b/internal/purchase/redrive_claimed_row_test.go @@ -64,6 +64,7 @@ func TestRecoverStrandedApprovals_RedrivesClaimedRowNotSnapshot(t *testing.T) { stsClient := new(MockSTSClient) factory := new(MockProviderFactory) prov := new(MockProvider) + assertMocksOnCleanup(t, emailSender, stsClient, factory, prov) client := new(MockServiceClient) t.Cleanup(func() { store.AssertExpectations(t) }) t.Cleanup(func() { client.AssertExpectations(t) }) diff --git a/internal/purchase/requested_at_test.go b/internal/purchase/requested_at_test.go index d7b3f2c02..c1fdd803d 100644 --- a/internal/purchase/requested_at_test.go +++ b/internal/purchase/requested_at_test.go @@ -27,6 +27,7 @@ func runFanOutCapturingOpts(t *testing.T, service common.ServiceType, mk func(st mockFactory := new(MockProviderFactory) mockProviderInst := new(MockProvider) mockServiceClient := new(MockServiceClient) + assertMocksOnCleanup(t, mockStore, mockEmail, mockFactory, mockProviderInst, mockServiceClient) exec := &config.PurchaseExecution{ ExecutionID: "root-exec", IdempotencyKey: "root-lineage", PlanID: "plan-x", @@ -104,6 +105,7 @@ func TestExecuteSinglePurchase_EmptyTokenOptsOtherwiseUntouched(t *testing.T) { mockFactory := new(MockProviderFactory) mockProviderInst := new(MockProvider) mockServiceClient := new(MockServiceClient) + assertMocksOnCleanup(t, mockFactory, mockProviderInst, mockServiceClient) mockFactory.On("CreateAndValidateProvider", mock.Anything, "aws", mock.Anything).Return(mockProviderInst, nil) mockProviderInst.On("GetServiceClient", mock.Anything, common.ServiceEC2, mock.Anything).Return(mockServiceClient, nil) var got common.PurchaseOptions diff --git a/internal/purchase/scheduled_fire_test.go b/internal/purchase/scheduled_fire_test.go index de9363768..e42f6fe50 100644 --- a/internal/purchase/scheduled_fire_test.go +++ b/internal/purchase/scheduled_fire_test.go @@ -172,6 +172,7 @@ func TestFireScheduledDelayedPurchases_EndToEnd(t *testing.T) { mockEmail := new(MockEmailSender) mockFactory := new(MockProviderFactory) mockProviderInst := new(MockProvider) + assertMocksOnCleanup(t, store, mockEmail, mockFactory, mockProviderInst) mockServiceClient := new(MockServiceClient) const acctID = "acct-fire" @@ -252,6 +253,7 @@ func TestFireScheduledDelayedPurchases_AuditGapAfterCASStillFires(t *testing.T) mockEmail := new(MockEmailSender) mockFactory := new(MockProviderFactory) mockProviderInst := new(MockProvider) + assertMocksOnCleanup(t, store, mockEmail, mockFactory, mockProviderInst) mockServiceClient := new(MockServiceClient) const acctID = "acct-auditgap" diff --git a/internal/purchase/step_cap_test.go b/internal/purchase/step_cap_test.go index ca35a40ba..e46e8243b 100644 --- a/internal/purchase/step_cap_test.go +++ b/internal/purchase/step_cap_test.go @@ -73,10 +73,11 @@ func gateHarness(t *testing.T, plan *config.PurchasePlan, exec config.PurchaseEx t.Helper() ctx := context.Background() store := new(MockConfigStore) + assertMocksOnCleanup(t, store) store.On("GetStaleApprovedExecutions", ctx, mock.Anything).Return([]config.PurchaseExecution{}, nil) store.On("GetPendingExecutions", ctx).Return([]config.PurchaseExecution{exec}, nil) store.On("GetPurchasePlan", ctx, exec.PlanID).Return(plan, nil) - store.On("GetGlobalConfig", ctx).Return(cfg, cfgErr) + expectKillSwitchRead(ctx, store, plan, cfg, cfgErr) manager := &Manager{config: store, email: new(MockEmailSender)} result, err := manager.ProcessScheduledPurchases(ctx) require.NoError(t, err) @@ -84,6 +85,16 @@ func gateHarness(t *testing.T, plan *config.PurchasePlan, exec config.PurchaseEx return result } +// expectKillSwitchRead registers the gate's global-config read. The gate reads +// the plan first and refuses a missing one before the kill switch is consulted, +// so that read is only required when the plan exists. +func expectKillSwitchRead(ctx context.Context, store *MockConfigStore, plan *config.PurchasePlan, cfg *config.GlobalConfig, cfgErr error) { + call := store.On("GetGlobalConfig", ctx).Return(cfg, cfgErr) + if plan == nil { + call.Maybe() + } +} + func dueRecExec(status string, recs ...config.RecommendationRecord) config.PurchaseExecution { return config.PurchaseExecution{ ExecutionID: "exec-gate", PlanID: "plan-gate", Status: status, StepNumber: 1, @@ -148,10 +159,11 @@ func TestSQSExecutePath_RefusesBeforeClaim(t *testing.T) { t.Run(tc.name, func(t *testing.T) { ctx := context.Background() store := new(MockConfigStore) + assertMocksOnCleanup(t, store) exec := tc.exec store.On("GetExecutionByID", ctx, exec.ExecutionID).Return(&exec, nil) store.On("GetPurchasePlan", ctx, exec.PlanID).Return(tc.plan, nil) - store.On("GetGlobalConfig", ctx).Return(tc.cfg, tc.cfgErr) + expectKillSwitchRead(ctx, store, tc.plan, tc.cfg, tc.cfgErr) manager := &Manager{config: store, email: new(MockEmailSender)} err := manager.handleExecutePurchase(ctx, &AsyncMessage{Type: MessageTypeExecutePurchase, ExecutionID: exec.ExecutionID}) @@ -168,6 +180,7 @@ func TestSQSExecutePath_RefusesBeforeClaim(t *testing.T) { func TestAutoExecuteGate_ApprovedRowIgnoresKillSwitchAndCap(t *testing.T) { ctx := context.Background() store := new(MockConfigStore) // no GetGlobalConfig / GetPurchasePlan expectation: any read panics + assertMocksOnCleanup(t, store) manager := &Manager{config: store, email: new(MockEmailSender)} exec := dueRecExec("approved", upfrontRec(1e9)) require.NoError(t, manager.checkAutoExecuteGate(ctx, &exec)) @@ -178,6 +191,7 @@ func TestAutoExecuteGate_ApprovedRowIgnoresKillSwitchAndCap(t *testing.T) { // A web-submitted direct purchase (no plan) never reaches the plan gate. func TestExecutableByScheduler_WebRowNeverReadsKillSwitch(t *testing.T) { store := new(MockConfigStore) + assertMocksOnCleanup(t, store) manager := &Manager{config: store} exec := config.PurchaseExecution{ExecutionID: "w", Status: "pending", Source: "cudly-web", Recommendations: []config.RecommendationRecord{upfrontRec(1)}} ok, err := manager.executableByScheduler(context.Background(), &exec) @@ -190,6 +204,7 @@ func TestExecutableByScheduler_WebRowNeverReadsKillSwitch(t *testing.T) { func TestExecutableByScheduler_NonAutoPlanNeverReadsKillSwitch(t *testing.T) { ctx := context.Background() store := new(MockConfigStore) + assertMocksOnCleanup(t, store) store.On("GetPurchasePlan", ctx, "plan-gate").Return(&config.PurchasePlan{ID: "plan-gate"}, nil) manager := &Manager{config: store} exec := dueRecExec("notified", upfrontRec(1)) @@ -214,6 +229,7 @@ func TestSendUpcomingPurchaseNotifications_KillSwitch(t *testing.T) { t.Run("off sends nothing and touches no plan", func(t *testing.T) { ctx := context.Background() store := new(MockConfigStore) + assertMocksOnCleanup(t, store) store.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return([]config.PurchasePlan{plan}, nil) store.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{}, nil) manager := &Manager{config: store, email: new(MockEmailSender)} @@ -226,6 +242,7 @@ func TestSendUpcomingPurchaseNotifications_KillSwitch(t *testing.T) { t.Run("read error fails the tick", func(t *testing.T) { ctx := context.Background() store := new(MockConfigStore) + assertMocksOnCleanup(t, store) store.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return([]config.PurchasePlan{plan}, nil) store.On("GetGlobalConfig", ctx).Return(nil, errors.New("db down")) manager := &Manager{config: store, email: new(MockEmailSender)} @@ -240,11 +257,14 @@ func TestSendUpcomingPurchaseNotifications_KillSwitch(t *testing.T) { func TestScheduler_DirectExecuteRowSkippedWithoutPlanLookup(t *testing.T) { ctx := context.Background() store := new(MockConfigStore) + assertMocksOnCleanup(t, store) exec := dueRecExec("pending", upfrontRec(10)) exec.PlanID = "" store.On("GetStaleApprovedExecutions", ctx, mock.Anything).Return([]config.PurchaseExecution{}, nil) store.On("GetPendingExecutions", ctx).Return([]config.PurchaseExecution{exec}, nil) - store.On("GetPurchasePlan", ctx, "").Return(nil, errors.New(`ERROR: invalid input syntax for type uuid: "" (SQLSTATE 22P02)`)) + // Must NOT be called (AssertNotCalled below); the stub only makes a + // regression fail the way Postgres would, hence .Maybe(). + store.On("GetPurchasePlan", ctx, "").Return(nil, errors.New(`ERROR: invalid input syntax for type uuid: "" (SQLSTATE 22P02)`)).Maybe() manager := &Manager{config: store, email: new(MockEmailSender)} result, err := manager.ProcessScheduledPurchases(ctx) diff --git a/internal/purchase/zonal_az_test.go b/internal/purchase/zonal_az_test.go index 86f6e480f..c79a21148 100644 --- a/internal/purchase/zonal_az_test.go +++ b/internal/purchase/zonal_az_test.go @@ -30,11 +30,12 @@ func zonalRecRecord(t *testing.T, az string) config.RecommendationRecord { } } -func managerServing(sc provider.ServiceClient) *Manager { +func managerServing(t *testing.T, sc provider.ServiceClient) *Manager { prov := new(MockProvider) prov.On("GetServiceClient", mock.Anything, mock.Anything, "us-east-1").Return(sc, nil) factory := new(MockProviderFactory) factory.On("CreateAndValidateProvider", mock.Anything, "aws", mock.Anything).Return(prov, nil) + assertMocksOnCleanup(t, prov, factory) return &Manager{providerFactory: factory} } @@ -42,12 +43,13 @@ func managerServing(sc provider.ServiceClient) *Manager { // (persisted blob -> decode -> PurchaseCommitment). func TestExecuteSinglePurchase_ZonalRecKeepsAvailabilityZone(t *testing.T) { sc := new(MockServiceClient) + assertMocksOnCleanup(t, sc) var got common.Recommendation sc.On("PurchaseCommitment", mock.Anything, mock.Anything, mock.Anything). Run(func(args mock.Arguments) { got = args.Get(1).(common.Recommendation) }). Return(common.PurchaseResult{Success: true, CommitmentID: "ri-1"}, nil) - _, err := managerServing(sc).executeSinglePurchase(context.Background(), zonalRecRecord(t, "us-east-1b"), nil, common.PurchaseOptions{ExecutionID: "exec-az"}) + _, err := managerServing(t, sc).executeSinglePurchase(context.Background(), zonalRecRecord(t, "us-east-1b"), nil, common.PurchaseOptions{ExecutionID: "exec-az"}) require.NoError(t, err) d, ok := got.Details.(*common.ComputeDetails) @@ -79,7 +81,7 @@ func TestExecuteSinglePurchase_StaleZonalRowWithoutAZFailsClosed(t *testing.T) { c := ec2svc.NewClient(awsConfigForTest()) c.SetEC2API(api) - _, err := managerServing(c).executeSinglePurchase(context.Background(), zonalRecRecord(t, ""), nil, common.PurchaseOptions{ExecutionID: "exec-stale"}) + _, err := managerServing(t, c).executeSinglePurchase(context.Background(), zonalRecRecord(t, ""), nil, common.PurchaseOptions{ExecutionID: "exec-stale"}) require.Error(t, err) assert.Contains(t, err.Error(), "re-fetch") assert.Zero(t, api.purchases, "no purchase call") From fbafeb0816bff9ddbbfc95a79f74ec577d4c4dda Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Wed, 28 Oct 2026 03:46:49 +0000 Subject: [PATCH 2/2] test(purchase): make the kill-switch read optional for a manual plan in gateHarness Main's TestProcessScheduledPurchases_CapNeverTouchesRowsAwaitingApproval (#1549) drives gateHarness with an AutoPurchase=false plan. The gate skips a manual plan before it reads the global config, so the required GetGlobalConfig expectation failed under assertMocksOnCleanup. Co-Authored-By: Claude Opus 5.5 --- internal/purchase/step_cap_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/purchase/step_cap_test.go b/internal/purchase/step_cap_test.go index e46e8243b..6b07225a3 100644 --- a/internal/purchase/step_cap_test.go +++ b/internal/purchase/step_cap_test.go @@ -86,11 +86,12 @@ func gateHarness(t *testing.T, plan *config.PurchasePlan, exec config.PurchaseEx } // expectKillSwitchRead registers the gate's global-config read. The gate reads -// the plan first and refuses a missing one before the kill switch is consulted, -// so that read is only required when the plan exists. +// the plan first and refuses a missing one, or skips a manual (AutoPurchase +// off) one, before the kill switch is consulted, so that read is only required +// when the plan exists and is armed. func expectKillSwitchRead(ctx context.Context, store *MockConfigStore, plan *config.PurchasePlan, cfg *config.GlobalConfig, cfgErr error) { call := store.On("GetGlobalConfig", ctx).Return(cfg, cfgErr) - if plan == nil { + if plan == nil || !plan.AutoPurchase { call.Maybe() } }