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

Test failures with pypy3.10 7.3.13 #118

Closed
mgorny opened this issue Nov 16, 2023 · 10 comments · Fixed by #119
Closed

Test failures with pypy3.10 7.3.13 #118

mgorny opened this issue Nov 16, 2023 · 10 comments · Fixed by #119

Comments

@mgorny
Copy link
Contributor

mgorny commented Nov 16, 2023

When running the test suite using PyPy3.10, I'm getting two test failures:

$ pypy3 -m pytest
========================================================= test session starts =========================================================
platform linux -- Python 3.10.13[pypy-7.3.13-final], pytest-7.4.3, pluggy-1.3.0
rootdir: /tmp/overrides
plugins: anyio-4.0.0, xprocess-0.23.0, asyncio-0.21.1, xdist-3.4.0
asyncio: mode=strict
collected 67 items                                                                                                                    

tests/test_decorator_params.py .                                                                                                [  1%]
tests/test_enforce__py38.py ...........................                                                                         [ 41%]
tests/test_final.py ........                                                                                                    [ 53%]
tests/test_issubtype_with_typeddict__py38.py .                                                                                  [ 55%]
tests/test_named_and_positional__py38.py ...........                                                                            [ 71%]
tests/test_new_union__py3_10.py ..                                                                                              [ 74%]
tests/test_overrides.py ...FF......                                                                                             [ 91%]
tests/test_signatures.py ...                                                                                                    [ 95%]
tests/test_special_cases.py ...                                                                                                 [100%]

============================================================== FAILURES ===============================================================
___________________________________ OverridesTests.test_overrides_builtin_method_correct_signature ____________________________________

self = <test_overrides.OverridesTests testMethod=test_overrides_builtin_method_correct_signature>

    def test_overrides_builtin_method_correct_signature(self):
>       class SubclassOfInt(int):

tests/test_overrides.py:156: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tests/test_overrides.py:158: in SubclassOfInt
    def bit_length(self):
overrides/overrides.py:143: in override
    return _overrides(method, check_signature, check_at_runtime)
overrides/overrides.py:170: in _overrides
    _validate_method(method, super_class, check_signature)
overrides/overrides.py:189: in _validate_method
    ensure_signature_is_compatible(super_method, method, is_static)
overrides/signature.py:99: in ensure_signature_is_compatible
    same_main_module = _is_same_module(sub_callable, super_callable)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

callable1 = <function OverridesTests.test_overrides_builtin_method_correct_signature.<locals>.SubclassOfInt.bit_length at 0x00007fc1b9dffb00>
callable2 = <function int.bit_length at 0x000055c6c541e180>

    def _is_same_module(callable1: _WrappedMethod, callable2: _WrappedMethod2) -> bool:
        mod1 = callable1.__module__.split(".")[0]
        try:
            mod2 = callable2.__module__
        except AttributeError:
            return False
>       mod2 = mod2.split(".")[0]
E       AttributeError: 'NoneType' object has no attribute 'split'

overrides/signature.py:61: AttributeError
__________________________________ OverridesTests.test_overrides_builtin_method_incorrect_signature ___________________________________

self = <test_overrides.OverridesTests testMethod=test_overrides_builtin_method_incorrect_signature>

    def test_overrides_builtin_method_incorrect_signature(self):
        if sys.version_info >= (3, 7):
            expected_error = self.assertRaises(TypeError)
        else:
            # In 3.6 inspecting signature's isn't possible for builtin
            # methods so this this passes the signature check.
            expected_error = no_error()
    
        with expected_error:
    
>           class SubclassOfInt(int):

tests/test_overrides.py:174: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tests/test_overrides.py:176: in SubclassOfInt
    def bit_length(self, _):
overrides/overrides.py:143: in override
    return _overrides(method, check_signature, check_at_runtime)
overrides/overrides.py:170: in _overrides
    _validate_method(method, super_class, check_signature)
overrides/overrides.py:189: in _validate_method
    ensure_signature_is_compatible(super_method, method, is_static)
overrides/signature.py:99: in ensure_signature_is_compatible
    same_main_module = _is_same_module(sub_callable, super_callable)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

    def _is_same_module(callable1: _WrappedMethod, callable2: _WrappedMethod2) -> bool:
        mod1 = callable1.__module__.split(".")[0]
        try:
            mod2 = callable2.__module__
        except AttributeError:
            return False
>       mod2 = mod2.split(".")[0]
E       AttributeError: 'NoneType' object has no attribute 'split'

overrides/signature.py:61: AttributeError
======================================================= short test summary info =======================================================
FAILED tests/test_overrides.py::OverridesTests::test_overrides_builtin_method_correct_signature - AttributeError: 'NoneType' object has no attribute 'split'
FAILED tests/test_overrides.py::OverridesTests::test_overrides_builtin_method_incorrect_signature - AttributeError: 'NoneType' object has no attribute 'split'
==================================================== 2 failed, 65 passed in 0.65s =====================================================

CC @mattip

@mattip
Copy link

mattip commented Nov 16, 2023

It looks like the failure is around properly adding the __module__ attribute to callable2. How is that created?

@mattip
Copy link

mattip commented Nov 16, 2023

@cfbolz
Copy link

cfbolz commented Nov 16, 2023

It seems to be int.bit_length.__module__, which does not exist in cpython, but is None in pypy. I suppose it's the usual problem with method_descriptor on CPython, which would mean this affects the methods of all builtin types. Method descriptors don't exist in pypy, instead we have regular function objects, but with __module__ set to None.

@cfbolz
Copy link

cfbolz commented Nov 16, 2023

right, overrides would produce a similar exception on CPython if you make a slightly weird method where you del the __module__ attribute, like this:

class ASuper:
    def some_method(self):
        return 1
    del some_method.__module__
class ASub(ASuper):
    @override
    def some_method(self):
        return 2

yields:

tests/test_overrides.py:97: in <module>
    class ASub(ASuper):
tests/test_overrides.py:98: in ASub
    @override
overrides/overrides.py:143: in override
    return _overrides(method, check_signature, check_at_runtime)
overrides/overrides.py:170: in _overrides
    _validate_method(method, super_class, check_signature)
overrides/overrides.py:189: in _validate_method
    ensure_signature_is_compatible(super_method, method, is_static)
overrides/signature.py:99: in ensure_signature_is_compatible
    same_main_module = _is_same_module(sub_callable, super_callable)
overrides/signature.py:61: in _is_same_module
    mod2 = mod2.split(".")[0]
E   AttributeError: 'NoneType' object has no attribute 'split'

@mattip
Copy link

mattip commented Nov 16, 2023

Method descriptors don't exist in pypy, instead we have regular function objects, but with __module__ set to None

Could we remove the __module__ attribute from methods on builtins?

@mattip
Copy link

mattip commented Nov 16, 2023

So I guess the fix here is to check for None as well as AttributeError

@cfbolz
Copy link

cfbolz commented Nov 17, 2023

yeah, pypy could try to make sure that the methods of builtins raise an error when reading __module__ instead of returning None. I can give that a try

@cfbolz
Copy link

cfbolz commented Nov 17, 2023

that said, the __module__ attribute of builtin functions (eg abs) is "builtin" in both pypy and cpython, maybe we should simply adopt that for builtin methods too? Anyone have an opinion on either of these options?

@mgorny
Copy link
Contributor Author

mgorny commented Nov 17, 2023

I don't know whether it's worth changing this in PyPy — either way, I'll try making a PR to support both missing and None attribute, if only to support prior PyPy versions.

mgorny added a commit to mgorny/overrides that referenced this issue Nov 17, 2023
Fix `_is_same_module()` to account for `__module__` attribute possibly
being `None`, as it currently is in PyPy, rather than non-present,
as it is in CPython.

Fixes mkorpela#118
mkorpela pushed a commit that referenced this issue Jan 20, 2024
Fix `_is_same_module()` to account for `__module__` attribute possibly
being `None`, as it currently is in PyPy, rather than non-present,
as it is in CPython.

Fixes #118
@mkorpela
Copy link
Owner

Thank you all for informing, debugging and solving the issue!

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

Successfully merging a pull request may close this issue.

4 participants