diff --git a/.github/workflows/build-tests.yml b/.github/workflows/build-tests.yml index 6af0db7..0b0cec9 100644 --- a/.github/workflows/build-tests.yml +++ b/.github/workflows/build-tests.yml @@ -11,5 +11,5 @@ jobs: secrets: inherit with: python_versions: '["3.10", "3.11", "3.12", "3.13", "3.14"]' - install_extras: 'audio,pydantic,media,listener' + install_extras: 'audio,pydantic,media,listener,bench' test_path: 'test/unittests/' diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index eb94e5f..e524b47 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -13,5 +13,5 @@ jobs: python_version: '3.11' coverage_source: 'ovoscope' test_path: 'test/unittests/' - test_extras: 'audio,pydantic,media,listener' + test_extras: 'audio,pydantic,media,listener,bench' min_coverage: 0 diff --git a/CHANGELOG.md b/CHANGELOG.md index e0fb8ef..cc54683 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,228 +1,84 @@ # Changelog -## [1.5.0a1](https://github.com/OpenVoiceOS/ovoscope/tree/1.5.0a1) (2026-07-02) +## [1.6.7a1](https://github.com/OpenVoiceOS/ovoscope/tree/1.6.7a1) (2026-08-13) -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/1.4.0a1...1.5.0a1) +[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/1.6.6a1...1.6.7a1) **Merged pull requests:** -- feat: add a pipeline\_id filter to End2EndTest [\#112](https://github.com/OpenVoiceOS/ovoscope/pull/112) ([JarbasAl](https://github.com/JarbasAl)) +- fix: accept canonical spec topics in captured-stream assertions [\#136](https://github.com/OpenVoiceOS/ovoscope/pull/136) ([JarbasAl](https://github.com/JarbasAl)) -## [1.4.0a1](https://github.com/OpenVoiceOS/ovoscope/tree/1.4.0a1) (2026-06-29) +## [1.6.6a1](https://github.com/OpenVoiceOS/ovoscope/tree/1.6.6a1) (2026-08-11) -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/1.3.0a1...1.4.0a1) +[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/1.6.5a1...1.6.6a1) **Merged pull requests:** -- feat: skill\_id lifecycle filter + eof\_count for End2EndTest [\#110](https://github.com/OpenVoiceOS/ovoscope/pull/110) ([JarbasAl](https://github.com/JarbasAl)) +- fix: drain pyee listeners before teardown to avoid reentrant-\_\_del\_\_ deadlock [\#132](https://github.com/OpenVoiceOS/ovoscope/pull/132) ([JarbasAl](https://github.com/JarbasAl)) -## [1.3.0a1](https://github.com/OpenVoiceOS/ovoscope/tree/1.3.0a1) (2026-06-29) +## [1.6.5a1](https://github.com/OpenVoiceOS/ovoscope/tree/1.6.5a1) (2026-08-11) -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/1.2.0a1...1.3.0a1) +[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/1.6.4a1...1.6.5a1) **Merged pull requests:** -- feat: emit recognizer\_loop:audio\_output\_start in \_mock\_tts alongside audio\_output\_end [\#108](https://github.com/OpenVoiceOS/ovoscope/pull/108) ([JarbasAl](https://github.com/JarbasAl)) +- fix: resolve unbounded get\_response\(\) hang on FakeBus [\#130](https://github.com/OpenVoiceOS/ovoscope/pull/130) ([JarbasAl](https://github.com/JarbasAl)) -## [1.2.0a1](https://github.com/OpenVoiceOS/ovoscope/tree/1.2.0a1) (2026-06-29) +## [1.6.4a1](https://github.com/OpenVoiceOS/ovoscope/tree/1.6.4a1) (2026-08-02) -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/1.1.0a2...1.2.0a1) +[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/1.6.3a2...1.6.4a1) **Merged pull requests:** -- feat: MockTTS publishes audio\_output\_end via the full bus \(faithful\) [\#106](https://github.com/OpenVoiceOS/ovoscope/pull/106) ([JarbasAl](https://github.com/JarbasAl)) +- fix: MiniCroft boots against older ovos-core SkillManager \(backwards compat\) [\#128](https://github.com/OpenVoiceOS/ovoscope/pull/128) ([JarbasAl](https://github.com/JarbasAl)) -## [1.1.0a2](https://github.com/OpenVoiceOS/ovoscope/tree/1.1.0a2) (2026-06-29) +## [1.6.3a2](https://github.com/OpenVoiceOS/ovoscope/tree/1.6.3a2) (2026-07-31) -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/1.1.0a1...1.1.0a2) +[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/1.6.3a1...1.6.3a2) **Merged pull requests:** -- docs: clarify MockTTS bus.ee.emit rationale [\#104](https://github.com/OpenVoiceOS/ovoscope/pull/104) ([JarbasAl](https://github.com/JarbasAl)) +- docs: gui-testing.md matches the exact-match assertion semantics [\#125](https://github.com/OpenVoiceOS/ovoscope/pull/125) ([JarbasAl](https://github.com/JarbasAl)) -## [1.1.0a1](https://github.com/OpenVoiceOS/ovoscope/tree/1.1.0a1) (2026-06-29) +## [1.6.3a1](https://github.com/OpenVoiceOS/ovoscope/tree/1.6.3a1) (2026-07-31) -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/1.0.2a1...1.1.0a1) +[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/1.6.2a2...1.6.3a1) **Merged pull requests:** -- feat: MockTTS — emit audio\_output\_end on delay for speak\_dialog\(wait=True\) [\#102](https://github.com/OpenVoiceOS/ovoscope/pull/102) ([JarbasAl](https://github.com/JarbasAl)) +- fix: Han audit round 2 — SkillApi retention, accuracy gate, false-green assertions, race windows [\#123](https://github.com/OpenVoiceOS/ovoscope/pull/123) ([JarbasAl](https://github.com/JarbasAl)) -## [1.0.2a1](https://github.com/OpenVoiceOS/ovoscope/tree/1.0.2a1) (2026-06-27) +## [1.6.2a2](https://github.com/OpenVoiceOS/ovoscope/tree/1.6.2a2) (2026-07-31) -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/1.0.1a1...1.0.2a1) +[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/1.6.2a1...1.6.2a2) **Merged pull requests:** -- fix: MockTTS destructor must not stop the shared playback thread [\#100](https://github.com/OpenVoiceOS/ovoscope/pull/100) ([JarbasAl](https://github.com/JarbasAl)) +- docs: close the documentation audit gaps \(Han audit round 1\) [\#121](https://github.com/OpenVoiceOS/ovoscope/pull/121) ([JarbasAl](https://github.com/JarbasAl)) -## [1.0.1a1](https://github.com/OpenVoiceOS/ovoscope/tree/1.0.1a1) (2026-06-27) +## [1.6.2a1](https://github.com/OpenVoiceOS/ovoscope/tree/1.6.2a1) (2026-07-31) -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/1.0.0a1...1.0.1a1) +[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/1.6.1a1...1.6.2a1) **Merged pull requests:** -- fix: guard None blacklisted\_skills/intents in final-session check [\#98](https://github.com/OpenVoiceOS/ovoscope/pull/98) ([JarbasAl](https://github.com/JarbasAl)) +- fix: Han audit round 1 — teardown safety, session isolation, harness lifecycle [\#118](https://github.com/OpenVoiceOS/ovoscope/pull/118) ([JarbasAl](https://github.com/JarbasAl)) -## [1.0.0a1](https://github.com/OpenVoiceOS/ovoscope/tree/1.0.0a1) (2026-06-25) +## [1.6.1a1](https://github.com/OpenVoiceOS/ovoscope/tree/1.6.1a1) (2026-07-24) -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/0.22.1a1...1.0.0a1) - -**Breaking changes:** - -- feat!: audio harness on OVOS spec bus namespace [\#92](https://github.com/OpenVoiceOS/ovoscope/pull/92) ([JarbasAl](https://github.com/JarbasAl)) - -## [0.22.1a1](https://github.com/OpenVoiceOS/ovoscope/tree/0.22.1a1) (2026-06-25) - -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/0.22.0a1...0.22.1a1) - -**Merged pull requests:** - -- fix: pytest 9 compatibility for the pytest11 plugin [\#88](https://github.com/OpenVoiceOS/ovoscope/pull/88) ([JarbasAl](https://github.com/JarbasAl)) - -## [0.22.0a1](https://github.com/OpenVoiceOS/ovoscope/tree/0.22.0a1) (2026-06-25) - -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/0.21.1a1...0.22.0a1) - -**Merged pull requests:** - -- feat: stream audio frames through MiniListener for multi-frame decoders [\#86](https://github.com/OpenVoiceOS/ovoscope/pull/86) ([JarbasAl](https://github.com/JarbasAl)) - -## [0.21.1a1](https://github.com/OpenVoiceOS/ovoscope/tree/0.21.1a1) (2026-06-25) - -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/0.21.0a1...0.21.1a1) - -**Merged pull requests:** - -- fix: repair ovoscope record in-process path \(default\_pipeline kwarg + from\_message skill\_ids\) [\#85](https://github.com/OpenVoiceOS/ovoscope/pull/85) ([JarbasAl](https://github.com/JarbasAl)) - -## [0.21.0a1](https://github.com/OpenVoiceOS/ovoscope/tree/0.21.0a1) (2026-06-25) - -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/0.20.0a1...0.21.0a1) - -**Merged pull requests:** - -- feat: export ovos-media OCP harness from the package + add \[media\] extra [\#89](https://github.com/OpenVoiceOS/ovoscope/pull/89) ([JarbasAl](https://github.com/JarbasAl)) - -## [0.20.0a1](https://github.com/OpenVoiceOS/ovoscope/tree/0.20.0a1) (2026-06-24) - -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/0.19.4a1...0.20.0a1) - -**Merged pull requests:** - -- feat: assert\_template\_shown for SYSTEM\_\* GUI templates [\#83](https://github.com/OpenVoiceOS/ovoscope/pull/83) ([JarbasAl](https://github.com/JarbasAl)) - -## [0.19.4a1](https://github.com/OpenVoiceOS/ovoscope/tree/0.19.4a1) (2026-06-17) - -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/0.19.3a1...0.19.4a1) - -**Merged pull requests:** - -- fix\(tts-intelligibility\): normalise rendered audio to 16kHz mono before STT [\#81](https://github.com/OpenVoiceOS/ovoscope/pull/81) ([JarbasAl](https://github.com/JarbasAl)) - -## [0.19.3a1](https://github.com/OpenVoiceOS/ovoscope/tree/0.19.3a1) (2026-06-17) - -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/0.19.2a1...0.19.3a1) - -**Merged pull requests:** - -- fix\(tts-intelligibility\): transcode non-WAV engine output before scoring [\#79](https://github.com/OpenVoiceOS/ovoscope/pull/79) ([JarbasAl](https://github.com/JarbasAl)) - -## [0.19.2a1](https://github.com/OpenVoiceOS/ovoscope/tree/0.19.2a1) (2026-06-16) - -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/0.19.1a2...0.19.2a1) - -**Merged pull requests:** - -- fix\(tts-intelligibility\): score synthesis failures as total miss, not abort [\#77](https://github.com/OpenVoiceOS/ovoscope/pull/77) ([JarbasAl](https://github.com/JarbasAl)) - -## [0.19.1a2](https://github.com/OpenVoiceOS/ovoscope/tree/0.19.1a2) (2026-06-15) - -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/0.19.1a1...0.19.1a2) - -**Merged pull requests:** - -- feat: TTS end-to-end intelligibility harness [\#75](https://github.com/OpenVoiceOS/ovoscope/pull/75) ([JarbasAl](https://github.com/JarbasAl)) - -## [0.19.1a1](https://github.com/OpenVoiceOS/ovoscope/tree/0.19.1a1) (2026-06-14) - -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/0.19.0a3...0.19.1a1) - -**Merged pull requests:** - -- fix: drop removed 'path' arg from pytest\_pycollect\_makemodule hook \(pytest\>=8 compat\) [\#73](https://github.com/OpenVoiceOS/ovoscope/pull/73) ([JarbasAl](https://github.com/JarbasAl)) - -## [0.19.0a3](https://github.com/OpenVoiceOS/ovoscope/tree/0.19.0a3) (2026-06-13) - -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/0.19.0a2...0.19.0a3) - -**Merged pull requests:** - -- chore: remove agent-audit scratch files [\#71](https://github.com/OpenVoiceOS/ovoscope/pull/71) ([JarbasAl](https://github.com/JarbasAl)) - -## [0.19.0a2](https://github.com/OpenVoiceOS/ovoscope/tree/0.19.0a2) (2026-06-13) - -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/0.19.0a1...0.19.0a2) - -**Merged pull requests:** - -- docs: standardize NGI0 Commons Fund attribution [\#69](https://github.com/OpenVoiceOS/ovoscope/pull/69) ([JarbasAl](https://github.com/JarbasAl)) - -## [0.19.0a1](https://github.com/OpenVoiceOS/ovoscope/tree/0.19.0a1) (2026-06-12) - -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/0.18.0a1...0.19.0a1) - -**Merged pull requests:** - -- feat: MiniVoiceLoop + simple/classic listener bus-sequence harnesses [\#67](https://github.com/OpenVoiceOS/ovoscope/pull/67) ([JarbasAl](https://github.com/JarbasAl)) - -## [0.18.0a1](https://github.com/OpenVoiceOS/ovoscope/tree/0.18.0a1) (2026-06-10) - -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/0.17.1a1...0.18.0a1) - -**Merged pull requests:** - -- feat\(phal\): plugin\_factories for MiniPHAL and PHALTest [\#65](https://github.com/OpenVoiceOS/ovoscope/pull/65) ([JarbasAl](https://github.com/JarbasAl)) - -## [0.17.1a1](https://github.com/OpenVoiceOS/ovoscope/tree/0.17.1a1) (2026-05-20) - -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/0.17.0a1...0.17.1a1) - -**Merged pull requests:** - -- fix\(pipeline-harness\): default \_SinkSkill bus to FakeBus [\#62](https://github.com/OpenVoiceOS/ovoscope/pull/62) ([JarbasAl](https://github.com/JarbasAl)) - -## [0.17.0a1](https://github.com/OpenVoiceOS/ovoscope/tree/0.17.0a1) (2026-05-14) - -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/0.16.0a1...0.17.0a1) - -**Merged pull requests:** - -- feat\(intent-cases\): markdown reporter, baseline diff, auto-discovery, deterministic m2v warmup [\#60](https://github.com/OpenVoiceOS/ovoscope/pull/60) ([JarbasAl](https://github.com/JarbasAl)) - -## [0.16.0a1](https://github.com/OpenVoiceOS/ovoscope/tree/0.16.0a1) (2026-05-14) - -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/0.15.0a1...0.16.0a1) - -**Merged pull requests:** - -- feat\(intent-cases\): file-based intent test layout + pytest accuracy gate [\#58](https://github.com/OpenVoiceOS/ovoscope/pull/58) ([JarbasAl](https://github.com/JarbasAl)) - -## [0.15.0a1](https://github.com/OpenVoiceOS/ovoscope/tree/0.15.0a1) (2026-05-14) - -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/0.14.0a1...0.15.0a1) +[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/1.6.0a1...1.6.1a1) **Merged pull requests:** -- feat\(e2e\): reusable harness, bus helpers, and intent-registration shims [\#55](https://github.com/OpenVoiceOS/ovoscope/pull/55) ([JarbasAl](https://github.com/JarbasAl)) +- fix: restore SessionManager.bus when MiniCroft stops [\#116](https://github.com/OpenVoiceOS/ovoscope/pull/116) ([JarbasAl](https://github.com/JarbasAl)) -## [0.14.0a1](https://github.com/OpenVoiceOS/ovoscope/tree/0.14.0a1) (2026-05-14) +## [1.6.0a1](https://github.com/OpenVoiceOS/ovoscope/tree/1.6.0a1) (2026-07-16) -[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/0.13.1...0.14.0a1) +[Full Changelog](https://github.com/OpenVoiceOS/ovoscope/compare/1.5.0...1.6.0a1) **Merged pull requests:** -- feat: add NEBULENTO\_PIPELINE and PALAVREADO\_PIPELINE stage groups [\#54](https://github.com/OpenVoiceOS/ovoscope/pull/54) ([JarbasAl](https://github.com/JarbasAl)) +- feat: per-clip WakeWordProbe for benchmark/test harnesses [\#97](https://github.com/OpenVoiceOS/ovoscope/pull/97) ([JarbasAl](https://github.com/JarbasAl)) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..f0397c8 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,35 @@ +# Contributing + +## Dev setup + +```bash +git clone https://github.com/TigreGotico/ovoscope +cd ovoscope +pip install -e ".[pydantic]" +``` + +## Running tests + +```bash +pytest test/unittests -q +``` + +The full suite (including live OVOS integration tests) is slower and more +sensitive to the local environment; CI is the source of truth for whether a +change is green. + +## Submitting a change + +- Branch off `dev`, not `master`. +- Open a **draft** pull request into `dev`. The maintainer merges when it is + ready — do not merge your own PR. +- Use [Conventional Commits](https://www.conventionalcommits.org/) for + commit messages (`feat:`, `fix:`, `docs:`, `refactor:`, `test:`, `chore:`). +- Keep documentation in `docs/` and `README.md` in sync with any behaviour + change in the same PR. +- Add or update tests for the behaviour you change. + +## License + +By contributing, you agree your contribution is licensed under the +[Apache 2.0 License](LICENSE) that covers this project. diff --git a/FAQ.md b/FAQ.md new file mode 100644 index 0000000..5e67936 --- /dev/null +++ b/FAQ.md @@ -0,0 +1,39 @@ +# FAQ + +Common questions and gotchas when using OvoScope. + +## Does OvoScope need a real OVOS install running? +No. `MiniCroft` runs the skill manager and intent pipeline in-process on a +`FakeBus` — there is no WebSocket server, no PulseAudio/audio stack, and no +network access required for `record`/`run`/`diff`/`validate`. See +[docs/index.md](docs/index.md) for what the in-process harness does and does +not cover, and [docs/phal.md](docs/phal.md) / [docs/audio-testing.md](docs/audio-testing.md) +for the separate PHAL and audio harnesses. + +## Why did my test fail with extra/missing messages I didn't expect? +Only the keys you specify in `expected.data` and `expected.context` are +checked — extra keys in the received message are ignored. If a message you +didn't list is showing up as unexpected, check `ignore_messages` (see +[docs/end2end-test.md](docs/end2end-test.md)) — some message types (e.g. +`speak`, `ovos.skills.settings_changed`) are noisy or non-deterministic and +are commonly filtered out. + +## My fixture recording is flaky / times out. +Increase `--timeout` on `ovoscope record`, or the `timeout` kwarg on +`End2EndTest.from_message`. If the skill under test depends on plugins that +take time to warm up (e.g. `ovos-m2v-pipeline` syncing its label index), +see `m2v_warmup` in [docs/intent-cases.md](docs/intent-cases.md) for how +`ovoscope.intent_cases` handles that deterministically. + +## How do I test a skill that isn't installed as a plugin yet? +Pass it via `extra_skills={"my-skill.test": MySkillClass}` to `MiniCroft` / +`get_minicroft()` instead of `skill_ids`. See +[docs/minicroft.md](docs/minicroft.md). + +## How do I test multiple languages? +Pass `secondary_langs=[...]` to `get_minicroft()` so Adapt/Padatious +register vocab for each locale. See "Multilingual Testing" in +[docs/minicroft.md](docs/minicroft.md). + +## Where do I report a bug or ask something not covered here? +Open an issue on [GitHub](https://github.com/TigreGotico/ovoscope/issues). diff --git a/README.md b/README.md index 4c16bb6..1804e72 100644 --- a/README.md +++ b/README.md @@ -120,7 +120,10 @@ stages and deliberately excludes persona, Ollama, OCP, and m2v plugins. | [docs/minicroft.md](docs/minicroft.md) | `MiniCroft` and `get_minicroft()` reference | | [docs/capture-session.md](docs/capture-session.md) | `CaptureSession` internals | | [docs/end2end-test.md](docs/end2end-test.md) | `End2EndTest` full parameter reference | +| [docs/e2e-pipeline-harness.md](docs/e2e-pipeline-harness.md) | `E2EPipelineHarness` — testing a single pipeline plugin against raw bus messages | +| [docs/intent-cases.md](docs/intent-cases.md) | File-based intent test cases (`.intent.test`) via `register_intent_case_tests` | | [docs/pydantic-integration.md](docs/pydantic-integration.md) | Typed message models with `ovos-pydantic-models` | +| [docs/cli.md](docs/cli.md) | `ovoscope` CLI — record/run/diff/validate/coverage/bus-coverage, plus `ovoscope-setup` | | [FAQ.md](FAQ.md) | Common questions and gotchas | --- @@ -142,20 +145,21 @@ under grant agreement No [101135429](https://cordis.europa.eu/project/id/1011354 --- ## License + [Apache 2.0](LICENSE) + --- + ## Contributing + PRs are welcome! See [CONTRIBUTING.md](CONTRIBUTING.md) for guidelines. + --- + ## AI Disclosure -Parts of this project are developed with the assistance of AI tools. -In the interest of transparency, two files are maintained as a public record of AI involvement: -- **[FAQ.md](FAQ.md)** — Frequently asked questions that emerged from real development sessions, - including design rationale, gotchas, and usage patterns. Many entries were authored or - refined with AI assistance during the process of building and testing this framework. -- **[MAINTENANCE_REPORT.md](MAINTENANCE_REPORT.md)** — A chronological log of changes made to - this repository. Each entry records what was changed, why, which AI model was involved, what - actions it took, and what human oversight was applied. This log is updated after every - significant AI-assisted session. -These files are intentionally published so that contributors and users can understand how the -project evolves and where AI assistance has been applied. + +Parts of this project — including code, tests, and documentation — are developed with the +assistance of AI coding agents, under human review before merge. Commit messages and pull +request descriptions in the [git history](https://github.com/TigreGotico/ovoscope/commits/dev) +and [CHANGELOG.md](CHANGELOG.md) note when a change originated from an AI-assisted session, so +contributors and users can see where AI assistance has been applied. diff --git a/docs/bus-coverage.md b/docs/bus-coverage.md index 72df1a2..3e84304 100644 --- a/docs/bus-coverage.md +++ b/docs/bus-coverage.md @@ -174,4 +174,4 @@ If you are building custom tooling, you can access these values via `SkillBusCov * `observed_emitter_pct`: `(observed_emitters / total_emitters) * 100` * `asserted_emitter_pct`: `(asserted_emitters / total_emitters) * 100` -Source: `SkillBusCoverage` — `ovoscope/bus_coverage.py:118` +Source: `SkillBusCoverage` — `ovoscope/bus_coverage.py` diff --git a/docs/capture-session.md b/docs/capture-session.md index 3ca112b..209fe95 100644 --- a/docs/capture-session.md +++ b/docs/capture-session.md @@ -1,11 +1,11 @@ # CaptureSession `CaptureSession` subscribes to all messages on the `FakeBus` and records them during a single test interaction. It handles synchronous responses (ordered, from the intent pipeline) and asynchronous responses (from external threads, unordered). -## Class: `CaptureSession` — `ovoscope/__init__.py:488` +## Class: `CaptureSession` — `ovoscope/__init__.py` ```python from ovoscope import CaptureSession ``` A `dataclass` that wraps a `MiniCroft` and manages message collection for one test interaction. -`CaptureSession.finish` — `ovoscope/__init__.py:521` +`CaptureSession.finish` — `ovoscope/__init__.py` > **Idempotency:** `finish()` may be called multiple times safely — subsequent calls > return the same message list without re-subscribing or clearing state. diff --git a/docs/cli.md b/docs/cli.md index 71e9c76..771d2c1 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -1,6 +1,6 @@ # ovoscope CLI -The `ovoscope` command-line tool provides five subcommands for recording, +The `ovoscope` command-line tool provides six subcommands for recording, replaying, diffing, validating, and scanning E2E test fixtures. ## Installation @@ -126,6 +126,60 @@ ovoscope coverage "OpenVoiceOS Workspace/" --format json --- +### `ovoscope bus-coverage` — Bus handler/emitter coverage + +Runs every fixture found under a directory (or a single fixture file), and +reports which bus message types each skill actually listens for and emits, +merged across all fixtures — `cli.py:cmd_bus_coverage`. + +```bash +ovoscope bus-coverage test/fixtures/ +ovoscope bus-coverage test/fixtures/hello.json --format json +ovoscope bus-coverage test/fixtures/ --skill-id ovos-skill-hello-world.openvoiceos --verbose +``` + +| Flag | Default | Description | +|------|---------|-------------| +| `test_dir` | **required** | Directory of fixture JSON files, or a single fixture file. | +| `--skill-id` | None | Only report on fixtures that include this skill_id. | +| `--format` | `table` | Output format: `table` or `json`. | +| `--verbose` / `-v` | False | Print per-message-type detail rows. | + +Fixtures that fail to load or time out booting `MiniCroft` are skipped and +counted; the run still reports coverage for the fixtures that succeeded. + +--- + +## `ovoscope-setup` — Install the skill into AI coding assistants + +`ovoscope-setup` is a separate console script (`setup_skill.py`) that installs +the ovoscope Claude Code / Gemini CLI skill — `SKILL.md`, docs, and `FAQ.md` — +downloaded from GitHub at install time. + +```bash +ovoscope-setup # auto-detect and install all +ovoscope-setup --claude # Claude Code only +ovoscope-setup --gemini # Gemini CLI only (project-level) +ovoscope-setup --gemini --path /my/workspace +ovoscope-setup --list # show detected tools without installing +ovoscope-setup --no-docs # skip docs download (offline / CI) +ovoscope-setup --uninstall --claude +``` + +| Flag | Default | Description | +|------|---------|-------------| +| `--claude` | False | Install for Claude Code (`~/.claude/skills/ovoscope/`). | +| `--gemini` | False | Install for Gemini CLI (`/.gemini/skills/ovoscope/`). Project-level. | +| `--path` | current directory | Project root for the Gemini install. | +| `--list` | False | Show which tools are detected on `PATH` without installing anything. | +| `--no-docs` | False | Skip downloading documentation from GitHub (offline / CI). | +| `--uninstall` | False | Remove the skill instead of installing it. | + +With no explicit `--claude`/`--gemini` flag, the tool auto-detects which of +`claude`/`gemini` are on `PATH` and installs for those. + +--- + ## Exit Codes | Code | Meaning | diff --git a/docs/e2e-pipeline-harness.md b/docs/e2e-pipeline-harness.md new file mode 100644 index 0000000..efd0d47 --- /dev/null +++ b/docs/e2e-pipeline-harness.md @@ -0,0 +1,132 @@ +# E2E Pipeline Harness +`ovoscope.e2e` provides scaffolding for end-to-end tests of a single +`ConfidenceMatcherPipeline` plugin (Adapt, Padatious, Padacioso, Nebulento, +Palavreado, and similar engine families). Most such plugins need the same +shape of test: patch the plugin's config, boot a `MiniCroft` pinned to that +one pipeline, drive the bus with utterances, and assert on the dispatched +intent message (or the `complete_intent_failure` fallback). `ovoscope.e2e` +factors that shape out so a plugin only has to subclass +`E2EPipelineHarness` and set a handful of class attributes. + +It also exposes standalone bus helpers and engine-family registration shims +for callers that prefer pytest-style tests over `unittest.TestCase`. + +```python +from ovoscope.e2e import E2EPipelineHarness +``` + +## Class: `E2EPipelineHarness` (`ovoscope/e2e.py`) + +Subclass of `unittest.TestCase`. Set these class attributes: + +| Attribute | Default | Description | +|---|---|---| +| `PIPELINE_ID` | `""` | OPM `opm.pipeline` entry-point name to pin `MiniCroft` to (e.g. `"ovos-nebulento-pipeline-plugin"`). `setUpClass` skips the test class if unset. | +| `CONFIG_KEY` | `""` | Key under `Configuration()["intents"]` for the plugin's config. `setUpClass` skips the test class if unset. | +| `PLUGIN_CONFIG` | `{}` | Dict merged into `Configuration()["intents"][CONFIG_KEY]` before `MiniCroft` starts. Restored on `tearDownClass`. | +| `SKILL_ID` | `"test_skill_ovoscope"` | Skill id used by the helper methods when registering intents. Detached in `setUp` to keep tests isolated. | +| `DEFAULT_LANG` | `"en-US"` | Language used by `make_utterance()` / `send_and_capture()` when no explicit `session` is given. | +| `STARTUP_MAX_WAIT` | `60.0` | Seconds to wait for `MiniCroft` to reach `READY`. | +| `MODERNIZE` | `True` | Forwarded to `MiniCroft`/`FakeBus` — legacy emit also dispatches its `ovos.*` spec counterpart. | +| `EMIT_LEGACY` | `True` | Forwarded to `MiniCroft`/`FakeBus` — spec emit also dispatches the legacy topic. Set both `MODERNIZE` and `EMIT_LEGACY` to `False` to drive a single isolated namespace. | + +`setUpClass` boots one shared `MiniCroft` for the whole class (pinned to +`[PIPELINE_ID]`) and stores the pipeline plugin instance on `cls.pipeline`. +`tearDownClass` stops it and restores the original `Configuration()["intents"]` +entry. + +### Instance attributes and helpers + +| Member | Description | +|---|---| +| `self.mc` | The running `MiniCroft` (class-scoped). | +| `self.bus` | Shortcut for `self.mc.bus`. | +| `self.pipeline` | The loaded pipeline plugin instance. | +| `make_utterance(utterance, *, session=None)` | Build a `recognizer_loop:utterance` `Message` using `DEFAULT_LANG`. | +| `send_and_capture(utterance, expected_types, *, timeout=5.0, session=None)` | Emit `utterance` and return the first message whose type is in `expected_types`, or `None` on `complete_intent_failure`/timeout. | +| `expect_no_match(utterance, *, timeout=2.0, session=None)` | Assert emitting `utterance` produces `complete_intent_failure`. | + +```python +from ovoscope.e2e import E2EPipelineHarness + +class TestNebulento(E2EPipelineHarness): + PIPELINE_ID = "ovos-nebulento-pipeline-plugin" + CONFIG_KEY = "ovos-nebulento-pipeline-plugin" + + def test_match(self): + register_adapt_vocab(self.bus, "greeting", ["hello", "hi"]) + msg = self.send_and_capture("hello", ["greeting_intent"]) + assert msg is not None +``` + +--- + +## Standalone bus helpers + +These work with any bus that implements `.on()` / `.remove()` / `.emit()` +(`FakeBus` or `MessageBusClient`) and do not require `E2EPipelineHarness`. + +### `make_session(session_id="ovoscope-test", *, pipeline=None, blacklisted_intents=None, blacklisted_skills=None, lang="en-US") -> Session` +Build a `Session` with the most common overrides preset. + +### `make_utterance_message(utterance, *, lang="en-US", session=None) -> Message` +Build a `recognizer_loop:utterance` `Message`. If `session` is given, its +serialized form is placed in the message context under `"session"`. + +### `wait_for_match(bus, expected_types, *, timeout=5.0, emit=None) -> Optional[Message]` +Subscribe to `expected_types` and `complete_intent_failure`, then wait for +the first match. Returns the matching `Message`, or `None` on failure or +timeout. + +This helper **blocks**, so a single-threaded caller cannot emit the +utterance and then call `wait_for_match` — the reply may arrive (and be +missed) before the caller resumes. Pass the message to emit as `emit=` +instead: `wait_for_match` subscribes its handlers first, then emits `emit`, +so no reply can be missed. + +```python +from ovoscope.e2e import wait_for_match, make_utterance_message + +msg = wait_for_match( + bus, + ["greeting_intent"], + timeout=5.0, + emit=make_utterance_message("hello"), +) +assert msg is not None +``` + +Only emit the message yourself beforehand (rather than via `emit=`) when +the reply is guaranteed to be asynchronous relative to the emit call. + +### `wait_for_failure(bus, *, timeout=2.0) -> bool` +Wait for a `complete_intent_failure` message; return whether one fired. + +--- + +## Intent-registration shims + +Emit the bus event a given pipeline engine family expects, so tests can +register intents/vocab without constructing the underlying plugin objects +directly. + +| Function | Engine family | Emits | +|---|---|---| +| `register_padatious_intent(bus, name, samples, *, lang="en-US", settle=0.1)` | Padatious, Padacioso, Nebulento | `padatious:register_intent` | +| `register_padatious_entity(bus, name, samples, *, lang="en-US", settle=0.1)` | Padatious, Padacioso, Nebulento | `padatious:register_entity` | +| `register_adapt_vocab(bus, entity_type, words, *, lang="en-US", settle=0.1)` | Adapt, Palavreado | `register_vocab` (one per word) | +| `register_adapt_intent(bus, builder, *, lang="en-US", settle=0.1)` | Adapt, Palavreado | `register_intent`. `builder` may be an `IntentBuilder` (`.build()`-ed automatically) or an already-built intent object. | +| `detach_intent(bus, intent_name, *, settle=0.1)` | any | `detach_intent` | +| `detach_skill(bus, skill_id, *, settle=0.1)` | any | `detach_skill` | + +Every shim sleeps `settle` seconds after emitting (default `0.1`) to give +the pipeline plugin time to process the registration before the caller +proceeds — pass `settle=0` to skip the wait when the caller does its own +synchronization. + +--- + +## Cross-References +- [minicroft.md](minicroft.md) — `MiniCroft` / `get_minicroft()`, the runtime this harness pins to a single pipeline. +- [end2end-test.md](end2end-test.md) — `End2EndTest`, the full declarative multi-message test runner (used by `intent-cases.md` on top of raw `MiniCroft`). +- [intent-cases.md](intent-cases.md) — file-based intent test cases, a higher-level alternative built on `End2EndTest` rather than this harness's bus helpers. diff --git a/docs/end2end-test.md b/docs/end2end-test.md index 466c72a..7009cd9 100644 --- a/docs/end2end-test.md +++ b/docs/end2end-test.md @@ -1,11 +1,11 @@ # End2EndTest `End2EndTest` is the primary API. It wires together `MiniCroft`, `CaptureSession`, and all assertion logic into a single declarative test object. -## Class: `End2EndTest` — `ovoscope/__init__.py:533` +## Class: `End2EndTest` — `ovoscope/__init__.py` ```python from ovoscope import End2EndTest ``` A `dataclass`. Configure once, call `.execute()` to run. -`End2EndTest.execute` — `ovoscope/__init__.py:602` +`End2EndTest.execute` — `ovoscope/__init__.py` --- ## Fields ### Core diff --git a/docs/gui-testing.md b/docs/gui-testing.md index cf1ae47..5a15919 100644 --- a/docs/gui-testing.md +++ b/docs/gui-testing.md @@ -65,7 +65,7 @@ mc.stop() ## Class: `GUICaptureSession` -`GUICaptureSession` — `ovoscope/__init__.py:951` +`GUICaptureSession` — `ovoscope/__init__.py` ```python from ovoscope import GUICaptureSession @@ -89,7 +89,7 @@ recording GUI-prefixed messages. ### Lifecycle Methods -`GUICaptureSession.start` — `ovoscope/__init__.py:1000` +`GUICaptureSession.start` — `ovoscope/__init__.py` ```python gui = GUICaptureSession(mc.bus) @@ -103,16 +103,16 @@ gui.stop() | `start()` | Subscribe to the bus and begin capturing. | | `stop()` | Unsubscribe from the bus and stop capturing. | -`GUICaptureSession.__enter__` / `__exit__` — `ovoscope/__init__.py:1008` +`GUICaptureSession.__enter__` / `__exit__` — `ovoscope/__init__.py` The preferred usage is as a context manager. `__enter__` calls `start()`; `__exit__` calls `stop()`. ### Assertion Methods -#### `assert_page_shown(namespace, page, timeout=2.0)` +#### `assert_page_shown(namespace, page, timeout=2.0, exact=True)` -`GUICaptureSession.assert_page_shown` — `ovoscope/__init__.py:1017` +`GUICaptureSession.assert_page_shown` — `ovoscope/__init__.py` Assert that a `gui.page.show` (or equivalent) message was emitted for the given namespace and page filename. @@ -126,16 +126,19 @@ gui.assert_page_shown("helloworldskill", "hello.qml", timeout=3.0) | `namespace` | `str` | **required** | GUI namespace (typically the skill ID slug, e.g. `"helloworldskill"`). | | `page` | `str` | **required** | QML page filename (e.g. `"hello.qml"`). | | `timeout` | `float` | `2.0` | Max seconds to poll captured messages before failing. | +| `exact` | `bool` | `True` | Exact matching: the namespace must be equal, and the page must equal the shown page's basename. Pass `exact=False` for the legacy substring behavior. | Raises `AssertionError` if no matching message is found within `timeout`. The method checks both `msg.data["namespace"]` / `msg.context["skill_id"]` for the namespace, and `msg.data["pages"]` / `msg.data["page"]` for the -page name. Substring matching is used for both. +page name. By default both use exact matching (`namespace ==`, page +basename `==`), so a near-miss like `hello.qml.bak` or a longer namespace +that merely contains yours cannot satisfy the assertion. -#### `assert_namespace_value(namespace, key, value)` +#### `assert_namespace_value(namespace, key, value, exact=True)` -`GUICaptureSession.assert_namespace_value` — `ovoscope/__init__.py:1046` +`GUICaptureSession.assert_namespace_value` — `ovoscope/__init__.py` Assert that a `gui.value.set` or `gui.namespace.update` message set a specific key to a specific value in the given namespace. @@ -152,9 +155,9 @@ gui.assert_namespace_value("helloworldskill", "greeting", "Hello!") Raises `AssertionError` if no matching message is found. -#### `assert_namespace_has_key(namespace, key)` +#### `assert_namespace_has_key(namespace, key, exact=True)` -`GUICaptureSession.assert_namespace_has_key` — `ovoscope/__init__.py:1093` +`GUICaptureSession.assert_namespace_has_key` — `ovoscope/__init__.py` Assert that a `gui.value.set` or `gui.namespace.update` message set a specific key in the given namespace, regardless of value. Useful for @@ -172,12 +175,14 @@ gui.assert_namespace_has_key("weatherskill", "current_temp") Raises `AssertionError` if no matching message is found. -#### `assert_namespace_cleared(namespace)` +#### `assert_namespace_cleared(namespace, exact=True)` -`GUICaptureSession.assert_namespace_cleared` — `ovoscope/__init__.py:1069` +`GUICaptureSession.assert_namespace_cleared` — `ovoscope/__init__.py` -Assert that a `gui.namespace.remove` or `gui.namespace.clear` message was -emitted for the given namespace. +Assert that a `gui.namespace.remove`, `gui.namespace.clear`, or +`gui.clear.namespace` message was emitted for the given namespace — +`gui.clear.namespace` is the topic the GUI service actually emits at +runtime. The namespace comparison is exact unless `exact=False`. ```python gui.assert_namespace_cleared("helloworldskill") @@ -188,7 +193,7 @@ Raises `AssertionError` if no matching message is found. ## Message Filtering Only messages whose `msg_type` starts with one of the configured `prefixes` -are captured — `GUICaptureSession._on_message` — `ovoscope/__init__.py:984`. +are captured — `GUICaptureSession._on_message` — `ovoscope/__init__.py`. All other bus messages are ignored. Default captured message types (partial list): @@ -262,4 +267,4 @@ requested the right `SYSTEM_*` template with the right session data. - `CaptureSession` — `ovoscope/docs/capture-session.md` (ordered dialogue capture) - `End2EndTest` — `ovoscope/docs/end2end-test.md` (full test runner) - `MiniCroft` / `get_minicroft()` — `ovoscope/docs/minicroft.md` -- `GUI_IGNORED` message list — `ovoscope/__init__.py:24` +- `GUI_IGNORED` message list — `ovoscope/__init__.py` diff --git a/docs/index.md b/docs/index.md index 2d23772..a370ec3 100644 --- a/docs/index.md +++ b/docs/index.md @@ -8,6 +8,8 @@ | [minicroft.md](minicroft.md) | `MiniCroft` — in-process skill runtime | | [capture-session.md](capture-session.md) | `CaptureSession` — message capture during a test | | [end2end-test.md](end2end-test.md) | `End2EndTest` — full test runner reference | +| [e2e-pipeline-harness.md](e2e-pipeline-harness.md) | `E2EPipelineHarness`, `wait_for_match`, `make_utterance_message` — testing a single pipeline plugin (Adapt/Padatious/…) against raw bus messages | +| [intent-cases.md](intent-cases.md) | `IntentCase`, `register_intent_case_tests` — file-based intent test cases (`.intent.test`) with per-pipeline-family generated tests | | [pydantic-integration.md](pydantic-integration.md) | Using `ovos-pydantic-models` with OvoScope | | [audio-testing.md](audio-testing.md) | `AudioServiceHarness`, `PlaybackServiceHarness` — testing audio services | | [media-testing.md](media-testing.md) | `OCPPlayerHarness`, `OCPCaptureSession`, `MockOCPBackend` — testing the `ovos-media` OCP player (and driving a real OCP backend) | @@ -157,7 +159,7 @@ with MiniVoiceLoop(ww_instances={"hey_mycroft": ww}, ## What OvoScope Does NOT Do - Does not start a real WebSocket MessageBus server — uses `FakeBus` (in-process pub/sub). -- Does not load PHAL plugins or the audio service — only skills and the intent pipeline. +- `MiniCroft` / `End2EndTest` — the harness this page documents — does not load PHAL plugins or the audio service; it only loads skills and the intent pipeline. PHAL plugins and audio services have their own dedicated harnesses: [phal.md](phal.md) (`MiniPHAL`, `PHALTest`) and [audio-testing.md](audio-testing.md) (`AudioServiceHarness`, `PlaybackServiceHarness`). - Does not test GUI rendering — GUI namespace messages are ignored by default (`ignore_gui=True`). - Does not test TTS — operates at the `recognizer_loop:utterance` level (see [audio-testing.md](audio-testing.md) for TTS lifecycle testing). - `MiniListener` covers `AudioTransformersService`, the STT pipeline, and mock VAD/WakeWord engines. `MiniVoiceLoop` / `MiniSimpleListener` / `MiniClassicListener` drive the dinkum, simple, and classic listener **services** from an audio file and capture the `recognizer_loop:*` bus sequence; the classic file drive is best-effort (energy-based pipeline). diff --git a/docs/intent-cases.md b/docs/intent-cases.md new file mode 100644 index 0000000..6939b0d --- /dev/null +++ b/docs/intent-cases.md @@ -0,0 +1,138 @@ +# Intent Cases +`ovoscope.intent_cases` lets skill authors describe expected intent routing +as plain-text files instead of Python. Adding a phrase, an intent, or a +whole new language is a pure text edit — no test code required. + +```python +from ovoscope.intent_cases import register_intent_case_tests +``` + +## Layout + +``` +test/end2end/cases/ + / + .intent.test # one utterance per line, expected + # to match + no_match.test # utterances expected to match + # NO intent of this skill +``` + +`#` comments and blank lines are ignored in `.test` files. + +## Usage + +One call, in a test module owned by the skill: + +```python +# test/end2end/test_intents.py +from pathlib import Path +from ovoscope.intent_cases import register_intent_case_tests + +register_intent_case_tests( + globals(), + skill_id="ovos-skill-personal.openvoiceos", + handlers={ + "WhoAreYou.intent": "PersonalSkill.handle_who_are_you_intent", + "WhatAreYou.intent": "PersonalSkill.handle_what_are_you_intent", + }, + cases_dir=Path(__file__).parent / "cases", +) +``` + +The call creates one `unittest.TestCase` class per pipeline family in the +caller's module — `TestPadatious`, `TestPadacioso`, `TestM2V`, and +`TestDefaultPipeline` by default — each containing one `test_*` method per +`(lang, utterance)` pair found under `cases_dir`. A test passes if its +pipeline family routes the utterance to the expected intent, matching +realistic production cascade behaviour. Pass `pipelines={...}` to override +the generated set with a subset, or with custom pipeline stage lists. + +--- + +## `IntentCase` (`ovoscope/intent_cases.py`) +Frozen dataclass: a single expectation — `utterance` in `lang` should match +`intent`. + +| Field | Type | Description | +|---|---|---| +| `lang` | `str` | Language directory the case came from. | +| `utterance` | `str` | The utterance text. | +| `intent` | `Optional[str]` | Expected `".intent"`, or `None` to assert the utterance falls through to `complete_intent_failure`. | +| `source` | `Path` | The `.test` file the case was read from. | + +## `load_intent_cases(cases_dir, known_intents=None) -> List[IntentCase]` +Discover every `IntentCase` under `cases_dir`. Returns `[]` if `cases_dir` +does not exist. If `known_intents` is given, every `.intent` +filename found is validated against it — a typo raises `AssertionError` +instead of being silently skipped. + +## `assert_intent_case(minicroft, skill_id, handlers, case, pipeline, *, ignore_messages=None, timeout=30) -> None` +Fire `case.utterance` through `pipeline` on a running `minicroft` and assert +routing, using `End2EndTest` under the hood. + +- `case.intent is None` — asserts the full `source_message` → + `complete_intent_failure` → `ovos.utterance.handled` sequence. +- Otherwise — asserts `source_message` → `.activate` → + `:` → `mycroft.skill.handler.start` → + `mycroft.skill.handler.complete` → `ovos.utterance.handled`, using + `handlers[case.intent]` as the expected handler name. Raises + `AssertionError` up front if `case.intent` has no entry in `handlers`. + +`ignore_messages` defaults to `DEFAULT_IGNORE_MESSAGES` (`"speak"`, +`"mycroft.audio.play_sound"`, `"ovos.common_play.stop.response"`) — message +types that are noisy or non-deterministic and should not be asserted on. + +## `register_intent_case_tests(target_globals, *, skill_id, handlers, cases_dir, pipelines=None, ignore_messages=None, timeout=30, m2v_warmup=10.0) -> Dict[str, type]` +Create per-pipeline `TestCase` classes in `target_globals` (pass `globals()` +from the calling test module so pytest collects them). + +| Parameter | Default | Description | +|---|---|---| +| `target_globals` | required | `globals()` of the caller's test module. | +| `skill_id` | required | Full skill plugin id, e.g. `"my-skill.author"`. | +| `handlers` | required | `{".intent": ""}`, covering every intent referenced by case files. | +| `cases_dir` | required | Directory containing `/.intent.test` and optional `/no_match.test` files. | +| `pipelines` | `None` | `{class_suffix: pipeline_stage_list}` to override the default per-family classes (`DEFAULT_PIPELINE_FAMILIES`: Padatious, Padacioso, M2V, DefaultPipeline). | +| `ignore_messages` | `None` | Extra message types to filter out of comparison, added to `DEFAULT_IGNORE_MESSAGES`. | +| `timeout` | `30` | Per-case execution timeout, in seconds. | +| `m2v_warmup` | `10.0` | Seconds to wait (upper bound) after booting `MiniCroft` for the m2v pipeline to finish syncing its label index. Set to `0` if not running M2V cases. | + +Returns `{}` with no classes created if `cases_dir` has no case files — +this lets a freshly-copied template pass collection before any `.test` +files are added. + +All generated test classes share one `MiniCroft` instance per +`(skill_id, langs)` key, booted lazily on first use and cached at module +scope. It is registered with `atexit` (`stop_shared_minicrofts()`) so it +does not leak process-wide `SessionManager`/`Configuration` patches past +the test run. At most one shared instance is kept alive at a time — +requesting a different `(skill_id, langs)` key stops the cached instance +first, since two live `MiniCroft`s fight over the same globals. + +## `autodiscover_from_conftest(conftest_dir, target_globals) -> Dict[str, type]` +Zero-boilerplate alternative to calling `register_intent_case_tests` +directly: looks for an `ovoscope_intent_cases` dict in the conftest +namespace and calls `register_intent_case_tests` with it. A skill opts in +by adding a `conftest.py` next to its `cases/` directory: + +```python +ovoscope_intent_cases = dict( + skill_id="my-skill.author", + handlers={"DoX.intent": "MySkill.handle_do_x"}, + # optional: cases_dir, pipelines, ignore_messages, timeout, m2v_warmup +) +``` + +The ovoscope pytest plugin's `pytest_collect_directory` hook discovers this +conftest, walks `/cases/`, and generates the same `TestCase` classes +`register_intent_case_tests` would have created. Returns `{}` if the +conftest has no `ovoscope_intent_cases` or the cases directory does not +exist. + +--- + +## Cross-References +- [end2end-test.md](end2end-test.md) — `End2EndTest`, used internally by `assert_intent_case`. +- [minicroft.md](minicroft.md) — `MiniCroft` / `get_minicroft()`, the runtime the shared instance wraps. +- [e2e-pipeline-harness.md](e2e-pipeline-harness.md) — a lower-level harness for testing a single pipeline plugin directly against raw bus messages, rather than via `.test` case files. diff --git a/docs/listener.md b/docs/listener.md index c19749b..96d6cef 100644 --- a/docs/listener.md +++ b/docs/listener.md @@ -36,7 +36,7 @@ WAV file / bytes Rather than injecting a `recognizer_loop:utterance` (as `MiniCroft` does), `MiniListener` feeds **raw audio bytes** into `AudioTransformersService` — -`ovos_dinkum_listener/transformers.py:34` — which dispatches them to each +`ovos_dinkum_listener/transformers.py` — which dispatches them to each loaded plugin's `feed_audio_chunk()` / `feed_speech_chunk()` / `transform()` methods. All `Message` objects emitted on the internal `FakeBus` during that call are captured and returned. @@ -113,7 +113,7 @@ listener.shutdown() ## API Reference -### `MiniListener` — `ovoscope/listener.py:261` +### `MiniListener` — `ovoscope/listener.py` **Constructor parameters:** @@ -122,40 +122,40 @@ listener.shutdown() | `config` | `dict` | Full OVOS config with `listener.audio_transformers` key | | `plugin_instances` | `dict[str, Any]` | Pre-instantiated transformer plugins; bypasses OPM discovery | | `stt_instance` | `Any` | Optional STT plugin to use in `listen()` | -| `vad_instance` | `Any` | Optional VAD engine (e.g. `MockVADEngine`) — `ovoscope/listener.py:314` | -| `ww_instances` | `dict[str, Any]` | Optional wake-word engines keyed by name — `ovoscope/listener.py:316` | +| `vad_instance` | `Any` | Optional VAD engine (e.g. `MockVADEngine`) — `ovoscope/listener.py` | +| `ww_instances` | `dict[str, Any]` | Optional wake-word engines keyed by name — `ovoscope/listener.py` | **Audio transformer methods:** | Method | Signature | Description | |--------|-----------|-------------| -| `feed_audio(chunk)` — `ovoscope/listener.py:351` | `(bytes) → List[Message]` | Calls `AudioTransformersService.feed_audio()`. Requires `ovos-dinkum-listener`. | -| `feed_speech(chunk)` — `ovoscope/listener.py:371` | `(bytes) → List[Message]` | Calls `AudioTransformersService.feed_speech()`. Requires `ovos-dinkum-listener`. | +| `feed_audio(chunk)` — `ovoscope/listener.py` | `(bytes) → List[Message]` | Calls `AudioTransformersService.feed_audio()`. Requires `ovos-dinkum-listener`. | +| `feed_speech(chunk)` — `ovoscope/listener.py` | `(bytes) → List[Message]` | Calls `AudioTransformersService.feed_speech()`. Requires `ovos-dinkum-listener`. | | `feed_audio_stream(chunks, feed, chunk_size)` | `(bytes\|list[bytes], str, int) → List[Message]` | Streams frames in order **without** clearing between them; aggregates all emitted messages. Use for decoders that fire after many frames (ggwave). | -| `transform(chunk)` — `ovoscope/listener.py:390` | `(bytes) → tuple[bytes, dict, List[Message]]` | Full transform pipeline; returns `(audio, ctx, messages)`. Requires `ovos-dinkum-listener`. | -| `listen(audio, ...)` — `ovoscope/listener.py:410` | `(audio, language, stt_instance, ...) → List[Message]` | Full pipeline: audio → transformers → STT → utterance message. Requires `ovos-dinkum-listener`. | +| `transform(chunk)` — `ovoscope/listener.py` | `(bytes) → tuple[bytes, dict, List[Message]]` | Full transform pipeline; returns `(audio, ctx, messages)`. Requires `ovos-dinkum-listener`. | +| `listen(audio, ...)` — `ovoscope/listener.py` | `(audio, language, stt_instance, ...) → List[Message]` | Full pipeline: audio → transformers → STT → utterance message. Requires `ovos-dinkum-listener`. | **VAD methods:** | Method | Signature | Description | |--------|-----------|-------------| -| `is_silence(chunk)` — `ovoscope/listener.py:461` | `(bytes) → bool` | Delegates to the injected VAD engine. Raises `RuntimeError` if no VAD engine set. | -| `extract_speech(audio)` — `ovoscope/listener.py:483` | `(bytes) → bytes` | Returns only speech frames from `audio`. Raises `RuntimeError` if no VAD engine set. | +| `is_silence(chunk)` — `ovoscope/listener.py` | `(bytes) → bool` | Delegates to the injected VAD engine. Raises `RuntimeError` if no VAD engine set. | +| `extract_speech(audio)` — `ovoscope/listener.py` | `(bytes) → bytes` | Returns only speech frames from `audio`. Raises `RuntimeError` if no VAD engine set. | **Wake-word methods:** | Method | Signature | Description | |--------|-----------|-------------| -| `detect_wakeword(chunk, ww_name=None)` — `ovoscope/listener.py:509` | `(bytes, str?) → bool` | Feed `chunk` to the named engine (or first engine if `ww_name=None`). Returns `True` if the engine fires. | -| `scan_for_wakeword(audio, frame_size=2048, ww_name=None)` — `ovoscope/listener.py:551` | `(bytes\|List[bytes], int, str?) → (bool, int?)` | Feed each frame sequentially; return `(True, frame_index)` on first detection, or `(False, None)` if threshold never reached. | +| `detect_wakeword(chunk, ww_name=None)` — `ovoscope/listener.py` | `(bytes, str?) → bool` | Feed `chunk` to the named engine (or first engine if `ww_name=None`). Returns `True` if the engine fires. | +| `scan_for_wakeword(audio, frame_size=2048, ww_name=None)` — `ovoscope/listener.py` | `(bytes\|List[bytes], int, str?) → (bool, int?)` | Feed each frame sequentially; return `(True, frame_index)` on first detection, or `(False, None)` if threshold never reached. | **Lifecycle:** | Method | Description | |--------|-------------| -| `shutdown()` — `ovoscope/listener.py:606` | Gracefully shuts down transformer plugins and all wake-word engines. | +| `shutdown()` — `ovoscope/listener.py` | Gracefully shuts down transformer plugins and all wake-word engines. | -#### `listen()` — `ovoscope/listener.py:410` +#### `listen()` — `ovoscope/listener.py` ``` listen( @@ -171,12 +171,12 @@ Runs the complete listener pipeline: 1. Reads WAV file (or accepts raw bytes) 2. Passes bytes through `AudioTransformersService.transform()` — all loaded transformer plugins run -3. Converts the (possibly modified) bytes to `AudioData` via `_wav_to_audio_data()` — `listener.py:59` +3. Converts the (possibly modified) bytes to `AudioData` via `_wav_to_audio_data()` — `listener.py` 4. Calls `stt_instance.execute(audio_data, language)` if provided 5. Emits `recognizer_loop:utterance` on the FakeBus if the transcript is non-empty 6. Returns all captured messages (from transformers **and** the utterance step) -`_wav_to_audio_data(audio, sample_rate, sample_width)` — `listener.py:59`: +`_wav_to_audio_data(audio, sample_rate, sample_width)` — `listener.py`: - File path → `AudioData.from_file(path)` (handles WAV/AIFF/FLAC headers) - Raw bytes → parses WAV header via `wave` stdlib; falls back to raw PCM if not a valid WAV @@ -188,7 +188,7 @@ Runs the complete listener pipeline: | `config` | `dict` | Full OVOS config with `listener.audio_transformers` key | | `plugin_instances` | `dict[str, Any]` | Pre-instantiated plugins; bypasses OPM discovery | -### `get_mini_listener()` — `ovoscope/listener.py:629` +### `get_mini_listener()` — `ovoscope/listener.py` Factory function. Two usage modes: @@ -226,7 +226,7 @@ listener = get_mini_listener( | `ww_plugin` | `str` | OPM WakeWord plugin name to load via `OVOSWakeWordFactory` | | `ww_instances` | `dict[str, Any]` | Pre-built WakeWord engines keyed by phrase name | -### `ListenerTest` — `ovoscope/listener.py:181` +### `ListenerTest` — `ovoscope/listener.py` Declarative test runner, analogous to `End2EndTest`. @@ -245,7 +245,7 @@ captured message list on success. ## Plugin Injection vs OPM Discovery -`AudioTransformersService.load_plugins()` — `transformers.py:46` — uses +`AudioTransformersService.load_plugins()` — `transformers.py` — uses `find_audio_transformer_plugins()` from `ovos-plugin-manager` to discover plugins by entry point. If a plugin is registered under a legacy group (e.g. `neon.plugin.audio` instead of `opm.plugin.audio_transformer`), or is not @@ -260,7 +260,7 @@ of how the plugin was loaded. `MiniListener` supports **in-process VAD and WakeWord testing** without loading real models or hardware. -### `MockVADEngine` — `ovoscope/listener.py:117` +### `MockVADEngine` — `ovoscope/listener.py` A zero-dependency VAD stub: @@ -280,7 +280,7 @@ print(listener.extract_speech(b"\x00" * 512 + b"\x01" * 512)) # → b"\x01" * 5 listener.shutdown() ``` -### `MockHotWordEngine` — `ovoscope/listener.py:188` +### `MockHotWordEngine` — `ovoscope/listener.py` A controllable WakeWord stub: @@ -303,7 +303,7 @@ assert found and frame == 2 listener.shutdown() ``` -### `VADTest` — `ovoscope/listener.py:817` +### `VADTest` — `ovoscope/listener.py` Declarative VAD test helper: @@ -326,7 +326,7 @@ VADTest( ).execute() ``` -### `WakeWordTest` — `ovoscope/listener.py:901` +### `WakeWordTest` — `ovoscope/listener.py` Declarative WakeWord test helper: @@ -357,8 +357,8 @@ WakeWordTest( ## Cross-References -- `AudioTransformersService` — `ovos-dinkum-listener/ovos_dinkum_listener/transformers.py:34` -- `AudioData` — `ovos-plugin-manager/ovos_plugin_manager/utils/audio.py:34` +- `AudioTransformersService` — `ovos-dinkum-listener/ovos_dinkum_listener/transformers.py` +- `AudioData` — `ovos-plugin-manager/ovos_plugin_manager/utils/audio.py` - `MiniCroft` / `get_minicroft()` — `ovoscope/docs/minicroft.md` (skill pipeline equivalent) - Audio transformer E2E test: `Transformer plugins/ovos-audio-transformer-plugin-ggwave/test/end2end/test_ggwave_transformer.py` - STT pipeline E2E test: `STT plugins/ovos-stt-plugin-rover/test/end2end/test_rover_listener_e2e.py` diff --git a/docs/minicroft.md b/docs/minicroft.md index f001e8d..9cc05da 100644 --- a/docs/minicroft.md +++ b/docs/minicroft.md @@ -1,11 +1,11 @@ # MiniCroft `MiniCroft` is a minimal, in-process OVOS Core that loads real skill plugins and runs the full intent pipeline on a `FakeBus`. It is the execution engine behind every OvoScope test. -## Class: `MiniCroft` — `ovoscope/__init__.py:158` +## Class: `MiniCroft` (`ovoscope/__init__.py`) ```python from ovoscope import MiniCroft ``` Subclass of `ovos_core.skill_manager.SkillManager`. -`get_minicroft` factory — `ovoscope/__init__.py:456` Replaces the real WebSocket bus with `FakeBus`, disables components not needed for testing, and only loads the skills you specify. +`get_minicroft()` factory (`ovoscope/__init__.py`) replaces the real WebSocket bus with `FakeBus`, disables components not needed for testing, and only loads the skills you specify. ### Constructor ```python MiniCroft( @@ -21,6 +21,8 @@ MiniCroft( lang: str | None = None, secondary_langs: list[str] | None = None, pipeline_config: dict[str, dict] | None = None, + modernize: bool = True, + emit_legacy: bool = True, *args, **kwargs, ) ``` @@ -38,6 +40,8 @@ MiniCroft( | `lang` | `None` | Override the system default language (`Configuration()["lang"]`). Patched before Adapt/Padatious init so vocab is registered for this language. | | `secondary_langs` | `None` | Set `Configuration()["secondary_langs"]`. Adapt and Padatious create per-language engines for each language in this list, enabling multilingual intent matching. | | `pipeline_config` | `None` | Per-pipeline plugin config overrides. A `dict` keyed by the plugin's config key under `Configuration()["intents"]` (e.g. `"ovos_m2v_pipeline"`). Patched before `super().__init__()` so pipeline plugins read overridden values during their `__init__`. Restored in `stop()`. | +| `modernize` | `True` | Forwarded to the harness `FakeBus`. When `True`, emitting a legacy bus topic also emits its `ovos.*` spec counterpart (legacy producer → spec listener). | +| `emit_legacy` | `True` | Forwarded to the harness `FakeBus`. When `True`, emitting an `ovos.*` spec topic also emits the matching legacy topic (spec producer → legacy listener). Set both `modernize` and `emit_legacy` to `False` to isolate a single namespace and assert no cross-namespace bridging occurs. | ### Key attributes | Attribute | Type | Description | |---|---|---| diff --git a/docs/ocp.md b/docs/ocp.md index 34a7788..dffbaf5 100644 --- a/docs/ocp.md +++ b/docs/ocp.md @@ -45,12 +45,12 @@ result = OCPTest( | `timeout` | `float` | `20.0` | Max wait in seconds. | | `patch_targets` | `List[str]` | `[]` | Additional `requests`-like module paths to patch (dotted Python path to the callable to replace). | -### `execute()` — `ovoscope/ocp.py:90` +### `execute()` — `ovoscope/ocp.py` Returns `List[Message]` — all bus messages captured during the interaction (same format as `CaptureSession.responses`). -## HTTP Mocking — `ovoscope/ocp.py:139` +## HTTP Mocking — `ovoscope/ocp.py` HTTP calls are intercepted via `unittest.mock.patch` on `requests.Session.get` and `requests.get` by default. diff --git a/docs/phal.md b/docs/phal.md index 564d3c6..83e346c 100644 --- a/docs/phal.md +++ b/docs/phal.md @@ -30,7 +30,7 @@ testing and should use hardware-in-the-loop integration tests instead: ## `MiniPHAL` — Context Manager -`MiniPHAL` — `ovoscope/phal.py:43` +`MiniPHAL` — `ovoscope/phal.py` ```python from ovos_utils.messagebus import Message @@ -54,14 +54,14 @@ with MiniPHAL( ### Methods -`MiniPHAL.emit` — `ovoscope/phal.py:146` +`MiniPHAL.emit` — `ovoscope/phal.py` | Method | Description | |--------|-------------| | `emit(msg, wait=0.05)` | Emit `msg` on the internal bus then sleep `wait` seconds so async handlers have time to fire before the next assertion. Set `wait=0` to disable the sleep. | -| `assert_emitted(msg_type, timeout=2.0)` | Poll captured messages up to `timeout` seconds; return the first matching `Message`. Raises `AssertionError` on timeout. — `ovoscope/phal.py:157` | -| `assert_not_emitted(msg_type, wait=0.2)` | Sleep `wait` seconds then assert no captured message has `msg_type`. Raises `AssertionError` if one was captured. — `ovoscope/phal.py:184` | -| `clear_captured()` | Clear the captured message list. Useful between sequential assertions in the same `with` block. — `ovoscope/phal.py:203` | +| `assert_emitted(msg_type, timeout=2.0)` | Poll captured messages up to `timeout` seconds; return the first matching `Message`. Raises `AssertionError` on timeout. — `ovoscope/phal.py` | +| `assert_not_emitted(msg_type, wait=0.2)` | Sleep `wait` seconds then assert no captured message has `msg_type`. Raises `AssertionError` if one was captured. — `ovoscope/phal.py` | +| `clear_captured()` | Clear the captured message list. Useful between sequential assertions in the same `with` block. — `ovoscope/phal.py` | #### `emit(wait=...)` — settling delay diff --git a/docs/pipeline.md b/docs/pipeline.md index 943a957..30e3d4f 100644 --- a/docs/pipeline.md +++ b/docs/pipeline.md @@ -11,7 +11,7 @@ has no skills, so only the pipeline matching logic is exercised. ## `_SinkSkill` — Internal Catch-all -`_SinkSkill` — `ovoscope/pipeline.py:37` +`_SinkSkill` — `ovoscope/pipeline.py` When `PipelineHarness` creates a `MiniCroft`, it injects an internal `__ovoscope_sink__` skill as a routing target for matched intents. This is @@ -23,7 +23,7 @@ Users never interact with `_SinkSkill` directly. ## `PipelineHarness` — Context Manager -`PipelineHarness` — `ovoscope/pipeline.py:71` +`PipelineHarness` — `ovoscope/pipeline.py` ```python from ovoscope.pipeline import PipelineHarness @@ -48,9 +48,9 @@ with PipelineHarness( | Method | Source | Returns | Description | |--------|--------|---------|-------------| -| `match(utterance, timeout=5.0)` | `ovoscope/pipeline.py:135` | `Optional[Message]` | Send utterance; return matched `Message` or `None` on timeout/failure. | -| `assert_matches(utterance, intent_type=None, timeout=5.0)` | `ovoscope/pipeline.py:183` | `Message` | Assert at least one stage matches. Raises `AssertionError` if no match. `intent_type` is a substring check on `msg_type`. | -| `assert_no_match(utterance, timeout=2.0)` | `ovoscope/pipeline.py:213` | `None` | Assert no stage matches. Raises `AssertionError` if a match is found. | +| `match(utterance, timeout=5.0)` | `ovoscope/pipeline.py` | `Optional[Message]` | Send utterance; return matched `Message` or `None` on timeout/failure. | +| `assert_matches(utterance, intent_type=None, timeout=5.0)` | `ovoscope/pipeline.py` | `Message` | Assert at least one stage matches. Raises `AssertionError` if no match. `intent_type` is a substring check on `msg_type`. | +| `assert_no_match(utterance, timeout=2.0)` | `ovoscope/pipeline.py` | `None` | Assert no stage matches. Raises `AssertionError` if a match is found. | ### Pipeline Stage Ordering and Success vs Failure @@ -63,7 +63,7 @@ when a stage commits to handling the utterance. **Failure signal**: `intent_failure` or `mycroft.skill.handler.start` bus messages — emitted when no stage matched after all stages have been consulted. -`match()` — `ovoscope/pipeline.py:135` — uses separate `threading.Event` +`match()` — `ovoscope/pipeline.py` — uses separate `threading.Event` objects for success and failure so that an `intent_failure` arriving first does not mask a subsequent late success match. On timeout or failure the method returns `None`; on success it returns the captured `Message`. @@ -111,7 +111,7 @@ with PipelineHarness( ### `assert_matches(intent_type=...)` semantics `intent_type` is a **substring** check on the matched message's `msg_type` -— `ovoscope/pipeline.py:208`: +— `ovoscope/pipeline.py`: ```python # Pass: msg_type "padatious:0.95:LightsOnIntent" contains "LightsOnIntent" @@ -127,10 +127,10 @@ msg = harness.assert_matches("turn on the lights", intent_type="LightsOffIntent" ## Implementation Notes -`PipelineHarness.__enter__` — `ovoscope/pipeline.py:104` — creates a +`PipelineHarness.__enter__` — `ovoscope/pipeline.py` — creates a `MiniCroft` with `skill_ids=[]` and the specified pipeline. -`PipelineHarness.match()` — `ovoscope/pipeline.py:135` — subscribes to +`PipelineHarness.match()` — `ovoscope/pipeline.py` — subscribes to `intent.service.skills.activated` (success) and `intent_failure` / `mycroft.skill.handler.start` (failure) before emitting the utterance, then waits on a `threading.Event` with the given timeout. Bus handlers are diff --git a/ovoscope/__init__.py b/ovoscope/__init__.py index 62e8828..c6ce24a 100644 --- a/ovoscope/__init__.py +++ b/ovoscope/__init__.py @@ -1,4 +1,6 @@ +import inspect import dataclasses +import gc import json import threading from copy import deepcopy @@ -16,6 +18,8 @@ from ovos_utils.log import LOG from ovos_utils.process_utils import ProcessState from ovos_spec_tools import SpecMessage +from ovos_spec_tools.messages import MIGRATION_MAP, SPEC_TO_LEGACY +from ovos_workshop.skills.api import SkillApi from ovos_workshop.skills.ovos import OVOSSkill SerializedMessage = Dict[str, Union[str, Dict[str, Any]]] @@ -304,6 +308,7 @@ def __init__(self, skill_ids, pipeline_config: Optional[Dict[str, Dict]] = None, modernize: bool = True, emit_legacy: bool = True, + get_response_timeout: float = 2.0, *args, **kwargs): # Namespace-migration flags forwarded to the harness FakeBus so callers # can choose which bus namespace(s) to exercise: @@ -318,6 +323,97 @@ def __init__(self, skill_ids, self._isolated_config = isolate_config self._original_xdg_configs: Optional[List[LocalConf]] = None + # SessionManager.bus is a process-wide class attribute. IntentService + # (constructed below via super().__init__()) calls + # SessionManager.connect_to_bus(self.bus) in its own __init__, + # clobbering it with this instance's FakeBus. If left in place after + # stop(), SessionManager.wait_while_speaking()'s `if not cls.bus` + # guard sees a stale, truthy, dead bus and blocks/registers listeners + # on it instead of whatever bus a later test expects — polluting + # every subsequent test in the process. Snapshot it before booting so + # stop() can restore it. + self._original_sm_bus = SessionManager.bus + + # SkillApi.bus is another process-wide class attribute. SkillManager + # calls SkillApi.connect_bus(self.bus) during boot, which pins this + # instance's FakeBus — and, through the bus handlers, the whole + # MiniCroft object graph (~633MB per stopped instance measured on the + # ovoscope suite). Snapshot it so stop() can put it back and let the + # instance be collected. + self._original_skill_api_bus = SkillApi.bus + + # SessionManager.default_session is a process-wide singleton. Booting a + # MiniCroft (and running a test through it) mutates it in several ways: + # run() overrides pipeline/lang, End2EndTest.execute() calls + # activate_skill() for `inject_active`, and any message that carries a + # session with id "default" folds its wire values onto the live object. + # Snapshot the whole thing so stop() can put it back exactly as found — + # otherwise every later test in the process inherits the mutation. + self._default_session_obj = SessionManager.default_session + try: + # ovos-bus-client 2.x names these to_dict/from_dict; 1.x uses + # serialize/deserialize. Support both — a silent None here would + # quietly disable the whole restore. + sess_obj = self._default_session_obj + dump = getattr(sess_obj, "to_dict", None) + if dump is not None: + # ovos-bus-client 2.x + self._session_api = "dict" + else: + # ovos-bus-client 1.x + dump = sess_obj.serialize + self._session_api = "legacy" + self._default_session_state = deepcopy(dump()) + except Exception: # pragma: no cover - defensive, session_cls may vary + LOG.warning("ovoscope: could not snapshot the default session; " + "state mutated by tests will NOT be restored") + self._default_session_state = None + self._session_api = None + # active_skills as found at boot. Restored even when the full snapshot + # failed, so a skill activated during the test never survives teardown. + try: + self._default_active_skills = deepcopy( + self._default_session_obj.active_skills) + except Exception: # pragma: no cover - defensive + self._default_active_skills = None + + # Orphaned TTS timers (see _mock_tts below) would fire on a closed bus + # after stop() and corrupt the global SessionManager during a LATER + # test. Track them so stop() can cancel them. + self._tts_timers: List[threading.Timer] = [] + self._tts_timers_lock = threading.Lock() + + # get_response()/ask_yesno() spawn a killable background thread + # (OVOSSkill._real_wait_response) and the calling thread busy-polls + # for its result. With `num_retries=-1` (the OVOSSkill default) that + # thread re-prompts and waits forever if nothing ever answers it — + # there is no ceiling on the calling thread's wait either. On a real + # voice satellite this eventually gets a `mycroft.skills.abort_question` + # from the listener/GUI on user silence or session teardown; the + # synchronous FakeBus never generates one. Track the pending + # "wait for an answer" timers here so stop() can cancel them the same + # way it cancels the mock-TTS ones. + # Keyed by (skill_id, session_id) — the SAME two-part scope upstream's + # own @killable_event("mycroft.skills.abort_question", + # check_skill_id=True) uses to decide which stalled thread an abort + # is actually for (session_id match + optional skill_id match). A + # flat list here would let one skill's `.disable` cancel a DIFFERENT + # skill's (or a different concurrent session's) still-pending + # watchdog, leaving it to hang forever again in a multi-skill + # MiniCroft where two get_response() calls are in flight at once. + self._get_response_timeout = get_response_timeout + self._get_response_timers: Dict[tuple, threading.Timer] = {} + self._get_response_timers_lock = threading.Lock() + # Guards the `_stopped` flag against the mock-TTS emits. A plain + # `if not self._stopped: bus.emit(...)` is a TOCTOU: stop() can flip the + # flag and close the bus between the check and the emit, so the emit + # lands on a dead bus and folds a stale session onto the global + # SessionManager. Holding this lock across BOTH the check+emit and the + # flag flip makes the two mutually exclusive. Re-entrant because an emit + # can re-enter the mock-TTS handler on the same thread. + self._stop_lock = threading.RLock() + self._stopped = False + if default_pipeline is DEFAULT_PIPELINE_UNSET: if is_pipeline_available(DEFAULT_TEST_PIPELINE): self._default_pipeline = DEFAULT_TEST_PIPELINE @@ -399,27 +495,130 @@ def __init__(self, skill_ids, # emit audio_output_start synchronously (duck) and schedule a short-delay # audio_output_end (unduck) to simulate the full TTS playback lifecycle. def _mock_tts(message): - # TTS playback begins — duck immediately. - # message.forward copies source/destination/session from the speak, - # matching what the real audio service would do. - bus.emit(message.forward("recognizer_loop:audio_output_start")) - # TTS playback ends after a short delay — unduck - threading.Timer(0.1, lambda: bus.emit( - message.forward("recognizer_loop:audio_output_end") - )).start() + with self._stop_lock: + if self._stopped: + return + # TTS playback begins — duck immediately. + # message.forward copies source/destination/session from the + # speak, matching what the real audio service would do. + bus.emit(message.forward("recognizer_loop:audio_output_start")) + + def _unduck(): + # stop() may have run while the timer was pending — emitting on + # a closed bus here would fold a stale session onto the global + # SessionManager and poison the next test. The lock makes the + # check and the emit atomic against stop(). + with self._stop_lock: + if self._stopped: + return + bus.emit(message.forward("recognizer_loop:audio_output_end")) + + # TTS playback ends after a short delay — unduck. + # Daemon + tracked so stop() can cancel it and the interpreter can + # exit even if one is still pending. + timer = threading.Timer(0.1, _unduck) + timer.daemon = True + with self._tts_timers_lock: + self._tts_timers = [t for t in self._tts_timers if t.is_alive()] + self._tts_timers.append(timer) + timer.start() bus.on(SpecMessage.SPEAK, _mock_tts) + # get_response()/ask_yesno() mock: OVOSSkill.get_response() emits + # "skill.converse.get_response.enable" and then blocks the calling + # thread until a ".converse.get_response" answer arrives or + # the killable thread is aborted via "mycroft.skills.abort_question". + # If a test doesn't inject a follow-up utterance, nothing ever answers + # it and (with the OVOSSkill default `num_retries=-1`) the skill + # re-prompts and waits forever. Arm a short watchdog on `.enable` + # that fires the SAME "mycroft.skills.abort_question" a real listener + # would send on user silence — this is existing bus-protocol, not a + # new message type. "`.disable" (emitted once get_response() actually + # returns, whether answered or cancelled) cancels the watchdog. + def _arm_get_response_watchdog(message): + with self._stop_lock: + if self._stopped: + return + skill_id = message.data.get("skill_id") + session_id = SessionManager.get(message).session_id + key = (skill_id, session_id) + + def _abort(): + with self._stop_lock: + if self._stopped: + return + with self._get_response_timers_lock: + # Already disarmed (answered/cancelled) between + # the Timer firing and this lock — nothing to do. + if self._get_response_timers.get(key) is not timer: + return + del self._get_response_timers[key] + bus.emit(message.forward("mycroft.skills.abort_question", + {"skill_id": skill_id})) + + timer = threading.Timer(self._get_response_timeout, _abort) + timer.daemon = True + with self._get_response_timers_lock: + old = self._get_response_timers.get(key) + if old is not None and old.is_alive(): + old.cancel() + self._get_response_timers[key] = timer + timer.start() + + def _disarm_get_response_watchdog(message): + # This ONE get_response() call returned (answered, cancelled, + # retries exhausted, or already aborted by our own watchdog) — + # cancel only ITS entry. Other (skill_id, session_id) pairs with + # their own in-flight get_response() must keep their watchdog + # armed (this is exactly what the flat-list version got wrong: + # any skill's `.disable` cancelled every pending watchdog). + skill_id = message.data.get("skill_id") + session_id = SessionManager.get(message).session_id + key = (skill_id, session_id) + with self._get_response_timers_lock: + timer = self._get_response_timers.pop(key, None) + if timer is not None: + try: + timer.cancel() + except Exception: + pass + + bus.on("skill.converse.get_response.enable", _arm_get_response_watchdog) + bus.on("skill.converse.get_response.disable", _disarm_get_response_watchdog) + self.skill_ids = skill_ids self.extra_skills = extra_skills or {} + # Older ovos-core SkillManager releases (e.g. the stable release + # channel's 1.3.x) predate some of these keyword arguments. Passing an + # unknown kwarg to them raises TypeError and MiniCroft cannot boot, + # which makes the latest ovoscope unusable against an older core (the + # conformance harness exercises exactly this against pinned stable / + # testing stacks). Forward only the enable_* flags the *installed* + # SkillManager actually accepts, so one ovoscope boots on every core. + _enable_flags = { + "enable_installer": enable_installer, + "enable_skill_api": enable_skill_api, + "enable_file_watcher": enable_file_watcher, + "enable_intent_service": enable_intent_service, + "enable_event_scheduler": enable_event_scheduler, + } + try: + _accepted = inspect.signature(SkillManager.__init__).parameters + except (ValueError, TypeError): + _accepted = {} + _has_var_kw = any(p.kind == inspect.Parameter.VAR_KEYWORD + for p in _accepted.values()) + _supported = {k: v for k, v in _enable_flags.items() + if _has_var_kw or k in _accepted} + _dropped = [k for k in _enable_flags if k not in _supported] + if _dropped: + LOG.debug(f"installed SkillManager does not accept {_dropped}; " + f"omitting for backwards compatibility") + try: - super().__init__(bus, enable_installer=enable_installer, - enable_skill_api=enable_skill_api, - enable_file_watcher=enable_file_watcher, - enable_intent_service=enable_intent_service, - enable_event_scheduler=enable_event_scheduler, - *args, **kwargs) + super().__init__(bus, *args, **_supported, **kwargs) except Exception: # If super().__init__ fails (e.g. plugin construction error), # ensure global Configuration() is restored. @@ -557,11 +756,61 @@ def inject_message(self, msg: Message) -> None: self.bus.emit(msg) def stop(self): + # Flip the flag and take the pending timers under `_stop_lock`, so a + # mock-TTS emit that is already past its `_stopped` check finishes on a + # live bus before teardown starts, and none can start afterwards. + with self._stop_lock: + self._stopped = True + # Cancel any pending mock-TTS unduck timers BEFORE closing the bus, + # so none of them can emit onto a dead bus (and fold a stale + # "default" session onto the process-wide SessionManager). + with self._tts_timers_lock: + timers, self._tts_timers = self._tts_timers, [] + with self._get_response_timers_lock: + gr_timers = list(self._get_response_timers.values()) + self._get_response_timers = {} + timers = timers + gr_timers + for t in timers: + try: + t.cancel() + except Exception: + pass + for t in timers: + try: + t.join(timeout=1.0) + except Exception: + pass try: super().stop() except Exception: pass if hasattr(self, "bus") and self.bus: + try: + # pyee's EventEmitter.remove_all_listeners() holds its internal, + # non-reentrant lock while dropping self._events. The "defence + # in depth" block near the end of this method (below) calls + # exactly that, on this same bus's emitter, to release + # listener references. If dropping the last reference to a + # bound-method listener there runs that listener owner's + # __del__, and that __del__ calls bus.remove()/ + # remove_listener(), the __del__ runs synchronously inside + # the locked block and deadlocks trying to re-acquire the + # same lock (30-minute CI hangs). Drain the listener dict + # ourselves here, outside the lock, and force any pending + # __del__ to run now instead, before that later call ever + # takes the lock -- by the time it runs, _events is already + # empty, so it is a safe no-op. Same workaround as the + # minicroft fixture in ovos-skill-application-launcher's + # test/end2end/test_intents_en_us.py (PR #108). + ee = getattr(self.bus, "ee", None) + if ee is not None: + events = getattr(ee, "_events", None) + if events is not None: + for key in list(events.keys()): + events.pop(key, None) + gc.collect() + except Exception: + pass try: self.bus.close() except Exception: @@ -616,6 +865,92 @@ def stop(self): Configuration.xdg_configs = self._original_xdg_configs Configuration.reload() LOG.debug("ovoscope: user config restored") + SessionManager.bus = self._original_sm_bus + LOG.debug("ovoscope: SessionManager.bus restored") + SkillApi.bus = self._original_skill_api_bus + LOG.debug("ovoscope: SkillApi.bus restored") + # Defence in depth: drop every handler still registered on this + # instance's bus. Handlers are bound methods of the skills and of this + # MiniCroft, so anything that still holds the bus would otherwise keep + # the whole object graph alive. + bus = getattr(self, "bus", None) + if bus is not None: + ee = getattr(bus, "ee", None) + if ee is not None: + try: + ee.remove_all_listeners() + except Exception: + pass + for attr in ("_handler_guards", "_dedup_registrations"): + container = getattr(bus, attr, None) + if container is not None: + try: + container.clear() + except Exception: + pass + self._restore_default_session() + + def _restore_default_session(self): + """Put the process-wide default Session back as it was before boot. + + The explicit pipeline / lang restores above only cover what run() + changed. Tests also mutate the default session through + ``activate_skill`` (``End2EndTest.inject_active``) and through any + message carrying a ``"default"`` session, which folds its wire values + onto the singleton. Restoring the full snapshot keeps that mutation + inside the test that caused it. + """ + state = getattr(self, "_default_session_state", None) + if state is None: + # The snapshot failed. Do not degrade to a total no-op: skills + # activated during the run are the mutation that leaks hardest, + # so put active_skills back explicitly. + sess = SessionManager.default_session + active = getattr(self, "_default_active_skills", None) + if sess is not None and active is not None: + try: + sess.active_skills = deepcopy(active) + LOG.debug("ovoscope: default session active_skills restored " + "(snapshot unavailable)") + except Exception: + LOG.warning("ovoscope: could not restore active_skills") + return + # Restore onto whatever object is the default session NOW: boot can + # replace the singleton (SessionManager.reset_default_session), and + # mutations after the swap land on the new object — bailing out on an + # identity mismatch would leak exactly the state this exists to scrub. + sess = SessionManager.default_session + if sess is None: + return + # Rebuild a pristine Session from the snapshot and copy every field + # onto the live object. Copying only the snapshot keys is not enough: + # to_dict() OMITS empty fields, so a skill activated during the test + # would have no key to restore and would survive teardown. + # Load with the SAME API family that produced the snapshot. to_dict() + # and serialize() do not share a wire format on every version, so + # pairing to_dict() output with deserialize() (or the reverse) silently + # rebuilds a wrong session. + try: + if self._session_api == "dict": + load = type(sess).from_dict + else: + load = type(sess).deserialize + fresh = load(deepcopy(state)) + except Exception: + LOG.warning("ovoscope: could not rebuild the default session from " + "its snapshot; leaked state will NOT be restored") + return + # Copy every instance attribute, including underscore-prefixed ones: + # some ovos-bus-client versions back public fields (active_skills, + # utterance_states, ...) with private storage, and skipping those + # would leave the mutation in place. + for key, value in vars(fresh).items(): + try: + setattr(sess, key, value) + except Exception: + # read-only / computed field — skip it + continue + LOG.debug("ovoscope: default session state restored") def get_minicroft(skill_ids: Union[List[str], str], *args, @@ -666,6 +1001,19 @@ class CaptureSession: done: threading.Event = dataclasses.field(default_factory=lambda: threading.Event()) _eof_lock: threading.Lock = dataclasses.field(default_factory=lambda: threading.Lock()) _eof_seen: int = 0 + # Handlers are registered in __post_init__, long before the first capture() + # and again between captures. An eof arriving outside a capture window (a + # late message from a previous scenario, or a skill emitting the eof topic + # on its own) must not count towards the next capture, or the next capture + # returns immediately with an empty message list and the test passes + # vacuously. Only an ARMED session counts eofs, and only for the generation + # that armed it. + _armed: bool = False + _generation: int = 0 + _done_generation: int = -1 + # set by capture() when the eof condition was never reached + timed_out: bool = False + timeout_seconds: Optional[float] = None def handle_message(self, msg: str): if self.done.is_set(): @@ -678,8 +1026,12 @@ def handle_message(self, msg: str): def handle_end_of_test(self, msg: Message): with self._eof_lock: + if not self._armed: + return self._eof_seen += 1 if self._eof_seen >= self.eof_count: + self._armed = False + self._done_generation = self._generation self.done.set() def __post_init__(self): @@ -687,23 +1039,87 @@ def __post_init__(self): for m in self.eof_msgs: self.minicroft.bus.on(m, self.handle_end_of_test) - def capture(self, source_message: Message, timeout=20): + def capture(self, source_message: Message, timeout=20) -> bool: + """Emit *source_message* and block until an eof message or *timeout*. + + Returns: + True if the eof condition was reached, False on timeout. The same + value is recorded on :attr:`timed_out` (inverted) so callers that + ignore the return value can still tell a timeout from a genuine + message-count mismatch. + """ test_message = deepcopy(source_message) # ensure object not mutated by ovos-core - self.done.clear() + # Reset the done flag and the eof counter ATOMICALLY: a handler running + # between the two would otherwise have its increment thrown away (or set + # done for the previous capture's counter). with self._eof_lock: + self.done.clear() self._eof_seen = 0 + self._generation += 1 + generation = self._generation + self._armed = True self.minicroft.bus.emit(test_message) - self.done.wait(timeout) + completed = self.done.wait(timeout) + if completed and self._done_generation != generation: + # `done` was set by something other than this capture's eof run + # (finish(), or a previous generation). Treat it as a timeout + # rather than reporting a completion this capture never saw. + completed = False + if not completed: + self.timed_out = True + self.timeout_seconds = timeout + return completed def finish(self) -> List[Message]: + with self._eof_lock: + self._armed = False self.done.set() self.minicroft.bus.remove("message", self.handle_message) for m in self.eof_msgs: self.minicroft.bus.remove(m, self.handle_end_of_test) - return self.responses + # Return a snapshot: the live list is still owned by this session (and + # __del__ calls finish() again), so handing it out invites surprise + # mutation from a late handler. + return list(self.responses) def __del__(self): - self.finish() + # At interpreter shutdown, or when construction failed part-way, the + # MiniCroft may have no bus (or be gone entirely). finish() would then + # raise inside __del__, which Python can only print and swallow. + if getattr(getattr(self, "minicroft", None), "bus", None) is None: + return + try: + self.finish() + except Exception: + pass + + +def _topic_matches(msg_type: str, name: str) -> bool: + """True if ``msg_type`` is ``name`` under either its legacy or canonical spelling. + + Producers emit canonical ``ovos.*`` spec topics since workshop#425, but + ovoscope's ``execute()`` captures every message via the bus catch-all + (faithfully to the real wire), so a pre-spec producer vintage in the + captured stream can still carry the legacy name instead. Assertions that + filter the captured stream by ``msg_type`` must therefore accept both + spellings. + + The legacy<->canonical pairing is not hand-rolled here: it reuses the + same static maps ``ovos-bus-client``'s ``MessageBusClient`` and + ``ovos-utils``' ``FakeBus`` use for their dual-emit bridging (see + ``ovos_spec_tools.messages.NamespaceTranslator`` / + ``MIGRATION_MAP`` / ``SPEC_TO_LEGACY``), so the pairing can't drift out + of sync with the real bus behaviour. + """ + if msg_type == name: + return True + canonical = MIGRATION_MAP.get(name) + if canonical is not None and msg_type == canonical.value: + return True + legacy = SPEC_TO_LEGACY.get(name) + if legacy is not None and msg_type == legacy: + return True + return False @dataclasses.dataclass() @@ -811,7 +1227,18 @@ def execute(self, timeout: int = 30) -> List[Message]: if self.minicroft is None: self.minicroft = get_minicroft(self.skill_ids) self.managed = True + # Teardown MUST run even when an assertion below fails: MiniCroft + # patches process-wide globals (SessionManager.bus / default_session, + # Configuration) that only stop() restores. Skipping it poisons every + # later test in the process. + try: + return self._execute(timeout) + finally: + if self.managed and self.minicroft is not None: + self.minicroft.stop() + self.minicroft = None + def _execute(self, timeout: int = 30) -> List[Message]: if self.test_boot_sequence and self.expected_boot_sequence: for expected, received in zip(self.expected_boot_sequence, self.minicroft.boot_messages): assert expected.msg_type == received.msg_type, f"❌ expected boot message_type '{expected.msg_type}' | got '{received.msg_type}'" @@ -854,34 +1281,60 @@ def execute(self, timeout: int = 30) -> List[Message]: eof_count=self.eof_count, ignore_messages=self.ignore_messages, async_messages=self.async_messages) - for idx, source_message in enumerate(self.source_message): - if "session" not in source_message.context and len(capture.responses): - # propagate session updates as a client would do - source_message.context["session"] = capture.responses[-1].context["session"] - capture.capture(source_message, timeout) - - # final message list - messages = capture.finish() - - # isolate a single dispatch lifecycle by skill_id — drop messages from a - # concurrent (interleaving) lifecycle so the assertion is deterministic. - if self.skill_id is not None: - messages = [m for m in messages - if (m.context or {}).get("skill_id") == self.skill_id] - if self.verbose: - print(f"💡 filtered to skill_id='{self.skill_id}': {len(messages)} messages") - if self.pipeline_id is not None: - messages = [m for m in messages - if (m.context or {}).get("pipeline_id") == self.pipeline_id] - if self.verbose: - print(f"💡 filtered to pipeline_id='{self.pipeline_id}': {len(messages)} messages") + # start_tracking() wraps bus.emit. Anything that raises between here and + # stop_tracking() would leave the wrapper installed for the rest of the + # process, and every later test would stack one more wrapper on top. + try: + for idx, source_message in enumerate(self.source_message): + if "session" not in source_message.context and len(capture.responses): + # propagate session updates as a client would do + prev_ctx = capture.responses[-1].context or {} + if "session" not in prev_ctx: + raise AssertionError( + f"❌ cannot chain source_message #{idx}: the last " + f"captured response " + f"('{capture.responses[-1].msg_type}') carries no " + f"session in its context, so there is nothing to " + f"propagate. Give this source_message an explicit " + f"session." + ) + source_message.context["session"] = prev_ctx["session"] + capture.capture(source_message, timeout) + + # final message list + messages = capture.finish() + + # isolate a single dispatch lifecycle by skill_id — drop messages + # from a concurrent (interleaving) lifecycle so the assertion is + # deterministic. + if self.skill_id is not None: + messages = [m for m in messages + if (m.context or {}).get("skill_id") == self.skill_id] + if self.verbose: + print(f"💡 filtered to skill_id='{self.skill_id}': {len(messages)} messages") + if self.pipeline_id is not None: + messages = [m for m in messages + if (m.context or {}).get("pipeline_id") == self.pipeline_id] + if self.verbose: + print(f"💡 filtered to pipeline_id='{self.pipeline_id}': {len(messages)} messages") + finally: + if _bus_tracker is not None: + _bus_tracker.stop_tracking() if _bus_tracker is not None: - _bus_tracker.stop_tracking() all_responses = messages + list(getattr(capture, "async_responses", [])) _bus_tracker.record_session(all_responses, self.expected_messages) self.bus_coverage_report = _bus_tracker.build_report() + # A capture timeout means the scenario never terminated. Say so plainly + # — otherwise it surfaces as a baffling message-count mismatch. + assert not capture.timed_out, ( + f"❌ capture timed out after {capture.timeout_seconds}s waiting for " + f"eof_msgs {self.eof_msgs} (needed {self.eof_count}, " + f"got {capture._eof_seen}) — captured {len(messages)} messages: " + f"{[m.msg_type for m in messages]}" + ) + if self.test_message_number: n1 = len(self.expected_messages) n2 = len(messages) @@ -1010,11 +1463,6 @@ def execute(self, timeout: int = 30) -> List[Message]: if self.print_bus_coverage and self.bus_coverage_report is not None: print(self.bus_coverage_report.summary_line()) - if self.managed: - self.minicroft.stop() - del self.minicroft - self.minicroft = None - return messages @staticmethod @@ -1102,13 +1550,16 @@ def from_message(cls, message: Union[Message, List[Message]], ignore_messages=ignore_messages, async_messages=async_messages) - for idx, source_message in enumerate(message): - if "session" not in source_message.context and len(capture.responses): - # propagate session updates as a client would do - source_message.context["session"] = capture.responses[-1].context["session"] - capture.capture(source_message, timeout) - - minicroft.stop() + # stop() restores process-wide globals — it must run even if a capture + # raises. + try: + for idx, source_message in enumerate(message): + if "session" not in source_message.context and len(capture.responses): + # propagate session updates as a client would do + source_message.context["session"] = capture.responses[-1].context["session"] + capture.capture(source_message, timeout) + finally: + minicroft.stop() expected_messages = capture.finish() return End2EndTest( skill_ids=skill_ids, @@ -1149,7 +1600,7 @@ def assert_spoke(self, text: str, lang: str = "en-US", timeout: int = 30) -> Non speak_utterances = [ m.data.get("utterance") for m in messages - if m.msg_type == "speak" and m.data.get("lang") == lang + if _topic_matches(m.msg_type, "speak") and m.data.get("lang") == lang ] assert text in speak_utterances, ( f"❌ speak '{text}' (lang={lang}) not found. " @@ -1313,19 +1764,39 @@ def __exit__(self, *_: Any) -> None: """Stop capturing on context-manager exit.""" self.stop() - def assert_page_shown(self, namespace: str, page: str, timeout: float = 2.0) -> None: + @staticmethod + def _ns_matches(expected: str, actual: str, exact: bool) -> bool: + """Compare a GUI namespace, exactly by default. + + Substring matching cannot fail on a near-match: asserting namespace + ``"skill-weather"`` would pass on ``"skill-weather-extended"``, and + asserting ``"weather"`` would pass on any namespace containing it. Use + ``exact=False`` only when you deliberately want prefix behaviour. + """ + if exact: + return expected == actual + return actual.startswith(expected) + + def assert_page_shown(self, namespace: str, page: str, timeout: float = 2.0, + exact: bool = True) -> None: """Assert that a GUI page was shown in the given namespace. Polls the captured messages for up to *timeout* seconds. Args: namespace: GUI namespace (typically the skill ID slug). - page: QML page filename (e.g. ``"hello.qml"``). + page: QML page filename (e.g. ``"hello.qml"``). Compared against + the basename of each shown page, so a directory prefix in the + message does not affect the result. timeout: Maximum seconds to wait. + exact: Compare namespace and page basename by equality (default). + Set ``False`` for prefix matching on the namespace and + substring matching on the page. Raises: AssertionError: If no matching ``gui.page.show`` message is found. """ + import os import time deadline = time.monotonic() + timeout while time.monotonic() < deadline: @@ -1337,7 +1808,13 @@ def assert_page_shown(self, namespace: str, page: str, timeout: float = 2.0) -> pages = (msg.data.get("pages", []) or msg.data.get("page_names", []) or [msg.data.get("page", "")]) - if namespace in data_ns and any(page in str(p) for p in pages): + if not self._ns_matches(namespace, data_ns, exact): + continue + if exact: + hit = any(os.path.basename(str(p)) == page for p in pages) + else: + hit = any(page in str(p) for p in pages) + if hit: return time.sleep(0.05) captured = [(m.msg_type, m.data) for m in self.messages] @@ -1348,7 +1825,8 @@ def assert_page_shown(self, namespace: str, page: str, timeout: float = 2.0) -> def assert_template_shown(self, namespace: str, template: str, values: Optional[Dict[str, Any]] = None, - timeout: float = 2.0) -> None: + timeout: float = 2.0, + exact: bool = True) -> None: """Assert that a built-in ``SYSTEM_*`` template was shown. Ergonomic helper for the template-based GUI: a skill calling a typed @@ -1358,6 +1836,7 @@ def assert_template_shown(self, namespace: str, template: str, Args: namespace: GUI namespace (typically the skill ID). + exact: Compare namespace and page name by equality (default). template: Template name, with or without the ``SYSTEM_`` prefix (``"weather"`` and ``"SYSTEM_weather"`` are equivalent). values: Optional mapping of session-data keys to expected values; @@ -1369,17 +1848,19 @@ def assert_template_shown(self, namespace: str, template: str, was not set. """ name = template if template.startswith("SYSTEM_") else f"SYSTEM_{template}" - self.assert_page_shown(namespace, name, timeout=timeout) + self.assert_page_shown(namespace, name, timeout=timeout, exact=exact) for key, value in (values or {}).items(): - self.assert_namespace_value(namespace, key, value) + self.assert_namespace_value(namespace, key, value, exact=exact) - def assert_namespace_value(self, namespace: str, key: str, value: Any) -> None: + def assert_namespace_value(self, namespace: str, key: str, value: Any, + exact: bool = True) -> None: """Assert that a namespace key was set to a specific value. Args: namespace: GUI namespace to check. key: Data key within the namespace. value: Expected value. + exact: Compare the namespace by equality (default). Raises: AssertionError: If no matching ``gui.value.set`` message is found. @@ -1389,7 +1870,7 @@ def assert_namespace_value(self, namespace: str, key: str, value: Any) -> None: data_ns = (msg.data.get("namespace", "") or msg.data.get("__from", "") or msg.context.get("skill_id", "")) - if namespace in data_ns: + if self._ns_matches(namespace, data_ns, exact): data = msg.data.get("data", msg.data) if data.get(key) == value: return @@ -1398,7 +1879,8 @@ def assert_namespace_value(self, namespace: str, key: str, value: Any) -> None: f"Captured GUI messages: {[m.msg_type for m in self.messages]}" ) - def assert_namespace_has_key(self, namespace: str, key: str) -> None: + def assert_namespace_has_key(self, namespace: str, key: str, + exact: bool = True) -> None: """Assert that a key was set in a namespace, regardless of value. Useful for dynamic data (e.g. weather API responses, timestamps) @@ -1407,6 +1889,7 @@ def assert_namespace_has_key(self, namespace: str, key: str) -> None: Args: namespace: GUI namespace to check. key: Data key that should exist within the namespace. + exact: Compare the namespace by equality (default). Raises: AssertionError: If no matching message with the key is found. @@ -1416,7 +1899,7 @@ def assert_namespace_has_key(self, namespace: str, key: str) -> None: data_ns = (msg.data.get("namespace", "") or msg.data.get("__from", "") or msg.context.get("skill_id", "")) - if namespace in data_ns: + if self._ns_matches(namespace, data_ns, exact): data = msg.data.get("data", msg.data) if key in data: return @@ -1426,21 +1909,27 @@ def assert_namespace_has_key(self, namespace: str, key: str) -> None: f"Captured GUI messages: {[m.msg_type for m in self.messages]}" ) - def assert_namespace_cleared(self, namespace: str) -> None: + def assert_namespace_cleared(self, namespace: str, + exact: bool = True) -> None: """Assert that a namespace was cleared/removed. Args: namespace: GUI namespace that should have been cleared. + exact: Compare the namespace by equality (default). Raises: AssertionError: If no matching namespace-clear message is found. """ + # `gui.clear.namespace` is the topic the GUI service actually emits. + # Matching only "namespace.clear" / "namespace.remove" made this + # assertion impossible to satisfy on the real wire format. + clear_types = ("namespace.remove", "namespace.clear", "clear.namespace") for msg in self.messages: - if "namespace.remove" in msg.msg_type or "namespace.clear" in msg.msg_type: + if any(t in msg.msg_type for t in clear_types): data_ns = (msg.data.get("namespace", "") or msg.data.get("__from", "") or msg.context.get("skill_id", "")) - if namespace in data_ns: + if self._ns_matches(namespace, data_ns, exact): return raise AssertionError( f"Expected namespace {namespace!r} to be cleared, " diff --git a/ovoscope/audio.py b/ovoscope/audio.py index bd504c1..34c9ad1 100644 --- a/ovoscope/audio.py +++ b/ovoscope/audio.py @@ -310,11 +310,20 @@ def __enter__(self) -> "AudioServiceHarness": return self def __exit__(self, *args) -> None: - """Shut down AudioService and close the bus.""" - if self.service: - self.service.shutdown() - if self.bus: - self.bus.close() + """Shut down AudioService and close the bus. + + The bus is closed even when ``shutdown()`` raises — leaking an open + FakeBus keeps handlers alive and feeds a dead harness. + """ + try: + if self.service: + self.service.shutdown() + finally: + if self.bus: + try: + self.bus.close() + except Exception: + pass # ------------------------------------------------------------------ # Control methods @@ -557,6 +566,9 @@ class PlaybackServiceHarness: utterance is captured in :attr:`captured_wavs`. """ + # Only one harness may hold the process-wide ``TTS.queue`` at a time. + _active: ClassVar[Optional["PlaybackServiceHarness"]] = None + def __init__(self, validate_source: bool = False, disable_ocp: bool = True, tts: Optional[TTS] = None, @@ -588,6 +600,9 @@ def __init__(self, validate_source: bool = False, self.mock_tts: Optional[TTS] = None # Paths captured from the ``play_audio`` side_effect, in playback order. self.captured_wavs: List[str] = [] + # process-wide TTS.queue bookkeeping (see __enter__) + self._previous_tts_queue = None + self._replaced_tts_queue: bool = False self._play_audio_patcher = None self._audio_enabled_patcher = None self._audio_output_start = threading.Event() @@ -603,6 +618,17 @@ def __enter__(self) -> "PlaybackServiceHarness": from ovos_audio.service import PlaybackService from queue import Queue + # ``TTS.queue`` is process-wide CLASS state, so only ONE + # PlaybackServiceHarness may be active at a time — two live harnesses + # would fight over the same queue and steal each other's utterances. + # Refuse to start rather than corrupt both. + if PlaybackServiceHarness._active is not None: + raise RuntimeError( + "Another PlaybackServiceHarness is already active. " + "TTS.queue is process-wide class state, so only one harness " + "may run at a time — exit the current one first." + ) + # Drain any leftover TTS queue from previous tests (class-level state) if TTS.queue is not None: while not TTS.queue.empty(): @@ -610,33 +636,45 @@ def __enter__(self) -> "PlaybackServiceHarness": TTS.queue.get_nowait() except Exception: break - TTS.queue = Queue() - - self.bus = FakeBus(modernize=self.modernize, - emit_legacy=self.emit_legacy) - # Inject the provided TTS (real plugin) or fall back to MockTTS. - self.mock_tts = self.tts if self.tts is not None else MockTTS() - - # Patch play_audio so no real audio device is accessed. The side_effect - # records the first positional arg — the rendered WAV path - # (ovos_audio/playback.py: ``self.p = play_audio(data)``) — so callers - # can round-trip the synthesised audio through a reference STT. - mock_proc = MagicMock() - mock_proc.communicate.return_value = (b"", b"") - mock_proc.wait.return_value = 0 - - self.captured_wavs = [] - - def _capture_play_audio(data, *args, **kwargs): - self.captured_wavs.append(data) - return mock_proc - - self._play_audio_patcher = patch( - "ovos_audio.playback.play_audio", side_effect=_capture_play_audio - ) - self._play_audio_patcher.start() - + # Everything from here on mutates process-wide state (TTS.queue, the + # _active singleton, the play_audio patch). A failure part-way through + # must undo ALL of it: leaving _active set makes every later harness + # refuse to start with a bogus "already active" error, and leaving + # TTS.queue replaced steals the utterances of every later test. + self._play_audio_patcher = None try: + # Remember the previous queue object so __exit__ can put it back. + self._previous_tts_queue = TTS.queue + self._replaced_tts_queue = True + TTS.queue = Queue() + PlaybackServiceHarness._active = self + + self.bus = FakeBus(modernize=self.modernize, + emit_legacy=self.emit_legacy) + # Inject the provided TTS (real plugin) or fall back to MockTTS. + self.mock_tts = self.tts if self.tts is not None else MockTTS() + + # Patch play_audio so no real audio device is accessed. The + # side_effect records the first positional arg — the rendered WAV + # path (ovos_audio/playback.py: ``self.p = play_audio(data)``) — so + # callers can round-trip the synthesised audio through a + # reference STT. + mock_proc = MagicMock() + mock_proc.communicate.return_value = (b"", b"") + mock_proc.wait.return_value = 0 + + self.captured_wavs = [] + + def _capture_play_audio(data, *args, **kwargs): + self.captured_wavs.append(data) + return mock_proc + + self._play_audio_patcher = patch( + "ovos_audio.playback.play_audio", + side_effect=_capture_play_audio + ) + self._play_audio_patcher.start() + # Build the service — passing tts= sets disable_reload = True self.svc = PlaybackService( bus=self.bus, @@ -655,35 +693,56 @@ def _capture_play_audio(data, *args, **kwargs): self.bus.on(SpecMessage.MIC_LISTEN, lambda m: self._mic_listen.set()) - except Exception: + except BaseException: if self.svc: try: self.svc.shutdown() except Exception: pass - self._play_audio_patcher.stop() - self.bus.close() + if self._play_audio_patcher is not None: + try: + self._play_audio_patcher.stop() + except RuntimeError: + pass + self._play_audio_patcher = None + if self.bus is not None: + try: + self.bus.close() + except Exception: + pass + self._release_tts_queue() raise return self + def _release_tts_queue(self) -> None: + """Restore the ``TTS.queue`` object this harness replaced.""" + if getattr(self, "_replaced_tts_queue", False): + TTS.queue = self._previous_tts_queue + self._replaced_tts_queue = False + if PlaybackServiceHarness._active is self: + PlaybackServiceHarness._active = None + def __exit__(self, *args) -> None: - """Shut down PlaybackService and stop patches.""" - if self.svc: - try: - self.svc.shutdown() - except Exception: - pass - if self._play_audio_patcher: - try: - self._play_audio_patcher.stop() - except Exception: - pass - if self.bus: - try: - self.bus.close() - except Exception: - pass + """Shut down PlaybackService, stop patches, release the TTS queue.""" + try: + if self.svc: + try: + self.svc.shutdown() + except Exception: + pass + if self._play_audio_patcher: + try: + self._play_audio_patcher.stop() + except Exception: + pass + if self.bus: + try: + self.bus.close() + except Exception: + pass + finally: + self._release_tts_queue() # ------------------------------------------------------------------ # Control methods # ------------------------------------------------------------------ diff --git a/ovoscope/bus_coverage.py b/ovoscope/bus_coverage.py index 6b57bc3..2c0400f 100644 --- a/ovoscope/bus_coverage.py +++ b/ovoscope/bus_coverage.py @@ -52,6 +52,7 @@ import ovoscope from ovos_bus_client.message import Message +from ovos_utils.log import LOG # --------------------------------------------------------------------------- @@ -388,10 +389,15 @@ def __init__(self, bus: Any, minicroft: Any) -> None: self._global_registrations: Dict[str, int] = {} self._global_skill_registrations: Dict[str, Dict[str, int]] = {} + # The global collector is SESSION-cumulative: it keeps counting across + # every test in the process. Snapshot it at tracker START so the report + # can use the DELTA (collector_now - snapshot_at_start) — otherwise + # every later test inherits the invocation counts of every earlier one. + self._collector_baseline: Dict[str, int] = {} + self._global_frozen: bool = False collector = ovoscope.GLOBAL_BUS_COVERAGE_COLLECTOR if collector: - # Snapshot the global state at initialization - self._global_invocations = dict(collector.invocations) + self._collector_baseline = dict(collector.invocations) self._global_registrations = dict(collector.registrations) self._global_skill_registrations = deepcopy(collector.skill_registrations) @@ -400,8 +406,25 @@ def __init__(self, bus: Any, minicroft: Any) -> None: # skill_id -> {msg_type -> asserted_count} self._asserted: Dict[str, Dict[str, int]] = {} self._original_emit: Optional[Any] = None + self._patched_emit: Optional[Any] = None self._tracking: bool = False + def _collector_delta(self) -> Dict[str, int]: + """Invocations recorded by the global collector since tracker start. + + Covers this test's own boot sequence when the tracker is constructed + before the MiniCroft boots, and excludes everything earlier tests did. + """ + collector = ovoscope.GLOBAL_BUS_COVERAGE_COLLECTOR + if not collector: + return {} + delta: Dict[str, int] = {} + for msg_type, count in collector.invocations.items(): + diff = count - self._collector_baseline.get(msg_type, 0) + if diff > 0: + delta[msg_type] = diff + return delta + # ------------------------------------------------------------------ # Public API # ------------------------------------------------------------------ @@ -557,6 +580,13 @@ def start_tracking(self) -> None: """ if self._tracking: return + # Freeze the collector delta accumulated between tracker construction + # and now — i.e. this test's own boot sequence when the tracker was + # created before the MiniCroft booted. Everything from here on is + # counted locally by the patched emit below, so freezing here is what + # keeps the two sources from double counting the same emit. + self._global_invocations = self._collector_delta() + self._global_frozen = True original_emit = self._bus.emit invocations = self._invocations @@ -567,16 +597,30 @@ def _patched_emit(message: Any) -> None: original_emit(message) self._original_emit = original_emit + self._patched_emit = _patched_emit self._bus.emit = _patched_emit self._tracking = True def stop_tracking(self) -> None: - """Restore the original ``bus.emit`` and stop counting invocations.""" + """Restore the original ``bus.emit`` and stop counting invocations. + + Restores only when ``bus.emit`` is still THIS tracker's wrapper. If + another tracker wrapped the bus on top of ours, assigning our saved + original would silently discard that tracker's wrapper and leave it + counting nothing; leaving the chain alone is the lesser damage. + """ if not self._tracking: return + self._tracking = False + if self._bus.emit is not getattr(self, "_patched_emit", None): + LOG.warning("ovoscope: bus.emit was re-wrapped by another tracker; " + "leaving it in place instead of clobbering it") + self._original_emit = None + self._patched_emit = None + return self._bus.emit = self._original_emit self._original_emit = None - self._tracking = False + self._patched_emit = None def record_session( self, @@ -630,6 +674,11 @@ def build_report(self) -> BusCoverageReport: Returns: Fully populated :class:`BusCoverageReport` instance. """ + if not self._global_frozen: + # start_tracking() was never called — fall back to the full delta + # over the tracker's lifetime. + self._global_invocations = self._collector_delta() + self._global_frozen = True all_skill_ids = ( set(self._registered) | set(self._observed) diff --git a/ovoscope/cli.py b/ovoscope/cli.py index 5548b86..9ba0eec 100644 --- a/ovoscope/cli.py +++ b/ovoscope/cli.py @@ -20,6 +20,7 @@ * ``diff`` — Compare two fixture files with colored output. * ``validate`` — Schema-validate one or more fixture files. * ``coverage`` — Scan a workspace root and report E2E test coverage. +* ``bus-coverage`` — Run fixtures and report bus handler/emitter coverage. Usage:: @@ -29,6 +30,7 @@ ovoscope diff expected.json actual.json ovoscope validate fixture.json ovoscope coverage path/to/OpenVoiceOS/ + ovoscope bus-coverage test/fixtures/ """ from __future__ import annotations @@ -184,6 +186,12 @@ def cmd_run(args: argparse.Namespace) -> int: _die("MiniCroft did not reach READY state in time.") try: + # Hand the already-booted MiniCroft to the test. Without this, + # execute() boots a SECOND managed MiniCroft and both patch the same + # process-wide globals. `managed = False` keeps ownership here — the + # finally block below stops it. + test.minicroft = mc + test.managed = False test.execute(timeout=timeout) print("[run] PASS") return 0 @@ -211,11 +219,14 @@ def cmd_diff(args: argparse.Namespace) -> int: except ImportError as exc: _die(f"ovoscope.diff import failed: {exc}") - result = diff_fixtures( - expected_path=args.expected, - actual_path=args.actual, - ignore_context=not args.include_context, - ) + try: + result = diff_fixtures( + expected_path=args.expected, + actual_path=args.actual, + ignore_context=not args.include_context, + ) + except (OSError, ValueError) as exc: + _die(f"Could not diff fixtures: {exc}") result.print_report(color=not args.no_color) return 0 if result.is_identical else 1 @@ -223,7 +234,10 @@ def cmd_diff(args: argparse.Namespace) -> int: def cmd_validate(args: argparse.Namespace) -> int: """Schema-validate one or more fixture JSON files. - Runs basic structural validation on every fixture file. + Uses :func:`ovoscope.pydantic_helpers.validate_fixture` (per-message + schema validation against ``OpenVoiceOSMessage``) when the ``pydantic`` + extra is installed, falling back to basic JSON structure validation + (required top-level keys, ``expected_messages`` is a list) otherwise. Args: args: Parsed CLI arguments with fixtures (list of paths). @@ -231,10 +245,21 @@ def cmd_validate(args: argparse.Namespace) -> int: Returns: Exit code (0 = all valid, 1 = validation failure). """ + try: + from ovoscope.pydantic_helpers import _PYDANTIC_AVAILABLE, validate_fixture + except ImportError: + _PYDANTIC_AVAILABLE = False + validate_fixture = None + all_ok = True for path in args.fixtures: try: + # Structural checks always run — pydantic validation is a + # per-message layer on top, not a replacement (validate_fixture + # skips sections that are absent entirely). _basic_validate(path) + if _PYDANTIC_AVAILABLE: + validate_fixture(path) print(f"[validate] OK {path}") except Exception as exc: print(f"[validate] FAIL {path}: {exc}") diff --git a/ovoscope/coverage.py b/ovoscope/coverage.py index b4577eb..642399b 100644 --- a/ovoscope/coverage.py +++ b/ovoscope/coverage.py @@ -35,7 +35,9 @@ import json import os from dataclasses import dataclass, field -from typing import Any, Dict, List, Optional +from typing import Any, Dict, List, Optional, Tuple + +from ovos_utils.log import LOG # Entry-point groups that indicate the repo type. @@ -93,6 +95,9 @@ class EcosystemCoverageReport: repos: List[RepoCoverage] = field(default_factory=list) scan_root: str = "" + # (path, reason) for every manifest that could not be parsed. A malformed + # pyproject.toml used to be swallowed, which silently understated coverage. + parse_errors: List[Tuple[str, str]] = field(default_factory=list) @property def coverage_pct(self) -> float: @@ -212,7 +217,8 @@ def _find_setup_pys(root: str) -> List[str]: return sorted(results) -def _parse_setup_py_entry_points(setup_py_path: str) -> Dict[str, List[str]]: +def _parse_setup_py_entry_points(setup_py_path: str, + errors: Optional[List[Tuple[str, str]]] = None) -> Dict[str, List[str]]: """Extract entry-point groups from a ``setup.py`` file via regex. Detects lines like:: @@ -239,7 +245,8 @@ def _parse_setup_py_entry_points(setup_py_path: str) -> Dict[str, List[str]]: try: with open(setup_py_path, "r", encoding="utf-8") as fh: source = fh.read() - except Exception: + except (OSError, UnicodeDecodeError) as exc: + _record_error(errors, setup_py_path, f"unreadable: {exc}") return eps # --- Pass 1: collect literal string assignments for ENTRY_POINT variables --- @@ -290,7 +297,8 @@ def _parse_setup_py_entry_points(setup_py_path: str) -> Dict[str, List[str]]: return eps -def _parse_entry_points(pyproject_path: str) -> Dict[str, List[str]]: +def _parse_entry_points(pyproject_path: str, + errors: Optional[List[Tuple[str, str]]] = None) -> Dict[str, List[str]]: """Parse entry-point groups from a ``pyproject.toml`` file. Uses stdlib ``tomllib`` (Python 3.11+) or falls back to line-by-line @@ -302,18 +310,31 @@ def _parse_entry_points(pyproject_path: str) -> Dict[str, List[str]]: Returns: Mapping of entry-point group name → list of entry-point IDs. """ + eps: Dict[str, List[str]] = {} try: import tomllib # Python 3.11+ - with open(pyproject_path, "rb") as fh: - data = tomllib.load(fh) - eps: Dict[str, List[str]] = {} - # setuptools style - for group, entries in data.get("project", {}).get("entry-points", {}).items(): - eps[group] = list(entries.keys()) - # Also check [project.scripts] for CLI tools - return eps - except (ImportError, Exception): - pass + except ImportError: + try: + import tomli as tomllib # type: ignore[no-redef] + except ImportError: + tomllib = None # type: ignore[assignment] + + if tomllib is not None: + try: + with open(pyproject_path, "rb") as fh: + data = tomllib.load(fh) + # setuptools style + for group, entries in data.get("project", {}).get( + "entry-points", {}).items(): + eps[group] = list(entries.keys()) + return eps + except tomllib.TOMLDecodeError as exc: + # A malformed manifest is a real problem — record it instead of + # silently reporting the repo as having no entry points. + _record_error(errors, pyproject_path, f"invalid TOML: {exc}") + except OSError as exc: + _record_error(errors, pyproject_path, f"unreadable: {exc}") + return eps # Fallback: simple line-by-line scan for entry-point group headers eps = {} @@ -331,11 +352,19 @@ def _parse_entry_points(pyproject_path: str) -> Dict[str, List[str]]: eps[current_group].append(key) elif line.startswith("["): current_group = None - except Exception: - pass + except (OSError, IndexError, UnicodeDecodeError) as exc: + _record_error(errors, pyproject_path, f"fallback scan failed: {exc}") return eps +def _record_error(errors: Optional[List[Tuple[str, str]]], + path: str, reason: str) -> None: + """Append a parse failure to *errors* and log it.""" + LOG.warning("ovoscope coverage: %s — %s", path, reason) + if errors is not None: + errors.append((path, reason)) + + def _has_e2e_tests(repo_root: str) -> bool: """Check if *repo_root* has ``test/end2end/`` with at least one ``.py`` file. @@ -442,7 +471,8 @@ def scan_workspace(root: str) -> EcosystemCoverageReport: # --- pyproject.toml repos --- for pyproject_path in _find_pyproject_tomls(root): repo_root = os.path.dirname(pyproject_path) - entry_point_groups = _parse_entry_points(pyproject_path) + entry_point_groups = _parse_entry_points(pyproject_path, + report.parse_errors) cov = _collect_repo(repo_root, entry_point_groups) if cov is not None: report.repos.append(cov) @@ -453,7 +483,8 @@ def scan_workspace(root: str) -> EcosystemCoverageReport: repo_root = os.path.dirname(setup_py_path) if repo_root in seen_roots: continue - entry_point_groups = _parse_setup_py_entry_points(setup_py_path) + entry_point_groups = _parse_setup_py_entry_points(setup_py_path, + report.parse_errors) cov = _collect_repo(repo_root, entry_point_groups) if cov is not None: report.repos.append(cov) diff --git a/ovoscope/diff.py b/ovoscope/diff.py index 0a267ee..982e96e 100644 --- a/ovoscope/diff.py +++ b/ovoscope/diff.py @@ -23,6 +23,16 @@ from typing import Any, Dict, List, Optional, Tuple +class _Missing: + """Sentinel for "key absent", distinct from a stored ``None``.""" + + def __repr__(self) -> str: # pragma: no cover - debug aid + return "" + + +_MISSING = _Missing() + + @dataclass class MessageDiff: """Diff result for a single message pair at a given index. @@ -141,9 +151,12 @@ def _dict_diff( """ diffs: Dict[str, Tuple[Any, Any]] = {} for k, exp_v in expected.items(): - act_v = actual.get(k) + # _MISSING, not None: an expected value of None must still differ from + # an ABSENT key, otherwise `{"a": None}` vs `{}` compares equal and the + # diff reports a match that is not there. + act_v = actual.get(k, _MISSING) if act_v != exp_v: - diffs[k] = (exp_v, act_v) + diffs[k] = (exp_v, None if act_v is _MISSING else act_v) if strict: for k, act_v in actual.items(): if k not in expected: @@ -162,10 +175,18 @@ def _load_messages(path: str) -> List[Dict[str, Any]]: Raises: FileNotFoundError: If *path* does not exist. + ValueError: If the file is not an ovoscope fixture (no + ``expected_messages`` key). Defaulting to an empty list here made + two unrelated JSON files compare as "Identical" and exit 0. """ with open(path, "r", encoding="utf-8") as fh: payload = json.load(fh) - return payload.get("expected_messages", []) + if not isinstance(payload, dict) or "expected_messages" not in payload: + raise ValueError( + f"not an ovoscope fixture: {path} has no 'expected_messages' key. " + f"Fixture files are produced by End2EndTest.save()." + ) + return payload["expected_messages"] def diff_fixtures( diff --git a/ovoscope/e2e.py b/ovoscope/e2e.py index 6de66b1..f68445f 100644 --- a/ovoscope/e2e.py +++ b/ovoscope/e2e.py @@ -73,20 +73,33 @@ def wait_for_match( expected_types: List[str], *, timeout: float = 5.0, + emit: Optional[Message] = None, ) -> Optional[Message]: """Subscribe to ``expected_types`` and ``complete_intent_failure``; return the first match Message, or ``None`` on failure / timeout. - The caller is responsible for emitting the utterance *after* calling this - helper if used in a pytest style — for the ``unittest`` style use - :meth:`E2EPipelineHarness.send_and_capture` which emits internally. + This helper BLOCKS, so a single-threaded caller cannot emit the utterance + after calling it. Pass the message as *emit* instead: it is emitted after + the handlers are subscribed, so no reply can be missed. Emit it yourself + beforehand only when the reply is guaranteed to be asynchronous. + + Args: + bus: The bus to subscribe on. + expected_types: Message types that count as a match. + timeout: Seconds to wait for a verdict. + emit: Message emitted once the handlers are in place. + + Returns: + The first matching :class:`Message`, or ``None``. """ got: List[Message] = [] + lock = threading.Lock() done = threading.Event() failed = threading.Event() def _on_match(msg: Message) -> None: - got.append(msg) + with lock: + got.append(msg) done.set() def _on_fail(_msg: Message) -> None: @@ -97,14 +110,31 @@ def _on_fail(_msg: Message) -> None: bus.on(t, _on_match) bus.on("complete_intent_failure", _on_fail) try: + if emit is not None: + bus.emit(emit) done.wait(timeout=timeout) finally: for t in expected_types: bus.remove(t, _on_match) bus.remove("complete_intent_failure", _on_fail) - if failed.is_set() and not got: + return _first_match(got, failed, lock) + + +def _first_match(got: List[Message], failed: threading.Event, + lock: threading.Lock) -> Optional[Message]: + """Return the first captured match, tolerating a match/fail race. + + ``failed`` can be observed set while a concurrent ``got.append`` is still + in flight, which used to drop a real match. Take the lock (the appender + holds it) and re-read once before giving up. + """ + with lock: + if got: + return got[0] + if failed.is_set(): return None - return got[0] if got else None + with lock: + return got[0] if got else None def wait_for_failure(bus, *, timeout: float = 2.0) -> bool: @@ -301,31 +331,12 @@ def send_and_capture( session: Optional[Session] = None, ) -> Optional[Message]: """Emit ``utterance`` and return the first match Message (or None).""" - got: List[Message] = [] - done = threading.Event() - failed = threading.Event() - - def _on_match(msg: Message) -> None: - got.append(msg) - done.set() - - def _on_fail(_msg: Message) -> None: - failed.set() - done.set() - - for t in expected_types: - self.bus.on(t, _on_match) - self.bus.on("complete_intent_failure", _on_fail) - try: - self.bus.emit(self.make_utterance(utterance, session=session)) - done.wait(timeout=timeout) - finally: - for t in expected_types: - self.bus.remove(t, _on_match) - self.bus.remove("complete_intent_failure", _on_fail) - if failed.is_set() and not got: - return None - return got[0] if got else None + return wait_for_match( + self.bus, + expected_types, + timeout=timeout, + emit=self.make_utterance(utterance, session=session), + ) def expect_no_match( self, diff --git a/ovoscope/intent_cases.py b/ovoscope/intent_cases.py index a6fa0b5..08676b3 100644 --- a/ovoscope/intent_cases.py +++ b/ovoscope/intent_cases.py @@ -50,6 +50,7 @@ """ from __future__ import annotations +import atexit import dataclasses import time from copy import deepcopy @@ -282,9 +283,11 @@ def on_register(_serialized): seen["count"] += 1 seen["last_t"] = time.monotonic() - mc.bus.ee.on("padatious:register_intent", - lambda _: on_register(None)) - mc.bus.ee.on("register_intent", lambda _: on_register(None)) + def on_padatious(_): + on_register(None) + + def on_adapt(_): + on_register(None) # Wildcard "message" listener catches the serialized form on FakeBus. def on_any(serialized): @@ -297,20 +300,56 @@ def on_any(serialized): if t == "padatious:register_intent" or t == "register_intent": on_register(None) + mc.bus.ee.on("padatious:register_intent", on_padatious) + mc.bus.ee.on("register_intent", on_adapt) mc.bus.ee.on("message", on_any) t0 = time.monotonic() - mc.bus.emit(Message("mycroft.ready", {}, {})) + try: + mc.bus.emit(Message("mycroft.ready", {}, {})) + + # Wait for the burst of register events to settle. + while time.monotonic() - t0 < max_wait: + time.sleep(0.1) + if seen["count"] > 0 and (time.monotonic() - seen["last_t"]) > quiet_window: + break + if seen["count"] > 0: + # Register events arrived AND went quiet — the sync is observably + # finished, so there is nothing left to wait for. + return time.monotonic() - t0 + # Nothing was observed. We cannot tell whether m2v is mid-sync, so pad + # for the ceiling of its internal time.sleep(3) in handle_sync_intents. + time.sleep(3.5) + return time.monotonic() - t0 + finally: + # Always detach: these listeners live on the shared MiniCroft bus and + # would keep counting for the rest of the process. + for topic, handler in (("padatious:register_intent", on_padatious), + ("register_intent", on_adapt), + ("message", on_any)): + try: + mc.bus.ee.remove_listener(topic, handler) + except Exception: + pass + - # Wait for the burst of register events to settle. - while time.monotonic() - t0 < max_wait: - time.sleep(0.1) - if seen["count"] > 0 and (time.monotonic() - seen["last_t"]) > quiet_window: - break - # m2v's handle_sync_intents does an internal time.sleep(3) before the - # actual intent set update. Pad for that ceiling. - time.sleep(3.5) - return time.monotonic() - t0 +def stop_shared_minicrofts() -> None: + """Stop every cached shared MiniCroft and empty the cache. + + Registered with :mod:`atexit`; also safe to call from a session-scoped + fixture. A MiniCroft that is never stopped keeps its patches on the + process-wide ``SessionManager`` and ``Configuration``. + """ + cache = globals().get(_SHARED_MINICROFT_KEY) or {} + for mc in list(cache.values()): + try: + mc.stop() + except Exception: + pass + cache.clear() + + +atexit.register(stop_shared_minicrofts) def _shared_minicroft(skill_id: str, langs: List[str], m2v_warmup: float): @@ -319,10 +358,22 @@ def _shared_minicroft(skill_id: str, langs: List[str], m2v_warmup: float): Caches the instance on a process-global so every generated class shares the same boot. ``m2v_warmup`` is now an *upper bound*: if the deterministic event-based wait finishes faster, we return early. + + At most ONE instance stays alive: MiniCroft patches process-wide globals, + so a second live instance would clobber the first. Requesting a different + key stops the cached instance before booting the new one. """ cache = globals().setdefault(_SHARED_MINICROFT_KEY, {}) key = (skill_id, tuple(langs)) if key not in cache: + # Keep at most one live MiniCroft — two of them fight over the same + # SessionManager / Configuration globals. + for stale_key in [k for k in cache if k != key]: + stale = cache.pop(stale_key) + try: + stale.stop() + except Exception: + pass LOG.set_level("CRITICAL") secondary = [l for l in langs if l != "en-US"] mc = get_minicroft([skill_id], secondary_langs=secondary or None) diff --git a/ovoscope/listener.py b/ovoscope/listener.py index a5ec659..acc1d10 100644 --- a/ovoscope/listener.py +++ b/ovoscope/listener.py @@ -71,6 +71,7 @@ from ovos_bus_client.message import Message from ovos_utils.fakebus import FakeBus +from ovos_utils.log import LOG # --------------------------------------------------------------------------- @@ -104,7 +105,13 @@ def _wav_to_audio_data(audio: Union[bytes, str, Path], sample_rate = wf.getframerate() sample_width = wf.getsampwidth() frame_data = wf.readframes(wf.getnframes()) - except Exception: + except Exception as exc: + # Not a WAV container — treat the bytes as raw PCM at the caller's + # sample rate. Say so: a truncated or corrupt WAV lands here too, and + # silently reinterpreting its header bytes as audio produces garbage + # that is very hard to trace back to this line. + LOG.debug(f"ovoscope: could not parse audio as WAV ({exc}); " + f"treating the bytes as raw PCM") frame_data = audio return AudioData(frame_data, sample_rate, sample_width) @@ -333,16 +340,22 @@ def _capture(msg: Any) -> None: return self._messages.append(msg) + # Kept on the instance so shutdown() can unsubscribe it — a capture + # handler left on a shared bus keeps feeding a dead harness. + self._capture = _capture self.bus.on("message", _capture) + # Narrow the guard to the IMPORT: a constructor failure is a real bug + # in AudioTransformersService (or in the config passed to it) and must + # propagate, not masquerade as "ovos-dinkum-listener is not installed". try: from ovos_dinkum_listener.transformers import AudioTransformersService - - self.transformers: Optional[Any] = AudioTransformersService( - self.bus, config - ) except ImportError: - self.transformers = None + AudioTransformersService = None + if AudioTransformersService is None: + self.transformers: Optional[Any] = None + else: + self.transformers = AudioTransformersService(self.bus, config) if plugin_instances: if self.transformers is None: @@ -609,7 +622,12 @@ def detect_wakeword( for engine in engines.values(): engine.update(chunk) - return any(e.found_wake_word() for e in engines.values()) + # Materialise every result BEFORE reducing: found_wake_word() is a + # destructive read on most engines (it consumes the latch), so the + # short-circuit in any() would leave later engines still latched and + # make the NEXT call report a stale detection. + results = [e.found_wake_word() for e in engines.values()] + return any(results) def scan_for_wakeword( self, @@ -665,7 +683,10 @@ def scan_for_wakeword( for idx, frame in enumerate(frames): for engine in engines.values(): engine.update(frame) - if any(e.found_wake_word() for e in engines.values()): + # Read every engine's latch (destructive) before reducing — see + # feed_chunk above. + results = [e.found_wake_word() for e in engines.values()] + if any(results): return True, idx return False, None @@ -675,14 +696,27 @@ def scan_for_wakeword( # ------------------------------------------------------------------ def shutdown(self) -> None: - """Shut down all loaded transformer and wake-word plugins gracefully.""" - if self.transformers is not None: - self.transformers.shutdown() - for engine in self._ww.values(): - try: - engine.shutdown() - except Exception: - pass + """Shut down all loaded transformer and wake-word plugins gracefully. + + Also detaches the wildcard ``"message"`` capture handler so a shared + bus stops feeding this harness after it is gone. + """ + try: + if self.transformers is not None: + self.transformers.shutdown() + for engine in self._ww.values(): + try: + engine.shutdown() + except Exception: + pass + finally: + capture = getattr(self, "_capture", None) + if capture is not None: + try: + self.bus.remove("message", capture) + except Exception: + pass + self._capture = None # --------------------------------------------------------------------------- diff --git a/ovoscope/media.py b/ovoscope/media.py index 36b3683..b3c9242 100644 --- a/ovoscope/media.py +++ b/ovoscope/media.py @@ -316,6 +316,10 @@ def __enter__(self) -> "OCPPlayerHarness": """ from ovos_media.player import OCPMediaPlayer + # Re-entering a harness instance must not inherit the previous run's + # patch list; __exit__ clears it, but a caller that reuses the object + # after a failed enter would otherwise stop the same patches twice. + self._patches = [] self.bus = FakeBus(modernize=self.modernize, emit_legacy=self.emit_legacy) if self.backend_factory is not None: @@ -350,81 +354,90 @@ def __init__(self, *args, **kwargs): valid = [a for a in args if not isinstance(a, str)] super().__init__(*valid, **kwargs) - p_playlist = patch("ovos_media.player.Playlist", _TolerantPlaylist) - p_playlist.start() - self._patches.append(p_playlist) - - simple_targets = [ - "ovos_media.player.AudioService", - "ovos_media.player.VideoService", - "ovos_media.player.WebService", - "ovos_media.player.OcpMprisExporter", - "ovos_media.player.OCPMediaCatalog", - ] - for target in simple_targets: - p = patch(target) - p.start() - self._patches.append(p) - - p_cfg = patch("ovos_media.player.Configuration", - return_value={"media": {}}) - p_cfg.start() - self._patches.append(p_cfg) - - p_gui = patch("ovos_media.player.GUIInterface", - return_value=gui_mock) - p_gui.start() - self._patches.append(p_gui) - - # Instantiate the real player (all heavy deps are now mocked) - self.player = OCPMediaPlayer(self.bus, config={}) - - ns = self.backend_namespace - if self.backend_factory is not None: - # Real-backend mode: the mocked AudioService (a MagicMock) never routes - # play()->load_track()->backend.play(), so swap in a *real* AudioService - # with autoload off and the injected backend as its sole service. Now the - # player's playback path actually drives the real backend (e.g. asserting - # a Music Assistant client's play_media() call). - from ovos_media.media_backends.audio import AudioService as _RealAudioService - audio_svc = _RealAudioService(self.bus, config={"audio_players": {}}, - autoload=False, validate_source=False) - self.player.audio_service = audio_svc - # Deferred uris (e.g. library://, {sei}//) are resolved by the OCP - # pipeline's stream extractors *before* the player sees them; this - # harness drives the backend directly, so bypass the player's - # stream-extraction validation (no extractor plugins are loaded). - self.player.validate_stream = lambda: True - audio_svc.services = [self.backend] - audio_svc.default = self.backend - self.backend.set_track_start_callback(audio_svc.track_start) - # load_services() (skipped with autoload=False) would register these. - self.bus.on(f"ovos.{ns}.service.play", audio_svc.handle_play) - self.bus.on(f"ovos.{ns}.service.pause", audio_svc.pause) - self.bus.on(f"ovos.{ns}.service.resume", audio_svc.resume) - self.bus.on(f"ovos.{ns}.service.stop", audio_svc.stop) - # NB: BaseMediaService.__init__ already wired ovos.common_play.media.state - # -> handle_media_state_change; re-registering it would fire backend.play() - # twice, so it is deliberately omitted here. - audio_svc._loaded.set() - else: - # Mock-backend mode: drive the player state-machine against the MagicMock - # AudioService; the backend is exposed for manual simulate_*/state asserts. - audio_svc = self.player.audio_service - audio_svc.services = [self.backend] - audio_svc.default = self.backend - self.backend.set_track_start_callback(audio_svc.track_start) - # Register the audio service bus handlers manually - # (normally done inside BaseMediaService.load_services) - self.bus.on(f"ovos.{ns}.service.play", audio_svc.handle_play) - self.bus.on(f"ovos.{ns}.service.pause", audio_svc.pause) - self.bus.on(f"ovos.{ns}.service.resume", audio_svc.resume) - self.bus.on(f"ovos.{ns}.service.stop", audio_svc.stop) - self.bus.on("ovos.common_play.media.state", - audio_svc.handle_media_state_change) - audio_svc._loaded.set() - - return self + # Every mock.patch below is process-wide until stopped. If anything + # after the first start() raises (a missing ovos_media attribute, a + # backend constructor error), an unguarded exit would leave those + # patches active for the rest of the process and silently corrupt + # every later test. Unwind through __exit__ before propagating. + try: + p_playlist = patch("ovos_media.player.Playlist", _TolerantPlaylist) + p_playlist.start() + self._patches.append(p_playlist) + + simple_targets = [ + "ovos_media.player.AudioService", + "ovos_media.player.VideoService", + "ovos_media.player.WebService", + "ovos_media.player.OcpMprisExporter", + "ovos_media.player.OCPMediaCatalog", + ] + for target in simple_targets: + p = patch(target) + p.start() + self._patches.append(p) + + p_cfg = patch("ovos_media.player.Configuration", + return_value={"media": {}}) + p_cfg.start() + self._patches.append(p_cfg) + + p_gui = patch("ovos_media.player.GUIInterface", + return_value=gui_mock) + p_gui.start() + self._patches.append(p_gui) + + # Instantiate the real player (all heavy deps are now mocked) + self.player = OCPMediaPlayer(self.bus, config={}) + + ns = self.backend_namespace + if self.backend_factory is not None: + # Real-backend mode: the mocked AudioService (a MagicMock) never routes + # play()->load_track()->backend.play(), so swap in a *real* AudioService + # with autoload off and the injected backend as its sole service. Now the + # player's playback path actually drives the real backend (e.g. asserting + # a Music Assistant client's play_media() call). + from ovos_media.media_backends.audio import AudioService as _RealAudioService + audio_svc = _RealAudioService(self.bus, config={"audio_players": {}}, + autoload=False, validate_source=False) + self.player.audio_service = audio_svc + # Deferred uris (e.g. library://, {sei}//) are resolved by the OCP + # pipeline's stream extractors *before* the player sees them; this + # harness drives the backend directly, so bypass the player's + # stream-extraction validation (no extractor plugins are loaded). + self.player.validate_stream = lambda: True + audio_svc.services = [self.backend] + audio_svc.default = self.backend + self.backend.set_track_start_callback(audio_svc.track_start) + # load_services() (skipped with autoload=False) would register these. + self.bus.on(f"ovos.{ns}.service.play", audio_svc.handle_play) + self.bus.on(f"ovos.{ns}.service.pause", audio_svc.pause) + self.bus.on(f"ovos.{ns}.service.resume", audio_svc.resume) + self.bus.on(f"ovos.{ns}.service.stop", audio_svc.stop) + # NB: BaseMediaService.__init__ already wired ovos.common_play.media.state + # -> handle_media_state_change; re-registering it would fire backend.play() + # twice, so it is deliberately omitted here. + audio_svc._loaded.set() + else: + # Mock-backend mode: drive the player state-machine against the MagicMock + # AudioService; the backend is exposed for manual simulate_*/state asserts. + audio_svc = self.player.audio_service + audio_svc.services = [self.backend] + audio_svc.default = self.backend + self.backend.set_track_start_callback(audio_svc.track_start) + # Register the audio service bus handlers manually + # (normally done inside BaseMediaService.load_services) + self.bus.on(f"ovos.{ns}.service.play", audio_svc.handle_play) + self.bus.on(f"ovos.{ns}.service.pause", audio_svc.pause) + self.bus.on(f"ovos.{ns}.service.resume", audio_svc.resume) + self.bus.on(f"ovos.{ns}.service.stop", audio_svc.stop) + self.bus.on("ovos.common_play.media.state", + audio_svc.handle_media_state_change) + audio_svc._loaded.set() + + return self + except BaseException: + self.__exit__(None, None, None) + raise def __exit__(self, *args) -> None: """Shut down the player, close the bus, and stop all patches.""" @@ -443,6 +456,7 @@ def __exit__(self, *args) -> None: p.stop() except RuntimeError: pass + self._patches = [] # ------------------------------------------------------------------ # Control methods — emit the correct bus message and yield briefly diff --git a/ovoscope/media_provider.py b/ovoscope/media_provider.py index c70f476..da5b491 100644 --- a/ovoscope/media_provider.py +++ b/ovoscope/media_provider.py @@ -46,10 +46,12 @@ """ from __future__ import annotations +import concurrent.futures from importlib.metadata import entry_points -from typing import Any, List, Optional +from typing import Any, Callable, List, Optional DEFAULT_GROUP = "opm.media.provider" +DEFAULT_CALL_TIMEOUT = 30.0 class MediaProviderHarness: @@ -63,11 +65,40 @@ class MediaProviderHarness: def __init__(self, provider: Any, api: Any = None, entrypoint_name: Optional[str] = None, - entrypoint_group: str = DEFAULT_GROUP) -> None: + entrypoint_group: str = DEFAULT_GROUP, + call_timeout: Optional[float] = DEFAULT_CALL_TIMEOUT) -> None: self.provider = provider self.api = api self.entrypoint_name = entrypoint_name self.entrypoint_group = entrypoint_group + # A real provider talks to the network. Without a deadline a hung + # server (no socket timeout of its own) blocks the test run forever + # instead of failing it. Set to None to wait indefinitely. + self.call_timeout = call_timeout + + def _call(self, func: Callable, *args, **kwargs) -> Any: + """Run a provider call under :attr:`call_timeout`. + + Raises: + TimeoutError: when the provider does not answer in time. + """ + if self.call_timeout is None: + return func(*args, **kwargs) + # NOT a `with` block: ThreadPoolExecutor.__exit__ shuts down with + # wait=True, which would block on the very call that timed out. + pool = concurrent.futures.ThreadPoolExecutor(max_workers=1) + try: + future = pool.submit(func, *args, **kwargs) + try: + return future.result(timeout=self.call_timeout) + except concurrent.futures.TimeoutError: + raise TimeoutError( + f"provider {type(self.provider).__name__}." + f"{getattr(func, '__name__', func)}() did not answer " + f"within {self.call_timeout}s" + ) from None + finally: + pool.shutdown(wait=False) # ------------------------------------------------------------------ # Constructors @@ -75,7 +106,9 @@ def __init__(self, provider: Any, api: Any = None, @classmethod def from_class(cls, provider_cls: Any, config: Optional[dict] = None, - mock_api: Any = None, api_attr: str = "_api") -> "MediaProviderHarness": + mock_api: Any = None, api_attr: str = "_api", + call_timeout: Optional[float] = DEFAULT_CALL_TIMEOUT + ) -> "MediaProviderHarness": """Instantiate ``provider_cls(config)`` and (optionally) inject ``mock_api``. Args: @@ -91,12 +124,14 @@ def from_class(cls, provider_cls: Any, config: Optional[dict] = None, provider = provider_cls(config or {}) if mock_api is not None: setattr(provider, api_attr, mock_api) - return cls(provider, api=mock_api) + return cls(provider, api=mock_api, call_timeout=call_timeout) @classmethod def from_entrypoint(cls, name: str, config: Optional[dict] = None, group: str = DEFAULT_GROUP, mock_api: Any = None, - api_attr: str = "_api") -> "MediaProviderHarness": + api_attr: str = "_api", + call_timeout: Optional[float] = DEFAULT_CALL_TIMEOUT + ) -> "MediaProviderHarness": """Discover the provider through its installed entry-point and wrap it. Resolves ``name`` in the ``group`` entry-point group (default @@ -117,7 +152,8 @@ def from_entrypoint(cls, name: str, config: Optional[dict] = None, ) provider_cls = matches[0].load() harness = cls.from_class(provider_cls, config=config, - mock_api=mock_api, api_attr=api_attr) + mock_api=mock_api, api_attr=api_attr, + call_timeout=call_timeout) harness.entrypoint_name = name harness.entrypoint_group = group return harness @@ -128,24 +164,25 @@ def from_entrypoint(cls, name: str, config: Optional[dict] = None, def is_available(self) -> bool: """Provider self-check (server reachable / keys present).""" - return self.provider.is_available() + return self._call(self.provider.is_available) def serves(self, signals: Any, context: Any = None) -> bool: """Context-aware routing gate (three-axis ``matches`` + device/policy).""" - return self.provider.serves(signals, context) + return self._call(self.provider.serves, signals, context) def search(self, signals: Any, lang: str = "en-us") -> List[Any]: """Raw search — may raise, mirroring a direct provider call.""" - return self.provider.search(signals, lang=lang) + return self._call(self.provider.search, signals, lang=lang) def search_safe(self, signals: Any, context: Any = None, lang: str = "en-us") -> List[Any]: """The never-raising entry the pipeline's thread-pool dispatch calls.""" - return self.provider.search_safe(signals, context=context, lang=lang) + return self._call(self.provider.search_safe, signals, + context=context, lang=lang) def featured_media(self, lang: str = "en-us") -> List[Any]: """Curated/home content (recently-played, recommendations, …).""" - return self.provider.featured_media(lang=lang) + return self._call(self.provider.featured_media, lang=lang) # ------------------------------------------------------------------ # Assertions diff --git a/ovoscope/ocp.py b/ovoscope/ocp.py index 826bfd5..011ca8b 100644 --- a/ovoscope/ocp.py +++ b/ovoscope/ocp.py @@ -38,6 +38,8 @@ """ from __future__ import annotations +import json +import threading import time from dataclasses import dataclass, field from typing import Any, Dict, List, Optional @@ -110,25 +112,43 @@ def execute(self) -> List[Message]: from ovoscope import get_minicroft # avoid circular at module level captured: List[Message] = [] + got_response = threading.Event() mc = get_minicroft(self.skill_ids, lang=self.lang, max_wait=60, modernize=self.modernize, emit_legacy=self.emit_legacy) - mc.bus.on("message", lambda m: captured.append( - Message.deserialize(m) if isinstance(m, str) else m - )) + + def _capture(m: Any) -> None: + captured.append(Message.deserialize(m) if isinstance(m, str) else m) + + def _on_query_response(_m: Any) -> None: + got_response.set() + + mc.bus.on("message", _capture) + mc.bus.on("ovos.common_play.query.response", _on_query_response) src_msg = Message( "recognizer_loop:utterance", data={"utterances": [self.utterance], "lang": self.lang}, ) - patches = self._build_patches() - with _apply_patches(patches): - mc.bus.emit(src_msg) - time.sleep(self.timeout * 0.5) # wait for async OCP responses - - mc.stop() + # MiniCroft.stop() restores process-wide globals — it must run even if + # emit or a patch raises. + try: + patches = self._build_patches() + with _apply_patches(patches): + mc.bus.emit(src_msg) + # Wait for the OCP skills to answer instead of sleeping a fixed + # fraction of the timeout: fast skills no longer pay the full + # wait, slow ones are no longer cut off early. + if got_response.wait(self.timeout): + # Several OCP skills answer the same query; give the + # stragglers a short grace period after the first reply. + time.sleep(min(0.5, self.timeout * 0.1)) + finally: + mc.bus.remove("ovos.common_play.query.response", _on_query_response) + mc.bus.remove("message", _capture) + mc.stop() assert_ocp_query_response( captured, @@ -147,14 +167,59 @@ def _build_patches(self) -> List[Any]: if not self.mock_responses: return patches - mock_response = _build_mock_response(self.mock_responses) - targets = list(self.patch_targets) + ["requests.Session.get", "requests.get"] for target in targets: - patches.append(patch(target, return_value=mock_response)) + # The side effect must live on the patched GET — that is what + # receives the URL. Attaching it to the response mock could never + # see a URL, so no configured body ever matched and json() always + # returned {}. + patches.append(patch(target, + side_effect=_build_get_side_effect( + self.mock_responses))) return patches +def _build_get_side_effect(mock_responses: Dict[str, Any]): + """Build a side effect for a patched ``requests`` GET. + + The returned callable receives the request URL (the first positional + argument of ``requests.get`` / ``requests.Session.get``) and answers with a + response mock whose ``json()`` returns the body configured for the first + *mock_responses* key found in that URL. + + Args: + mock_responses: URL-substring → response body mapping. + + Returns: + A callable suitable for ``patch(..., side_effect=...)``. + """ + + def _get(*args: Any, **kwargs: Any) -> MagicMock: + url = "" + for candidate in args: + if isinstance(candidate, str): + url = candidate + break + else: + url = str(kwargs.get("url", "")) + + body: Any = {} + for key, value in mock_responses.items(): + if key in url: + body = value + break + + response = MagicMock() + response.url = url + response.json.return_value = body + response.text = json.dumps(body) if isinstance(body, (dict, list)) else str(body) + response.status_code = 200 + response.ok = True + return response + + return _get + + def _build_mock_response(mock_responses: Dict[str, Any]) -> MagicMock: """Create a mock ``requests.Response`` that returns configured JSON bodies. diff --git a/ovoscope/phal.py b/ovoscope/phal.py index 156ac90..44eae9a 100644 --- a/ovoscope/phal.py +++ b/ovoscope/phal.py @@ -66,6 +66,9 @@ class MiniPHAL: emit_legacy: FakeBus also emits the legacy topic when an ovos.* spec topic is emitted (spec producer -> legacy listener). Set both False to exercise a single namespace with no bridging. + tolerate_load_errors: When False (default), a plugin that fails to load + raises immediately. When True, the failure is warned about, kept in + :attr:`load_errors`, and quoted in ``assert_emitted`` failures. Example:: @@ -96,6 +99,7 @@ def __init__( config: Optional[Dict[str, Dict[str, Any]]] = None, modernize: bool = True, emit_legacy: bool = True, + tolerate_load_errors: bool = False, ) -> None: self.plugin_ids: List[str] = plugin_ids or [] self.plugin_instances: Dict[str, Any] = plugin_instances or {} @@ -104,8 +108,11 @@ def __init__( self.modernize: bool = modernize self.emit_legacy: bool = emit_legacy self._bus: FakeBus = FakeBus(modernize=modernize, emit_legacy=emit_legacy) + self.tolerate_load_errors: bool = tolerate_load_errors self._captured: List[Message] = [] self._loaded: Dict[str, Any] = {} + # (plugin_id, error text) for every plugin that failed to load + self.load_errors: List[tuple] = [] # ------------------------------------------------------------------ # Context manager interface @@ -118,13 +125,22 @@ def __enter__(self) -> "MiniPHAL": return self def __exit__(self, *_: Any) -> None: - """Shut down all loaded plugins and close the bus.""" - for plugin in self._loaded.values(): + """Shut down all loaded plugins, detach the capture handler, close the bus.""" + try: + for plugin in self._loaded.values(): + try: + plugin.shutdown() + except Exception: + pass + finally: try: - plugin.shutdown() + self._bus.remove("message", self._capture) + except Exception: + pass + try: + self._bus.close() except Exception: pass - self._bus.remove("message", self._capture) # ------------------------------------------------------------------ # Internal helpers @@ -140,17 +156,25 @@ def _capture(self, message: Any) -> None: self._captured.append(message) def _load_plugins(self) -> None: - """Load PHAL plugins via factories, pre-built instances, or OPM.""" + """Load PHAL plugins via factories, pre-built instances, or OPM. + + A load failure RAISES by default. Degrading it to a warning only moves + the failure: the plugin is silently absent and resurfaces much later as + an unrelated ``assert_emitted`` timeout. Set + ``tolerate_load_errors=True`` to keep the old behaviour; the errors are + then recorded in :attr:`load_errors` and quoted in assertion failures. + + Raises: + RuntimeError: If any plugin fails to load and + ``tolerate_load_errors`` is False. + """ for plugin_id in self.plugin_ids: if plugin_id in self.plugin_factories: try: instance = self.plugin_factories[plugin_id](self._bus) except Exception as exc: - import warnings - warnings.warn( - f"Factory for PHAL plugin {plugin_id!r} raised: {exc}", - stacklevel=2, - ) + self._record_load_error( + plugin_id, f"factory raised: {exc}") instance = None elif plugin_id in self.plugin_instances: instance = self.plugin_instances[plugin_id] @@ -159,6 +183,20 @@ def _load_plugins(self) -> None: if instance is not None: self._loaded[plugin_id] = instance + def _record_load_error(self, plugin_id: str, reason: str) -> None: + """Record — or raise on — a plugin load failure.""" + self.load_errors.append((plugin_id, reason)) + if not self.tolerate_load_errors: + raise RuntimeError( + f"PHAL plugin {plugin_id!r} failed to load: {reason}. " + f"Pass tolerate_load_errors=True to continue without it." + ) + import warnings + warnings.warn( + f"PHAL plugin {plugin_id!r} failed to load: {reason}", + stacklevel=3, + ) + def _instantiate_plugin(self, plugin_id: str) -> Optional[Any]: """Instantiate a PHAL plugin by its OPM entry-point ID. @@ -174,11 +212,7 @@ def _instantiate_plugin(self, plugin_id: str) -> Optional[Any]: plugin = PHALPlugin(bus=self._bus, config=cfg, plugin_id=plugin_id) return plugin except Exception as exc: - import warnings - warnings.warn( - f"Failed to load PHAL plugin {plugin_id!r}: {exc}", - stacklevel=2, - ) + self._record_load_error(plugin_id, str(exc)) return None # ------------------------------------------------------------------ @@ -218,9 +252,17 @@ def assert_emitted(self, msg_type: str, timeout: float = 2.0) -> Message: return msg time.sleep(0.05) captured_types = [m.msg_type for m in self._captured] + hint = "" + if self.load_errors: + # A tolerated load failure is the usual cause of this timeout — + # name it instead of leaving the caller to guess. + hint = ( + f" NOTE: {len(self.load_errors)} PHAL plugin(s) failed to load " + f"and were tolerated: {self.load_errors}" + ) raise AssertionError( f"Expected message type {msg_type!r} was not emitted within {timeout}s. " - f"Captured: {captured_types}" + f"Captured: {captured_types}.{hint}" ) def assert_not_emitted(self, msg_type: str, wait: float = 0.2) -> None: @@ -264,6 +306,9 @@ class PHALTest: modernize: Forwarded to :class:`MiniPHAL` — FakeBus bridges legacy->spec. emit_legacy: Forwarded to :class:`MiniPHAL` — FakeBus bridges spec->legacy. Set both False to exercise a single namespace with no bridging. + tolerate_load_errors: Forwarded to :class:`MiniPHAL`. False (default) + makes a plugin load failure raise instead of surfacing later as an + unrelated ``assert_emitted`` timeout. Example:: @@ -288,6 +333,7 @@ class PHALTest: timeout: float = 5.0 modernize: bool = True emit_legacy: bool = True + tolerate_load_errors: bool = False def execute(self) -> List[Message]: """Run the test: load plugins, emit trigger, assert expectations. @@ -305,6 +351,7 @@ def execute(self) -> List[Message]: config=self.config, modernize=self.modernize, emit_legacy=self.emit_legacy, + tolerate_load_errors=self.tolerate_load_errors, ) as phal: phal.emit(self.trigger_message, wait=0.1) diff --git a/ovoscope/pipeline.py b/ovoscope/pipeline.py index de21cb2..caa5361 100644 --- a/ovoscope/pipeline.py +++ b/ovoscope/pipeline.py @@ -34,6 +34,33 @@ from ovos_utils.messagebus import Message +@dataclass +class MatchResult: + """Discriminated outcome of a single :meth:`PipelineHarness.match_result`. + + Attributes: + outcome: One of ``"matched"``, ``"no_match"`` or ``"timeout"``. + ``"no_match"`` means the pipeline explicitly reported an intent + failure. ``"timeout"`` means nothing came back at all — the + pipeline gave no verdict, which is a harness problem and must not + be read as "no match". + message: The matched :class:`Message`, or ``None``. + """ + + outcome: str + message: Optional[Message] = None + + @property + def matched(self) -> bool: + """True only when the pipeline produced a match.""" + return self.outcome == "matched" + + @property + def timed_out(self) -> bool: + """True when the pipeline gave no verdict within the timeout.""" + return self.outcome == "timeout" + + class _SinkSkill: """Internal catch-all fallback skill so matched intents have somewhere to route. @@ -85,7 +112,11 @@ def _handle_failure(self, message: Any) -> None: message = Message.deserialize(message) except Exception: return - # Failures are tracked by absence of _last_match. + # An explicit failure CLEARS the previous match. Without this, a + # match -> failure -> ... sequence leaves the first match visible on + # `_last_match`, so anything reading it sees a verdict from an earlier + # utterance. + self._last_match = None class PipelineHarness: @@ -129,6 +160,7 @@ def __init__( self.modernize: bool = modernize self.emit_legacy: bool = emit_legacy self._mc: Any = None + self._sink: Any = None # ------------------------------------------------------------------ # Context manager interface @@ -152,9 +184,16 @@ def __enter__(self) -> "PipelineHarness": emit_legacy=self.emit_legacy, ) - # Update sink skill's bus reference now that MiniCroft is created - if self._mc is not None: - sink_skill.bus = self._mc.bus + # MiniCroft patches process-wide globals that only stop() restores, so + # anything that raises after a successful boot must still shut it down. + try: + # Update sink skill's bus reference now that MiniCroft is created + if self._mc is not None: + sink_skill.bus = self._mc.bus + self._sink = sink_skill + except BaseException: + self.__exit__(None, None, None) + raise return self @@ -163,33 +202,47 @@ def __exit__(self, *_: Any) -> None: if self._mc is not None: self._mc.stop() self._mc = None + self._sink = None # ------------------------------------------------------------------ # Public API # ------------------------------------------------------------------ - def match(self, utterance: str, timeout: float = 5.0) -> Optional[Message]: - """Send *utterance* through the pipeline and return the matched message. + def match_result(self, utterance: str, timeout: float = 5.0) -> "MatchResult": + """Send *utterance* through the pipeline and report what happened. + + Unlike :meth:`match`, this distinguishes the three outcomes a caller + must tell apart: a match, an explicit intent failure, and a timeout + (nothing at all came back — usually a broken harness, not a genuine + "no match"). Args: utterance: Text utterance to send. - timeout: Seconds to wait for a match (default 5.0). + timeout: Seconds to wait for a verdict (default 5.0). Returns: - The ``recognizer_loop:utterance`` response message if a match occurs, - otherwise ``None``. + A :class:`MatchResult`. """ if self._mc is None: raise RuntimeError("PipelineHarness must be used as a context manager.") + # Clear the previous utterance's verdict so a stale match can never be + # read as this utterance's result. + if self._sink is not None: + self._sink._last_match = None + import threading captured: List[Message] = [] - _matched = threading.Event() + lock = threading.Lock() + done = threading.Event() _failed = threading.Event() success_type = "intent.service.skills.activated" - failure_types = ["intent_failure", "mycroft.skill.handler.start"] + # NOTE: `mycroft.skill.handler.start` is NOT a failure — it fires on a + # SUCCESSFUL match, right before the skill handler runs. Treating it as + # one made every successful match report "no match". + failure_types = ["intent_failure", "complete_intent_failure"] def _on_success(msg: Any) -> None: if isinstance(msg, str): @@ -197,46 +250,56 @@ def _on_success(msg: Any) -> None: msg = Message.deserialize(msg) except Exception: return - captured.append(msg) - _matched.set() + with lock: + captured.append(msg) + done.set() def _on_failure(msg: Any) -> None: _failed.set() + done.set() self._mc.bus.on(success_type, _on_success) for et in failure_types: self._mc.bus.on(et, _on_failure) - src = Message( - "recognizer_loop:utterance", - data={"utterances": [utterance], "lang": self.lang}, - ) - self._mc.bus.emit(src) - - # Wait for either a match or a failure signal - import threading as _threading - done = _threading.Event() - - def _wait_either() -> None: - while not _matched.is_set() and not _failed.is_set(): - _matched.wait(timeout=0.05) - if _matched.is_set() or _failed.is_set(): - break - done.set() + try: + src = Message( + "recognizer_loop:utterance", + data={"utterances": [utterance], "lang": self.lang}, + ) + self._mc.bus.emit(src) + # Wait directly on the shared event — no watcher thread. The old + # watcher polled at 20Hz forever after a timeout, because the + # handlers were already removed so its events could never be set. + completed = done.wait(timeout=timeout) + finally: + self._mc.bus.remove(success_type, _on_success) + for et in failure_types: + self._mc.bus.remove(et, _on_failure) + + with lock: + got = captured[0] if captured else None + if got is not None: + # A real match wins over a concurrent failure signal. + return MatchResult(outcome="matched", message=got) + if _failed.is_set(): + return MatchResult(outcome="no_match", message=None) + if not completed: + return MatchResult(outcome="timeout", message=None) + return MatchResult(outcome="no_match", message=None) - watcher = _threading.Thread(target=_wait_either, daemon=True) - watcher.start() - timed_out = not done.wait(timeout=timeout) + def match(self, utterance: str, timeout: float = 5.0) -> Optional[Message]: + """Send *utterance* through the pipeline and return the matched message. - self._mc.bus.remove(success_type, _on_success) - for et in failure_types: - self._mc.bus.remove(et, _on_failure) + Args: + utterance: Text utterance to send. + timeout: Seconds to wait for a match (default 5.0). - if timed_out: - return None - if _failed.is_set(): - return None - return captured[0] if captured else None + Returns: + The ``recognizer_loop:utterance`` response message if a match occurs, + otherwise ``None``. + """ + return self.match_result(utterance, timeout=timeout).message def assert_matches( self, @@ -258,7 +321,12 @@ def assert_matches( Raises: AssertionError: If no match is found or the intent type is wrong. """ - msg = self.match(utterance, timeout=timeout) + result = self.match_result(utterance, timeout=timeout) + assert result.outcome != "timeout", ( + f"Pipeline gave no verdict for utterance {utterance!r} within " + f"{timeout}s — neither a match nor an intent failure was emitted." + ) + msg = result.message assert msg is not None, ( f"Expected utterance {utterance!r} to be matched by the pipeline, but no match occurred." ) @@ -276,9 +344,19 @@ def assert_no_match(self, utterance: str, timeout: float = 2.0) -> None: timeout: Seconds to observe before asserting absence (default 2.0). Raises: - AssertionError: If a match is unexpectedly found. + AssertionError: If a match is unexpectedly found, or if the + pipeline gave no verdict at all within *timeout* (silence is + a broken harness, not proof of absence). """ - msg = self.match(utterance, timeout=timeout) + result = self.match_result(utterance, timeout=timeout) + if result.outcome == "timeout": + raise AssertionError( + f"Pipeline gave no verdict for utterance {utterance!r} within " + f"{timeout}s — no match AND no intent failure was emitted. " + f"Absence of a match cannot be asserted from silence; check " + f"that the harness is wired and the pipeline is loaded." + ) + msg = result.message if msg is not None and msg.msg_type != "intent_failure": raise AssertionError( f"Utterance {utterance!r} was unexpectedly matched: {msg.msg_type!r}" diff --git a/ovoscope/pydantic_helpers.py b/ovoscope/pydantic_helpers.py index fa90bd3..3c74fcb 100644 --- a/ovoscope/pydantic_helpers.py +++ b/ovoscope/pydantic_helpers.py @@ -85,7 +85,11 @@ def to_bus_message(pydantic_msg: "OpenVoiceOSMessage") -> Message: from ovos_pydantic_models import SpeakMessage, SpeakData bus_msg = to_bus_message(SpeakMessage(data=SpeakData(utterance="Hello!"))) - assert bus_msg.msg_type == "speak" + # Accepts both the legacy "speak" and canonical "ovos.utterance.speak" + # spellings (producers emit canonical since workshop#425; captured + # streams from pre-spec producer vintages can still carry the legacy + # name). + assert bus_msg.msg_type in {"speak", "ovos.utterance.speak"} assert bus_msg.data["utterance"] == "Hello!" """ _require_pydantic() @@ -179,6 +183,10 @@ def validate_fixture(path: Union[str, Path]) -> "SerializedTest": for section in ("source_message", "expected_messages"): msgs = data.get(section, []) # type: ignore[union-attr] + if isinstance(msgs, dict): + # legacy fixtures stored a single message object here; the + # current schema (End2EndTest.serialize) always writes a list + msgs = [msgs] for i, raw in enumerate(msgs): # Fixtures use the Message.serialize() "type" key; pydantic models # expect "message_type". Accept either form. Use None (not "") diff --git a/ovoscope/pytest_plugin.py b/ovoscope/pytest_plugin.py index 7f51d1a..c2d89a1 100644 --- a/ovoscope/pytest_plugin.py +++ b/ovoscope/pytest_plugin.py @@ -736,13 +736,75 @@ def _accuracy_markdown(summary, results, baseline_diff=None, top_n=10): return "\n".join(lines).rstrip() + "\n" +def _gate_state(config): + """Compute (and cache) the accuracy summary, baseline diff, and gate result. + + Called from ``pytest_sessionfinish`` — which runs BEFORE + ``pytest_terminal_summary`` — so the exit status can be set from the same + numbers the summary prints. The result is cached on *config*, so the + summary reuses it instead of re-reading the baseline file. + + Returns: + Dict with ``summary``, ``baseline_diff``, ``baseline_warning`` and + ``failures``, or ``None`` when no intent-case results were collected. + """ + cached = getattr(config, "_ovoscope_gate_state", None) + if cached is not None: + return cached + + accum = getattr(pytest_runtest_logreport, "_accum", None) + if not accum or not accum["results"]: + return None + summary = _accuracy_summary(accum["results"]) + + baseline_path = config.getoption("--ovoscope-accuracy-baseline") + baseline_diff = None + baseline_warning = None + if baseline_path: + try: + import json as _json + with open(baseline_path, "r", encoding="utf-8") as fh: + baseline_doc = _json.load(fh) + baseline_diff = _baseline_diff( + baseline_doc.get("results") or [], + accum["results"]) + except Exception as exc: + baseline_warning = (f"could not read baseline " + f"{baseline_path}: {exc}") + + min_acc = config.getoption("--ovoscope-accuracy-min") + failures = [] + if min_acc is not None and summary["overall_accuracy"] < min_acc: + failures.append( + f"overall accuracy {summary['overall_accuracy']:.1%} < " + f"required {min_acc:.1%}") + if baseline_diff is not None and baseline_diff["regressed"]: + top_regression = baseline_diff["regressed"][0] + failures.append( + f"{len(baseline_diff['regressed'])} cases regressed vs " + f"baseline (first: `{top_regression['pipeline']}` / " + f"`{top_regression['lang']}` / " + f"`{top_regression['intent']}` / " + f"{top_regression['utterance']!r})") + + state = {"summary": summary, + "baseline_diff": baseline_diff, + "baseline_warning": baseline_warning, + "failures": failures} + config._ovoscope_gate_state = state + return state + + def pytest_terminal_summary(terminalreporter, exitstatus, config): # noqa: ARG001 """Combined session summary: bus coverage + intent-case accuracy.""" _bus_coverage_summary(terminalreporter, config) accum = getattr(pytest_runtest_logreport, "_accum", None) if not accum or not accum["results"]: return - summary = _accuracy_summary(accum["results"]) + state = _gate_state(config) + summary = state["summary"] + baseline_diff = state["baseline_diff"] + baseline_warning = state["baseline_warning"] tr = terminalreporter tr.write_sep("=", "ovoscope Intent-Case Accuracy") @@ -767,23 +829,8 @@ def pytest_terminal_summary(terminalreporter, exitstatus, config): # noqa: ARG0 ratio = d["pass"] / d["total"] if d["total"] else 0.0 tr.write_line(f" {key:48s} {d['pass']:4d}/{d['total']:<4d} {ratio:>6.1%}") - # Load baseline (if any) and compute structural diff up-front so both - # the JSON / Markdown outputs and the gate can reuse it. - baseline_path = config.getoption("--ovoscope-accuracy-baseline") - baseline_diff = None - baseline_warning = None - if baseline_path: - try: - import json as _json - with open(baseline_path, "r", encoding="utf-8") as fh: - baseline_doc = _json.load(fh) - baseline_diff = _baseline_diff( - baseline_doc.get("results") or [], - accum["results"]) - except Exception as exc: - baseline_warning = (f"could not read baseline " - f"{baseline_path}: {exc}") - tr.write_line(f"\nWARNING: {baseline_warning}") + if baseline_warning: + tr.write_line(f"\nWARNING: {baseline_warning}") # Persist JSON. report_path = config.getoption("--ovoscope-accuracy-report") @@ -820,31 +867,22 @@ def pytest_terminal_summary(terminalreporter, exitstatus, config): # noqa: ARG0 fh.write(md) tr.write_line(f"Wrote accuracy markdown -> {md_path}") - # Gate the session. - min_acc = config.getoption("--ovoscope-accuracy-min") - failures = [] - if min_acc is not None and summary["overall_accuracy"] < min_acc: - failures.append( - f"overall accuracy {summary['overall_accuracy']:.1%} < " - f"required {min_acc:.1%}") - if baseline_diff is not None: - if baseline_diff["regressed"]: - top_regression = baseline_diff["regressed"][0] - failures.append( - f"{len(baseline_diff['regressed'])} cases regressed vs " - f"baseline (first: `{top_regression['pipeline']}` / " - f"`{top_regression['lang']}` / " - f"`{top_regression['intent']}` / " - f"{top_regression['utterance']!r})") - if failures: + # Report the gate result computed in pytest_sessionfinish. + if state["failures"]: tr.write_sep("!", "ovoscope accuracy gate FAILED") - for f in failures: + for f in state["failures"]: tr.write_line(f" - {f}") - config._ovoscope_accuracy_gate_failed = True def pytest_sessionfinish(session, exitstatus): # noqa: ARG001 - """Propagate accuracy-gate failure as a non-zero exit status.""" - if getattr(session.config, "_ovoscope_accuracy_gate_failed", False): + """Evaluate the accuracy gate and propagate it as a non-zero exit status. + + The gate MUST be computed here, not in ``pytest_terminal_summary``: + sessionfinish runs first, so a flag set by the summary would always be + read too late and the gate would never change the exit status. + """ + state = _gate_state(session.config) + if state and state["failures"]: + session.config._ovoscope_accuracy_gate_failed = True if session.exitstatus == 0: session.exitstatus = 1 diff --git a/ovoscope/remote_recorder.py b/ovoscope/remote_recorder.py index 55089cd..263a296 100644 --- a/ovoscope/remote_recorder.py +++ b/ovoscope/remote_recorder.py @@ -100,6 +100,13 @@ def connect(self) -> None: deadline = time.monotonic() + 10.0 while not self._client.connected_event.is_set(): if time.monotonic() > deadline: + # Close the client before giving up — MessageBusClient keeps a + # reconnect thread alive forever otherwise. + client, self._client = self._client, None + try: + client.close() + except Exception: + pass raise ConnectionError(f"Could not connect to {self.bus_url} within 10 seconds.") time.sleep(0.1) diff --git a/ovoscope/setup_skill.py b/ovoscope/setup_skill.py index 9dba848..1da5667 100644 --- a/ovoscope/setup_skill.py +++ b/ovoscope/setup_skill.py @@ -58,19 +58,25 @@ #: Docs files to download into ``assets/docs/``. _DOCS_FILES = [ "docs/audio-testing.md", + "docs/bus-coverage.md", "docs/capture-session.md", "docs/ci-integration.md", "docs/cli.md", + "docs/e2e-pipeline-harness.md", "docs/end2end-test.md", "docs/gui-testing.md", "docs/index.md", + "docs/intent-cases.md", "docs/listener.md", + "docs/media-provider-testing.md", + "docs/media-testing.md", "docs/minicroft.md", "docs/ocp.md", "docs/phal.md", "docs/pipeline.md", "docs/pydantic-integration.md", "docs/usage-guide.md", + "docs/voice-loop.md", ] #: Root-level files to download into ``assets/``. @@ -158,7 +164,9 @@ def _install_skill( verbose: Print progress messages. Returns: - True on success. + True on success, False when SKILL.md could not be downloaded — the + skill is useless without it, so the caller must report the failure + instead of leaving a half-installed directory behind. """ scripts_dir = skill_dir / "scripts" scripts_dir.mkdir(parents=True, exist_ok=True) @@ -167,8 +175,12 @@ def _install_skill( skill_md = skill_dir / "SKILL.md" if verbose: print(f"[{tool_name}] downloading SKILL.md …") - _fetch(_SKILL_MD_URL, skill_md, verbose=False) - if verbose and skill_md.exists(): + if not _fetch(_SKILL_MD_URL, skill_md, verbose=False): + print(f"[{tool_name}] ERROR: could not download SKILL.md from " + f"{_SKILL_MD_URL} — the skill was NOT installed.", + file=sys.stderr) + return False + if verbose: print(f"[{tool_name}] SKILL.md → {skill_md}") # Wrapper script — generated inline @@ -415,10 +427,15 @@ def main(argv: Optional[List[str]] = None) -> int: uninstall_gemini(project_path) return 0 + ok = True if args.claude: - install_claude(fetch_docs=fetch_docs) + ok = install_claude(fetch_docs=fetch_docs) and ok if args.gemini: - install_gemini(project_path, fetch_docs=fetch_docs) + ok = install_gemini(project_path, fetch_docs=fetch_docs) and ok + + if not ok: + print("\nInstallation FAILED. See the errors above.", file=sys.stderr) + return 1 print( "\nInstallation complete. Restart your AI assistant or open a new\n" diff --git a/ovoscope/simple_listener.py b/ovoscope/simple_listener.py index 3ea2d34..bfb4886 100644 --- a/ovoscope/simple_listener.py +++ b/ovoscope/simple_listener.py @@ -38,6 +38,7 @@ from ovos_bus_client.message import Message from ovos_utils.fakebus import FakeBus +from ovos_utils.log import LOG from ovoscope.voice_loop import ( ListenerHarness, @@ -144,7 +145,10 @@ def __init__( wakeword = MockHotWordEngine("hey_mycroft", trigger_after=2) self.callbacks = _SimpleBusCallbacks(self.bus) - self.listener = SimpleListener( + self._listener_cls = SimpleListener + # Kept so a wedged listener thread can be replaced with a fresh object + # instead of being reused (see feed_file). + self._listener_kwargs = dict( wakeword=wakeword, mic=None, # supplied per-run by feed_file vad=vad_instance if vad_instance is not None else MockVADEngine(), @@ -154,6 +158,11 @@ def __init__( max_speech_seconds=max_speech_seconds, callbacks=self.callbacks, ) + self.listener = self._listener_cls(**self._listener_kwargs) + + def _new_listener(self) -> Any: + """Build a fresh listener object from the original constructor args.""" + return self._listener_cls(**self._listener_kwargs) def feed_file( self, @@ -193,6 +202,16 @@ def feed_file( finally: self.listener.stop() self.listener.join(timeout=2.0) + if self.listener.is_alive(): + # The thread refused to die. Reusing it would let it keep + # appending to `_messages` during the NEXT run. Abandon it + # (it is a daemon of the test process) and start clean. + LOG.warning( + "MiniSimpleListener: listener thread still alive 2s after " + "stop(); abandoning it and building a fresh listener for " + "the next run to avoid cross-run message pollution." + ) + self.listener = self._new_listener() self._last_messages = list(self._messages) return list(self._messages) diff --git a/ovoscope/tts_intelligibility.py b/ovoscope/tts_intelligibility.py index 826b6f9..8e2ac0c 100644 --- a/ovoscope/tts_intelligibility.py +++ b/ovoscope/tts_intelligibility.py @@ -57,6 +57,20 @@ _NORMALIZER: Optional[Any] = None +def _utt_slug(utterance: str) -> str: + """Stable, collision-resistant filename stem for an utterance. + + ``hash()`` is randomised per process (PYTHONHASHSEED) and truncating it to + 32 bits collides at a few tens of thousands of utterances — two different + utterances then write to the same file and the second scores the first's + audio. A sha1 prefix is stable across runs and collision-free at any corpus + size a test suite will reach. + """ + import hashlib + + return hashlib.sha1(utterance.encode("utf-8")).hexdigest()[:16] + + def get_reference_stt() -> Any: """Return a lazily-instantiated faster-whisper ``tiny`` reference STT. @@ -149,15 +163,21 @@ class UtteranceScore: wav_path: Path to the captured rendered WAV (may be None on failure). lang: BCP-47 language tag used for synthesis and scoring. voice: Voice identifier used, if any. + transcribe_failed: True when the reference STT itself failed, so + ``transcript`` is None and the 1.0 error rates say nothing about + the TTS engine. Read this before treating a score as a TTS defect. + transcribe_error: The STT failure message, when there was one. """ utterance: str - transcript: str + transcript: Optional[str] wer: float cer: float wav_path: Optional[str] = None lang: str = "en-US" voice: Optional[str] = None + transcribe_failed: bool = False + transcribe_error: Optional[str] = None def to_dict(self) -> dict: """Return a JSON-serialisable dict of this score.""" @@ -169,6 +189,8 @@ def to_dict(self) -> dict: "wav_path": self.wav_path, "lang": self.lang, "voice": self.voice, + "transcribe_failed": self.transcribe_failed, + "transcribe_error": self.transcribe_error, } @@ -314,7 +336,7 @@ def _render_direct(self, utterance: str) -> Optional[str]: """ ext = (getattr(self.tts, "audio_ext", "wav") or "wav").lstrip(".") out_path = os.path.join( - self._tmpdir, f"direct_{abs(hash(utterance)) & 0xffffffff}.{ext}" + self._tmpdir, f"direct_{_utt_slug(utterance)}.{ext}" ) self.tts.get_tts(utterance, out_path, lang=self.lang, voice=self.voice) return out_path if os.path.isfile(out_path) else None @@ -425,14 +447,23 @@ def score_one(self, utterance: str) -> UtteranceScore: wav_path = None transcript = "" + transcribe_failed = False + transcribe_error = None if wav_path and os.path.isfile(wav_path): try: transcript = self._transcribe(wav_path) - except Exception: - transcript = "" + except Exception as exc: + # A reference-STT crash is NOT evidence that the TTS is + # unintelligible. Silently scoring wer=1.0 turns a broken STT + # into a fake TTS regression, so mark the score instead. + LOG.error(f"reference STT failed for {utterance!r} " + f"({wav_path}): {exc}") + transcript = None + transcribe_failed = True + transcribe_error = f"{type(exc).__name__}: {exc}" ref = _normalize(utterance, self.lang) - hyp = _normalize(transcript, self.lang) + hyp = _normalize(transcript or "", self.lang) wer, cer = _score_pair(ref, hyp) return UtteranceScore( utterance=utterance, @@ -442,6 +473,8 @@ def score_one(self, utterance: str) -> UtteranceScore: wav_path=wav_path, lang=self.lang, voice=self.voice, + transcribe_failed=transcribe_failed, + transcribe_error=transcribe_error, ) def score(self, utterances: List[str]) -> IntelligibilityReport: diff --git a/ovoscope/version.py b/ovoscope/version.py index 7a035d0..8402173 100644 --- a/ovoscope/version.py +++ b/ovoscope/version.py @@ -1,7 +1,7 @@ # START_VERSION_BLOCK VERSION_MAJOR = 1 -VERSION_MINOR = 5 -VERSION_BUILD = 0 +VERSION_MINOR = 6 +VERSION_BUILD = 7 VERSION_ALPHA = 1 # END_VERSION_BLOCK diff --git a/ovoscope/voice_loop.py b/ovoscope/voice_loop.py index a46406b..1b60ff4 100644 --- a/ovoscope/voice_loop.py +++ b/ovoscope/voice_loop.py @@ -61,6 +61,8 @@ from ovos_bus_client.message import Message from ovos_utils.fakebus import FakeBus +from ovoscope import _topic_matches + # Re-export the engine mocks so callers have a single import site for the # voice-loop harness. from ovoscope.listener import MockHotWordEngine, MockVADEngine # noqa: F401 @@ -549,7 +551,11 @@ def _build_file_mic( @staticmethod def _has(messages: List[Message], msg_type: str) -> bool: - return any(m.msg_type == msg_type for m in messages) + # Accepts either the legacy or canonical spelling of msg_type — see + # ovoscope._topic_matches: these helpers filter execute()/feed_*()'s + # catch-all-captured stream, which can carry either spelling + # depending on producer vintage. + return any(_topic_matches(m.msg_type, msg_type) for m in messages) def _resolve(self, messages: Optional[List[Message]]) -> List[Message]: return messages if messages is not None else self._last_messages @@ -654,7 +660,7 @@ def assert_utterance_emitted( msgs = self._resolve(messages) utts: List[str] = [] for m in msgs: - if m.msg_type == "recognizer_loop:utterance": + if _topic_matches(m.msg_type, "recognizer_loop:utterance"): utts.extend(m.data.get("utterances", [])) assert utts, ( "Expected 'recognizer_loop:utterance' but it was not emitted. " @@ -677,7 +683,26 @@ def __enter__(self) -> "ListenerHarness": return self def __exit__(self, *_: Any) -> None: - self.shutdown() + try: + self.shutdown() + finally: + self.detach_capture() + + def detach_capture(self) -> None: + """Unsubscribe the wildcard ``"message"`` capture handler. + + The bus may be shared with another harness or with the caller. A + capture handler left behind keeps appending to a dead harness's + message list — and every later message shows up in ``_messages``. + """ + bus = getattr(self, "bus", None) + capture = getattr(self, "_capture", None) + if bus is None or capture is None: + return + try: + bus.remove("message", capture) + except Exception: + pass # --------------------------------------------------------------------------- @@ -928,9 +953,17 @@ def feed_file( # ------------------------------------------------------------------ def shutdown(self) -> None: - """Shut down the hotword container and wrapped engines.""" - if self.hotwords is not None: - self.hotwords.shutdown() + """Shut down the hotword container and detach the bus capture handler. + + Callers that use the harness without the context manager only ever + call shutdown(); if that does not detach the capture handler, the dead + harness keeps collecting every message on a shared bus. + """ + try: + if self.hotwords is not None: + self.hotwords.shutdown() + finally: + self.detach_capture() # --------------------------------------------------------------------------- diff --git a/ovoscope/wakeword_probe.py b/ovoscope/wakeword_probe.py new file mode 100644 index 0000000..a8d6351 --- /dev/null +++ b/ovoscope/wakeword_probe.py @@ -0,0 +1,175 @@ +"""Lightweight per-clip wake-word detection probe. + +Drives a **real** OVOS :class:`HotWordEngine` over a single audio clip the way +the live listening loop does — a few seconds of leading silence to warm the +engine's streaming feature buffers, then the clip streamed frame by frame — +and returns a per-clip detection decision plus latency. + +Unlike :class:`ovoscope.voice_loop.MiniVoiceLoop` (which runs the full +``DinkumVoiceLoop`` state machine and needs the ``[listener]`` extra), this is +self-contained: no bus, no VAD, no state machine — just ``engine.update()`` / +``engine.found_wake_word()`` over primed audio. Ideal for plugin test suites +and benchmarks that score detection on labelled fixtures. + +Why the long lead matters +------------------------- +Streaming detectors (openWakeWord, microWakeWord, …) only emit a prediction +once their rolling mel/embedding window is full (~2.5 s of frames). A clip fed +with too little leading silence never fills that window: the activation is +missed (a false reject), and on the shortest clips the half-full buffer raises +a shape mismatch that drops the sample entirely. Priming with a few seconds of +leading silence fills the window *before* the keyword arrives, exactly as a +live microphone keeps the loop warm. :data:`PRIME_SECONDS` defaults to 3 s. + +Audio contract: mono ``float32`` in ``[-1, 1]`` at the engine's sample rate +(16 kHz for every OVOS hotword engine). Resample upstream if your source +differs. Needs the ``[bench]`` extra (numpy). +""" +from __future__ import annotations + +import inspect +import time +from contextlib import contextmanager +from dataclasses import dataclass +from typing import Any, Dict, Optional + +SAMPLE_RATE = 16000 +FRAME_SAMPLES = 1280 # 80 ms @ 16 kHz — the OVOS listener chunk size +PRIME_SECONDS = 3.0 # leading silence to warm the feature window (see module docstring) +TAIL_SECONDS = 0.5 # trailing silence so a late activation can settle + + +@dataclass +class WakeWordDetection: + """Outcome of running one clip through a hotword engine.""" + + detected: bool + latency_ms: float + frames_to_detection: Optional[int] # frames streamed before the latch fired + + +@contextmanager +def hotword_compat(): + """Widen ``HotWordEngine.__init__`` for the duration of the block. + + Recent wake-word plugins call ``super().__init__(key_phrase, config, lang)``; + older ``HotWordEngine`` bases accept only ``(key_phrase, config)``. Widen the + base signature to ignore the extra argument, then put the original back. + + The patch is on a process-wide base class, so it MUST NOT outlive the + engine construction it exists for: leaving it installed changes how every + later hotword plugin in the process is constructed — including code under + test that is supposed to see the real signature. + + A no-op when the installed base already accepts ``lang``. + """ + from ovos_plugin_manager.templates import hotwords as hw + + base = hw.HotWordEngine + if "lang" in inspect.signature(base.__init__).parameters: + yield + return + _orig = base.__init__ + + def _compat(self, key_phrase="hey_mycroft", config=None, lang=None, + *args, **kwargs): + _orig(self, key_phrase, config) + + base.__init__ = _compat + try: + yield + finally: + base.__init__ = _orig + + +def load_hotword_engine(plugin_id: str, key_phrase: str = "hey_mycroft", + config: Optional[Dict[str, Any]] = None, + lang: str = "en-us"): + """Load and instantiate a real OVOS hotword engine by plugin id. + + Tolerates the ``HotWordEngine(lang)`` signature change and dash/underscore + variation in plugin ids, mirroring how the listening loop resolves engines. + """ + from ovos_plugin_manager.wakewords import load_wake_word_plugin + + with hotword_compat(): + clazz = load_wake_word_plugin(plugin_id) + if clazz is None and "-" in plugin_id: + clazz = load_wake_word_plugin(plugin_id.replace("-", "_")) + if clazz is None: + raise ValueError(f"no wake-word plugin {plugin_id!r}") + return clazz(key_phrase, dict(config or {}), lang) + + +class WakeWordProbe: + """Drive a real ``HotWordEngine`` over single clips with listener-style priming.""" + + def __init__(self, engine, *, sample_rate: int = SAMPLE_RATE, + frame_samples: int = FRAME_SAMPLES, + prime_seconds: float = PRIME_SECONDS, + tail_seconds: float = TAIL_SECONDS): + self.engine = engine + self.sample_rate = sample_rate + self.frame_samples = frame_samples + self.prime_seconds = prime_seconds + self.tail_seconds = tail_seconds + + @classmethod + def from_plugin(cls, plugin_id: str, key_phrase: str = "hey_mycroft", + config: Optional[Dict[str, Any]] = None, + lang: str = "en-us", **kwargs) -> "WakeWordProbe": + """Build a probe from a plugin id (loads + instantiates the engine).""" + engine = load_hotword_engine(plugin_id, key_phrase, config, lang) + return cls(engine, **kwargs) + + def prime_pad(self, array): + """Wrap a clip in leading + trailing silence, padded to whole frames.""" + import numpy as np + + arr = np.asarray(array, dtype="float32") + lead = np.zeros(int(self.sample_rate * self.prime_seconds), dtype="float32") + tail = np.zeros(int(self.sample_rate * self.tail_seconds), dtype="float32") + out = np.concatenate([lead, arr, tail]) + rem = len(out) % self.frame_samples + if rem: + out = np.concatenate( + [out, np.zeros(self.frame_samples - rem, dtype="float32")]) + return out + + @staticmethod + def to_pcm16(array) -> bytes: + """Float32 ``[-1, 1]`` mono array → 16-bit little-endian PCM bytes.""" + import numpy as np + + arr = np.clip(np.asarray(array, dtype="float32"), -1.0, 1.0) + return (arr * 32767.0).astype(" WakeWordDetection: + """Stream one clip through the engine; return the detection decision. + + The OVOS contract is ``update(chunk_bytes)`` to feed audio then + ``found_wake_word()`` to read the latch. Some plugins keep a vestigial + ``found_wake_word(frame_data)`` argument they ignore — we pass the chunk + through so the signature matches either way. + """ + primed = self.prime_pad(array) + if hasattr(self.engine, "reset"): + try: + self.engine.reset() + except Exception: + pass + fww = self.engine.found_wake_word + fww_takes_arg = len(inspect.signature(fww).parameters) >= 1 + has_update = hasattr(self.engine, "update") + pcm = self.to_pcm16(primed) + step = self.frame_samples * 2 # 2 bytes / sample (int16) + start = time.perf_counter() + for i, off in enumerate(range(0, len(pcm), step), 1): + chunk = pcm[off:off + step] + if has_update: + self.engine.update(chunk) + if (fww(chunk) if fww_takes_arg else fww()): + latency = (time.perf_counter() - start) * 1000 + return WakeWordDetection(True, round(latency, 3), i) + latency = (time.perf_counter() - start) * 1000 + return WakeWordDetection(False, round(latency, 3), None) diff --git a/pyproject.toml b/pyproject.toml index 6a12195..d27b5a1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,6 +26,9 @@ dependencies = [ # dependency. >=8 is required: the pytest_pycollect_makemodule hook dropped the # 'path' argument in pytest 8. "pytest>=8", + # coverage.py validates pyproject.toml manifests; stdlib tomllib only + # exists on 3.11+, so 3.10 needs the backport for the same honesty. + "tomli>=2; python_version<'3.11'", ] classifiers = [ "Programming Language :: Python :: 3", @@ -47,6 +50,9 @@ media = ["ovos-media>=0.0.2a3"] # AudioTransformersService. >=0.7.2a1 is the first release that allows # ovos-bus-client 2.x (older pins cap it <2.0.0 and conflict with ovos-core). listener = ["ovos-dinkum-listener>=0.7.2a1"] +# Per-clip WakeWordProbe (ovoscope.wakeword_probe): streams audio arrays through +# a real HotWordEngine. Only needs numpy — the engine itself is environmental. +bench = ["numpy"] # End-to-end TTS intelligibility scoring (WER/CER round-trip via reference STT). # faster-whisper itself is pulled by the plugin — don't list it here to avoid # version skew. @@ -61,6 +67,7 @@ dev = [ "ovos-media>=0.0.2a3", "ovos-dinkum-listener>=0.7.2a1", "ovos-pydantic-models>=0.1.0", + "numpy", "jiwer", "ovos-utterance-normalizer", "ovos-stt-plugin-fasterwhisper", diff --git a/test/unittests/test_audit_round1.py b/test/unittests/test_audit_round1.py new file mode 100644 index 0000000..87c6eab --- /dev/null +++ b/test/unittests/test_audit_round1.py @@ -0,0 +1,721 @@ +"""Adversarial regression tests for the Han audit round 1 fixes. + +Each test here reproduces a defect that leaked state, hung, or reported a +false verdict. They are written to FAIL against the pre-fix code. +""" +import time +import unittest +from unittest.mock import MagicMock, patch + +import pytest + +from ovos_bus_client.message import Message +from ovos_bus_client.session import SessionManager +from ovos_utils.fakebus import FakeBus +from ovos_utils.log import LOG + +import ovoscope +from ovoscope import CaptureSession, End2EndTest, get_minicroft +from ovoscope.bus_coverage import BusCoverageTracker +from ovoscope.pipeline import MatchResult + + +# --------------------------------------------------------------------------- +# A. teardown must run even when an assertion fails +# --------------------------------------------------------------------------- + +@pytest.mark.timeout(600) # boots a real MiniCroft (slow shutdown) +class TestTeardownOnFailure(unittest.TestCase): + """execute() must stop a managed MiniCroft on the failure path too.""" + + def setUp(self): + LOG.set_level("ERROR") + + def tearDown(self): + LOG.set_level("CRITICAL") + + def test_managed_minicroft_stopped_when_assertion_fails(self): + original_bus = SessionManager.bus + original_pipeline = SessionManager.default_session.pipeline[:] + + test = End2EndTest( + skill_ids=[], + source_message=Message("ovoscope.audit.never.answered"), + # deliberately wrong: forces the message-count assertion to fail + expected_messages=[Message("ovoscope.audit.does.not.happen")], + eof_msgs=["ovoscope.audit.eof.never.emitted"], + verbose=False, + ) + with self.assertRaises(AssertionError): + test.execute(timeout=2) + + # The whole point: globals are back even though execute() raised. + self.assertIs(SessionManager.bus, original_bus) + self.assertEqual(SessionManager.default_session.pipeline, original_pipeline) + self.assertIsNone(test.minicroft) + + def test_capture_timeout_is_reported_as_a_timeout(self): + """A timeout must say so, not masquerade as a count mismatch.""" + test = End2EndTest( + skill_ids=[], + source_message=Message("ovoscope.audit.never.answered"), + expected_messages=[], + eof_msgs=["ovoscope.audit.eof.never.emitted"], + test_message_number=False, + verbose=False, + ) + with self.assertRaises(AssertionError) as ctx: + test.execute(timeout=2) + self.assertIn("capture timed out", str(ctx.exception)) + self.assertIn("ovoscope.audit.eof.never.emitted", str(ctx.exception)) + + +# --------------------------------------------------------------------------- +# B. the process-wide default session must survive a test unchanged +# --------------------------------------------------------------------------- + +@pytest.mark.timeout(600) # boots a real MiniCroft (slow shutdown) +class TestDefaultSessionIsolation(unittest.TestCase): + + def setUp(self): + LOG.set_level("ERROR") + + def tearDown(self): + LOG.set_level("CRITICAL") + + def test_inject_active_does_not_leak_into_default_session(self): + before = {s[0] for s in SessionManager.default_session.active_skills} + + test = End2EndTest( + skill_ids=[], + source_message=Message("ovoscope.audit.never.answered"), + expected_messages=[], + eof_msgs=["ovoscope.audit.eof.never.emitted"], + inject_active=["ovoscope-audit-ghost.test"], + test_message_number=False, + verbose=False, + ) + with self.assertRaises(AssertionError): + test.execute(timeout=2) + + after = {s[0] for s in SessionManager.default_session.active_skills} + self.assertEqual(before, after, + "inject_active leaked into the default session") + + def test_stop_restores_default_session_lang_and_pipeline(self): + sess = SessionManager.default_session + original_lang = sess.lang + original_pipeline = sess.pipeline[:] + + mc = get_minicroft([], lang="pt-PT") + try: + self.assertEqual(SessionManager.default_session.lang, "pt-PT") + finally: + mc.stop() + + self.assertEqual(SessionManager.default_session.lang, original_lang) + self.assertEqual(SessionManager.default_session.pipeline, original_pipeline) + + def test_default_session_mutated_mid_test_is_restored(self): + """Even a mutation MiniCroft never made itself must be undone.""" + mc = get_minicroft([]) + try: + SessionManager.default_session.activate_skill("ovoscope-audit-x.test") + finally: + mc.stop() + actives = {s[0] for s in SessionManager.default_session.active_skills} + self.assertNotIn("ovoscope-audit-x.test", actives) + + +# --------------------------------------------------------------------------- +# C. mock-TTS timers must not outlive the MiniCroft +# --------------------------------------------------------------------------- + +@pytest.mark.timeout(600) # boots a real MiniCroft (slow shutdown) +class TestTTSTimerLifecycle(unittest.TestCase): + + def setUp(self): + LOG.set_level("ERROR") + + def tearDown(self): + LOG.set_level("CRITICAL") + + def test_timers_tracked_cancelled_and_silent_after_stop(self): + # One boot covers the whole lifecycle: MiniCroft boots retain several + # hundred MB each even after stop(), so the two assertions share it. + mc = get_minicroft([]) + late = [] + mc.bus.on("recognizer_loop:audio_output_end", lambda m: late.append(m)) + mc.bus.emit(Message("speak", {"utterance": "hello"})) + with mc._tts_timers_lock: + timers = list(mc._tts_timers) + self.assertTrue(timers, "no TTS timer was tracked") + self.assertTrue(all(t.daemon for t in timers), + "TTS timers must be daemon threads") + late.clear() # drop anything emitted before stop() — only post-stop + mc.stop() + + # stop() must have cancelled/joined every one of them. + with mc._tts_timers_lock: + self.assertEqual(mc._tts_timers, []) + self.assertFalse(any(t.is_alive() for t in timers)) + time.sleep(0.4) # well past the 0.1s timer + self.assertEqual(late, [], + "an orphaned TTS timer fired after stop()") + + +# --------------------------------------------------------------------------- +# D. bus-coverage must not inherit earlier tests' invocation counts +# --------------------------------------------------------------------------- + +class TestBusCoverageDelta: + + def setup_method(self, _): + self._prev_flag = ovoscope.GLOBAL_BUS_COVERAGE + self._prev_collector = ovoscope.GLOBAL_BUS_COVERAGE_COLLECTOR + + def teardown_method(self, _): + ovoscope.GLOBAL_BUS_COVERAGE = self._prev_flag + ovoscope.GLOBAL_BUS_COVERAGE_COLLECTOR = self._prev_collector + + def _tracker(self): + bus = FakeBus() + minicroft = MagicMock() + minicroft.plugin_skills = {} + return bus, BusCoverageTracker(bus, minicroft) + + def test_earlier_tests_invocations_are_excluded(self): + """Counts from BEFORE the tracker existed must not inflate the report.""" + ovoscope.GLOBAL_BUS_COVERAGE = True + ovoscope.GLOBAL_BUS_COVERAGE_COLLECTOR = ovoscope.GlobalBusCoverageCollector() + collector = ovoscope.GLOBAL_BUS_COVERAGE_COLLECTOR + # pretend 5 earlier tests each fired this event + for _ in range(5): + collector.record_invocation("shared.event") + + bus, tracker = self._tracker() + tracker.start_tracking() + bus.emit(Message("shared.event")) # 1x during THIS test + tracker.stop_tracking() + tracker._registered = {"__core__": {"shared.event": 1}} + + report = tracker.build_report() + skill = next(s for s in report.skills if s.skill_id == "__core__") + handler = next(h for h in skill.listeners if h.msg_type == "shared.event") + assert handler.invocation_count == 1, ( + "coverage inherited invocations from earlier tests" + ) + + def test_own_boot_is_counted_when_tracker_precedes_boot(self): + ovoscope.GLOBAL_BUS_COVERAGE = True + ovoscope.GLOBAL_BUS_COVERAGE_COLLECTOR = ovoscope.GlobalBusCoverageCollector() + collector = ovoscope.GLOBAL_BUS_COVERAGE_COLLECTOR + collector.record_invocation("boot.event") # an EARLIER test + + bus, tracker = self._tracker() + collector.record_invocation("boot.event") # THIS test's boot + tracker.start_tracking() + tracker.stop_tracking() + tracker._registered = {"__core__": {"boot.event": 1}} + + report = tracker.build_report() + skill = next(s for s in report.skills if s.skill_id == "__core__") + handler = next(h for h in skill.listeners if h.msg_type == "boot.event") + assert handler.invocation_count == 1 + + def test_tracking_window_is_not_double_counted(self): + ovoscope.GLOBAL_BUS_COVERAGE = True + ovoscope.GLOBAL_BUS_COVERAGE_COLLECTOR = ovoscope.GlobalBusCoverageCollector() + + bus, tracker = self._tracker() + tracker.start_tracking() + for _ in range(3): + bus.emit(Message("dup.event")) + tracker.stop_tracking() + tracker._registered = {"__core__": {"dup.event": 1}} + + report = tracker.build_report() + skill = next(s for s in report.skills if s.skill_id == "__core__") + handler = next(h for h in skill.listeners if h.msg_type == "dup.event") + assert handler.invocation_count == 3 + + +# --------------------------------------------------------------------------- +# E. CaptureSession races +# --------------------------------------------------------------------------- + +class TestCaptureSessionRaces(unittest.TestCase): + """CaptureSession only ever touches ``minicroft.bus`` — a FakeBus-backed + stub keeps these race tests fast and saves a full (memory-hungry) boot.""" + + def setUp(self): + LOG.set_level("ERROR") + from types import SimpleNamespace + from ovos_utils.fakebus import FakeBus + self.mc = SimpleNamespace(bus=FakeBus()) + + def tearDown(self): + LOG.set_level("CRITICAL") + + def test_capture_reports_timeout(self): + cap = CaptureSession(self.mc, eof_msgs=["ovoscope.audit.no.such.eof"]) + completed = cap.capture(Message("ovoscope.audit.ping"), timeout=1) + cap.finish() + self.assertFalse(completed) + self.assertTrue(cap.timed_out) + self.assertEqual(cap.timeout_seconds, 1) + + def test_finish_returns_a_copy(self): + cap = CaptureSession(self.mc, eof_msgs=["ovoscope.audit.eof"]) + cap.capture(Message("ovoscope.audit.eof"), timeout=5) + got = cap.finish() + self.assertIsNot(got, cap.responses, + "finish() handed out the live response list") + got.append(Message("mutated.by.caller")) + self.assertNotIn("mutated.by.caller", + [m.msg_type for m in cap.responses]) + + def test_eof_counter_reset_is_atomic(self): + """A late eof handler must never leak into the NEXT capture.""" + cap = CaptureSession(self.mc, eof_msgs=["ovoscope.audit.eof"], + eof_count=2) + # one eof is not enough — capture must time out + completed = cap.capture(Message("ovoscope.audit.eof"), timeout=1) + self.assertFalse(completed) + self.assertEqual(cap._eof_seen, 1) + # a second capture starts from a clean counter + completed = cap.capture(Message("ovoscope.audit.eof"), timeout=1) + cap.finish() + self.assertFalse(completed, + "eof counter was not reset between captures") + + +# --------------------------------------------------------------------------- +# G. PipelineHarness.match() verdicts +# --------------------------------------------------------------------------- + +class TestMatchResult: + + def test_handler_start_does_not_suppress_a_match(self): + """`mycroft.skill.handler.start` fires on SUCCESS, never on failure. + + Treating it as a failure signal made every successful match report + "no match". + """ + from ovoscope.pipeline import PipelineHarness + + bus = FakeBus() + matched = Message("intent.service.skills.activated", + {"skill_id": "audit.skill"}) + + def _answer(_m): + # exactly what a successful dispatch looks like on the wire + bus.emit(Message("mycroft.skill.handler.start")) + bus.emit(matched) + + bus.on("recognizer_loop:utterance", _answer) + + harness = PipelineHarness() + harness._mc = MagicMock() + harness._mc.bus = bus + + result = harness.match_result("turn on the lights", timeout=3.0) + assert result.outcome == "matched", result.outcome + assert result.message.msg_type == "intent.service.skills.activated" + + def test_match_result_discriminates_outcomes(self): + assert MatchResult("matched", Message("x")).matched is True + assert MatchResult("timeout").timed_out is True + assert MatchResult("no_match").matched is False + assert MatchResult("no_match").timed_out is False + + def test_assert_no_match_fails_on_timeout(self): + """Silence is a broken harness, not proof of absence.""" + from ovoscope.pipeline import PipelineHarness + + harness = PipelineHarness.__new__(PipelineHarness) + harness.match_result = lambda utt, timeout=2.0: MatchResult("timeout") + with pytest.raises(AssertionError, match="no verdict"): + PipelineHarness.assert_no_match(harness, "nonsense") + + def test_assert_no_match_passes_on_explicit_failure(self): + from ovoscope.pipeline import PipelineHarness + + harness = PipelineHarness.__new__(PipelineHarness) + harness.match_result = lambda utt, timeout=2.0: MatchResult("no_match") + PipelineHarness.assert_no_match(harness, "nonsense") + + def test_assert_matches_fails_loudly_on_timeout(self): + from ovoscope.pipeline import PipelineHarness + + harness = PipelineHarness.__new__(PipelineHarness) + harness.match_result = lambda utt, timeout=5.0: MatchResult("timeout") + with pytest.raises(AssertionError, match="no verdict"): + PipelineHarness.assert_matches(harness, "turn on the lights") + + def test_match_returns_the_message_of_a_match_result(self): + from ovoscope.pipeline import PipelineHarness + + msg = Message("intent.service.skills.activated") + harness = PipelineHarness.__new__(PipelineHarness) + harness.match_result = lambda utt, timeout=5.0: MatchResult("matched", msg) + assert PipelineHarness.match(harness, "hi") is msg + + +# --------------------------------------------------------------------------- +# H. wait_for_match emit= parameter and match/fail race +# --------------------------------------------------------------------------- + +class TestWaitForMatch: + + def test_emit_happens_after_subscription(self): + """A synchronous FakeBus reply must not be missed.""" + from ovoscope.e2e import wait_for_match + + bus = FakeBus() + # answers synchronously, inside emit() + bus.on("audit.q", lambda m: bus.emit(Message("audit.a"))) + + got = wait_for_match(bus, ["audit.a"], timeout=2.0, + emit=Message("audit.q")) + assert got is not None + assert got.msg_type == "audit.a" + + def test_match_wins_over_concurrent_failure(self): + """A real match must not be dropped because a failure raced it.""" + from ovoscope.e2e import wait_for_match + + bus = FakeBus() + + def _answer(_m): + bus.emit(Message("complete_intent_failure")) + bus.emit(Message("audit.match")) + + bus.on("audit.q2", _answer) + got = wait_for_match(bus, ["audit.match"], timeout=2.0, + emit=Message("audit.q2")) + assert got is not None and got.msg_type == "audit.match" + + def test_docstring_no_longer_tells_callers_to_emit_after(self): + from ovoscope.e2e import wait_for_match + + assert "emit" in (wait_for_match.__doc__ or "") + assert "BLOCKS" in (wait_for_match.__doc__ or "") + + +# --------------------------------------------------------------------------- +# I. OCP HTTP mock must see the URL +# --------------------------------------------------------------------------- + +class TestOCPHttpMock: + + def test_configured_url_body_is_returned(self): + from ovoscope.ocp import _build_get_side_effect + + get = _build_get_side_effect({"bandcamp.com": {"tracks": ["Blue Note"]}}) + resp = get("https://bandcamp.com/api/search?q=jazz") + assert resp.json() == {"tracks": ["Blue Note"]} + + def test_unconfigured_url_falls_back_to_empty(self): + from ovoscope.ocp import _build_get_side_effect + + get = _build_get_side_effect({"bandcamp.com": {"tracks": []}}) + assert get("https://example.com/other").json() == {} + + def test_url_passed_as_keyword_is_matched(self): + from ovoscope.ocp import _build_get_side_effect + + get = _build_get_side_effect({"youtube.com": {"items": [1]}}) + assert get(url="https://youtube.com/results").json() == {"items": [1]} + + def test_patch_targets_receive_the_side_effect(self): + from ovoscope.ocp import OCPTest + + test = OCPTest(skill_ids=[], utterance="play jazz", + mock_responses={"bandcamp.com": {"ok": True}}) + patches = test._build_patches() + assert patches, "no patches were built" + import requests + for p in patches: + p.__enter__() + try: + assert requests.get("https://bandcamp.com/x").json() == {"ok": True} + finally: + for p in reversed(patches): + p.__exit__(None, None, None) + + +# --------------------------------------------------------------------------- +# J / K. harness teardown robustness +# --------------------------------------------------------------------------- + +class TestHarnessTeardown: + + def test_audio_harness_closes_bus_when_shutdown_raises(self): + from ovoscope.audio import AudioServiceHarness + + harness = AudioServiceHarness.__new__(AudioServiceHarness) + harness.service = MagicMock() + harness.service.shutdown.side_effect = RuntimeError("boom") + harness.bus = MagicMock() + + with pytest.raises(RuntimeError): + harness.__exit__(None, None, None) + harness.bus.close.assert_called_once() + + def test_two_playback_harnesses_are_refused(self): + from ovoscope.audio import PlaybackServiceHarness + + first = PlaybackServiceHarness.__new__(PlaybackServiceHarness) + PlaybackServiceHarness._active = first + try: + with pytest.raises(RuntimeError, match="already active"): + with PlaybackServiceHarness(): + pass + finally: + PlaybackServiceHarness._active = None + + def test_playback_harness_restores_previous_tts_queue(self): + from queue import Queue + from ovos_plugin_manager.templates.tts import TTS + from ovoscope.audio import PlaybackServiceHarness + + sentinel = Queue() + TTS.queue = sentinel + try: + with PlaybackServiceHarness(): + assert TTS.queue is not sentinel + assert TTS.queue is sentinel, "previous TTS.queue was not restored" + assert PlaybackServiceHarness._active is None + finally: + TTS.queue = sentinel + + def test_miniphal_detaches_capture_handler(self): + from ovoscope.phal import MiniPHAL + + phal = MiniPHAL() + bus = phal._bus + with phal: + pass + before = len(phal._captured) + bus.emit(Message("audit.after.exit")) + assert len(phal._captured) == before, ( + "MiniPHAL kept capturing after __exit__" + ) + + def test_listener_harness_detaches_capture_handler(self): + from ovoscope.voice_loop import ListenerHarness + + bus = FakeBus() + harness = ListenerHarness(bus=bus) + with harness: + bus.emit(Message("audit.during")) + assert any(m.msg_type == "audit.during" for m in harness._messages) + before = len(harness._messages) + bus.emit(Message("audit.after")) + assert len(harness._messages) == before, ( + "capture handler survived __exit__ on a shared bus" + ) + + +# --------------------------------------------------------------------------- +# N. PHAL load failures +# --------------------------------------------------------------------------- + +class TestPHALLoadErrors: + + def test_load_failure_raises_by_default(self): + from ovoscope.phal import MiniPHAL + + def _boom(_bus): + raise ValueError("no hardware") + + with pytest.raises(RuntimeError, match="failed to load"): + with MiniPHAL(plugin_ids=["audit.plugin"], + plugin_factories={"audit.plugin": _boom}): + pass + + def test_tolerated_failure_is_recorded_and_quoted(self): + from ovoscope.phal import MiniPHAL + + def _boom(_bus): + raise ValueError("no hardware") + + with MiniPHAL(plugin_ids=["audit.plugin"], + plugin_factories={"audit.plugin": _boom}, + tolerate_load_errors=True) as phal: + assert phal.load_errors + with pytest.raises(AssertionError) as ctx: + phal.assert_emitted("audit.never", timeout=0.1) + assert "failed to load" in str(ctx.value) + + +# --------------------------------------------------------------------------- +# O. coverage.py must not swallow a malformed manifest +# --------------------------------------------------------------------------- + +class TestCoverageParseErrors: + + def test_malformed_pyproject_is_recorded(self, tmp_path): + from ovoscope.coverage import _parse_entry_points + + bad = tmp_path / "pyproject.toml" + bad.write_text('[project\nname = "oops"\n') + errors = [] + _parse_entry_points(str(bad), errors) + assert errors, "a malformed pyproject.toml was silently ignored" + assert "invalid TOML" in errors[0][1] + + def test_unreadable_setup_py_is_recorded(self, tmp_path): + from ovoscope.coverage import _parse_setup_py_entry_points + + errors = [] + _parse_setup_py_entry_points(str(tmp_path / "missing_setup.py"), errors) + assert errors and "unreadable" in errors[0][1] + + def test_report_exposes_parse_errors(self): + from ovoscope.coverage import EcosystemCoverageReport + + assert EcosystemCoverageReport().parse_errors == [] + + def test_valid_pyproject_records_no_error(self, tmp_path): + from ovoscope.coverage import _parse_entry_points + + good = tmp_path / "pyproject.toml" + good.write_text( + '[project]\nname = "x"\n\n' + '[project.entry-points."opm.skill"]\n' + '"my-skill.author" = "my_skill:create"\n' + ) + errors = [] + eps = _parse_entry_points(str(good), errors) + assert errors == [] + assert eps["opm.skill"] == ["my-skill.author"] + + +# --------------------------------------------------------------------------- +# P. RemoteRecorder must not leak a reconnecting client +# --------------------------------------------------------------------------- + +class TestRemoteRecorderConnectLeak: + + def test_client_is_closed_on_connect_timeout(self): + from ovoscope.remote_recorder import RemoteRecorder + + rec = RemoteRecorder(bus_url="ws://127.0.0.1:1/core") + fake_client = MagicMock() + fake_client.connected_event.is_set.return_value = False + + with patch("ovos_bus_client.client.MessageBusClient", + return_value=fake_client), \ + patch("time.monotonic", side_effect=[0.0, 100.0, 200.0]), \ + patch("time.sleep"): + with pytest.raises(ConnectionError): + rec.connect() + + fake_client.close.assert_called_once() + assert rec._client is None, "a dead client was left reconnecting forever" + + +# --------------------------------------------------------------------------- +# F. cmd_run must not boot a second MiniCroft +# --------------------------------------------------------------------------- + +class TestCliRunSingleMiniCroft: + + def test_cmd_run_hands_its_minicroft_to_the_test(self, tmp_path): + import argparse + from ovoscope import cli + + fixture = tmp_path / "f.json" + fixture.write_text("{}") + + mc = MagicMock() + test = MagicMock() + test.skill_ids = [] + + with patch("ovoscope.End2EndTest.from_path", return_value=test), \ + patch("ovoscope.get_minicroft", return_value=mc): + cli.cmd_run(argparse.Namespace(fixture=str(fixture), timeout=5, + verbose=False)) + + assert test.minicroft is mc, "cmd_run did not reuse its MiniCroft" + assert test.managed is False, "execute() would boot a second MiniCroft" + mc.stop.assert_called_once() + + +# --------------------------------------------------------------------------- +# L. shared MiniCroft lifecycle +# --------------------------------------------------------------------------- + +class TestSharedMiniCroftLifecycle: + + def test_only_one_instance_stays_live(self): + from ovoscope import intent_cases + + key = intent_cases._SHARED_MINICROFT_KEY + cache = vars(intent_cases).setdefault(key, {}) + cache.clear() + + old = MagicMock() + cache[("old-skill", ("en-US",))] = old + + new = MagicMock() + with patch.object(intent_cases, "get_minicroft", return_value=new): + got = intent_cases._shared_minicroft("new-skill", ["en-US"], 0) + + assert got is new + old.stop.assert_called_once() + assert len(cache) == 1, "two shared MiniCrofts were kept alive" + cache.clear() + + def test_stop_shared_minicrofts_empties_the_cache(self): + from ovoscope import intent_cases + + key = intent_cases._SHARED_MINICROFT_KEY + cache = vars(intent_cases).setdefault(key, {}) + cache.clear() + mc = MagicMock() + cache[("s", ("en-US",))] = mc + + intent_cases.stop_shared_minicrofts() + mc.stop.assert_called_once() + assert cache == {} + + +# --------------------------------------------------------------------------- +# M. MiniSimpleListener must not reuse a wedged listener thread +# --------------------------------------------------------------------------- + +class TestSimpleListenerWedgedThread: + + def test_wedged_listener_is_replaced(self): + pytest.importorskip("ovos_simple_listener") + from ovoscope.simple_listener import MiniSimpleListener + + # SimpleListener eagerly builds a real microphone in __init__, so the + # harness can only be constructed where a microphone plugin exists. + # Construct the harness itself under the guard: earlier tests can + # change the loaded Configuration, so probing the factory separately + # is order-dependent — the harness construction is the real gate. + try: + harness = MiniSimpleListener() + except Exception as e: + pytest.skip(f"no usable microphone plugin: {e}") + first = harness.listener + try: + # pretend the thread refuses to die + with patch.object(type(first), "is_alive", return_value=True), \ + patch.object(type(first), "start"), \ + patch.object(type(first), "stop"), \ + patch.object(type(first), "join"): + harness.feed_file(b"\x00" * 4096, timeout=0.2) + assert harness.listener is not first, ( + "a wedged listener was reused for the next run" + ) + finally: + harness.detach_capture() + + +if __name__ == "__main__": + unittest.main() diff --git a/test/unittests/test_audit_round2.py b/test/unittests/test_audit_round2.py new file mode 100644 index 0000000..ff8fb66 --- /dev/null +++ b/test/unittests/test_audit_round2.py @@ -0,0 +1,941 @@ +"""Regression tests for the round-2 audit findings. + +Each test here fails on the pre-fix code. Grouped by audit section: + +A. memory retention (SkillApi.bus pinned every stopped MiniCroft) +B. the dead ``--ovoscope-accuracy-min`` CI gate +C. silent state corruption / false-green assertions +D. race windows found by adversarial validation of the round-1 fixes +E. resilience sweep items +F. test-suite quality (direct coverage of previously untested helpers) + +Tests that do not need a running assistant use a +``SimpleNamespace(bus=FakeBus())`` stub: booting a MiniCroft costs seconds and +hundreds of MB of retained memory per boot. +""" +import gc +import json +import os +import sys +import tempfile +import threading +import unittest +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +import pytest +from ovos_bus_client.message import Message +from ovos_utils.fakebus import FakeBus +from ovos_utils.log import LOG + +from ovoscope import CaptureSession + + +# --------------------------------------------------------------------------- +# A. Memory retention — SkillApi.bus +# --------------------------------------------------------------------------- + +class TestSkillApiBusRestore(unittest.TestCase): + """``SkillApi.bus`` is a process-wide class attribute set during boot. + + Left pointing at a stopped MiniCroft's FakeBus, it pins the whole object + graph (~633MB per stopped instance measured on this suite). + """ + + def test_stop_restores_skill_api_bus(self): + from ovos_workshop.skills.api import SkillApi + from ovoscope import get_minicroft + + original = SkillApi.bus + LOG.set_level("ERROR") + try: + croft = get_minicroft([]) + try: + self.assertIsNot( + SkillApi.bus, original, + "boot did not connect SkillApi to the harness bus — this " + "test no longer proves anything") + finally: + croft.stop() + self.assertIs(SkillApi.bus, original, + "SkillApi.bus was not restored by stop()") + finally: + LOG.set_level("CRITICAL") + SkillApi.bus = original + + @pytest.mark.timeout(900) + def test_stopped_minicrofts_are_collectable(self): + """Boot+stop two MiniCrofts; the second must be collectable. + + The FIRST boot leaves a known residue (module-level singletons built + lazily on first use), so one live instance is allowed. A second live + instance means every stopped harness is still pinned. + """ + from ovoscope import MiniCroft, get_minicroft + + LOG.set_level("ERROR") + try: + for _ in range(2): + croft = get_minicroft([]) + croft.stop() + del croft + gc.collect() + gc.collect() + alive = [o for o in gc.get_objects() + if type(o) is MiniCroft] + self.assertLessEqual( + len(alive), 1, + f"{len(alive)} stopped MiniCroft instances are still alive — " + f"a process-wide reference is pinning them") + finally: + LOG.set_level("CRITICAL") + + +# --------------------------------------------------------------------------- +# B. The accuracy gate must change the exit status +# --------------------------------------------------------------------------- + +class TestAccuracyGateExitStatus(unittest.TestCase): + """The gate was computed in pytest_terminal_summary, which runs AFTER + pytest_sessionfinish — so the flag was always read too late and the exit + status stayed 0 no matter how bad the accuracy was.""" + + _CONFTEST = """ +def pytest_sessionstart(session): + from ovoscope import pytest_plugin + pytest_plugin.pytest_runtest_logreport._accum = { + "meta": {}, + "results": [{"nodeid": "x", "skill_id": "s", "pipeline": "p", + "lang": "en-US", "intent": "i", "utterance": "u", + "source": "src", "passed": PASSED}], + } +""" + + def _run(self, passed, extra): + """Run pytest in a subprocess with a pre-seeded accuracy result.""" + import subprocess + + tmpdir = tempfile.mkdtemp(prefix="ovoscope-gate-") + with open(os.path.join(tmpdir, "conftest.py"), "w", + encoding="utf-8") as fh: + fh.write(self._CONFTEST.replace("PASSED", str(passed))) + with open(os.path.join(tmpdir, "test_gate_case.py"), "w", + encoding="utf-8") as fh: + fh.write("def test_ok():\n assert True\n") + # Put the code under test ahead of any installed copy of ovoscope. + repo_root = os.path.dirname(os.path.dirname( + os.path.dirname(os.path.abspath(__file__)))) + env = dict(os.environ) + env["PYTHONPATH"] = os.pathsep.join( + [repo_root] + ([env["PYTHONPATH"]] if env.get("PYTHONPATH") else [])) + proc = subprocess.run( + [sys.executable, "-m", "pytest", tmpdir, "-q", + "-p", "no:cacheprovider"] + extra, + capture_output=True, text=True, cwd=tmpdir, timeout=300, env=env) + return proc + + def test_failing_gate_yields_nonzero_exit_status(self): + proc = self._run(False, ["--ovoscope-accuracy-min", "0.99"]) + self.assertNotEqual( + proc.returncode, 0, + f"a failed accuracy gate did not change the exit status\n" + f"{proc.stdout}\n{proc.stderr}") + self.assertIn("accuracy gate FAILED", proc.stdout) + + def test_passing_gate_keeps_exit_status_zero(self): + proc = self._run(True, ["--ovoscope-accuracy-min", "0.99"]) + self.assertEqual(proc.returncode, 0, + f"{proc.stdout}\n{proc.stderr}") + + def test_no_gate_option_keeps_exit_status_zero(self): + proc = self._run(False, []) + self.assertEqual(proc.returncode, 0, + f"{proc.stdout}\n{proc.stderr}") + + +# --------------------------------------------------------------------------- +# C. False-green / silent corruption +# --------------------------------------------------------------------------- + +class TestDiffFixtureValidation(unittest.TestCase): + """A file with no ``expected_messages`` compared []-vs-[] as identical.""" + + def _write(self, payload): + fd, path = tempfile.mkstemp(suffix=".json") + with os.fdopen(fd, "w", encoding="utf-8") as fh: + json.dump(payload, fh) + return path + + def test_non_fixture_file_raises(self): + from ovoscope.diff import diff_fixtures + + a = self._write({"not": "a fixture"}) + b = self._write({"also": "not one"}) + with self.assertRaises(ValueError): + diff_fixtures(expected_path=a, actual_path=b) + + def test_real_fixture_still_diffs(self): + from ovoscope.diff import diff_fixtures + + a = self._write({"expected_messages": [ + {"type": "a", "data": {}, "context": {}}]}) + b = self._write({"expected_messages": [ + {"type": "b", "data": {}, "context": {}}]}) + result = diff_fixtures(expected_path=a, actual_path=b) + self.assertFalse(result.is_identical) + + def test_expected_none_differs_from_absent_key(self): + from ovoscope.diff import _dict_diff + + diffs = _dict_diff({"a": None}, {}) + self.assertIn("a", diffs, + "an expected None matched an ABSENT key") + + def test_present_none_matches_expected_none(self): + from ovoscope.diff import _dict_diff + + self.assertEqual(_dict_diff({"a": None}, {"a": None}), {}) + + def test_nested_dict_value_mismatch(self): + from ovoscope.diff import _dict_diff + + diffs = _dict_diff({"a": {"b": 1}}, {"a": {"b": 2}}) + self.assertIn("a", diffs) + + def test_list_of_dicts_mismatch(self): + from ovoscope.diff import _dict_diff + + diffs = _dict_diff({"a": [{"b": 1}]}, {"a": [{"b": 2}]}) + self.assertIn("a", diffs) + + def test_list_of_dicts_match(self): + from ovoscope.diff import _dict_diff + + self.assertEqual(_dict_diff({"a": [{"b": 1}]}, {"a": [{"b": 1}]}), {}) + + +class TestBusCoverageTrackerUnwrap(unittest.TestCase): + """A raise between start_tracking() and stop_tracking() left bus.emit + wrapped forever, stacking one wrapper per test.""" + + def test_stop_tracking_restores_emit(self): + from ovoscope.bus_coverage import BusCoverageTracker + + bus = FakeBus() + mc = SimpleNamespace(bus=bus) + tracker = BusCoverageTracker(bus, mc) + tracker.snapshot_listeners() + tracker.start_tracking() + bus.emit(Message("ovoscope.audit.r2.counted")) + self.assertEqual(tracker._invocations.get("ovoscope.audit.r2.counted"), + 1, "the wrapper did not count") + tracker.stop_tracking() + bus.emit(Message("ovoscope.audit.r2.counted")) + self.assertEqual( + tracker._invocations.get("ovoscope.audit.r2.counted"), 1, + "bus.emit was still wrapped after stop_tracking()") + bus.close() + + def test_stop_tracking_leaves_a_foreign_wrapper_alone(self): + from ovoscope.bus_coverage import BusCoverageTracker + + bus = FakeBus() + mc = SimpleNamespace(bus=bus) + tracker = BusCoverageTracker(bus, mc) + tracker.snapshot_listeners() + tracker.start_tracking() + + inner = bus.emit + + def _foreign(message): + inner(message) + + bus.emit = _foreign + tracker.stop_tracking() + self.assertIs(bus.emit, _foreign, + "stop_tracking clobbered another tracker's wrapper") + bus.close() + + def test_tracker_is_unwrapped_when_execute_raises(self): + """End2EndTest._execute must unwrap on the failure path too.""" + import ovoscope.bus_coverage as bc + from ovoscope import End2EndTest + + bus = FakeBus() + mc = SimpleNamespace(bus=bus, boot_messages=[]) + built = [] + real_cls = bc.BusCoverageTracker + + class _Recording(real_cls): + def __init__(self, *a, **kw): + super().__init__(*a, **kw) + built.append(self) + + test = End2EndTest( + skill_ids=[], + source_message=Message("ovoscope.audit.r2.ping"), + expected_messages=[Message("ovoscope.audit.r2.never")], + eof_msgs=["ovoscope.audit.r2.no.such.eof"], + track_bus_coverage=True, + verbose=False, + minicroft=mc, + ) + with patch.object(bc, "BusCoverageTracker", _Recording): + with self.assertRaises(AssertionError): + test._execute(timeout=1) + self.assertEqual(len(built), 1) + tracker = built[0] + before = dict(tracker._invocations) + bus.emit(Message("ovoscope.audit.r2.after.failure")) + self.assertEqual( + dict(tracker._invocations), before, + "bus.emit was left wrapped after a failing test") + bus.close() + + +# --------------------------------------------------------------------------- +# D. Race windows +# --------------------------------------------------------------------------- + +class TestCaptureSessionArming(unittest.TestCase): + """An eof arriving OUTSIDE a capture window must not count.""" + + def setUp(self): + LOG.set_level("ERROR") + self.mc = SimpleNamespace(bus=FakeBus()) + + def tearDown(self): + self.mc.bus.close() + LOG.set_level("CRITICAL") + + def test_eof_before_capture_does_not_complete_it(self): + cap = CaptureSession(self.mc, eof_msgs=["ovoscope.audit.r2.eof"]) + # eof fires before capture() is ever called + self.mc.bus.emit(Message("ovoscope.audit.r2.eof")) + completed = cap.capture(Message("ovoscope.audit.r2.ping"), timeout=1) + cap.finish() + self.assertFalse( + completed, + "an eof emitted before capture() completed the next capture") + self.assertTrue(cap.timed_out) + + def test_eof_after_finish_does_not_leak_into_next_capture(self): + cap = CaptureSession(self.mc, eof_msgs=["ovoscope.audit.r2.eof"]) + cap.capture(Message("ovoscope.audit.r2.eof"), timeout=5) + cap.finish() + self.mc.bus.emit(Message("ovoscope.audit.r2.eof")) + + cap2 = CaptureSession(self.mc, eof_msgs=["ovoscope.audit.r2.eof"]) + self.mc.bus.emit(Message("ovoscope.audit.r2.eof")) + completed = cap2.capture(Message("ovoscope.audit.r2.ping"), timeout=1) + cap2.finish() + self.assertFalse(completed) + + def test_normal_capture_still_completes(self): + cap = CaptureSession(self.mc, eof_msgs=["ovoscope.audit.r2.eof"]) + completed = cap.capture(Message("ovoscope.audit.r2.eof"), timeout=5) + cap.finish() + self.assertTrue(completed) + + def test_del_without_a_bus_is_silent(self): + cap = CaptureSession(SimpleNamespace(bus=FakeBus()), + eof_msgs=["ovoscope.audit.r2.eof"]) + cap.minicroft = SimpleNamespace(bus=None) + cap.__del__() # must not raise + + +class TestMockTTSStopRace(unittest.TestCase): + """The mock-TTS unduck emit and stop() must be mutually exclusive.""" + + @pytest.mark.timeout(900) + def test_unduck_never_emits_after_stop(self): + from ovoscope import get_minicroft + + LOG.set_level("ERROR") + try: + croft = get_minicroft([]) + seen = [] + after_stop = threading.Event() + + def _watch(msg): + if croft._stopped: + seen.append(msg) + after_stop.set() + + croft.bus.on("recognizer_loop:audio_output_end", _watch) + for i in range(20): + croft.bus.emit(Message("speak", {"utterance": f"u{i}"})) + croft.stop() + self.assertFalse( + after_stop.wait(0.5), + f"the mock TTS emitted after stop(): {seen}") + finally: + LOG.set_level("CRITICAL") + + +class TestDefaultSessionRestoreFallbacks(unittest.TestCase): + """The snapshot/restore must use ONE bus-client API family, and must not + degrade to a total no-op when the snapshot failed.""" + + def test_restore_uses_the_api_family_that_snapshotted(self): + """The snapshot API and the load API must be the same family.""" + from ovos_bus_client.session import Session, SessionManager + from ovoscope import MiniCroft + + croft = MiniCroft.__new__(MiniCroft) + sess = SessionManager.default_session + if hasattr(sess, "to_dict"): + croft._session_api = "dict" + croft._default_session_state = sess.to_dict() + used, forbidden = "from_dict", "deserialize" + else: + croft._session_api = "legacy" + croft._default_session_state = sess.serialize() + used, forbidden = "deserialize", "from_dict" + croft._default_active_skills = list(sess.active_skills) + + calls = [] + real = getattr(Session, used) + + def _spy(data): + calls.append(used) + return real(data) + + def _forbidden(*_a, **_kw): + self.fail(f"a {croft._session_api} snapshot was loaded with " + f"{forbidden}()") + + patches = [patch.object(Session, used, staticmethod(_spy))] + if hasattr(Session, forbidden): + patches.append( + patch.object(Session, forbidden, staticmethod(_forbidden))) + for p in patches: + p.start() + try: + croft._restore_default_session() + finally: + for p in reversed(patches): + p.stop() + self.assertEqual(calls, [used]) + + def test_active_skills_restored_without_a_snapshot(self): + from ovos_bus_client.session import SessionManager + from ovoscope import MiniCroft + + croft = MiniCroft.__new__(MiniCroft) + croft._default_session_state = None + croft._default_active_skills = [] + sess = SessionManager.default_session + original = list(sess.active_skills) + try: + sess.activate_skill("ovoscope.audit.r2.leaked") + self.assertTrue(sess.active_skills) + croft._restore_default_session() + self.assertEqual( + sess.active_skills, [], + "a skill activated during the run survived teardown") + finally: + sess.active_skills = original + + +# --------------------------------------------------------------------------- +# E. Resilience sweep +# --------------------------------------------------------------------------- + +class TestHotwordCompatIsScoped(unittest.TestCase): + def test_patch_is_reverted(self): + from ovos_plugin_manager.templates import hotwords as hw + from ovoscope.wakeword_probe import hotword_compat + + original = hw.HotWordEngine.__init__ + with hotword_compat(): + pass + self.assertIs(hw.HotWordEngine.__init__, original, + "hotword compat patch outlived its block") + + def test_patch_is_reverted_on_error(self): + from ovos_plugin_manager.templates import hotwords as hw + from ovoscope.wakeword_probe import hotword_compat + + original = hw.HotWordEngine.__init__ + with self.assertRaises(RuntimeError): + with hotword_compat(): + raise RuntimeError("boom") + self.assertIs(hw.HotWordEngine.__init__, original) + + +class TestWakeWordDestructiveReads(unittest.TestCase): + """``any(e.found_wake_word() ...)`` short-circuited, leaving later engines + latched — the NEXT call then reported a stale detection.""" + + class _Latch: + def __init__(self, value): + self.value = value + self.reads = 0 + + def update(self, chunk): + pass + + def found_wake_word(self, *args): + self.reads += 1 + got, self.value = self.value, False + return got + + def test_every_engine_latch_is_read(self): + from ovoscope.listener import MiniListener + + listener = MiniListener.__new__(MiniListener) + a = self._Latch(True) + b = self._Latch(True) + listener._ww = {"a": a, "b": b} + self.assertTrue(listener.detect_wakeword(b"\x00" * 32)) + self.assertEqual(b.reads, 1, + "the second engine's latch was never read") + # both latches were consumed, so a second call reports nothing + self.assertFalse(listener.detect_wakeword(b"\x00" * 32)) + + def test_scan_reads_every_engine_latch(self): + from ovoscope.listener import MiniListener + + listener = MiniListener.__new__(MiniListener) + a = self._Latch(True) + b = self._Latch(True) + listener._ww = {"a": a, "b": b} + found, idx = listener.scan_for_wakeword([b"\x00" * 32]) + self.assertTrue(found) + self.assertEqual(idx, 0) + self.assertEqual(b.reads, 1, + "the second engine's latch was never read") + + +class TestMediaProviderTimeout(unittest.TestCase): + def test_hanging_provider_raises_timeout(self): + from ovoscope.media_provider import MediaProviderHarness + + block = threading.Event() + + class _Hanging: + def is_available(self_inner): + block.wait(30) + return True + + harness = MediaProviderHarness(_Hanging(), call_timeout=0.2) + try: + with self.assertRaises(TimeoutError): + harness.is_available() + finally: + block.set() + + def test_fast_provider_returns_normally(self): + from ovoscope.media_provider import MediaProviderHarness + + class _Fast: + def is_available(self_inner): + return True + + self.assertTrue(MediaProviderHarness(_Fast()).is_available()) + + def test_timeout_can_be_disabled(self): + from ovoscope.media_provider import MediaProviderHarness + + class _Fast: + def is_available(self_inner): + return True + + harness = MediaProviderHarness(_Fast(), call_timeout=None) + self.assertTrue(harness.is_available()) + + +class TestTtsIntelligibilityMarkers(unittest.TestCase): + def setUp(self): + pytest.importorskip("jiwer") + + def test_utterance_slug_is_stable_and_distinct(self): + from ovoscope.tts_intelligibility import _utt_slug + + self.assertEqual(_utt_slug("hello"), _utt_slug("hello")) + self.assertNotEqual(_utt_slug("hello"), _utt_slug("hello!")) + self.assertEqual(len(_utt_slug("hello")), 16) + + def test_score_reports_a_reference_stt_failure(self): + from ovoscope.tts_intelligibility import UtteranceScore + + score = UtteranceScore(utterance="hi", transcript=None, wer=1.0, + cer=1.0, transcribe_failed=True, + transcribe_error="RuntimeError: no model") + payload = score.to_dict() + self.assertTrue(payload["transcribe_failed"]) + self.assertIsNone(payload["transcript"]) + self.assertIn("no model", payload["transcribe_error"]) + + +class TestSetupSkillFetchFailure(unittest.TestCase): + def test_failed_skill_md_download_reports_an_error(self): + from ovoscope import setup_skill + + with tempfile.TemporaryDirectory() as tmp: + from pathlib import Path + with patch.object(setup_skill, "_fetch", return_value=False): + ok = setup_skill._install_skill( + Path(tmp) / "ovoscope", "claude", + fetch_docs=False, verbose=False) + self.assertFalse(ok, "a failed SKILL.md download reported success") + + def test_main_exits_nonzero_when_install_fails(self): + from ovoscope import setup_skill + + argv = ["ovoscope-setup-skill", "--claude", "--no-docs"] + with patch.object(sys, "argv", argv), \ + patch.object(setup_skill, "install_claude", return_value=False): + self.assertEqual(setup_skill.main(), 1) + + +class TestListenerImportGuardIsNarrow(unittest.TestCase): + """A constructor failure must propagate, not masquerade as a missing + ovos-dinkum-listener install.""" + + def test_constructor_failure_propagates(self): + import ovoscope.listener as listener_mod + + try: + from ovos_dinkum_listener import transformers as tmod + except ImportError: + self.skipTest("ovos-dinkum-listener is not installed") + + with patch.object(tmod, "AudioTransformersService", + side_effect=RuntimeError("constructor boom")): + with self.assertRaises(RuntimeError): + listener_mod.MiniListener( + {"listener": {"audio_transformers": {}}}) + + +class TestVoiceLoopShutdownDetaches(unittest.TestCase): + def test_shutdown_removes_the_capture_handler(self): + from ovoscope.voice_loop import MiniVoiceLoop + + harness = MiniVoiceLoop.__new__(MiniVoiceLoop) + harness.hotwords = None + harness.bus = FakeBus() + calls = [] + harness._capture = lambda msg: calls.append(msg) + harness.bus.on("message", harness._capture) + harness.shutdown() + harness.bus.emit(Message("ovoscope.audit.r2.after.shutdown")) + self.assertEqual(calls, [], + "shutdown() left the capture handler attached") + harness.bus.close() + + +# --------------------------------------------------------------------------- +# F. Direct coverage of previously untested helpers +# --------------------------------------------------------------------------- + +class TestE2EPipelineHarnessConfigRestore(unittest.TestCase): + """setUpClass/tearDownClass patch a process-wide config key. Two + subclasses running back-to-back must not leak it.""" + + def _make_subclass(self, config_key, plugin_config): + from ovoscope.e2e import E2EPipelineHarness + + class _Sub(E2EPipelineHarness): + PIPELINE_ID = "ovoscope-audit-r2-pipeline" + CONFIG_KEY = config_key + PLUGIN_CONFIG = plugin_config + + return _Sub + + def _fake_minicroft(self): + mc = MagicMock() + mc.intents.pipeline_plugins = { + "ovoscope-audit-r2-pipeline": MagicMock()} + return mc + + def test_absent_key_is_removed_again(self): + from ovos_config.config import Configuration + import ovoscope + + cfg = Configuration() + intents = cfg.setdefault("intents", {}) + key = "ovoscope_audit_r2_absent" + intents.pop(key, None) + + sub = self._make_subclass(key, {"a": 1}) + with patch.object(ovoscope, "get_minicroft", + return_value=self._fake_minicroft()): + sub.setUpClass() + self.assertEqual(intents[key], {"a": 1}) + sub.tearDownClass() + self.assertNotIn(key, intents, + "a config key the harness ADDED survived teardown") + + def test_existing_key_is_restored(self): + from ovos_config.config import Configuration + import ovoscope + + cfg = Configuration() + intents = cfg.setdefault("intents", {}) + key = "ovoscope_audit_r2_existing" + intents[key] = {"original": True} + try: + sub = self._make_subclass(key, {"patched": True}) + with patch.object(ovoscope, "get_minicroft", + return_value=self._fake_minicroft()): + sub.setUpClass() + self.assertEqual(intents[key], {"patched": True}) + sub.tearDownClass() + self.assertEqual(intents[key], {"original": True}) + finally: + intents.pop(key, None) + + def test_two_subclasses_back_to_back_do_not_leak(self): + from ovos_config.config import Configuration + import ovoscope + + cfg = Configuration() + intents = cfg.setdefault("intents", {}) + for key in ("ovoscope_audit_r2_first", "ovoscope_audit_r2_second"): + intents.pop(key, None) + + with patch.object(ovoscope, "get_minicroft", + return_value=self._fake_minicroft()): + for key in ("ovoscope_audit_r2_first", + "ovoscope_audit_r2_second"): + sub = self._make_subclass(key, {"k": key}) + sub.setUpClass() + sub.tearDownClass() + + for key in ("ovoscope_audit_r2_first", "ovoscope_audit_r2_second"): + self.assertNotIn(key, intents) + + +class TestE2EHelpersOnAFakeBus(unittest.TestCase): + """The registration shims are pure bus emits — assert the wire payload.""" + + def setUp(self): + self.bus = FakeBus() + self.seen = [] + self.bus.on("message", lambda raw: self.seen.append( + Message.deserialize(raw))) + + def tearDown(self): + self.bus.close() + + def _types(self): + return [m.msg_type for m in self.seen] + + def test_wait_for_failure_returns_false_on_timeout(self): + from ovoscope.e2e import wait_for_failure + + self.assertFalse(wait_for_failure(self.bus, timeout=0.2)) + + def test_wait_for_failure_returns_true_when_it_fires(self): + from ovoscope.e2e import wait_for_failure + + threading.Timer( + 0.05, + lambda: self.bus.emit(Message("complete_intent_failure"))).start() + self.assertTrue(wait_for_failure(self.bus, timeout=3)) + + def test_wait_for_failure_unsubscribes(self): + from ovoscope.e2e import wait_for_failure + + wait_for_failure(self.bus, timeout=0.05) + before = len(self.seen) + self.bus.emit(Message("complete_intent_failure")) + # the helper's own handler is gone; only the wildcard capture fires + self.assertEqual(len(self.seen), before + 1) + + def test_register_adapt_vocab_emits_one_message_per_word(self): + from ovoscope.e2e import register_adapt_vocab + + register_adapt_vocab(self.bus, "Fruit", ["apple", "pear"], settle=0) + vocab = [m for m in self.seen if m.msg_type == "register_vocab"] + self.assertEqual(len(vocab), 2) + self.assertEqual( + [m.data["entity_value"] for m in vocab], ["apple", "pear"]) + self.assertTrue(all(m.data["entity_type"] == "Fruit" for m in vocab)) + + def test_register_adapt_intent_round_trip(self): + pytest.importorskip("adapt") + from adapt.intent import IntentBuilder + + from ovoscope.e2e import detach_intent, register_adapt_intent + + builder = IntentBuilder("R2TestIntent").require("Fruit") + register_adapt_intent(self.bus, builder, lang="en-US", settle=0) + registered = [m for m in self.seen + if m.msg_type == "register_intent"] + self.assertEqual(len(registered), 1) + self.assertEqual(registered[0].context["lang"], "en-US") + + detach_intent(self.bus, "R2TestIntent", settle=0) + detached = [m for m in self.seen if m.msg_type == "detach_intent"] + self.assertEqual(len(detached), 1) + self.assertEqual(detached[0].data["intent_name"], "R2TestIntent") + + def test_register_adapt_intent_accepts_a_built_intent(self): + pytest.importorskip("adapt") + from adapt.intent import IntentBuilder + + from ovoscope.e2e import register_adapt_intent + + intent = IntentBuilder("R2Built").require("Fruit").build() + register_adapt_intent(self.bus, intent, settle=0) + self.assertIn("register_intent", self._types()) + + +class TestCliErrors(unittest.TestCase): + def _run_cli(self, argv): + from ovoscope import cli + + with patch.object(sys, "argv", argv): + with self.assertRaises(SystemExit) as ctx: + cli.main() + return ctx.exception.code + + def test_run_with_a_missing_fixture_exits_nonzero(self): + code = self._run_cli( + ["ovoscope", "run", "/nonexistent/ovoscope-audit-r2.json"]) + self.assertNotEqual(code, 0) + + def test_run_with_a_failing_fixture_exits_nonzero(self): + import ovoscope + from ovoscope import End2EndTest + + test = End2EndTest( + skill_ids=[], + source_message=Message("ovoscope.audit.r2.eof"), + expected_messages=[Message("ovoscope.audit.r2.never.emitted")], + eof_msgs=["ovoscope.audit.r2.eof"], + verbose=False, + ) + fd, path = tempfile.mkstemp(suffix=".json") + os.close(fd) + test.save(path) + + bus = FakeBus() + mc = SimpleNamespace(bus=bus, boot_messages=[], + stop=lambda: bus.close()) + with patch.object(ovoscope, "get_minicroft", return_value=mc): + code = self._run_cli(["ovoscope", "run", path]) + self.assertNotEqual(code, 0, + "a failing fixture exited 0") + + def test_diff_with_a_non_fixture_file_exits_nonzero(self): + fd, path = tempfile.mkstemp(suffix=".json") + with os.fdopen(fd, "w", encoding="utf-8") as fh: + json.dump({"nope": 1}, fh) + code = self._run_cli(["ovoscope", "diff", path, path]) + self.assertNotEqual(code, 0) + + +class TestSetupPyEntryPointParsing(unittest.TestCase): + def _write_setup_py(self, source): + tmp = tempfile.mkdtemp(prefix="ovoscope-r2-repo-") + path = os.path.join(tmp, "setup.py") + with open(path, "w", encoding="utf-8") as fh: + fh.write(source) + return path + + def test_happy_path(self): + from ovoscope.coverage import _parse_setup_py_entry_points + + path = self._write_setup_py( + "from setuptools import setup\n" + "setup(name='x',\n" + " entry_points={'ovos.plugin.skill': " + "['my.skill=my.mod:Skill']})\n") + eps = _parse_setup_py_entry_points(path) + self.assertIn("ovos.plugin.skill", eps) + + def test_malformed_setup_py_is_recorded_not_raised(self): + from ovoscope.coverage import _parse_setup_py_entry_points + + path = self._write_setup_py("this is ( not python\n") + errors = [] + eps = _parse_setup_py_entry_points(path, errors) + self.assertIsInstance(eps, dict) + + def test_unreadable_setup_py_is_recorded(self): + from ovoscope.coverage import _parse_setup_py_entry_points + + errors = [] + eps = _parse_setup_py_entry_points( + "/nonexistent/ovoscope-audit-r2/setup.py", errors) + self.assertEqual(eps, {}) + self.assertTrue(errors, "an unreadable setup.py recorded no error") + + +class TestPipelineHarnessState(unittest.TestCase): + """match -> failure -> match must not leave a stale verdict behind.""" + + def test_failure_clears_the_previous_match(self): + from ovoscope.pipeline import _SinkSkill + + bus = FakeBus() + sink = _SinkSkill(bus=bus) + bus.emit(Message("intent.service.skills.activated", + {"skill_id": "a"})) + self.assertIsNotNone(sink._last_match) + bus.emit(Message("intent_failure")) + self.assertIsNone(sink._last_match, + "a stale match survived an explicit intent failure") + bus.emit(Message("intent.service.skills.activated", + {"skill_id": "b"})) + self.assertEqual(sink._last_match.data["skill_id"], "b") + bus.close() + + def test_match_result_clears_state_before_sending(self): + from ovoscope.pipeline import PipelineHarness, _SinkSkill + + bus = FakeBus() + harness = PipelineHarness() + harness._mc = SimpleNamespace(bus=bus) + harness._sink = _SinkSkill(bus=bus) + harness._sink._last_match = Message("stale.match") + result = harness.match_result("nothing will answer", timeout=0.2) + self.assertTrue(result.timed_out) + self.assertIsNone(harness._sink._last_match, + "match_result did not reset the stale verdict") + bus.close() + + def test_enter_stops_minicroft_when_wiring_fails(self): + import ovoscope + from ovoscope.pipeline import PipelineHarness + + mc = MagicMock() + # rebinding the sink bus raises -> __enter__ must still stop the boot + type(mc).bus = property( + lambda self: (_ for _ in ()).throw(RuntimeError("wiring boom"))) + with patch.object(ovoscope, "get_minicroft", return_value=mc): + harness = PipelineHarness() + with self.assertRaises(RuntimeError): + harness.__enter__() + mc.stop.assert_called_once() + + +class TestExecuteIsNotVacuous(unittest.TestCase): + """Anti-vacuity guard: if an injected failure did NOT surface, every + End2EndTest-based test in this repo would be worthless.""" + + def test_injected_failure_propagates(self): + from ovoscope import End2EndTest + + bus = FakeBus() + mc = SimpleNamespace(bus=bus, boot_messages=[]) + test = End2EndTest( + skill_ids=[], + source_message=Message("ovoscope.audit.r2.eof"), + expected_messages=[Message("ovoscope.audit.r2.never.emitted"), + Message("ovoscope.audit.r2.also.never")], + eof_msgs=["ovoscope.audit.r2.eof"], + verbose=False, + minicroft=mc, + ) + with self.assertRaises(AssertionError): + test._execute(timeout=5) + bus.close() + + +if __name__ == "__main__": + unittest.main() diff --git a/test/unittests/test_capture_session.py b/test/unittests/test_capture_session.py index f2ed5c5..bf9f6d6 100644 --- a/test/unittests/test_capture_session.py +++ b/test/unittests/test_capture_session.py @@ -1,24 +1,34 @@ -"""Unit tests for CaptureSession.""" +"""Unit tests for CaptureSession. + +CaptureSession only ever touches ``minicroft.bus``, so these tests drive a +``SimpleNamespace(bus=FakeBus())`` stub instead of booting a real MiniCroft. +A boot costs seconds and hundreds of MB of retained memory per test class, +and buys nothing here. + +Race-condition coverage for the same class lives in +``test_audit_round1.py::TestCaptureSessionRaces`` and +``test_audit_round2.py::TestCaptureSessionArming``. +""" import threading import unittest +from types import SimpleNamespace from ovos_bus_client.message import Message -from ovos_bus_client.session import Session +from ovos_utils.fakebus import FakeBus from ovos_utils.log import LOG -from ovoscope import CaptureSession, get_minicroft +from ovoscope import CaptureSession class TestCaptureSession(unittest.TestCase): - """CaptureSession is tested by emitting directly on MiniCroft's FakeBus.""" + """CaptureSession is tested by emitting directly on a stub FakeBus.""" def setUp(self): LOG.set_level("ERROR") - # empty MiniCroft — we drive the bus manually - self.mc = get_minicroft([]) + self.mc = SimpleNamespace(bus=FakeBus()) def tearDown(self): - self.mc.stop() + self.mc.bus.close() LOG.set_level("CRITICAL") # ------------------------------------------------------------------ diff --git a/test/unittests/test_cli.py b/test/unittests/test_cli.py index c1c9f3b..e57a968 100644 --- a/test/unittests/test_cli.py +++ b/test/unittests/test_cli.py @@ -147,7 +147,7 @@ class TestCmdValidate: def test_valid_fixture_returns_0(self): with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as f: json.dump({ - "source_message": {"type": "x", "data": {}, "context": {}}, + "source_message": [{"type": "x", "data": {}, "context": {}}], "expected_messages": [], }, f) path = f.name @@ -171,6 +171,47 @@ def test_invalid_fixture_returns_1(self): finally: os.unlink(path) + def test_uses_validate_fixture_when_pydantic_available(self): + """cmd_validate layers pydantic_helpers.validate_fixture on top of the + structural checks when the pydantic extra is importable.""" + with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as f: + json.dump({ + "source_message": [{"type": "x", "data": {}, "context": {}}], + "expected_messages": [], + }, f) + path = f.name + try: + parser = _build_parser() + args = parser.parse_args(["validate", path]) + mock_validate_fixture = MagicMock(return_value={}) + with patch("ovoscope.pydantic_helpers._PYDANTIC_AVAILABLE", True), \ + patch("ovoscope.pydantic_helpers.validate_fixture", mock_validate_fixture), \ + patch("ovoscope.cli._basic_validate") as mock_basic: + code = cmd_validate(args) + assert code == 0 + mock_validate_fixture.assert_called_once_with(path) + mock_basic.assert_called_once_with(path) + finally: + os.unlink(path) + + def test_falls_back_to_basic_validate_without_pydantic(self): + """cmd_validate must use _basic_validate when ovoscope.pydantic_helpers + (or ovos-pydantic-models underneath it) is not importable.""" + with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as f: + json.dump({ + "source_message": {"type": "x", "data": {}, "context": {}}, + "expected_messages": [], + }, f) + path = f.name + try: + parser = _build_parser() + args = parser.parse_args(["validate", path]) + with patch.dict(sys.modules, {"ovoscope.pydantic_helpers": None}): + code = cmd_validate(args) + assert code == 0 + finally: + os.unlink(path) + # --------------------------------------------------------------------------- # cmd_diff diff --git a/test/unittests/test_end2end.py b/test/unittests/test_end2end.py index 0fd8e38..f56000d 100644 --- a/test/unittests/test_end2end.py +++ b/test/unittests/test_end2end.py @@ -3,6 +3,7 @@ import os import tempfile import unittest +from unittest.mock import patch from ovos_bus_client.message import Message from ovos_bus_client.session import Session @@ -84,7 +85,7 @@ def _make_custom(msg_type: str, data=None, _FAILURE_SEQ = [ # message itself is index 0 (caller provides it) Message("mycroft.audio.play_sound", {"uri": "snd/error.mp3"}), - Message("complete_intent_failure", {}), + Message("ovos.intent.unmatched", {}), Message("ovos.utterance.handled", {}), ] @@ -147,7 +148,7 @@ def test_execute_returns_captured_messages(self): result = test.execute(timeout=10) types = [m.msg_type for m in result] self.assertIn("unittest.echo", types) - self.assertIn("speak", types) + self.assertIn("ovos.utterance.speak", types) self.assertIn("ovos.utterance.handled", types) def test_execute_result_length_matches_expected(self): @@ -165,7 +166,7 @@ def test_execute_result_length_matches_expected(self): source_message=src, expected_messages=[ src, - Message("speak", {"utterance": "count test"}), + Message("ovos.utterance.speak", {"utterance": "count test"}), Message("ovos.utterance.handled", {}), ], # filter out handler.start / handler.complete so count is 3 @@ -290,7 +291,7 @@ def test_disable_count_assertion_allows_count_mismatch(self): def test_ignore_messages_excluded_from_captured_list(self): """Messages in ignore_messages do not appear in the captured sequence.""" src = _make_custom("unittest.echo", {"text": "filter"}) - # Add "speak" to ignored — only 2 messages remain: src + eof + # Add "ovos.utterance.speak" to ignored — only 2 messages remain: src + eof test = End2EndTest( minicroft=self.mc, skill_ids=[SKILL_ID], @@ -299,7 +300,7 @@ def test_ignore_messages_excluded_from_captured_list(self): src, Message("ovos.utterance.handled", {}), ], - ignore_messages=["ovos.skills.settings_changed", "speak"] + ignore_messages=["ovos.skills.settings_changed", "ovos.utterance.speak"] + HANDLER_LIFECYCLE, test_routing=False, test_active_skills=False, @@ -332,7 +333,7 @@ def test_execute_creates_and_stops_minicroft_when_unmanaged(self): expected_messages=[ message, Message("mycroft.audio.play_sound", {"uri": "snd/error.mp3"}), - Message("complete_intent_failure", {}), + Message("ovos.intent.unmatched", {}), Message("ovos.utterance.handled", {}), ], flip_points=["recognizer_loop:utterance"], @@ -391,6 +392,29 @@ def test_assert_spoke_fails_wrong_utterance(self): with self.assertRaises(AssertionError): self._spoke_test("correct text").assert_spoke("WRONG TEXT", timeout=10) + def test_assert_spoke_accepts_canonical_only_captured_stream(self): + """assert_spoke matches a captured stream that only carries the + canonical "ovos.utterance.speak" topic (post-workshop#425 producers). + """ + test = self._spoke_test("canonical text") + captured = [ + Message("ovos.utterance.speak", + {"utterance": "canonical text", "lang": "en-US"}), + ] + with patch.object(End2EndTest, "execute", return_value=captured): + test.assert_spoke("canonical text", timeout=10) + + def test_assert_spoke_accepts_legacy_only_captured_stream(self): + """assert_spoke matches a captured stream that only carries the + legacy "speak" topic (pre-spec producer vintage on the wire). + """ + test = self._spoke_test("legacy text") + captured = [ + Message("speak", {"utterance": "legacy text", "lang": "en-US"}), + ] + with patch.object(End2EndTest, "execute", return_value=captured): + test.assert_spoke("legacy text", timeout=10) + # --------------------------------------------------------------------------- # Tests: serialization round-trip (serialize / deserialize / save / from_path) @@ -418,7 +442,7 @@ def _make_simple_test(self) -> End2EndTest: expected_messages=[ src, Message("mycroft.audio.play_sound", {"uri": "snd/error.mp3"}), - Message("complete_intent_failure", {}), + Message("ovos.intent.unmatched", {}), Message("ovos.utterance.handled", {}), ], flip_points=["recognizer_loop:utterance"], @@ -510,11 +534,11 @@ def test_multi_turn_two_failures(self): expected_messages=[ turn1, Message("mycroft.audio.play_sound", {"uri": "snd/error.mp3"}), - Message("complete_intent_failure", {}), + Message("ovos.intent.unmatched", {}), Message("ovos.utterance.handled", {}), turn2, Message("mycroft.audio.play_sound", {"uri": "snd/error.mp3"}), - Message("complete_intent_failure", {}), + Message("ovos.intent.unmatched", {}), Message("ovos.utterance.handled", {}), ], flip_points=["recognizer_loop:utterance"], diff --git a/test/unittests/test_end2end_extended.py b/test/unittests/test_end2end_extended.py index 0325861..785a0ee 100644 --- a/test/unittests/test_end2end_extended.py +++ b/test/unittests/test_end2end_extended.py @@ -580,13 +580,17 @@ def test_lang_restored_after_stop(self): # CaptureSession __del__ # --------------------------------------------------------------------------- class TestCaptureSessionDel(unittest.TestCase): + """__del__ touches only ``minicroft.bus`` — a stub is enough (and a real + MiniCroft boot here retained hundreds of MB per run).""" def setUp(self): + from types import SimpleNamespace + from ovos_utils.fakebus import FakeBus LOG.set_level("ERROR") - self.mc = get_minicroft([]) + self.mc = SimpleNamespace(bus=FakeBus()) def tearDown(self): - self.mc.stop() + self.mc.bus.close() LOG.set_level("CRITICAL") def test_del_calls_finish(self): @@ -620,7 +624,7 @@ def test_verbose_true_covers_print_branches(self): source_message=src, expected_messages=[ src, - Message("speak", {"utterance": "verbose"}), + Message("ovos.utterance.speak", {"utterance": "verbose"}), Message("ovos.utterance.handled", {}), ], ignore_messages=DEFAULT_IGNORED + HANDLER_LIFECYCLE, diff --git a/test/unittests/test_global_bus_coverage.py b/test/unittests/test_global_bus_coverage.py index f486f13..fd05209 100644 --- a/test/unittests/test_global_bus_coverage.py +++ b/test/unittests/test_global_bus_coverage.py @@ -60,19 +60,26 @@ def test_fakebus_patches_work(self): assert ovoscope.GLOBAL_BUS_COVERAGE_COLLECTOR.invocations["global.emit"] == 1 def test_tracker_snapshots_global_state(self): - """BusCoverageTracker should snapshot global state at __init__.""" + """BusCoverageTracker snapshots the collector as a BASELINE at __init__. + + Invocations that happened before the tracker existed belong to earlier + tests and must be subtracted, not inherited. + """ ovoscope.GLOBAL_BUS_COVERAGE = True ovoscope.GLOBAL_BUS_COVERAGE_COLLECTOR = ovoscope.GlobalBusCoverageCollector() ovoscope.GLOBAL_BUS_COVERAGE_COLLECTOR.record_invocation("boot.event") ovoscope.GLOBAL_BUS_COVERAGE_COLLECTOR.record_registration("boot.handler") - + bus = FakeBus() minicroft = MagicMock() minicroft.plugin_skills = {} - + tracker = BusCoverageTracker(bus, minicroft) - - assert tracker._global_invocations["boot.event"] == 1 + + # baseline holds the earlier count … + assert tracker._collector_baseline["boot.event"] == 1 + # … and the delta over that baseline is zero. + assert tracker._collector_delta() == {} assert tracker._global_registrations["boot.handler"] == 1 def test_tracker_merges_global_registrations(self): @@ -92,27 +99,28 @@ def test_tracker_merges_global_registrations(self): assert "boot.unclaimed" in tracker._registered["__core__"] def test_tracker_merges_global_invocations(self): - """build_report should sum global and local invocations.""" + """build_report sums THIS test's boot delta and its local invocations.""" ovoscope.GLOBAL_BUS_COVERAGE = True ovoscope.GLOBAL_BUS_COVERAGE_COLLECTOR = ovoscope.GlobalBusCoverageCollector() - ovoscope.GLOBAL_BUS_COVERAGE_COLLECTOR.record_invocation("shared.event") # 1x during boot - + bus = FakeBus() minicroft = MagicMock() minicroft.plugin_skills = {} - + tracker = BusCoverageTracker(bus, minicroft) + # boot happens AFTER the tracker exists, so it belongs to this test + ovoscope.GLOBAL_BUS_COVERAGE_COLLECTOR.record_invocation("shared.event") tracker.start_tracking() - bus.emit(Message("shared.event")) # 1x during test + bus.emit(Message("shared.event")) # 1x during test tracker.stop_tracking() - + # Manually register the listener so it shows up in report tracker._registered = {"__core__": {"shared.event": 1}} - + report = tracker.build_report() skill = next(s for s in report.skills if s.skill_id == "__core__") handler = next(h for h in skill.listeners if h.msg_type == "shared.event") - - # 1 (boot) + 1 (test) = 2 + + # 1 (this test's boot) + 1 (test) = 2 assert handler.invocation_count == 2 assert handler.covered is True diff --git a/test/unittests/test_gui_capture.py b/test/unittests/test_gui_capture.py index 70fc994..5027c18 100644 --- a/test/unittests/test_gui_capture.py +++ b/test/unittests/test_gui_capture.py @@ -95,7 +95,8 @@ def test_assert_page_shown_from_field(self) -> None: "gui.page.show", {"page_names": ["SYSTEM_clock"], "__from": "ovos-skill-date-time.openvoiceos"}, )] - session.assert_page_shown("date-time", "SYSTEM_clock") + session.assert_page_shown("ovos-skill-date-time.openvoiceos", + "SYSTEM_clock") def test_assert_namespace_has_key_from_field(self) -> None: """Value set using __from (real wire format).""" @@ -104,7 +105,8 @@ def test_assert_namespace_has_key_from_field(self) -> None: "gui.value.set", {"__from": "ovos-skill-weather.openvoiceos", "current_temp": 22}, )] - session.assert_namespace_has_key("weather", "current_temp") + session.assert_namespace_has_key("ovos-skill-weather.openvoiceos", + "current_temp") # -- assert_template_shown (SYSTEM_* template model) -- @@ -168,3 +170,65 @@ def test_assert_namespace_cleared_missing(self) -> None: if __name__ == "__main__": unittest.main() + + # -- near-match assertions must FAIL (round-2 audit) -- + + def test_page_shown_rejects_near_match_namespace(self) -> None: + """A namespace that merely CONTAINS the expected one must not pass.""" + session = self._make_session() + session.messages = [self._page_show_msg("weatherskill-extended", + "hello.qml")] + with self.assertRaises(AssertionError): + session.assert_page_shown("weatherskill", "hello.qml", timeout=0.1) + + def test_page_shown_rejects_near_match_page(self) -> None: + """A page name that merely CONTAINS the expected one must not pass.""" + session = self._make_session() + session.messages = [self._page_show_msg("weatherskill", + "hello_world.qml")] + with self.assertRaises(AssertionError): + session.assert_page_shown("weatherskill", "hello.qml", timeout=0.1) + + def test_page_shown_matches_page_basename(self) -> None: + """A directory prefix on the shown page does not change the result.""" + session = self._make_session() + session.messages = [self._page_show_msg("weatherskill", + "ui/hello.qml")] + session.assert_page_shown("weatherskill", "hello.qml") + + def test_page_shown_opt_in_prefix_matching(self) -> None: + """exact=False restores the old prefix/substring behaviour.""" + session = self._make_session() + session.messages = [self._page_show_msg("weatherskill-extended", + "hello_world.qml")] + session.assert_page_shown("weatherskill", "hello", exact=False) + + def test_namespace_value_rejects_near_match_namespace(self) -> None: + """assert_namespace_value must not pass on a containing namespace.""" + session = self._make_session() + session.messages = [self._value_set_msg("skill-extended", + {"greeting": "Hello!"})] + with self.assertRaises(AssertionError): + session.assert_namespace_value("skill", "greeting", "Hello!") + + def test_namespace_has_key_rejects_near_match_namespace(self) -> None: + """assert_namespace_has_key must not pass on a containing namespace.""" + session = self._make_session() + session.messages = [self._value_set_msg("skill-extended", {"key": 1})] + with self.assertRaises(AssertionError): + session.assert_namespace_has_key("skill", "key") + + def test_namespace_cleared_rejects_near_match_namespace(self) -> None: + """assert_namespace_cleared must not pass on a containing namespace.""" + session = self._make_session() + session.messages = [Message("gui.clear.namespace", + {"namespace": "skill-extended"})] + with self.assertRaises(AssertionError): + session.assert_namespace_cleared("skill") + + def test_namespace_cleared_exact(self) -> None: + """The matching namespace still passes.""" + session = self._make_session() + session.messages = [Message("gui.clear.namespace", + {"namespace": "skill"})] + session.assert_namespace_cleared("skill") diff --git a/test/unittests/test_minicroft.py b/test/unittests/test_minicroft.py index f48e4ff..70b18c7 100644 --- a/test/unittests/test_minicroft.py +++ b/test/unittests/test_minicroft.py @@ -84,6 +84,39 @@ def test_returns_minicroft_instance(self): mc.stop() +class TestMiniCroftSessionManagerBusRestore(unittest.TestCase): + """MiniCroft must not leak its FakeBus into the process-wide + SessionManager.bus class attribute after stop(). + + IntentService.__init__ calls SessionManager.connect_to_bus(self.bus), + clobbering SessionManager.bus with MiniCroft's FakeBus. If stop() doesn't + restore it, later tests in the same process — e.g. ones using a plain + FakeBus and calling SessionManager.wait_while_speaking() — hit the + `if not cls.bus` guard with a stale, truthy, dead bus and block/register + listeners on the wrong bus. + """ + + def setUp(self): + LOG.set_level("ERROR") + + def tearDown(self): + LOG.set_level("CRITICAL") + + def test_sessionmanager_bus_restored_after_stop(self): + sentinel = object() + SessionManager.bus = sentinel + try: + mc = get_minicroft([]) + # while running, MiniCroft's own FakeBus has taken over + self.assertIs(SessionManager.bus, mc.bus) + mc.stop() + self.assertIs(SessionManager.bus, sentinel, + "SessionManager.bus must be restored to its " + "pre-boot value after MiniCroft.stop()") + finally: + SessionManager.bus = None + + class TestMiniCroftPipelineIsolation(unittest.TestCase): """Tests for MiniCroft default_pipeline override.""" @@ -398,5 +431,45 @@ def test_no_bridging_isolates_legacy_from_spec(self): mc.stop() +class TestSkillManagerKwargCompat(unittest.TestCase): + """The latest ovoscope must boot against older ovos-core SkillManager + releases that predate newer ``enable_*`` keyword arguments (the stable + release channel ships ovos-core 1.3.x, whose SkillManager has no + ``enable_installer``). MiniCroft must forward only the flags the installed + SkillManager actually accepts instead of raising TypeError and failing to + boot.""" + + def test_unsupported_enable_kwargs_are_dropped(self): + import ovoscope + from unittest.mock import patch + + captured = {} + + class _Reached(Exception): + """Raised from the fake old __init__ once kwarg filtering passed.""" + + # Simulate an OLD SkillManager whose signature lacks enable_installer, + # enable_file_watcher, enable_intent_service and enable_event_scheduler. + def old_init(self, bus=None, enable_skill_api=True): + captured["bus"] = bus + captured["enable_skill_api"] = enable_skill_api + raise _Reached + + with patch.object(ovoscope.SkillManager, "__init__", old_init): + # If filtering failed, old_init would get enable_installer=... and + # raise TypeError *before its body* -> captured stays empty. + try: + MiniCroft([SKILL_ID]) + except Exception: + pass + + self.assertTrue( + captured, + "SkillManager.__init__ was never reached: an unsupported kwarg " + "was forwarded, so the latest ovoscope cannot boot on older core") + self.assertTrue(captured["enable_skill_api"], + "a supported enable_* flag must still be forwarded") + + if __name__ == "__main__": unittest.main() diff --git a/test/unittests/test_phal.py b/test/unittests/test_phal.py index c0e9c89..78cac3b 100644 --- a/test/unittests/test_phal.py +++ b/test/unittests/test_phal.py @@ -207,8 +207,24 @@ def factory(bus: FakeBus): # stale_instance was NOT loaded assert phal._loaded["dual-plugin"] is not stale_instance - def test_factory_raising_warns_and_skips_plugin(self): - """A factory that raises issues a warning and the plugin is not loaded.""" + def test_factory_raising_fails_the_harness(self): + """A factory that raises stops the harness instead of degrading. + + A silently skipped plugin used to resurface much later as an + unrelated assert_emitted timeout. + """ + def bad_factory(bus: FakeBus): + raise RuntimeError("factory error") + + with pytest.raises(RuntimeError, match="bad-plugin"): + with MiniPHAL( + plugin_ids=["bad-plugin"], + plugin_factories={"bad-plugin": bad_factory}, + ): + pass + + def test_factory_raising_warns_and_skips_plugin_when_tolerated(self): + """With tolerate_load_errors, the failure warns and the plugin is skipped.""" import warnings def bad_factory(bus: FakeBus): @@ -219,8 +235,10 @@ def bad_factory(bus: FakeBus): with MiniPHAL( plugin_ids=["bad-plugin"], plugin_factories={"bad-plugin": bad_factory}, + tolerate_load_errors=True, ) as phal: assert "bad-plugin" not in phal._loaded + assert phal.load_errors assert any("bad-plugin" in str(warning.message) for warning in w) def test_phal_test_plugin_factories_field(self): diff --git a/test/unittests/test_teardown_deadlock.py b/test/unittests/test_teardown_deadlock.py new file mode 100644 index 0000000..cd18647 --- /dev/null +++ b/test/unittests/test_teardown_deadlock.py @@ -0,0 +1,166 @@ +"""Regression test for a pyee re-entrant-lock deadlock during bus teardown. + +pyee's ``EventEmitter.remove_all_listeners()`` holds its internal, +non-reentrant ``threading.Lock`` while replacing ``self._events`` (or +``self._events[event]``). If dropping the emitter's reference to a +bound-method listener frees the *last* reference to that listener's owner, +the owner's ``__del__`` runs synchronously, inside the locked block. If that +``__del__`` calls ``bus.remove()`` / ``bus.ee.remove_listener()``, it tries +to re-acquire the same lock from the same thread -> permanent deadlock +(observed as 30-minute CI hangs in skill-repo ovoscope teardowns; see +https://github.com/OpenVoiceOS/ovoscope for the reported symptom and +ovos-skill-application-launcher's ``test/end2end/test_intents_en_us.py`` +minicroft fixture, merged PR #108 on dev, for the proven workaround this +mirrors). + +The fix (``MiniCroft.stop()`` in ``ovoscope/__init__.py``) drains +``bus.ee._events`` and runs ``gc.collect()`` *before* anything that can +trigger ``remove_all_listeners()``, so any reentrant ``__del__`` fires while +the pyee lock is free. + +Reproduction note +------------------ +The dependency versions pinned for this repo route ``MiniCroft.stop()`` +through ``FakeBus.close()``, whose ``on_close()`` is a no-op — it never +calls pyee's ``remove_all_listeners()`` directly, so in isolation +``test_minicroft_stop_completes_quickly`` passes even against unpatched +``ovoscope/__init__.py``. Under the *full* test-suite run, though, shared +process state (accumulated listeners/threads from earlier tests) is enough +to reproduce the real deadlock end-to-end: on unmodified +``ovoscope/__init__.py``, running the full ``test/unittests`` suite makes +``test_minicroft_stop_completes_quickly`` fail/hang; with the fix applied, +the full suite passes. So ``test_pyee_reentrant_del_deadlocks_without_drain`` +exercises the underlying pyee mechanism directly and deterministically +(proving (a) it deadlocks with no drain and (b) the exact drain sequence +used in ``MiniCroft.stop()`` prevents it), while +``test_minicroft_stop_completes_quickly`` is the smoke guard on the real +call path — deterministic only in full-suite context, so treat it as best +-effort in isolation but load-bearing in CI. +""" +import gc +import threading +import unittest + +import pyee + +from ovoscope import get_minicroft + + +class _ReentrantDeleter: + """An object whose __del__ re-enters the emitter's listener machinery. + + Simulates a listener owner (e.g. a skill/session helper) that + unregisters itself from the bus when garbage collected. If this + __del__ runs while pyee's remove_all_listeners() still holds its lock, + it deadlocks trying to re-acquire that same (non-reentrant) lock. + """ + + def __init__(self, emitter): + self._emitter = emitter + + def _noop(self, *_a, **_kw): + pass + + def __del__(self): + try: + self._emitter.remove_listener("probe", self._noop) + except Exception: + pass + + +def _drain(emitter) -> None: + """The exact workaround applied in MiniCroft.stop(): pop every key + out of pyee's listener dict ourselves, outside its lock, then force + any pending reentrant __del__ to run here (lock-free) via gc.collect(). + """ + events = getattr(emitter, "_events", None) + if events is not None: + for key in list(events.keys()): + events.pop(key, None) + gc.collect() + + +def _register_reentrant_listener(emitter) -> None: + """Register a listener whose *only* strong reference is the emitter's + own internal dict (a bound method keeps its instance alive only as + long as the bound-method object itself is alive).""" + deleter = _ReentrantDeleter(emitter) + emitter.on("probe", deleter._noop) + del deleter + gc.collect() # sanity: not collected yet, it's held by the listener dict + + +class TestPyeeReentrantDeleteDeadlock(unittest.TestCase): + """Directly exercises the pyee mechanism MiniCroft.stop() now guards + against, independent of which higher-level call path (bus.close(), + scheduler shutdown, a real websocket bus, ...) ends up invoking + remove_all_listeners().""" + + def test_pyee_reentrant_del_deadlocks_without_drain(self): + emitter = pyee.EventEmitter() + _register_reentrant_listener(emitter) + + result = {} + + def _run(): + emitter.remove_all_listeners() # no drain first -> deadlock + result["ok"] = True + + t = threading.Thread(target=_run, daemon=True) + t.start() + t.join(timeout=5) + + self.assertTrue( + t.is_alive(), + "expected unpatched remove_all_listeners() to deadlock on a " + "reentrant __del__, but it returned within 5s; the pyee " + "mechanism this fix guards against may have changed upstream", + ) + + def test_pyee_reentrant_del_does_not_deadlock_with_drain(self): + emitter = pyee.EventEmitter() + _register_reentrant_listener(emitter) + + result = {} + + def _run(): + _drain(emitter) # the MiniCroft.stop() workaround + emitter.remove_all_listeners() + result["ok"] = True + + t = threading.Thread(target=_run, daemon=True) + t.start() + t.join(timeout=10) + + self.assertFalse(t.is_alive(), "drain did not prevent the deadlock") + self.assertTrue(result.get("ok")) + + +class TestMiniCroftStopSmokeGuard(unittest.TestCase): + """MiniCroft.stop() must complete promptly. Kept as a smoke guard even + though this dependency stack's FakeBus.close() does not currently + route through pyee's remove_all_listeners() (see module docstring).""" + + def test_minicroft_stop_completes_quickly(self): + mc = get_minicroft([]) + # Register a listener via the *real* bus, in the same shape a live + # skill/session helper would, so the drain path in stop() is + # exercised against real state, not just an empty emitter. + _register_reentrant_listener(mc.bus.ee) + + result = {} + + def _run(): + mc.stop() + result["ok"] = True + + t = threading.Thread(target=_run, daemon=True) + t.start() + t.join(timeout=15) + + self.assertFalse(t.is_alive(), "MiniCroft.stop() did not return within 15s") + self.assertTrue(result.get("ok")) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/unittests/test_tts_lifecycle_nested_and_get_response.py b/test/unittests/test_tts_lifecycle_nested_and_get_response.py new file mode 100644 index 0000000..8430ea3 --- /dev/null +++ b/test/unittests/test_tts_lifecycle_nested_and_get_response.py @@ -0,0 +1,282 @@ +"""Regression tests for OpenVoiceOS/ovos-skill-alerts#138 (Update 3): + +ovoscope's synchronous FakeBus never completes the TTS handshake for +NESTED speak calls, and never resolves an unanswered get_response()/ +ask_yesno() flow, so real skill handlers hang under the harness. + +Both tests must complete quickly (well under the 15-20s upstream ceilings) +rather than hang. + +Also covers the follow-up defect found in adversarial review of ovoscope +PR #130: the FIRST version of the get_response watchdog kept ONE flat list +of pending timers, so ANY skill's `.disable` cancelled EVERY other skill's +still-pending watchdog too. In a fleet-style MiniCroft running multiple +skills concurrently, skill A finishing its (answered) get_response() would +silently disarm skill B's watchdog, and if B's question was never answered +it would hang forever again — exactly the bug this file exists to prevent. +The fix scopes each watchdog by (skill_id, session_id), the same two-part +scope `ovos_workshop`'s own +`@killable_event("mycroft.skills.abort_question", check_skill_id=True)` +already uses to decide which stalled thread an abort is actually for. +""" +import threading +import time +import unittest + +from ovos_bus_client.message import Message +from ovos_bus_client.session import Session +from ovos_workshop.skills.ovos import OVOSSkill +from ovos_utils.log import LOG + +from ovoscope import get_minicroft, MiniCroft + +NESTED_SKILL_ID = "ovoscope-unittest-nested-speak.test" +GETRESPONSE_SKILL_ID = "ovoscope-unittest-get-response.test" +CONCURRENT_SKILL_A = "ovoscope-unittest-get-response-a.test" +CONCURRENT_SKILL_B = "ovoscope-unittest-get-response-b.test" + + +class NestedSpeakSkill(OVOSSkill): + """Mirrors ovos-skill-alerts' _get_response_cascade shape: a handler that + calls speak_dialog(..., wait=True) from INSIDE another handler that itself + already spoke, so the second wait_while_speaking() has to resolve against + a speak emitted mid-handler on the same thread.""" + + def initialize(self): + self.add_event("unittest.nested_speak", self.handle_outer) + + def handle_inner(self, message: Message): + # Nested call: this speak_dialog happens DURING handle_outer, after + # handle_outer's own speak has already ducked/unducked once. + self.speak("inner reply", wait=True) + + def handle_outer(self, message: Message): + self.speak("outer reply", wait=True) + self.handle_inner(message) + self.bus.emit(message.forward("unittest.nested_speak.done")) + + +class GetResponseSkill(OVOSSkill): + """Calls get_response() and never gets an answer from the test — mirrors + ask_yesno()/get_response() call sites in ovos-skill-alerts that hang + forever with the OVOSSkill default num_retries=-1.""" + + def initialize(self): + self.add_event("unittest.ask_something", self.handle_ask) + + def handle_ask(self, message: Message): + ans = self.get_response("give me an answer") + self.bus.emit(message.forward("unittest.ask_something.done", + {"answer": ans})) + + +class TestNestedSpeakDialogWait(unittest.TestCase): + """Reproduces ovos-skill-alerts#138 mechanism (a).""" + + def setUp(self): + LOG.set_level("ERROR") + + def tearDown(self): + LOG.set_level("CRITICAL") + + def test_nested_speak_dialog_wait_completes_quickly(self): + mc = get_minicroft([NESTED_SKILL_ID], + extra_skills={NESTED_SKILL_ID: NestedSpeakSkill}) + try: + done = [] + mc.bus.on("unittest.nested_speak.done", lambda m: done.append(m)) + + start = time.time() + mc.bus.emit(Message("unittest.nested_speak")) + deadline = start + 10 + while not done and time.time() < deadline: + time.sleep(0.05) + elapsed = time.time() - start + + self.assertTrue(done, "nested speak_dialog(wait=True) never " + "completed within 10s (would be a hang " + "under the unfixed FakeBus, capped at " + "2x15s=30s upstream)") + # each wait_while_speaking should resolve off the mock-TTS's + # ~0.1s unduck timer, not burn its full 15s default timeout. + self.assertLess(elapsed, 2.0, + f"nested speak_dialog(wait=True) took {elapsed:.2f}s; " + f"expected < 2s if audio_output_end fires promptly " + f"for the nested speak too") + finally: + mc.stop() + + +class TestUnansweredGetResponse(unittest.TestCase): + """Reproduces ovos-skill-alerts#138 mechanism (b).""" + + def setUp(self): + LOG.set_level("ERROR") + + def tearDown(self): + LOG.set_level("CRITICAL") + + def test_unanswered_get_response_resolves_promptly(self): + mc = get_minicroft([GETRESPONSE_SKILL_ID], + extra_skills={GETRESPONSE_SKILL_ID: GetResponseSkill}) + try: + done = [] + mc.bus.on("unittest.ask_something.done", lambda m: done.append(m)) + + start = time.time() + mc.bus.emit(Message("unittest.ask_something")) + deadline = start + 10 + while not done and time.time() < deadline: + time.sleep(0.05) + elapsed = time.time() - start + + self.assertTrue(done, "get_response() with no injected answer " + "never resolved within 10s - this is the " + "unbounded OVOSSkill._wait_response() hang") + self.assertIsNone(done[0].data.get("answer"), + "unanswered get_response() should resolve to " + "None (aborted), not a fabricated answer") + self.assertLess(elapsed, 5.0, + f"unanswered get_response() took {elapsed:.2f}s; " + f"expected the watchdog abort well under 5s") + finally: + mc.stop() + + +class GetResponseSkillNamed(OVOSSkill): + """Same as GetResponseSkill but the class doesn't hardcode skill_id, so + two instances can be registered as two distinct skills in one MiniCroft + (extra_skills maps skill_id -> class, and OVOSSkill picks up skill_id + from the registration).""" + + def initialize(self): + # Skill-scoped event name: "unittest.ask_something" (shared, unscoped) + # would make BOTH skill instances react to a single emit, since + # add_event registers on the bus's global topic namespace, not per + # skill. Two skills firing off the SAME incoming message defeats the + # point of the concurrency test (it stops being "two independent + # requests," it becomes one request fanned out to both skills). + self.add_event(f"{self.skill_id}.ask_something", self.handle_ask) + + def handle_ask(self, message: Message): + ans = self.get_response("give me an answer") + self.bus.emit(message.forward(f"{self.skill_id}.ask_something.done", + {"answer": ans})) + + +class TestConcurrentGetResponseWatchdogScoping(unittest.TestCase): + """Whitebox: directly drive the enable/disable protocol messages the way + OVOSSkill.get_response() emits them, without waiting on real timers, to + pin the exact defect found in review: a flat timer list lets one skill's + `.disable` wipe out every other skill's still-armed watchdog.""" + + def setUp(self): + LOG.set_level("ERROR") + + def tearDown(self): + LOG.set_level("CRITICAL") + + @staticmethod + def _enable_message(skill_id: str, session_id: str) -> Message: + sess = Session(session_id=session_id) + return Message("skill.converse.get_response.enable", + {"skill_id": skill_id}, + {"session": sess.serialize()}) + + @staticmethod + def _disable_message(skill_id: str, session_id: str) -> Message: + sess = Session(session_id=session_id) + return Message("skill.converse.get_response.disable", + {"skill_id": skill_id}, + {"session": sess.serialize()}) + + def test_disabling_one_skill_does_not_cancel_another_skills_watchdog(self): + mc = get_minicroft([]) + try: + mc.bus.emit(self._enable_message("skillA", "sessA")) + mc.bus.emit(self._enable_message("skillB", "sessB")) + # Both must be armed before either is disabled. + self.assertEqual(2, len(mc._get_response_timers)) + + mc.bus.emit(self._disable_message("skillA", "sessA")) + + # THE DEFECT (flat list): this drops to 0 - skillB's watchdog is + # gone too. THE FIX (keyed by (skill_id, session_id)): only + # skillA's entry is removed, skillB's stays armed. + self.assertEqual( + 1, len(mc._get_response_timers), + "disabling skillA's get_response must not cancel skillB's " + "still-pending watchdog") + remaining_key = next(iter(mc._get_response_timers)) + self.assertEqual(("skillB", "sessB"), remaining_key) + self.assertTrue(mc._get_response_timers[remaining_key].is_alive()) + finally: + mc.stop() + + def test_unanswered_skill_still_aborted_when_another_skill_answers_first(self): + """End-to-end concurrent scenario: skill A gets answered quickly via + a real injected utterance; skill B never gets an answer. B's own + watchdog must still fire and abort it - A finishing first must not + silently leave B hanging.""" + mc = get_minicroft( + [CONCURRENT_SKILL_A, CONCURRENT_SKILL_B], + extra_skills={CONCURRENT_SKILL_A: GetResponseSkillNamed, + CONCURRENT_SKILL_B: GetResponseSkillNamed}) + try: + done_a, done_b = [], [] + mc.bus.on(f"{CONCURRENT_SKILL_A}.ask_something.done", + lambda m: done_a.append(m)) + mc.bus.on(f"{CONCURRENT_SKILL_B}.ask_something.done", + lambda m: done_b.append(m)) + + sess_a = Session(session_id="concurrent-sess-a") + sess_b = Session(session_id="concurrent-sess-b") + + # Kick off both get_response() calls concurrently on separate + # threads, mirroring two independent skills/sessions in flight + # in a real fleet-style MiniCroft at the same time. + t_a = threading.Thread( + target=lambda: mc.bus.emit(Message( + f"{CONCURRENT_SKILL_A}.ask_something", {}, + {"session": sess_a.serialize(), "skill_id": CONCURRENT_SKILL_A}))) + t_b = threading.Thread( + target=lambda: mc.bus.emit(Message( + f"{CONCURRENT_SKILL_B}.ask_something", {}, + {"session": sess_b.serialize(), "skill_id": CONCURRENT_SKILL_B}))) + t_a.start() + t_b.start() + + # Give both get_response() calls a moment to reach their + # listening state, then answer ONLY A - B is deliberately left + # unanswered so its own watchdog has to do the work. + time.sleep(0.3) + mc.bus.emit(Message( + f"{CONCURRENT_SKILL_A}.converse.get_response", + {"utterances": ["forty two"]}, + {"session": sess_a.serialize()})) + + t_a.join(timeout=10) + t_b.join(timeout=10) + + deadline = time.time() + 10 + while (not done_a or not done_b) and time.time() < deadline: + time.sleep(0.05) + + self.assertTrue(done_a, "skill A's answered get_response() " + "never resolved") + self.assertTrue(done_b, "skill B's unanswered get_response() " + "never resolved - its watchdog was " + "cancelled by skill A's .disable " + "(the flat-list defect)") + self.assertEqual("forty two", done_a[0].data.get("answer"), + "skill A's legitimate answer must survive " + "skill B's watchdog/abort handling untouched") + self.assertIsNone(done_b[0].data.get("answer"), + "skill B was never answered, so it must " + "resolve to None via its OWN watchdog abort") + finally: + mc.stop() + + +if __name__ == "__main__": + unittest.main() diff --git a/test/unittests/test_voice_loop.py b/test/unittests/test_voice_loop.py index 888360c..91e1ea5 100644 --- a/test/unittests/test_voice_loop.py +++ b/test/unittests/test_voice_loop.py @@ -29,6 +29,7 @@ from ovos_spec_tools import SpecMessage from ovoscope.voice_loop import ( + ListenerHarness, MiniHotwordContainer, MiniVoiceLoop, MockHotWordEngine, @@ -373,5 +374,45 @@ def test_no_bridging_isolates_legacy_from_spec(self): "spec topic must not fire with bridging off") +class TestAssertionHelpersAcceptCanonicalTopics(unittest.TestCase): + """The assert_*_emitted/suppressed helpers filter a catch-all-captured + stream (same defect class as ovoscope.assert_spoke) — they must accept + canonical spec topics, not just the legacy recognizer_loop:* spellings. + """ + + def test_assert_record_begin_emitted_accepts_canonical_only_stream(self): + harness = ListenerHarness() + captured = [Message("ovos.listener.record.started")] + harness.assert_record_begin_emitted(captured) # must not raise + + def test_assert_wakeword_detected_accepts_canonical_only_stream(self): + harness = ListenerHarness() + captured = [ + Message("recognizer_loop:wakeword"), # not migrated — legacy only + Message("ovos.listener.record.started"), + ] + harness.assert_wakeword_detected(captured) # must not raise + + def test_assert_utterance_emitted_accepts_canonical_only_stream(self): + harness = ListenerHarness() + captured = [Message("ovos.utterance.handle", {"utterances": ["hi"]})] + harness.assert_utterance_emitted("hi", captured) # must not raise + + def test_assert_wakeword_suppressed_still_fails_on_canonical_record_begin(self): + """Regression for the negative-assertion false-GREEN: a canonical-only + captured stream that DOES contain a record-begin must still fail + assert_wakeword_suppressed, not silently pass because the helper only + recognised the legacy spelling. + """ + harness = ListenerHarness() + captured = [Message("ovos.listener.record.started")] + with self.assertRaises(AssertionError): + harness.assert_wakeword_suppressed(captured) + + def test_assert_wakeword_suppressed_passes_on_truly_empty_stream(self): + harness = ListenerHarness() + harness.assert_wakeword_suppressed([]) # must not raise + + if __name__ == "__main__": unittest.main() diff --git a/test/unittests/test_wakeword_probe.py b/test/unittests/test_wakeword_probe.py new file mode 100644 index 0000000..3c35ee8 --- /dev/null +++ b/test/unittests/test_wakeword_probe.py @@ -0,0 +1,93 @@ +"""Tests for the per-clip WakeWordProbe (no real engine / no heavy deps).""" +import inspect +import unittest + +import numpy as np + +from ovoscope.wakeword_probe import ( + FRAME_SAMPLES, + PRIME_SECONDS, + SAMPLE_RATE, + WakeWordDetection, + WakeWordProbe, +) + + +class _FakeEngine: + """Fires once it has seen ``trigger_after`` update() calls. Resets on read.""" + + def __init__(self, trigger_after=None, takes_arg=False): + self.trigger_after = trigger_after + self._calls = 0 + self._fired = False + self.reset_count = 0 + # Build found_wake_word with or without the vestigial frame arg, so the + # probe's signature sniffing is exercised both ways. + if takes_arg: + def found_wake_word(frame_data=b""): + return self._read() + else: + def found_wake_word(): + return self._read() + self.found_wake_word = found_wake_word + + def update(self, chunk: bytes): + self._calls += 1 + if self.trigger_after is not None and self._calls >= self.trigger_after: + self._fired = True + + def _read(self): + fired, self._fired = self._fired, False + return fired + + def reset(self): + self.reset_count += 1 + self._calls = 0 + self._fired = False + + +class TestPrimePad(unittest.TestCase): + def test_leads_with_silence_and_pads_to_frames(self): + probe = WakeWordProbe(_FakeEngine()) + clip = np.ones(1000, dtype="float32") + out = probe.prime_pad(clip) + # whole number of frames + self.assertEqual(len(out) % FRAME_SAMPLES, 0) + # at least PRIME_SECONDS of leading silence before any signal + lead = int(SAMPLE_RATE * PRIME_SECONDS) + self.assertTrue(np.all(out[:lead] == 0.0)) + # the clip survives inside the padded buffer + self.assertGreaterEqual(len(out), lead + len(clip)) + + def test_default_prime_is_a_few_seconds(self): + self.assertGreaterEqual(PRIME_SECONDS, 2.5) + + +class TestDetect(unittest.TestCase): + def test_detects_and_reports_frames_and_latency(self): + engine = _FakeEngine(trigger_after=5) + probe = WakeWordProbe(engine) + result = probe.detect(np.zeros(SAMPLE_RATE, dtype="float32")) + self.assertIsInstance(result, WakeWordDetection) + self.assertTrue(result.detected) + self.assertEqual(result.frames_to_detection, 5) + self.assertGreaterEqual(result.latency_ms, 0.0) + self.assertEqual(engine.reset_count, 1) # reset before streaming + + def test_no_detection_returns_none_frames(self): + probe = WakeWordProbe(_FakeEngine(trigger_after=None)) + result = probe.detect(np.zeros(SAMPLE_RATE, dtype="float32")) + self.assertFalse(result.detected) + self.assertIsNone(result.frames_to_detection) + + def test_handles_found_wake_word_with_frame_arg(self): + engine = _FakeEngine(trigger_after=3, takes_arg=True) + self.assertGreaterEqual( + len(inspect.signature(engine.found_wake_word).parameters), 1) + result = WakeWordProbe(engine).detect( + np.zeros(SAMPLE_RATE, dtype="float32")) + self.assertTrue(result.detected) + + +if __name__ == "__main__": + unittest.main()