Introduce github-actions (push & pull_requests) #2025

opacam · 2019-11-17T16:02:32Z

I just want to share some of our travis tests but for gh-actions:

In here we perform the same tox tests that we do in travis but for linux and MacOs (for python versions 3.6 and 3.7...so we use a matrix) plus an flake8 test (which will make stop all the other tests if not succeed).

Regarding testsapp, we only perform one of the tests (Python3 for arm64-v8a in ubuntu with docker), so we make less tests than travis regarding testapps. The good thing of this is that we run the tox tests in MacOs, which make me notice that we had wrong some of our tests so we solve this in here.

Why introduce gh-actions?

Well, because it's one more asset that we can use, and this is only one small part of what can be achieved with gh-actions. IMHO, I think that we can benefit from gh-actions, I wouldn't remove travis tests at all, but I would use gh-actions for any operation that requires to use passwords, since it's more easy to deal than with travis. In matter fact, I already used a github.secret (COVERALLS_REPO_TOKEN, to send coveralls report), that we may need to setup for kivy (I'm not sure of this...it maybe already added to kivy organization...).

Note: I leave this PR in WIP status because:

we never talked about introducing gh-actions for p4a...maybe the kivy/p4a team doesn't want this at all...we will see...
with this we will send the coveralls report with travis and with gh-actions...probably we only want to send the reports with one of the services...so this PR may require some adjustments...
While doing this (some weeks ago), I felt that gh-actions was a little clumsy...gh-actions was still in beta status...now are official but it still may be a little clumsy...I suppose that it will get better with time...)

Some references:

https://github.com/features/actions
Github actions video presentation: https://www.youtube.com/watch?v=E1OunoCyuhY

Because the compiler path depends on the platform where the tests are run.

opacam · 2019-11-17T16:12:10Z

The CI tests for gh-actions for this PR can be seen at my fork, here the one for the last commit

AndreMiras

Awesome work 👏
I'm not familiar with github-actions yet. My only concern is its pretty recent and we don't have much hindsight about the tool.
Also I wish we didn't repeat ourselves and leveraged tox or Makefile more if possible

AndreMiras · 2019-11-17T22:22:11Z

.github/workflows/push.yml

+      run: |
+        python -m pip install --upgrade pip
+        pip install -U flake8
+        flake8 --ignore=E123,E124,E126,E226,E402,E501,W503,W504 pythonforandroid/ tests/ ci/


why not a tox -e flake8?

Aaa...yes...that should work, honestly, I had a different idea about flake8 tests (and never finished that) ...let me explain...

With some gh-actions, we can interact with the web ui interface of github, so, I was thinking that it would be great that the detected PEP8 errors could be notified automatically at PRs ...mmm...better check this action to get a better idea: https://github.com/suo/flake8-github-action . So... I did not implement this because that flake8 action cannot be configured (at least for now, but it could be another options...needs investigation)

So, said this, we could give it a try to the tox command (I will try this when I have some time), but I would keep one eye on this flake8 action mentioned above, because we could have some improvements here...specially thinking on PR reviewers...it seems that the red cross in travis tests, somehow and sometimes, is not seen, right? 😆

AndreMiras · 2019-11-17T22:25:05Z

.github/workflows/push.yml

+        pip install -e .
+    - name: Tests
+      run: |
+        python -m pytest tests/ --ignore tests/test_pythonpackage.py --cov pythonforandroid/ --cov-branch


I'm missing why we don't run these through tox since it's already configured with dependencies and run commands? Is running tox cumbersome with github-actions?

AndreMiras · 2019-11-17T22:30:08Z

.github/workflows/push.yml

+    runs-on: ubuntu-latest
+    steps:
+    - name: Checkout python-for-android
+      uses: actions/checkout@master


Shouldn't the branch be develop here and the other places? Or this is not related to the p4a branch?

Aaa...first of all, I'm not an gh-actions expert at all, but, I will try to put some light in here, so here we go:

gh-actions works based on events (In this PR we listen and react to push and pull_requests github events...so we kind of imitate travis behaviour). But, there is a lot of events that we can use, just check this if you want to get more detailed info about events.

gh-actions is not just a name, it's a set of repos: https://github.com/actions. Those repos can be used by any other repo.

Said this, when we call the line:

uses: actions/checkout@master

What are we doing is executing the code located at: https://github.com/actions/checkout, which simply will run the proper git commands, so our workflow can access the content of our repo. The @master just point to the version that we use for the specific github action (master branch in this case but it could be a release...).

