Skip to content
This repository has been archived by the owner on Sep 8, 2024. It is now read-only.

Old skill tester: Fix Padatious setup and fallback intents #2894

Merged

Conversation

forslund
Copy link
Collaborator

@forslund forslund commented May 1, 2021

Description

This was broken when Padatious moved from a separate "skill" into the
service manager. This updates the code to call Padatious through the intent service. Resolves initial description in #2893.

The old skill tester didn't support wait_for_response() which hindered the fallback service to function, this PR includes a commit adding a basic implementation for this.

In addition some extra handling of the queue was needed to make sure that the events emitted in the fallback handler were handled before considering the skill to be completely handled.

How to test

Run ./bin/mycroft-skill-testrunner /opt/mycroft/skills/mycroft-stop.mycroftai/ for example and make sure the skill test launches.
fallack-query can be used to check a fallback skill.

Contributor license agreement signed?

CLA [ Yes ]

This was broken when Padatious moved from a separate "skill" into the
service manager. This updates the code to call the correct methods
@forslund forslund added the Type: Tests Addition or improvement of automated tests label May 1, 2021
@devops-mycroft devops-mycroft added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label May 1, 2021
@forslund forslund added the Type: Bug - quick Bug fixes that are quick to review and the implications of the change are clear and contained. label May 1, 2021
@forslund forslund changed the title Fix Padatious setup Old skill tester: Fix Padatious setup May 1, 2021
@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@codecov-commenter
Copy link

codecov-commenter commented May 1, 2021

Codecov Report

Merging #2894 (6dd059e) into dev (b712b0e) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev    #2894   +/-   ##
=======================================
  Coverage   52.59%   52.59%           
=======================================
  Files         123      123           
  Lines       10989    10989           
=======================================
  Hits         5780     5780           
  Misses       5209     5209           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b712b0e...6dd059e. Read the comment docs.

Previously this was just skipped over but is now needed for the fallback intents
This will handle any extra messages after the skill complete message was detected. For example messages from certain fallback skills
@forslund forslund changed the title Old skill tester: Fix Padatious setup Old skill tester: Fix Padatious setup and fallback intents May 2, 2021
@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

Copy link
Contributor

@krisgesling krisgesling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and working great - thanks heaps!!

@krisgesling krisgesling merged commit 3d296bf into MycroftAI:dev May 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) Type: Bug - quick Bug fixes that are quick to review and the implications of the change are clear and contained. Type: Tests Addition or improvement of automated tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants