From 757444d50549d8cb1b85c235a52de639c35f7bdd Mon Sep 17 00:00:00 2001 From: MHSanaei Date: Sun, 1 Nov 2026 14:56:40 +0100 Subject: [PATCH] fix(tgbot): let a screen-art reset outlast a probe still in flight TestScreenArtReprobeWindow failed under -race -shuffle in CI: the probe slot is process-wide, and a probe an earlier test started against its own (now closing) server still held it when the next test asked for a re-probe, so that request was dropped and the stale avatar never re-probed. The leftover probe could also write its result over the state the next test had just reset. Probes now run in a WaitGroup that resetScreenArt waits on, so a reset between mock servers always starts from an idle probe slot. --- internal/web/service/tgbot/screens_art.go | 7 ++++-- internal/web/service/tgbot/screens_test.go | 28 ++++++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/internal/web/service/tgbot/screens_art.go b/internal/web/service/tgbot/screens_art.go index 44887a7c9..0ae4b5bdc 100644 --- a/internal/web/service/tgbot/screens_art.go +++ b/internal/web/service/tgbot/screens_art.go @@ -32,6 +32,8 @@ var ( artProbedAt time.Time // artProbeRunning keeps a burst of renders from starting a burst of probes. artProbeRunning atomic.Bool + // artProbes lets resetScreenArt outlast a probe still running against a previous bot. + artProbes sync.WaitGroup blackTileOnce sync.Once blackTileBytes []byte @@ -109,10 +111,10 @@ func (t *Tgbot) ensureScreenArt() { if !artProbeRunning.CompareAndSwap(false, true) { return } - go func() { + artProbes.Go(func() { defer artProbeRunning.Store(false) t.resolveScreenArt() - }() + }) } // artFileID returns the cached file_id for a screen kind. A fallback tile cached @@ -163,6 +165,7 @@ func cacheArtFileID(kind, fileID string) { // resetScreenArt drops the cached file_ids; tests use it between mock servers. func resetScreenArt() { + artProbes.Wait() screenArtMu.Lock() screenArtIDs = map[string]string{} artOverrides = map[string]string{} diff --git a/internal/web/service/tgbot/screens_test.go b/internal/web/service/tgbot/screens_test.go index 34ccb68ae..048c0de4d 100644 --- a/internal/web/service/tgbot/screens_test.go +++ b/internal/web/service/tgbot/screens_test.go @@ -508,6 +508,34 @@ func TestScreenArtReprobeWindow(t *testing.T) { } } +// A probe a previous test left in flight held the process-wide probe slot, so under -race -shuffle +// the next test's stale avatar was never re-probed (CI flake in TestScreenArtReprobeWindow). +func TestScreenArtResetWaitsForAProbeLeftInFlight(t *testing.T) { + slow := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + time.Sleep(300 * time.Millisecond) + http.Error(w, "gone", http.StatusServiceUnavailable) + })) + defer slow.Close() + swapTestBot(t, slow.URL) + (&Tgbot{}).ensureScreenArt() + + srv, calls := recordingServer(t, false) + defer srv.Close() + swapTestBot(t, srv.URL) + screenArtMu.Lock() + artProbedAt = time.Now().Add(-2 * time.Minute) + screenArtMu.Unlock() + + (&Tgbot{}).ensureScreenArt() + for range 100 { + if countMethod(calls(), "getUserProfilePhotos") > 0 { + return + } + time.Sleep(10 * time.Millisecond) + } + t.Fatal("the probe left in flight by the previous bot kept the new one from probing") +} + func TestScreenKeepsArtFromTheBotAvatar(t *testing.T) { tb, calls := newScreenTgbot(t, false) tb.resolveScreenArt()