wrapt icon indicating copy to clipboard operation
wrapt copied to clipboard

Static/class method __func__ attribute bypasses wrapper.

Open ashb opened this issue 3 years ago • 3 comments

Edit: see https://github.com/GrahamDumpleton/wrapt/issues/172#issuecomment-822305509 for non-pytest version.

Example code:

import wrapt

@wrapt.decorator
def pass_through(wrapped, instance, args, kwargs):
    return wrapped(*args, foo=1, **kwargs)

class TestMyClass:
    @pass_through
    @classmethod
    def setup_class(cls, **kwargs):
        cls.kwargs = kwargs

    def test_something(self):
        assert self.kwargs == {'foo': 1}

Run this with pytest and it will fail, (remove the @classmethod decorator and it passes) as the decorator is never actually called!

I think the reason for this in this code block: https://github.com/pytest-dev/pytest/blob/32ad70dea8fe2978e5f6025070278cc3796f4e47/src/_pytest/python.py#L804-L831

specifically the func = getimfunc(setup_class) call, which does this:

def getimfunc(func):
    try:
        return func.__func__
    except AttributeError:
        return func

The "issue" here is that the __func__ attribute on the wrapped classmethod is passed through, meaning that the unwrapped function is called by pytest.

This seems like a bug to me :)

An "easy" fix would be to not proxy func for classmethods, but I don't know if that would have other adverse impact.

ashb avatar Mar 12 '21 12:03 ashb

Will need to do more research, but this looks to be a flaw in pytest implementation.

Specifically, it doesn't support properly that when dealing with any sort of function in a class context, that it should be doing it in a way that ensures that it triggers the descriptor protocol. It should not be simply accessing __func__ as that skips the descriptor protocol meaning that any wrapper implementation for a function which relies on the descriptor protocol will fail.

This probably should be raised as a problem against pytest.

Details on descriptor protocol if you aren't entirely familiar with it:

  • https://docs.python.org/3/howto/descriptor.html#descriptor-protocol

GrahamDumpleton avatar Mar 13 '21 21:03 GrahamDumpleton

What version of Python and pytest are you using?

What is the exact error result you get from pytest and what were you expecting?

When I test with latest pytest and both Python 3.7 and Python 3.9, I get the exact same result whether or not the wrapt decorator is in place.

$ pytest test_xxx.py -v
============================================================== test session starts ==============================================================
platform darwin -- Python 3.7.5, pytest-6.2.2, py-1.10.0, pluggy-0.13.1 -- /private/tmp/wrapt-issue/bin/python
cachedir: .pytest_cache
rootdir: /private/tmp/wrapt-issue/src
collected 1 item

test_xxx.py::TestMyClass::test_something FAILED                                                                                           [100%]

=================================================================== FAILURES ====================================================================
__________________________________________________________ TestMyClass.test_something ___________________________________________________________

self = <test_xxx.TestMyClass object at 0x7f80802940d0>

    def test_something(self):
>       assert self.kwargs == {'foo': 1}
E       AssertionError: assert {} == {'foo': 1}
E         Right contains 1 more item:
E         {'foo': 1}
E         Full diff:
E         - {'foo': 1}
E         + {}

test_xxx.py:14: AssertionError
============================================================ short test summary info ============================================================
FAILED test_xxx.py::TestMyClass::test_something - AssertionError: assert {} == {'foo': 1}
=============================================================== 1 failed in 0.03s ===============================================================

I don't know about this setup feature of pytest, but the error seems to be correct for how the test is written.

GrahamDumpleton avatar Mar 15 '21 23:03 GrahamDumpleton

Hey, sorry notification bankruptcy, didn't see your response.

Sorry for not making it clear -- they "error" is that the wrapped function is called with an empty kwargs, meaning that the deocrator is never called. Here is a (maybe) clearer example:

import functools
import wrapt

@wrapt.decorator
def pass_through(wrapped, instance, args, kwargs):
    raise RuntimeError("We _WANT_ to see this error")
    return wrapped(*args, foo=1, **kwargs)

class TestMyClass:
    @pass_through
    @classmethod
    def setup_class(cls, **kwargs):
        cls.kwargs = kwargs

    def test_something(self):
        assert self.kwargs == {'foo': 1}

I'm willing to believe this is a bug in pytest, but also: proxying __func__ seems slightly odd -- that is one that should be special cased to return the wrapped function.

The __func__ attribute of classmethods doesn't seem to be documented that I could see, but if you want to call a classmethod in the class definition (i.e. before the class method is bound) then this is the only way of doing it.

This maybe makes more sense with a static method (both class and static have the __func__ attribute)

import wrapt

@wrapt.decorator
def pass_through(wrapped, instance, args, kwargs):
    return wrapped(*args, foo=1, **kwargs)

class TestMyClass:
    @staticmethod
    def some_static(**kwargs):
        assert kwargs == {'foo': 0}

    @pass_through
    @staticmethod
    def wrapped_static(**kwargs):
        assert kwargs == {'foo': 1}

    # Using __func__ is the only way to call a class/static method whilst still in the class-body, i.e. before they are "bound".
    some_static.__func__()
    wrapped_static.__func__()

This simple (although slightly contrived) example fails -- the pass_through decorator is never called, because __func__ is set to point do the wrapped function directly, instead of the the wrapper.

This means that wrapt' functions don't behave like "normal" static/class methods when you try to execute them like this.

An edge case to be sure, but having wrapped_static.__func__ is wrapped_static.__wrapped__.__func__ seems unexpected.

Thoughts?

ashb avatar Apr 19 '21 09:04 ashb