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.

Sign up for GitHub

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

Introduce github-actions (push & pull_requests) #2025

Merged
merged 13 commits into from
Nov 23, 2019
Merged

Introduce github-actions (push & pull_requests) #2025

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
508e94d
[actions] Add push workflow (linting)
opacam Sep 21, 2019
23b8907
[actions] Fix `expected compiler` for tests
opacam Sep 21, 2019
3b6448f
[actions] Mock environ variables for `test_create_no_sdk_dir`
opacam Sep 21, 2019
5e0d7ed
[actions] Fix `test_toolchain.test_create` for macos
opacam Sep 21, 2019
f0d6a62
[actions] Fix venv/virtualenv tests
opacam Sep 21, 2019
23d35a6
[actions] Add build stage: pull docker image & build an testapp
opacam Sep 21, 2019
3611a9e
[actions] Make flake8 tests via tox
opacam Nov 23, 2019
4f0f86a
[actions] Run pytest via tox
opacam Nov 23, 2019
8416bfe
[actions] Reintroduce `COVERALLS_REPO_TOKEN`
opacam Nov 23, 2019
b9e66ea
[actions] Make tox tests via Makefile
opacam Nov 23, 2019
643a7f7
[actions] Add `rebuild updated recipes` test
opacam Nov 23, 2019
c6c3ea8
[actions] Shorten long line below 79 characters for libtorrent
opacam Nov 23, 2019
0a93908
[actions] Revert "[actions] Shorten long line ..."
opacam Nov 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions .github/workflows/push.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
name: Unit tests & Build Testapp

on: ['push', 'pull_request']

jobs:

flake8:
name: Flake8 tests
runs-on: ubuntu-latest
steps:
- name: Checkout python-for-android
uses: actions/checkout@master
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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 😉

Copy link
Member

Choose a reason for hiding this comment

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

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

- name: Set up Python 3.7
uses: actions/setup-python@v1.1.0
with:
python-version: 3.7
- name: Run flake8
# we use exactly the same exceptions than tox.ini file
run: |
python -m pip install --upgrade pip
pip install -U flake8
flake8 --ignore=E123,E124,E126,E226,E402,E501,W503,W504 pythonforandroid/ tests/ ci/
Copy link
Member

Choose a reason for hiding this comment

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

why not a tox -e flake8?

Copy link
Member Author

Choose a reason for hiding this comment

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

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? 😆


linting:
name: Pytest [Python ${{ matrix.python-version }} | ${{ matrix.os }}]
needs: flake8
runs-on: ${{ matrix.os }}
strategy:
matrix:
python-version: [3.6, 3.7]
os: [ubuntu-latest, macOs-latest]
steps:
- name: Checkout python-for-android
uses: actions/checkout@master
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v1.1.0
with:
python-version: ${{ matrix.python-version }}
- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install -U virtualenv pytest pytest-cov backports.tempfile pyOpenSSL coveralls
pip install -e .
- name: Tests
run: |
python -m pytest tests/ --ignore tests/test_pythonpackage.py --cov pythonforandroid/ --cov-branch
Copy link
Member

Choose a reason for hiding this comment

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

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?

- name: Coveralls
run: |
python -m coveralls
env:
COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_REPO_TOKEN }}
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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.


build:
name: Build testapp
needs: [flake8]
runs-on: ubuntu-latest
steps:
- name: Checkout python-for-android
uses: actions/checkout@master
- name: Pull docker image
run: |
make docker/pull
- name: Build apk for Python 3 arm64-v8a
run: |
make docker/run/make/testapps/python3/arm64-v8a
9 changes: 5 additions & 4 deletions tests/recipes/recipe_lib_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from unittest import mock
from platform import system
from tests.recipes.recipe_ctx import RecipeCtx


Expand All @@ -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"
Copy link
Member

Choose a reason for hiding this comment

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

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

)

