8000 Voight Kampff test suite by forslund · Pull Request #2506 · MycroftAI/mycroft-core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on Sep 8, 2024. It is now read-only.

Voight Kampff test suite #2506

Merged
merged 54 commits into from
Mar 25, 2020
Merged

Conversation

forslund
Copy link
Collaborator

Add the voight_kampff test suite.

This allows creating behavioral tests for skills using the behave framework. It specifies a couple of pre-defined Given, When and Then covering the most common test cases but can be expanded with custom "step files".

In addition of being a test suite for skill creators, it will be used as an integration test validating pull requests to mycroft-core, making sure the essential skills work.

There are very little changes to core, but a couple of modifications are included in all these commits:

  • To allow the test to easier identify the dialog files correctly a meta field is added to the speak messages with dialog and origin information.
  • A fix to handling converse errors.
  • Adds a "dummy" tts which doesn't do any audio output

Contributor license agreement signed?

CLA [ Yes ]

@devs-mycroft devs-mycroft added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label Mar 20, 2020
@forslund forslund force-pushed the feature/voight-kampff-pr branch from 2daa651 to 7adefb3 Compare March 20, 2020 16:06
@JarbasAl
Copy link
Contributor

shouldn't the "meta" field simply be a part of message.context ?

@forslund
Copy link
Collaborator Author

It could definitely be moved there, I did consider it but felt like the context should be more "standardized" and not depend to much on the message type. Like the skill shouldn't interfere with the context and let the "surrounding machinery" handle that field. Not sure if that makes sense?

I don't really have a strong opinion one way or another, but that was my thoughts at the time.

@forslund forslund force-pushed the feature/voight-kampff-pr branch from 7adefb3 to 1fcb961 Compare March 20, 2020 16:35
Copy link
Member
@chrisveilleux chrisveilleux left a comment

Choose a reason for hiding this comment

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

Mostly minor tidying and refactoring stuff.

@forslund forslund force-pushed the feature/voight-kampff-pr branch 2 times, most recently from a3ae425 to 75b5a6b Compare March 23, 2020 09:33
forslund and others added 21 commits March 23, 2020 11:41
The Voight kampff test is an integration test collecting and running
behave test of skills.
Some branches have a "/" in their name (e.g. feature/new-and-cool)
Some commands, such as those tha deal with directories, don't
play nice with this naming convention. Define an alias for the
branch name that can be used in these scenarios.
Behave will generate test results in the allure format, this will be
picked up by Jenkins and sent of to a standalone webserver.
Docker build will now perform most actions of the dev-setup making it
possible to use caches in a greater extent speeding up the build
This reduces the verbosity during the operation. This is not of much
interest, mainly success or failure is what matters.
Allure commandline and chown command are now run through the Jenkinsfile
instead of through the run_test_suite.sh
Provides meta data such as dialog used and data that was inserted.
Override the bus clients on_message method and collect all messages in a
list. This will make it harder to miss a message during a test
Will now print all responses received from Mycroft
- Sharing only the identity file removes the need for clearing the skill
sandbox dir and padatious cache
- Make things a bit cleaner with separate Allure volume
@forslund forslund force-pushed the feature/voight-kampff-pr branch from 75b5a6b to fe6d2f5 Compare March 23, 2020 10:45
@forslund
Copy link
Collaborator Author

I think I've addressed all concerns here, either with a comment or changed code.

@forslund forslund changed the title Feature/voight kampff pr Voight Kampff test suite Mar 23, 2020
Copy link
Member
@chrisveilleux chrisveilleux left a comment

Choose a reason for hiding this comment

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

I like the changes you made. Just one other minor fix (see comment)



if __name__ == '__main__':
main(sys.argv[1:])
Copy link
Member

Choose a reason for hiding this comment

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

you don't need to pass the command line arguments to the arg parser. it looks at sys.argv by default.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I personally like the traceability. main get's an argument to operate on so all actions can be traced back to it without any questions of where the settings originate.

@forslund forslund force-pushed the feature/voight-kampff-pr branch from fe6d2f5 to 9fe158d Compare March 24, 2020 09:42
Takes in arguments for both test_setup.py and behave test runner. Parses
any args for test_setup and passes any remaining arguments to behave.

This moves argparsing out of the test_setup main() allowing the helper commands
to pass in pre-parsed arguments rather than adding logic inside main to
differentiate between a list and a preparsed arument object
@forslund forslund merged commit 92c716d into MycroftAI:dev Mar 25, 2020
@forslund forslund deleted the feature/voight-kampff-pr branch March 25, 2020 09:39
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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0