test: wait for the player state instead of sleeping - #186
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Hello there! Your PR checks are ready for review. ✨I've aggregated the results of the automated checks for this PR below. 📋 Repo HealthChecking if the repo is following its diet. 🥗 ✅ All required files present. Latest Version: ✅ 🔍 LintHere's the latest update on this check. 🗞️ ❌ ruff: issues found — see job log 🏷️ Release PreviewI've checked the 'Migration Guide' for clarity. 🗺️ Current:
✅ PR title follows conventional commit format. 🚀 Release Channel Compatibility Predicted next version:
🔒 Security (pip-audit)Scanning for any potential man-in-the-middle risks. 👨💻 ✅ No known vulnerabilities found (129 packages scanned). ⚖️ License CheckEnsuring no unlicensed code has snuck in. 🕵️ ✅ No license violations found. Policy: Apache 2.0 (universal donor). StrongCopyleft / NetworkCopyleft / WeakCopyleft / Other / Error categories fail. MPL allowed. 📊 CoverageCalculating the density of our test suite. 🧮 Files below 80% coverage (15 files)
Full report: download the 🔨 Build TestsThe build process has successfully terminated. 🏁 ✅ All versions pass
The automation engine never sleeps. 🚂 |
TestOCPHarnessNamespaceBridging asserted the player state right after a fixed time.sleep() following each bus emit. Under full-suite load the FakeBus dispatch thread can take longer than the sleep to update the player, so test_no_bridging_isolates_spec_from_legacy intermittently observed PlayerState.STOPPED instead of PLAYING/PAUSED (it passes in isolation, where dispatch is fast). Replaced every fixed sleep in the class, including the one inside the shared _play_then() helper, with a bounded poll (10ms interval, 2s timeout) on h.player.state via a small local helper. The assertions themselves are unchanged. Full unit suite: 688 passed, 24 skipped on Python 3.10 (x2) and 3.11 (x1).
57b147c to
d475857
Compare
1 participant
TestOCPHarnessNamespaceBridging::test_no_bridging_isolates_spec_from_legacyintermittently fails when the full unit suite runs, withExpected PlayerState.PLAYING, got PlayerState.STOPPED, but always passes when run alone. Each test in that class emits a bus message, sleeps for a fixed 0.05–0.2s, then asserts the player state. That includes the shared_play_then()helper, which starts playback and immediately checks forPLAYING. Under load theFakeBusdispatch thread can take longer than the fixed sleep to actually updateplayer.state, so the assertion races the dispatch.The fix drops every fixed sleep in this class in favor of a small local helper that polls
player.stateevery 10ms for up to 2 seconds and returns as soon as it matches the expected state (or the timeout elapses). This is applied to_play_then()and to all three tests that follow the same emit-sleep-assert shape:test_cork_via_legacy_topic_natively,test_cork_via_spec_topic_through_bridging, andtest_no_bridging_isolates_spec_from_legacy. What each test asserts is unchanged.I could not force the flake to reproduce locally despite several attempts (CPU-quota throttling down to 0.25 cores via
cap, and running with six CPU-bound competing processes), so I cannot paste a captured failing run. The fix targets the documented race (fixed sleep racing an async dispatch thread) directly and cannot regress passing behavior, since it only makes assertions wait longer for the same expected state rather than changing what is checked. Full unit suite (pytest test/unittests) passed at 688 passed / 24 skipped on Python 3.10 (two runs) and Python 3.11 (one run) after the change.