PyPy3.10 test failures due to datetime mocking: `TypeError: isinstance() arg 2 must be a class, type, or tuple of classes and types, got MagicMock` #2708

mgorny · 2024-09-27T13:57:02Z

Description of issue or feature request:

The test suite currently fails 3 tests on PyPy3.10 7.3.17. This is due to a known limitation of pure Python datetime module from CPython. However, CPython normally covers it with a C extension, hiding the problem, but PyPy suffers from it.

Current behavior:

$ tox -e pypy310
pypy310: commands[0]> python3 --version
Python 3.10.14 (39dc8d3c85a7, Aug 29 2024, 13:06:01)
[PyPy 7.3.17 with GCC 13.3.1 20240614]
pypy310: commands[1]> python3 -m coverage run aggregate_tests.py
.................................................................................................................................................EEE.....................................
======================================================================
ERROR: test_expired_metadata (test_updater_top_level_update.TestRefresh)
Verifies that expired local timestamp/snapshot can be used for
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/pypy3.10/unittest/mock.py", line 1379, in patched
    return func(*newargs, **newkeywargs)
  File "/tmp/python-tuf/tests/test_updater_top_level_update.py", line 752, in test_expired_metadata
    now = datetime.datetime.now(timezone.utc)
  File "/usr/lib/pypy3.10/unittest/mock.py", line 1114, in __call__
    return self._mock_call(*args, **kwargs)
  File "/usr/lib/pypy3.10/unittest/mock.py", line 1118, in _mock_call
    return self._execute_mock_call(*args, **kwargs)
  File "/usr/lib/pypy3.10/unittest/mock.py", line 1188, in _execute_mock_call
    return self._mock_wraps(*args, **kwargs)
  File "/usr/lib/pypy3.10/datetime.py", line 1720, in now
    return cls.fromtimestamp(t, tz)
  File "/usr/lib/pypy3.10/datetime.py", line 1709, in fromtimestamp
    return cls._fromtimestamp(t, tz is not None, tz)
  File "/usr/lib/pypy3.10/datetime.py", line 1698, in _fromtimestamp
    result = tz.fromutc(result)
  File "/usr/lib/pypy3.10/datetime.py", line 2362, in fromutc
    if isinstance(dt, datetime):
TypeError: isinstance() arg 2 must be a class, type, or tuple of classes and types, got MagicMock

======================================================================
ERROR: test_expired_timestamp_snapshot_rollback (test_updater_top_level_update.TestRefresh)
Verifies that rollback protection is done even if local timestamp has expired.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/pypy3.10/unittest/mock.py", line 1379, in patched
    return func(*newargs, **newkeywargs)
  File "/tmp/python-tuf/tests/test_updater_top_level_update.py", line 358, in test_expired_timestamp_snapshot_rollback
    now = datetime.datetime.now(timezone.utc)
  File "/usr/lib/pypy3.10/unittest/mock.py", line 1114, in __call__
    return self._mock_call(*args, **kwargs)
  File "/usr/lib/pypy3.10/unittest/mock.py", line 1118, in _mock_call
    return self._execute_mock_call(*args, **kwargs)
  File "/usr/lib/pypy3.10/unittest/mock.py", line 1188, in _execute_mock_call
    return self._mock_wraps(*args, **kwargs)
  File "/usr/lib/pypy3.10/datetime.py", line 1720, in now
    return cls.fromtimestamp(t, tz)
  File "/usr/lib/pypy3.10/datetime.py", line 1709, in fromtimestamp
    return cls._fromtimestamp(t, tz is not None, tz)
  File "/usr/lib/pypy3.10/datetime.py", line 1698, in _fromtimestamp
    result = tz.fromutc(result)
  File "/usr/lib/pypy3.10/datetime.py", line 2362, in fromutc
    if isinstance(dt, datetime):
TypeError: isinstance() arg 2 must be a class, type, or tuple of classes and types, got MagicMock

