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

Mock assertion issue with ICU recipe under Python 3.12 #3002

Closed
5 tasks done
s-t-e-v-e-n-k opened this issue Apr 8, 2024 · 3 comments · Fixed by #3003
Closed
5 tasks done

Mock assertion issue with ICU recipe under Python 3.12 #3002

s-t-e-v-e-n-k opened this issue Apr 8, 2024 · 3 comments · Fixed by #3003

Comments

@s-t-e-v-e-n-k
Copy link
Contributor

Checklist

  • the issue is indeed a bug and not a support request
  • issue doesn't already exist: https://github.com/kivy/python-for-android/issues
  • I have a short, runnable example that reproduces the issue
  • I reproduced the problem with the latest development version (p4a.branch = develop)
  • I used the grave accent (aka backticks) to format code or logs when appropriated

Versions

  • Python: 3.12
  • OS: openSUSE Tumbleweed
  • Kivy:
  • Cython:
  • OpenJDK:

Description

Python 3.12 shows an issue with ICU recipe, with respect to mock usage:

[   13s] ________________________ TestIcuRecipe.test_build_arch _________________________
[   13s]
[   13s] self = <tests.recipes.test_icu.TestIcuRecipe testMethod=test_build_arch>
[   13s] mock_shutil_which = <MagicMock name='which' id='140502634298352'>
[   13s] mock_ensure_dir = <MagicMock name='ensure_dir' id='140502633992880'>
[   13s] mock_sh_make = <MagicMock name='make' id='140502634286352'>
[   13s] mock_sh_command = <MagicMock name='Command' id='140502637079024'>
[   13s] mock_chdir = <MagicMock name='chdir' id='140502637078016'>
[   13s] mock_makedirs = <MagicMock name='makedirs' id='140502637073360'>
[   13s]
[   13s]     @mock.patch("pythonforandroid.util.makedirs")
[   13s]     @mock.patch("pythonforandroid.util.chdir")
[   13s]     @mock.patch("pythonforandroid.bootstrap.sh.Command")
[   13s]     @mock.patch("pythonforandroid.recipes.icu.sh.make")
[   13s]     @mock.patch("pythonforandroid.build.ensure_dir")
[   13s]     @mock.patch("shutil.which")
[   13s]     def test_build_arch(
[   13s]         self,
[   13s]         mock_shutil_which,
[   13s]         mock_ensure_dir,
[   13s]         mock_sh_make,
[   13s]         mock_sh_command,
[   13s]         mock_chdir,
[   13s]         mock_makedirs,
[   13s]     ):
[   13s]         mock_shutil_which.return_value = os.path.join(
[   13s]             self.ctx._ndk_dir,
[   13s]             f"toolchains/llvm/prebuilt/{self.ctx.ndk.host_tag}/bin/clang",
[   13s]         )
[   13s]         self.ctx.toolchain_version = "4.9"
[   13s]         self.recipe.build_arch(self.arch)
[   13s]
[   13s]         # We expect some calls to `sh.Command`
[   13s]         build_root = self.recipe.get_build_dir(self.arch.arch)
[   13s] >       mock_sh_command.has_calls(
[   13s]             [
[   13s]                 mock.call(
[   13s]                     os.path.join(build_root, "source", "runConfigureICU")
[   13s]                 ),
[   13s]                 mock.call(os.path.join(build_root, "source", "configure")),
[   13s]             ]
[   13s]         )
[   13s]
[   13s] tests/recipes/test_icu.py:55:
[   13s] _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
[   13s]
[   13s] self = <MagicMock name='Command' id='140502637079024'>, name = 'has_calls'
[   13s]
[   13s]     def __getattr__(self, name):
[   13s]         if name in {'_mock_methods', '_mock_unsafe'}:
[   13s]             raise AttributeError(name)
[   13s]         elif self._mock_methods is not None:
[   13s]             if name not in self._mock_methods or name in _all_magics:
[   13s]                 raise AttributeError("Mock object has no attribute %r" % name)
[   13s]         elif _is_magic(name):
[   13s]             raise AttributeError(name)
[   13s]         if not self._mock_unsafe and (not self._mock_methods or name not in self._mock_methods):
[   13s]             if name.startswith(('assert', 'assret', 'asert', 'aseert', 'assrt')) or name in _ATTRIB_DENY_LIST:
[   13s] >               raise AttributeError(
[   13s]                     f"{name!r} is not a valid assertion. Use a spec "
[   13s]                     f"for the mock if {name!r} is meant to be an attribute.")
[   13s] E               AttributeError: 'has_calls' is not a valid assertion. Use a spec for the mock if 'has_calls' is meant to be an attribute.
[   13s]
[   13s] /usr/lib64/python3.12/unittest/mock.py:663: AttributeError

Python interpreters before 3.12 would just blindly call any method they were given -- which also means this hasn't been asserting correctly, either. The correct method name is assert_has_calls.

@T-Dynamos
Copy link
Contributor

How did you build python3.12? As p4a does not yet have 3.12 support. like this?:

requirements = python==3.12.2, hostpython==3.12.2

@s-t-e-v-e-n-k
Copy link
Contributor Author

Oh, sorry, I meant running the testsuite using pytest under Python 3.12.

@AndreMiras
Copy link
Member

Python interpreters before 3.12 would just blindly call any method they were given -- which also means this hasn't been asserting correctly, either. The correct method name is assert_has_calls

Good catch and I'm so glad they fixed that in Python 3.12! This was definitely a pit fall. I never liked the assertion helpers anyway because I think it better reads to use the play assert (all the assert are aligned, highlighted and easier to catch with the eye when using play assert).
Would you mind writing a fix for that test?

s-t-e-v-e-n-k added a commit to s-t-e-v-e-n-k/python-for-android that referenced this issue Apr 9, 2024
Python 3.12 now does not blindly any methods on mocked objects, which
masks issues. Correct the assertion of has_calls to assert_has_calls in
the ICU recipe.

Fixes kivy#3002
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants