search

MycroftAI / mycroft-core

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

Extended the idea of the VK CriteriaWaiter #3002

Closed chrisveilleux closed 2 years ago

chrisveilleux commented 2 years ago

Description

Recently a new CriteriaWaiter class was added to the VoightKampff test framework. This did an excellent job for its intended use case, but there are other similar use cases that could use some of the same logic. This PR adds a base event matching class that is extended by a dialog matching class and a new version of the criteria matching class. The latter was only changed to fit in with the new class structure. The new classes also handle the error messaging that was once in a separate function.

Existing functions in the VoightKampff tools.py file that did the things these new classes now take care of in a reusable manner have been marked as deprecated in the next major release, 21.08

How to test

Run the VK tests. The new criteria matcher is called in then_wait() which is called throughout the test suite. The other classes are used in new tests defined in the TimerSkill (PR yet to be created).

Contributor license agreement signed?

CLA [Y] (Whether you have signed a CLA - Contributor Licensing Agreement

pep8speaks commented 2 years ago

Hello @chrisveilleux! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2021-09-29 22:57:41 UTC
devops-mycroft commented 2 years ago

Voight Kampff Integration Test Succeeded (Results)

chrisveilleux commented 2 years ago

Did the wrapper functions get removed? I thought they were a good addition.

Edit: but loving this overall 👍

I did remove the wrapper functions. Instead I added instructions on how to use the classes in the class level docstring. These classes are pretty easy to use. I am not sure the extra abstraction is necessary.