search

MycroftAI / skill-homeassistant

Mycroft Skill/Integration for Homeassistant
GNU Lesser General Public License v3.0
115 stars 62 forks source link

VK tests, GitHub actions, GitHub pages, allure report and linters #77

Closed Tony763 closed 2 years ago

Tony763 commented 2 years ago

Hi, I finally managed to finish next step in dividing and applying changes from PR #38. Check README.md for some detailed information Actual report can be seen there

Description

Fix old unittests Remove old tests Add VK tests (behave) Add GitHub Actions workflow: Contain linters, VK tests, allure report Add docstrings Fix various errors found by linters Add Readme.md to test directory Add .venv and .vscode to .gitignore

Type of PR

Note

PR contain partially changes from #74 to fix pylint errors, but not all. It is still valid PR. Changes are divided by commits.

Tony763 commented 2 years ago

Hi @krisgesling, thanks, glad you like it. I original prepared this for Travis, but on it, was impossible to upload Allure report to GitHub pages as secret was not shard to runs from PR only from commits. So useless as You want allure reports mainly from PRs. GitHub Actions is whole game changer, as no secret is needed to upload to Pages as they run in same repository as gh-pages branch.

Template for all skills is my intent, take this as first touch on which we all can build. Also with Mycroft-core, when new release is made, there could be workflow to crate a docker image with released version and then it can be used in skills workflows to speed up runs (skip whole cloning and installing of Mycroft core).

I started working on your comments, I made changes locally and push them in wan go.

I will try to specify as much details as possible and add them to 'README.md' in tests, so anybody can understood them. It took me some time and many runs to figure out things so documentation is really needed.

krisgesling commented 2 years ago

Also with Mycroft-core, when new release is made, there could be workflow to crate a docker image with released version and then it can be used in skills workflows to speed up runs (skip whole cloning and installing of Mycroft core).

This exists! It was getting forgotten but I've added it to our standard release process now. So anytime we release a new minor or major version of mycroft-core this docker image will get updated: https://hub.docker.com/r/mycroftai/docker-mycroft/

Really appreciate how much time it takes to work this stuff out so really glad to hear you're documenting it as you go.

Tony763 commented 2 years ago

Hi @krisgesling, check comments please, I replied to all.

Also check README.md inside test directory.

Tony763 commented 2 years ago

Not Yet, I missed few comments, so working on them.

Tony763 commented 2 years ago

@krisgesling done. Rebased to include latest commit. Unittests fixed and added to GA workflow. Their outcome is added to allure report. All tests green.

Let me just know before merging if there is another commit added, I will rebase this a clear all linter errors. So we start with green.

pfefferle commented 2 years ago

@Tony763 there seems to be some "conflicting files"

Tony763 commented 2 years ago

Here we go, rebase did not help so I had to solve merge issues with new commit.

Workflow OK Linters OK Behave OK Unittests OK

Ready to go.

Tony763 commented 2 years ago

Could You check @krisgesling? Another PR is on the way and I would like to make it on top of this, fixing merge issues in this beast was something this time so I would like to avoid to rebase it again :smile:

Tony763 commented 2 years ago

Memo for future implementation: Always add sudo apt update to workflow.

krisgesling commented 2 years ago

Nice little clean up of the action step names.

I've also added a branch protection rule so in future all of these tests must pass, and there must be an approving review for any PR, before they they can be merged.

Merging this now though! :rocket: :rainbow: :tada:

krisgesling commented 2 years ago

Helps if I setup GH pages first...

Re-running actions now

pfefferle commented 2 years ago

Hmmm, I get a lot of errors like:

Resource not accessible by integration

pfefferle commented 2 years ago

it seems that branches/pull requests are not able to write to the gh-pages branch

Tony763 commented 2 years ago

@pfefferle could you send PR with any small change to my repository - feature/github-actions-pr-fix ?

I need to test changes for fixing workflow run for PRs.

pfefferle commented 2 years ago

@Tony763 done: https://github.com/Tony763/skill-homeassistant/pull/37