Skip to content

Commit a8b4ed5

Browse files
feat: send bus message on skill load (#727)
* feat: send bus message on skill load * Update ovos_core/skill_manager.py Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update test/unittests/test_skill_manager.py Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix AI suggestion formatting --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 parent 7e44649 commit a8b4ed5

File tree

2 files changed

+104
-0
lines changed

2 files changed

+104
-0
lines changed

ovos_core/skill_manager.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,8 @@ def _load_plugin_skill(self, skill_id, skill_plugin):
355355
skill_loader = self._get_plugin_skill_loader(skill_id, skill_class=skill_plugin)
356356
try:
357357
load_status = skill_loader.load(skill_plugin)
358+
if load_status:
359+
self.bus.emit(Message("mycroft.skill.loaded", {"skill_id": skill_id}))
358360
except Exception:
359361
LOG.exception(f'Load of skill {skill_id} failed!')
360362
load_status = False

test/unittests/test_skill_manager.py

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,3 +175,105 @@ def test_activate_skill(self):
175175
self.skill_manager.activate_skill(message)
176176
test_skill_loader.activate.assert_called_once()
177177
message.response.assert_called_once()
178+
179+
def test_load_plugin_skill_success(self):
180+
"""Test successful plugin skill loading emits the correct message."""
181+
skill_id = 'test.plugin.skill'
182+
mock_plugin = Mock()
183+
184+
# Setup mock loader following existing patterns
185+
mock_loader = Mock(spec=SkillLoader)
186+
mock_loader.skill_id = skill_id
187+
mock_loader.load.return_value = True
188+
189+
# Mock _get_plugin_skill_loader to return our mock
190+
self.skill_manager._get_plugin_skill_loader = Mock(return_value=mock_loader)
191+
192+
# Reset message tracking
193+
self.message_bus_mock.message_types = []
194+
self.message_bus_mock.message_data = []
195+
self.skill_manager.plugin_skills = {}
196+
197+
# Call the method
198+
result = self.skill_manager._load_plugin_skill(skill_id, mock_plugin)
199+
200+
# Verify message was emitted
201+
self.assertIn('mycroft.skill.loaded', self.message_bus_mock.message_types)
202+
loaded_msg_idx = self.message_bus_mock.message_types.index('mycroft.skill.loaded')
203+
self.assertEqual(
204+
{'skill_id': skill_id},
205+
self.message_bus_mock.message_data[loaded_msg_idx]
206+
)
207+
208+
# Verify loader was called
209+
mock_loader.load.assert_called_once_with(mock_plugin)
210+
211+
# Verify skill was added to plugin_skills
212+
self.assertIn(skill_id, self.skill_manager.plugin_skills)
213+
self.assertEqual(mock_loader, self.skill_manager.plugin_skills[skill_id])
214+
215+
# Verify return value
216+
self.assertEqual(result, mock_loader)
217+
218+
def test_load_plugin_skill_failure(self):
219+
"""Test failed plugin skill loading is handled gracefully."""
220+
skill_id = 'test.failing.skill'
221+
mock_plugin = Mock()
222+
223+
# Setup mock loader to raise exception
224+
mock_loader = Mock(spec=SkillLoader)
225+
mock_loader.skill_id = skill_id
226+
mock_loader.load.side_effect = Exception("Skill load failed!")
227+
228+
# Mock _get_plugin_skill_loader to return our mock
229+
self.skill_manager._get_plugin_skill_loader = Mock(return_value=mock_loader)
230+
231+
# Reset message tracking
232+
self.message_bus_mock.message_types = []
233+
self.message_bus_mock.message_data = []
234+
self.skill_manager.plugin_skills = {}
235+
236+
# Call the method
237+
result = self.skill_manager._load_plugin_skill(skill_id, mock_plugin)
238+
239+
# Verify NO success message was emitted
240+
self.assertNotIn('mycroft.skill.loaded', self.message_bus_mock.message_types)
241+
242+
# Verify exception was logged
243+
self.log_mock.exception.assert_called_once()
244+
245+
# Verify skill was still added to plugin_skills (even on failure)
246+
self.assertIn(skill_id, self.skill_manager.plugin_skills)
247+
self.assertEqual(mock_loader, self.skill_manager.plugin_skills[skill_id])
248+
249+
# Verify return value is None on failure
250+
self.assertIsNone(result)
251+
252+
def test_load_plugin_skill_returns_false(self):
253+
"""Test plugin skill loading that returns False (load failed gracefully)."""
254+
skill_id = 'test.false.skill'
255+
mock_plugin = Mock()
256+
257+
# Setup mock loader to return False (failed but no exception)
258+
mock_loader = Mock(spec=SkillLoader)
259+
mock_loader.skill_id = skill_id
260+
mock_loader.load.return_value = False
261+
262+
# Mock _get_plugin_skill_loader to return our mock
263+
self.skill_manager._get_plugin_skill_loader = Mock(return_value=mock_loader)
264+
265+
# Reset message tracking
266+
self.message_bus_mock.message_types = []
267+
self.skill_manager.plugin_skills = {}
268+
269+
# Call the method
270+
result = self.skill_manager._load_plugin_skill(skill_id, mock_plugin)
271+
272+
# Verify NO success message was emitted (load returned False)
273+
self.assertNotIn('mycroft.skill.loaded', self.message_bus_mock.message_types)
274+
275+
# Verify skill was added to plugin_skills
276+
self.assertIn(skill_id, self.skill_manager.plugin_skills)
277+
278+
# Verify return value is None when load returns False
279+
self.assertIsNone(result)

0 commit comments

Comments
 (0)