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

Jump to bottom

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

Merged
merged 15 commits into from Oct 22, 2021
Merged

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

merged 15 commits into from Oct 22, 2021

Conversation

Tony763
Copy link

@Tony763 Tony763 commented Oct 3, 2021 •
edited

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

  • Bugfix
  • Feature implementation
  • Refactor of code (without functional changes)
  • Documentation improvements
  • Test improvements

Note

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

krisgesling
krisgesling reviewed Oct 4, 2021
View changes
Copy link

@krisgesling krisgesling left a comment

Choose a reason for hiding this comment

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

First up - this is awesome!

I can't comment on each change - but really love all the code style cleanup.

I've also been thinking about setting up a Github Actions template that any Skill can grab and use - so really excited to see what you've done here. I hadn't thought about using Github Pages for the allure output either - very nice!

__init__.py Outdated Show resolved Hide resolved
ha_client.py Outdated Show resolved Hide resolved
ha_client.py Outdated Show resolved Hide resolved
test/behave/07_set_brightness.feature Show resolved Hide resolved
test/behave/02_turn_on_switch.feature Outdated Show resolved Hide resolved
.github/workflows/build.yml Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Show resolved Hide resolved
@Tony763
Copy link
Author

Tony763 commented Oct 4, 2021

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.

@Tony763 Tony763 force-pushed the feature/github_actions branch 3 times, most recently from a0573b2 to 8a5b5d1 Compare October 5, 2021 06:47
@krisgesling
Copy link

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 Tony763 force-pushed the feature/github_actions branch 22 times, most recently from 3ea4a00 to d9c3505 Compare October 11, 2021 20:44
@Tony763
Copy link
Author

Tony763 commented Oct 11, 2021 •
edited

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

Also check README.md inside test directory.

@pfefferle
Copy link

@Tony763 there seems to be some "conflicting files"

@Tony763
Copy link
Author

Tony763 commented Oct 19, 2021

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
Copy link
Author

Tony763 commented Oct 20, 2021

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 😄

@Tony763
Copy link
Author

Tony763 commented Oct 20, 2021

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

@Tony763 Tony763 mentioned this pull request Oct 21, 2021
Merged
5 tasks
@krisgesling
Copy link

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! 🚀 🌈 🎉

@krisgesling krisgesling merged commit 3b3d1cc into MycroftAI:20.08 Oct 22, 2021
@krisgesling
Copy link

Helps if I setup GH pages first...

Re-running actions now

pfefferle added a commit to pfefferle/skill-homeassistant that referenced this pull request Oct 22, 2021
@pfefferle
changes based on MycroftAI#77
4d030d3
@pfefferle
Copy link

Hmmm, I get a lot of errors like:

Resource not accessible by integration

@pfefferle
Copy link

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

@Tony763
Copy link
Author

Tony763 commented Oct 22, 2021

@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
Copy link

@Tony763 done: Tony763#37

stratus-ss pushed a commit that referenced this pull request Oct 25, 2021
@pfefferle @Tony763
Basic cover support (#80)
b2c1d4e 
* initial

* add german translation

* fix wrong entity type

* add `cover_stop` intent

* nicer german intents

* updated readme

* add cover to `_handle_sensor` method, to request status of cover

* moved `cover` to the right place

* fix text syntax

* check cover states

* fix typo

* Czech translation

* fix typo

* nicer german wordings

* added TODO

* remove logging

* some text changes

based on feedback by @stratus-ss

* add optional "the"

* changes based on #77

* add some code documentations

* pylint fix

thanks @Tony763

* fix another pylint error

* add missing f

* add audio feedback

Co-authored-by: Antonín Skala <skala.antonin@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@krisgesling krisgesling

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Tony763 @krisgesling @pfefferle