Skip to content
This repository has been 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
@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.

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.

mycroft/tts/tts.py Show resolved Hide resolved
test/integrationtests/voight_kampff/test_setup.py Outdated Show resolved Hide resolved
test/integrationtests/voight_kampff/test_setup.py Outdated Show resolved Hide resolved
test/integrationtests/voight_kampff/test_setup.py Outdated Show resolved Hide resolved
test/integrationtests/voight_kampff/tools.py Outdated Show resolved Hide resolved
test/integrationtests/voight_kampff/test_setup.py Outdated Show resolved Hide resolved
@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
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.

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