Timer harness - #217
Merged
Merged
Conversation
Abstract the Sub.Every scheduler behind a Clock interface so timer-driven apps become snapshot-testable without real-time delays. - Clock (instant/zone + schedulePeriodic) with production SystemClock; RuntimeCtx.clock defaults to SystemClock. - Sub.Every schedules through the ctx clock and is tagged Sub.TimerSub. - ManualClock (testkit) advances virtual time on explicit calls; TestRuntimeCtx uses it and TuiTestDriver exposes advanceTime. - DigitalClock reads time via ctx.clock for deterministic display. - Snapshot/time-advance tests for DigitalClock and SineWaveApp, plus ManualClock unit tests and golden frames. - Document virtual-time testing in testing.md / RENDER_PIPELINE.md.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Deterministic timer harness for Sub.Every (issue #140)
What changed
Clockinterface (termflow-app,Clock.scala): abstracts wall-clockreads (
instant(),zone) and the periodic scheduler(
schedulePeriodic(periodMillis, task) -> Cancelable) behindSub.Every.SystemClockis the production impl (real time + per-schedule executor).RuntimeCtx.clock: new member with default= SystemClock(non-breakingfor existing implementers).
TestRuntimeCtxoverrides it with aManualClock.Sub.Everyrewired to schedule via the sink's clock (resolved fromRuntimeCtx.clock, elseSystemClockfor bare sinks) instead of building itsown
ScheduledExecutorService. Also tagged with a newSub.TimerSubmarker(mirrors the existing
InputSubmarker) so the testkit can start exactly theclock-driven subs.
ManualClock(termflow-testkit): virtual clock, fires scheduled taskssynchronously on
advance(...). Default start = epoch 0, zone = UTC.TuiTestDriver.advanceTime(duration | millis): starts registeredTimerSubs against the manual clock (no threads), advances it, then drains &applies the resulting ticks through
app.update.DigitalClocknow reads time viactx.clock.instant()/zoneinstead ofLocalTime.now()— required to make its display deterministic.DigitalClockSnapshotSpec,SineWaveAppSnapshotSpec(golden +structural, including time-advance),
ManualClockSpec(direct clock unitcoverage). Goldens recorded under
termflow-sampletest resources.Sub.Everywith virtual time" section indocs/guide/testing.md; updated notes inRENDER_PIPELINE.md.Key trade-offs / decisions
ManualClockfires a task once per full elapsed period(first fire one period after scheduling;
advance(N*p)-> exactly N ticks).Deliberately no immediate fire at scheduling time, so the advance→tick mapping
is exact and easy to reason about in tests.
SystemClockkeepsinitialDelay=0(immediate first tick) to preserve existing runtime behaviour exactly; the
ManualClock's "no immediate fire" differs only on the very first tick. Apps
refresh display each tick, so this is observationally irrelevant; documented.
clockhas a default on theRuntimeCtxtrait so the change issource-compatible for the published SPI (downstream
RuntimeCtximpls).TimerSubmarker vs. introspection: a marker is the idiomatic match forthe existing
InputSubpattern and letsadvanceTimestart timers withoutalso starting thread-spawning input/resize subs.
order (asserted in
ManualClockSpec).Rejected alternatives
Sub.Everyactually needs, so a customClockwas cleaner than bolting ascheduler beside it.
DigitalClock's displayed time deterministic (it readLocalTime.now()), andthe brief explicitly asked to abstract the scheduler.
SystemClockto defer first tick by one period: avoided — keeps areal behaviour change out of the production hot path.
Risk
SystemClock.schedulePeriodic. Samethread-per-schedule model and
initialDelay=0as before;SubSpecreal-timertests still pass. Lowest-risk spot is cancellation, which delegates to the same
executor-shutdown logic as the old inline code.
advanceTimeonly firesSub.Every;Sub.TerminalResizeis intentionallyleft dormant in tests (it would spawn a real executor), matching prior testkit
behaviour.
Known limitations / follow-ups
Code review (codex) surfaced two items deliberately scoped out of this PR — the
issue's acceptance criteria (
ManualClockused by the test driver;DigitalClocktime-advance snapshot test) are met, and these are separable design work:
RuntimeCtx.clockdefaults toSystemClock, andDevtools.innerCtxdoesn't override it. A devtools-wrappedSub.Everyappunder
TuiTestDrivertherefore still starts on the real scheduler. Bounded fix(delegate
clockin the wrapper), but coupled to TuiTestDriver.advanceTime fires all due timer ticks before applying updates #215.advanceTime(N*p)fires all N due ticks upfront before any
updateruns, so a tick handler that cancels/replaces itstimer or reads
ctx.clock.instant()sees behaviour the real runtime wouldn't.Faithful modelling means draining updates between due firings, which changes the
ManualClock.advancecontract — a deliberate design pass, tracked separately.