Skip to content

Commit

Permalink
fix: stop DRY
Browse files Browse the repository at this point in the history
no need for 2 handlers, decrease complexity

the only difference is an emitted event that is not listened too anywhere
  • Loading branch information
JarbasAl committed Jan 9, 2025
1 parent e2a1b08 commit 040042f
Showing 1 changed file with 29 additions and 45 deletions.
74 changes: 29 additions & 45 deletions test/end2end/session/test_stop.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,22 +67,12 @@ def wait_for_n_messages(n):
"recognizer_loop:utterance",
# global stop trigger
"mycroft.stop",
# intent pipelines
"common_query.openvoiceos.stop",
"common_query.openvoiceos.stop.response", # reporting nothing to stop
"ovos.common_play.stop",
"ovos.common_play.stop.response", # reporting nothing to stop

# skill reporting
f"{self.skill_id}.stop", # internal, @killable_events
f"{self.skill_id}.stop.response", # skill reporting nothing to stop

"common_query.openvoiceos.stop.response",
"ovos.common_play.stop.response",
f"{self.skill_id}.stop.response",
# sanity check in test skill that method was indeed called
"speak", # "utterance":"old stop called"

# NOTE: messages below might show up before enclosure.active_skill
f"{self.new_skill_id}.stop", # internal, @killable_events
f"{self.new_skill_id}.stop.response", # skill reporting nothing to stop
f"{self.new_skill_id}.stop.response", # nothing to stop

"ovos.utterance.handled"
]
Expand Down Expand Up @@ -151,30 +141,17 @@ def wait_for_n_messages(n):
f"{self.skill_id}.stop.ping", # check if active skill wants to stop
"skill.stop.pong", # "can_handle":true
f"{self.skill_id}.stop", # skill specific stop trigger
f"{self.skill_id}.stop.response", # skill fails to stop (old style)
"speak", # "old stop called" in the test skill stop method
f"{self.skill_id}.stop.response", # skill stops and reports back

# stop medium
f"{self.skill_id}.stop.ping",
"skill.stop.pong",
f"{self.skill_id}.stop", # skill specific stop trigger
f"{self.skill_id}.stop.response", # skill fails to stop (old style)
# skill reports it stopped, so core ensures any threaded activity is also killed
"mycroft.skills.abort_question", # core kills any ongoing get_response
"ovos.skills.converse.force_timeout", # core kills any ongoing converse
"mycroft.audio.speech.stop", # core kills any ongoing TTS

# stop fallback
"mycroft.stop", # global stop for backwards compat
f"{self.skill_id}.stop",
f"{self.skill_id}.stop.response", # apparently fails to stop (old style)

# test in skill that global stop was called
"speak", # "utterance":"stop"

# report old-style stop handled event
"mycroft.stop.handled", # {"by":"skill:skill-old-stop.openvoiceos"}
f"{self.skill_id}.activate", # update of skill last usage timestamp
"ovos.utterance.handled"

# old style unwanted side effects (global stop is global)
f"{self.new_skill_id}.stop",
f"{self.new_skill_id}.stop.response",
# other test skill also speaks
"speak" # "utterance":"old stop called"
]

wait_for_n_messages(len(expected_messages))
Expand Down Expand Up @@ -246,14 +223,15 @@ def wait_for_n_messages(n):
"recognizer_loop:utterance",
# global stop trigger
"mycroft.stop",
f"{self.skill_id}.stop", # internal, @killable_events
"common_query.openvoiceos.stop.response", # common_query framework reporting nothing to stop
"ovos.common_play.stop.response", # OCP framework reporting nothing to stop
f"{self.skill_id}.stop.response", # skill reporting nothing to stop
f"{self.new_skill_id}.stop", # internal, @killable_events
f"{self.new_skill_id}.stop.response", # skill reporting nothing to stop

# sanity check in test skill that method was indeed called
"speak", # "utterance":"old stop called"

f"{self.new_skill_id}.stop.response", # skill reporting it stopped

"ovos.utterance.handled",

]
Expand Down Expand Up @@ -331,8 +309,11 @@ def wait_for_n_messages(n):

# test session specific stop was called
"speak", # "utterance":"stop 123"

f"{self.new_skill_id}.stop.response", # skill reports it stopped (new style),

"mycroft.skills.abort_question", # core kills any ongoing get_response
"ovos.skills.converse.force_timeout", # core kills any ongoing converse
"mycroft.audio.speech.stop", # core kills any ongoing TTS
f"{self.new_skill_id}.activate", # update timestamp of last interaction with skill
"ovos.utterance.handled" # handle_utterance returned (intent service)
]
Expand All @@ -350,7 +331,7 @@ def wait_for_n_messages(n):
self.assertEqual(m.data["utterance"], "stop 123")

# confirm "skill-new-stop" was the one that reported success
handler = messages[-3]
handler = messages[-6]
self.assertEqual(handler.msg_type, f"{self.new_skill_id}.stop.response")
self.assertEqual(handler.data["result"], True)

Expand All @@ -373,24 +354,27 @@ def wait_for_n_messages(n):
f"{self.new_skill_id}.stop.ping", # check if active skill wants to stop
"skill.stop.pong", # "can_handle":true
f"{self.new_skill_id}.stop", # skill specific stop trigger
"speak", # it's in the stop method even if it returns False!
f"{self.new_skill_id}.stop.response", # dont want to stop (new style)

# rest of pipeline
# stop low
f"{self.new_skill_id}.stop.ping",
"skill.stop.pong",
f"{self.new_skill_id}.stop", # skill specific stop trigger
"speak", # it's in the stop method even if it returns False!
f"{self.new_skill_id}.stop.response", # dont want to stop (new style)

# global stop fallback
"mycroft.stop",
f"{self.skill_id}.stop", # skill specific stop trigger
"common_query.openvoiceos.stop.response", # dont want to stop
"ovos.common_play.stop.response", # dont want to stop

f"{self.skill_id}.stop.response", # old style, never stops
f"{self.new_skill_id}.stop", # skill specific stop trigger
f"{self.skill_id}.stop.response", # dont want to stop (new style)
"speak", # it's in the stop method even if it returns False!
f"{self.new_skill_id}.stop.response", # dont want to stop (new style)

# check the global stop handlers are called
"speak", # "utterance":"old stop called"
"ovos.utterance.handled"
]

wait_for_n_messages(len(expected_messages))
Expand Down

0 comments on commit 040042f

Please sign in to comment.