Skip to content

Commit d8ded22

Browse files
committed
fix(rebase): restore saveExecutionStatusBestEffort; update param-23 guard test
Two issues introduced when rebasing onto main (which added ladder_execution_enabled in parallel): 1. saveExecutionStatusBestEffort helper was dropped during conflict resolution of the applyAccountOutcome refactor commit; restored from the pre-rebase branch. 2. TestSaveGlobalConfig_OfferingClassBindsAt22 expected 22 args; with ladder_execution_enabled now at $22, offering_class moved to $23. Renamed the test to TestSaveGlobalConfig_OfferingClassBindsAt23 and added the $22 AnyArg placeholder.
1 parent 72082e6 commit d8ded22

2 files changed

Lines changed: 20 additions & 8 deletions

File tree

‎internal/config/store_postgres_coverage_test.go‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -538,16 +538,17 @@ func TestPostgresStore_GetAllPurchaseHistory_NilDB(t *testing.T) {
538538
assert.True(t, panicked, "expected panic with nil db connection")
539539
}
540540

541-
// TestSaveGlobalConfig_OfferingClassBindsAt22 is the HOLE 2 regression guard
541+
// TestSaveGlobalConfig_OfferingClassBindsAt23 is the HOLE 2 regression guard
542542
// (issue #694): the real PostgresStore.SaveGlobalConfig must bind offering_class
543-
// as the 22nd positional argument ($22), with laddering_enabled at $21.
543+
// as the 23rd positional argument ($23), with laddering_enabled at $21 and
544+
// ladder_execution_enabled at $22.
544545
// The testablePostgresStore in store_postgres_mock_test.go is a hand-maintained
545546
// copy that omits the field entirely, so no fast test guarded the placeholder
546547
// count until now.
547548
//
548549
// Using pgxmock directly against the real PostgresStore (not the hand-maintained
549550
// testablePostgresStore wrapper) ensures the live query is tested, not a stale copy.
550-
func TestSaveGlobalConfig_OfferingClassBindsAt22(t *testing.T) {
551+
func TestSaveGlobalConfig_OfferingClassBindsAt23(t *testing.T) {
551552
ctx := context.Background()
552553
mock, err := pgxmock.NewPool()
553554
require.NoError(t, err)
@@ -569,8 +570,9 @@ func TestSaveGlobalConfig_OfferingClassBindsAt22(t *testing.T) {
569570
OfferingClass: "standard",
570571
}
571572

572-
// Expect exactly 22 args; pgxmock validates arg count and types.
573-
// The 21st arg is laddering_enabled; the 22nd arg must be "standard" (offering_class).
573+
// Expect exactly 23 args; pgxmock validates arg count and types.
574+
// The 21st arg is laddering_enabled; the 22nd is ladder_execution_enabled;
575+
// the 23rd arg must be "standard" (offering_class).
574576
// If the real query regresses to a different arg count, pgxmock
575577
// will return an unexpected-call error and the test will fail.
576578
mock.ExpectExec(`INSERT INTO global_config`).
@@ -596,13 +598,14 @@ func TestSaveGlobalConfig_OfferingClassBindsAt22(t *testing.T) {
596598
pgxmock.AnyArg(), // $19 recommendations_lookback_days
597599
pgxmock.AnyArg(), // $20 purchase_delay_hours
598600
pgxmock.AnyArg(), // $21 laddering_enabled
599-
"standard", // $22 offering_class -- the field this test guards
601+
pgxmock.AnyArg(), // $22 ladder_execution_enabled
602+
"standard", // $23 offering_class -- the field this test guards
600603
).
601604
WillReturnResult(pgxmock.NewResult("INSERT", 1))
602605

603606
err = store.SaveGlobalConfig(ctx, cfg)
604-
require.NoError(t, err, "SaveGlobalConfig must succeed when the DB accepts all 22 args")
607+
require.NoError(t, err, "SaveGlobalConfig must succeed when the DB accepts all 23 args")
605608

606609
require.NoError(t, mock.ExpectationsWereMet(),
607-
"offering_class must be bound as the 22nd argument to SaveGlobalConfig")
610+
"offering_class must be bound as the 23rd argument to SaveGlobalConfig")
608611
}

‎internal/purchase/execution.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,15 @@ func (m *Manager) executeMultiAccount(ctx context.Context, baseExec *config.Purc
201201
return fmt.Errorf("%w: %s", errAllAccountsFailed, strings.Join(errs, "; "))
202202
}
203203

204+
// saveExecutionStatusBestEffort saves the execution record and logs any error.
205+
// Used in error paths where we need to persist the failure status but cannot
206+
// propagate the save error (the original error is already being returned).
207+
func (m *Manager) saveExecutionStatusBestEffort(ctx context.Context, exec *config.PurchaseExecution) {
208+
if err := m.config.SavePurchaseExecution(ctx, exec); err != nil {
209+
logging.Warnf("execution: failed to persist error status for account %v: %v", exec.CloudAccountID, err)
210+
}
211+
}
212+
204213
// executeForAccount runs a single plan execution for one cloud account.
205214
// It creates a new PurchaseExecution record tagged with cloud_account_id, resolves
206215
// per-account credentials, executes purchases, and saves the result.

0 commit comments

Comments
 (0)