Skip to content

Commit 5e90f06

Browse files
opacamopacam
authored
✅ Add tests for hostpython3 recipe (kivy#2196)
This will cover **100%** of the code for the hostpython3 recipe. Also we migrate `os.path.exists` and `os.path.isfile` to `pathlib.Path`'s implementation because when we mock these functions, it seems that property `return_value` is being ignored.
1 parent 5448f34 commit 5e90f06

File tree

2 files changed

+183
-12
lines changed

2 files changed

+183
-12
lines changed

‎pythonforandroid/recipes/hostpython3/__init__.py

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import sh
22

33
from multiprocessing import cpu_count
4-
from os.path import exists, join, isfile
4+
from pathlib import Path
5+
from os.path import join
56

67
from pythonforandroid.logger import shprint
78
from pythonforandroid.patching import is_version_lt
@@ -12,6 +13,14 @@
1213
ensure_dir,
1314
)
1415

16+
HOSTPYTHON_VERSION_UNSET_MESSAGE = (
17+
'The hostpython recipe must have set version'
18+
)
19+
20+
SETUP_DIST_NOT_FIND_MESSAGE = (
21+
'Could not find Setup.dist or Setup in Python build'
22+
)
23+
1524

1625
class HostPython3Recipe(Recipe):
1726
'''
@@ -46,19 +55,16 @@ def _exe_name(self):
4655
Returns the name of the python executable depending on the version.
4756
'''
4857
if not self.version:
49-
raise BuildInterruptingException(
50-
'The hostpython recipe must have set version'
51-
)
52-
version = self.version.split('.')[0]
53-
return 'python{major_version}'.format(major_version=version)
58+
raise BuildInterruptingException(HOSTPYTHON_VERSION_UNSET_MESSAGE)
59+
return f'python{self.version.split(".")[0]}'
5460

5561
@property
5662
def python_exe(self):
5763
'''Returns the full path of the hostpython executable.'''
5864
return join(self.get_path_to_python(), self._exe_name)
5965

6066
def should_build(self, arch):
61-
if exists(self.python_exe):
67+
if Path(self.python_exe).exists():
6268
# no need to build, but we must set hostpython for our Context
6369
self.ctx.hostpython = self.python_exe
6470
return False
@@ -88,23 +94,24 @@ def build_arch(self, arch):
8894

8995
# Configure the build
9096
with current_directory(build_dir):
91-
if not exists('config.status'):
97+
if not Path('config.status').exists():
9298
shprint(sh.Command(join(recipe_build_dir, 'configure')))
9399

94100
with current_directory(recipe_build_dir):
95101
# Create the Setup file. This copying from Setup.dist is
96102
# the normal and expected procedure before Python 3.8, but
97103
# after this the file with default options is already named "Setup"
98104
setup_dist_location = join('Modules', 'Setup.dist')
99-
if exists(setup_dist_location):
105+
if Path(setup_dist_location).exists():
100106
shprint(sh.cp, setup_dist_location,
101107
join(build_dir, 'Modules', 'Setup'))
102108
else:
103109
# Check the expected file does exist
104110
setup_location = join('Modules', 'Setup')
105-
if not exists(setup_location):
111+
if not Path(setup_location).exists():
106112
raise BuildInterruptingException(
107-
"Could not find Setup.dist or Setup in Python build")
113+
SETUP_DIST_NOT_FIND_MESSAGE
114+
)
108115

109116
shprint(sh.make, '-j', str(cpu_count()), '-C', build_dir)
110117

@@ -115,7 +122,7 @@ def build_arch(self, arch):
115122
# for our hostpython, regarding the used fs
116123
for exe_name in ['python.exe', 'python']:
117124
exe = join(self.get_path_to_python(), exe_name)
118-
if isfile(exe):
125+
if Path(exe).is_file():
119126
shprint(sh.cp, exe, self.python_exe)
120127
break
121128