sh_command_calls = ["./configure"]
Expand Down Expand Up @@ -48,7 +49,7 @@ def test_get_recipe_env(
some internal methods has been called.
"""
mock_find_executable.return_value = self.expected_compiler.format(
android_ndk=self.ctx._ndk_dir
android_ndk=self.ctx._ndk_dir, system=system().lower()
)
mock_glob.return_value = ["llvm"]
mock_check_recipe_choices.return_value = sorted(
Expand Down Expand Up @@ -85,7 +86,7 @@ def test_build_arch(
mock_current_directory,
):
mock_find_executable.return_value = self.expected_compiler.format(
android_ndk=self.ctx._ndk_dir
android_ndk=self.ctx._ndk_dir, system=system().lower()
)
mock_glob.return_value = ["llvm"]

Expand Down Expand Up @@ -133,7 +134,7 @@ def test_build_arch(
mock_current_directory,
):
mock_find_executable.return_value = self.expected_compiler.format(
android_ndk=self.ctx._ndk_dir
android_ndk=self.ctx._ndk_dir, system=system().lower()
)
mock_glob.return_value = ["llvm"]

Expand Down
3 changes: 2 additions & 1 deletion tests/recipes/test_icu.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import os
import unittest
from unittest import mock
from platform import system

from tests.recipes.recipe_ctx import RecipeCtx
from pythonforandroid.recipes.icu import ICURecipe
Expand Down Expand Up @@ -47,7 +48,7 @@ def test_build_arch(
):
mock_find_executable.return_value = os.path.join(
self.ctx._ndk_dir,
"toolchains/llvm/prebuilt/linux-x86_64/bin/clang",
f"toolchains/llvm/prebuilt/{system().lower()}-x86_64/bin/clang",
)
mock_archs_glob.return_value = [
os.path.join(self.ctx._ndk_dir, "toolchains", "llvm")
Expand Down
5 changes: 3 additions & 2 deletions tests/test_archs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import unittest
from os import environ
from unittest import mock
from platform import system

from pythonforandroid.bootstrap import Bootstrap
from pythonforandroid.distribution import Distribution
Expand Down Expand Up @@ -70,8 +71,8 @@ def setUp(self):
# Here we define the expected compiler, which, as per ndk >= r19,
# should be the same for all the tests (no more gcc compiler)
self.expected_compiler = (
"/opt/android/android-ndk/toolchains/"
"llvm/prebuilt/linux-x86_64/bin/clang"
f"/opt/android/android-ndk/toolchains/"
f"llvm/prebuilt/{system().lower()}-x86_64/bin/clang"
)


Expand Down
4 changes: 3 additions & 1 deletion tests/test_bootstrap.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import unittest

from unittest import mock
from platform import system

from pythonforandroid.bootstrap import (
_cmp_bootstraps_by_priority, Bootstrap, expand_dependencies,
)
Expand Down Expand Up @@ -546,7 +548,7 @@ def test_bootstrap_strip(
):
mock_find_executable.return_value = os.path.join(
self.ctx._ndk_dir,
"toolchains/llvm/prebuilt/linux-x86_64/bin/clang",
f"toolchains/llvm/prebuilt/{system().lower()}-x86_64/bin/clang",
)
mock_glob.return_value = [
os.path.join(self.ctx._ndk_dir, "toolchains", "llvm")
Expand Down
8 changes: 6 additions & 2 deletions tests/test_pythonpackage_basic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,9 @@ def test_virtualenv(self):
sys_python_path = self.run__get_system_python_executable(
os.path.join(test_dir, "virtualenv", "bin", "python")
)
assert os.path.normpath(sys_python_path) == os.path.normpath(pybin)
assert os.path.normpath(sys_python_path).startswith(
os.path.normpath(pybin)
)
finally:
shutil.rmtree(test_dir)

Expand Down Expand Up @@ -341,6 +343,8 @@ def test_venv(self):
sys_python_path = self.run__get_system_python_executable(
os.path.join(test_dir, "venv", "bin", "python")
)
assert os.path.normpath(sys_python_path) == os.path.normpath(pybin)
assert os.path.normpath(sys_python_path).startswith(
os.path.normpath(pybin)
)
finally:
shutil.rmtree(test_dir)
6 changes: 4 additions & 2 deletions tests/test_recipe.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import warnings
from unittest import mock
from backports import tempfile
from platform import system

from pythonforandroid.build import Context
from pythonforandroid.recipe import Recipe, import_recipe
from pythonforandroid.archs import ArchAarch_64
Expand Down Expand Up @@ -278,8 +280,8 @@ def test_get_recipe_env_with(
should be tested in the proper test.
"""
expected_compiler = (
"/opt/android/android-ndk/toolchains/"
"llvm/prebuilt/linux-x86_64/bin/clang"
f"/opt/android/android-ndk/toolchains/"
f"llvm/prebuilt/{system().lower()}-x86_64/bin/clang"
)
mock_find_executable.return_value = expected_compiler
mock_glob.return_value = ["llvm"]
Expand Down
16 changes: 12 additions & 4 deletions tests/test_toolchain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,14 @@ def test_create(self):
m_get_ndk_platform_dir.return_value = (
'/tmp/android-ndk/platforms/android-21/arch-arm', True)
ToolchainCL()
assert m_get_available_apis.call_args_list == [
mock.call('/tmp/android-sdk')]
assert m_get_toolchain_versions.call_args_list == [
mock.call('/tmp/android-ndk', mock.ANY)]
assert m_get_available_apis.call_args_list in [
[mock.call('/tmp/android-sdk')], # linux case
[mock.call('/private/tmp/android-sdk')] # macos case
]
assert m_get_toolchain_versions.call_args_list in [
[mock.call('/tmp/android-ndk', mock.ANY)], # linux case
[mock.call('/private/tmp/android-ndk', mock.ANY)], # macos case
]
build_order = [
'hostpython3', 'libffi', 'openssl', 'sqlite3', 'python3',
'genericndkbuild', 'setuptools', 'six', 'pyjnius', 'android',
Expand All @@ -100,6 +104,10 @@ def test_create(self):
]
assert m_run_distribute.call_args_list == [mock.call()]

@mock.patch(
'pythonforandroid.build.environ',
# Make sure that no environ variable modifies `sdk_dir`
{'ANDROIDSDK': None, 'ANDROID_HOME': None})
def test_create_no_sdk_dir(self):
"""
The `--sdk-dir` is mandatory to `create` a distribution.
Expand Down