test(swing_speed): cover the capture loop's swing detection state machine - #3
Open
noahjobse wants to merge 1 commit into
Open
test(swing_speed): cover the capture loop's swing detection state machine#3noahjobse wants to merge 1 commit into
noahjobse wants to merge 1 commit into
Conversation
…hine `SwingSpeedMonitor._capture_loop` is the decision core of swing speed training: it decides when a swing starts, when silence means it ended, whether motion was too brief to count, and how long to suppress readings so a follow-through does not register as a second rep. None of it was covered. swing_speed.py sat at 55%, with lines 165-240 entirely untested, so any of those decisions could break in a refactor with the suite still green. Adds nine tests over the loop's distinct behaviours. They replace the module's `time` reference with a controlled clock, so the shipped defaults (end_quiet_ms=1000, cooldown_ms=750) are the values under test, the suite never waits on real elapsed time, and it cannot flake. The harness derives its poll interval from monitor.poll_interval_ms and caps its own iterations, so it fails loudly rather than hanging if the scripted radar ever stops terminating the loop. Coverage of swing_speed.py: 55% -> 85%. The remainder is thread plumbing and trivial accessors. No source changes.
noahjobse
force-pushed
the
test/swing-speed-capture-loop
branch
from
August 22, 2026 09:08
acbcb6f to
41f47fb
Compare
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 does this PR do?
Adds nine tests covering
SwingSpeedMonitor._capture_loop, which had no test coverage. Test-only: no source changes.Why was this required?
_capture_loopis the decision core of swing speed training. It decides when a swing starts, when silence means it ended, whether motion was too brief to count, and how long to suppress readings so a follow-through does not register as a second rep.None of that was tested.
swing_speed.pysat at 55%, with lines 165-240 (the entire loop body) uncovered, so any of those decisions could be broken by a refactor with the suite still green. The existing tests intests/test_swing_speed.pycover_build_event,_select_swing_reading,connectanddisconnect, but nothing drives the loop itself.CONTRIBUTING lists test coverage improvements as a high-priority contribution.
Automated tests
Nine tests, one per distinct loop behaviour:
starts_swing_on_first_qualifying_readingaccumulates_readings_into_a_single_swingend_quiet_msof silencewaits_for_end_quiet_ms_before_emittingsingle_reading_peak_mphis rejected as noiserejects_motion_with_too_few_readingsaccepts_a_single_reading_above_the_peak_thresholdcooldown_msblocks a second swing openingsuppresses_readings_during_cooldownforwards_selected_readings_to_live_callbackignores_non_qualifying_readingsread_speed_nonblockingfalls_back_to_single_reading_when_batch_is_emptyOn the approach. The loop's behaviour is defined by elapsed time, so the tests replace the module's
timereference with a controlled clock rather than shrinking the constructor timings. Two reasons: the shipped defaults (end_quiet_ms=1000,cooldown_ms=750) stay the values actually under test, and the tests cannot flake, since time only advances when the test says so. The whole file runs in 0.21s.The harness derives its poll interval from
monitor.poll_interval_msrather than a hardcoded copy, so_idle(ms)keeps spanning real simulated time if that default ever changes. It also caps its own iterations and asserts the loop logged no errors, so a future drift between_ScriptedRadarand the realOPS243Radarfails loudly instead of spinning forever on a clock that only advances in memory.Coverage of
swing_speed.py, measured with the new tests deselected and then selected:The only lines still uncovered inside the loop are
227(the_event_callbackdispatch branch, since the tests readget_events()instead of registering a callback) and233-240(the broadexcept Exceptionhandler). Everything else still missing is inconnect/disconnect/get_radar_info, outside this PR's scope.Manual (human) testing
1. Ran the feature for real. Started
openflight-server --web-port 8082 --mock --mock-swing-speedand fired simulated swings over the websocket. Got well-formed events:peak_speed_mph93.2 / 98.3 / 90.9,reading_count: 4,trigger_speed_mph: 76.0,mode: "swing-speed", with session stats aggregating correctly.To be precise about what that proves: that path runs
MockSwingSpeedMonitor, which bypasses_capture_loopentirely. So it confirms theSwingSpeedEventshape my tests construct matches what the app really emits, but it does not exercise the loop. I do not have an OPS243, so the loop itself is covered by the tests only.2. Mutation tested the tests. Broke one loop behaviour at a time in the source and confirmed the intended test failed each time, with a control run to confirm the harness was not failing for unrelated reasons:
Source restored and re-verified clean afterwards.
3. Verified the harness guard actually works. Renamed
read_speed_candidates_nonblockingin the source so the loop can never reach the scripted radar that stops it. Because_capture_loopswallows every exception and the fake clock only advances in memory, an unguarded harness would hang the suite here. Result:1 failedin 0.3s withAssertionError: _capture_loop logged 605 error(s): ... object has no attribute 'read_speed_candidates_renamed'. Fails fast and loudly rather than hanging.4. Full suite and linters: 1350 passed, 8 skipped.
pylint src/openflight/ --fail-under=9scores 9.72.ruff check src/openflight/clean. UI builds and lints clean (untouched by this change).One observation, not part of this PR
While writing these I noticed the three conditions after
reading is not Nonein thequalifiesexpression (swing_speed.py:184-188) are already guaranteed by_select_swing_reading, which filters on direction,trigger_threshold_mphand_within_max_speedbefore returning. Deleting them fromqualifiesbreaks no test. Not touching it here to keep this scoped to one thing; happy to follow up separately if that is useful.No CHANGELOG entry, matching the two most recent merged test-only commits (
907b125,a716b8d), neither of which added one.Checklist
_capture_loopuv run pytest tests/ -v) - 1350 passed, 8 skippeduv run pylint src/openflight/ --fail-under=9) - 9.72/10uv run ruff check src/openflight/)cd ui && npm run build)cd ui && npm run lint)