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

Upgrade homeassistant #1621

Closed
wants to merge 2 commits into from
Closed

Upgrade homeassistant #1621

wants to merge 2 commits into from

Conversation

pfefferle
Copy link

Name of your skill: homeassistant

Description:

Update homeassistant skill to the latest master, to fix current problems with tracking and light handlings.

@pfefferle pfefferle changed the title Update homeassistant Upgrade homeassistant Mar 23, 2022
@devops-mycroft
Copy link
Collaborator

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

@krisgesling
Copy link
Contributor

Hmm, it looks like a combination of:

  1. intent clashes and
  2. the HomeAssistant tests failing because it doesn't have the same mocked data as the Github Actions in that repo.

For the intent clashes, we probably just need to work through them 1 by 1 to tighten up the language.

For the HA failing tests we can probably do two things:

  • Create some basic tests that don't require a HA instance to be running just to verify that the Skill loaded correctly and is responding to some expected intents. Even if the success criteria is the error message that it's not connected to a HA instance. This will at least tell us if some change to mycroft-core or some dependency majorly breaks the Skill.
  • Mark the rest with an @xfail tag which our CI processes will then ignore, but we can continue running them in the HA Skill's Github Actions. Here's an example of a test tagged with xfail:
    https://github.com/MycroftAI/mycroft-timer/blob/8f0cc967ceeefed43bfe1d97194aee72547c175e/test/behave/cancel_timer.feature#L123

@pfefferle
Copy link
Author

@Tony763 can you help here?

@Tony763
Copy link

Tony763 commented Oct 14, 2022

Hi friends, let's dig into it.

In allure report I see two errors:

AssertionError: Mycroft responded with:
Mycroft: homeassistant.error.setup.dialog(HomeAssistantSkill)
"Please configure the home assistant skill settings at home dot mycroft dot ai, I.P. is missing or wrongly set."
Mycroft: (HomeAssistantSkill)
"An error occurred while processing a request in Home Assistant Skill"

As pointed by Kris, VK test in HA skill needs a specific environment and running HA instance.

Second one:

AssertionError: Volume hasn't decreased!

Which does not seem to me as HA related, I would expect some random response from HA skill.

I would definitely like to keep current GitHub Actions CI in HA skill as it can be run by anybody who forked repository.

@xfail will be necessary to suppress errors from HA VK tests, but still first error must be handled as HA skill can throw this message any time, which could lead into random skills fail.
I think, this will have to be handled on Your CI. Loading HA skill configuration from test directory should be enough.

Dark side of @xfail is, that we will not run HA VK tests, so we won't get possible collisions from others skills (example: we test HA where is and wiki skill respond instead). But with a list of skills, we could check this on GitHub Action side.

@Tony763
Copy link

Tony763 commented Oct 14, 2022

After addressing these, I would do a quick review of all issues in HA skill repository. I didn't have much time lately, so I lost a track of them.

@Tony763
Copy link

Tony763 commented Oct 23, 2022

Hi friend, small progress update:

  • PR #109 Fix GitHub Actions run.
  • PR #110 Fix issue #92
  • PR #111 Add @xfail to allow pass of HA VK tests on Mycroft CI (Jenkins).
  • PR #112 Fix Padatious crash with tracker intent. Issue #104 and #106
  • PR #113 Add behave test for script automation. Issue #101

Issue #67 just wait for skill update at marketplace.

For second failure when HA throws Pairing action into random skills while running VK tests, I need to inject HA skill settings into Jenkins instance, so HA skill thinks it is registered.
On GitHub Actions, it is cp ${{ github.workspace }}/test/ci/Mycroft/settings.json /opt/mycroft/skills/homeassistant.mycroftai/.

https://raw.githubusercontent.com/MycroftAI/skill-homeassistant/20.08/test/ci/Mycroft/settings.json

File need to be in place before starting tests. It could probably be done at the end of Dockerfile when an image for testing is build. @krisgesling Could I see details from Jenkins run?

@Tony763
Copy link

Tony763 commented Dec 22, 2022

Hi @pfefferle, all major issues were resolved as tracked in comment above. Try to update referenced commit to latest merged in HA skill repo, please. Mycroft CI in this repo should pass.

@pfefferle pfefferle closed this by deleting the head repository Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants