TODO.md

@@ -29,6 +29,15 @@ P2: security: referer blacklist
 
 # Completed Steps
 
+- 2026-10-04 `TestEvictionRunsOnPeriodicSchedule` no longer races the evictor
+  (closes #183): it wrote each variant file and then inserted its accounting row
+  by hand, and a reconciliation pass between the two adopted the file first, so
+  the insert failed. It now writes the files only, while holding the test
+  database's only connection so the evictor's startup pass waits after walking
+  the empty variant directory; a periodic reconciliation pass then adopts the
+  files and the eviction pass after it evicts them. No other test in
+  `internal/imgcache` inserts a row by hand after starting the evictor. Test
+  only.
 - 2026-10-04 deployment guide and example Caddy config (closes #89):
   "Deployment" in `README.md` says what the reverse proxy in front of pixa must
   do (terminate TLS; pass `Host`, `Origin` and `Referer` on unchanged; set

internal/imgcache/eviction_internal_test.go

@@ -681,6 +681,11 @@ func TestEvictionRunsUnderWritePressure(t *testing.T) {
 	assertNoDanglingReferences(t, cache)
 }
 
+// TestEvictionRunsOnPeriodicSchedule writes three variant files straight
+// to disk, bypassing StoreVariant, so they have no accounting rows and no
+// write-pressure notification fires. Only a periodic reconciliation pass
+// can then adopt them, and only the eviction pass that follows it can
+// evict them.
 func TestEvictionRunsOnPeriodicSchedule(t *testing.T) {
 	t.Parallel()
 
@@ -688,13 +693,29 @@ func TestEvictionRunsOnPeriodicSchedule(t *testing.T) {
 
 	cache, _ := newEvictionTestCache(t, limit)
 
-	// Start the evictor while the cache is empty, then create tracked
+	// Hold the test database's only connection, so the startup pass
-	// over-limit state WITHOUT going through the store methods, so no
+	// waits for it after walking the still empty variant directory: the
-	// write-pressure notification fires and only the periodic ticker
+	// files written while it waits are first seen by a periodic pass.
-	// can trigger eviction.
+	conn, err := cache.db.Conn(t.Context())
+	if err != nil {
+		t.Fatalf("failed to take the database connection: %v", err)
+	}
+
+	defer func() { _ = conn.Close() }()
+
 	cache.StartEviction(100 * time.Millisecond)
 	defer func() { _ = cache.StopEviction(t.Context()) }()
 
+	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)
+	}
+
 	keys := []VariantKey{
 		testVariantKeyOne, testVariantKeyTwo, testVariantKeyThree,
 	}
@@ -703,25 +724,43 @@ func TestEvictionRunsOnPeriodicSchedule(t *testing.T) {
 	for i, key := range keys {
 		content := bytes.Repeat([]byte{fills[i]}, 1000)
 
-		_, err := cache.variants.Store(key, bytes.NewReader(content), "image/webp")
+		_, err = cache.variants.Store(key, bytes.NewReader(content), "image/webp")
 		if err != nil {
 			t.Fatalf("failed to store variant file: %v", err)
 		}
+	}
 
-		_, err = cache.db.ExecContext(t.Context(),
+	_ = conn.Close()
-			`INSERT INTO variant_content (cache_key, size_bytes, content_type)
+
-			 VALUES (?, ?, ?)`,
+	// Only one of the 1000-byte files fits under the limit: wait until
-			string(key), len(content), "image/webp",
+	// the evictor has removed the other two.
-		)
+	stored := len(keys)
-		if err != nil {
+	deadline = time.Now().Add(5 * time.Second)
-			t.Fatalf("failed to insert variant accounting row: %v", err)
+
+	for stored > 1 && time.Now().Before(deadline) {
+		time.Sleep(25 * time.Millisecond)
+
+		stored = 0
+
+		for _, key := range keys {
+			if cache.variants.Exists(key) {
+				stored++
+			}
 		}
 	}
 
-	usage := waitForUsageAtOrBelow(t, cache, limit, 5*time.Second)
+	if stored > 1 {
+		t.Fatalf("periodic schedule did not trigger eviction: %d of %d "+
+			"variant files still on disk, want at most 1", stored, len(keys))
+	}
+
+	usage, err := cache.UsageBytes(t.Context())
+	if err != nil {
+		t.Fatalf("UsageBytes failed: %v", err)
+	}
+
 	if usage > limit {
-		t.Errorf("periodic schedule did not trigger eviction: usage = %d, want <= %d",
+		t.Errorf("usage after eviction = %d, want <= %d", usage, limit)
-			usage, limit)
 	}
 
 	assertNoDanglingReferences(t, cache)