diff --git a/src/vocalinux/ui/settings_dialog.py b/src/vocalinux/ui/settings_dialog.py index b524ce5..dd54022 100644 --- a/src/vocalinux/ui/settings_dialog.py +++ b/src/vocalinux/ui/settings_dialog.py @@ -4780,7 +4780,7 @@ class SettingsDialog(Gtk.Dialog): cancel_check_id = GLib.timeout_add(100, check_cancelled) try: - self._apply_settings_internal(settings) + self._apply_settings_internal(settings, raise_errors=True) GLib.idle_add(download_dialog.set_complete, True, "") GLib.idle_add(self._populate_model_options) finally: @@ -5070,7 +5070,7 @@ For now, the engine has been reverted to VOSK.""" cancel_check_id = GLib.timeout_add(100, check_cancelled) try: - self._apply_settings_internal(settings) + self._apply_settings_internal(settings, raise_errors=True) GLib.idle_add(download_dialog.set_complete, True, "") finally: GLib.source_remove(cancel_check_id) @@ -5095,8 +5095,16 @@ For now, the engine has been reverted to VOSK.""" return self._apply_settings_internal(settings) - def _apply_settings_internal(self, settings: dict) -> bool: - """Internal method to apply settings.""" + def _apply_settings_internal(self, settings: dict, raise_errors: bool = False) -> bool: + """Internal method to apply settings. + + Args: + settings: The settings to persist and hand to the engine. + raise_errors: Re-raise failures instead of showing an error dialog. + The download threads pass True: their own handlers report the + failure through the progress dialog, and building a Gtk dialog + off the main loop is not safe anyway. + """ try: self._save_selected_settings(settings) @@ -5111,6 +5119,8 @@ For now, the engine has been reverted to VOSK.""" return True except Exception as e: logger.error(f"Failed to apply settings: {e}", exc_info=True) + if raise_errors: + raise if "whisper" in str(e).lower() and "no module named" in str(e).lower(): self._show_whisper_install_dialog() diff --git a/tests/test_download_error_reporting.py b/tests/test_download_error_reporting.py new file mode 100644 index 0000000..3c7942c --- /dev/null +++ b/tests/test_download_error_reporting.py @@ -0,0 +1,101 @@ +"""Tests for reporting failed model downloads instead of swallowing them.""" + +import importlib +import sys +from unittest.mock import MagicMock, Mock, patch + +import pytest + + +def _load_settings_dialog(): + """Import settings_dialog with real base classes for its GTK subclasses. + + conftest swaps gi for a MagicMock, which leaves every ``class X(Gtk.Y)`` in + the module as a mock and makes its methods unreachable. Handing the three + bases the module subclasses a real class keeps the classes intact, while + the rest of GTK stays mocked. + """ + repository = sys.modules["gi.repository"] + bases = {name: type(name, (), {}) for name in ("Box", "ListBoxRow", "Dialog")} + saved = sys.modules.pop("vocalinux.ui.settings_dialog", None) + try: + with patch.object(repository, "Gtk", MagicMock(**bases)): + module = importlib.import_module("vocalinux.ui.settings_dialog") + finally: + sys.modules.pop("vocalinux.ui.settings_dialog", None) + if saved is not None: + sys.modules["vocalinux.ui.settings_dialog"] = saved + return module + + +settings_dialog = _load_settings_dialog() +SettingsDialog = settings_dialog.SettingsDialog + + +def _dialog_stub(): + dialog = Mock() + dialog.speech_engine.state = "idle" + return dialog + + +def test_download_threads_get_the_failure(): + """With raise_errors the caller's handler sees the exception. + + The download threads rely on this: their except blocks turn the failure + into set_complete(False, reason) on the progress dialog. Without it they + reported "Complete!" for downloads that were cancelled or never finished. + """ + dialog = _dialog_stub() + dialog.speech_engine.reconfigure.side_effect = RuntimeError("Download cancelled") + + with pytest.raises(RuntimeError, match="Download cancelled"): + SettingsDialog._apply_settings_internal(dialog, {"engine": "vosk"}, raise_errors=True) + + +def test_no_gtk_dialog_is_built_when_errors_are_raised(): + """The failing thread path must not touch Gtk — that is the caller's job. + + Building and running a Gtk.MessageDialog off the main loop is what the + swallowed-error path used to do from the download thread. + """ + dialog = _dialog_stub() + dialog.speech_engine.reconfigure.side_effect = RuntimeError("boom") + + with patch.object(settings_dialog, "Gtk") as gtk: + with pytest.raises(RuntimeError): + SettingsDialog._apply_settings_internal(dialog, {"engine": "vosk"}, raise_errors=True) + + gtk.MessageDialog.assert_not_called() + + +def test_main_loop_callers_keep_the_dialog_and_the_false_return(): + """Without the flag the old contract stays: swallow, show, return False.""" + dialog = _dialog_stub() + dialog.speech_engine.reconfigure.side_effect = RuntimeError("boom") + + with patch.object(settings_dialog, "Gtk") as gtk: + result = SettingsDialog._apply_settings_internal(dialog, {"engine": "vosk"}) + + assert result is False + gtk.MessageDialog.assert_called_once() + + +def test_success_still_returns_true(): + dialog = _dialog_stub() + + assert SettingsDialog._apply_settings_internal(dialog, {"engine": "vosk"}) is True + assert ( + SettingsDialog._apply_settings_internal(dialog, {"engine": "vosk"}, raise_errors=True) + is True + ) + + +def test_both_download_threads_ask_for_the_failure(): + """Source guard: every _apply_settings_internal call made off the main loop + passes raise_errors=True, so a regression cannot silently reintroduce the + swallowed-error path in a download thread.""" + import inspect + + source = inspect.getsource(settings_dialog) + thread_calls = source.count("_apply_settings_internal(settings, raise_errors=True)") + assert thread_calls == 2