From d2b284013d8009e1ef3d3dabc76c6315d741840b Mon Sep 17 00:00:00 2001 From: Julian Dice <19397727+windoze95@users.noreply.github.com> Date: Sun, 12 Jul 2026 23:01:00 -0500 Subject: [PATCH] fix(test): give the async polling helpers a deadline that survives a loaded suite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TestVideoImport_NativeVideoUsed failed a full-suite run with "video import job 1 did not finish in time" and then passed 3/3 in isolation. Nothing was broken — the helper polls a background goroutine against a hard 3s wall-clock deadline, which is really a timing assertion on the Go scheduler. Under `go test ./...` (the CI deploy gate) every package compiles and runs at once, and a goroutine can be starved past 3s. Five helpers had 2-3s deadlines: video job completion, the quota-refund counter, multi-recipe resolution, the video status endpoint, and hub room teardown. They now share testutil.AsyncDeadline (30s), which documents why it's generous. This costs nothing. Every one of those loops exits the moment the work lands, so the deadline is never actually waited on in a passing run — TestVideoImport_ NativeVideoUsed still completes in 0.32s. It only bounds how long a genuinely stuck test waits before failing. A flaky deploy gate gets ignored, and an ignored gate is worse than a slow one. Left finder_extraction_fix_test.go alone; it already allows 90s. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0194PdH4wDTnz5SWfzyoKagc --- internal/handlers/import_video_handler_test.go | 2 +- internal/service/import_video_test.go | 4 ++-- internal/service/multi_recipe_resolver_test.go | 2 +- internal/testutil/async.go | 18 ++++++++++++++++++ internal/ws/hub_rooms_test.go | 4 +++- 5 files changed, 25 insertions(+), 5 deletions(-) create mode 100644 internal/testutil/async.go diff --git a/internal/handlers/import_video_handler_test.go b/internal/handlers/import_video_handler_test.go index 25b2ba1..b97883b 100644 --- a/internal/handlers/import_video_handler_test.go +++ b/internal/handlers/import_video_handler_test.go @@ -185,7 +185,7 @@ func TestImportFromVideo_Accepted_AndPoll(t *testing.T) { // Poll the status endpoint until the async job completes. path := "/recipes/import/video/" + itoa(accepted.Job.ID) - deadline := time.Now().Add(3 * time.Second) + deadline := time.Now().Add(testutil.AsyncDeadline) var lastStatus string for time.Now().Before(deadline) { gw := httptest.NewRecorder() diff --git a/internal/service/import_video_test.go b/internal/service/import_video_test.go index 65d8004..4a110d2 100644 --- a/internal/service/import_video_test.go +++ b/internal/service/import_video_test.go @@ -184,7 +184,7 @@ func newVideoTestService(repo *testutil.MockRecipeRepo, vrepo *testutil.MockVide // waitForVideoJob polls until the job reaches a terminal state or times out. func waitForVideoJob(t *testing.T, svc *ImportService, id uint) *models.VideoImport { t.Helper() - deadline := time.Now().Add(3 * time.Second) + deadline := time.Now().Add(testutil.AsyncDeadline) for time.Now().Before(deadline) { job, err := svc.GetVideoImport(id) if err == nil && (job.Status == models.VideoImportDone || job.Status == models.VideoImportFailed) { @@ -470,7 +470,7 @@ func TestVideoImport_RefundsQuotaOnFailure(t *testing.T) { } // The refund runs in the goroutine after the job is marked failed; poll the // lock-synchronized counter until it lands. - deadline := time.Now().Add(2 * time.Second) + deadline := time.Now().Add(testutil.AsyncDeadline) for time.Now().Before(deadline) && userRepo.SubscriptionUsage(user.ID, "video_imports_used") != 0 { time.Sleep(5 * time.Millisecond) } diff --git a/internal/service/multi_recipe_resolver_test.go b/internal/service/multi_recipe_resolver_test.go index 695be6f..1c4e7d3 100644 --- a/internal/service/multi_recipe_resolver_test.go +++ b/internal/service/multi_recipe_resolver_test.go @@ -63,7 +63,7 @@ func newResolverForTest(preview ai.TextProvider, canonicalRepo *testutil.MockCan // waitResolved polls the entry until background extraction finishes. func waitResolved(t *testing.T, entry *MultiRecipeEntry) { t.Helper() - deadline := time.Now().Add(3 * time.Second) + deadline := time.Now().Add(testutil.AsyncDeadline) for time.Now().Before(deadline) { if entry.GetStatus() == "resolved" { return diff --git a/internal/testutil/async.go b/internal/testutil/async.go new file mode 100644 index 0000000..11f87a9 --- /dev/null +++ b/internal/testutil/async.go @@ -0,0 +1,18 @@ +package testutil + +import "time" + +// AsyncDeadline bounds the test helpers that poll for a background goroutine to +// finish (video import jobs, multi-recipe resolution, hub room teardown). +// +// It is deliberately generous, and that costs nothing: every one of those loops +// exits the moment the work lands, so the happy path is unaffected. The deadline +// only decides how long a genuinely stuck test waits before failing. +// +// A tight bound here is really a timing assertion on the Go scheduler. Under a +// loaded parallel suite — `go test ./...` compiling and running every package at +// once, which is exactly what the CI deploy gate does — a background goroutine +// can be starved for seconds. That is how TestVideoImport_NativeVideoUsed failed +// at a 3-second deadline while passing in isolation: nothing was broken, the +// machine was just busy. Flaky gates get ignored, so buy the margin. +const AsyncDeadline = 30 * time.Second diff --git a/internal/ws/hub_rooms_test.go b/internal/ws/hub_rooms_test.go index eaea47d..969ab83 100644 --- a/internal/ws/hub_rooms_test.go +++ b/internal/ws/hub_rooms_test.go @@ -3,6 +3,8 @@ package ws import ( "testing" "time" + + "github.com/windoze95/saltybytes-api/internal/testutil" ) // roomExists checks the hub's room map under its lock. @@ -16,7 +18,7 @@ func roomExists(h *Hub, roomID string) bool { // waitForRoomGone polls until the room disappears from the hub or times out. func waitForRoomGone(t *testing.T, h *Hub, roomID string) { t.Helper() - deadline := time.Now().Add(2 * time.Second) + deadline := time.Now().Add(testutil.AsyncDeadline) for roomExists(h, roomID) { if time.Now().After(deadline) { t.Fatalf("room %q was not removed from the hub", roomID)