feat: MockTTS — emit audio_output_end on delay for speak_dialog(wait=True)#102
Conversation
TTS.playback is a class-level attribute shared by every TTS instance in the process. The inherited TTS.__del__ chains into TTS.stop() -> TTS.playback.stop(), so when an earlier PlaybackServiceHarness's MockTTS is garbage-collected its destructor terminated whatever PlaybackThread was *currently* registered there — which by then belongs to a later, still-running harness. The victim thread had _terminated set and exited its loop, so its queued speak never played and ovos.audio.output.ended was never emitted, hanging the next speak() until timeout. GC timing made this a flaky TimeoutError that surfaced only after several harness create/destroy cycles (e.g. mid-file in a consumer's test/end2end suite). Override MockTTS.__del__ as a no-op: the harness already manages playback-thread lifecycle explicitly via PlaybackService.shutdown() on context exit, so a mock instance must never tear down the shared thread on collection. Add regression tests: a deterministic guard that fires a stale mock's destructor while a later harness owns TTS.playback and asserts the live thread is neither terminated nor unable to keep speaking, plus a many-sequential-harnesses smoke test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…True) Skills calling speak_dialog(..., wait=True) block on recognizer_loop:audio_output_end via SessionManager.wait_while_speaking. Without a real TTS the handler thread blocks for 15+s, tripping the §8.3 10s handler backstop and spurious handler.error. MockTTS schedules audio_output_end on a 0.1s Timer from the speak handler. Uses bus.ee.emit (not bus.emit) to bypass FakeBus namespace-migration and on_message side effects so the synthetic event is invisible to test captures.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthrough
MockTTS GC fix and synthetic TTS completion signal
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks 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 |
Ping! I've got your results right here. 🛎️I've aggregated the results of the automated checks for this PR below. 🏷️ Release PreviewI've generated a preview of the upcoming changes. 🎬 Current:
✅ PR title follows conventional commit format. 🚀 Release Channel Compatibility Predicted next version:
🔒 Security (pip-audit)I've scanned the dependencies for any hidden surprises. 🔍 ✅ No known vulnerabilities found (78 packages scanned). 📋 Repo HealthEnsuring the repo's heart is beating steady (aka main branch). ❤️ ✅ All required files present. Latest Version: ✅ ⚖️ License CheckA detailed legal audit of your PR. 📖 ✅ No license violations found. Policy: Apache 2.0 (universal donor). StrongCopyleft / NetworkCopyleft / WeakCopyleft / Other / Error categories fail. MPL allowed. 🔍 LintI've double-checked the data for any anomalies. 🔍 ❌ ruff: issues found — see job log 🔨 Build TestsThe build report has been filed and is ready. 📁 ✅ All versions pass
📊 CoverageA comprehensive review of our code coverage. 📖 ❌ 53.2% total coverage Files below 80% coverage (17 files)
Full report: download the Helping you build the future of voice, one check at a time. 🎙️ |
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Skills calling
speak_dialog(..., wait=True)block onrecognizer_loop:audio_output_endviaSessionManager.wait_while_speaking. Without a real TTS the handler thread blocks for 15+s, tripping the §8.3 10s handler backstop and producing spurioushandler.error.MockTTS schedules
audio_output_endon a 0.1s Timer from the speak handler. Usesbus.ee.emit(notbus.emit) to bypass FakeBus namespace-migration andon_messageside effects so the synthetic event is invisible to test captures.Fixes the
test_adapt_matchandtest_fallback_matchtimeouts in ovos-core's end2end suite.Summary by CodeRabbit