test(api): make scheduled-revoke mock explicit + deterministic reservation test

CodeRabbit round 4:

TestRevokePurchase_ScheduledExecution_AdminFreeCancel relied on the shared mock
default for its CAS return and carried a comment naming the wrong method
(CancelExecutionAtomic), while a scheduled row is actually cancelled via
CancelScheduledExecutionAtomic. Stub that method explicitly with
Return(true, "cancelled", nil).Once() plus DeleteSuppressionsByExecutionTx,
matching the sibling scheduled-revoke tests. AssertExpectations now enforces the
stub, so the setup and the asserted status can never drift.

TestBuildReservationName_NilFieldsReturnsFallback rebuilt its expected value via
SanitizeReservationID("", prefix), which calls time.Now() a second time -- a
second-boundary cross between the actual call and the rebuild would flake the
equality assertion. Replace it with deterministic structural assertions: regexp
^rds-reserved-[0-9]+$ (sanitized prefix + unix timestamp), no trailing hyphen,
and the length cap. No second time.Now() comparison.

Verified by execution: go test ./internal/api/... (1636) and -run Revoke (33)
both green; reservation_name nil tests pass.

Author: cristim

internal/api/handler_purchases_revoke_test.go

@@ -580,9 +580,13 @@ func TestRevokePurchase_ScheduledExecution_AdminFreeCancel(t *testing.T) {
 	mockAuth.On("ValidateSession", ctx, "tok").Return(adminSess, nil)
 	exec := scheduledExecution(execID, "")
 	mockStore.On("GetExecutionByID", ctx, execID).Return(exec, nil)
-	// CancelExecutionAtomic and DeleteSuppressionsByExecutionTx use mock defaults
-	// (WithTx calls fn(nil), CancelExecutionAtomic returns canceled=true plus the
-	// persisted cancel status and a nil error).
+	// A scheduled row is canceled via CancelScheduledExecutionAtomic. Stub its
+	// return explicitly (matching the assertion below) rather than leaning on the
+	// shared mock default, so the setup and the asserted status can never drift.
+	mockStore.On("CancelScheduledExecutionAtomic", ctx, mock.Anything, execID, mock.Anything).
+		//nolint:misspell // DB schema value 'cancelled' -- see migration 000001_initial_schema.up.sql
+		Return(true, "cancelled", nil).Once()
+	mockStore.On("DeleteSuppressionsByExecutionTx", ctx, mock.Anything, execID).Return(nil).Once()
 
 	h := &Handler{config: mockStore, auth: mockAuth}
 	result, err := h.revokePurchase(ctx, sessionReq(), execID)

pkg/common/reservation_name_test.go

@@ -16,12 +16,17 @@ func testFixedRand() []byte { return []byte{0xa1, 0xb2, 0xc3, 0xd4} }
 
 // TestBuildReservationName_NilFieldsReturnsFallback is the CR #1276 guard: the
 // exported helper must return the fallback prefix (a safe, non-empty no-op) on
-// a nil pointer argument rather than panicking.
+// a nil pointer argument rather than panicking. Asserts structural properties
+// (prefix + timestamp shape + cap) rather than equality against a value rebuilt
+// with a second time.Now() call, which would flake on a second-boundary cross.
 func TestBuildReservationName_NilFieldsReturnsFallback(t *testing.T) {
 	got := BuildReservationName(nil, "rds-reserved-")
 	assert.NotEmpty(t, got, "nil fields must not panic and must yield a non-empty name")
-	// Under the cap, the nil path equals the plain sanitized fallback.
-	assert.Equal(t, SanitizeReservationID("", "rds-reserved-"), got)
+	// SanitizeReservationID("", prefix) returns prefix + unix-seconds, so the
+	// output is the sanitized prefix followed by digits only.
+	assert.Regexp(t, `^rds-reserved-[0-9]+$`, got,
+		"nil path must yield the sanitized fallback prefix plus a unix timestamp")
+	assert.False(t, strings.HasSuffix(got, "-"), "name must not end in a hyphen")
 	assert.LessOrEqual(t, len(got), awsReservationNameMaxLen)
 }