Repository navigation
fix(tests): pin midnight_utc in the multi-car IOG tests so they don't fail for an hour a day - #4998
Merged
Merged
Conversation
… fail for an hour a day multi_car_iog_load_slots_regression and multi_car_iog_model_limit_fetch_4967 failed on PR #4995's docs-only branch while passing on main. Nothing in that PR touched code: the two tests pin now_utc to noon on the current UTC date but leave midnight_utc on the real clock, and update_time() derives midnight_utc from datetime.now(Europe/London). Between 23:00 and 00:00 UTC under BST the London date is already the next day, so midnight_utc lands on the following UTC date - after the pinned now_utc. Every IOG slot built as now+1h..now+3h then decodes to negative minutes relative to it, decode_octopus_slot() clamps start/end to 0, start == end returns kwh 0 and the slot is dropped, so the tests see no charging slots. #4995's test step ran at 23:11:37 UTC. Pin midnight_utc alongside now_utc via a shared pin_test_clock() helper, so slot offsets are a fixed 780/840/900 minutes whatever the wall clock says. run_multi_car_iog_adhoc_dispatch_test had the same latent hazard - in that window its IOG slot was silently clipped away and the test only passed on the car_charging_now fallback slot - so it uses the helper too. The clock is now restored on the way out rather than left pinned: restore_test_clock() in the two tests without a snapshot harness, and midnight_utc added to snapshot_attrs in run_iog_model_limit_fetch_test. Previously now_utc leaked out pinned while midnight_utc stayed real. Verified by running the module under an emulation of the CI instant (00:11 Europe/London on the day after the current UTC date): 2 failures before this change with the same error text as the CI log, 8/8 passing after, and 8/8 passing on the real clock both ways. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Clock restoration is not exception-safe in the updated tests, so an unexpected error can still leak pinned time into later tests via the shared PredBat fixture.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR stabilizes the multi-car Intelligent Octopus Go (IOG) unit tests by pinning midnight_utc alongside now_utc, avoiding a BST-only 23:00–00:00 UTC window where dispatch slots can be decoded as negative minutes and clamped away.
Changes:
- Introduces
pin_test_clock()/restore_test_clock()helpers to pinnow_utc,midnight_utc, andminutes_nowconsistently for these tests. - Updates
run_multi_car_iog_load_slots_testandrun_multi_car_iog_adhoc_dispatch_testto pin and then restore the clock state. - Extends the snapshot/restore harness in
run_iog_model_limit_fetch_testto includemidnight_utc, so the pinned value is restored after the test.
File summaries
| File | Description |
|---|---|
| apps/predbat/tests/test_multi_car_iog.py | Adds clock pin/restore helpers and uses them to prevent time-dependent IOG slot decoding failures during BST. |
Review details
Suppressed comments (2)
apps/predbat/tests/test_multi_car_iog.py:488
- Similar to the load-slots test: restore_test_clock() is not guaranteed to run if an exception is raised after pin_test_clock(), which can leak the pinned clock into later tests because unit_test.py shares a single PredBat instance across the suite.
saved_clock = pin_test_clock(my_predbat)
my_predbat.args["car_charging_loss"] = 0.0
apps/predbat/tests/test_multi_car_iog.py:586
- pin_test_clock() mutates now_utc/midnight_utc/minutes_now but this test only restores state at the end; if fetch_sensor_data_cars() (or later assertions) throws unexpectedly, the pinned clock can still leak into subsequent tests via the shared PredBat fixture. Consider putting the restore of saved_args/saved attrs into a finally block so it always executes.
pin_test_clock(my_predbat)
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+225
to
229
| # Pin the clock so slot times (now+1h..now+3h) sit at a fixed offset from midnight | ||
| # regardless of when the test runs | ||
| saved_clock = pin_test_clock(my_predbat) | ||
|
|
||
| # apps.yaml config args needed by fetch_sensor_data_cars |
springfall2008
added a commit
that referenced
this pull request
Sep 9, 2026
…t the host's (#5013) test_energydataservice built its rate fixture from datetime.now() - the machine's clock - while reading the timezone offset two lines later from my_predbat.midnight_utc, which is local midnight in Predbat's configured timezone. Between 23:00 and 00:00 UTC under BST those land on different dates, so the published rates are a day out from the day the code is modelling and rates[1440] does not exist: KeyError: 1440. This is not intermittent. The nightly journal flush runs at local midnight, which during BST is 23:0x UTC, so it lands in the failing hour every night - PR #5011 is a docs-only change that failed CI for exactly this reason, at 23:12 UTC. Same trap and same fix as PR #4998 for the multi-car IOG tests, and the trap already has a bullet in the debug journal ("Build test dates from Predbat's clock, not the machine's"). Verified by running the suite with TZ=Pacific/Pitcairn, which puts the host a calendar day behind Europe/London: energydataservice reproduces the KeyError before the change and passes after, as does the full quick suite (82.98s, 4 slow tests skipped). It also still passes on an unshifted clock. The suite aborts on the first uncaught exception, so this was masking anything later in the run; with it fixed, nothing else in the quick suite fails in that window. Nineteen other test files call datetime.now(), none of them currently affected, but the pattern is latent - a scheduled CI run in a shifted timezone would catch the next one before a maintainer does. Co-authored-by: CI <ci@example.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Two
multi_car_iogtests fail for roughly one hour a day. They took down CI on #4995 — a docs-only PR (tools/debug-journal.md+ one cspell word) whose test step happened to run at 23:11:37 UTC:Cause
run_multi_car_iog_load_slots_testandrun_iog_model_limit_fetch_testpinnow_utcto noon on the current UTC date, but leavemidnight_utcon the real clock.update_time()derivesmidnight_utcfromdatetime.now(Europe/London)— London-local midnight, independent of the runner's TZ.Between 23:00 and 00:00 UTC under BST the London date is already the next day, so
midnight_utclands on the following UTC date, after the pinnednow_utc. On #4995 that wasmidnight_utc= 7 Sept 23:00 UTC against a pinnednow_utcof 7 Sept 12:00 UTC — eleven hours earlier.Every IOG dispatch built as
now+1h..now+3htherefore decodes to negative minutes relative tomidnight_utc, anddecode_octopus_slot()clamps them away:kwh > 0never holds, no slots load, and the assertions fail. Outside that hour the London date matches the UTC date,midnight_utcis at or before the pinned noon, and the slots land at +780/+840 minutes. Under GMT there is no offset, so the window is BST-only.Fix
pin_test_clock()/restore_test_clock()helpers. The pin setsmidnight_utc = now_utc.replace(hour=0)alongsidenow_utc/minutes_now, so slot offsets are a fixed 780/840/900 minutes whatever the wall clock says.run_multi_car_iog_adhoc_dispatch_testuses the helper too — it had the same latent hazard. In the failing window its IOG slot was silently clipped away and the test passed only on thecar_charging_nowfallback slot, which is not what it means to assert.restore_test_clock()in the two tests without a snapshot harness, andmidnight_utcadded tosnapshot_attrsinrun_iog_model_limit_fetch_test. Previouslynow_utcleaked out pinned whilemidnight_utcstayed real.No production code is touched.
Verification
Ran the module under the real clock and under a faithful emulation of the CI instant (00:11 Europe/London on the day after the current UTC date = 23:11 UTC), before and after the change:
Full quick suite:
All tests passed (4 slow tests skipped, total time: 83.03s). Pre-commit hooks on the changed file all pass.#4995 needs only a CI re-run once this lands.
🤖 Generated with Claude Code