Skip to content

Commit 9e1369c

Browse files
authored
fix(test): keep a slow template checkpoint from poisoning the Windows suite (#951)
The shared test-DB templates from #943 checkpoint their WAL under a 1-second context deadline inside a `sync.Once` that caches the error. On a cold windows-latest runner, one checkpoint that misses the deadline permanently poisons the template, and every subsequent test in that binary fails with the same cached `checkpointing db template: wal checkpoint truncate: context deadline exceeded`. That is what happened on #948's CI run — a frontend-only diff — where the `cmd/agentsview` and `internal/db` binaries each hit the deadline once and 599 tests failed in cascade, while the identical SHA passed on main's push run. Two changes, by blast radius: - The two base templates (`internal/dbtest/dbtest.go`, `internal/db/db_test.go`) get a best-effort checkpoint with a generous once-per-binary deadline: the copy step already carries the `-wal`/`-shm` files along, so an uncheckpointed template is still a consistent database, and losing the checkpoint only costs compactness. Independently, a failed template build now degrades to creating the per-test database from scratch (the pre-#943 path) instead of failing the test — the template is a setup-cost optimization and should never be a single point of failure for the whole binary. - The five seeded fixture builders (store contract, chunked analytics, daily usage, server analytics, cmd stats golden) keep their fatal checkpoint semantics — their consumers' copy semantics vary — but move from the 1s deadline to the same 30s one, which is effectively free since each runs once. Two regression tests cover the new fallback, including cleanup of a partial template copy left behind by a mid-copy failure and the existing-file short-circuit. Not addressed here: the unrelated `TestEnsureBackgroundServeLaunchLoser…` "server did not become ready within 2s" readiness flake that hit #949's run. Where to look: `buildTestDBTemplate` in `internal/dbtest/dbtest.go` and `copyTestDBTemplate`/`openTestDBWithTemplate` in `internal/db/db_test.go`. Co-authored-by: Matthew Jacobs <mjacobs@users.noreply.github.com>
1 parent 32e6caf commit 9e1369c

6 files changed

Lines changed: 144 additions & 20 deletions

File tree

internal/db/db_test.go

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -351,8 +351,23 @@ func TestMain(m *testing.M) {
351351
}
352352

353353
func openCopiedTestDB(path string) (*DB, error) {
354-
if err := copyTestDBTemplate(path); err != nil {
355-
return nil, err
354+
return openTestDBWithTemplate(path, copyTestDBTemplate)
355+
}
356+
357+
func openTestDBWithTemplate(
358+
path string, copyTemplate func(string) error,
359+
) (*DB, error) {
360+
if err := copyTemplate(path); err != nil {
361+
// The shared template is only a setup-cost optimization.
362+
// Never let a template failure poison every test in the
363+
// binary; build this database from scratch instead.
364+
fmt.Fprintf(os.Stderr,
365+
"db test: template unavailable, creating %s from scratch: %v\n",
366+
path, err)
367+
for _, suffix := range []string{"", "-wal", "-shm"} {
368+
_ = os.Remove(path + suffix)
369+
}
370+
return Open(path)
356371
}
357372
return OpenPreparedTestDB(path)
358373
}
@@ -380,13 +395,19 @@ func copyTestDBTemplate(dst string) error {
380395
)
381396
return
382397
}
383-
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
398+
// Checkpointing keeps the copied template compact, but it
399+
// is best-effort: the copy below carries the -wal/-shm
400+
// files along, so a checkpoint that cannot finish in time
401+
// (slow Windows CI runners) must not fail the build. The
402+
// timeout is generous because this runs once per binary.
403+
ctx, cancel := context.WithTimeout(
404+
context.Background(), 30*time.Second,
405+
)
384406
defer cancel()
385407
if err := template.CheckpointWALTruncate(ctx); err != nil {
386-
testDBTemplateErr = fmt.Errorf(
387-
"checkpointing db template: %w",
388-
err,
389-
)
408+
fmt.Fprintf(os.Stderr,
409+
"db test: template wal checkpoint failed, copying wal as-is: %v\n",
410+
err)
390411
}
391412
if err := template.Close(); err != nil && testDBTemplateErr == nil {
392413
testDBTemplateErr = fmt.Errorf(

internal/db/store_contract_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ func storeContractSQLiteTemplate(t *testing.T) (string, storeContractFixture) {
105105
return
106106
}
107107
storeContractSQLiteTemplateFixture = seedStoreContractSQLite(t, d)
108-
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
108+
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
109109
defer cancel()
110110
storeContractSQLiteTemplateErr = d.CheckpointWALTruncate(ctx)
111111
if closeErr := d.Close(); storeContractSQLiteTemplateErr == nil {
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
package db
2+
3+
import (
4+
"errors"
5+
"os"
6+
"path/filepath"
7+
"testing"
8+
9+
"github.com/stretchr/testify/require"
10+
)
11+
12+
func TestOpenTestDBWithTemplateFallsBackWhenTemplateUnavailable(t *testing.T) {
13+
path := filepath.Join(t.TempDir(), "test.db")
14+
15+
// Simulate a poisoned template build that also left a partial
16+
// copy behind: the fallback must discard it and open a fresh,
17+
// fully migrated database instead of failing the test.
18+
d, err := openTestDBWithTemplate(path, func(dst string) error {
19+
require.NoError(t, os.WriteFile(dst, []byte("garbage"), 0o600),
20+
"writing partial template copy")
21+
return errors.New("template poisoned")
22+
})
23+
require.NoError(t, err, "fallback open")
24+
t.Cleanup(func() { require.NoError(t, d.Close()) })
25+
26+
var sessions int
27+
require.NoError(t,
28+
d.getReader().QueryRow("SELECT COUNT(*) FROM sessions").Scan(&sessions),
29+
"querying sessions on fallback db")
30+
require.Zero(t, sessions, "fresh fallback db should be empty")
31+
}

internal/dbtest/dbtest.go

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,13 @@ func OpenTestDBAt(t *testing.T, path string) *db.DB {
9494
// does not already exist. Existing files are left intact so callers can reopen
9595
// and add more fixture rows without losing earlier writes.
9696
func EnsureTestDBAt(t *testing.T, path string) {
97+
t.Helper()
98+
ensureTestDBAtWith(t, path, copyTestDBTemplate)
99+
}
100+
101+
func ensureTestDBAtWith(
102+
t *testing.T, path string, copyTemplate func(string) error,
103+
) {
97104
t.Helper()
98105
_, err := os.Stat(path)
99106
if err == nil {
@@ -102,8 +109,22 @@ func EnsureTestDBAt(t *testing.T, path string) {
102109
if !errors.Is(err, os.ErrNotExist) {
103110
t.Fatalf("checking test db %s: %v", path, err)
104111
}
105-
if err := copyTestDBTemplate(path); err != nil {
106-
t.Fatalf("copying test db template: %v", err)
112+
if err := copyTemplate(path); err != nil {
113+
// The shared template is only a setup-cost optimization.
114+
// Never let a template failure poison every test in the
115+
// binary; build this database from scratch instead.
116+
t.Logf("test db template unavailable, creating %s from scratch: %v",
117+
path, err)
118+
for _, suffix := range []string{"", "-wal", "-shm"} {
119+
_ = os.Remove(path + suffix)
120+
}
121+
d, openErr := db.Open(path)
122+
if openErr != nil {
123+
t.Fatalf("creating test db from scratch: %v", openErr)
124+
}
125+
if closeErr := d.Close(); closeErr != nil {
126+
t.Fatalf("closing scratch test db: %v", closeErr)
127+
}
107128
}
108129
}
109130

@@ -147,18 +168,19 @@ func buildTestDBTemplate() (map[string][]byte, error) {
147168
if err != nil {
148169
return nil, fmt.Errorf("opening db template: %w", err)
149170
}
150-
// The deadline only guards against a hung checkpoint: flushing
151-
// the schema-creation WAL can take well over a second on slow
152-
// Windows CI disks under parallel package load.
171+
// Checkpointing keeps the copied template compact, but it is best-effort:
172+
// the copy below carries the -wal/-shm files along, so a checkpoint that
173+
// cannot finish in time must not fail the build. The generous deadline only
174+
// guards against a hung checkpoint on slow Windows CI disks.
153175
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
154176
defer cancel()
155-
checkpointErr := template.CheckpointWALTruncate(ctx)
156-
closeErr := template.Close()
157-
if checkpointErr != nil {
158-
return nil, fmt.Errorf("checkpointing db template: %w", checkpointErr)
177+
if err := template.CheckpointWALTruncate(ctx); err != nil {
178+
fmt.Fprintf(os.Stderr,
179+
"dbtest: template wal checkpoint failed, copying wal as-is: %v\n",
180+
err)
159181
}
160-
if closeErr != nil {
161-
return nil, fmt.Errorf("closing db template: %w", closeErr)
182+
if err := template.Close(); err != nil {
183+
return nil, fmt.Errorf("closing db template: %w", err)
162184
}
163185

164186
files := make(map[string][]byte, 3)
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
package dbtest
2+
3+
import (
4+
"errors"
5+
"os"
6+
"path/filepath"
7+
"testing"
8+
9+
"github.com/stretchr/testify/require"
10+
11+
"go.kenn.io/agentsview/internal/db"
12+
)
13+
14+
func TestEnsureTestDBAtFallsBackWhenTemplateUnavailable(t *testing.T) {
15+
dir := t.TempDir()
16+
path := filepath.Join(dir, "test.db")
17+
18+
// Simulate a poisoned template build that also left a partial
19+
// copy behind: the fallback must discard it and still produce a
20+
// usable current-schema database.
21+
ensureTestDBAtWith(t, path, func(dst string) error {
22+
require.NoError(t, os.WriteFile(dst, []byte("garbage"), 0o600),
23+
"writing partial template copy")
24+
return errors.New("template poisoned")
25+
})
26+
27+
d, err := db.Open(path)
28+
require.NoError(t, err, "opening fallback test db")
29+
require.NoError(t, d.Close(), "closing fallback test db")
30+
}
31+
32+
func TestEnsureTestDBAtLeavesExistingFileIntact(t *testing.T) {
33+
dir := t.TempDir()
34+
path := filepath.Join(dir, "test.db")
35+
36+
EnsureTestDBAt(t, path)
37+
info, err := os.Stat(path)
38+
require.NoError(t, err, "statting created test db")
39+
40+
// A second call must not rebuild or replace the existing file,
41+
// and must not consult the template at all.
42+
ensureTestDBAtWith(t, path, func(string) error {
43+
t.Fatal("copyTemplate must not run for an existing file")
44+
return nil
45+
})
46+
again, err := os.Stat(path)
47+
require.NoError(t, err, "statting test db after second ensure")
48+
require.Equal(t, info.ModTime(), again.ModTime(),
49+
"existing test db was modified")
50+
}

internal/server/analytics_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ func buildAnalyticsDBFixture(
259259
)
260260
}
261261
stats := seed(t, &testEnv{db: database})
262-
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
262+
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
263263
defer cancel()
264264
checkpointErr := database.CheckpointWALTruncate(ctx)
265265
closeErr := database.Close()

0 commit comments

Comments
 (0)