perf(tests): cut the unit test suite from 123s to 78s - #4966
Merged
Conversation
Profiling `--quick` by wrapping time.sleep/asyncio.sleep showed 38s of its 123s was spent literally asleep, and most of the rest of the hot spots were one avoidable cost: parsing multi-megabyte debug dumps with PyYAML's pure-Python parser. Production changes: - read_debug_yaml() now loads through libyaml's CLoader. It pairs the C parser with the very same (unsafe) Constructor yaml.unsafe_load uses, so the Python-object tags these dumps carry still load and the result is identical - verified equal on the committed cases - about 6x faster on a 5MB dump. Falls back to the pure-Python loader when PyYAML was not built with libyaml. This speeds up real --debug_file replays too, not just tests. - ComponentBase.wait_api_started() polls every 0.1s instead of every 1s. Components come up in milliseconds, so the old cadence spent nearly the whole wait asleep after the component was already live. The loop is now bounded by a monotonic deadline rather than by counting ticks, so `timeout` stays honest in seconds however often the flag is checked. Test changes - each of these waited on a real delay that no assertion depended on: - test_givtcp_component: _make_component() records the REST retry waits instead of taking them, mirroring test_givtcp_rest.py's _client. One test asserting a write never verifies was walking the whole 5 x 2s ladder. - test_alphaess_api: MockAlphaESS was inheriting initialize()'s real 2s api_delay, and the transport-failure test served the full retry backoff. - test_ohme / test_ge_cloud: skip the retry wait and the rate-limit pacing, as the neighbouring tests in those files already do. - test_chat: _fast_confirm_poll() shortens (not removes) await_confirmation's 0.2s poll for tests whose background thread answers immediately. Tooling: - --quick's help named three tests that are not marked slow while the four that are went unmentioned; it is now derived from TEST_REGISTRY so it cannot drift again. - Each test logs a wall-clock start/end stamp, so a run that stalls can be read straight from the log by its unmatched start stamp. debug_cases 22.56s -> 12.01s random 27.94s -> 22.44s givtcp_component 10.04s -> 0.03s web_if 3.37s -> 1.57s alphaess_api 7.05s -> 0.04s chat 3.50s -> 2.52s window_cache 7.47s -> 2.09s debug_yaml_scope 2.04s -> 0.19s ge_cloud 1.67s -> 1.04s ohme 1.01s -> 0.01s Sleeps left in place are the ones that are themselves the assertion - the tests proving the event loop is not blocked, and annual_job's real subprocesses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The changes are localized, consistent with existing module patterns (loader selection and sleep patching), and don’t show any correctness or contract regressions in the modified code paths.
Pull request overview
This PR reduces overall unit-test wall-clock time by eliminating unneeded real sleeps in tests, speeding debug YAML replay parsing via libyaml when available, and improving --quick UX/logging in the test runner—while keeping behavior/coverage intact.
Changes:
- Speed up debug dump replays by switching
read_debug_yaml()to use PyYAML’s C-backed loader when present, with a safe fallback. - Make component startup waits more responsive by polling
api_startedat 0.1s with a monotonic deadline. - Remove dead wall-clock delays in several tests by patching
asyncio.sleep, and improveunit_test.pyoutput/help text (dynamic slow-test list + start/end stamps).
File summaries
| File | Description |
|---|---|
| apps/predbat/userinterface.py | Use a faster YAML loader for debug dump replays with fallback when libyaml isn’t available. |
| apps/predbat/component_base.py | Reduce wait polling interval and enforce a monotonic timeout deadline. |
| apps/predbat/unit_test.py | Derive --quick help from TEST_REGISTRY and add per-test start/end timestamps. |
| apps/predbat/tests/test_ohme.py | Patch ohme.asyncio.sleep to skip an unasserted retry delay. |
| apps/predbat/tests/test_givtcp_component.py | Record REST retry “sleeps” instead of serving real waits to cut runtime. |
| apps/predbat/tests/test_ge_cloud.py | Patch gecloud.asyncio.sleep to skip rate-limit pacing in mock-only reads. |
| apps/predbat/tests/test_chat.py | Add a context manager to temporarily shorten confirmation polling in fast-answer tests. |
| apps/predbat/tests/test_alphaess_api.py | Default mock api_delay to 0 and patch retry backoff sleeps in the transport-failure test. |
| .cspell/custom-dictionary-workspace.txt | Add libyaml to the custom dictionary. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
What
./run_all --quickwent from 122.9s to 78s (−37%), and the full suite from 155s to ~110s. Two production changes carry most of it; the rest are tests that were waiting on real delays no assertion depended on.How this was found
Profiled the suite by wrapping
time.sleep/asyncio.sleepand attributing real seconds to each call site: 38.2s of the 123s was spent literally asleep. Profiling the remaining hot spots with cProfile turned up one avoidable cost dominating the rest — parsing multi-megabyte debug dumps with PyYAML's pure-Python parser.Production changes
read_debug_yaml()now loads through libyaml'sCLoader.CLoaderpairs the C parser with the very same (unsafe)Constructorthatyaml.unsafe_loaduses, so the Python-object tags these dumps carry still load and the result is identical:It falls back to the pure-Python loader when PyYAML was not built with libyaml. This speeds up real
--debug_filereplays for users too, not just tests — every path that reads a dump was paying it.ComponentBase.wait_api_started()polls every 0.1s instead of every 1s. Components come up in milliseconds, so the old cadence spent nearly the whole wait asleep after the component was already live. The loop is now bounded by a monotonic deadline rather than by counting ticks, sotimeoutstays honest in seconds however often the flag is checked.Test changes
Each of these waited on a real delay nothing asserted on:
test_givtcp_component—_make_component()records the REST retry waits instead of taking them, mirroringtest_givtcp_rest.py's_client. One test asserting a write never verifies was walking the whole 5 × 2s ladder.test_alphaess_api—MockAlphaESSwas inheritinginitialize()'s real 2sapi_delay; only 11 of 50 tests overrode it by hand. The transport-failure test also served the full 3s retry backoff.test_ohme/test_ge_cloud— skip the retry wait and the rate-limit pacing, as neighbouring tests in those same files already do.test_chat—_fast_confirm_poll()shortens (rather than removes, so the loop is still exercised)await_confirmation's 0.2s poll for tests whose background thread answers immediately. Same idiom the file already uses forCONFIRM_TIMEOUT_SECONDS.Tooling
--quick's help namedoptimise_levels,optimise_windowsanddebug_cases— none of which are marked slow — while the four that are went unmentioned. It is now derived fromTEST_REGISTRYso it cannot drift again.Results
debug_casesrandomgivtcp_componentalphaess_apiwindow_cacheweb_ifchatdebug_yaml_scopege_cloudohmewindow_cacheis representative: 93% of it was parsing a 5.3MB dump, not exercising the cache.What was deliberately left alone
Roughly 19s of sleep remains and most of it should. The
test_chat,test_agent_toolsandtest_db_managersleeps are the assertion — they prove the event loop is not blocked, and mocking them would delete what the test checks.annual_jobspawns real subprocesses. Theweb.pyserver heartbeat andtest_component_base's already-scaledfast_sleeprun concurrently with real work rather than adding wall-clock.Testing
./run_pre_commitgreen on the rebased branch — all hooks pass, all 306 tests pass. No test behaviour was weakened: every stubbed sleep is a delay the surrounding assertions never read.🤖 Generated with Claude Code