Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lng: Add cs-cz noise words #3135

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

Tony763
Copy link
Contributor

@Tony763 Tony763 commented Oct 13, 2022

Description

Hi, PR contain Czech version of noise words.

Contributor license agreement signed?

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

@devops-mycroft devops-mycroft added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label Oct 13, 2022
@devops-mycroft
Copy link

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

@Tony763
Copy link
Contributor Author

Tony763 commented Oct 13, 2022

Hi, VK for Wiki failed, what about adding Github action to it (same as in HA skill)?

@forslund
Copy link
Collaborator

I think the latest version of wiki doesn't have the issue but is held back waiting for a mycroft release.

The wiki-skill is a bit troublesome, the problem is due to wikipedia returning a new answer for a question. It was working on last release of the wiki-skill but has since changed. I think using something like betamax in VK providing a set of stable data from the web might be one way to go. The problem is that it means that the tests won't tell the devs whether the skill is actually working alright with real world data.

That said, per skill unit/integrationtests is a great idea :)

Copy link
Collaborator

@forslund forslund left a comment

Choose a reason for hiding this comment

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

Google translate tells me this matches well with the english noise words. Nice! (or should I say Noise!)

@Tony763
Copy link
Contributor Author

Tony763 commented Oct 14, 2022

I think the latest version of wiki doesn't have the issue but is held back waiting for a mycroft release.

The wiki-skill is a bit troublesome, the problem is due to wikipedia returning a new answer for a question. It was working on last release of the wiki-skill but has since changed. I think using something like betamax in VK providing a set of stable data from the web might be one way to go. The problem is that it means that the tests won't tell the devs whether the skill is actually working alright with real world data.

That said, per skill unit/integrationtests is a great idea :)

Testing against real integration is always better than just testing against fixed data. That is why I use real Home Assistant installation and not just mocked subset. Also, CI per repo give you instant feedback when doing changes.

If you want it for wiki skill, let me know and I will prepare it.

@JarbasAl
Copy link
Contributor

JarbasAl commented Oct 14, 2022

In ovos and neon we have unittests per skill, i agree it is the way to go and way more useful than VK imho

Eg https://github.com/OpenVoiceOS/skill-ovos-wikipedia/tree/dev/tests/unittests

@Tony763
Copy link
Contributor Author

Tony763 commented Oct 14, 2022

With HA skill, I run both VK and unittest (allure report is generated).

@forslund
Copy link
Collaborator

I think using vk in core is valid, even with fixed data. It helps to ensure that a change in core doesn't break. I think both unit tests and the high level tests vk provides are needed to guard against regressions.

For validating skills functionality during (skill) development unittest are probably more valuable.

@Tony763
Copy link
Contributor Author

Tony763 commented Oct 14, 2022

To wrap it, should I prepare PR with GitHub Action?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) hacktoberfest-accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants