Skip to content

Commit 8f1ef58

Browse files
tae2089claude
andcommitted
Repair broken and flaky tests exposed by live-Postgres and load runs
- RebuildNodes tests staged a stale tsv via UPDATE, but the BEFORE UPDATE trigger recomputed tsv from content and erased the stale state, failing both tests against a real Postgres. Disable the trigger during staging so RebuildNodes' own SQL is what gets verified. - The rapid-push dedup test raced three Adds against an already-running worker and flaked under parallel load. Hold the single worker with a gated blocker repo so all pushes deterministically arrive while queued. - main_postgres_test referenced migrateSchemaVersion, a type renamed to migration.MigrationSchemaVersion; the postgres-tagged file no longer compiled (unnoticed because CI never builds that tag). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 54ef7c5 commit 8f1ef58

3 files changed

Lines changed: 37 additions & 12 deletions

File tree

cmd/ccg/main_postgres_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ func TestRunMigrations_PostgresDownRestoresNullableColumns(t *testing.T) {
157157
t.Fatalf("run down migration: %v", err)
158158
}
159159

160-
var version migrateSchemaVersion
160+
var version migration.MigrationSchemaVersion
161161
if err := db.Table("schema_migrations").First(&version).Error; err != nil {
162162
t.Fatalf("load schema version: %v", err)
163163
}
@@ -190,7 +190,7 @@ func TestRunMigrations_PostgresDownFromVersionThreeDropsPolicyTables(t *testing.
190190
t.Fatalf("run down migration: %v", err)
191191
}
192192

193-
var version migrateSchemaVersion
193+
var version migration.MigrationSchemaVersion
194194
if err := db.Table("schema_migrations").First(&version).Error; err != nil {
195195
t.Fatalf("load schema version: %v", err)
196196
}

internal/store/search/postgres_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,15 @@ func TestPostgresFTS_Rebuild(t *testing.T) {
133133
}
134134
}
135135

136+
// disableTSVTrigger turns off the tsv BEFORE INSERT/UPDATE trigger so tests can stage
137+
// stale tsv values by hand. Tables are dropped per test, so re-enabling is unnecessary.
138+
func disableTSVTrigger(t *testing.T, db *gorm.DB) {
139+
t.Helper()
140+
if err := db.Exec("ALTER TABLE search_documents DISABLE TRIGGER trg_search_documents_tsv").Error; err != nil {
141+
t.Fatalf("disable tsv trigger: %v", err)
142+
}
143+
}
144+
136145
func TestPostgresFTS_RebuildNodes_RefreshesOnlyScopedRows(t *testing.T) {
137146
db := setupPostgresDB(t)
138147
backend := NewPostgresBackend()
@@ -157,6 +166,10 @@ func TestPostgresFTS_RebuildNodes_RefreshesOnlyScopedRows(t *testing.T) {
157166
if err := db.Create(&model.SearchDocument{Namespace: "ns-b", NodeID: foreign.ID, Content: "fresh foreign", Language: "go"}).Error; err != nil {
158167
t.Fatal(err)
159168
}
169+
// The tsv BEFORE UPDATE trigger would recompute tsv from content and overwrite the
170+
// manual stale marker; disable it so the stale state actually exists and RebuildNodes'
171+
// own UPDATE statement is what gets verified.
172+
disableTSVTrigger(t, db)
160173
if err := db.Exec("UPDATE search_documents SET tsv = to_tsvector('simple', 'stale')").Error; err != nil {
161174
t.Fatal(err)
162175
}
@@ -193,6 +206,9 @@ func TestPostgresFTS_RebuildNodes_EmptyScopeIsNoOp(t *testing.T) {
193206
if err := db.Create(&model.SearchDocument{Namespace: ctxns.DefaultNamespace, NodeID: node.ID, Content: "fresh keep", Language: "go"}).Error; err != nil {
194207
t.Fatal(err)
195208
}
209+
// See TestPostgresFTS_RebuildNodes_RefreshesOnlyScopedRows: the trigger must be off
210+
// for the stale marker to persist.
211+
disableTSVTrigger(t, db)
196212
if err := db.Exec("UPDATE search_documents SET tsv = to_tsvector('simple', 'stale')").Error; err != nil {
197213
t.Fatal(err)
198214
}

internal/webhook/syncqueue_test.go

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,35 +14,44 @@ import (
1414
)
1515

1616
func TestSyncQueue_DeduplicatesRapidPushes(t *testing.T) {
17-
var callCount atomic.Int32
18-
done := make(chan struct{})
17+
var svcCalls atomic.Int32
18+
gate := make(chan struct{})
19+
svcDone := make(chan struct{})
1920

2021
handler := func(_ context.Context, repoFullName, cloneURL, branch string) error {
21-
callCount.Add(1)
22-
time.Sleep(50 * time.Millisecond)
23-
if callCount.Load() == 1 {
24-
close(done)
22+
switch repoFullName {
23+
case "org/blocker":
24+
<-gate
25+
case "org/svc":
26+
if svcCalls.Add(1) == 1 {
27+
close(svcDone)
28+
}
2529
}
2630
return nil
2731
}
2832

29-
q := NewSyncQueue(2, handler)
33+
// A single worker held by the gated blocker guarantees all three pushes arrive while
34+
// org/svc is queued (never processing), so dedup must collapse them into one run.
35+
// Racing the pushes against a free worker made this test flaky under parallel load.
36+
q := NewSyncQueue(1, handler)
3037
defer q.Shutdown()
3138

39+
q.Add(context.Background(), "org/blocker", "https://github.com/org/blocker.git", "main")
3240
q.Add(context.Background(), "org/svc", "https://github.com/org/svc.git", "main")
3341
q.Add(context.Background(), "org/svc", "https://github.com/org/svc.git", "main")
3442
q.Add(context.Background(), "org/svc", "https://github.com/org/svc.git", "main")
43+
close(gate)
3544

3645
select {
37-
case <-done:
46+
case <-svcDone:
3847
case <-time.After(5 * time.Second):
3948
t.Fatal("timed out waiting for handler")
4049
}
4150

51+
// Grace window so an erroneous duplicate run would surface before the assertion.
4252
time.Sleep(100 * time.Millisecond)
4353

44-
got := callCount.Load()
45-
if got != 1 {
54+
if got := svcCalls.Load(); got != 1 {
4655
t.Errorf("handler called %d times, want 1 (dedup failed)", got)
4756
}
4857
}

0 commit comments

Comments
 (0)