Add unittest for `pythonforandroid.util` and... #1855

opacam · 2019-06-10T10:52:58Z

Also:

Remove unused functions from pythonforandroid.util
Make tox use Python 3.7 for the travis's lint stage (so we can use some unittest methods introduced in python 3.6 (see also: mock docs)
Enhance some tests for test_distribution verifying the number of calls we made to some functions. @AndreMiras, this is for you, that reviewed the pr where we add those tests and you noticed those missing verifications 👍 (completes the work started at Add unittest for module pythonforandroid.distribution #1847)
Temporary skip some pythonpackage's tests for python > 3.5 in travis (Because it fails and I don't know why it fails. So this needs further investigation and a fix, @Jonast...could you take a look on this?)
Add docs for the recent added tests: test_archs, test_distribution and test_util

Notes:

In order to optimize the tests, I mocked any operation that can slow down our tests
The last point of the things done may be removed in case that we fix the pythonpackage tests before merging this

As a side note: ~~The pythonpackage's tests that fails are not failing for the same reason addressed in #1852 (see travis error). It seems that fails on _get_system_python_executable...~~ The pythonpackage tests issues mentioned are addressed at #1856 and #1852

pythonforandroid/util.py

ghost · 2019-06-10T11:24:32Z

Can you give me the failing tests output? Also it would be really better if you didn't mark them as skip even temporarily, if we ever accidentally merge this that might be a bit of a problem

ghost · 2019-06-10T11:27:27Z

tests/test_pythonpackage_basic.py

@@ -80,6 +93,11 @@ def test_get_package_name():
        shutil.rmtree(temp_d)


