Test infrastructure keeps producing fixture state-leak bugs

[chalfontchubby]

(Posted by Rik; written by Claude.)

Pattern

unit_test.py runs every registered test against one shared PredBat/HA fixture (create_predbat()), and this keeps producing the same class of bug: a test mutates shared state, restores it incompletely or unconditionally-to-a-default instead of to what was actually there, and leaks into whichever test runs next.

Recent instances, all found by review rather than by the leak actually being caught:

• fix(fetch): exclude saving-session/Axle boosted minutes from automatic rate thresholds (#5050) #5052: compare.fetch_rates()/annual._apply_rates() mutate ~24 fields on the shared fixture (dashboard_values, control_ledger, window lists among them) when driven directly in a test - broke optimise_levels when tried.
• fix(inverter): auto-create charge_rate entity for script-driven "power" inverters (#3311) #4645: four new inverter-constructor tests restored args but not the HA interface's dummy_items, leaking 12 dummy entities into every later test in the module - the same gap existed in the pre-existing test Copilot cited as the model to follow.
• fix(output): savings_today_predbat feeds the total with the same figure the sensor state shows (#3894) #5061 (PR closed, premise rejected, but the bug it fixed along the way was real): a test's cleanup reset soc_kwh_history to {} unconditionally instead of restoring whatever an earlier test had left there.
• fix(security): redact secrets at log-write time, not just at serve time (#4770) #5053: two tests left PREDBAT_APPS_FILE/args["chat"] mutated for the rest of the process.

Each was fixed individually as found. The pattern itself isn't tracked anywhere, and there's no mechanism that would catch the next one before a maintainer or reviewer spots it by hand.

Why it keeps happening

• The shared-fixture-across-all-tests design means any test that mutates instance state is a landmine for every test that runs after it, and the blast radius depends on run order.
• "Snapshot before mutating, restore what was actually there" (not = {} or a hardcoded default) is a convention, not something enforced - easy to get right for the field you're thinking about and miss another one a function touches incidentally.
• Nothing currently detects a leak automatically; it surfaces as an unrelated test failing later, which is confusing to debug and easy to misattribute (see test_state_fingerprint idea already on Rik's backlog, aimed at exactly this).

Possible directions (not scoped, for discussion)

• A generic snapshot/restore helper other test modules could reuse instead of each writing its own (a couple of ad-hoc versions of this already exist, e.g. in test_inverter.py).
• A registry-level assertion that fixture state is unchanged after each test (or each module) - would catch a leak at its source test rather than downstream.
• Isolating truly destructive tests (ones that call something like fetch_rates()/_apply_rates() that touch dozens of fields) onto a throwaway PredBat instance rather than the shared one, since restoring 24 fields by hand doesn't scale.

Filed to track the pattern; no fix attempted here.

[springfall2008]

Automated first-pass triage (a maintainer will review before any action is taken).

Classification: enhancement (maintainer-filed tracking/discussion issue — already carries the enhancement label; no priority set for a feature request).

What I verified against main (v9.0.2-28-gbc853a0e):

• The shared-fixture design is as described: one create_predbat() instance is built in main() and passed to every registered test (unit_test.py:388, unit_test.py:899). The run loop does no state checking between tests (unit_test.py:886-915) and stops at the first failure (unit_test.py:910), so a leak's visible blast radius depends on run order exactly as the issue says.
• The ad-hoc snapshot/restore convention is real and already spread wider than "a couple of versions": a copy.deepcopy(my_predbat.args)-style snapshot is hand-rolled in at least 9 test modules (e.g. test_inverter.py:400, plus test_chat_tools.py, test_cloud_modulation.py, test_multi_car_iog.py, test_octopus_slots.py, test_saving_session.py, test_validate_config.py, test_web_annual.py, test_web_mcp.py), and an env-var restore exists in test_annual_bootstrap.py:330. Each is its own local variant of the same idea.
• The fix(output): savings_today_predbat feeds the total with the same figure the sensor state shows (#3894) #5061-shaped restore-to-default is still present in the tree: _test_soc_kw_h0_fallback sets my_predbat.soc_kwh_history = {} at both entry and cleanup (test_calculate_yesterday.py:1331 and :1379), with a comment asserting "the default after reset". Currently latent — nothing else in that module populates the attribute — but it is exactly the fragile shape the issue describes: correct only as long as no earlier test ever leaves data there.
• Direction 3 (throwaway instance for destructive tests) already has precedent in the tree: test_plan_preclip.py:29 and test_random_scenarios.py:1415 each build their own create_predbat(), and unit_test.py:346 does so for the debug-case replay. So that direction is not new ground.

Duplicate check: no prior issue tracks the pattern itself. #5026 is the same class (fixture-state leak — the wall-clock residue) and was fixed at the fixture layer by pinning the clock in create_predbat(), which supports the issue's thesis that the fixture/runner is where this belongs rather than per-test fixes.

One observation for the discussion (not a proposal): the natural choke point for the registry-level assertion is the run loop at unit_test.py:886-915 — a fingerprint taken right after create_predbat() returns and compared after each func(my_predbat). A caveat if that direction is taken: not every attribute can be expected to be byte-identical between tests (timer/stamp fields legitimately move), so a fingerprint over a curated set of mutable attributes — or a warn-first mode before it gates — is the pragmatic version. That's the maintainer's call.

No test run was performed: the issue is explicitly "filed to track the pattern; no fix attempted here", and nothing it claims is settled by executing the suite — the evidence above is read directly from the tree. No further information is needed.

[chalfontchubby]

(Posted by Rik; written by Claude.)

Built a shuffle-fuzzer rather than chasing individual leaks by hand, and used it to sample how common this pattern actually is. Summary first, mechanism below.

What was built

unit_test.py gained --shuffle/--shuffle-seed N (deterministic random test ordering). tools/shuffle_fuzz.py drives it three ways:

• plain fuzzing: repeat shuffled runs, stop and log the first failure
• --bisect SEED: replay a known-failing shuffle, then narrow the tests that ran before the failure down to a minimal reproducing subsequence (binary search when a single prior test is the culprit, which is the common case in practice; falls back to full delta-debugging for multi-test interactions)
• --campaign N: repeat shuffle-then-bisect for N independent rounds and tally which (culprit, victim) pairs recur - separates common leaks from one-off orderings

Findings

11 independently-confirmed ordering-dependent failures so far, each reproducible with a 2-test command:

Culprit (runs first)	Victim (fails)	Repro
calculate_yesterday	prediction_batch	--test calculate_yesterday --test prediction_batch
load_forecast_history	model_kernel	--test load_forecast_history --test model_kernel
load_forecast_history	iboost_smart	--test load_forecast_history --test iboost_smart
debug_history_capture_alignment	web_functions	--test debug_history_capture_alignment --test web_functions
kernel_static_cache	model	--test kernel_static_cache --test model
kernel_static_cache	model_kernel	--test kernel_static_cache --test model_kernel
plot	iog_charge_skew	--test plot --test iog_charge_skew
octopus_url	plan_persistence	--test octopus_url --test plan_persistence
rate_text_scan	cloud_modulation	--test rate_text_scan --test cloud_modulation
multi_car_iog	kernel_parity	--test multi_car_iog --test kernel_parity
metric_battery_cycle_fractional	model_kernel	--test metric_battery_cycle_fractional --test model_kernel

All reproduce with python3 apps/predbat/unit_test.py <repro column>. 10/10 rounds of a --quick campaign bisected to a single culprit test, not a combination - the leak pattern really is "one earlier test taints shared fixture state," matching this issue's premise.

Two things stand out in the shape of the data, not just the count:

• load_forecast_history and kernel_static_cache each poisoned two different victims across independent rounds - candidates for whichever cleanup is broadest/most-skipped, worth looking at first.
• Victims cluster around model/model_kernel/kernel_parity (4 of 11) - these assert on tight numeric prediction output, so they're the most sensitive canary for any leaked state, not necessarily evidence the leak originates near them.

One important caveat, not yet triaged per-pair

A failing pair here means shared fixture state was left mutated - it does not by itself say whether the fix belongs in the test's cleanup or in the production code path it exercises. The calculate_yesterday -> prediction_batch pair looked initially like it might be case 2: prediction.py's run_prediction indexes self.car_charging_battery_size[car_n] over range(self.num_cars) with no bounds check, which would be a real latent crash if those two ever diverge outside of tests too - but calculate_yesterday() itself never touches car_charging_battery_size, so the divergence traced back to test-fixture state, not a live code path. Each of the 11 pairs above needs that same "test hygiene vs. real gap" judgement call before a fix is written; this comment only establishes that they reproduce, not that they're all copy-paste-fixable the same way.

Not proposing

This isn't fast enough for a per-commit CI gate (full shuffled runs plus bisection), but it's cheap to run periodically or before a release to catch new leaks that individual review misses.

[chalfontchubby]

(Posted by Rik; written by Claude.)

Retiring the per-test patches for one change in the runner

The fixes so far named the attributes each leaky test mutated and restored them by hand. That was the wrong shape: it scaled with the number of tests, and it was wrong in review more than once - the first version of #5097 missed 13 fields and a later audit found 7 more.

The cause is simpler than the symptoms. Every test ran against a single PredBat built once at startup, so any test that mutated it without restoring broke whichever unrelated test happened to follow. #5102 gives each test its own instance instead.

Measured rather than argued:

	result
Known-bad orderings before	55 / 55 fail
Known-bad orderings after	0 / 55
Full suite, shared fixture	174s, 316 passed
Full suite, per-test fixture	173s, 316 passed

The assumption that kept this at per-test patching - that rebuilding would be too slow - does not hold. create_predbat() is about 22ms, and the suite's cost is dominated by the prediction work inside the tests.

#5097 and #5098 are therefore closed in favour of #5102.

Also worth recording: all 316 tests pass when run individually, each in its own process. So every ordering failure found here was shared-fixture contamination, not a test quietly depending on an earlier one to set it up. The single exception was plan_persistence, which failed alone because of a real product bug (#5100) - found precisely because isolation was checked.

What the fuzzer found next, and a correction to it

An overnight run after #5102 produced 12 further "pairs". None were real. Seven named teslemetry and five plan_persistence, with culprits as unrelated as solax, carbon and web_annual - and not one reproduced.

Both victims were failing on the wall clock, not on ordering. plan_persistence builds an "already expired" timestamp from the fixture clock (pinned to midday) while storage.load() judges it against the real one, so it failed for any run before ~11:00 local. That is the mirror of #5100, where save_plan() computed a future expiry the same way and every saved plan was born expired after ~19:00. Fixed in #5104.

This exposed a real weakness in the tool. bisect_order() assumes the failure it is narrowing is deterministic and caused by something earlier; when it is not, the search still returns a minimal subset, but the "culprit" is just whichever test preceded the victim. #5096 has been updated to check the victim passes on its own before recording a pair - a victim that fails alone needs no culprit - and to log failures it cannot reduce rather than discarding them, since a clock-dependent or flaky test is a real bug worth keeping the evidence for.

Current state

PR	What
#5096	the fuzzer, plus the confirmation/logging fix above
#5100	save_plan() expiry computed from the wrong clock - product bug
#5102	per-test fixtures; supersedes the per-test patches
#5104	the same clock mistake in plan_persistence Test 5
#5097, #5098	closed in favour of #5102

Worth saying plainly that none of this is settled as a direction yet - it rests on the premise that order-dependent tests are worth fixing, and that has not been confirmed by a maintainer. #5100 and #5104 stand on their own regardless, since both are real bugs found along the way.