Skip to content

fix(tests): pin midnight_utc in the multi-car IOG tests so they don't fail for an hour a day - #4998

Merged
springfall2008 merged 1 commit into
mainfrom
fix/multi-car-iog-test-clock-pin
Sep 8, 2026
Merged

springfall2008 merged 1 commit into
mainfrom
fix/multi-car-iog-test-clock-pin

Conversation

@springfall2008

Copy link
Copy Markdown
Owner

Two multi_car_iog tests 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:

ERROR: Expected car 0 to have charging slots from IOG, got none
ERROR: Expected car 1 to have charging slots from IOG, got none
Test: multi_car_iog_load_slots_regression FAILED
ERROR: car 0 should have IOG slots, got none
ERROR: car_charging_limit_model should be set with consider_full off and IOG slots present, got None
Test: multi_car_iog_model_limit_fetch_4967 FAILED

Cause

run_multi_car_iog_load_slots_test and run_iog_model_limit_fetch_test pin now_utc to noon on the current UTC date, but leave midnight_utc on the real clock. update_time() derives midnight_utc from datetime.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_utc lands on the following UTC date, after the pinned now_utc. On #4995 that was midnight_utc = 7 Sept 23:00 UTC against a pinned now_utc of 7 Sept 12:00 UTC — eleven hours earlier.

Every IOG dispatch built as now+1h..now+3h therefore decodes to negative minutes relative to midnight_utc, and decode_octopus_slot() clamps them away:

start_minutes = max(start_minutes, 0)
end_minutes = max(min(end_minutes, self.forecast_minutes + self.minutes_now), start_minutes)
if start_minutes == end_minutes:
    return 0, 0, 0, source, location

kwh > 0 never holds, no slots load, and the assertions fail. Outside that hour the London date matches the UTC date, midnight_utc is 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

  • Shared pin_test_clock() / restore_test_clock() helpers. The pin sets midnight_utc = now_utc.replace(hour=0) alongside now_utc/minutes_now, so slot offsets are a fixed 780/840/900 minutes whatever the wall clock says.
  • run_multi_car_iog_adhoc_dispatch_test uses 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 the car_charging_now fallback slot, which is not what it means to assert.
  • The clock is restored on the way out instead of 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.

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:

real clock simulated 23:11 UTC
before 8/8 pass 2 fail (identical error text to the CI log)
after 8/8 pass 8/8 pass

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

… 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>
Copilot AI lite review requested due to automatic review settings September 8, 2026 08:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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 pin now_utc, midnight_utc, and minutes_now consistently for these tests.
  • Updates run_multi_car_iog_load_slots_test and run_multi_car_iog_adhoc_dispatch_test to pin and then restore the clock state.
  • Extends the snapshot/restore harness in run_iog_model_limit_fetch_test to include midnight_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
springfall2008 merged commit 3c391cf into main Sep 8, 2026
3 checks passed
@springfall2008
springfall2008 deleted the fix/multi-car-iog-test-clock-pin branch September 8, 2026 08:17
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>
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.

2 participants