======================================================================
ERROR: test_expired_timestamp_version_rollback (test_updater_top_level_update.TestRefresh)
Verifies that local timestamp is used in rollback checks even if it is expired.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/pypy3.10/unittest/mock.py", line 1379, in patched
    return func(*newargs, **newkeywargs)
  File "/tmp/python-tuf/tests/test_updater_top_level_update.py", line 322, in test_expired_timestamp_version_rollback
    now = datetime.datetime.now(timezone.utc)
  File "/usr/lib/pypy3.10/unittest/mock.py", line 1114, in __call__
    return self._mock_call(*args, **kwargs)
  File "/usr/lib/pypy3.10/unittest/mock.py", line 1118, in _mock_call
    return self._execute_mock_call(*args, **kwargs)
  File "/usr/lib/pypy3.10/unittest/mock.py", line 1188, in _execute_mock_call
    return self._mock_wraps(*args, **kwargs)
  File "/usr/lib/pypy3.10/datetime.py", line 1720, in now
    return cls.fromtimestamp(t, tz)
  File "/usr/lib/pypy3.10/datetime.py", line 1709, in fromtimestamp
    return cls._fromtimestamp(t, tz is not None, tz)
  File "/usr/lib/pypy3.10/datetime.py", line 1698, in _fromtimestamp
    result = tz.fromutc(result)
  File "/usr/lib/pypy3.10/datetime.py", line 2362, in fromutc
    if isinstance(dt, datetime):
TypeError: isinstance() arg 2 must be a class, type, or tuple of classes and types, got MagicMock

----------------------------------------------------------------------
Ran 185 tests in 6.709s

FAILED (errors=3)
pypy310: exit 1 (7.69 seconds) /tmp/python-tuf/tests> python3 -m coverage run aggregate_tests.py pid=490151
  pypy310: FAIL code 1 (7.78=setup[0.07]+cmd[0.02,7.69] seconds)
  evaluation failed :( (7.84 seconds)

Expected behavior:

Tests passing :-).

One possible solution would be to use a more complete time mocking solution, e.g. freezegun. I've been able to quickly confirm that it solves the problem, using a quick patch:

 from tests.repository_simulator import RepositorySimulator
 from tuf.api.exceptions import (
@@ -306,8 +308,7 @@ def test_new_timestamp_unsigned(self) -> None:
 
         self._assert_files_exist([Root.type])
 
-    @patch.object(datetime, "datetime", wraps=datetime.datetime)
-    def test_expired_timestamp_version_rollback(self, mock_time: Mock) -> None:
+    def test_expired_timestamp_version_rollback(self) -> None:
         """Verifies that local timestamp is used in rollback checks even if it is expired.
 
         The timestamp updates and rollback checks are performed
@@ -331,14 +332,12 @@ def test_expired_timestamp_version_rollback(self, mock_time: Mock) -> None:
 
         self.sim.timestamp.version = 1
 
-        mock_time.now.return_value = datetime.datetime.now(
-            timezone.utc
-        ) + datetime.timedelta(days=18)
-        patcher = patch("datetime.datetime", mock_time)
-        # Check that a rollback protection is performed even if
-        # local timestamp has expired
-        with patcher, self.assertRaises(BadVersionNumberError):
-            self._run_refresh()
+        with freezegun.freeze_time(datetime.datetime.now( timezone.utc) +
+                                   datetime.timedelta(days=18)):
+            # Check that a rollback protection is performed even if
+            # local timestamp has expired
+            with self.assertRaises(BadVersionNumberError):
+                self._run_refresh()
 
         self._assert_version_equals(Timestamp.type, 2)

If you agree with this approach, I can make a proper pull request.

The text was updated successfully, but these errors were encountered:

jku · 2024-09-30T13:23:13Z

Well that does seem annoying... I would really like to avoid new dependencies if possible. This looks like it would be manageable as just a test dependency, but still: unittest has an advantage of being part of the standard library.

Trying to come up with some alternatives:

just skip these tests when running on pypy -- this feels acceptable since the expiry tests would still run on other Pythons. They are also included in tuf-conformance tests that run on CI
do the test without mocking, like maybe run a separate exectuable under libfaketime? -- this sounds like a lot of complexity that is not worth it

Skipping seems preferable to me when compared to a new dependency.
What do you think of @unittest.skipIf(platform.python_implementation() == "PyPy", 'skipping time mocking tests on pypy') or something like it?

mgorny · 2024-09-30T13:36:29Z

Skipping them would mean that there would be an uncovered code path on PyPy, wouldn't it? Then that path would be susceptible to actually hitting another problem on PyPy. And libfaketime sounds like a much heavier dependency than freezegun.

Perhaps the mocking could be modified not to cause the problem. After all, the problem stems from using MagicMock. I'm going to experiment with a bit and get back to you.

Mock `datetime.datetime.now()` directly rather than the complete `datetime` class, in order to fix compatibility with the pure Python `datetime` stdlib implementation used on PyPy. The original code failed as it replaced the class with a `MagicMock` instance, while the new one just overrides the method with a lambda. Fixes theupdateframework#2708

mgorny · 2024-09-30T13:57:04Z

Sigh, what an annoying thing. PyPy works fine if we override datetime.datetime.now directly but CPython doesn't tolerate modifying it.

mgorny · 2024-09-30T14:08:58Z

I'm afraid I don't have a good way of doing this, short of having conditional mocking code that detects whether we're using pure Python or C implementation, and uses a different code path appropriately.

jku · 2024-09-30T16:20:37Z

Thanks for taking another look. I'll play with it this week a bit as well, we can decide then.

jku · 2024-10-09T11:26:58Z

Yeah, I can't think of a better way to solve this than your original suggestion. freezegun also looks like a sensible module.

I'd be happy with a patch like you described, sorry it took a while to get here.

I imagine adding freezegun to requirements/test.txt will suffice.

Use freezegun for time mocking instead of manually patching the datetime module, as it provides a more streamlined solution that works both on CPython and on PyPy. Unfortunately, due to differences between the C datetime extension used by CPython, and the pure Python version of datetime (used by PyPy, and as a fallback on CPython), there does not seem to be a trivial way to mock time that would work with both versions. Fixes theupdateframework#2708 Signed-off-by: Michał Górny <[email protected]>

mgorny · 2024-10-09T11:43:43Z

Filed #2716

Use freezegun for time mocking instead of manually patching the datetime module, as it provides a more streamlined solution that works both on CPython and on PyPy. Unfortunately, due to differences between the C datetime extension used by CPython, and the pure Python version of datetime (used by PyPy, and as a fallback on CPython), there does not seem to be a trivial way to mock time that would work with both versions. Fixes theupdateframework#2708 Signed-off-by: Michał Górny <[email protected]>

mgorny mentioned this issue Oct 9, 2024

tests: Use freezegun for time mocking to fix pypy3 compatibility #2716

Merged

jku closed this as completed in #2716 Oct 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PyPy3.10 test failures due to datetime mocking: `TypeError: isinstance() arg 2 must be a class, type, or tuple of classes and types, got MagicMock` #2708

PyPy3.10 test failures due to datetime mocking: `TypeError: isinstance() arg 2 must be a class, type, or tuple of classes and types, got MagicMock` #2708

mgorny commented Sep 27, 2024

jku commented Sep 30, 2024

mgorny commented Sep 30, 2024

mgorny commented Sep 30, 2024

mgorny commented Sep 30, 2024

jku commented Sep 30, 2024

jku commented Oct 9, 2024

mgorny commented Oct 9, 2024

PyPy3.10 test failures due to datetime mocking: TypeError: isinstance() arg 2 must be a class, type, or tuple of classes and types, got MagicMock #2708

PyPy3.10 test failures due to datetime mocking: TypeError: isinstance() arg 2 must be a class, type, or tuple of classes and types, got MagicMock #2708

Comments

mgorny commented Sep 27, 2024

jku commented Sep 30, 2024

mgorny commented Sep 30, 2024

mgorny commented Sep 30, 2024

mgorny commented Sep 30, 2024

jku commented Sep 30, 2024

jku commented Oct 9, 2024

mgorny commented Oct 9, 2024

PyPy3.10 test failures due to datetime mocking: `TypeError: isinstance() arg 2 must be a class, type, or tuple of classes and types, got MagicMock` #2708

PyPy3.10 test failures due to datetime mocking: `TypeError: isinstance() arg 2 must be a class, type, or tuple of classes and types, got MagicMock` #2708