From 167e00dc4fbcb3e28fae3046150c0d10fe9f2d7e Mon Sep 17 00:00:00 2001 From: Evan Jarrett Date: Mon, 14 Sep 2026 09:14:12 -0500 Subject: [PATCH] tests: open the scan broadcaster test database the way production does, and run CI with the testmode tag Four scan broadcaster tests failed in CI with "database is locked" on a status query. The helper opened the scan database with a bare sql.Open, while NewScanBroadcaster goes through holddb.OpenLocalDB. That left the test pool with no busy_timeout on any connection and the file in rollback-journal mode, so a test polling a job's status every 2ms on one pooled connection raced the storage goroutine's commit on another, and a read that landed inside the commit failed immediately instead of waiting. The window is sub-millisecond on a local disk and reproduced only under fsync-heavy load here, but the CI runner's disk hits it regularly. Both hand-opened scan databases now go through OpenLocalDB. Under the same disk load, 120 runs of the four tests pass where one in sixty failed before. The CI workflow also now passes -tags testmode, matching make test. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01GPiKVQcxGYwxAbGnZv2tir --- .tangled/workflows/tests.yml | 2 +- pkg/hold/pds/scan_broadcaster_stall_test.go | 8 +++++--- pkg/hold/pds/scan_broadcaster_test.go | 10 +++++++++- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/.tangled/workflows/tests.yml b/.tangled/workflows/tests.yml index 93c465d..1b8d3e6 100644 --- a/.tangled/workflows/tests.yml +++ b/.tangled/workflows/tests.yml @@ -20,4 +20,4 @@ steps: environment: CGO_ENABLED: 1 command: | - go test -cover ./... + go test -tags testmode -cover ./... diff --git a/pkg/hold/pds/scan_broadcaster_stall_test.go b/pkg/hold/pds/scan_broadcaster_stall_test.go index 1028126..727ae67 100644 --- a/pkg/hold/pds/scan_broadcaster_stall_test.go +++ b/pkg/hold/pds/scan_broadcaster_stall_test.go @@ -1,9 +1,11 @@ package pds import ( - "database/sql" + "path/filepath" "testing" "time" + + holddb "atcr.io/pkg/hold/db" ) // backdateJob ages a job's created_at so the staleness windows in @@ -279,7 +281,7 @@ func TestScanDrainPendingJobs_SkipsClaimedJob(t *testing.T) { // not: after a small budget of consecutive failures the check fails open, and // says so distinctly in the log. func TestScanHasActiveJobs_FailsOpenAfterPersistentDBErrors(t *testing.T) { - dbPath := "file:" + t.TempDir() + "/scan.db" + dbPath := filepath.Join(t.TempDir(), "scan.db") sb := newTestScanBroadcaster(t) seedPendingJobs(t, sb, 1) @@ -305,7 +307,7 @@ func TestScanHasActiveJobs_FailsOpenAfterPersistentDBErrors(t *testing.T) { // A working database restores the budget, so a later blip is absorbed // rather than landing on an already-exhausted counter. - db, err := sql.Open("libsql", dbPath) + db, err := holddb.OpenLocalDB(dbPath) if err != nil { t.Fatalf("reopen db: %v", err) } diff --git a/pkg/hold/pds/scan_broadcaster_test.go b/pkg/hold/pds/scan_broadcaster_test.go index 8c4eccb..b55a180 100644 --- a/pkg/hold/pds/scan_broadcaster_test.go +++ b/pkg/hold/pds/scan_broadcaster_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + holddb "atcr.io/pkg/hold/db" "atcr.io/pkg/s3" ) @@ -15,10 +16,17 @@ import ( // background goroutines. Subscriber lifecycle is all that is under test here, // so the constructor's discovery/dispatch/stale loops (and their s3 and PDS // dependencies) are deliberately skipped. +// +// The database is opened the way NewScanBroadcaster opens it, through +// holddb.OpenLocalDB, so every pooled connection carries busy_timeout and the +// file is in WAL mode. A bare sql.Open gives neither: the tests that poll a +// job's status while the storage goroutine writes it then race the writer's +// commit on a second connection, and on a slow CI disk that read fails with +// "database is locked" instead of waiting. func newTestScanBroadcaster(t *testing.T) *ScanBroadcaster { t.Helper() - db, err := sql.Open("libsql", "file:"+t.TempDir()+"/scan.db") + db, err := holddb.OpenLocalDB(filepath.Join(t.TempDir(), "scan.db")) if err != nil { t.Fatalf("open scan db: %v", err) }