TestUpsertUserScheduleRecurring and TestUpsertOrganizationScheduleRecurring in internal/store/subjects/scheduled_test.go fail most of the time on a pristine main worktree, but not always — which is what makes them worth fixing rather than just noticing.
Reproduction
From a clean checkout of main (eea493a7):
go test ./internal/store/subjects/ -run 'TestUpsertUserScheduleRecurring' -count=5
Five failures out of five here. Run together with TestUpsertOrganizationScheduleRecurring in the same invocation, they passed. Same commit, same machine, minutes apart.
The failing assertion is scheduled_test.go:1801:
require.True(t, us.ScheduledAt.After(time.Now()))
This is distinct from #297, which is Ryuk reaping the shared container — these fail with TESTCONTAINERS_RYUK_DISABLED=true and a healthy container.
Likely cause
The fixture puts the next occurrence exactly on the current instant:
startAt := time.Now().Add(-14 * 24 * time.Hour).UTC().Truncate(time.Microsecond)
interval := "7 days"
14 days back with a 7-day interval means occurrences fall at startAt, now-7d, and now — so the first one not in the past lands on now itself. Whether ScheduledAt.After(time.Now()) then holds depends on whether the occurrence computation treats the boundary as strictly greater or greater-or-equal, and on how many microseconds elapse between the insert and the assertion. Either way the test is deciding a strict inequality on a value sitting exactly on the boundary.
That would explain the intermittency without any concurrency involved.
Suggested fix
Choose a startAt that is not an exact multiple of the interval before now — for example -15 * 24 * time.Hour, which puts the next occurrence a clear six days out. That keeps what the test is actually checking (a recurring schedule rolls forward to a future occurrence) while removing the boundary case.
If the boundary behaviour is itself worth pinning, it deserves its own test asserting the intended semantics explicitly, rather than being load-bearing by accident inside a test about upserting.
Found while verifying that an unrelated branch had not caused these failures. It had not — they reproduce on main alone.
TestUpsertUserScheduleRecurringandTestUpsertOrganizationScheduleRecurringininternal/store/subjects/scheduled_test.gofail most of the time on a pristinemainworktree, but not always — which is what makes them worth fixing rather than just noticing.Reproduction
From a clean checkout of
main(eea493a7):Five failures out of five here. Run together with
TestUpsertOrganizationScheduleRecurringin the same invocation, they passed. Same commit, same machine, minutes apart.The failing assertion is
scheduled_test.go:1801:This is distinct from #297, which is Ryuk reaping the shared container — these fail with
TESTCONTAINERS_RYUK_DISABLED=trueand a healthy container.Likely cause
The fixture puts the next occurrence exactly on the current instant:
14 days back with a 7-day interval means occurrences fall at
startAt,now-7d, andnow— so the first one not in the past lands onnowitself. WhetherScheduledAt.After(time.Now())then holds depends on whether the occurrence computation treats the boundary as strictly greater or greater-or-equal, and on how many microseconds elapse between the insert and the assertion. Either way the test is deciding a strict inequality on a value sitting exactly on the boundary.That would explain the intermittency without any concurrency involved.
Suggested fix
Choose a
startAtthat is not an exact multiple of the interval before now — for example-15 * 24 * time.Hour, which puts the next occurrence a clear six days out. That keeps what the test is actually checking (a recurring schedule rolls forward to a future occurrence) while removing the boundary case.If the boundary behaviour is itself worth pinning, it deserves its own test asserting the intended semantics explicitly, rather than being load-bearing by accident inside a test about upserting.
Found while verifying that an unrelated branch had not caused these failures. It had not — they reproduce on
mainalone.