+# Todo: Fix CI tests for python > 3.5
+@pytest.mark.skipif(


please don't 😱 let me look at the output and I'll see what I can do, these shouldn't be skipped

opacam · 2019-06-10T11:28:42Z

@Jonast, thanks for the very quick response!!
See travis job at this link

tests/test_util.py

tests/test_distribution.py

tests/test_util.py

AndreMiras · 2019-06-10T21:57:38Z

WIP but already looking good well done 💪
Let us know once this is ready for another review/merge

Because we make use of some mock methods in module `test_util` (eg: `mock.assert_called_once`) which were introduced in python 3.6, so if the system python of the test system is lower than 3.6, then the tox tests will fail

Verifying the number of calls we made to some functions: - test_folder_exist - test_delete

…ggestions Reviewers suggestions are: - Remove forgotten debug code - `style code` change for `test_current_directory_exception` - add test `mock_shutil_rmtree.assert_not_called()` for `test_temp_directory` Also changed some lines that shouldn't be split (because are pep8 friendly without breaking it in multiple lines)

opacam · 2019-06-11T10:38:51Z

Aaaa, @Jonast...we still have an error in our tox tests for pythonpackage_basic:

https://travis-ci.org/kivy/python-for-android/jobs/544152688#L423-L428

Now it's different than before, we only have an error so we almost have it 😄

AndreMiras

Nice work thanks!

ghost · 2019-06-11T17:05:32Z

@opacam you need to make sure the virtualenv module is installed in the travis environment for python 3.7, it is not present by default and it's not optional, that should fix the error

opacam · 2019-06-11T22:50:19Z

Oh yes, I already saw that, the thing is that virtualenv it's already in there:

python-for-android/.travis.yml

Line 18 in d2ff49d

python2.7 python3 python3-venv python3-virtualenv python3-pip

To demonstrate that it is already installed in the travis's tox job, I created another branch from this pr's branch, with an extra commit with a couple of lines, just before the command to trigger tox:

      before_script:
        - pip install virtualenv
      script:
        # we want to fail fast on tox errors without having to `docker build` first
        - tox -- tests/ --ignore tests/test_pythonpackage.py

Yo can see that it's already installed in this travis job:

https://travis-ci.org/opacam/python-for-android/jobs/544471193#L263-L264

So the pythonpackage test isn't detecting it...or maybe I miss something....:thinking:

ghost · 2019-06-12T01:07:35Z

the thing is that virtualenv it's already in there:

most likely not for 3.7. the python 3.7 in that travis image is in a really weird directory, the mechanism you used for install might have installed it additionally and not as replacement. so python3-virtualenv is probably for 3.6 or whatever the regular system default is, and won't go into the 3.7 site-packages. you could check if there's pip3.7, that could be one way to get it installed (via pip3.7 install -U virtualenv or similar)

(that could also mean that venv is missing by the way. but that can't be installed via pip since it's supposed to be a core module which only the misguided ubuntu & debian packagers rip out. I'm not sure how to fix that, maybe there's a python3.7-venv package?)

summed up, the way 3.7 is installed in that travis image is really weird. I'm personally not a fan, I'm not sure why they do it like that. that is also one of the reasons why the function I wrote to find the system python initially broke

…tage Because we only need those dependencies for our `tox` tests so we may speed up a little the overall CI tests timings. We also remove all the other apt commands performed in `before_install` because we don't need it. Note: in order to successfully pass the test `test_pythonpackage_basic.test_virtualenv`, we need to deactivate the system virtualenv See also: travis-ci/travis-ci#8589

opacam · 2019-06-12T20:19:00Z

@Jonast 👍, thanks for your help with this issues with virtualenv

Now we have a green tick mark in our tox tests 😄

AndreMiras

Great work and more coverage, thanks 👏

opacam commented Jun 10, 2019

View reviewed changes

pythonforandroid/util.py Show resolved Hide resolved

ghost reviewed Jun 10, 2019

View reviewed changes

opacam force-pushed the unittest-util branch from b01c74b to 646f48d Compare June 10, 2019 13:46

opacam added the Priority: Medium This issue may be useful, and needs some attention. label Jun 10, 2019

inclement reviewed Jun 10, 2019

View reviewed changes

tests/test_util.py Outdated Show resolved Hide resolved

inclement reviewed Jun 10, 2019

View reviewed changes

tests/test_util.py Outdated Show resolved Hide resolved

AndreMiras reviewed Jun 10, 2019

View reviewed changes

tests/test_distribution.py Show resolved Hide resolved

AndreMiras reviewed Jun 10, 2019

View reviewed changes

tests/test_util.py Show resolved Hide resolved

AndreMiras reviewed Jun 10, 2019

View reviewed changes

tests/test_util.py Outdated Show resolved Hide resolved

AndreMiras reviewed Jun 10, 2019

View reviewed changes

tests/test_util.py Outdated Show resolved Hide resolved

opacam added 4 commits June 11, 2019 00:20

[tests] Add unittest for module pythonforandroid.util

38cfb89

[util] Remove unused functions from pythonforandroid.util

a24f1e3

[travis] Make tox use Python 3.7 for the travis's lint stage

e230b27

Because we make use of some mock methods in module `test_util` (eg: `mock.assert_called_once`) which were introduced in python 3.6, so if the system python of the test system is lower than 3.6, then the tox tests will fail

[tests] Enhance some tests for test_distribution

6f65aaf

Verifying the number of calls we made to some functions: - test_folder_exist - test_delete

opacam force-pushed the unittest-util branch from 646f48d to c89d312 Compare June 11, 2019 10:16

opacam added 3 commits June 11, 2019 12:17

[docs] Add documentation for module tests.test_distribution

c2b4c80

[docs] Add documentation for module tests.test_archs

d2ff49d

opacam force-pushed the unittest-util branch from c89d312 to d2ff49d Compare June 11, 2019 10:22

AndreMiras previously approved these changes Jun 11, 2019

View reviewed changes

opacam dismissed AndreMiras’s stale review via 32a4e45 June 12, 2019 20:04

opacam changed the title ~~[WIP] Add unittest for pythonforandroid.util and...~~ Add unittest for pythonforandroid.util and... Jun 12, 2019

AndreMiras approved these changes Jun 13, 2019

View reviewed changes

AndreMiras merged commit e599383 into kivy:develop Jun 13, 2019

opacam deleted the unittest-util branch June 15, 2019 11:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add unittest for `pythonforandroid.util` and... #1855

Add unittest for `pythonforandroid.util` and... #1855

opacam commented Jun 10, 2019 •

edited

Loading

ghost commented Jun 10, 2019

ghost Jun 10, 2019

opacam commented Jun 10, 2019

AndreMiras commented Jun 10, 2019

opacam commented Jun 11, 2019

AndreMiras left a comment

ghost commented Jun 11, 2019 •

edited by ghost

Loading

opacam commented Jun 11, 2019

ghost commented Jun 12, 2019 •

edited by ghost

Loading

opacam commented Jun 12, 2019

AndreMiras left a comment

Add unittest for pythonforandroid.util and... #1855

Add unittest for pythonforandroid.util and... #1855

Conversation

opacam commented Jun 10, 2019 • edited Loading

ghost commented Jun 10, 2019

ghost Jun 10, 2019

Choose a reason for hiding this comment

opacam commented Jun 10, 2019

AndreMiras commented Jun 10, 2019

opacam commented Jun 11, 2019

AndreMiras left a comment

Choose a reason for hiding this comment

ghost commented Jun 11, 2019 • edited by ghost Loading

opacam commented Jun 11, 2019

ghost commented Jun 12, 2019 • edited by ghost Loading

opacam commented Jun 12, 2019

AndreMiras left a comment

Choose a reason for hiding this comment

Add unittest for `pythonforandroid.util` and... #1855

Add unittest for `pythonforandroid.util` and... #1855

opacam commented Jun 10, 2019 •

edited

Loading

ghost commented Jun 11, 2019 •

edited by ghost

Loading

ghost commented Jun 12, 2019 •

edited by ghost

Loading