From 7e1e269d36459bfa4bd7f570b296186cbd975f5d Mon Sep 17 00:00:00 2001 From: Brandur Leach Date: Thu, 4 Jul 2024 07:24:31 -0700 Subject: [PATCH] Fix more flaky periodic job enqueuer `*AfterStart` tests (#422) This one continues #416. I fixed a pair of the `*AfterStart` tests, but didn't look further to see that there were three more, and I guess by a stroke of luck I was running GitHub Actions at the right time and didn't encounter anymore failures (today I immediately started seeing them again when pushing). Rationale there is similar to #416: * 500 ms is a short enough period that it seems to be possible in GitHub Actions for that amount of time to have elapsed by the time we check for the first set of jobs (amazing, I know, but I can't explain it any other way). * The way the tests are written, they're also very slow because each requires waiting for one of the 500 ms periods to elapse, so the minimum run time of each is 500 ms. Technically, with the rewrites it can be argued that the test cases are checking slightly less, but IMO it's good enough, and I was having trouble thinking of alternative ways to add additional checks without raciness. Fixes #386. --- .../maintenance/periodic_job_enqueuer_test.go | 50 +++++++------------ 1 file changed, 19 insertions(+), 31 deletions(-) diff --git a/internal/maintenance/periodic_job_enqueuer_test.go b/internal/maintenance/periodic_job_enqueuer_test.go index 5cead754..a4efd008 100644 --- a/internal/maintenance/periodic_job_enqueuer_test.go +++ b/internal/maintenance/periodic_job_enqueuer_test.go @@ -401,28 +401,25 @@ func TestPeriodicJobEnqueuer(t *testing.T) { startService(t, svc) handles := svc.AddMany([]*PeriodicJob{ - {ScheduleFunc: periodicIntervalSchedule(500 * time.Millisecond), ConstructorFunc: jobConstructorFunc("periodic_job_500ms", false)}, - {ScheduleFunc: periodicIntervalSchedule(500 * time.Millisecond), ConstructorFunc: jobConstructorFunc("periodic_job_500ms_start", false), RunOnStart: true}, + {ScheduleFunc: periodicIntervalSchedule(5 * time.Second), ConstructorFunc: jobConstructorFunc("periodic_job_5s", false)}, + {ScheduleFunc: periodicIntervalSchedule(5 * time.Second), ConstructorFunc: jobConstructorFunc("periodic_job_5s_start", false), RunOnStart: true}, }) svc.TestSignals.InsertedJobs.WaitOrTimeout() - requireNJobs(t, bundle.exec, "periodic_job_500ms", 0) - requireNJobs(t, bundle.exec, "periodic_job_500ms_start", 1) + requireNJobs(t, bundle.exec, "periodic_job_5s", 0) + requireNJobs(t, bundle.exec, "periodic_job_5s_start", 1) svc.Clear() + require.Empty(t, svc.periodicJobs) + handleAfterClear := svc.Add( - &PeriodicJob{ScheduleFunc: periodicIntervalSchedule(500 * time.Millisecond), ConstructorFunc: jobConstructorFunc("periodic_job_500ms_new", false)}, + &PeriodicJob{ScheduleFunc: periodicIntervalSchedule(5 * time.Second), ConstructorFunc: jobConstructorFunc("periodic_job_5s_new", false)}, ) // Handles are not reused. require.NotEqual(t, handles[0], handleAfterClear) require.NotEqual(t, handles[1], handleAfterClear) - - svc.TestSignals.InsertedJobs.WaitOrTimeout() - requireNJobs(t, bundle.exec, "periodic_job_500ms", 0) // same as before - requireNJobs(t, bundle.exec, "periodic_job_500ms_start", 1) // same as before - requireNJobs(t, bundle.exec, "periodic_job_500ms_new", 1) // new row }) t.Run("RemoveAfterStart", func(t *testing.T) { @@ -433,21 +430,17 @@ func TestPeriodicJobEnqueuer(t *testing.T) { startService(t, svc) handles := svc.AddMany([]*PeriodicJob{ - {ScheduleFunc: periodicIntervalSchedule(500 * time.Millisecond), ConstructorFunc: jobConstructorFunc("periodic_job_500ms", false)}, - {ScheduleFunc: periodicIntervalSchedule(500 * time.Millisecond), ConstructorFunc: jobConstructorFunc("periodic_job_500ms_start", false), RunOnStart: true}, + {ScheduleFunc: periodicIntervalSchedule(5 * time.Second), ConstructorFunc: jobConstructorFunc("periodic_job_5s", false)}, + {ScheduleFunc: periodicIntervalSchedule(5 * time.Second), ConstructorFunc: jobConstructorFunc("periodic_job_5s_start", false), RunOnStart: true}, }) svc.TestSignals.InsertedJobs.WaitOrTimeout() - requireNJobs(t, bundle.exec, "periodic_job_500ms", 0) - requireNJobs(t, bundle.exec, "periodic_job_500ms_start", 1) + requireNJobs(t, bundle.exec, "periodic_job_5s", 0) + requireNJobs(t, bundle.exec, "periodic_job_5s_start", 1) svc.Remove(handles[1]) - // Each is one because the second job was removed before it was worked - // again. - svc.TestSignals.InsertedJobs.WaitOrTimeout() - requireNJobs(t, bundle.exec, "periodic_job_500ms", 1) - requireNJobs(t, bundle.exec, "periodic_job_500ms_start", 1) + require.Len(t, svc.periodicJobs, 1) }) t.Run("RemoveManyAfterStart", func(t *testing.T) { @@ -458,24 +451,19 @@ func TestPeriodicJobEnqueuer(t *testing.T) { startService(t, svc) handles := svc.AddMany([]*PeriodicJob{ - {ScheduleFunc: periodicIntervalSchedule(500 * time.Millisecond), ConstructorFunc: jobConstructorFunc("periodic_job_500ms", false)}, - {ScheduleFunc: periodicIntervalSchedule(500 * time.Millisecond), ConstructorFunc: jobConstructorFunc("periodic_job_500ms_other", false)}, - {ScheduleFunc: periodicIntervalSchedule(500 * time.Millisecond), ConstructorFunc: jobConstructorFunc("periodic_job_500ms_start", false), RunOnStart: true}, + {ScheduleFunc: periodicIntervalSchedule(5 * time.Second), ConstructorFunc: jobConstructorFunc("periodic_job_5s", false)}, + {ScheduleFunc: periodicIntervalSchedule(5 * time.Second), ConstructorFunc: jobConstructorFunc("periodic_job_5s_other", false)}, + {ScheduleFunc: periodicIntervalSchedule(5 * time.Second), ConstructorFunc: jobConstructorFunc("periodic_job_5s_start", false), RunOnStart: true}, }) svc.TestSignals.InsertedJobs.WaitOrTimeout() - requireNJobs(t, bundle.exec, "periodic_job_500ms", 0) - requireNJobs(t, bundle.exec, "periodic_job_500ms_other", 0) - requireNJobs(t, bundle.exec, "periodic_job_500ms_start", 1) + requireNJobs(t, bundle.exec, "periodic_job_5s", 0) + requireNJobs(t, bundle.exec, "periodic_job_5s_other", 0) + requireNJobs(t, bundle.exec, "periodic_job_5s_start", 1) svc.RemoveMany([]rivertype.PeriodicJobHandle{handles[1], handles[2]}) - // Each is one because the second job was removed before it was worked - // again. - svc.TestSignals.InsertedJobs.WaitOrTimeout() - requireNJobs(t, bundle.exec, "periodic_job_500ms", 1) - requireNJobs(t, bundle.exec, "periodic_job_500ms_other", 0) - requireNJobs(t, bundle.exec, "periodic_job_500ms_start", 1) + require.Len(t, svc.periodicJobs, 1) }) // To suss out any race conditions in the add/remove/clear/run loop code,