From dbdc3f1e353b05accd93094f0ce12288caa91701 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=85ke=20Forslund?= Date: Sun, 26 Jan 2020 17:56:47 +0100 Subject: [PATCH 1/3] Add tests for CommonQuerySkill --- .../skills/test_common_query_skill.py | 145 ++++++++++++++++++ 1 file changed, 145 insertions(+) create mode 100644 test/unittests/skills/test_common_query_skill.py diff --git a/test/unittests/skills/test_common_query_skill.py b/test/unittests/skills/test_common_query_skill.py new file mode 100644 index 0000000000..0900f859ca --- /dev/null +++ b/test/unittests/skills/test_common_query_skill.py @@ -0,0 +1,145 @@ +from unittest import TestCase, mock + +from mycroft.messagebus import Message +from mycroft.skills.common_query_skill import (CommonQuerySkill, CQSMatchLevel, + CQSVisualMatchLevel, + handles_visuals) + + +class AnyCallable: + """Class matching any callable. + Useful for assert_called_with arguments. + """ + def __eq__(self, other): + return callable(other) + + +class TestCommonQuerySkill(TestCase): + def setUp(self): + self.skill = CQSTest() + self.bus = mock.Mock(name='bus') + self.skill.bind(self.bus) + self.skill.config_core = {'enclosure': {'platform': 'mycroft_mark_1'}} + + def test_lifecycle(self): + """Test startup and shutdown.""" + skill = CQSTest() + bus = mock.Mock(name='bus') + skill.bind(bus) + bus.on.assert_any_call('question:query', AnyCallable()) + bus.on.assert_any_call('question:action', AnyCallable()) + skill.shutdown() + + def test_handles_visuals(self): + """Test the helper method to determine if the skill handles visuals.""" + self.assertTrue(handles_visuals('mycroft_mark_2')) + self.assertFalse(handles_visuals('mycroft_mark_1')) + + def test_common_test_skill_action(self): + """Test that the optional action is triggered.""" + query_action = self.bus.on.call_args_list[-1][0][1] + query_action(Message('query:action', data={ + 'phrase': 'What\'s the meaning of life', + 'skill_id': 'asdf'})) + self.skill.CQS_action.assert_not_called() + query_action(Message('query:action', data={ + 'phrase': 'What\'s the meaning of life', + 'skill_id': 'CQSTest'})) + self.skill.CQS_action.assert_called_once_with( + 'What\'s the meaning of life', None) + + +class TestCommonQueryMatching(TestCase): + """Tests for CQS_match_query_phrase.""" + def setUp(self): + self.skill = CQSTest() + self.bus = mock.Mock(name='bus') + self.skill.bind(self.bus) + self.skill.config_core = {'enclosure': {'platform': 'mycroft_mark_1'}} + # Get the method for handle_query_phrase + self.query_phrase = self.bus.on.call_args_list[-2][0][1] + + def test_failing_match_query_phrase(self): + self.skill.CQS_match_query_phrase.return_value = None + self.query_phrase(Message('question:query', + data={ + 'phrase': 'What\'s the meaning of life' + })) + + # Check that the skill replied that it was searching + extension = self.bus.emit.call_args_list[-2][0][0] + self.assertEqual(extension.data['phrase'], + 'What\'s the meaning of life') + self.assertEqual(extension.data['skill_id'], self.skill.skill_id) + self.assertEqual(extension.data['searching'], True) + + # Assert that the skill reported that it couldn't find the phrase + response = self.bus.emit.call_args_list[-1][0][0] + self.assertEqual(response.data['phrase'], + 'What\'s the meaning of life') + + self.assertEqual(response.data['skill_id'], self.skill.skill_id) + self.assertEqual(response.data['searching'], False) + + def test_successful_match_query_phrase(self): + self.skill.CQS_match_query_phrase.return_value = ( + 'What\'s the meaning of life', CQSMatchLevel.EXACT, '42') + + self.query_phrase(Message('question:query', + data={ + 'phrase': 'What\'s the meaning of life' + })) + + # Check that the skill replied that it was searching + extension = self.bus.emit.call_args_list[-2][0][0] + self.assertEqual(extension.data['phrase'], + 'What\'s the meaning of life') + self.assertEqual(extension.data['skill_id'], self.skill.skill_id) + self.assertEqual(extension.data['searching'], True) + + # Assert that the skill responds with answer and confidence level + response = self.bus.emit.call_args_list[-1][0][0] + self.assertEqual(response.data['phrase'], + 'What\'s the meaning of life') + self.assertEqual(response.data['skill_id'], self.skill.skill_id) + self.assertEqual(response.data['answer'], '42') + self.assertEqual(response.data['conf'], 1.0) + + def test_successful_visual_match_query_phrase(self): + self.skill.config_core['enclosure']['platform'] = 'mycroft_mark_2' + query_phrase = self.bus.on.call_args_list[-2][0][1] + self.skill.CQS_match_query_phrase.return_value = ( + 'What\'s the meaning of life', CQSVisualMatchLevel.EXACT, '42') + + query_phrase(Message('question:query', + data={'phrase': 'What\'s the meaning of life'})) + + # Check that the skill replied that it was searching + extension = self.bus.emit.call_args_list[-2][0][0] + self.assertEqual(extension.data['phrase'], + 'What\'s the meaning of life') + self.assertEqual(extension.data['skill_id'], self.skill.skill_id) + self.assertEqual(extension.data['searching'], True) + + # Assert that the skill responds with answer and confidence level + response = self.bus.emit.call_args_list[-1][0][0] + self.assertEqual(response.data['phrase'], + 'What\'s the meaning of life') + self.assertEqual(response.data['skill_id'], self.skill.skill_id) + self.assertEqual(response.data['answer'], '42') + self.assertEqual(response.data['conf'], 1.1) + + +class CQSTest(CommonQuerySkill): + """Simple skill for testing the CommonQuerySkill""" + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.CQS_match_query_phrase = mock.Mock(name='match_phrase') + self.CQS_action = mock.Mock(name='selected_action') + self.skill_id = 'CQSTest' + + def CQS_match_query_phrase(self, phrase): + pass + + def CQS_action(self, phrase, data): + pass From 9abb00f6815eac982e869ac86a30a55c52a8c0e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=85ke=20Forslund?= Date: Sun, 26 Jan 2020 17:58:05 +0100 Subject: [PATCH 2/3] Fix func checking if a platform supports visuals --- mycroft/skills/common_query_skill.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mycroft/skills/common_query_skill.py b/mycroft/skills/common_query_skill.py index 4c56d62091..87a18d711d 100644 --- a/mycroft/skills/common_query_skill.py +++ b/mycroft/skills/common_query_skill.py @@ -35,7 +35,7 @@ def is_CQSVisualMatchLevel(match_level): VISUAL_DEVICES = ['mycroft_mark_2'] -def handles_visuals(self, platform): +def handles_visuals(platform): return platform in VISUAL_DEVICES From 6740eea82c97aa029f49453d524c695642897f37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=85ke=20Forslund?= Date: Sun, 26 Jan 2020 18:01:03 +0100 Subject: [PATCH 3/3] Fix formatting of docstrings --- mycroft/skills/common_query_skill.py | 32 ++++++++++++++++------------ 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/mycroft/skills/common_query_skill.py b/mycroft/skills/common_query_skill.py index 87a18d711d..790bc74afc 100644 --- a/mycroft/skills/common_query_skill.py +++ b/mycroft/skills/common_query_skill.py @@ -40,10 +40,11 @@ def handles_visuals(platform): class CommonQuerySkill(MycroftSkill, ABC): - """ Question answering skills should be based on this class. The skill - author needs to implement `CQS_match_query_phrase` returning an answer - and can optionally implement `CQS_action` to perform additional actions - if the skill's answer is selected. + """Question answering skills should be based on this class. + + The skill author needs to implement `CQS_match_query_phrase` returning an + answer and can optionally implement `CQS_action` to perform additional + actions if the skill's answer is selected. This class works in conjunction with skill-query which collects answers from several skills presenting the best one available. @@ -52,7 +53,7 @@ class CommonQuerySkill(MycroftSkill, ABC): super().__init__(name, bus) def bind(self, bus): - """ Overrides the default bind method of MycroftSkill. + """Overrides the default bind method of MycroftSkill. This registers messagebus handlers for the skill during startup but is nothing the skill author needs to consider. @@ -114,8 +115,11 @@ class CommonQuerySkill(MycroftSkill, ABC): return 0.0 # should never happen def __handle_query_action(self, message): - """ Message handler for question:action. Extracts phrase and data from - message forward this to the skills CQS_action method. """ + """Message handler for question:action. + + Extracts phrase and data from message forward this to the skills + CQS_action method. + """ if message.data["skill_id"] != self.skill_id: # Not for this skill! return @@ -126,12 +130,12 @@ class CommonQuerySkill(MycroftSkill, ABC): @abstractmethod def CQS_match_query_phrase(self, phrase): - """ - Analyze phrase to see if it is a play-able phrase with this - skill. Needs to be implemented by the skill. + """Analyze phrase to see if it is a play-able phrase with this skill. - Args: - phrase (str): User phrase uttered after "Play", e.g. "some music" + Needs to be implemented by the skill. + + Arguments: + phrase (str): User phrase, "What is an aardwark" Returns: (match, CQSMatchLevel[, callback_data]) or None: Tuple containing @@ -143,8 +147,8 @@ class CommonQuerySkill(MycroftSkill, ABC): return None def CQS_action(self, phrase, data): - """ - Take additional action IF the skill is selected. + """Take additional action IF the skill is selected. + The speech is handled by the common query but if the chosen skill wants to display media, set a context or prepare for sending information info over e-mail this can be implemented here.