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

Improve the speed of waiting for dialogs helper function #2946

Closed chrisveilleux closed 3 years ago

chrisveilleux commented 3 years ago

Description

The wait_for_dialogs step function in the Voight-Kampff test suite would loop until timeout regardless of whether or not a match was found. Also, there was no error condition if no match was found, so the step would always pass. This change adds logic to break out of the loop after a matching dialog is spoken and provides the status of the matching process. A new method was added to format a message detailing the error condition.

How to test

Tests that use this helper function should run faster.

Contributor license agreement signed?

CLA [Y]

pep8speaks commented 3 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-08-04 22:06:56 UTC
devops-mycroft commented 3 years ago

Voight Kampff Integration Test Succeeded (Results)

forslund commented 3 years ago

Also the wait_for_dialogs() function is not a step-function, just a helper to do a wait. then_dialog() is the step function that checks the dialogs. This new function sort of duplicates that function.

devops-mycroft commented 3 years ago

Voight Kampff Integration Test Failed (Results). Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

devops-mycroft commented 3 years ago

Voight Kampff Integration Test Failed (Results). Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

chrisveilleux commented 3 years ago

Also the wait_for_dialogs() function is not a step-function, just a helper to do a wait. then_dialog() is the step function that checks the dialogs. This new function sort of duplicates that function.

Yes, wait_for_dialogs() also sort of duplicated then_wait(). I can't use then_dialog() in this case. The timer skill tests need the helper function. If more refactoring is needed for these functions to make more sense, I am open to that.

devops-mycroft commented 3 years ago

Voight Kampff Integration Test Failed (Results). Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

devops-mycroft commented 3 years ago

Voight Kampff Integration Test Failed (Results). Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

devops-mycroft commented 3 years ago

Voight Kampff Integration Test Failed (Results). Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

codecov-commenter commented 3 years ago

Codecov Report

Merging #2946 (e6187fc) into dev (2deab67) will increase coverage by 0.20%. The diff coverage is 89.18%.

:exclamation: Current head e6187fc differs from pull request most recent head 8f41d17. Consider uploading reports for the commit 8f41d17 to get more accurate results Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #2946      +/-   ##
==========================================
+ Coverage   52.64%   52.85%   +0.20%     
==========================================
  Files         123      123              
  Lines       11021    11044      +23     
==========================================
+ Hits         5802     5837      +35     
+ Misses       5219     5207      -12     
Impacted Files Coverage Δ
mycroft/skills/mycroft_skill/mycroft_skill.py 74.69% <ø> (ø)
mycroft/audio/audioservice.py 80.59% <40.00%> (-0.61%) :arrow_down:
mycroft/util/network_utils.py 94.44% <95.45%> (+40.59%) :arrow_up:
mycroft/skills/intent_services/adapt_service.py 80.71% <100.00%> (+0.13%) :arrow_up:
mycroft/tts/tts.py 79.60% <100.00%> (+0.13%) :arrow_up:
mycroft/util/format.py 100.00% <100.00%> (ø)
mycroft/util/parse.py 82.35% <100.00%> (+1.10%) :arrow_up:
mycroft/client/speech/listener.py 48.04% <0.00%> (-0.36%) :arrow_down:

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 2deab67...8f41d17. Read the comment docs.