From 3e5450d41ac57e843a8b9d9843eb47b00077859d Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sun, 4 Oct 2026 16:37:10 +0000 Subject: [PATCH] Make the periodic reconciliation test wait for the startup pass (closes #189) TestPeriodicReconciliationAdoptsFileThatAppearsAfterStartup slept for three eviction intervals before writing its file, so on a slow start the startup pass could still be running and adopt the file itself, and the test passed without a periodic pass. It now holds the test database's only connection until the startup pass waits for it, writes the file and lets the connection go, as TestEvictionRunsOnPeriodicSchedule does, so only a periodic reconciliation pass can adopt the file. Test only. Model: opus-5-5 --- TODO.md | 8 ++++++ internal/imgcache/eviction_internal_test.go | 31 +++++++++++++++------ 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/TODO.md b/TODO.md index 3c1d04b..8c3c6df 100644 --- a/TODO.md +++ b/TODO.md @@ -31,6 +31,14 @@ P2: security: referer blacklist # Completed Steps +- 2026-10-04 `TestPeriodicReconciliationAdoptsFileThatAppearsAfterStartup` + only passes through a periodic pass (closes #189): it slept for three + eviction intervals before writing its file, and a startup pass still running + then could adopt the file itself. It now holds the test database's only + connection until the startup pass waits for it after walking the empty + variant directory, writes the file and lets the connection go, as + `TestEvictionRunsOnPeriodicSchedule` does, so only a periodic reconciliation + pass can adopt the file. Test only. - 2026-10-04 logging in, logging out, the URL generator and `/v1/e/` have handler tests (closes #77): new tests in `internal/handlers`, with no network, check that `GET /` without a login session shows the login form; a diff --git a/internal/imgcache/eviction_internal_test.go b/internal/imgcache/eviction_internal_test.go index 4b0bbf7..af91ac5 100644 --- a/internal/imgcache/eviction_internal_test.go +++ b/internal/imgcache/eviction_internal_test.go @@ -847,15 +847,28 @@ func TestPeriodicReconciliationAdoptsFileThatAppearsAfterStartup(t *testing.T) { cache, _ := newEvictionTestCache(t, 1<<30) - const interval = 100 * time.Millisecond + // Hold the test database's only connection, so the startup pass + // waits for it after walking the still empty variant directory: the + // file written while it waits is first seen by a periodic pass. + conn, err := cache.db.Conn(t.Context()) + if err != nil { + t.Fatalf("failed to take the database connection: %v", err) + } - cache.StartEviction(interval) + defer func() { _ = conn.Close() }() + + cache.StartEviction(100 * time.Millisecond) defer func() { _ = cache.StopEviction(t.Context()) }() - // Let startup reconciliation run and settle on an empty cache - // before introducing the untracked file, so the adoption we assert - // below can only be the work of a later, periodic pass. - time.Sleep(3 * interval) + deadline := time.Now().Add(5 * time.Second) + + for cache.db.Stats().WaitCount == 0 { + if time.Now().After(deadline) { + t.Fatal("the startup pass never waited for the database") + } + + time.Sleep(10 * time.Millisecond) + } // Simulate a variant whose accounting insert failed after the // process was already running and serving requests: the content @@ -864,14 +877,16 @@ func TestPeriodicReconciliationAdoptsFileThatAppearsAfterStartup(t *testing.T) { // insert had failed and only the file write had succeeded. untracked := bytes.Repeat([]byte{0x41}, 900) - _, err := cache.variants.Store( + _, err = cache.variants.Store( "aabbccdd0099", bytes.NewReader(untracked), "image/webp", ) if err != nil { t.Fatalf("failed to store untracked variant file: %v", err) } - deadline := time.Now().Add(5 * time.Second) + _ = conn.Close() + + deadline = time.Now().Add(5 * time.Second) var usage int64