Repository navigation
test: pin the amp controller's apply path and probe precedence; a front-load test that can fail - #40
Merged
Merged
Conversation
…nt-load test that can fail A review of the suite found one test that passed while showing the opposite of its title, and three paths in the deployed controller that no test reached. test_front_load_drops_once_... asserted only "at least one cap". Its handle climbed 1.2 C a tick whatever was commanded, so after the planned 48 -> 41 A drop the emergency rule fired every tick: 13 caps down to 17 A. It now runs decide() against a handle that follows the cap and asserts exactly one move in two hours, to the largest whole amp that fits. derate_amp_control: - a thermal cap on the tick a probe is due is applied, and the probe does not start (only abandoning a running probe was covered); - main()'s apply path: the charger is written before the state file is saved, and a failed BLE write returns 1, leaves the state file alone and records amp_adjust_failed; - test_probe_one_per_session was held by the 7-day plan interval, never by the one-per-session rule. A second case with an interval shorter than the session leaves only that rule. Each of the four was checked by breaking the behavior it pins. The two discovery tests registered bare lambdas as aiohttp handlers, the source of the suite's 4 deprecation warnings; they are async functions now. Co-Authored-By: Claude Opus 5.5 <[email protected]>
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.
Problem
A review of the test suite (176 passing) found that a green run did not mean what it appeared to in four places:
test_front_load_drops_once_to_the_target...asserted onlylen(caps) >= 1. Its fixture's handle climbed 1.2 C per tick whatever the controller commanded, so after the planned 48 -> 41 A drop the emergency rule fired on every tick: 13 caps, down to 17 A. The test passed while showing the opposite of its title.derate_amp_control._apply_probewas never executed. Only abandoning a probe already running was covered.derate_amp_control.main()had no coverage: the BLE write, the state save after it, the event post, and theamp_adjust_failedbranch. It is the only code that writes to the charger.test_probe_one_per_sessionpassed because the 7-day plan interval held the second probe back. The one-per-session guard never ran anywhere in the suite.The suite also emitted 4 deprecation warnings, all from two bare-lambda aiohttp handlers in the discovery tests.
Code touched
Tests only.
tests/test_settle_amp_control.py: new_closed_loop()helper runsdecide()against a handle that follows the cap (one time constant toward theta + rise(I), the car drawing 0.6 A over the cap, a trajectory forecast from two minutes into each stretch). The front-load test uses it with effective ambient 33 C and a 31 C prior, and asserts exactly one move in two hours, to 42 A, with the peak under the soak margin. 42 A settles at 63.1 C; 43 A would settle at 64.2 C.tests/test_derate_amp_control.py:test_thermal_cap_outranks_starting_a_probe: a due probe and a confirmed trip on the same tick; the cap is applied, no probe starts, the slot is not consumed.test_main_applies_a_cap_then_saves_state_then_records_the_event: the fake BLE write reads the state file at the moment it is called, pinning that the cap is not on disk yet.test_main_leaves_state_alone_when_the_ble_write_fails: returns 1, state file unchanged,amp_adjust_failedrecorded.test_probe_one_per_session: a second case with a one-hour plan interval in a three-hour session, where only the one-per-session rule can stop the warm probe.tests/test_discovery_identity.py: the two handlers areasync def.contrib/andwallmonitor/. No behavior changes.Risk
min_ampsfloor), the poller's clear-warning-while-charging branch, and the amp-control backtest harnesses.Verification
uv run pytest: 179 passed, 0 warnings (was 176 passed, 4 warnings).derate_amp_controltest was run against the behavior it pins broken, and failed: the outranks-probe return removed; the one-per-session guard removed;save_statemoved before the BLE write; the failure branch returning 0.tests/are unchanged.🤖 Generated with Claude Code