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

spy: handle AttributeError with __getattribute__ #42

Merged
merged 1 commit into from
May 17, 2016

Conversation

blueyed
Copy link
Contributor

@blueyed blueyed commented May 17, 2016

I have noticed that spy did not work on functions that are not defined
in the class itself, but a parent class. This patch fixes this.

@blueyed blueyed force-pushed the fix-spy-for-derived-methods branch from 7bf8e07 to 590defc Compare May 17, 2016 13:13
def test_class_method_subclass_spy(mocker):
class Base(object):

@classmethod
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this supposed to be a classmethod?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, it is now - I was pushing a fixup/amended commit.

@nicoddemus
Copy link
Member

Thanks a lot @blueyed!

Could you please also write a new CHANGELOG entry?

The timing is great, I was planning on publishing a new release today! 😄

@blueyed blueyed force-pushed the fix-spy-for-derived-methods branch from 590defc to b46075d Compare May 17, 2016 13:26
@blueyed
Copy link
Contributor Author

blueyed commented May 17, 2016

It does not work with Python 2 apparently:

    @skip_pypy
    def test_static_method_subclass_spy(mocker):
        class Base(object):

            @staticmethod
            def bar(arg):
                return arg * 2

        class Foo(Base):
            pass

        spy = mocker.spy(Foo, 'bar')
>       assert Foo.bar(arg=10) == 20
E       TypeError: unbound method bar() must be called with Foo instance as first argument (got nothing instead)

(same with classmethod).

Should the tests be skipped there?

@nicoddemus
Copy link
Member

Hmmm that's a shame.

Please xfail them in Python 2 instead (@pytest.mark.xfail(sys.python_version[0] == 2, reason='does not work on Python 2') should do the trick. We can try to make it work later, but the PR as it stands is already an improvement.

I have noticed that `spy` did not work on functions that are not defined
in the class itself, but a parent class.  This patch fixes this.
@blueyed blueyed force-pushed the fix-spy-for-derived-methods branch from b46075d to 645680a Compare May 17, 2016 13:45
@blueyed
Copy link
Contributor Author

blueyed commented May 17, 2016

Done.

@nicoddemus nicoddemus merged commit 4ddc1c9 into pytest-dev:master May 17, 2016
@nicoddemus
Copy link
Member

Thanks again! 😄

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 this pull request may close these issues.

2 participants