Skip to content

Commit

Permalink
Fix for intermittency in unique job tests that use ByPeriod
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
brandur committed Jun 14, 2024
1 parent 5592599 commit 7f3f235
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 1 deletion.
10 changes: 10 additions & 0 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
}

Expand Down
8 changes: 7 additions & 1 deletion internal/dbunique/db_unique_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 7f3f235

Please sign in to comment.