From 813192e5ba5b1b005cf36e3b1a3c610a24c675b5 Mon Sep 17 00:00:00 2001 From: Mathias Date: Sat, 3 Oct 2026 21:04:15 +0200 Subject: [PATCH] test: heartbeats are stamped with the application clock, not the database's NOW() (#51), red Co-Authored-By: Claude Sonnet 5.5 --- cmd/tapir/uploads_test.go | 2 +- .../uploaded_transcripts_progress_test.go | 8 ++--- internal/progress/tracker_stall_test.go | 2 +- internal/progress/tracker_test.go | 33 +++++++++++++++---- internal/uploads/worker_test.go | 2 +- 5 files changed, 33 insertions(+), 14 deletions(-) diff --git a/cmd/tapir/uploads_test.go b/cmd/tapir/uploads_test.go index d96aeb2..cbedef2 100644 --- a/cmd/tapir/uploads_test.go +++ b/cmd/tapir/uploads_test.go @@ -44,7 +44,7 @@ func (f *fakeUploadsStore) SaveProgress(context.Context, string, string, []domai return nil } -func (f *fakeUploadsStore) Heartbeat(context.Context, string, string) error { return nil } +func (f *fakeUploadsStore) Heartbeat(context.Context, string, string, time.Time) error { return nil } func (f *fakeUploadsStore) FailInterruptedUploads(context.Context) (int, error) { f.sweeps++ diff --git a/internal/adapters/store/uploaded_transcripts_progress_test.go b/internal/adapters/store/uploaded_transcripts_progress_test.go index 3456df2..7fdf827 100644 --- a/internal/adapters/store/uploaded_transcripts_progress_test.go +++ b/internal/adapters/store/uploaded_transcripts_progress_test.go @@ -68,13 +68,13 @@ func TestUploadProgress_AHeartbeatMovesAliveAtAndNotProgressAt(t *testing.T) { ctx := context.Background() at := time.Date(2026, 10, 2, 8, 30, 0, 0, time.UTC) require.NoError(t, s.SaveProgress(ctx, userA, id, []domain.ProgressStep{{Key: "audio", Label: "a", State: domain.StepRunning}}, at)) - before := time.Now().Add(-time.Second) + beatAt := at.Add(90 * time.Second) // deliberately not the database's NOW() - require.NoError(t, s.Heartbeat(ctx, userA, id)) + require.NoError(t, s.Heartbeat(ctx, userA, id, beatAt)) got, err := s.GetUpload(ctx, userA, id) require.NoError(t, err) - assert.True(t, got.AliveAt.After(before), "the heartbeat must stamp the row with now") + assert.True(t, got.AliveAt.Equal(beatAt), "the heartbeat is stamped with the application's clock, the same one progress_at uses, not the database's NOW() (#51)") assert.True(t, got.ProgressAt.Equal(at), "alive is not progress: a heartbeat must not move progress_at") } @@ -99,7 +99,7 @@ func TestUploadProgress_ByAnotherUserChangesNothing(t *testing.T) { ctx := context.Background() errSave := s.SaveProgress(ctx, userB, id, []domain.ProgressStep{{Key: "x", Label: "X", State: domain.StepDone}}, time.Now()) - errBeat := s.Heartbeat(ctx, userB, id) + errBeat := s.Heartbeat(ctx, userB, id, time.Now()) assert.ErrorIs(t, errSave, store.ErrUploadNotFound) assert.ErrorIs(t, errBeat, store.ErrUploadNotFound) diff --git a/internal/progress/tracker_stall_test.go b/internal/progress/tracker_stall_test.go index ce59436..f31f425 100644 --- a/internal/progress/tracker_stall_test.go +++ b/internal/progress/tracker_stall_test.go @@ -28,7 +28,7 @@ func (s *stallSink) SaveProgress(ctx context.Context, _ []domain.ProgressStep, _ return ctx.Err() } -func (s *stallSink) Beat(ctx context.Context) error { +func (s *stallSink) Beat(ctx context.Context, _ time.Time) error { s.beats.Add(1) <-ctx.Done() return ctx.Err() diff --git a/internal/progress/tracker_test.go b/internal/progress/tracker_test.go index 52b00ee..8fc2aff 100644 --- a/internal/progress/tracker_test.go +++ b/internal/progress/tracker_test.go @@ -17,11 +17,12 @@ import ( // fakeSink records every saved snapshot and every heartbeat. type fakeSink struct { - mu sync.Mutex - saves [][]domain.ProgressStep - at []time.Time - beats int - err error + mu sync.Mutex + saves [][]domain.ProgressStep + at []time.Time + beats int + beatAt []time.Time + err error } func (f *fakeSink) SaveProgress(_ context.Context, steps []domain.ProgressStep, at time.Time) error { @@ -32,10 +33,11 @@ func (f *fakeSink) SaveProgress(_ context.Context, steps []domain.ProgressStep, return f.err } -func (f *fakeSink) Beat(context.Context) error { +func (f *fakeSink) Beat(_ context.Context, at time.Time) error { f.mu.Lock() defer f.mu.Unlock() f.beats++ + f.beatAt = append(f.beatAt, at) return f.err } @@ -297,7 +299,7 @@ func (r *retainingSink) SaveProgress(_ context.Context, steps []domain.ProgressS r.got = append(r.got, steps) return nil } -func (r *retainingSink) Beat(context.Context) error { return nil } +func (r *retainingSink) Beat(context.Context, time.Time) error { return nil } func TestTracker_ASnapshotHandedToTheSinkIsNotChangedByLaterTransitions(t *testing.T) { sink := &retainingSink{} @@ -400,3 +402,20 @@ func TestTracker_AnnotateAttachesANoteToAStepAndSavesItAtOnce(t *testing.T) { require.Equal(t, "20 min of the 79 the file claimed", sink.last(t)[0].Note) require.NotPanics(t, func() { tr.Annotate("no-such-step", "x") }) } + +func TestTracker_BeatIsStampedWithTheTrackersClock(t *testing.T) { + // alive_at used the database's NOW() while the page compared it with the application + // clock; a skewed database clock showed a healthy job as lost (#51). + tr, sink, clk, _ := newTracker(t) + clk.tick(42 * time.Second) + want := clk.now() + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + go tr.Heartbeat(ctx, 5*time.Millisecond, time.Hour) + + require.Eventually(t, func() bool { return sink.beatCount() >= 1 }, 2*time.Second, 5*time.Millisecond) + sink.mu.Lock() + defer sink.mu.Unlock() + require.True(t, sink.beatAt[0].Equal(want), "beat stamped %v, the tracker's clock says %v", sink.beatAt[0], want) +} diff --git a/internal/uploads/worker_test.go b/internal/uploads/worker_test.go index 8fac296..728e404 100644 --- a/internal/uploads/worker_test.go +++ b/internal/uploads/worker_test.go @@ -70,7 +70,7 @@ func (s *wStore) SaveProgress(_ context.Context, _, _ string, steps []domain.Pro return nil } -func (s *wStore) Heartbeat(context.Context, string, string) error { +func (s *wStore) Heartbeat(context.Context, string, string, time.Time) error { time.Sleep(s.beatDelay) s.mu.Lock() defer s.mu.Unlock()