diff --git a/ovos_core/skill_manager.py b/ovos_core/skill_manager.py index 9168a9cbb7a2..6d994dbaa5e6 100644 --- a/ovos_core/skill_manager.py +++ b/ovos_core/skill_manager.py @@ -689,8 +689,10 @@ def deactivate_skill(self, message): if message.data['skill'] == skill_loader.skill_id: LOG.info("Deactivating skill: " + skill_loader.skill_id) skill_loader.deactivate() - except Exception: + self.bus.emit(message.response()) + except Exception as err: LOG.exception('Failed to deactivate ' + message.data['skill']) + self.bus.emit(message.response({'error': f'failed: {err}'})) def deactivate_except(self, message): """Deactivate all skills except the provided.""" @@ -715,8 +717,10 @@ def activate_skill(self, message): if (message.data['skill'] in ('all', skill_loader.skill_id) and not skill_loader.active): skill_loader.activate() - except Exception: - LOG.exception('Couldn\'t activate skill') + self.bus.emit(message.response()) + except Exception as err: + LOG.exception(f'Couldn\'t activate skill {message.data["skill"]}') + self.bus.emit(message.response({'error': f'failed: {err}'})) def stop(self): """Tell the manager to shutdown.""" diff --git a/test/unittests/skills/test_skill_manager.py b/test/unittests/skills/test_skill_manager.py index 3790932c0c3d..7737bcb3d6b5 100644 --- a/test/unittests/skills/test_skill_manager.py +++ b/test/unittests/skills/test_skill_manager.py @@ -18,6 +18,7 @@ from mycroft.skills.skill_loader import SkillLoader from mycroft.skills.skill_manager import SkillManager, UploadQueue from ..base import MycroftUnitTestBase +from ovos_bus_client.message import Message class TestUploadQueue(TestCase): @@ -153,14 +154,25 @@ def test_handle_paired(self): reload_skills_manifest=True) def test_deactivate_skill(self): - message = Mock() - message.data = dict(skill='test_skill') + message = Message("test.message", {'skill': 'test_skill'}) + message.response = Mock() self.skill_manager.deactivate_skill(message) - self.skill_loader_mock.deactivate.assert_called_once_with() + self.skill_loader_mock.deactivate.assert_called_once() + message.response.assert_called_once() + + @patch("ovos_utils.log.LOG.exception") + def test_deactivate_skill_exception(self, mock_exception_logger): + message = Message("test.message", {'skill': 'test_skill'}) + message.response = Mock() + self.skill_loader_mock.deactivate.side_effect = Exception() + self.skill_manager.deactivate_skill(message) + self.skill_loader_mock.deactivate.assert_called_once() + message.response.assert_called_once() + mock_exception_logger.assert_called_once_with('Failed to deactivate test_skill') def test_deactivate_except(self): - message = Mock() - message.data = dict(skill='test_skill') + message = Message("test.message", {'skill': 'test_skill'}) + message.response = Mock() self.skill_loader_mock.active = True foo_skill_loader = Mock(spec=SkillLoader) foo_skill_loader.skill_id = 'foo' @@ -173,14 +185,30 @@ def test_deactivate_except(self): self.skill_manager.skill_loaders['test_skill'] = test_skill_loader self.skill_manager.deactivate_except(message) - foo_skill_loader.deactivate.assert_called_once_with() - foo2_skill_loader.deactivate.assert_called_once_with() + foo_skill_loader.deactivate.assert_called_once() + foo2_skill_loader.deactivate.assert_called_once() self.assertFalse(test_skill_loader.deactivate.called) def test_activate_skill(self): - message = Mock() - message.data = dict(skill='test_skill') + message = Message("test.message", {'skill': 'test_skill'}) + message.response = Mock() + test_skill_loader = Mock(spec=SkillLoader) + test_skill_loader.skill_id = 'test_skill' + test_skill_loader.active = False + + self.skill_manager.skill_loaders = {} + self.skill_manager.skill_loaders['test_skill'] = test_skill_loader + + self.skill_manager.activate_skill(message) + test_skill_loader.activate.assert_called_once() + message.response.assert_called_once() + + @patch("ovos_utils.log.LOG.exception") + def test_activate_skill_exception(self, mock_exception_logger): + message = Message("test.message", {'skill': 'test_skill'}) + message.response = Mock() test_skill_loader = Mock(spec=SkillLoader) + test_skill_loader.activate.side_effect = Exception() test_skill_loader.skill_id = 'test_skill' test_skill_loader.active = False @@ -188,4 +216,6 @@ def test_activate_skill(self): self.skill_manager.skill_loaders['test_skill'] = test_skill_loader self.skill_manager.activate_skill(message) - test_skill_loader.activate.assert_called_once_with() + test_skill_loader.activate.assert_called_once() + message.response.assert_called_once() + mock_exception_logger.assert_called_once_with('Couldn\'t activate skill test_skill')