Skip to content

test(scheduler): add unit tests for Healthy, LastTickAt, ComputeNextRun, panic recovery - #111

Closed
HongmingWang-Rabbit wants to merge 1 commit into
mainfrom
test/scheduler-unit-tests
Closed

test(scheduler): add unit tests for Healthy, LastTickAt, ComputeNextRun, panic recovery#111
HongmingWang-Rabbit wants to merge 1 commit into
mainfrom
test/scheduler-unit-tests

Conversation

@HongmingWang-Rabbit

Copy link
Copy Markdown
Contributor

Summary

Adds platform/internal/scheduler/scheduler_test.go — the only previously untested security-critical package from PR #90.

  • 8 new tests covering every testable code path in the scheduler
  • 4 minimal production changes to scheduler.go required to make the code testable (all additive, no behaviour change for existing callers)
  • 1 bug fix: ComputeNextRun now returns an error on invalid timezone instead of silently falling back to UTC

Tests added

Test What it covers
TestLastTickAt_zero LastTickAt() returns zero time.Time before the first tick
TestHealthy_beforeStart Healthy() is false on a fresh scheduler (zero lastTickAt)
TestHealthy_freshTick Healthy() is true when lastTickAt == now
TestHealthy_stale Healthy() is false when lastTickAt is 3×pollInterval ago (beyond 2× threshold)
TestComputeNextRun_valid "0 * * * *" / UTC returns a future top-of-hour time, no error
TestComputeNextRun_invalid Unparseable cron expression returns non-nil error
TestComputeNextRun_invalidTimezone Unrecognised IANA timezone returns non-nil error
TestPanicRecovery panicProxy panics inside ProxyA2ARequest; scheduler goroutine recovers and Healthy() remains true — an unrecovered panic would crash the test binary, so the test completing is proof

Production changes to scheduler.go

Change Rationale
Added mu, lastTickAt, tickInterval fields Required for LastTickAt(), Healthy(), and test-speed control
Added LastTickAt() / Healthy() methods The observable surface the tests exercise
tick() records lastTickAt after wg.Wait() Single mutex write per tick; zero hot-path cost
fireSchedule() deferred recover() Without this, a panicking proxy crashes the goroutine pool; the test binary itself would crash without it
ComputeNextRun returns error on bad timezone Previously silently fell back to UTC; handler already validates tz before calling, so no callers break

Test output

=== RUN   TestLastTickAt_zero          --- PASS (0.00s)
=== RUN   TestHealthy_beforeStart      --- PASS (0.00s)
=== RUN   TestHealthy_freshTick        --- PASS (0.00s)
=== RUN   TestHealthy_stale            --- PASS (0.00s)
=== RUN   TestComputeNextRun_valid     --- PASS (0.00s)
=== RUN   TestComputeNextRun_invalid   --- PASS (0.00s)
=== RUN   TestComputeNextRun_invalidTimezone --- PASS (0.00s)
=== RUN   TestPanicRecovery            --- PASS (0.00s)
ok   github.com/Molecule-AI/molecule-monorepo/platform/internal/scheduler   0.003s

Full suite: all packages pass, zero failures.

Test plan

  • go test ./internal/scheduler/... -v — 8/8 pass
  • go test ./... — full suite passes, no regressions

🤖 Generated with Claude Code

…un, panic recovery

Added scheduler_test.go with 8 test cases covering all previously untested
security-critical code paths from PR #90:

  TestLastTickAt_zero            — zero time before first tick
  TestHealthy_beforeStart        — false on fresh scheduler (zero lastTickAt)
  TestHealthy_freshTick          — true when lastTickAt == now
  TestHealthy_stale              — false when lastTickAt is 3×pollInterval ago
  TestComputeNextRun_valid       — "0 * * * *" / UTC returns top-of-hour future time
  TestComputeNextRun_invalid     — unparseable expression returns non-nil error
  TestComputeNextRun_invalidTimezone — unrecognised IANA zone returns non-nil error
  TestPanicRecovery              — panicProxy crashes ProxyA2ARequest; scheduler
                                   goroutine recovers and remains Healthy

To support these tests, scheduler.go gained four changes (minimal surface):

1. Added mu sync.RWMutex, lastTickAt time.Time, and tickInterval time.Duration
   fields to Scheduler. tickInterval defaults to pollInterval so production
   behaviour is unchanged; tests can override it directly.

2. Added LastTickAt() and Healthy() methods with read-lock protection.

3. tick() now records lastTickAt after wg.Wait() — a single atomic write under
   the mutex, no hot-path cost.

4. fireSchedule() got a deferred recover() so a panicking A2A proxy cannot
   crash the goroutine pool. Without this, TestPanicRecovery itself crashes
   the test binary — the test passing proves recovery is in place.

Bug fix: ComputeNextRun previously silently fell back to UTC on an invalid
timezone; it now returns a non-nil error. The schedules handler already
validates the timezone before calling ComputeNextRun so this is a no-op for
callers, but it makes the contract explicit and testable.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@HongmingWang-Rabbit

Copy link
Copy Markdown
Contributor Author

Superseded by #119 (cherry-picked onto a clean base from main — the original branch was pre-#95/#99/#106 and showed -965 spurious deletions). The commit 9a66a4c is included verbatim in #119.

HongmingWang-Rabbit added a commit that referenced this pull request Apr 15, 2026
fix(security+scheduler): IPv6 SSRF gap + scheduler unit tests [supersedes #111, #112]
@HongmingWang-Rabbit
HongmingWang-Rabbit deleted the test/scheduler-unit-tests branch April 16, 2026 12:33
molecule-ai Bot pushed a commit that referenced this pull request Apr 21, 2026
fix(security+scheduler): IPv6 SSRF gap + scheduler unit tests [supersedes #111, #112]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant