From fcdf79774dd4033b8a64debdbe8a39a35e6afc8f Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Tue, 5 May 2026 03:39:22 -0700 Subject: [PATCH] test(delegations): tighten integration-test assertions + integrationDB doc (#321) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three small follow-ups from #2866 self-review: 1. TestIntegration_Sweeper_StaleHeartbeatIsMarkedStuck — assert strings.Contains(errDet, "no heartbeat for") instead of != "". The original "non-empty" check passes for any error_detail value; if a future regression swaps the message format, the test wouldn't catch it. Pin the production format string explicitly. 2. TestIntegration_Sweeper_DeadlineExceededIsMarkedFailed — drop the redundant `last_heartbeat = now()` write. The sweeper checks deadline FIRST (the stronger statement) and short-circuits before evaluating heartbeat staleness, so the heartbeat field is irrelevant for that test path. 3. integrationDB doc comment now warns explicitly that the helper is NOT t.Parallel()-safe — it hot-swaps the package-level mdb.DB and restores via t.Cleanup. If a future contributor adds t.Parallel() to one of these tests they race on the global. Comment makes the constraint discoverable instead of a debugging surprise. All 7 integration tests still pass against real Postgres locally. --- .../delegation_ledger_integration_test.go | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/workspace-server/internal/handlers/delegation_ledger_integration_test.go b/workspace-server/internal/handlers/delegation_ledger_integration_test.go index c493bcb8..78c0a874 100644 --- a/workspace-server/internal/handlers/delegation_ledger_integration_test.go +++ b/workspace-server/internal/handlers/delegation_ledger_integration_test.go @@ -37,6 +37,7 @@ import ( "context" "database/sql" "os" + "strings" "testing" mdb "github.com/Molecule-AI/molecule-monorepo/platform/internal/db" @@ -47,6 +48,12 @@ import ( // skips the test if INTEGRATION_DB_URL is unset. Local devs run the // docker-postgres incantation in the file header; CI's workflow sets the // env var via a service container. +// +// NOT SAFE FOR `t.Parallel()`. Each call hot-swaps the package-level +// `mdb.DB` and restores via `t.Cleanup`. If two tests using this helper +// run in parallel they race on the global; tests that need parallelism +// should drive a local `*sql.DB` they own and pass it into helpers +// directly rather than going through the package global. func integrationDB(t *testing.T) *sql.DB { t.Helper() url := os.Getenv("INTEGRATION_DB_URL") @@ -227,9 +234,12 @@ func TestIntegration_Sweeper_DeadlineExceededIsMarkedFailed(t *testing.T) { recordLedgerStatus(context.Background(), id, "dispatched", "", "") // Force the deadline into the past — Insert defaults to now+6h, so - // we override. + // we override. We don't touch last_heartbeat: the sweeper checks + // deadline FIRST (it's the stronger statement) and short-circuits + // before evaluating heartbeat staleness, so a NULL or stale beat is + // irrelevant for the deadline-failure path. if _, err := conn.ExecContext(context.Background(), - `UPDATE delegations SET deadline = now() - interval '1 minute', last_heartbeat = now() WHERE delegation_id = $1`, id, + `UPDATE delegations SET deadline = now() - interval '1 minute' WHERE delegation_id = $1`, id, ); err != nil { t.Fatalf("backdate deadline: %v", err) } @@ -280,8 +290,8 @@ func TestIntegration_Sweeper_StaleHeartbeatIsMarkedStuck(t *testing.T) { if status != "stuck" { t.Errorf("status: want stuck, got %q", status) } - if errDet == "" { - t.Errorf("error_detail should mention 'no heartbeat for Xs'; got empty") + if !strings.Contains(errDet, "no heartbeat for") { + t.Errorf("error_detail should contain 'no heartbeat for'; got %q", errDet) } }