From 6068a85f8dcc28ae14dda830277ecbbf98a84142 Mon Sep 17 00:00:00 2001 From: Brandur Date: Sat, 26 Apr 2025 13:31:28 -0700 Subject: [PATCH] Fix intermittent test: `TestNotifier/MultipleSubscribersStress` Try to fix one of the most commonly failing intermittent tests we have left, which is the stress test in the notifier [1]. --- FAIL: TestNotifier (0.00s) --- FAIL: TestNotifier/MultipleSubscribersStress (0.98s) notifier_test.go:409: Generated schema "notifier_2025_04_26t20_08_50_schema_04" with migrations [1 2 3 4 5 6] on line "main" in 376.579906ms [4 generated] [0 reused] notifier_test.go:434: Sending notification on "test_topic1": msg0 notifier_test.go:434: Sending notification on "test_topic1": msg1 notifier_test.go:434: Sending notification on "test_topic1": msg2 notifier_test.go:434: Sending notification on "test_topic1": msg3 notifier_test.go:434: Sending notification on "test_topic1": msg4 notifier_test.go:434: Sending notification on "test_topic1": msg5 notifier_test.go:434: Sending notification on "test_topic1": msg6 notifier_test.go:472: Channel 0 contains 5 message(s) notifier_test.go:472: Channel 1 contains 7 message(s) notifier_test.go:472: Channel 2 contains 0 message(s) notifier_test.go:477: Error Trace: /home/runner/work/river/river/internal/notifier/notifier_test.go:477 Error: Should NOT be empty, but was 0xc000244770 Test: TestNotifier/MultipleSubscribersStress riverdbtest.go:277: Checked in schema "notifier_2025_04_26t20_08_50_schema_04"; 1 idle schema(s) [4 generated] [2 reused] FAIL FAIL github.com/riverqueue/river/internal/notifier 2.025s Rebuild the test a little so that each listen goroutine waits for at least one message to come through instead of only an arbitrary sleep. The sleep ~always passes locally, but it seems to be that in CI it's possible for the sending goroutine to be entirely parked around when a listen/unlisten loop is happening. [1] https://github.com/riverqueue/river/actions/runs/14684674639/job/41211780010 --- internal/notifier/notifier_test.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/internal/notifier/notifier_test.go b/internal/notifier/notifier_test.go index 15d7550c..28e75dec 100644 --- a/internal/notifier/notifier_test.go +++ b/internal/notifier/notifier_test.go @@ -459,6 +459,13 @@ func TestNotifier(t *testing.T) { // Pause a random brief amount of time. serviceutil.CancellableSleep(ctx, randutil.DurationBetween(15*time.Millisecond, 50*time.Millisecond)) + // Wait for at least one message to come through. This + // generally isn't necessary, but we can run into strange + // problems in slow environments like GitHub Actions where + // the send goroutine can apparently be paused entirely + // around the sleep above between listen and unlisten. + riversharedtest.WaitOrTimeout(t, notifyChan) + sub.Unlisten(ctx) } }() @@ -469,12 +476,12 @@ func TestNotifier(t *testing.T) { <-sendNotificationsDone // wait for notifications goroutine to finish for i := range notifyChans { - t.Logf("Channel %2d contains %3d message(s)", i, len(notifyChans[i])) - // Don't require a specific number of messages to have been received // since it's non-deterministic, but every channel should've gotten - // at least one message. It my test runs, they receive ~15 each. - require.NotEmpty(t, notifyChans[i]) + // at least one message (as measured by the WaitOrTimeout above). + // In my test runs, they receive ~15 each. The WaitOrTimeout + // consumes one message, so we add +1 when measuring each channel. + t.Logf("Channel %2d contains %3d message(s)", i, len(notifyChans[i])+1) } })