mycroft-skills icon indicating copy to clipboard operation
mycroft-skills copied to clipboard

Upgrade homeassistant

Open pfefferle opened this issue 2 years ago • 2 comments

Name of your skill: homeassistant

Description:

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

pfefferle avatar Mar 23 '22 14:03 pfefferle

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

devops-mycroft avatar Mar 23 '22 16:03 devops-mycroft

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

krisgesling avatar Mar 24 '22 03:03 krisgesling

@Tony763 can you help here?

pfefferle avatar Oct 14 '22 07:10 pfefferle

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 avatar Oct 14 '22 19:10 Tony763

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 avatar Oct 14 '22 19:10 Tony763

Hi friend, small progress update:

  • [x] PR #109 Fix GitHub Actions run.
  • [x] PR #110 Fix issue #92
  • [x] PR #111 Add @xfail to allow pass of HA VK tests on Mycroft CI (Jenkins).
  • [x] PR #112 Fix Padatious crash with tracker intent. Issue #104 and #106
  • [x] 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 avatar Oct 23 '22 20:10 Tony763

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.

Tony763 avatar Dec 22 '22 10:12 Tony763