From 441b4de98c178a42916169c96f5c646259336dc2 Mon Sep 17 00:00:00 2001 From: Rusty Eddy Date: Fri, 28 Aug 2026 12:06:05 -0700 Subject: [PATCH 1/2] M5-03: Implement deterministic simulation clock for backtests (#211) Adds clock.Simulated.AdvanceTo(t time.Time) error: the primitive a historical-timestamp-driven backtest scheduler (#213) needs, so it never has to compute t.Sub(Now()) itself at every call site across irregularly-spaced bars (weekends, holidays, gaps). Per design-notes review: no context.Context parameter. AdvanceTo is a synchronous, bounded state mutation with no blocking work, the same character Advance already has; adding ctx here would put cancellation policy at the wrong layer -- the scheduler owns the potentially long-running replay loop and should check ctx between observations before calling the clock, not make every deterministic domain primitive context-aware. Implementation shares a new private advanceBy(d) core with the existing Advance, both acquiring the lock once and delegating to it, so the two can never disagree about what "advance" means and AdvanceTo never races against a concurrent Advance/AdvanceTo call between reading Now() and applying the delta. Semantics, each covered by a dedicated test: - t canonicalized to UTC with any monotonic reading stripped (matching NewSimulated's own canonicalization) before comparison, so an equivalent instant in a different location or carrying monotonic metadata never produces different clock state. - t == Now() is a valid no-op, matching Advance(0) -- it still fires any timer already due. - t < Now() returns ErrNegativeAdvance, leaving both time and timer state unchanged. - Crossing multiple timers' deadlines in one call preserves the existing deadline/creation-order firing behavior, and Now() lands exactly on the requested target. - Two independently constructed clocks given the same start, timers, and AdvanceTo sequence produce identical observations. No new package or type -- purely additive to clock, the same way ADR-025/027/030 each added one missing primitive to num when a consuming M3/M4 issue needed it. The repo-wide TestDomainCodeDoesNotCallTimeDirectly guard (clock/arch_test.go) already mechanically covers this issue's own "no wall-clock dependency" acceptance criterion for every package outside clock/cmd/adapters; a fuller end-to-end demonstration that a deterministic run's observable results depend only on simulated time is expected once #213's own scheduler loop exists to exercise, per review. Tested: go build ./..., go vet ./..., gofmt -l ., go test ./... -race all clean. clock package coverage: 100.0%. --- clock/advance_to_test.go | 163 +++++++++++++++++++++++++++++++++++++++ clock/simulated.go | 35 ++++++++- 2 files changed, 195 insertions(+), 3 deletions(-) create mode 100644 clock/advance_to_test.go diff --git a/clock/advance_to_test.go b/clock/advance_to_test.go new file mode 100644 index 0000000..bfb8af5 --- /dev/null +++ b/clock/advance_to_test.go @@ -0,0 +1,163 @@ +package clock + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSimulatedAdvanceToMovesToExactTarget(t *testing.T) { + start := mustParse(t, "2026-01-01T00:00:00Z") + c := NewSimulated(start) + target := mustParse(t, "2026-01-01T04:00:00Z") + + require.NoError(t, c.AdvanceTo(target)) + assert.True(t, c.Now().Equal(target), "Now() must be exactly the requested target") +} + +// TestSimulatedAdvanceToEqualToNowIsANoOp mirrors +// TestSimulatedNowIsExactAfterAdvance's own Advance(0) case: a target +// equal to the current time is valid and leaves time unchanged. +func TestSimulatedAdvanceToEqualToNowIsANoOp(t *testing.T) { + start := mustParse(t, "2026-01-01T00:00:00Z") + c := NewSimulated(start) + + require.NoError(t, c.AdvanceTo(start)) + assert.True(t, c.Now().Equal(start)) +} + +// TestSimulatedAdvanceToEqualToNowStillFiresDueTimers proves the +// no-op case still fires any timer already due, matching Advance(0)'s +// own documented behavior. +func TestSimulatedAdvanceToEqualToNowStillFiresDueTimers(t *testing.T) { + start := mustParse(t, "2026-01-01T00:00:00Z") + c := NewSimulated(start) + timer := c.NewTimer(0) + + require.NoError(t, c.AdvanceTo(start)) + select { + case <-timer.C(): + default: + t.Fatal("a timer already due must fire on a no-op AdvanceTo") + } +} + +func TestSimulatedAdvanceToRejectsTargetBeforeNow(t *testing.T) { + start := mustParse(t, "2026-01-01T00:00:00Z") + c := NewSimulated(start) + require.NoError(t, c.AdvanceTo(start.Add(time.Hour))) + + err := c.AdvanceTo(start) + require.ErrorIs(t, err, ErrNegativeAdvance) + assert.True(t, c.Now().Equal(start.Add(time.Hour)), "a rejected AdvanceTo must not change the clock") +} + +// TestSimulatedAdvanceToRejectsTargetBeforeNowLeavesTimersUnchanged +// proves a rejected AdvanceTo mutates neither time nor timer state: a +// timer that would have fired had the (rejected) advance actually +// happened must still be pending afterward. +func TestSimulatedAdvanceToRejectsTargetBeforeNowLeavesTimersUnchanged(t *testing.T) { + start := mustParse(t, "2026-01-01T00:00:00Z") + c := NewSimulated(start) + timer := c.NewTimer(30 * time.Minute) + + err := c.AdvanceTo(start.Add(-time.Hour)) + require.ErrorIs(t, err, ErrNegativeAdvance) + + select { + case <-timer.C(): + t.Fatal("a rejected AdvanceTo must not fire a timer") + default: + } +} + +// TestSimulatedAdvanceToCanonicalizesTarget proves a target expressed +// in a different location, or carrying a monotonic reading, produces +// identical clock state to the equivalent UTC instant -- matching +// NewSimulated's own canonicalization (review feedback on #211). +func TestSimulatedAdvanceToCanonicalizesTarget(t *testing.T) { + start := mustParse(t, "2026-01-01T00:00:00Z") + + loc, err := time.LoadLocation("America/Los_Angeles") + require.NoError(t, err) + // 2026-01-01T04:00:00Z is 2025-12-31T20:00:00-08:00 in Los Angeles. + targetInLA := time.Date(2025, 12, 31, 20, 0, 0, 0, loc) + + c := NewSimulated(start) + require.NoError(t, c.AdvanceTo(targetInLA)) + + assert.Equal(t, time.UTC, c.Now().Location()) + assert.True(t, c.Now().Equal(mustParse(t, "2026-01-01T04:00:00Z"))) + + // A monotonic-carrying target (time.Now()) canonicalizes the same + // way NewSimulated already does -- the resulting Now() carries no + // monotonic reading. + c2 := NewSimulated(start) + require.NoError(t, c2.AdvanceTo(time.Now().Add(time.Hour))) + assert.NotContains(t, c2.Now().String(), " m=") +} + +// TestSimulatedAdvanceToFiresMultipleDeadlinesInOneCall mirrors +// TestSimulatedMultipleDeadlinesCrossedInOneAdvance for AdvanceTo: +// crossing several timers' deadlines in one call preserves the +// existing deadline/creation-order firing behavior. +func TestSimulatedAdvanceToFiresMultipleDeadlinesInOneCall(t *testing.T) { + start := mustParse(t, "2026-01-01T00:00:00Z") + c := NewSimulated(start) + t1 := c.NewTimer(1 * time.Second) + t2 := c.NewTimer(2 * time.Second) + t3 := c.NewTimer(10 * time.Second) + + require.NoError(t, c.AdvanceTo(start.Add(5*time.Second))) + + for _, timer := range []Timer{t1, t2} { + select { + case <-timer.C(): + default: + t.Fatal("a timer whose deadline was crossed must be ready") + } + } + select { + case <-t3.C(): + t.Fatal("a timer whose deadline was not crossed must not be ready") + default: + } + + assert.True(t, c.Now().Equal(start.Add(5*time.Second)), "Now() must equal the requested target exactly") +} + +// TestSimulatedAdvanceToDeterministicAcrossIndependentInstances proves +// two independently constructed clocks given the same start, timers, +// and AdvanceTo sequence produce identical observations (review +// feedback on #211). +func TestSimulatedAdvanceToDeterministicAcrossIndependentInstances(t *testing.T) { + start := mustParse(t, "2026-01-01T00:00:00Z") + targets := []time.Time{ + start.Add(1 * time.Hour), + start.Add(90 * time.Minute), + start.Add(4 * time.Hour), + } + + run := func() (time.Time, []bool) { + c := NewSimulated(start) + timer := c.NewTimer(2 * time.Hour) + var fired []bool + for _, target := range targets { + require.NoError(t, c.AdvanceTo(target)) + select { + case <-timer.C(): + fired = append(fired, true) + default: + fired = append(fired, false) + } + } + return c.Now(), fired + } + + firstNow, firstFired := run() + secondNow, secondFired := run() + assert.True(t, firstNow.Equal(secondNow)) + assert.Equal(t, firstFired, secondFired) +} diff --git a/clock/simulated.go b/clock/simulated.go index 95e1ead..eddc967 100644 --- a/clock/simulated.go +++ b/clock/simulated.go @@ -82,13 +82,42 @@ func (s *Simulated) NewTimer(d time.Duration) Timer { // whose deadline is now due. Advance never waits on wall-clock time and // returns ErrNegativeAdvance without changing the clock if d is negative. func (s *Simulated) Advance(d time.Duration) error { - if d < 0 { - return ErrNegativeAdvance - } + s.mu.Lock() + defer s.mu.Unlock() + return s.advanceBy(d) +} +// AdvanceTo moves the clock's current time forward to exactly t, firing +// every timer due at or before t — the primitive a historical-timestamp- +// driven backtest scheduler needs (issue #211, M5-03), so it never has to +// compute t.Sub(Now()) itself at every call site. t is canonicalized to +// UTC with any monotonic reading stripped (matching NewSimulated's own +// canonicalization) before comparison, so equivalent instants expressed +// in different locations or carrying monotonic metadata never produce +// different clock state. AdvanceTo rejects a t before the clock's +// current time with ErrNegativeAdvance, leaving both time and timer +// state unchanged, the same way Advance itself rejects a negative +// duration; t equal to the current time is a valid no-op (matching +// Advance(0), which still fires any timer already due). AdvanceTo takes +// no context.Context: like Advance, it is a synchronous, bounded state +// mutation with no blocking work, and cancellation policy for a +// potentially long-running replay loop belongs to the caller driving +// that loop (the scheduler, #213), not to this primitive. +func (s *Simulated) AdvanceTo(t time.Time) error { + target := t.UTC().Round(0) s.mu.Lock() defer s.mu.Unlock() + return s.advanceBy(target.Sub(s.now)) +} +// advanceBy must be called with s.mu held. It applies duration d, +// rejecting a negative one without changing state — the shared core +// both Advance and AdvanceTo delegate to, so the two can never +// disagree about what "advance" means. +func (s *Simulated) advanceBy(d time.Duration) error { + if d < 0 { + return ErrNegativeAdvance + } s.now = s.now.Add(d) s.fireDue() return nil From cec5ca6e4cf21eff3f220d8be038a62948f9dd78 Mon Sep 17 00:00:00 2001 From: Rusty Eddy Date: Fri, 28 Aug 2026 12:21:18 -0700 Subject: [PATCH 2/2] fix: address PR #229 review feedback on ineffective timer test Rusty + Copilot both caught the same real issue: TestSimulatedAdvanceToEqualToNowStillFiresDueTimers didn't prove what it claimed. NewTimer(0) fires immediately inside NewTimer itself (never added to the active timer set at all), so the assertion passed regardless of whether AdvanceTo(start) called fireDue() -- the test was ineffective, not a real regression guard. Removed the test rather than reaching into private timer state to manufacture an otherwise-unreachable condition (through the public API there cannot be a pending timer whose deadline is already <= Now(): non-positive timers fire immediately, and positive ones become due only as the clock advances, which itself already calls fireDue). Softened AdvanceTo's own doc comment to state only that t == Now() is a valid no-op, without asserting a timer-firing detail that was never actually externally observable or meaningfully testable at this target-equals-now boundary. Tested: go build ./..., go vet ./..., gofmt -l ., go test ./... -race all clean. clock package coverage unchanged at 100.0%. --- clock/advance_to_test.go | 16 ---------------- clock/simulated.go | 3 +-- 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/clock/advance_to_test.go b/clock/advance_to_test.go index bfb8af5..effffde 100644 --- a/clock/advance_to_test.go +++ b/clock/advance_to_test.go @@ -28,22 +28,6 @@ func TestSimulatedAdvanceToEqualToNowIsANoOp(t *testing.T) { assert.True(t, c.Now().Equal(start)) } -// TestSimulatedAdvanceToEqualToNowStillFiresDueTimers proves the -// no-op case still fires any timer already due, matching Advance(0)'s -// own documented behavior. -func TestSimulatedAdvanceToEqualToNowStillFiresDueTimers(t *testing.T) { - start := mustParse(t, "2026-01-01T00:00:00Z") - c := NewSimulated(start) - timer := c.NewTimer(0) - - require.NoError(t, c.AdvanceTo(start)) - select { - case <-timer.C(): - default: - t.Fatal("a timer already due must fire on a no-op AdvanceTo") - } -} - func TestSimulatedAdvanceToRejectsTargetBeforeNow(t *testing.T) { start := mustParse(t, "2026-01-01T00:00:00Z") c := NewSimulated(start) diff --git a/clock/simulated.go b/clock/simulated.go index eddc967..7500ff9 100644 --- a/clock/simulated.go +++ b/clock/simulated.go @@ -97,8 +97,7 @@ func (s *Simulated) Advance(d time.Duration) error { // different clock state. AdvanceTo rejects a t before the clock's // current time with ErrNegativeAdvance, leaving both time and timer // state unchanged, the same way Advance itself rejects a negative -// duration; t equal to the current time is a valid no-op (matching -// Advance(0), which still fires any timer already due). AdvanceTo takes +// duration; t equal to the current time is a valid no-op. AdvanceTo takes // no context.Context: like Advance, it is a synchronous, bounded state // mutation with no blocking work, and cancellation policy for a // potentially long-running replay loop belongs to the caller driving