From 72b18489e3a6d923c2ac09c6d5d17c4382c1bf31 Mon Sep 17 00:00:00 2001 From: Chris Veilleux Date: Tue, 14 Sep 2021 15:09:12 -0500 Subject: [PATCH 1/3] Added an event indicating when an expired timer is stopped. --- __init__.py | 35 +++++++----- test/behave/steps/timer.py | 76 +++++++++++++------------- test/behave/stop_expired_timer.feature | 19 +++++-- 3 files changed, 73 insertions(+), 57 deletions(-) diff --git a/__init__.py b/__init__.py index 0c0abdb..b25062f 100644 --- a/__init__.py +++ b/__init__.py @@ -61,6 +61,10 @@ def __init__(self): self.all_timers_words = [word.strip() for word in self.translate_list("all")] self.save_path = Path(self.file_system.path).joinpath("save_timers") + @property + def expired_timers(self): + return [timer for timer in self.active_timers if timer.expired] + def initialize(self): """Initialization steps to execute after the skill is loaded.""" self._load_timers() @@ -516,6 +520,8 @@ def _cancel_all_timers(self): self.speak_dialog("cancelled-single-timer") else: self.speak_dialog("cancel-all", data={"count": len(self.active_timers)}) + if self.expired_timers: + self.bus.emit(Message("timer.stopped-expired")) self.active_timers = list() def _cancel_single_timer(self, utterance: str): @@ -535,6 +541,8 @@ def _cancel_single_timer(self, utterance: str): if reply == "no": timer = None if timer is not None: + if timer in self.expired_timers: + self.bus.emit(Message("timer.stopped-expired")) self.active_timers.remove(timer) self.speak_dialog("cancelled-single-timer") @@ -589,6 +597,8 @@ def _determine_which_timer_to_cancel(self, utterance: str): if matches: timer = matches[0] + if timer in self.expired_timers: + self.bus.emit(Message("timer.stopped-expired")) self.active_timers.remove(timer) dialog = TimerDialog(timer, self.lang) dialog.build_cancel_dialog() @@ -728,12 +738,11 @@ def check_for_expired_timers(self): Runs once every two seconds via a repeating event. """ - expired_timers = [timer for timer in self.active_timers if timer.expired] - if expired_timers: + if self.expired_timers: play_proc = play_wav(str(self.sound_file_path)) if self.platform == MARK_I: self._flash_eyes() - self._speak_expired_timer(expired_timers) + self._speak_expired_timer() play_proc.wait() def _flash_eyes(self): @@ -743,7 +752,7 @@ def _flash_eyes(self): else: self.enclosure.eyes_off() - def _speak_expired_timer(self, expired_timers): + def _speak_expired_timer(self): """Announce the expiration of any timers not already announced. This occurs every two seconds, so only announce one expired timer per pass. @@ -753,7 +762,7 @@ def _speak_expired_timer(self, expired_timers): On the Mark I, pause the display of any active timers so that the mouth can do the "talking". """ - for timer in expired_timers: + for timer in self.expired_timers: if not timer.expiration_announced: dialog = TimerDialog(timer, self.lang) dialog.build_expiration_announcement_dialog(len(self.active_timers)) @@ -776,9 +785,8 @@ def stop(self) -> bool: A boolean indicating if the stop message was consumed by this skill. """ stop_handled = False - expired_timers = [timer for timer in self.active_timers if timer.expired] - if expired_timers: - self._clear_expired_timers(expired_timers) + if self.expired_timers: + self._clear_expired_timers() stop_handled = True elif self.active_timers: # We shouldn't initiate dialog during Stop handling because there is @@ -790,14 +798,11 @@ def stop(self) -> bool: return stop_handled - def _clear_expired_timers(self, expired_timers: List[CountdownTimer]): - """The user wants the beeping to stop so cancel all expired timers. - - Args: - expired_timers: list of timer objects representing expired timers - """ - for timer in expired_timers: + def _clear_expired_timers(self): + """The user wants the beeping to stop so cancel all expired timers.""" + for timer in self.expired_timers: self.active_timers.remove(timer) + self.bus.emit(Message("timer.stopped-expired")) self._save_timers() if not self.active_timers: self._reset() diff --git a/test/behave/steps/timer.py b/test/behave/steps/timer.py index 207dc12..44804be 100644 --- a/test/behave/steps/timer.py +++ b/test/behave/steps/timer.py @@ -1,12 +1,12 @@ -import time -from typing import List +"""Steps to support the Timer Skill feature files.""" +from typing import Any, List from behave import given, then from test.integrationtests.voight_kampff import ( emit_utterance, - format_dialog_match_error, - wait_for_dialog_match, + VoightKampffDialogMatcher, + VoightKampffEventMatcher, ) CANCEL_RESPONSES = ( @@ -19,95 +19,95 @@ @given("an active {duration} timer") -def start_single_timer(context, duration): +def start_single_timer(context: Any, duration: str): """Clear any active timers and start a single timer for a specified duration.""" _cancel_all_timers(context) _start_a_timer( - context.bus, utterance="set a timer for " + duration, response=["started-timer"] + context, utterance="set a timer for " + duration, response=["started-timer"] ) @given("an active timer named {name}") -def start_single_named_timer(context, name): +def start_single_named_timer(context: Any, name: str): """Clear any active timers and start a single named timer for 90 minutes.""" _cancel_all_timers(context) _start_a_timer( - context.bus, + context, utterance="set a timer for 90 minutes named " + name, response=["started-timer-named"], ) @given("an active timer for {duration} named {name}") -def start_single_named_dialog_timer(context, duration, name): +def start_single_named_dialog_timer(context: Any, duration: str, name: str): """Clear any active timers and start a single named timer for specified duration.""" _cancel_all_timers(context) _start_a_timer( - context.bus, + context, utterance=f"set a timer for {duration} named {name}", response=["started-timer-named"], ) @given("multiple active timers") -def start_multiple_timers(context): +def start_multiple_timers(context: Any): """Clear any active timers and start multiple timers by duration.""" _cancel_all_timers(context) for row in context.table: _start_a_timer( - context.bus, + context, utterance="set a timer for " + row["duration"], response=["started-timer", "started-timer-named"], ) -def _start_a_timer(bus, utterance: str, response: List[str]): +def _start_a_timer(context, utterance: str, response: List[str]): """Helper function to start a timer. If one of the expected responses is not spoken, cause the step to error out. """ - emit_utterance(bus, utterance) - match_found, speak_messages = wait_for_dialog_match(bus, response) - assert match_found, format_dialog_match_error(response, speak_messages) + emit_utterance(context.bus, utterance) + dialog_matcher = VoightKampffDialogMatcher(context, response) + dialog_matcher.match() + assert dialog_matcher.match_found, dialog_matcher.error_message @given("no active timers") -def reset_timers(context): +def reset_timers(context: Any): """Cancel all active timers to test how skill behaves when no timers are set.""" _cancel_all_timers(context) @given("an expired timer") -def let_timer_expire(context): +def let_timer_expire(context: Any): """Start a short timer and let it expire to test expiration logic.""" _cancel_all_timers(context) emit_utterance(context.bus, "set a 3 second timer") expected_response = ["started-timer"] - match_found, speak_messages = wait_for_dialog_match(context.bus, expected_response) - assert match_found, format_dialog_match_error(expected_response, speak_messages) + dialog_matcher = VoightKampffDialogMatcher(context, expected_response) + dialog_matcher.match() + assert dialog_matcher.match_found, dialog_matcher.error_message expected_response = ["timer-expired"] - match_found, speak_messages = wait_for_dialog_match(context.bus, expected_response) - assert match_found, format_dialog_match_error(expected_response, speak_messages) + dialog_matcher = VoightKampffDialogMatcher(context, expected_response) + dialog_matcher.match() + assert dialog_matcher.match_found, dialog_matcher.error_message -def _cancel_all_timers(context): +def _cancel_all_timers(context: Any): """Cancel all active timers. If one of the expected responses is not spoken, cause the step to error out. """ emit_utterance(context.bus, "cancel all timers") - match_found, speak_messages = wait_for_dialog_match(context.bus, CANCEL_RESPONSES) - assert match_found, format_dialog_match_error(CANCEL_RESPONSES, speak_messages) - - -@then('the expired timer should stop beeping') -def then_stop_beeping(context): - # TODO: Better check! - import psutil - - for i in range(10): - if "paplay" not in [p.name() for p in psutil.process_iter()]: - break - time.sleep(1) - else: - assert False, "Timer is still ringing" + dialog_matcher = VoightKampffDialogMatcher(context, CANCEL_RESPONSES) + dialog_matcher.match() + assert dialog_matcher.match_found, dialog_matcher.error_message + + +@then("the expired timer is no longer active") +def check_expired_timer_removal(context: Any): + """Confirm that expired timers have been cleared when requested.""" + expected_event = "timer.stopped-expired" + event_matcher = VoightKampffEventMatcher(expected_event, context) + event_matcher.match() + assert event_matcher.match_found, event_matcher.error_message diff --git a/test/behave/stop_expired_timer.feature b/test/behave/stop_expired_timer.feature index 4393353..0b3146a 100644 --- a/test/behave/stop_expired_timer.feature +++ b/test/behave/stop_expired_timer.feature @@ -7,15 +7,26 @@ Feature: Stop an expired timer Given an english speaking user And an expired timer When the user says "" - Then the expired timer should stop beeping + Then the expired timer is no longer active Examples: | stop request | | stop | - | cancel | - | turn it off | | silence | | shut up | + + @xfail + # Jira SKILL-271 https://mycroft.atlassian.net/browse/SKILL-271 + Scenario Outline: Failing stop an expired timer using a "stop" command + Given an english speaking user + And an expired timer + When the user says "" + Then the expired timer is no longer active + + Examples: + | stop request | + | cancel | + | turn it off | | I got it | | mute | | disable | @@ -25,7 +36,7 @@ Feature: Stop an expired timer Given an english speaking user And an expired timer When the user says "" - Then the expired timer should stop beeping + Then the expired timer is no longer active And "mycroft-timer" should reply with dialog from "cancelled-single-timer.dialog" Examples: From 09116613a5de6de4baf4aee4c2e99f2ad839ca6c Mon Sep 17 00:00:00 2001 From: Chris Veilleux Date: Mon, 27 Sep 2021 18:50:15 -0500 Subject: [PATCH 2/3] update to use new convenience functions for message bus event matchers --- test/behave/steps/timer.py | 33 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/test/behave/steps/timer.py b/test/behave/steps/timer.py index 44804be..d477770 100644 --- a/test/behave/steps/timer.py +++ b/test/behave/steps/timer.py @@ -4,18 +4,16 @@ from behave import given, then from test.integrationtests.voight_kampff import ( - emit_utterance, - VoightKampffDialogMatcher, - VoightKampffEventMatcher, + emit_utterance, match_dialog, match_message ) -CANCEL_RESPONSES = ( +CANCEL_RESPONSES = [ "no-active-timer", "cancel-all", "cancelled-single-timer", "cancelled-timer-named", "cancelled-timer-named-ordinal", -) +] @given("an active {duration} timer") @@ -67,9 +65,8 @@ def _start_a_timer(context, utterance: str, response: List[str]): If one of the expected responses is not spoken, cause the step to error out. """ emit_utterance(context.bus, utterance) - dialog_matcher = VoightKampffDialogMatcher(context, response) - dialog_matcher.match() - assert dialog_matcher.match_found, dialog_matcher.error_message + match_found, error_message = match_dialog(context, response) + assert match_found, error_message @given("no active timers") @@ -84,13 +81,11 @@ def let_timer_expire(context: Any): _cancel_all_timers(context) emit_utterance(context.bus, "set a 3 second timer") expected_response = ["started-timer"] - dialog_matcher = VoightKampffDialogMatcher(context, expected_response) - dialog_matcher.match() - assert dialog_matcher.match_found, dialog_matcher.error_message + match_found, error_message = match_dialog(context, expected_response) + assert match_found, error_message expected_response = ["timer-expired"] - dialog_matcher = VoightKampffDialogMatcher(context, expected_response) - dialog_matcher.match() - assert dialog_matcher.match_found, dialog_matcher.error_message + match_found, error_message = match_dialog(context, expected_response) + assert match_found, error_message def _cancel_all_timers(context: Any): @@ -99,15 +94,13 @@ def _cancel_all_timers(context: Any): If one of the expected responses is not spoken, cause the step to error out. """ emit_utterance(context.bus, "cancel all timers") - dialog_matcher = VoightKampffDialogMatcher(context, CANCEL_RESPONSES) - dialog_matcher.match() - assert dialog_matcher.match_found, dialog_matcher.error_message + match_found, error_message = match_dialog(context, CANCEL_RESPONSES) + assert match_found, error_message @then("the expired timer is no longer active") def check_expired_timer_removal(context: Any): """Confirm that expired timers have been cleared when requested.""" expected_event = "timer.stopped-expired" - event_matcher = VoightKampffEventMatcher(expected_event, context) - event_matcher.match() - assert event_matcher.match_found, event_matcher.error_message + match_found, error_message = match_message(expected_event, context) + assert match_found, error_message From 1c57bbc366d04dc21a9f8e7eedd72295730cb550 Mon Sep 17 00:00:00 2001 From: Chris Veilleux Date: Tue, 28 Sep 2021 13:06:01 -0500 Subject: [PATCH 3/3] code review changes --- __init__.py | 8 ++++---- test/behave/steps/timer.py | 17 +++++++++++------ 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/__init__.py b/__init__.py index b25062f..e437e6a 100644 --- a/__init__.py +++ b/__init__.py @@ -521,7 +521,7 @@ def _cancel_all_timers(self): else: self.speak_dialog("cancel-all", data={"count": len(self.active_timers)}) if self.expired_timers: - self.bus.emit(Message("timer.stopped-expired")) + self.bus.emit(Message("skill.timer.stopped-expired")) self.active_timers = list() def _cancel_single_timer(self, utterance: str): @@ -542,7 +542,7 @@ def _cancel_single_timer(self, utterance: str): timer = None if timer is not None: if timer in self.expired_timers: - self.bus.emit(Message("timer.stopped-expired")) + self.bus.emit(Message("skill.timer.stopped-expired")) self.active_timers.remove(timer) self.speak_dialog("cancelled-single-timer") @@ -598,7 +598,7 @@ def _determine_which_timer_to_cancel(self, utterance: str): if matches: timer = matches[0] if timer in self.expired_timers: - self.bus.emit(Message("timer.stopped-expired")) + self.bus.emit(Message("skill.timer.stopped-expired")) self.active_timers.remove(timer) dialog = TimerDialog(timer, self.lang) dialog.build_cancel_dialog() @@ -802,7 +802,7 @@ def _clear_expired_timers(self): """The user wants the beeping to stop so cancel all expired timers.""" for timer in self.expired_timers: self.active_timers.remove(timer) - self.bus.emit(Message("timer.stopped-expired")) + self.bus.emit(Message("skill.timer.stopped-expired")) self._save_timers() if not self.active_timers: self._reset() diff --git a/test/behave/steps/timer.py b/test/behave/steps/timer.py index d477770..5def654 100644 --- a/test/behave/steps/timer.py +++ b/test/behave/steps/timer.py @@ -4,7 +4,7 @@ from behave import given, then from test.integrationtests.voight_kampff import ( - emit_utterance, match_dialog, match_message + emit_utterance, VoightKampffDialogMatcher, VoightKampffMessageMatcher ) CANCEL_RESPONSES = [ @@ -65,7 +65,8 @@ def _start_a_timer(context, utterance: str, response: List[str]): If one of the expected responses is not spoken, cause the step to error out. """ emit_utterance(context.bus, utterance) - match_found, error_message = match_dialog(context, response) + dialog_matcher = VoightKampffDialogMatcher(context, response) + match_found, error_message = dialog_matcher.match() assert match_found, error_message @@ -81,10 +82,12 @@ def let_timer_expire(context: Any): _cancel_all_timers(context) emit_utterance(context.bus, "set a 3 second timer") expected_response = ["started-timer"] - match_found, error_message = match_dialog(context, expected_response) + dialog_matcher = VoightKampffDialogMatcher(context, expected_response) + match_found, error_message = dialog_matcher.match() assert match_found, error_message expected_response = ["timer-expired"] - match_found, error_message = match_dialog(context, expected_response) + dialog_matcher = VoightKampffDialogMatcher(context, expected_response) + match_found, error_message = dialog_matcher.match() assert match_found, error_message @@ -94,7 +97,8 @@ def _cancel_all_timers(context: Any): If one of the expected responses is not spoken, cause the step to error out. """ emit_utterance(context.bus, "cancel all timers") - match_found, error_message = match_dialog(context, CANCEL_RESPONSES) + dialog_matcher = VoightKampffDialogMatcher(context, CANCEL_RESPONSES) + match_found, error_message = dialog_matcher.match() assert match_found, error_message @@ -102,5 +106,6 @@ def _cancel_all_timers(context: Any): def check_expired_timer_removal(context: Any): """Confirm that expired timers have been cleared when requested.""" expected_event = "timer.stopped-expired" - match_found, error_message = match_message(expected_event, context) + message_matcher = VoightKampffMessageMatcher(expected_event, context) + match_found, error_message = message_matcher.match() assert match_found, error_message