From d33c52b7745b693afb81baca6ace6c1283f7e20f Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 8 Aug 2026 00:26:54 +0200 Subject: [PATCH 1/3] fix(test): record every MockConfigStore call so AssertNotCalled can fail MockConfigStore serves many methods from a hardcoded default without reaching mock.Called: the isExpected short-circuit when no expectation is registered, and the Fn-override fields. testify appends to mock.Mock.Calls from inside mock.Called, so those invocations never landed there, and any assertion reading that slice was structurally incapable of failing. AssertNotCalled walked a slice the call could not have reached and reported success no matter what the code under test did. A second, independent cause compounds it: testify diffs an empty expectation against the real arguments and counts each one as a difference, so the name-only form AssertNotCalled(t, "WithTx") never matches a method that takes parameters. 18 of the affected sites were unfailable both ways, so recording alone would not have made them bite. Every MockConfigStore method now records its own invocation in a callLog before branching, and AssertCalled / AssertNotCalled / AssertNumberOfCalls shadow the promoted testify methods to read that log. Naming a method with no arguments now means "not called at all", which is the only thing that form can usefully assert. Argument matchers, mock.Anything, registered expectations and AssertExpectations are unchanged. This repairs 77 assertion sites across internal/api, internal/purchase and internal/scheduler that could not fail: 46 blocked by the short-circuit, 31 by the empty-expectation diff. Closes #1595 --- internal/mocks/assertions.go | 133 ++++++++++++++++++++++++++ internal/mocks/assertions_test.go | 153 ++++++++++++++++++++++++++++++ internal/mocks/stores.go | 118 +++++++++++++++++++++++ 3 files changed, 404 insertions(+) create mode 100644 internal/mocks/assertions.go create mode 100644 internal/mocks/assertions_test.go diff --git a/internal/mocks/assertions.go b/internal/mocks/assertions.go new file mode 100644 index 000000000..b1dd1b5fb --- /dev/null +++ b/internal/mocks/assertions.go @@ -0,0 +1,133 @@ +package mocks + +import ( + "fmt" + "strings" + "sync" + + "github.com/stretchr/testify/mock" +) + +// recordedCall is one invocation of a MockConfigStore method, captured whether +// or not the method dispatched through mock.Called. +type recordedCall struct { + method string + args mock.Arguments +} + +// callLog captures every MockConfigStore invocation so the assertion helpers +// below can see what the code under test actually did. +// +// testify appends to mock.Mock.Calls from inside mock.Called. A MockConfigStore +// method that serves a hardcoded default -- the isExpected short-circuit for an +// unregistered method, or an Fn override -- returns before reaching it, so the +// invocation never lands in mock.Mock.Calls. Any assertion reading that slice is +// then unfailable for such a method: AssertNotCalled walks a slice the call +// could not have reached, and reports success no matter what happened. +type callLog struct { + mu sync.Mutex + calls []recordedCall +} + +func (l *callLog) record(method string, args ...interface{}) { + l.mu.Lock() + defer l.mu.Unlock() + l.calls = append(l.calls, recordedCall{method: method, args: args}) +} + +// matching counts the recorded invocations of method whose arguments satisfy +// expected. An empty expected matches every invocation of the method. +// +// testify instead diffs an empty expectation against the real arguments and +// counts each one as a difference, so its name-only form never matches a method +// that takes parameters -- a second, independent way for an assertion to become +// unfailable. Naming the method is the only way to express "not called at all", +// so that is what it means here. +func (l *callLog) matching(method string, expected []interface{}) int { + l.mu.Lock() + defer l.mu.Unlock() + n := 0 + for _, c := range l.calls { + if c.method != method { + continue + } + if len(expected) == 0 { + n++ + continue + } + if _, diffs := mock.Arguments(expected).Diff(c.args); diffs == 0 { + n++ + } + } + return n +} + +// summary renders the recorded invocations of method for failure messages. +func (l *callLog) summary(method string) string { + l.mu.Lock() + defer l.mu.Unlock() + var b strings.Builder + for _, c := range l.calls { + if c.method == method { + fmt.Fprintf(&b, "\n\t%s(%v)", c.method, []interface{}(c.args)) + } + } + if b.Len() == 0 { + return "\n\t(no recorded calls to " + method + ")" + } + return b.String() +} + +// record captures an invocation. Every MockConfigStore method calls it before +// branching, so the assertion helpers below never depend on whether the method +// reached mock.Called. +func (m *MockConfigStore) record(method string, args ...interface{}) { + m.callLog.record(method, args...) +} + +func markHelper(t mock.TestingT) { + if h, ok := t.(interface{ Helper() }); ok { + h.Helper() + } +} + +// AssertCalled asserts that method was invoked with arguments satisfying the +// given matchers. It shadows the promoted testify method so it reads the +// complete call log rather than mock.Mock.Calls; pass no arguments to assert +// the method was called at all. +func (m *MockConfigStore) AssertCalled(t mock.TestingT, method string, arguments ...interface{}) bool { + markHelper(t) + if m.callLog.matching(method, arguments) > 0 { + return true + } + t.Errorf("mock: %s was not called with the expected arguments %v. Recorded calls:%s", + method, arguments, m.callLog.summary(method)) + return false +} + +// AssertNotCalled asserts that method was never invoked with arguments +// satisfying the given matchers. It shadows the promoted testify method so it +// reads the complete call log rather than mock.Mock.Calls; pass no arguments to +// assert the method was not called at all. +func (m *MockConfigStore) AssertNotCalled(t mock.TestingT, method string, arguments ...interface{}) bool { + markHelper(t) + if n := m.callLog.matching(method, arguments); n > 0 { + t.Errorf("mock: %s was called %d time(s) matching %v, expected none. Recorded calls:%s", + method, n, arguments, m.callLog.summary(method)) + return false + } + return true +} + +// AssertNumberOfCalls asserts how many times method was invoked, with any +// arguments. It shadows the promoted testify method so it reads the complete +// call log rather than mock.Mock.Calls. +func (m *MockConfigStore) AssertNumberOfCalls(t mock.TestingT, method string, expectedCalls int) bool { + markHelper(t) + if actual := m.callLog.matching(method, nil); actual != expectedCalls { + t.Errorf("mock: expected %d call(s) to %s, got %d. Recorded calls:%s", + expectedCalls, method, actual, m.callLog.summary(method)) + return false + } + return true +} diff --git a/internal/mocks/assertions_test.go b/internal/mocks/assertions_test.go new file mode 100644 index 000000000..600b51ec2 --- /dev/null +++ b/internal/mocks/assertions_test.go @@ -0,0 +1,153 @@ +package mocks + +import ( + "context" + "testing" + + "github.com/LeanerCloud/CUDly/internal/config" + "github.com/jackc/pgx/v5" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +// recordingT captures assertion failures without failing the enclosing test, so +// these tests can assert that an assertion helper *fails*. +type recordingT struct { + failed bool +} + +func (r *recordingT) Logf(string, ...interface{}) {} + +func (r *recordingT) Errorf(string, ...interface{}) { r.failed = true } + +func (r *recordingT) FailNow() { r.failed = true } + +var _ mock.TestingT = (*recordingT)(nil) + +// TestAssertNotCalled_SeesDefaultServedCall is the regression test for #1595. +// GetGlobalConfig has no registered expectation, so it serves its hardcoded +// default and never reaches mock.Called. Before the fix, mock.Mock.Calls stayed +// empty and AssertNotCalled reported success even though the call happened. +func TestAssertNotCalled_SeesDefaultServedCall(t *testing.T) { + m := &MockConfigStore{} + _, err := m.GetGlobalConfig(context.Background()) + require.NoError(t, err) + + rt := &recordingT{} + assert.False(t, m.AssertNotCalled(rt, "GetGlobalConfig", mock.Anything), + "AssertNotCalled must fail: GetGlobalConfig was called") + assert.True(t, rt.failed, "the failure must be reported to the TestingT") +} + +// TestAssertNotCalled_SeesFnOverriddenCall covers the other path that bypasses +// mock.Called: an Fn override. +func TestAssertNotCalled_SeesFnOverriddenCall(t *testing.T) { + m := &MockConfigStore{ + GetUserEmailByIDFn: func(context.Context, string) (string, error) { return "a@b.c", nil }, + } + email, err := m.GetUserEmailByID(context.Background(), "user-1") + require.NoError(t, err) + require.Equal(t, "a@b.c", email) + + rt := &recordingT{} + assert.False(t, m.AssertNotCalled(rt, "GetUserEmailByID", mock.Anything, "user-1"), + "AssertNotCalled must fail: the Fn override served the call") + assert.True(t, rt.failed) +} + +// TestAssertNotCalled_NameOnlyFormMatchesCallWithArguments pins the second way +// these assertions were unfailable: testify diffs an empty expectation against +// the real arguments and counts each as a difference, so the name-only form +// never matched a method that takes parameters. +func TestAssertNotCalled_NameOnlyFormMatchesCallWithArguments(t *testing.T) { + m := &MockConfigStore{} + require.NoError(t, m.WithTx(context.Background(), func(pgx.Tx) error { return nil })) + + rt := &recordingT{} + assert.False(t, m.AssertNotCalled(rt, "WithTx"), + "the name-only form must fail when the method was called") + assert.True(t, rt.failed) + + // testify's promoted implementation is the behavior being replaced: it + // still passes, because WithTx never reached mock.Called and because an + // empty expectation cannot match a two-argument call. + assert.True(t, m.Mock.AssertNotCalled(&recordingT{}, "WithTx")) +} + +// TestAssertNotCalled_PassesWhenNotCalled guards against the opposite failure: +// an assertion that always fails is no more useful than one that never does. +func TestAssertNotCalled_PassesWhenNotCalled(t *testing.T) { + m := &MockConfigStore{} + rt := &recordingT{} + assert.True(t, m.AssertNotCalled(rt, "WithTx")) + assert.True(t, m.AssertNotCalled(rt, "GetGlobalConfig", mock.Anything)) + assert.False(t, rt.failed) +} + +// TestAssertNotCalled_DiscriminatesOnArguments confirms argument matching still +// narrows the assertion rather than being ignored. +func TestAssertNotCalled_DiscriminatesOnArguments(t *testing.T) { + m := &MockConfigStore{} + _, err := m.GetPurchasePlan(context.Background(), "plan-a") + require.NoError(t, err) + + rt := &recordingT{} + assert.True(t, m.AssertNotCalled(rt, "GetPurchasePlan", mock.Anything, "plan-b"), + "a call with different arguments must not satisfy the matcher") + assert.False(t, rt.failed) + + assert.False(t, m.AssertNotCalled(rt, "GetPurchasePlan", mock.Anything, "plan-a")) + assert.True(t, rt.failed) +} + +// TestAssertCalled_SeesDefaultServedCall covers the mirror image: a call served +// from a default previously could not satisfy AssertCalled either. +func TestAssertCalled_SeesDefaultServedCall(t *testing.T) { + m := &MockConfigStore{} + _, err := m.GetPurchasePlan(context.Background(), "plan-a") + require.NoError(t, err) + + rt := &recordingT{} + assert.True(t, m.AssertCalled(rt, "GetPurchasePlan", mock.Anything, "plan-a")) + assert.False(t, rt.failed) + + assert.False(t, m.AssertCalled(rt, "GetPurchasePlan", mock.Anything, "plan-missing")) + assert.True(t, rt.failed) +} + +// TestAssertNumberOfCalls_CountsDefaultServedCalls checks the count-based form +// reads the same log, and does not double count a call that also reached +// mock.Called via a registered expectation. +func TestAssertNumberOfCalls_CountsDefaultServedCalls(t *testing.T) { + m := &MockConfigStore{} + for i := 0; i < 3; i++ { + _, err := m.GetPurchasePlan(context.Background(), "plan-a") + require.NoError(t, err) + } + rt := &recordingT{} + assert.True(t, m.AssertNumberOfCalls(rt, "GetPurchasePlan", 3)) + assert.False(t, rt.failed) + + registered := &MockConfigStore{} + registered.On("GetPurchasePlan", mock.Anything, "plan-a"). + Return(&config.PurchasePlan{ID: "plan-a"}, nil) + _, err := registered.GetPurchasePlan(context.Background(), "plan-a") + require.NoError(t, err) + assert.True(t, registered.AssertNumberOfCalls(&recordingT{}, "GetPurchasePlan", 1), + "a call that also reached mock.Called must be counted once, not twice") +} + +// TestRegisteredExpectationsStillDispatch confirms the recording does not +// disturb the existing default-or-dispatch behavior or AssertExpectations. +func TestRegisteredExpectationsStillDispatch(t *testing.T) { + m := &MockConfigStore{} + m.On("GetGlobalConfig", mock.Anything). + Return(&config.GlobalConfig{}, nil) + + cfg, err := m.GetGlobalConfig(context.Background()) + require.NoError(t, err) + require.NotNil(t, cfg) + m.AssertExpectations(t) + m.AssertCalled(t, "GetGlobalConfig", mock.Anything) +} diff --git a/internal/mocks/stores.go b/internal/mocks/stores.go index 37c25b82c..01b3975f1 100644 --- a/internal/mocks/stores.go +++ b/internal/mocks/stores.go @@ -31,6 +31,12 @@ const MockOwnerToken = "mock-owner-token" // 1. FnField (non-nil closure wins first) // 2. Registered .On() expectation (dispatches through m.Called) // 3. Hardcoded default (zero-value / sensible stub) +// +// Because paths 1 and 3 never reach m.Called, every method records its own +// invocation in callLog before branching, and AssertCalled / AssertNotCalled / +// AssertNumberOfCalls read that log instead of mock.Mock.Calls. See +// assertions.go for why reading mock.Mock.Calls made those assertions +// unfailable. type MockConfigStore struct { GetPurchasePlanFn func(ctx context.Context, planID string) (*config.PurchasePlan, error) GetCloudAccountFn func(ctx context.Context, id string) (*config.CloudAccount, error) @@ -46,12 +52,16 @@ type MockConfigStore struct { SavePurchaseExecutionFn func(ctx context.Context, exec *config.PurchaseExecution) error GetUserEmailByIDFn func(ctx context.Context, userID string) (string, error) mock.Mock + // callLog records every invocation, including the ones served from a + // default without reaching mock.Called. See assertions.go. + callLog callLog } // GetGlobalConfig mocks the GetGlobalConfig operation. Returns an empty // GlobalConfig when no expectation is registered so callers that only // need default field values don't require explicit mock setup. func (m *MockConfigStore) GetGlobalConfig(ctx context.Context) (*config.GlobalConfig, error) { + m.record("GetGlobalConfig", ctx) if !isExpected(&m.Mock, "GetGlobalConfig") { return &config.GlobalConfig{}, nil } @@ -68,6 +78,7 @@ func (m *MockConfigStore) GetGlobalConfig(ctx context.Context) (*config.GlobalCo // SaveGlobalConfig mocks the SaveGlobalConfig operation. func (m *MockConfigStore) SaveGlobalConfig(ctx context.Context, cfg *config.GlobalConfig) error { + m.record("SaveGlobalConfig", ctx, cfg) args := m.Called(ctx, cfg) return args.Error(0) } @@ -78,6 +89,7 @@ func (m *MockConfigStore) SaveGlobalConfig(ctx context.Context, cfg *config.Glob // read-modify-write by routing through the mocked GetGlobalConfig + apply + // SaveGlobalConfig, so existing tests that seed those two continue to work. func (m *MockConfigStore) UpdateGlobalConfigAtomic(ctx context.Context, apply func(*config.GlobalConfig) error) (*config.GlobalConfig, error) { + m.record("UpdateGlobalConfigAtomic", ctx, apply) if isExpected(&m.Mock, "UpdateGlobalConfigAtomic") { args := m.Called(ctx, apply) if args.Get(0) == nil { @@ -107,6 +119,7 @@ func (m *MockConfigStore) UpdateGlobalConfigAtomic(ctx context.Context, apply fu // GetServiceConfig mocks the GetServiceConfig operation. func (m *MockConfigStore) GetServiceConfig(ctx context.Context, provider, service string) (*config.ServiceConfig, error) { + m.record("GetServiceConfig", ctx, provider, service) args := m.Called(ctx, provider, service) if args.Get(0) == nil { return nil, args.Error(1) @@ -120,12 +133,14 @@ func (m *MockConfigStore) GetServiceConfig(ctx context.Context, provider, servic // SaveServiceConfig mocks the SaveServiceConfig operation. func (m *MockConfigStore) SaveServiceConfig(ctx context.Context, cfg *config.ServiceConfig) error { + m.record("SaveServiceConfig", ctx, cfg) args := m.Called(ctx, cfg) return args.Error(0) } // ListServiceConfigs mocks the ListServiceConfigs operation. func (m *MockConfigStore) ListServiceConfigs(ctx context.Context) ([]config.ServiceConfig, error) { + m.record("ListServiceConfigs", ctx) args := m.Called(ctx) if args.Get(0) == nil { return nil, args.Error(1) @@ -139,6 +154,7 @@ func (m *MockConfigStore) ListServiceConfigs(ctx context.Context) ([]config.Serv // CreatePurchasePlan mocks the CreatePurchasePlan operation. func (m *MockConfigStore) CreatePurchasePlan(ctx context.Context, plan *config.PurchasePlan) error { + m.record("CreatePurchasePlan", ctx, plan) args := m.Called(ctx, plan) return args.Error(0) } @@ -148,6 +164,7 @@ func (m *MockConfigStore) CreatePurchasePlan(ctx context.Context, plan *config.P // stub {ID: planID} is returned so tests that don't care about the plan fields // keep working without explicit setup. func (m *MockConfigStore) GetPurchasePlan(ctx context.Context, planID string) (*config.PurchasePlan, error) { + m.record("GetPurchasePlan", ctx, planID) if m.GetPurchasePlanFn != nil { return m.GetPurchasePlanFn(ctx, planID) } @@ -167,12 +184,14 @@ func (m *MockConfigStore) GetPurchasePlan(ctx context.Context, planID string) (* // UpdatePurchasePlan mocks the UpdatePurchasePlan operation. func (m *MockConfigStore) UpdatePurchasePlan(ctx context.Context, plan *config.PurchasePlan) error { + m.record("UpdatePurchasePlan", ctx, plan) args := m.Called(ctx, plan) return args.Error(0) } // IncrementPlanCurrentStep mocks the atomic step-advance operation. func (m *MockConfigStore) IncrementPlanCurrentStep(ctx context.Context, planID string) error { + m.record("IncrementPlanCurrentStep", ctx, planID) args := m.Called(ctx, planID) return args.Error(0) } @@ -182,6 +201,7 @@ func (m *MockConfigStore) IncrementPlanCurrentStep(ctx context.Context, planID s // that don't care about the Tx variant stay green -- same pattern as // SavePurchaseExecutionTx below. func (m *MockConfigStore) UpdatePurchasePlanTx(ctx context.Context, tx pgx.Tx, plan *config.PurchasePlan) error { + m.record("UpdatePurchasePlanTx", ctx, tx, plan) if !isExpected(&m.Mock, "UpdatePurchasePlanTx") { return m.UpdatePurchasePlan(ctx, plan) } @@ -191,12 +211,14 @@ func (m *MockConfigStore) UpdatePurchasePlanTx(ctx context.Context, tx pgx.Tx, p // DeletePurchasePlan mocks the DeletePurchasePlan operation. func (m *MockConfigStore) DeletePurchasePlan(ctx context.Context, planID string) error { + m.record("DeletePurchasePlan", ctx, planID) args := m.Called(ctx, planID) return args.Error(0) } // ListPurchasePlans mocks the ListPurchasePlans operation. func (m *MockConfigStore) ListPurchasePlans(ctx context.Context, filter config.PurchasePlanFilter) ([]config.PurchasePlan, error) { + m.record("ListPurchasePlans", ctx, filter) args := m.Called(ctx, filter) if args.Get(0) == nil { return nil, args.Error(1) @@ -211,6 +233,7 @@ func (m *MockConfigStore) ListPurchasePlans(ctx context.Context, filter config.P // SavePurchaseExecution mocks the SavePurchaseExecution operation. // SavePurchaseExecutionFn takes priority when non-nil. func (m *MockConfigStore) SavePurchaseExecution(ctx context.Context, exec *config.PurchaseExecution) error { + m.record("SavePurchaseExecution", ctx, exec) if m.SavePurchaseExecutionFn != nil { return m.SavePurchaseExecutionFn(ctx, exec) } @@ -220,6 +243,7 @@ func (m *MockConfigStore) SavePurchaseExecution(ctx context.Context, exec *confi // TransitionExecutionStatus mocks the TransitionExecutionStatus operation. func (m *MockConfigStore) TransitionExecutionStatus(ctx context.Context, executionID string, fromStatuses []string, toStatus string, actor *string) (*config.PurchaseExecution, error) { + m.record("TransitionExecutionStatus", ctx, executionID, fromStatuses, toStatus, actor) args := m.Called(ctx, executionID, fromStatuses, toStatus, actor) if args.Get(0) == nil { return nil, args.Error(1) @@ -233,6 +257,7 @@ func (m *MockConfigStore) TransitionExecutionStatus(ctx context.Context, executi // SetCancelledBy mocks the SetCancelledBy targeted-update operation. func (m *MockConfigStore) SetCancelledBy(ctx context.Context, executionID, cancelledBy string) error { + m.record("SetCancelledBy", ctx, executionID, cancelledBy) args := m.Called(ctx, executionID, cancelledBy) return args.Error(0) } @@ -243,6 +268,7 @@ func (m *MockConfigStore) SetCancelledBy(ctx context.Context, executionID, cance // Tests exercising the CAS-race path (zero rows affected) register an // expectation that returns (false, , nil). func (m *MockConfigStore) CancelExecutionAtomic(ctx context.Context, tx pgx.Tx, executionID string, cancelledBy *string) (bool, string, error) { //nolint:gocritic // unnamedResult: return names would conflict with body locals + m.record("CancelExecutionAtomic", ctx, tx, executionID, cancelledBy) if !isExpected(&m.Mock, "CancelExecutionAtomic") { return true, "cancelled", nil } @@ -258,6 +284,7 @@ func (m *MockConfigStore) CancelExecutionAtomic(ctx context.Context, tx pgx.Tx, // an expectation that returns (false, , nil), typically // (false, "approved", nil) to simulate the scheduler winning the race. func (m *MockConfigStore) CancelScheduledExecutionAtomic(ctx context.Context, tx pgx.Tx, executionID string, cancelledBy *string) (bool, string, error) { //nolint:gocritic // unnamedResult: return names would conflict with body locals + m.record("CancelScheduledExecutionAtomic", ctx, tx, executionID, cancelledBy) if !isExpected(&m.Mock, "CancelScheduledExecutionAtomic") { return true, "cancelled", nil } @@ -267,6 +294,7 @@ func (m *MockConfigStore) CancelScheduledExecutionAtomic(ctx context.Context, tx // GetPendingExecutions mocks the GetPendingExecutions operation. func (m *MockConfigStore) GetPendingExecutions(ctx context.Context) ([]config.PurchaseExecution, error) { + m.record("GetPendingExecutions", ctx) args := m.Called(ctx) if args.Get(0) == nil { return nil, args.Error(1) @@ -280,6 +308,7 @@ func (m *MockConfigStore) GetPendingExecutions(ctx context.Context) ([]config.Pu // GetExecutionByID mocks the GetExecutionByID operation. func (m *MockConfigStore) GetExecutionByID(ctx context.Context, executionID string) (*config.PurchaseExecution, error) { + m.record("GetExecutionByID", ctx, executionID) args := m.Called(ctx, executionID) if args.Get(0) == nil { return nil, args.Error(1) @@ -293,6 +322,7 @@ func (m *MockConfigStore) GetExecutionByID(ctx context.Context, executionID stri // GetExecutionByPlanAndDate mocks the GetExecutionByPlanAndDate operation. func (m *MockConfigStore) GetExecutionByPlanAndDate(ctx context.Context, planID string, scheduledDate time.Time) (*config.PurchaseExecution, error) { + m.record("GetExecutionByPlanAndDate", ctx, planID, scheduledDate) args := m.Called(ctx, planID, scheduledDate) if args.Get(0) == nil { return nil, args.Error(1) @@ -309,6 +339,7 @@ func (m *MockConfigStore) GetExecutionByPlanAndDate(ctx context.Context, planID // exercise the 4-eyes identity-resolution path (mode off, the default) don't // need to stub this method. func (m *MockConfigStore) GetUserEmailByID(ctx context.Context, userID string) (string, error) { + m.record("GetUserEmailByID", ctx, userID) if m.GetUserEmailByIDFn != nil { return m.GetUserEmailByIDFn(ctx, userID) } @@ -322,6 +353,7 @@ func (m *MockConfigStore) GetUserEmailByID(ctx context.Context, userID string) ( // CountPendingExecutionsForAccount mocks the CountPendingExecutionsForAccount operation. // Defaults to (0, nil) when no Fn is set and no expectation is registered. func (m *MockConfigStore) CountPendingExecutionsForAccount(ctx context.Context, accountID string) (int, error) { + m.record("CountPendingExecutionsForAccount", ctx, accountID) if m.CountPendingExecutionsForAccountFn != nil { return m.CountPendingExecutionsForAccountFn(ctx, accountID) } @@ -335,6 +367,7 @@ func (m *MockConfigStore) CountPendingExecutionsForAccount(ctx context.Context, // ListPendingExecutionIDsForAccount mocks the ListPendingExecutionIDsForAccount operation. // Defaults to (nil, nil) when no Fn is set and no expectation is registered. func (m *MockConfigStore) ListPendingExecutionIDsForAccount(ctx context.Context, accountID string) ([]string, error) { + m.record("ListPendingExecutionIDsForAccount", ctx, accountID) if m.ListPendingExecutionIDsForAccountFn != nil { return m.ListPendingExecutionIDsForAccountFn(ctx, accountID) } @@ -354,12 +387,14 @@ func (m *MockConfigStore) ListPendingExecutionIDsForAccount(ctx context.Context, // SavePurchaseHistory mocks the SavePurchaseHistory operation. func (m *MockConfigStore) SavePurchaseHistory(ctx context.Context, record *config.PurchaseHistoryRecord) error { + m.record("SavePurchaseHistory", ctx, record) args := m.Called(ctx, record) return args.Error(0) } // GetPurchaseHistory mocks the GetPurchaseHistory operation. func (m *MockConfigStore) GetPurchaseHistory(ctx context.Context, accountID string, limit int) ([]config.PurchaseHistoryRecord, error) { + m.record("GetPurchaseHistory", ctx, accountID, limit) args := m.Called(ctx, accountID, limit) if args.Get(0) == nil { return nil, args.Error(1) @@ -373,6 +408,7 @@ func (m *MockConfigStore) GetPurchaseHistory(ctx context.Context, accountID stri // GetAllPurchaseHistory mocks the GetAllPurchaseHistory operation. func (m *MockConfigStore) GetAllPurchaseHistory(ctx context.Context, limit int) ([]config.PurchaseHistoryRecord, error) { + m.record("GetAllPurchaseHistory", ctx, limit) args := m.Called(ctx, limit) if args.Get(0) == nil { return nil, args.Error(1) @@ -386,6 +422,7 @@ func (m *MockConfigStore) GetAllPurchaseHistory(ctx context.Context, limit int) // GetActivePurchaseHistory mocks the GetActivePurchaseHistory operation. func (m *MockConfigStore) GetActivePurchaseHistory(ctx context.Context, asOf time.Time, accountIDs []string, externalIDsByProvider map[string][]string) ([]config.PurchaseHistoryRecord, error) { + m.record("GetActivePurchaseHistory", ctx, asOf, accountIDs, externalIDsByProvider) args := m.Called(ctx, asOf, accountIDs, externalIDsByProvider) if args.Get(0) == nil { return nil, args.Error(1) @@ -399,6 +436,7 @@ func (m *MockConfigStore) GetActivePurchaseHistory(ctx context.Context, asOf tim // GetPurchaseHistoryFiltered mocks the GetPurchaseHistoryFiltered operation (issue #701). func (m *MockConfigStore) GetPurchaseHistoryFiltered(ctx context.Context, filter config.PurchaseHistoryFilter) ([]config.PurchaseHistoryRecord, error) { + m.record("GetPurchaseHistoryFiltered", ctx, filter) args := m.Called(ctx, filter) if args.Get(0) == nil { return nil, args.Error(1) @@ -412,6 +450,7 @@ func (m *MockConfigStore) GetPurchaseHistoryFiltered(ctx context.Context, filter // GetPurchaseHistoryByPurchaseID mocks the single-row lookup by purchase_id (issues #290, #292). func (m *MockConfigStore) GetPurchaseHistoryByPurchaseID(ctx context.Context, purchaseID string) (*config.PurchaseHistoryRecord, error) { + m.record("GetPurchaseHistoryByPurchaseID", ctx, purchaseID) args := m.Called(ctx, purchaseID) if args.Get(0) == nil { return nil, args.Error(1) @@ -425,6 +464,7 @@ func (m *MockConfigStore) GetPurchaseHistoryByPurchaseID(ctx context.Context, pu // MarkPurchaseRevoked mocks the MarkPurchaseRevoked operation (issue #290). func (m *MockConfigStore) MarkPurchaseRevoked(ctx context.Context, purchaseID string, revokedAt time.Time, revokedVia, supportCaseID string, calcRefundAmount *float64, calcRefundCurrency string) error { + m.record("MarkPurchaseRevoked", ctx, purchaseID, revokedAt, revokedVia, supportCaseID, calcRefundAmount, calcRefundCurrency) args := m.Called(ctx, purchaseID, revokedAt, revokedVia, supportCaseID, calcRefundAmount, calcRefundCurrency) return args.Error(0) } @@ -433,6 +473,7 @@ func (m *MockConfigStore) MarkPurchaseRevoked(ctx context.Context, purchaseID st // Uses the isExpected default-or-dispatch pattern: tests that only verify // MarkPurchaseRevoked do not need to set an expectation for this best-effort call. func (m *MockConfigStore) FlipPurchaseRevocationInFlight(ctx context.Context, purchaseID string) error { + m.record("FlipPurchaseRevocationInFlight", ctx, purchaseID) if !isExpected(&m.Mock, "FlipPurchaseRevocationInFlight") { return nil } @@ -444,6 +485,7 @@ func (m *MockConfigStore) FlipPurchaseRevocationInFlight(ctx context.Context, pu // Uses the isExpected default-or-dispatch pattern: tests that only test error paths // where Azure never actually returned do not need to register an expectation. func (m *MockConfigStore) ClearRevocationInFlight(ctx context.Context, purchaseID string) error { + m.record("ClearRevocationInFlight", ctx, purchaseID) if !isExpected(&m.Mock, "ClearRevocationInFlight") { return nil } @@ -453,6 +495,7 @@ func (m *MockConfigStore) ClearRevocationInFlight(ctx context.Context, purchaseI // GetPurchaseHistoryInFlight mocks GetPurchaseHistoryInFlight (issue #290 Finding #6). func (m *MockConfigStore) GetPurchaseHistoryInFlight(ctx context.Context) ([]*config.PurchaseHistoryRecord, error) { + m.record("GetPurchaseHistoryInFlight", ctx) args := m.Called(ctx) if args.Get(0) == nil { return nil, args.Error(1) @@ -466,28 +509,33 @@ func (m *MockConfigStore) GetPurchaseHistoryInFlight(ctx context.Context) ([]*co // UpdatePurchaseHistoryListing mocks stamping listing_id and listing_state (issue #292). func (m *MockConfigStore) UpdatePurchaseHistoryListing(ctx context.Context, purchaseID, listingID, listingState string) error { + m.record("UpdatePurchaseHistoryListing", ctx, purchaseID, listingID, listingState) args := m.Called(ctx, purchaseID, listingID, listingState) return args.Error(0) } // StampOfferingClass mocks persisting the offering_class fetched from AWS (issue #292). func (m *MockConfigStore) StampOfferingClass(ctx context.Context, purchaseID, offeringClass string) error { + m.record("StampOfferingClass", ctx, purchaseID, offeringClass) args := m.Called(ctx, purchaseID, offeringClass) return args.Error(0) } // ClaimMarketplaceListingSlot mocks the atomic listing-slot claim (issue #292). func (m *MockConfigStore) ClaimMarketplaceListingSlot(ctx context.Context, purchaseID string) (bool, error) { + m.record("ClaimMarketplaceListingSlot", ctx, purchaseID) args := m.Called(ctx, purchaseID) return args.Bool(0), args.Error(1) } func (m *MockConfigStore) SaveRIExchangeRecord(ctx context.Context, record *config.RIExchangeRecord) error { + m.record("SaveRIExchangeRecord", ctx, record) args := m.Called(ctx, record) return args.Error(0) } func (m *MockConfigStore) GetRIExchangeRecord(ctx context.Context, id string) (*config.RIExchangeRecord, error) { + m.record("GetRIExchangeRecord", ctx, id) args := m.Called(ctx, id) if args.Get(0) == nil { return nil, args.Error(1) @@ -500,6 +548,7 @@ func (m *MockConfigStore) GetRIExchangeRecord(ctx context.Context, id string) (* } func (m *MockConfigStore) GetRIExchangeRecordByToken(ctx context.Context, token string) (*config.RIExchangeRecord, error) { + m.record("GetRIExchangeRecordByToken", ctx, token) args := m.Called(ctx, token) if args.Get(0) == nil { return nil, args.Error(1) @@ -512,6 +561,7 @@ func (m *MockConfigStore) GetRIExchangeRecordByToken(ctx context.Context, token } func (m *MockConfigStore) GetRIExchangeHistory(ctx context.Context, since time.Time, limit int) ([]config.RIExchangeRecord, error) { + m.record("GetRIExchangeHistory", ctx, since, limit) args := m.Called(ctx, since, limit) if args.Get(0) == nil { return nil, args.Error(1) @@ -524,6 +574,7 @@ func (m *MockConfigStore) GetRIExchangeHistory(ctx context.Context, since time.T } func (m *MockConfigStore) TransitionRIExchangeStatus(ctx context.Context, id, fromStatus, toStatus string, actor *string) (*config.RIExchangeRecord, error) { + m.record("TransitionRIExchangeStatus", ctx, id, fromStatus, toStatus, actor) args := m.Called(ctx, id, fromStatus, toStatus, actor) if args.Get(0) == nil { return nil, args.Error(1) @@ -536,26 +587,31 @@ func (m *MockConfigStore) TransitionRIExchangeStatus(ctx context.Context, id, fr } func (m *MockConfigStore) CompleteRIExchange(ctx context.Context, id, exchangeID string) error { + m.record("CompleteRIExchange", ctx, id, exchangeID) args := m.Called(ctx, id, exchangeID) return args.Error(0) } func (m *MockConfigStore) CompleteRIExchangeWithPayment(ctx context.Context, id, exchangeID, acceptedPaymentDue string) error { + m.record("CompleteRIExchangeWithPayment", ctx, id, exchangeID, acceptedPaymentDue) args := m.Called(ctx, id, exchangeID, acceptedPaymentDue) return args.Error(0) } func (m *MockConfigStore) FailRIExchange(ctx context.Context, id, errorMsg string) error { + m.record("FailRIExchange", ctx, id, errorMsg) args := m.Called(ctx, id, errorMsg) return args.Error(0) } func (m *MockConfigStore) GetRIExchangeDailySpend(ctx context.Context, date time.Time) (string, error) { + m.record("GetRIExchangeDailySpend", ctx, date) args := m.Called(ctx, date) return args.String(0), args.Error(1) } func (m *MockConfigStore) CancelAllPendingExchanges(ctx context.Context) (int64, error) { + m.record("CancelAllPendingExchanges", ctx) args := m.Called(ctx) v, ok := args.Get(0).(int64) if !ok { @@ -565,6 +621,7 @@ func (m *MockConfigStore) CancelAllPendingExchanges(ctx context.Context) (int64, } func (m *MockConfigStore) CancelPendingExchangesByOrigin(ctx context.Context, origin common.ExchangeOrigin) (int64, error) { + m.record("CancelPendingExchangesByOrigin", ctx, origin) args := m.Called(ctx, origin) v, ok := args.Get(0).(int64) if !ok { @@ -574,6 +631,7 @@ func (m *MockConfigStore) CancelPendingExchangesByOrigin(ctx context.Context, or } func (m *MockConfigStore) GetStaleProcessingExchanges(ctx context.Context, olderThan time.Duration) ([]config.RIExchangeRecord, error) { + m.record("GetStaleProcessingExchanges", ctx, olderThan) args := m.Called(ctx, olderThan) if args.Get(0) == nil { return nil, args.Error(1) @@ -839,6 +897,7 @@ func (m *MockAuthStore) Ping(ctx context.Context) error { // Cloud accounts func (m *MockConfigStore) CreateCloudAccount(ctx context.Context, account *config.CloudAccount) error { + m.record("CreateCloudAccount", ctx, account) if m.CreateCloudAccountFn != nil { return m.CreateCloudAccountFn(ctx, account) } @@ -853,6 +912,7 @@ func (m *MockConfigStore) CreateCloudAccount(ctx context.Context, account *confi // set and no expectation is registered, so tests that don't care about account // fields keep working without explicit setup. func (m *MockConfigStore) GetCloudAccount(ctx context.Context, id string) (*config.CloudAccount, error) { + m.record("GetCloudAccount", ctx, id) if m.GetCloudAccountFn != nil { return m.GetCloudAccountFn(ctx, id) } @@ -871,6 +931,7 @@ func (m *MockConfigStore) GetCloudAccount(ctx context.Context, id string) (*conf } func (m *MockConfigStore) GetCloudAccountByExternalID(ctx context.Context, provider, externalID string) (*config.CloudAccount, error) { + m.record("GetCloudAccountByExternalID", ctx, provider, externalID) if m.GetCloudAccountByExternalIDFn != nil { return m.GetCloudAccountByExternalIDFn(ctx, provider, externalID) } @@ -889,6 +950,7 @@ func (m *MockConfigStore) GetCloudAccountByExternalID(ctx context.Context, provi } func (m *MockConfigStore) UpdateCloudAccount(ctx context.Context, account *config.CloudAccount) error { + m.record("UpdateCloudAccount", ctx, account) if !isExpected(&m.Mock, "UpdateCloudAccount") { return nil } @@ -897,6 +959,7 @@ func (m *MockConfigStore) UpdateCloudAccount(ctx context.Context, account *confi } func (m *MockConfigStore) DeleteCloudAccount(ctx context.Context, id string) error { + m.record("DeleteCloudAccount", ctx, id) if m.DeleteCloudAccountFn != nil { return m.DeleteCloudAccountFn(ctx, id) } @@ -908,6 +971,7 @@ func (m *MockConfigStore) DeleteCloudAccount(ctx context.Context, id string) err } func (m *MockConfigStore) ListCloudAccounts(ctx context.Context, filter config.CloudAccountFilter) ([]config.CloudAccount, error) { + m.record("ListCloudAccounts", ctx, filter) if m.ListCloudAccountsFn != nil { return m.ListCloudAccountsFn(ctx, filter) } @@ -928,6 +992,7 @@ func (m *MockConfigStore) ListCloudAccounts(ctx context.Context, filter config.C // Account credentials func (m *MockConfigStore) SaveAccountCredential(ctx context.Context, accountID, credentialType, encryptedBlob string) error { + m.record("SaveAccountCredential", ctx, accountID, credentialType, encryptedBlob) if !isExpected(&m.Mock, "SaveAccountCredential") { return nil } @@ -936,6 +1001,7 @@ func (m *MockConfigStore) SaveAccountCredential(ctx context.Context, accountID, } func (m *MockConfigStore) GetAccountCredential(ctx context.Context, accountID, credentialType string) (string, error) { + m.record("GetAccountCredential", ctx, accountID, credentialType) if !isExpected(&m.Mock, "GetAccountCredential") { return "", nil } @@ -944,6 +1010,7 @@ func (m *MockConfigStore) GetAccountCredential(ctx context.Context, accountID, c } func (m *MockConfigStore) DeleteAccountCredentials(ctx context.Context, accountID string) error { + m.record("DeleteAccountCredentials", ctx, accountID) if !isExpected(&m.Mock, "DeleteAccountCredentials") { return nil } @@ -952,6 +1019,7 @@ func (m *MockConfigStore) DeleteAccountCredentials(ctx context.Context, accountI } func (m *MockConfigStore) HasAccountCredentials(ctx context.Context, accountID string) (bool, error) { + m.record("HasAccountCredentials", ctx, accountID) if !isExpected(&m.Mock, "HasAccountCredentials") { return false, nil } @@ -962,6 +1030,7 @@ func (m *MockConfigStore) HasAccountCredentials(ctx context.Context, accountID s // Account service overrides func (m *MockConfigStore) GetAccountServiceOverride(ctx context.Context, accountID, provider, service string) (*config.AccountServiceOverride, error) { + m.record("GetAccountServiceOverride", ctx, accountID, provider, service) if !isExpected(&m.Mock, "GetAccountServiceOverride") { return nil, nil } @@ -977,6 +1046,7 @@ func (m *MockConfigStore) GetAccountServiceOverride(ctx context.Context, account } func (m *MockConfigStore) SaveAccountServiceOverride(ctx context.Context, override *config.AccountServiceOverride) error { + m.record("SaveAccountServiceOverride", ctx, override) if m.SaveAccountServiceOverrideFn != nil { return m.SaveAccountServiceOverrideFn(ctx, override) } @@ -988,6 +1058,7 @@ func (m *MockConfigStore) SaveAccountServiceOverride(ctx context.Context, overri } func (m *MockConfigStore) DeleteAccountServiceOverride(ctx context.Context, accountID, provider, service string) error { + m.record("DeleteAccountServiceOverride", ctx, accountID, provider, service) if !isExpected(&m.Mock, "DeleteAccountServiceOverride") { return nil } @@ -996,6 +1067,7 @@ func (m *MockConfigStore) DeleteAccountServiceOverride(ctx context.Context, acco } func (m *MockConfigStore) ListAccountServiceOverrides(ctx context.Context, accountID string) ([]config.AccountServiceOverride, error) { + m.record("ListAccountServiceOverrides", ctx, accountID) if !isExpected(&m.Mock, "ListAccountServiceOverrides") { return nil, nil } @@ -1013,6 +1085,7 @@ func (m *MockConfigStore) ListAccountServiceOverrides(ctx context.Context, accou // Plan ↔ account association func (m *MockConfigStore) SetPlanAccounts(ctx context.Context, planID string, accountIDs []string) error { + m.record("SetPlanAccounts", ctx, planID, accountIDs) if m.SetPlanAccountsFn != nil { return m.SetPlanAccountsFn(ctx, planID, accountIDs) } @@ -1024,6 +1097,7 @@ func (m *MockConfigStore) SetPlanAccounts(ctx context.Context, planID string, ac } func (m *MockConfigStore) GetPlanAccounts(ctx context.Context, planID string) ([]config.CloudAccount, error) { + m.record("GetPlanAccounts", ctx, planID) if m.GetPlanAccountsFn != nil { return m.GetPlanAccountsFn(ctx, planID) } @@ -1043,6 +1117,7 @@ func (m *MockConfigStore) GetPlanAccounts(ctx context.Context, planID string) ([ // CleanupOldExecutions mocks the CleanupOldExecutions operation. func (m *MockConfigStore) CleanupOldExecutions(ctx context.Context, retentionDays int) (int64, error) { + m.record("CleanupOldExecutions", ctx, retentionDays) args := m.Called(ctx, retentionDays) v, ok := args.Get(0).(int64) if !ok { @@ -1058,6 +1133,7 @@ func (m *MockConfigStore) CleanupOldExecutions(ctx context.Context, retentionDay // register an explicit expectation via .On(...).Return(...). func (m *MockConfigStore) ReplaceRecommendations(ctx context.Context, collectedAt time.Time, recs []config.RecommendationRecord) error { + m.record("ReplaceRecommendations", ctx, collectedAt, recs) if !isExpected(&m.Mock, "ReplaceRecommendations") { return nil } @@ -1066,6 +1142,7 @@ func (m *MockConfigStore) ReplaceRecommendations(ctx context.Context, collectedA } func (m *MockConfigStore) UpsertRecommendations(ctx context.Context, collectedAt time.Time, recs []config.RecommendationRecord, successfulCollects []config.SuccessfulCollect) error { + m.record("UpsertRecommendations", ctx, collectedAt, recs, successfulCollects) if !isExpected(&m.Mock, "UpsertRecommendations") { return nil } @@ -1074,6 +1151,7 @@ func (m *MockConfigStore) UpsertRecommendations(ctx context.Context, collectedAt } func (m *MockConfigStore) ListStoredRecommendations(ctx context.Context, filter config.RecommendationFilter) ([]config.RecommendationRecord, error) { + m.record("ListStoredRecommendations", ctx, filter) if !isExpected(&m.Mock, "ListStoredRecommendations") { return nil, nil } @@ -1089,6 +1167,7 @@ func (m *MockConfigStore) ListStoredRecommendations(ctx context.Context, filter } func (m *MockConfigStore) GetRecommendationsFreshness(ctx context.Context) (*config.RecommendationsFreshness, error) { + m.record("GetRecommendationsFreshness", ctx) if !isExpected(&m.Mock, "GetRecommendationsFreshness") { return &config.RecommendationsFreshness{}, nil } @@ -1104,6 +1183,7 @@ func (m *MockConfigStore) GetRecommendationsFreshness(ctx context.Context) (*con } func (m *MockConfigStore) SetRecommendationsCollectionError(ctx context.Context, errMsg string) error { + m.record("SetRecommendationsCollectionError", ctx, errMsg) if !isExpected(&m.Mock, "SetRecommendationsCollectionError") { return nil } @@ -1112,6 +1192,7 @@ func (m *MockConfigStore) SetRecommendationsCollectionError(ctx context.Context, } func (m *MockConfigStore) GetRIUtilizationCache(ctx context.Context, region string, lookbackDays int) (*config.RIUtilizationCacheEntry, error) { + m.record("GetRIUtilizationCache", ctx, region, lookbackDays) if !isExpected(&m.Mock, "GetRIUtilizationCache") { return nil, nil } @@ -1127,6 +1208,7 @@ func (m *MockConfigStore) GetRIUtilizationCache(ctx context.Context, region stri } func (m *MockConfigStore) UpsertRIUtilizationCache(ctx context.Context, region string, lookbackDays int, payload []byte, fetchedAt time.Time) error { + m.record("UpsertRIUtilizationCache", ctx, region, lookbackDays, payload, fetchedAt) if !isExpected(&m.Mock, "UpsertRIUtilizationCache") { return nil } @@ -1135,11 +1217,13 @@ func (m *MockConfigStore) UpsertRIUtilizationCache(ctx context.Context, region s } func (m *MockConfigStore) CreateAccountRegistration(ctx context.Context, reg *config.AccountRegistration) error { + m.record("CreateAccountRegistration", ctx, reg) args := m.Called(ctx, reg) return args.Error(0) } func (m *MockConfigStore) GetAccountRegistration(ctx context.Context, id string) (*config.AccountRegistration, error) { + m.record("GetAccountRegistration", ctx, id) args := m.Called(ctx, id) if args.Get(0) == nil { return nil, args.Error(1) @@ -1152,6 +1236,7 @@ func (m *MockConfigStore) GetAccountRegistration(ctx context.Context, id string) } func (m *MockConfigStore) GetAccountRegistrationByToken(ctx context.Context, token string) (*config.AccountRegistration, error) { + m.record("GetAccountRegistrationByToken", ctx, token) args := m.Called(ctx, token) if args.Get(0) == nil { return nil, args.Error(1) @@ -1164,6 +1249,7 @@ func (m *MockConfigStore) GetAccountRegistrationByToken(ctx context.Context, tok } func (m *MockConfigStore) ListAccountRegistrations(ctx context.Context, filter config.AccountRegistrationFilter) ([]config.AccountRegistration, error) { + m.record("ListAccountRegistrations", ctx, filter) args := m.Called(ctx, filter) if args.Get(0) == nil { return nil, args.Error(1) @@ -1176,16 +1262,19 @@ func (m *MockConfigStore) ListAccountRegistrations(ctx context.Context, filter c } func (m *MockConfigStore) UpdateAccountRegistration(ctx context.Context, reg *config.AccountRegistration) error { + m.record("UpdateAccountRegistration", ctx, reg) args := m.Called(ctx, reg) return args.Error(0) } func (m *MockConfigStore) TransitionRegistrationStatus(ctx context.Context, reg *config.AccountRegistration, fromStatus string, actor *string) error { + m.record("TransitionRegistrationStatus", ctx, reg, fromStatus, actor) args := m.Called(ctx, reg, fromStatus, actor) return args.Error(0) } func (m *MockConfigStore) DeleteAccountRegistration(ctx context.Context, id string) error { + m.record("DeleteAccountRegistration", ctx, id) args := m.Called(ctx, id) return args.Error(0) } @@ -1195,6 +1284,7 @@ func (m *MockConfigStore) DeleteAccountRegistration(ctx context.Context, id stri // existing tests don't need to be aware of the suppression lifecycle. func (m *MockConfigStore) CreateSuppression(ctx context.Context, sup *config.PurchaseSuppression) error { + m.record("CreateSuppression", ctx, sup) if !isExpected(&m.Mock, "CreateSuppression") { return nil } @@ -1203,6 +1293,7 @@ func (m *MockConfigStore) CreateSuppression(ctx context.Context, sup *config.Pur } func (m *MockConfigStore) CreateSuppressionTx(ctx context.Context, tx pgx.Tx, sup *config.PurchaseSuppression) error { + m.record("CreateSuppressionTx", ctx, tx, sup) if !isExpected(&m.Mock, "CreateSuppressionTx") { return nil } @@ -1211,6 +1302,7 @@ func (m *MockConfigStore) CreateSuppressionTx(ctx context.Context, tx pgx.Tx, su } func (m *MockConfigStore) DeleteSuppressionsByExecution(ctx context.Context, executionID string) error { + m.record("DeleteSuppressionsByExecution", ctx, executionID) if !isExpected(&m.Mock, "DeleteSuppressionsByExecution") { return nil } @@ -1219,6 +1311,7 @@ func (m *MockConfigStore) DeleteSuppressionsByExecution(ctx context.Context, exe } func (m *MockConfigStore) DeleteSuppressionsByExecutionTx(ctx context.Context, tx pgx.Tx, executionID string) error { + m.record("DeleteSuppressionsByExecutionTx", ctx, tx, executionID) if !isExpected(&m.Mock, "DeleteSuppressionsByExecutionTx") { return nil } @@ -1227,6 +1320,7 @@ func (m *MockConfigStore) DeleteSuppressionsByExecutionTx(ctx context.Context, t } func (m *MockConfigStore) ListActiveSuppressions(ctx context.Context) ([]config.PurchaseSuppression, error) { + m.record("ListActiveSuppressions", ctx) if !isExpected(&m.Mock, "ListActiveSuppressions") { return nil, nil } @@ -1246,6 +1340,7 @@ func (m *MockConfigStore) ListActiveSuppressions(ctx context.Context) ([]config. // (same pattern as SavePurchaseExecutionTx) so existing tests that exercise // the WithTx path transparently get the same pending-execution list. func (m *MockConfigStore) GetPendingExecutionsTx(ctx context.Context, tx pgx.Tx) ([]config.PurchaseExecution, error) { + m.record("GetPendingExecutionsTx", ctx, tx) if !isExpected(&m.Mock, "GetPendingExecutionsTx") { return m.GetPendingExecutions(ctx) } @@ -1261,6 +1356,7 @@ func (m *MockConfigStore) GetPendingExecutionsTx(ctx context.Context, tx pgx.Tx) } func (m *MockConfigStore) SavePurchaseExecutionTx(ctx context.Context, tx pgx.Tx, execution *config.PurchaseExecution) error { + m.record("SavePurchaseExecutionTx", ctx, tx, execution) if !isExpected(&m.Mock, "SavePurchaseExecutionTx") { return m.SavePurchaseExecution(ctx, execution) } @@ -1272,6 +1368,7 @@ func (m *MockConfigStore) SavePurchaseExecutionTx(ctx context.Context, tx pgx.Tx // *Tx mocks directly. Tests that want to assert on WithTx itself // register .On("WithTx", ...) and that takes precedence. func (m *MockConfigStore) WithTx(ctx context.Context, fn func(tx pgx.Tx) error) error { + m.record("WithTx", ctx, fn) if isExpected(&m.Mock, "WithTx") { args := m.Called(ctx, fn) return args.Error(0) @@ -1281,6 +1378,7 @@ func (m *MockConfigStore) WithTx(ctx context.Context, fn func(tx pgx.Tx) error) // GetExecutionsByStatuses mocks the GetExecutionsByStatuses operation. func (m *MockConfigStore) GetExecutionsByStatuses(ctx context.Context, statuses []string, limit int) ([]config.PurchaseExecution, error) { + m.record("GetExecutionsByStatuses", ctx, statuses, limit) args := m.Called(ctx, statuses, limit) if args.Get(0) == nil { return nil, args.Error(1) @@ -1294,6 +1392,7 @@ func (m *MockConfigStore) GetExecutionsByStatuses(ctx context.Context, statuses // CountExecutionsByPlanAndStatus mocks the CountExecutionsByPlanAndStatus operation. func (m *MockConfigStore) CountExecutionsByPlanAndStatus(ctx context.Context, statuses []string, since time.Time) (map[string]config.ExecutionStatusCounts, error) { + m.record("CountExecutionsByPlanAndStatus", ctx, statuses, since) args := m.Called(ctx, statuses, since) if args.Get(0) == nil { return nil, args.Error(1) @@ -1307,6 +1406,7 @@ func (m *MockConfigStore) CountExecutionsByPlanAndStatus(ctx context.Context, st // GetPlannedExecutions mocks the GetPlannedExecutions operation. func (m *MockConfigStore) GetPlannedExecutions(ctx context.Context, statuses []string, limit int) ([]config.PurchaseExecution, error) { + m.record("GetPlannedExecutions", ctx, statuses, limit) args := m.Called(ctx, statuses, limit) if args.Get(0) == nil { return nil, args.Error(1) @@ -1320,6 +1420,7 @@ func (m *MockConfigStore) GetPlannedExecutions(ctx context.Context, statuses []s // GetStaleApprovedExecutions mocks the GetStaleApprovedExecutions operation. func (m *MockConfigStore) GetStaleApprovedExecutions(ctx context.Context, olderThan time.Duration) ([]config.PurchaseExecution, error) { + m.record("GetStaleApprovedExecutions", ctx, olderThan) args := m.Called(ctx, olderThan) if args.Get(0) == nil { return nil, args.Error(1) @@ -1333,6 +1434,7 @@ func (m *MockConfigStore) GetStaleApprovedExecutions(ctx context.Context, olderT // ListStuckExecutions mocks the ListStuckExecutions operation. func (m *MockConfigStore) ListStuckExecutions(ctx context.Context, statuses []string, olderThan time.Duration) ([]config.PurchaseExecution, error) { + m.record("ListStuckExecutions", ctx, statuses, olderThan) args := m.Called(ctx, statuses, olderThan) if args.Get(0) == nil { return nil, args.Error(1) @@ -1348,6 +1450,7 @@ func (m *MockConfigStore) ListStuckExecutions(ctx context.Context, statuses []st // Defaults to (nil, nil) when no expectation is registered so existing scheduler // tests that don't exercise the pre-fire delay path keep working without changes. func (m *MockConfigStore) GetScheduledExecutionsDue(ctx context.Context) ([]config.PurchaseExecution, error) { + m.record("GetScheduledExecutionsDue", ctx) if !isExpected(&m.Mock, "GetScheduledExecutionsDue") { return nil, nil } @@ -1365,6 +1468,7 @@ func (m *MockConfigStore) GetScheduledExecutionsDue(ctx context.Context) ([]conf // MarkCollectionStarted mocks the MarkCollectionStarted operation. // Defaults to (MockOwnerToken, true, nil) when no expectation is registered. func (m *MockConfigStore) MarkCollectionStarted(ctx context.Context) (token string, ok bool, err error) { + m.record("MarkCollectionStarted", ctx) if !isExpected(&m.Mock, "MarkCollectionStarted") { return MockOwnerToken, true, nil } @@ -1375,6 +1479,7 @@ func (m *MockConfigStore) MarkCollectionStarted(ctx context.Context) (token stri // ClearCollectionStarted mocks the ClearCollectionStarted operation. // Defaults to nil when no expectation is registered. func (m *MockConfigStore) ClearCollectionStarted(ctx context.Context, token string) error { + m.record("ClearCollectionStarted", ctx, token) if !isExpected(&m.Mock, "ClearCollectionStarted") { return nil } @@ -1383,6 +1488,7 @@ func (m *MockConfigStore) ClearCollectionStarted(ctx context.Context, token stri // StampRIExchangeApprovedBy mocks the StampRIExchangeApprovedBy operation. func (m *MockConfigStore) StampRIExchangeApprovedBy(ctx context.Context, id, approverEmail string) error { + m.record("StampRIExchangeApprovedBy", ctx, id, approverEmail) args := m.Called(ctx, id, approverEmail) return args.Error(0) } @@ -1391,6 +1497,7 @@ func (m *MockConfigStore) StampRIExchangeApprovedBy(ctx context.Context, id, app // Returns an empty slice when no expectation is registered so tests that do // not exercise laddering keep working without mock setup. func (m *MockConfigStore) GetLadderConfigs(ctx context.Context) ([]config.LadderConfigDB, error) { + m.record("GetLadderConfigs", ctx) if !isExpected(&m.Mock, "GetLadderConfigs") { return []config.LadderConfigDB{}, nil } @@ -1408,6 +1515,7 @@ func (m *MockConfigStore) GetLadderConfigs(ctx context.Context) ([]config.Ladder // GetLadderConfig mocks the GetLadderConfig operation. // Returns (nil, nil) when no expectation is registered. func (m *MockConfigStore) GetLadderConfig(ctx context.Context, cloudAccountID, provider string) (*config.LadderConfigDB, error) { + m.record("GetLadderConfig", ctx, cloudAccountID, provider) if !isExpected(&m.Mock, "GetLadderConfig") { return nil, nil } @@ -1424,6 +1532,7 @@ func (m *MockConfigStore) GetLadderConfig(ctx context.Context, cloudAccountID, p // UpsertLadderConfig mocks the UpsertLadderConfig operation. func (m *MockConfigStore) UpsertLadderConfig(ctx context.Context, cfg *config.LadderConfigDB) (*config.LadderConfigDB, error) { + m.record("UpsertLadderConfig", ctx, cfg) args := m.Called(ctx, cfg) if args.Get(0) == nil { return nil, args.Error(1) @@ -1438,6 +1547,7 @@ func (m *MockConfigStore) UpsertLadderConfig(ctx context.Context, cfg *config.La // SaveLadderRun mocks the SaveLadderRun operation. // Returns (nil, nil) when no expectation is registered. func (m *MockConfigStore) SaveLadderRun(ctx context.Context, run *config.LadderRunDB) (*config.LadderRunDB, error) { + m.record("SaveLadderRun", ctx, run) if !isExpected(&m.Mock, "SaveLadderRun") { return nil, nil } @@ -1456,6 +1566,7 @@ func (m *MockConfigStore) SaveLadderRun(ctx context.Context, run *config.LadderR // Returns the run unchanged (transaction succeeds) when no expectation is // registered, so tests that only care about other calls stay green. func (m *MockConfigStore) SaveLadderRunWithTranches(ctx context.Context, run *config.LadderRunDB, tranches []config.LadderTrancheDB) (*config.LadderRunDB, error) { + m.record("SaveLadderRunWithTranches", ctx, run, tranches) if !isExpected(&m.Mock, "SaveLadderRunWithTranches") { return run, nil } @@ -1473,6 +1584,7 @@ func (m *MockConfigStore) SaveLadderRunWithTranches(ctx context.Context, run *co // GetLadderRun mocks the GetLadderRun operation. // Returns (nil, nil) when no expectation is registered. func (m *MockConfigStore) GetLadderRun(ctx context.Context, id string) (*config.LadderRunDB, error) { + m.record("GetLadderRun", ctx, id) if !isExpected(&m.Mock, "GetLadderRun") { return nil, nil } @@ -1490,6 +1602,7 @@ func (m *MockConfigStore) GetLadderRun(ctx context.Context, id string) (*config. // SaveLadderTranches mocks the SaveLadderTranches operation. // Returns nil (no-op) when no expectation is registered. func (m *MockConfigStore) SaveLadderTranches(ctx context.Context, tranches []config.LadderTrancheDB) error { + m.record("SaveLadderTranches", ctx, tranches) if !isExpected(&m.Mock, "SaveLadderTranches") { return nil } @@ -1499,6 +1612,7 @@ func (m *MockConfigStore) SaveLadderTranches(ctx context.Context, tranches []con // LatestLadderRunStartedAt mocks the LatestLadderRunStartedAt operation. // Returns (nil, nil) when no expectation is registered. func (m *MockConfigStore) LatestLadderRunStartedAt(ctx context.Context, configID string) (*time.Time, error) { + m.record("LatestLadderRunStartedAt", ctx, configID) if !isExpected(&m.Mock, "LatestLadderRunStartedAt") { return nil, nil } @@ -1517,6 +1631,7 @@ func (m *MockConfigStore) LatestLadderRunStartedAt(ctx context.Context, configID // operation. Returns a pointer to zero (no in-flight commitment) when no // expectation is registered, matching the happy-path default. func (m *MockConfigStore) GetInFlightLadderCommitUSDHr(ctx context.Context, configID string) (*float64, error) { + m.record("GetInFlightLadderCommitUSDHr", ctx, configID) if !isExpected(&m.Mock, "GetInFlightLadderCommitUSDHr") { zero := 0.0 return &zero, nil @@ -1535,6 +1650,7 @@ func (m *MockConfigStore) GetInFlightLadderCommitUSDHr(ctx context.Context, conf // TransitionLadderRunStatus mocks the TransitionLadderRunStatus operation. // Returns (nil, nil) when no expectation is registered (CAS race-lost path). func (m *MockConfigStore) TransitionLadderRunStatus(ctx context.Context, id string, fromStatuses []ladder.RunStatus, toStatus ladder.RunStatus) (*config.LadderRunDB, error) { + m.record("TransitionLadderRunStatus", ctx, id, fromStatuses, toStatus) if !isExpected(&m.Mock, "TransitionLadderRunStatus") { return nil, nil } @@ -1552,6 +1668,7 @@ func (m *MockConfigStore) TransitionLadderRunStatus(ctx context.Context, id stri // UpsertNotificationMute mocks the UpsertNotificationMute operation. // Defaults to nil when no expectation is registered. func (m *MockConfigStore) UpsertNotificationMute(ctx context.Context, recipientEmail, scope, unmuteToken string) error { + m.record("UpsertNotificationMute", ctx, recipientEmail, scope, unmuteToken) if !isExpected(&m.Mock, "UpsertNotificationMute") { return nil } @@ -1561,6 +1678,7 @@ func (m *MockConfigStore) UpsertNotificationMute(ctx context.Context, recipientE // IsNotificationMuted mocks the IsNotificationMuted operation. // Defaults to (false, nil) when no expectation is registered. func (m *MockConfigStore) IsNotificationMuted(ctx context.Context, recipientEmail, scope string) (bool, error) { + m.record("IsNotificationMuted", ctx, recipientEmail, scope) if !isExpected(&m.Mock, "IsNotificationMuted") { return false, nil } From 46ac437b4fdd3f1c886517d08e28363abd5550b9 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 8 Aug 2026 01:44:17 +0200 Subject: [PATCH 2/3] fix(test): attribute assertion failures to the call site, arm an inert fixture Two findings from mutation-testing the 77 repaired assertion sites. t.Helper() marks its own caller, so routing it through a shared markHelper function marked markHelper as the helper and reported every failure at assertions.go rather than at the failing assertion. With 25 assertion sites in one test file, the output could not say which one fired. The type-assertion is now inlined in each of the three shadowed methods. TestProcessMessage_ApproveRejectsTokenMismatch asserts GetCloudAccount is not called, guarding that token validation precedes approver resolution. Its fixture carries no recommendation with a CloudAccountID, so gatherApproverContactEmails iterates an empty slice and the guarded bug could not produce the call: the assertion was mechanically failable but semantically inert. The fixture now mirrors its sibling at TestProcessMessage_ApproveRejectsNonMatchingActor. Hoisting approver resolution above the token check in messages.go now fails the test; without the fixture change it stayed green. Refs #1595 --- internal/mocks/assertions.go | 23 ++++++++++++++--------- internal/purchase/coverage_extra_test.go | 8 ++++++++ 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/internal/mocks/assertions.go b/internal/mocks/assertions.go index b1dd1b5fb..677ce0e82 100644 --- a/internal/mocks/assertions.go +++ b/internal/mocks/assertions.go @@ -85,18 +85,19 @@ func (m *MockConfigStore) record(method string, args ...interface{}) { m.callLog.record(method, args...) } -func markHelper(t mock.TestingT) { - if h, ok := t.(interface{ Helper() }); ok { - h.Helper() - } -} - // AssertCalled asserts that method was invoked with arguments satisfying the // given matchers. It shadows the promoted testify method so it reads the // complete call log rather than mock.Mock.Calls; pass no arguments to assert // the method was called at all. +// +// mock.TestingT has no Helper method, so the assertion helpers type-assert for +// it inline. t.Helper() marks its own caller, so delegating that to a shared +// function would report every failure at that function's line instead of the +// assertion's call site. func (m *MockConfigStore) AssertCalled(t mock.TestingT, method string, arguments ...interface{}) bool { - markHelper(t) + if h, ok := t.(interface{ Helper() }); ok { + h.Helper() + } if m.callLog.matching(method, arguments) > 0 { return true } @@ -110,7 +111,9 @@ func (m *MockConfigStore) AssertCalled(t mock.TestingT, method string, arguments // reads the complete call log rather than mock.Mock.Calls; pass no arguments to // assert the method was not called at all. func (m *MockConfigStore) AssertNotCalled(t mock.TestingT, method string, arguments ...interface{}) bool { - markHelper(t) + if h, ok := t.(interface{ Helper() }); ok { + h.Helper() + } if n := m.callLog.matching(method, arguments); n > 0 { t.Errorf("mock: %s was called %d time(s) matching %v, expected none. Recorded calls:%s", method, n, arguments, m.callLog.summary(method)) @@ -123,7 +126,9 @@ func (m *MockConfigStore) AssertNotCalled(t mock.TestingT, method string, argume // arguments. It shadows the promoted testify method so it reads the complete // call log rather than mock.Mock.Calls. func (m *MockConfigStore) AssertNumberOfCalls(t mock.TestingT, method string, expectedCalls int) bool { - markHelper(t) + if h, ok := t.(interface{ Helper() }); ok { + h.Helper() + } if actual := m.callLog.matching(method, nil); actual != expectedCalls { t.Errorf("mock: expected %d call(s) to %s, got %d. Recorded calls:%s", expectedCalls, method, actual, m.callLog.summary(method)) diff --git a/internal/purchase/coverage_extra_test.go b/internal/purchase/coverage_extra_test.go index 3c2aaf7a6..d4beba693 100644 --- a/internal/purchase/coverage_extra_test.go +++ b/internal/purchase/coverage_extra_test.go @@ -621,10 +621,18 @@ func TestProcessMessage_ApproveRejectsTokenMismatch(t *testing.T) { mockStore := new(MockConfigStore) mockEmail := new(MockEmailSender) + // The recommendation carries a CloudAccountID so that approver resolution, + // if it were ever hoisted above the token check, would have an account to + // look up. Without it the AssertNotCalled below is inert: the ordering bug + // it guards could not produce a GetCloudAccount call against this fixture. + accountID := "acct-1" exec := &config.PurchaseExecution{ ExecutionID: "exec-bad-token", Status: "pending", ApprovalToken: "correct-token", + Recommendations: []config.RecommendationRecord{ + {CloudAccountID: &accountID}, + }, } mockStore.On("GetExecutionByID", ctx, "exec-bad-token").Return(exec, nil) From c8dfb46dd811461b628c343a2d0f54e10327eb90 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 8 Aug 2026 03:21:58 +0200 Subject: [PATCH 3/3] fix(test): fail loudly on a matcher count that cannot match A third structural cause of unfailable assertions, distinct from the two this PR already repaired and not covered by either fix. scheduled_fire_test.go asserted TransitionExecutionStatus was not called using four matchers. The method takes five arguments. matching() special -cased only len(expected)==0, so a non-zero but wrong count fell through to testify's Diff, which pads the shorter side with "(Missing)" and scores the padded position as a difference. diffs was then non-zero for every recorded call, the assertion matched nothing, and it could not fire whatever the code under test did. Mutating FireScheduledDelayedPurchases to issue a CAS on the nothing-is-due branch -- exactly what the assertion guards -- leaves AssertNotCalled reporting success. The assertion now names the method with no matchers, which is how this mock spells "never called at all, whatever the arguments", and bites on that mutation. matching() now treats a matcher count that differs from the recorded arity as a test bug and reports it through Errorf + FailNow, naming the method, its real arity and the count given. len(expected)==0 keeps its meaning. The check reads the arity off a recorded call, so it is inert when the method was never called -- the case where any matcher count yields the same correct verdict anyway. The helpers also surface it as a failed assertion, so a TestingT whose FailNow returns cannot turn a miscount back into a silent pass. An arity sweep over all 206 MockConfigStore assertion sites in the repo, comparing each non-zero matcher count against the arity the method records, finds this one site and no other. The full suite confirms it: 6692 pass with the check armed. Both new regression tests fail with the check disabled and pass with it armed. The mock-internal fallback delegations gain a note that the delegate records itself, so one Tx call appears in the log under both names. Refs #1595 --- internal/mocks/assertions.go | 59 +++++++++++++++++-- internal/mocks/assertions_test.go | 72 +++++++++++++++++++++++- internal/mocks/stores.go | 7 +++ internal/purchase/scheduled_fire_test.go | 5 +- 4 files changed, 135 insertions(+), 8 deletions(-) diff --git a/internal/mocks/assertions.go b/internal/mocks/assertions.go index 677ce0e82..cd829265d 100644 --- a/internal/mocks/assertions.go +++ b/internal/mocks/assertions.go @@ -35,6 +35,17 @@ func (l *callLog) record(method string, args ...interface{}) { l.calls = append(l.calls, recordedCall{method: method, args: args}) } +// fatalf reports an error in how a test wrote its assertion, then stops that +// test. mock.TestingT has no Fatalf, so this is Errorf plus FailNow. It marks +// itself a helper inline for the same reason the assertion helpers below do. +func fatalf(t mock.TestingT, format string, args ...interface{}) { + if h, ok := t.(interface{ Helper() }); ok { + h.Helper() + } + t.Errorf(format, args...) + t.FailNow() +} + // matching counts the recorded invocations of method whose arguments satisfy // expected. An empty expected matches every invocation of the method. // @@ -43,7 +54,25 @@ func (l *callLog) record(method string, args ...interface{}) { // that takes parameters -- a second, independent way for an assertion to become // unfailable. Naming the method is the only way to express "not called at all", // so that is what it means here. -func (l *callLog) matching(method string, expected []interface{}) int { +// +// A non-empty expected whose length differs from the method's real arity is a +// third way, and it is a bug in the test rather than a legitimate intent: +// Diff pads the shorter side with "(Missing)" and scores every padded position +// as a difference, so diffs is unconditionally non-zero and the assertion can +// never fire. There is no argument list such an expectation could be asking +// about, so it fails loudly instead of silently matching nothing. The check +// needs a recorded call to read the arity from, which is exactly the case +// where the miscount would otherwise produce a false pass; with no call at +// all, any matcher count yields the same correct verdict. +// +// The second result is false when the matcher count was wrong. A real +// *testing.T never returns from the FailNow inside fatalf, but the callers +// still surface it as a failed assertion so that a TestingT whose FailNow +// returns cannot turn a miscounted assertion back into a silent pass. +func (l *callLog) matching(t mock.TestingT, method string, expected []interface{}) (int, bool) { + if h, ok := t.(interface{ Helper() }); ok { + h.Helper() + } l.mu.Lock() defer l.mu.Unlock() n := 0 @@ -55,11 +84,19 @@ func (l *callLog) matching(method string, expected []interface{}) int { n++ continue } + if len(expected) != len(c.args) { + fatalf(t, "mock: %s takes %d argument(s) but the assertion passed %d matcher(s). "+ + "A matcher count that differs from the real arity can never match, which would "+ + "make this assertion unfailable. Pass %d matcher(s), or none to mean "+ + "\"not called at all, whatever the arguments\".", + method, len(c.args), len(expected), len(c.args)) + return 0, false + } if _, diffs := mock.Arguments(expected).Diff(c.args); diffs == 0 { n++ } } - return n + return n, true } // summary renders the recorded invocations of method for failure messages. @@ -98,7 +135,11 @@ func (m *MockConfigStore) AssertCalled(t mock.TestingT, method string, arguments if h, ok := t.(interface{ Helper() }); ok { h.Helper() } - if m.callLog.matching(method, arguments) > 0 { + n, ok := m.callLog.matching(t, method, arguments) + if !ok { + return false + } + if n > 0 { return true } t.Errorf("mock: %s was not called with the expected arguments %v. Recorded calls:%s", @@ -114,7 +155,11 @@ func (m *MockConfigStore) AssertNotCalled(t mock.TestingT, method string, argume if h, ok := t.(interface{ Helper() }); ok { h.Helper() } - if n := m.callLog.matching(method, arguments); n > 0 { + n, ok := m.callLog.matching(t, method, arguments) + if !ok { + return false + } + if n > 0 { t.Errorf("mock: %s was called %d time(s) matching %v, expected none. Recorded calls:%s", method, n, arguments, m.callLog.summary(method)) return false @@ -129,7 +174,11 @@ func (m *MockConfigStore) AssertNumberOfCalls(t mock.TestingT, method string, ex if h, ok := t.(interface{ Helper() }); ok { h.Helper() } - if actual := m.callLog.matching(method, nil); actual != expectedCalls { + actual, ok := m.callLog.matching(t, method, nil) + if !ok { + return false + } + if actual != expectedCalls { t.Errorf("mock: expected %d call(s) to %s, got %d. Recorded calls:%s", expectedCalls, method, actual, m.callLog.summary(method)) return false diff --git a/internal/mocks/assertions_test.go b/internal/mocks/assertions_test.go index 600b51ec2..8b158a016 100644 --- a/internal/mocks/assertions_test.go +++ b/internal/mocks/assertions_test.go @@ -15,11 +15,17 @@ import ( // these tests can assert that an assertion helper *fails*. type recordingT struct { failed bool + // msgs holds the format strings passed to Errorf, so a test can assert + // which diagnosis fired and not merely that something did. + msgs []string } func (r *recordingT) Logf(string, ...interface{}) {} -func (r *recordingT) Errorf(string, ...interface{}) { r.failed = true } +func (r *recordingT) Errorf(format string, _ ...interface{}) { + r.failed = true + r.msgs = append(r.msgs, format) +} func (r *recordingT) FailNow() { r.failed = true } @@ -75,6 +81,70 @@ func TestAssertNotCalled_NameOnlyFormMatchesCallWithArguments(t *testing.T) { assert.True(t, m.Mock.AssertNotCalled(&recordingT{}, "WithTx")) } +// TestAssertNotCalled_WrongMatcherCountFailsLoudly pins the third way these +// assertions were unfailable, the one that survived the name-only repair. +// TransitionExecutionStatus takes five arguments; an assertion written with +// four matchers reaches Diff, which pads the fifth position with "(Missing)" +// and scores it as a difference. diffs is then non-zero for every recorded +// call, so the assertion matches nothing and can never fire, whatever the code +// under test does. It is a bug in the test, and is now reported as one. +func TestAssertNotCalled_WrongMatcherCountFailsLoudly(t *testing.T) { + m := &MockConfigStore{} + m.On("TransitionExecutionStatus", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(&config.PurchaseExecution{}, nil) + _, err := m.TransitionExecutionStatus(context.Background(), "exec-1", []string{"scheduled"}, "approved", nil) + require.NoError(t, err) + + rt := &recordingT{} + assert.False(t, m.AssertNotCalled(rt, "TransitionExecutionStatus", + mock.Anything, mock.Anything, mock.Anything, mock.Anything), + "four matchers against a five-argument method must not report success") + assert.True(t, rt.failed, "the miscount must be reported to the TestingT") + require.Len(t, rt.msgs, 1) + assert.Contains(t, rt.msgs[0], "matcher(s)", + "the failure must diagnose the arity, not the call itself") + + // The two counts that can express an intent both still fire on the call. + full := &recordingT{} + assert.False(t, m.AssertNotCalled(full, "TransitionExecutionStatus", + mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)) + assert.True(t, full.failed) + + nameOnly := &recordingT{} + assert.False(t, m.AssertNotCalled(nameOnly, "TransitionExecutionStatus")) + assert.True(t, nameOnly.failed) +} + +// TestAssertCalled_WrongMatcherCountFailsLoudly covers the mirror image. Here +// the miscount would have produced a spurious failure rather than a false pass, +// but the diagnosis is the same and must name the arity rather than dump an +// argument diff the reader cannot act on. +func TestAssertCalled_WrongMatcherCountFailsLoudly(t *testing.T) { + m := &MockConfigStore{} + _, err := m.GetPurchasePlan(context.Background(), "plan-a") + require.NoError(t, err) + + rt := &recordingT{} + assert.False(t, m.AssertCalled(rt, "GetPurchasePlan", mock.Anything)) + assert.True(t, rt.failed) + require.Len(t, rt.msgs, 1) + assert.Contains(t, rt.msgs[0], "matcher(s)", + "the failure must diagnose the arity, not report the call as missing") +} + +// TestMatching_WrongMatcherCountIsInertWithoutACall records the deliberate +// limit of the arity check: it reads the arity off a recorded call, so with no +// call to the method there is nothing to compare against. That is the case +// where the miscount cannot cause a false pass anyway, because every matcher +// count yields the same correct "not called" verdict. +func TestMatching_WrongMatcherCountIsInertWithoutACall(t *testing.T) { + m := &MockConfigStore{} + rt := &recordingT{} + assert.True(t, m.AssertNotCalled(rt, "TransitionExecutionStatus", + mock.Anything, mock.Anything, mock.Anything, mock.Anything)) + assert.False(t, rt.failed) +} + // TestAssertNotCalled_PassesWhenNotCalled guards against the opposite failure: // an assertion that always fails is no more useful than one that never does. func TestAssertNotCalled_PassesWhenNotCalled(t *testing.T) { diff --git a/internal/mocks/stores.go b/internal/mocks/stores.go index 01b3975f1..1b75dd8e1 100644 --- a/internal/mocks/stores.go +++ b/internal/mocks/stores.go @@ -101,6 +101,8 @@ func (m *MockConfigStore) UpdateGlobalConfigAtomic(ctx context.Context, apply fu } return v, args.Error(1) } + // This fallback routes through the mocked GetGlobalConfig and + // SaveGlobalConfig, which record themselves; see UpdatePurchasePlanTx. existing, err := m.GetGlobalConfig(ctx) if err != nil { return nil, err @@ -203,6 +205,9 @@ func (m *MockConfigStore) IncrementPlanCurrentStep(ctx context.Context, planID s func (m *MockConfigStore) UpdatePurchasePlanTx(ctx context.Context, tx pgx.Tx, plan *config.PurchasePlan) error { m.record("UpdatePurchasePlanTx", ctx, tx, plan) if !isExpected(&m.Mock, "UpdatePurchasePlanTx") { + // The delegate records itself too, so a single Tx call shows up in the + // call log under both names. Intended: assertions about the non-Tx + // method see the write the Tx variant performed on their behalf. return m.UpdatePurchasePlan(ctx, plan) } args := m.Called(ctx, tx, plan) @@ -1342,6 +1347,7 @@ func (m *MockConfigStore) ListActiveSuppressions(ctx context.Context) ([]config. func (m *MockConfigStore) GetPendingExecutionsTx(ctx context.Context, tx pgx.Tx) ([]config.PurchaseExecution, error) { m.record("GetPendingExecutionsTx", ctx, tx) if !isExpected(&m.Mock, "GetPendingExecutionsTx") { + // Recorded under both names; see UpdatePurchasePlanTx. return m.GetPendingExecutions(ctx) } args := m.Called(ctx, tx) @@ -1358,6 +1364,7 @@ func (m *MockConfigStore) GetPendingExecutionsTx(ctx context.Context, tx pgx.Tx) func (m *MockConfigStore) SavePurchaseExecutionTx(ctx context.Context, tx pgx.Tx, execution *config.PurchaseExecution) error { m.record("SavePurchaseExecutionTx", ctx, tx, execution) if !isExpected(&m.Mock, "SavePurchaseExecutionTx") { + // Recorded under both names; see UpdatePurchasePlanTx. return m.SavePurchaseExecution(ctx, execution) } args := m.Called(ctx, tx, execution) diff --git a/internal/purchase/scheduled_fire_test.go b/internal/purchase/scheduled_fire_test.go index 7f707ca0c..0639e3cc2 100644 --- a/internal/purchase/scheduled_fire_test.go +++ b/internal/purchase/scheduled_fire_test.go @@ -48,8 +48,9 @@ func TestFireScheduledDelayedPurchases_NoDueRows(t *testing.T) { assert.Equal(t, 0, result.RaceLost) assert.Equal(t, 0, result.Errored) store.AssertExpectations(t) - // No CAS attempted when nothing is due. - store.AssertNotCalled(t, "TransitionExecutionStatus", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + // No CAS attempted when nothing is due. Named without matchers: that is + // how this mock spells "never called at all, whatever the arguments". + store.AssertNotCalled(t, "TransitionExecutionStatus") } func TestFireScheduledDelayedPurchases_ListErrorSurfaces(t *testing.T) {