From 853bafe4425263f516b749f2e7e543649b47a576 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 16 Oct 2026 13:56:12 +0200 Subject: [PATCH] test(config): fail instead of skipping when no pending execution exists GetExecutionByID success depended on a value captured by the earlier GetPendingExecutions subtest and t.Skip'd when it was empty. If GetPendingExecutions regressed to return nothing, the dependent test skipped and the suite stayed green. GetPendingExecutions now requires a non-empty result and ID, and GetExecutionByID success saves its own execution so it no longer depends on subtest ordering. Also drop the redundant idx_purchase_executions_execution_id_unique DDL (A04-020). Its comment was wrong: migration 000008 already adds the unique_execution_id constraint that ON CONFLICT (execution_id) uses. Refs #202 Co-Authored-By: Claude Opus 5.5 --- internal/config/store_postgres_db_test.go | 33 +++++++++-------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/internal/config/store_postgres_db_test.go b/internal/config/store_postgres_db_test.go index 2c5929853..04a458be0 100644 --- a/internal/config/store_postgres_db_test.go +++ b/internal/config/store_postgres_db_test.go @@ -51,14 +51,6 @@ func setupTestContainerDB(t *testing.T) *database.Connection { migrations.RunMigrations(ctx, container.DB.Pool(), getTestMigrationsPath(), "", ""), "migrations failed to apply to a fresh database") - // The store code uses ON CONFLICT (execution_id) but the migration schema - // does not create a UNIQUE constraint on execution_id. Add it for tests. - _, err := container.DB.Exec(ctx, - "CREATE UNIQUE INDEX IF NOT EXISTS idx_purchase_executions_execution_id_unique ON purchase_executions(execution_id)") - require.NoError(t, err, - "could not add the unique index on execution_id the ON CONFLICT paths need; "+ - "a migrated schema this DDL cannot be applied to is a failure, not a reason to skip") - return container.DB } @@ -561,26 +553,27 @@ func TestPostgresStoreDB_PurchaseExecutions(t *testing.T) { assert.Equal(t, existingExecID, exec.ExecutionID) }) - // Store a generated exec ID for use in later subtests - var generatedExecID string t.Run("GetPendingExecutions", func(t *testing.T) { pending, err := store.GetPendingExecutions(ctx) require.NoError(t, err) - assert.NotNil(t, pending) - assert.GreaterOrEqual(t, len(pending), 1) - // Store the first pending execution ID for later use - if len(pending) > 0 { - generatedExecID = pending[0].ExecutionID - } + require.NotEmpty(t, pending, "earlier subtests saved pending executions") + require.NotEmpty(t, pending[0].ExecutionID) }) t.Run("GetExecutionByID success", func(t *testing.T) { - if generatedExecID == "" { - t.Skip("no execution ID available from prior test") + saved := &PurchaseExecution{ + PlanID: plan.ID, + Status: "pending", + StepNumber: 1, + ScheduledDate: time.Now().Add(96 * time.Hour), + Recommendations: []RecommendationRecord{}, } - exec, err := store.GetExecutionByID(ctx, generatedExecID) + require.NoError(t, store.SavePurchaseExecution(ctx, saved)) + require.NotEmpty(t, saved.ExecutionID) + + exec, err := store.GetExecutionByID(ctx, saved.ExecutionID) require.NoError(t, err) - assert.Equal(t, generatedExecID, exec.ExecutionID) + assert.Equal(t, saved.ExecutionID, exec.ExecutionID) assert.Equal(t, plan.ID, exec.PlanID) })