test(infra): cover scheduler/hosted-service loop orchestration + idempotency (#243) - #377
Conversation
…potency (#243) Close the remaining tests-audit gaps for four background services with behaviour tests that exercise the tick loop and orchestration, not just the pure helpers: - OpenAiBatchPoller: a mixed poll finalizes a completed batch while leaving an in-flight one untouched; the hosted loop runs one tick and cancels cleanly. - StreakFreezeAutoActivation: DB-backed idempotency — an existing freeze or a recorded guard for the missed date blocks re-activation and re-push; the hosted loop activates once and cancels cleanly. - PlayNotificationCleanup: the hosted loop runs the purge on tick and respects the 30/90-day Play/Stripe retention windows. - XpAwardLogBackfillHostedService: the backfill is idempotent across a restart and StopAsync during the startup delay unwinds without faulting. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
There was a problem hiding this comment.
Review Complete
Scope: PR #377 — test(infra): cover scheduler/hosted-service loop orchestration + idempotency (#243)
Recommendation: APPROVE
Summary
This is a test-only PR (four files under tests/Orbit.Infrastructure.Tests/Services/, no src/ changes) adding hosted-lifecycle and idempotency-behavior tests for OpenAiBatchPollerService, StreakFreezeAutoActivationService, PlayNotificationCleanupService, and XpAwardLogBackfillHostedService. Each new test was checked against its corresponding source service (src/Orbit.Infrastructure/Services/*.cs and ScheduledServiceBase.cs) to verify the assertions actually match production behavior and that the new StartAsync/await <signal>/StopAsync pattern is race-free.
Key correctness checks performed:
ScheduledServiceBase.ExecuteAsync(src/Orbit.Infrastructure/Services/Hosting/ScheduledServiceBase.cs:24-48) runs the first tick immediately (no startup delay), so waiting on a signal fired mid-tick and then callingStopAsyncis deterministic —BackgroundService.StopAsyncawaitsExecuteTaskto full completion, so there's no read/write race on sharedDbContext/state between the background loop and the post-StopAsyncassertions in any of the four new lifecycle tests.PlayNotificationCleanupServiceTests's newSignalOnStripePurgeLoggercorrectly keys offEventId == 5(LogStripeEventsPurged), which only fires whenpurgedEvents > 0(PlayNotificationCleanupService.cs:78) — the test seeds an expired Stripe row so the signal is guaranteed to fire.StreakFreezeAutoActivationService's two new idempotency tests correctly exercise the two guard checks inStageFreeze(StreakFreezeAutoActivationService.cs:130-134) and assert no freeze/push occurs.OpenAiBatchPollerServiceTests's completed-vs-in-flight test correctly matchesProcessBatchAsync's branching (completed→ finalize/delete files;in_progress→ left untouched, no delete).XpAwardLogBackfillHostedServiceTests's two new tests correctly cover restart idempotency (flag write blocks a second backfill) and cancellation during the 5s startup delay (no backfill runs,ExecuteTaskcompletes non-faulted).
No narration comments were added. No TODO/workarounds/dead code/type-safety issues found.
Findings
Critical
None.
High
None.
Medium
None.
Low / Info
None posted (per the rubric's signal gate — no concretely actionable Medium+ issues surfaced; two very minor observations were considered and dropped as non-actionable).
Subagents
| Agent | Verdict |
|---|---|
| security-reviewer | N/A — gate is "any src/ code changed"; this diff touches only tests/, no src/ files |
| contract-aligner | N/A — no DTO, Controller route, or shared-type change |
Validation
| Check | Result |
|---|---|
| Build (dotnet) | N/A — skipped per caller instruction; separate CI check (Build) covers it |
| Tests (dotnet) | N/A — skipped per caller instruction; separate CI check (Unit Tests) covers it |
What's good
- Genuinely exercises the hosted
BackgroundServicelifecycle (StartAsync/StopAsync/ExecuteTask) rather than only the pure tick-body helper methods. - Uses
TaskCompletionSourcesignals tied to a real side effect (last DB op / mock invocation) instead of wall-clockTask.Delaysleeps — avoids flakiness while staying deterministic. - New tests are tightly scoped to the specific idempotency/orchestration behavior claimed in the PR body and each one maps cleanly to a specific guard/branch in the corresponding service.
Recommendation
No changes requested — ready to merge (subject to the separate Build/Unit Tests/SonarCloud CI checks passing).



Closes the remaining
/prod-readinesstests-audit gaps for four background services with behaviour tests that drive the tick loop and orchestration (not just the pure helpers), each able to fail if the behaviour breaks. Timers are driven via the hosted lifecycle + injected abstractions — no wall-clock sleeps. Test-only; no source change was required.What's added
OpenAiBatchPollerService
PollPendingBatches_CompletedAndInFlightBatches_FinalizesCompletedAndLeavesInFlight— a completed batch is downloaded/persisted/finalized and its files deleted, while anin_progressbatch staysSubmittedand is never downloaded or cleaned up.ExecuteAsync_HostedLifecycle_RunsOnePollTickThenStopsGracefully—StartAsyncruns one poll tick (signalled off the unit-of-work save),StopAsynccancels the loop cleanly (ExecuteTaskcompleted, not faulted).StreakFreezeAutoActivationService
ActivateMissedDayFreezes_ExistingFreezeForMissedDate_DoesNotDoubleActivate— DB-backed idempotency: a freeze already present for the missed date blocks a second freeze/consume/push.ActivateMissedDayFreezes_GuardAlreadyRecordedForMissedDate_DoesNotReactivateOrRepush— a recordedSentStreakFreezeAlertguard blocks re-activation.ExecuteAsync_HostedLifecycle_RunsOneFreezePassThenStopsGracefully— the hosted loop activates one freeze then cancels cleanly.PlayNotificationCleanupService
ExecuteAsync_HostedLifecycle_RunsOnePurgePassThenStopsGracefully— the hosted loop runs the purge on tick and respects the 30-day Play / 90-day Stripe retention windows (records inside the window survive), then stops cleanly. The purge signal keys off the last DB log op to avoid racing the shared context.XpAwardLogBackfillHostedService
RunBackfillAsync_RunTwiceOnRestart_SecondRunSkipsAndDoesNotDoubleBackfill— idempotent across a restart: the second run is a no-op (flag/rows written once, "already complete" logged).StartAsync_CancelledDuringStartupDelay_StopsCleanlyWithoutRunningBackfill—StopAsyncduring the startup delay unwinds without running the backfill and without faulting.Verification
dotnet build— succeeds.dotnet test(Orbit.Infrastructure.Tests) — 1933 passed, 0 failed.Refs thomasluizon/orbit-ui-mobile#243