diff --git a/ovoscope/__init__.py b/ovoscope/__init__.py index 7b28537..ce46824 100644 --- a/ovoscope/__init__.py +++ b/ovoscope/__init__.py @@ -978,7 +978,10 @@ def get_minicroft(skill_ids: Union[List[str], str], *args, ) sleep(0.1) return croft - except Exception: + except BaseException: + # pytest-timeout's Failed and KeyboardInterrupt derive from + # BaseException, not Exception; catching only Exception here let + # them skip cleanup and leak the started MiniCroft process. croft.stop() raise diff --git a/test/unittests/test_minicroft.py b/test/unittests/test_minicroft.py index 70b18c7..a49fce6 100644 --- a/test/unittests/test_minicroft.py +++ b/test/unittests/test_minicroft.py @@ -1,6 +1,7 @@ """Unit tests for MiniCroft and get_minicroft().""" import threading import unittest +from unittest.mock import patch from ovos_bus_client.message import Message from ovos_spec_tools import SpecMessage @@ -83,6 +84,18 @@ def test_returns_minicroft_instance(self): finally: mc.stop() + def test_basedexception_during_boot_still_stops_croft(self): + """A BaseException (e.g. pytest-timeout's Failed, or a real + KeyboardInterrupt) raised while waiting for READY must still trigger + croft.stop() before propagating. Regression test for get_minicroft's + cleanup handler only catching `Exception`, which let BaseException + subclasses skip cleanup and leak the started MiniCroft process.""" + with patch.object(MiniCroft, "start", side_effect=KeyboardInterrupt), \ + patch.object(MiniCroft, "stop") as mock_stop: + with self.assertRaises(KeyboardInterrupt): + get_minicroft([]) + mock_stop.assert_called_once() + class TestMiniCroftSessionManagerBusRestore(unittest.TestCase): """MiniCroft must not leak its FakeBus into the process-wide