Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the test suite to use pytest, replacing manual test scripts with proper unit tests and adding CI integration. It introduces new test coverage for TTS streaming functionality and tool calling error handling while improving test organization and maintainability.
- Migrated existing test scripts to pytest-based unit tests with proper fixtures and assertions
- Added new tests for TTS streaming response updates and tool calling delegation
- Integrated pytest into the build system with Makefile target and GitHub Actions CI workflow
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_tts_streaming.py | New pytest-based tests for TTS streaming response state management |
| tests/test_tool_calling.py | New tests for tool caller delegation and error handling |
| tests/test_message_bus.py | Refactored message bus tests to use pytest fixtures and proper assertions |
| tests/test_interruptible_conversation.py | Converted conversation manager tests to pytest format with mocking |
| Makefile | Added pytest test target for local development |
| .github/workflows/ci.yml | New CI workflow running pytest on Python 3.11 |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| sys.modules.setdefault('sounddevice', types.SimpleNamespace()) | ||
| sys.modules.setdefault('soundfile', types.SimpleNamespace()) | ||
| sys.modules.setdefault('psutil', types.SimpleNamespace()) | ||
| sys.modules.setdefault('requests', types.SimpleNamespace(get=lambda *a, **k: None, post=lambda *a, **k: None)) |
There was a problem hiding this comment.
Module stubbing with sys.modules is duplicated across test files. Consider moving this to a shared test utility or conftest.py to reduce duplication.
| sys.modules.setdefault('sounddevice', types.SimpleNamespace(OutputStream=object)) | ||
| sys.modules.setdefault('soundfile', types.SimpleNamespace()) |
There was a problem hiding this comment.
Module stubbing with sys.modules is duplicated across test files. Consider moving this to a shared test utility or conftest.py to reduce duplication.
| with: | ||
| python-version: '3.11' | ||
| - run: python -m pip install --upgrade pip | ||
| - run: pip install -r requirements-dev.txt |
There was a problem hiding this comment.
The CI workflow references 'requirements-dev.txt' but this file is not included in the PR. This will cause the CI build to fail if the file doesn't exist.
| - run: pip install -r requirements-dev.txt | |
| - run: if [ -f requirements-dev.txt ]; then pip install -r requirements-dev.txt; fi |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| - uses: actions/checkout@v4 | ||
| - uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: '3.11' | ||
| - run: python -m pip install --upgrade pip | ||
| - run: pip install -r requirements-dev.txt | ||
| - run: pytest |
There was a problem hiding this comment.
[P1] Install base requirements before running pytest
The CI workflow only installs requirements-dev.txt before executing pytest. That file contains tooling dependencies but omits the project’s runtime libraries such as numpy, requests, and psutil defined in requirements.txt. On a clean GitHub runner these packages are absent, so importing modules inside the tests will raise ModuleNotFoundError and the CI job will fail immediately. Please install the core requirements (e.g. pip install -r requirements.txt) before or in addition to the dev extras.
Useful? React with 👍 / 👎.
lukifer23
deleted the
codex/convert-tests-to-pytest-style-and-add-features
branch
Summary
Testing
pytest -vvhttps://chatgpt.com/codex/tasks/task_e_68bdf3f8a6888323a132c61a043ce84d