Added an event indicating when an expired timer is stopped.

__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("skill.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("skill.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("skill.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("skill.timer.stopped-expired"))
         self._save_timers()
         if not self.active_timers:
             self._reset()

test/behave/steps/timer.py

@@ -1,113 +1,111 @@
-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,
+    emit_utterance, VoightKampffDialogMatcher, VoightKampffMessageMatcher
 )
 
-CANCEL_RESPONSES = (
+CANCEL_RESPONSES = [
     "no-active-timer",
     "cancel-all",
     "cancelled-single-timer",
     "cancelled-timer-named",
     "cancelled-timer-named-ordinal",
-)
+]
 
 
 @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)
+    match_found, error_message = dialog_matcher.match()
+    assert match_found, 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)
+    match_found, error_message = dialog_matcher.match()
+    assert match_found, 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)
+    match_found, error_message = dialog_matcher.match()
+    assert match_found, 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)
+    match_found, error_message = dialog_matcher.match()
+    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"
+    message_matcher = VoightKampffMessageMatcher(expected_event, context)
+    match_found, error_message = message_matcher.match()
+    assert match_found, error_message

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 "<stop request>"
-    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 "<stop request>"
+    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 "<cancel request>"
-    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: