From 7f3f23549deda9f8a8069f2312234e53d1c6870b Mon Sep 17 00:00:00 2001 From: Brandur Date: Thu, 13 Jun 2024 19:16:11 -0700 Subject: [PATCH] Fix for intermittency in unique job tests that use `ByPeriod` Fix for #392 in which under rare cases, a test can start right up against the boundary of a period in use for unique opts, cross over that period, and produce the wrong result as a new row is inserted. For example, using a period of 15 minutes like in `db_unique_test.go`, if the tests run at 14:59:59.998, there's a good chance that they'll finish after 15:00:00, thereby encapsulating two separate 15 minutes and allowing separate jobs to be inserted. This is more likely to happen in CI where tests take long to run. The fix is to use a more stable time for this time of test. We use the current hour, but assign a fixed minute and second that's right in the middle and nowhere near any boundaries. I gave `client_test.go`'s unique tests the same treatment since it's possible for this problem to happen there too, although they're using a 24 hour period so it was already very unlikely. (The test case would have to start at exactly the wrong moment before a UTC day boundary.) Fixes #392. --- client_test.go | 10 ++++++++++ internal/dbunique/db_unique_test.go | 8 +++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/client_test.go b/client_test.go index 0701259a..d60ed180 100644 --- a/client_test.go +++ b/client_test.go @@ -4539,6 +4539,16 @@ func TestUniqueOpts(t *testing.T) { client := newTestClient(t, dbPool, newTestConfig(t, nil)) + // Tests that use ByPeriod below can be sensitive to intermittency if + // the tests run at say 23:59:59.998, then it's possible to accidentally + // cross a period threshold, even if very unlikely. So here, seed mostly + // the current time, but make sure it's nicened up a little to be + // roughly in the middle of the hour and well clear of any period + // boundaries. + client.uniqueInserter.TimeNowUTC = func() time.Time { + return time.Now().Truncate(1 * time.Hour).Add(37*time.Minute + 23*time.Second) + } + return client, &testBundle{} } diff --git a/internal/dbunique/db_unique_test.go b/internal/dbunique/db_unique_test.go index 87e6ebab..8fe255c9 100644 --- a/internal/dbunique/db_unique_test.go +++ b/internal/dbunique/db_unique_test.go @@ -41,7 +41,13 @@ func TestUniqueInserter_JobInsert(t *testing.T) { ) bundle := &testBundle{ - baselineTime: time.Now(), + // Tests that use ByPeriod below can be sensitive to intermittency + // if the tests run at say 14:59:59.998, then it's possible to + // accidentally cross a period threshold, even if very unlikely. So + // here, seed mostly the current time, but make sure it's nicened up + // a little to be roughly in the middle of the hour and well clear + // of any period boundaries. + baselineTime: time.Now().Truncate(1 * time.Hour).Add(37*time.Minute + 23*time.Second), driver: driver, exec: driver.UnwrapExecutor(tx), tx: tx,