Ok, hope that this clarifies a little this particular gh-actions thing 😉

Yes it clarifies, I was exactly not sure if this branch was our or the tooling one, thanks 👍

AndreMiras · 2019-11-17T22:34:07Z

.github/workflows/push.yml

+      run: |
+        python -m coveralls
+      env:
+        COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_REPO_TOKEN }}


For Travis this isn't required somehow because Coveralls has some "magic integration" that makes it seamless. Is it for sure required with github-actions?

Yes, we need it: https://github.com/opacam/python-for-android/runs/317382295#step:5:6

Thank you for trying out. I also link the actual log output here "for posterity":

Not on Travis or CircleCI. You have to provide either repo_token in .coveralls.yml or set the COVERALLS_REPO_TOKEN env var. Traceback (most recent call last): File "/opt/hostedtoolcache/Python/3.7.5/x64/lib/python3.7/site-packages/coveralls/cli.py", line 61, in main service_name=options['--service']) File "/opt/hostedtoolcache/Python/3.7.5/x64/lib/python3.7/site-packages/coveralls/api.py", line 58, in __init__ self.ensure_token() File "/opt/hostedtoolcache/Python/3.7.5/x64/lib/python3.7/site-packages/coveralls/api.py", line 67, in ensure_token self.config_filename)) coveralls.exception.CoverallsException: Not on Travis or CircleCI. You have to provide either repo_token in .coveralls.yml or set the COVERALLS_REPO_TOKEN env var. ##[error]Process completed with exit code 1.

AndreMiras · 2019-11-17T22:35:41Z

tests/recipes/recipe_lib_test.py

@@ -13,7 +14,7 @@ class BaseTestForMakeRecipe(RecipeCtx):

    recipe_name = None
    expected_compiler = (
-        "{android_ndk}/toolchains/llvm/prebuilt/linux-x86_64/bin/clang"
+        "{android_ndk}/toolchains/llvm/prebuilt/{system}-x86_64/bin/clang"


So basically unit test was failing in osx? Maybe it could be a PR on its own with these fixes

We also remove the env var `COVERALLS_REPO_TOKEN`, so we test if coveralls is working with `gh-actions` Note: Since Python 2 it's almost at the end of his life we only perform the Python3 tests, but we still have the travis tests for Python 2

AndreMiras

Thanks again for giving it a try and addressing the comments.
I think it's looking good enough already

AndreMiras · 2019-11-23T13:58:47Z

.github/workflows/push.yml

+        tox -e py3 -- tests/ --ignore tests/test_pythonpackage.py
+    - name: Send Coverage to coveralls
+      run: |
+        coveralls


Since coveralls is also part of our tox.ini, I'm wondering if this one could also be merged.
Or maybe better, we could try reusing the Makefile which is already dealing with --ignore tests/test_pythonpackage.py and coveralls report in the test target.
https://github.com/kivy/python-for-android/blob/b27ff0b/Makefile#L29-L31

Yeah, I like it, this way the dilemma of deciding when we should send the report to coveralls it will be solved, since CI environment variable will only exist for travis, so no report will be send with gh-actions unless we change that in the makefile by the gh-actions equivalent (which is GITHUB_ACTIONS...for the record 😉).

Thanks again @AndreMiras 😄

Nice it looks simpler. Good to know for the env variable

The coveralls report will only be send with our travis tests, since the `CI` environment variable it will only exist for travis.

opacam · 2019-11-23T15:52:20Z

BTW, I think that we should include the rebuild updated recipes test in here, to try to solve the errors we have with some recipes in travis like at #1997 ...what do you think?

update: Let's try it, and see what happens with one of those heavy recipes...

The purpose of this commit is to test the rebuild updated recipes behaviour with a long time consuming recipe

opacam · 2019-11-23T16:53:25Z

As expected, the test for rebuild updated recipes is failling with travis with:

No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.

Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#build-times-out-because-no-output-was-received

The build has been terminated

Ahahaha, but it succeed with gh-actions 😄

This reverts commit c6c3ea8, to keep travis happy

AndreMiras

LGTM: Nice to also see it in action with rebuild_updated_recipes.

opacam · 2019-11-23T18:32:24Z

Ok then, I think that we can merge this already, but before, here I leave the last gh-actions CI test, corresponding to the last commit (remember that all the gh-actions tests for this PR are in my fork)

Please @AndreMiras, do the merge for this one (or give me the order to do the squash/merge and I will do it), since you had some concerns about this gh-actions thing, I would like having your consent to merge this one (btw...whatever is your decision, is fine to me 😉)

AndreMiras · 2019-11-23T20:02:41Z

I'm happy with it that's why it's approved, I let you the pleasure to hit that (squash &) merge button 👍
Thanks again for your time investigating it

opacam · 2019-11-23T20:11:06Z

Done, here we have it, also accessible via the ▶️ Actions tab at github interface 😄

opacam added 6 commits November 17, 2019 15:25

[actions] Add push workflow (linting)

508e94d

[actions] Fix expected compiler for tests

23b8907

Because the compiler path depends on the platform where the tests are run.

[actions] Mock environ variables for test_create_no_sdk_dir

3b6448f

[actions] Fix test_toolchain.test_create for macos

5e0d7ed

[actions] Fix venv/virtualenv tests

f0d6a62

[actions] Add build stage: pull docker image & build an testapp

23d35a6

AndreMiras previously approved these changes Nov 17, 2019

View reviewed changes

opacam dismissed AndreMiras’s stale review via 397b3c0 November 23, 2019 12:04

[actions] Make flake8 tests via tox

3611a9e

opacam force-pushed the feature-github-actions branch from 397b3c0 to 3611a9e Compare November 23, 2019 12:07

[actions] Run pytest via tox

4f0f86a

We also remove the env var `COVERALLS_REPO_TOKEN`, so we test if coveralls is working with `gh-actions` Note: Since Python 2 it's almost at the end of his life we only perform the Python3 tests, but we still have the travis tests for Python 2

opacam force-pushed the feature-github-actions branch from 468d647 to 4f0f86a Compare November 23, 2019 13:25

[actions] Reintroduce COVERALLS_REPO_TOKEN

8416bfe

AndreMiras previously approved these changes Nov 23, 2019

View reviewed changes

[actions] Make tox tests via Makefile

b9e66ea

The coveralls report will only be send with our travis tests, since the `CI` environment variable it will only exist for travis.

opacam dismissed AndreMiras’s stale review via b9e66ea November 23, 2019 15:37

opacam added 2 commits November 23, 2019 17:01

[actions] Add rebuild updated recipes test

643a7f7

[actions] Shorten long line below 79 characters for libtorrent

c6c3ea8

The purpose of this commit is to test the rebuild updated recipes behaviour with a long time consuming recipe

[actions] Revert "[actions] Shorten long line ..."

0a93908

This reverts commit c6c3ea8, to keep travis happy

AndreMiras approved these changes Nov 23, 2019

View reviewed changes

opacam changed the title ~~[WIP] Introduce github-actions (push & pull_requests)~~ Introduce github-actions (push & pull_requests) Nov 23, 2019

opacam merged commit 11a9eb0 into kivy:develop Nov 23, 2019

opacam deleted the feature-github-actions branch November 23, 2019 20:11

AndreMiras mentioned this pull request Nov 24, 2019

Revamp .travis.yml file #2008

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce github-actions (push & pull_requests) #2025

Introduce github-actions (push & pull_requests) #2025

opacam commented Nov 17, 2019

opacam commented Nov 17, 2019

AndreMiras left a comment

AndreMiras Nov 17, 2019

opacam Nov 19, 2019

AndreMiras Nov 17, 2019

AndreMiras Nov 17, 2019

opacam Nov 19, 2019

AndreMiras Nov 19, 2019

AndreMiras Nov 17, 2019

opacam Nov 23, 2019

AndreMiras Nov 23, 2019

AndreMiras Nov 17, 2019

AndreMiras left a comment

AndreMiras Nov 23, 2019

opacam Nov 23, 2019

AndreMiras Nov 23, 2019

opacam commented Nov 23, 2019 •

edited

Loading

opacam commented Nov 23, 2019

AndreMiras left a comment

opacam commented Nov 23, 2019

AndreMiras commented Nov 23, 2019

opacam commented Nov 23, 2019

Introduce github-actions (push & pull_requests) #2025

Introduce github-actions (push & pull_requests) #2025

Conversation

opacam commented Nov 17, 2019

opacam commented Nov 17, 2019

AndreMiras left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndreMiras left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

opacam commented Nov 23, 2019 • edited Loading

opacam commented Nov 23, 2019

AndreMiras left a comment

Choose a reason for hiding this comment

opacam commented Nov 23, 2019

AndreMiras commented Nov 23, 2019

opacam commented Nov 23, 2019

opacam commented Nov 23, 2019 •

edited

Loading