‎tests/recipes/test_hostpython3.py

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
import unittest
2+
3+
from os.path import join
4+
from unittest import mock
5+
6+
from pythonforandroid.recipes.hostpython3 import (
7+
HOSTPYTHON_VERSION_UNSET_MESSAGE, SETUP_DIST_NOT_FIND_MESSAGE,
8+
)
9+
from pythonforandroid.util import BuildInterruptingException
10+
from tests.recipes.recipe_lib_test import RecipeCtx
11+
12+
13+
class TestHostPython3Recipe(RecipeCtx, unittest.TestCase):
14+
"""
15+
TestCase for recipe :mod:`~pythonforandroid.recipes.hostpython3`
16+
"""
17+
recipe_name = "hostpython3"
18+
19+
def test_property__exe_name_no_version(self):
20+
hostpython_version = self.recipe.version
21+
self.recipe._version = None
22+
with self.assertRaises(BuildInterruptingException) as e:
23+
py_exe = self.recipe._exe_name # noqa: F841
24+
self.assertEqual(e.exception.args[0], HOSTPYTHON_VERSION_UNSET_MESSAGE)
25+
# restore recipe's version or we will get failures with other test,
26+
# since we share `self.recipe with all the tests of the class
27+
self.recipe._version = hostpython_version
28+
29+
def test_property__exe_name(self):
30+
self.assertEqual(self.recipe._exe_name, 'python3')
31+
32+
def test_property_python_exe(self):
33+
self.assertEqual(
34+
self.recipe.python_exe,
35+
join(self.recipe.get_path_to_python(), 'python3')
36+
)
37+
38+
@mock.patch("pythonforandroid.recipes.hostpython3.Path.exists")
39+
def test_should_build(self, mock_exists):
40+
# test case for existing python exe which shouldn't trigger the build
41+
mock_exists.return_value = True
42+
self.assertFalse(self.recipe.should_build(self.arch))
43+
# test case for existing python exe which should trigger the build
44+
mock_exists.return_value = False
45+
self.assertTrue(self.recipe.should_build(self.arch))
46+
47+
@mock.patch("pythonforandroid.util.chdir")
48+
@mock.patch("pythonforandroid.util.makedirs")
49+
def test_build_arch(self, mock_makedirs, mock_chdir):
50+
"""
51+
Test case for
52+
:meth:`~pythonforandroid.recipes.python3.HostPython3Recipe.build_arch`,
53+
where we simulate the build for Python 3.8+.
54+
"""
55+
56+
with mock.patch(
57+
"pythonforandroid.recipes.hostpython3.Path.exists"
58+
) as mock_path_exists, mock.patch(
59+
"pythonforandroid.recipes.hostpython3.sh.Command"
60+
) as mock_sh_command, mock.patch(
61+
"pythonforandroid.recipes.hostpython3.sh.make"
62+
) as mock_make, mock.patch(
63+
"pythonforandroid.recipes.hostpython3.Path.is_file"
64+
) as mock_path_isfile, mock.patch(
65+
"pythonforandroid.recipes.hostpython3.sh.cp"
66+
) as mock_sh_cp:
67+
# here we simulate the expected behaviour for Python 3.8+
68+
mock_path_exists.side_effect = [
69+
False, # "config.status" not exists, so we trigger.configure
70+
False, # "Modules/Setup.dist" shouldn't exist (3.8+ case)
71+
True, # "Modules/Setup" exists, so we skip raise exception
72+
]
73+
self.recipe.build_arch(self.arch)
74+
75+
# make sure that the mocked methods are actually called
76+
mock_path_exists.assert_called()
77+
recipe_src = self.recipe.get_build_dir(self.arch.arch)
78+
self.assertIn(
79+
mock.call(f"{recipe_src}/configure"),
80+
mock_sh_command.mock_calls,
81+
)
82+
mock_make.assert_called()
83+
84+
exe = join(self.recipe.get_path_to_python(), 'python.exe')
85+
mock_path_isfile.assert_called()
86+
87+
self.assertEqual(mock_sh_cp.call_count, 1)
88+
mock_call_args, mock_call_kwargs = mock_sh_cp.call_args_list[0]
89+
self.assertEqual(mock_call_args[0], exe)
90+
self.assertEqual(mock_call_args[1], self.recipe.python_exe)
91+
mock_makedirs.assert_called()
92+
mock_chdir.assert_called()
93+
94+
@mock.patch("pythonforandroid.util.chdir")
95+
@mock.patch("pythonforandroid.util.makedirs")
96+
def test_build_arch_python_lower_than_3_8(self, mock_makedirs, mock_chdir):
97+
"""
98+
Test case for
99+
:meth:`~pythonforandroid.recipes.python3.HostPython3Recipe.build_arch`,
100+
where we simulate a Python 3.7 build. Here we copy an extra file:
101+
- Modules/Setup.dist -> Modules/Setup.
102+
103+
.. note:: We omit some checks because we already dit that at
104+
`test_build_arch`. Also we skip configure command for the
105+
same reason.
106+
"""
107+
with mock.patch(
108+
"pythonforandroid.recipes.hostpython3.Path.exists"
109+
) as mock_path_exists, mock.patch(
110+
"pythonforandroid.recipes.hostpython3.sh.make"
111+
) as mock_make, mock.patch(
112+
"pythonforandroid.recipes.hostpython3.Path.is_file"
113+
) as mock_path_isfile, mock.patch(
114+
"pythonforandroid.recipes.hostpython3.sh.cp"
115+
) as mock_sh_cp:
116+
mock_path_exists.side_effect = [
117+
True, # simulate that "config.status" exists to skip configure
118+
True, # "Modules/Setup.dist" exist for Python 3.7
119+
True, # "Modules/Setup" exists...we skip raise exception
120+
]
121+
self.recipe.build_arch(self.arch)
122+
123+
build_dir = join(
124+
self.recipe.get_build_dir(self.arch.arch), self.recipe.build_subdir
125+
)
126+
# we expect two calls to copy command, The one described below and the
127+
# copy of the python binary which already chechet at `test_build_arch`.
128+
self.assertEqual(mock_sh_cp.call_count, 2)
129+
mock_call_args, mock_call_kwargs = mock_sh_cp.call_args_list[0]
130+
self.assertEqual(mock_call_args[0], "Modules/Setup.dist")
131+
self.assertEqual(mock_call_args[1], join(build_dir, "Modules/Setup"))
132+
133+
mock_path_exists.assert_called()
134+
mock_make.assert_called()
135+
mock_path_isfile.assert_called()
136+
mock_makedirs.assert_called()
137+
mock_chdir.assert_called()
138+
139+
@mock.patch("pythonforandroid.util.chdir")
140+
@mock.patch("pythonforandroid.util.makedirs")
141+
def test_build_arch_setup_dist_exception(self, mock_makedirs, mock_chdir):
142+
"""
143+
Test case for
144+
:meth:`~pythonforandroid.recipes.python3.HostPython3Recipe.build_arch`,
145+
where we simulate that the sources hasn't Setup.dist file, which should
146+
raise an exception.
147+
148+
.. note:: We skip configure command because already tested at
149+
`test_build_arch`.
150+
"""
151+
with mock.patch(
152+
"pythonforandroid.recipes.hostpython3.Path.exists"
153+
) as mock_path_exists:
154+
mock_path_exists.side_effect = [
155+
True, # simulate that "config.status" exists to skip configure
156+
False, # "Modules/Setup.dist" shouldn't exist (3.8+ case)
157+
False, # "Modules/Setup" doesn't exists...raise exception
158+
]
159+
160+
with self.assertRaises(BuildInterruptingException) as e:
161+
self.recipe.build_arch(self.arch)
162+
self.assertEqual(e.exception.args[0], SETUP_DIST_NOT_FIND_MESSAGE)
163+
mock_makedirs.assert_called()
164+
mock_chdir.assert_called()

0 commit comments

Comments
 (0)