Run project tests and debug failures #9
Conversation
Add module-level skip logic to tests that depend on sounddevice/PortAudio. When the PortAudio system library is not installed, these test modules will be skipped gracefully instead of failing at import time. Changes: - Add PortAudio availability check in conftest.py - Update 6 test modules to skip when PortAudio is missing - Tests now run successfully even without PortAudio installed
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Pull Request Overview
This PR adds graceful handling for missing PortAudio dependencies in the test suite. When the PortAudio system library is not installed, test modules that depend on sounddevice will now be skipped at the module level instead of failing at import time.
- Added
is_portaudio_available()check function andrequires_portaudiomarker in conftest.py - Implemented module-level skip logic in 6 test files that transitively depend on sounddevice
- Updated
mock_oscillatorfixture to skip when PortAudio is unavailable
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/conftest.py | Added PortAudio availability check function, pytest marker, and conditional skip in mock_oscillator fixture |
| tests/test_synth.py | Added module-level skip when sounddevice/PortAudio is not available |
| tests/test_controller.py | Added module-level skip when sounddevice/PortAudio is not available |
| tests/test_midi_integration.py | Added module-level skip when sounddevice/PortAudio is not available, moved pytest import to top |
| tests/test_keyboard_integration.py | Added module-level skip when sounddevice/PortAudio is not available, reordered pytest import |
| tests/test_input.py | Added module-level skip when sounddevice/PortAudio is not available, reordered pytest import |
| tests/test_arpeggiator.py | Added module-level skip when sounddevice/PortAudio is not available |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Create a pytest marker for tests requiring PortAudio | ||
| requires_portaudio = pytest.mark.skipif( | ||
| not is_portaudio_available(), | ||
| reason="PortAudio library not found (install portaudio19-dev system package)" | ||
| ) |
There was a problem hiding this comment.
[nitpick] A requires_portaudio marker is defined here but is never used. The test modules use module-level pytest.skip() instead. This creates two different mechanisms for the same purpose, which could lead to confusion.
Consider either:
- Removing this unused marker if module-level skips are the preferred approach, or
- Using this marker with
@requires_portaudiodecorators on test functions/classes instead of module-level skips
The current implementation works correctly, but having both mechanisms reduces code clarity.
| # Create a pytest marker for tests requiring PortAudio | |
| requires_portaudio = pytest.mark.skipif( | |
| not is_portaudio_available(), | |
| reason="PortAudio library not found (install portaudio19-dev system package)" | |
| ) |
|
|
||
| # Skip entire module if sounddevice/PortAudio is not available | ||
| try: | ||
| import sounddevice |
There was a problem hiding this comment.
Import of 'sounddevice' is not used.
|
|
||
| # Skip entire module if sounddevice/PortAudio is not available | ||
| try: | ||
| import sounddevice |
There was a problem hiding this comment.
Import of 'sounddevice' is not used.
|
|
||
| # Skip entire module if sounddevice/PortAudio is not available | ||
| try: | ||
| import sounddevice |
There was a problem hiding this comment.
Import of 'sounddevice' is not used.
|
|
||
| # Skip entire module if sounddevice/PortAudio is not available | ||
| try: | ||
| import sounddevice |
There was a problem hiding this comment.
Import of 'sounddevice' is not used.
|
|
||
| # Skip entire module if sounddevice/PortAudio is not available | ||
| try: | ||
| import sounddevice |
There was a problem hiding this comment.
Import of 'sounddevice' is not used.
|
|
||
| # Skip entire module if sounddevice/PortAudio is not available | ||
| try: | ||
| import sounddevice |
There was a problem hiding this comment.
Import of 'sounddevice' is not used.
Add module-level skip logic to tests that depend on sounddevice/PortAudio. When the PortAudio system library is not installed, these test modules will be skipped gracefully instead of failing at import time.
Changes: