MycroftAI / mycroft-core

Mycroft Core, the Mycroft Artificial Intelligence platform.
https://mycroft.ai
Apache License 2.0
6.48k stars 1.27k forks source link

VK test example dialog feature not working #3067

Closed devvmh closed 2 years ago

devvmh commented 2 years ago

Describe the bug I'm writing a VK test for my new skill: https://github.com/devvmh/tv-remote-control-skill. When I use the Then {skill} should reply with dialog from {file} format, then everything works fine. However, when I try to use example dialog it fails and I don't know why.

Documentation I'm following: https://mycroft-ai.gitbook.io/docs/skill-development/voight-kampff/test-steps#reply-with-example-phrase

To Reproduce

  1. Clone https://github.com/devvmh/tv-remote-control-skill at commit 77828b9f8f375644dfa3fc1ea3d746ee51e759ca
  2. Apply the following diff:
    diff --git a/test/behave/turn-on-tv.feature b/test/behave/turn-on-tv.feature
    index 74d68e2..44ace08 100644
    --- a/test/behave/turn-on-tv.feature
    +++ b/test/behave/turn-on-tv.feature
    @@ -2,4 +2,4 @@ Feature: turn-on-tv
    Scenario: Turning on the TV
     Given an English speaking user
     When the user says "turn on the tv"
    -    Then "tv-remote-control-skill" should reply with dialog from "control.remote.tv.turnon.dialog"
    +    Then "tv-remote-control-skill" should reply with "Turning on the TV"

Expected behavior

Many Skills provide multiple possible responses to the same query by adding additional responses to the dialog file. The Voight Kampff test framework has been designed to handle this situation. The test will be successful if the both the intended and actual responses are contained within the same dialog file, even if they do not match directly.

Log files

Here's the output of mycroft-start vktest -t tv-remote-control-skill: https://pastebin.com/VDaNWBrF

relevant lines from audio.log: https://pastebin.com/htimyqnj relevant lines from skills.log: https://pastebin.com/vvTQwVFY

(nothing was output during the time of test to the other files)

Environment (please complete the following information):

forslund commented 2 years ago

Did a quick check on this during lunch and found two issues,

  1. The locale folder isn't scanned for dialog files
  2. There is an issue with one of the regexes that is used to determine if a dialog file is matching
devvmh commented 2 years ago

@forslund Thanks a lot for your comment! With that guidance I was able to find the issues in my local code.

I have a local commit that fixes it - is there a certain branch I should cut a PR against?

here's the diff in case I lose track of it

onses.pyit a/test/integrationtests/voight_kampff/features/steps/utterance_responses.py b/test/integrationtests/voight_kampff/features/steps/utterance_res
index e7422712b2..c0991882f1 100644
--- a/test/integrationtests/voight_kampff/features/steps/utterance_responses.py
+++ b/test/integrationtests/voight_kampff/features/steps/utterance_responses.py
@@ -75,9 +75,12 @@ def dialog_from_sentence(sentence, skill_path, lang):

     Returns (str): Dialog file best matching the sentence.
     """
-    dialog_paths = join(skill_path, 'dialog', lang, '*.dialog')
+    dialog_paths = [
+        *glob(join(skill_path, 'dialog', lang, '*.dialog')),
+        *glob(join(skill_path, 'locale', lang, '**', '*.dialog'))
+    ]
     best = (None, 0)
-    for path in glob(dialog_paths):
+    for path in dialog_paths:

         patterns = load_dialog_file(path)
         match, _ = _match_dialog_patterns(patterns, sentence.lower())
         if match is not False:
@@ -98,7 +101,7 @@ def _match_dialog_patterns(dialogs, sentence):
     dialogs = [re.sub(r'{.*?\}', r'.*', dia) for dia in dialogs]
     # Remove left over '}'
     dialogs = [re.sub(r'\}', r'', dia) for dia in dialogs]
-    dialogs = [re.sub(r' .* ', r' .*', dia) for dia in dialogs]
+    dialogs = [re.sub(r' [^\)]* ', r' .*', dia) for dia in dialogs]
     # Merge consequtive .*'s into a single .*
     dialogs = [re.sub(r'\.\*( \.\*)+', r'.*', dia) for dia in dialogs]
     # Remove double whitespaces
devvmh commented 2 years ago

nevermind! I see https://github.com/MycroftAI/mycroft-core/pull/3068 now :D much nicer code