Skip to content

Commit 5e9f267

Browse files
JarbasAlclaude
andcommitted
fix: address CodeRabbit review feedback
Docs: - AUDIT.md: fix typo "Pining" → "Pinning" - FAQ.md: fix install/test paths (ovos-audio/ → . / test/) - QUICK_FACTS.md: update version 1.1.1a2 → 1.1.2a1 Production bug fix: - service.py:395: _maybe_reload_tts compared _fallback_tts_hash to ftts_m (module name string) instead of _ftts_hash (computed hash integer), causing the fallback TTS to always reload on every config check Tests: - test_playback_play.py: capture _now_playing[4] before _play() clears it; move stray `import unittest.mock` from after main() to top-level ANY import - test_utils.py: narrow guarded import to only ovos_utils.signal and signal-based helpers; split into TestSignalInterface (skipped when signal unavailable) and TestStopSpeaking (also skipped — stop_speaking was removed with the signal API); stops hiding real ImportErrors from unrelated modules Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent be1f99e commit 5e9f267

7 files changed

Lines changed: 21 additions & 12 deletions

File tree

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,4 @@ dist
1919
# Created by unit tests
2020
.pytest_cache/
2121
/.gtm/
22+
.env

AUDIT.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
## Technical Debt & Issues
2323
- **Transitional Architecture**: Currently in a transitional state between the legacy OCP (OpenVoiceOS Common Play) system and the upcoming `ovos-media` stack.
2424
- **Legacy Support**: Maintains significant code for legacy audio service support, which is deprecated but still default in some configurations.
25-
- **Dependency Pining**: Dependencies in `pyproject.toml` use loose upper bounds (e.g., `<3.0.0`), which might lead to breaking changes if sub-dependencies don't follow semver strictly.
25+
- **Dependency Pinning**: Dependencies in `pyproject.toml` use loose upper bounds (e.g., `<3.0.0`), which might lead to breaking changes if sub-dependencies don't follow semver strictly.
2626
- **Dynamic Versioning**: Uses `ovos_audio.version` for dynamic versioning, which is standard but requires manual updates to `version.py`.
2727

2828
## Next Steps

FAQ.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,15 @@ pip install ovos-audio
3434
```
3535
Or for development:
3636
```bash
37-
uv pip install -e ovos-audio/
37+
uv pip install -e .
3838
```
3939

4040
## Where do I report bugs?
4141
Open an issue on the GitHub repository. Ensure you are targeting the `dev` branch for fixes.
4242

4343
## How do I run tests?
4444
```bash
45-
uv run pytest ovos-audio/test/ --cov=ovos_audio
45+
uv run pytest test/ --cov=ovos_audio
4646
```
4747

4848
## How do I contribute?

QUICK_FACTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ ovos-core audio daemon client
66
| Feature | Details |
77
|---------|---------|
88
| Package Name | `ovos-audio` |
9-
| Version | `1.1.1a2` |
9+
| Version | `1.1.2a1` |
1010
| License | Apache-2.0 |
1111
| Repository | [https://github.com/OpenVoiceOS/ovos-audio](https://github.com/OpenVoiceOS/ovos-audio) |
1212
| Python Support | >=3.9 |

ovos_audio/service.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ def _maybe_reload_tts(self):
392392
return
393393

394394
if not self._fallback_tts_hash or \
395-
self._fallback_tts_hash != ftts_m:
395+
self._fallback_tts_hash != _ftts_hash:
396396
with self.lock:
397397
if self.fallback_tts:
398398
self.fallback_tts.shutdown()

test/unittests/test_playback_play.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
"""
99
import queue
1010
import unittest
11-
from unittest.mock import MagicMock, patch, call
11+
from unittest.mock import MagicMock, patch, call, ANY
1212

1313
from ovos_bus_client.message import Message
1414

@@ -140,9 +140,10 @@ def test_play_waits_for_process(self):
140140

141141
def test_play_calls_on_end_when_queue_empty(self):
142142
t, mock_proc = self._setup_thread_for_play(listen=True)
143+
expected_msg = t._now_playing[4] # capture before _play() clears _now_playing
143144
with patch("ovos_audio.playback.play_audio", return_value=mock_proc):
144145
t._play()
145-
t.on_end.assert_called_once_with(True, t._now_playing[4] if t._now_playing else unittest.mock.ANY)
146+
t.on_end.assert_called_once_with(True, expected_msg)
146147

147148
def test_play_clears_now_playing(self):
148149
t, mock_proc = self._setup_thread_for_play()
@@ -233,5 +234,3 @@ def boom_then_stop(timeout=None):
233234

234235
if __name__ == "__main__":
235236
unittest.main()
236-
237-
import unittest.mock # ensure import for the mock reference in test_play_calls_on_end

test/unittests/test_utils.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,15 @@
1919

2020
from os.path import exists
2121

22+
# ovos_utils.signal and the signal-based helpers in ovos_audio.utils were
23+
# removed in ovos-utils>=0.8.5. Guard all of their imports together so
24+
# collection never fails; tests are individually skipped when unavailable.
2225
try:
2326
from ovos_utils.signal import create_signal, check_for_signal
2427
from ovos_utils.file_utils import get_temp_path
2528
from ovos_audio.utils import wait_while_speaking, is_speaking, stop_speaking
2629
_SIGNAL_API_AVAILABLE = True
27-
except (ImportError, AttributeError):
30+
except ImportError:
2831
_SIGNAL_API_AVAILABLE = False
2932

3033

@@ -38,7 +41,9 @@ def wait_while_speaking_thread():
3841

3942

4043
@unittest.skipUnless(_SIGNAL_API_AVAILABLE, "ovos_utils.signal removed in ovos-utils>=0.8.5")
41-
class TestInterface(unittest.TestCase):
44+
class TestSignalInterface(unittest.TestCase):
45+
"""Tests that require the legacy ovos_utils.signal API."""
46+
4247
def setUp(self):
4348
if exists(get_temp_path('mycroft')):
4449
rmtree(get_temp_path('mycroft'))
@@ -60,14 +65,18 @@ def test_wait_while_speaking(self):
6065
sleep(2)
6166
self.assertTrue(done_waiting)
6267

68+
69+
@unittest.skipUnless(_SIGNAL_API_AVAILABLE, "ovos_audio.utils signal helpers removed in ovos-utils>=0.8.5")
70+
class TestStopSpeaking(unittest.TestCase):
71+
"""Tests for stop_speaking() — requires signal-based ovos_audio.utils."""
72+
6373
@mock.patch('ovos_audio.utils.is_speaking')
6474
@mock.patch('ovos_audio.utils.send')
6575
def test_stop_speaking(self, mock_send, mock_is_speaking):
6676
"""Test that stop speak message is sent."""
6777
mock_is_speaking.return_value = True
6878
stop_speaking()
6979
mock_send.assert_called()
70-
#mock_send.assert_called_with('mycroft.audio.speech.stop')
7180

7281
@mock.patch('ovos_audio.utils.is_speaking')
7382
@mock.patch('ovos_audio.utils.send')

0 commit comments

Comments
 (0)