Skip to content

bpo-32092: Mock patch fix autospec #4476

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

claudiubelu
Copy link

@claudiubelu claudiubelu commented Nov 20, 2017

bpo-32092: Fixes mock.patch autospec self / cls argument consumption issue

https://bugs.python.org/issue32092

@claudiubelu claudiubelu force-pushed the mock-patch-fix-autospec branch 2 times, most recently from d59fe18 to 797f190 Compare November 20, 2017 16:12
@AraHaan
Copy link
Contributor

AraHaan commented Nov 20, 2017

@BCLAU mind editing the PR title to add bpo-32092: on the beginning to fix the CI-bot?

Also is bpo-32092 the replacement for bpo-30587?

@claudiubelu claudiubelu changed the title Mock patch fix autospec bpo-32092: Mock patch fix autospec Nov 20, 2017
@claudiubelu
Copy link
Author

claudiubelu commented Nov 20, 2017

Hi @AraHaan , sure, will do.

And I agree. Yeah, I would have sent the fixes separately, but this depends on the other one, which I've sent separately: #1982

Once that one merges, I can simply rebase this one. Or, I could simply separate them, but they'll have a merge conflict.

@AraHaan
Copy link
Contributor

AraHaan commented Nov 20, 2017

Or, just merge the 2 and close out the older one?

@claudiubelu
Copy link
Author

Hm, tbh, I'd like to keep it simple, since they're both addressing different issues.

Anyways, I guess I'll just separate this patch from the other. We'll deal with the merge conflict when it happens.

@claudiubelu
Copy link
Author

Actually, nevermind, the unit tests are not passing without the 1st patch. :)

@AraHaan
Copy link
Contributor

AraHaan commented Nov 20, 2017

alright then.

Mock can accept an spec object / class as argument, making sure
that accessing attributes that do not exist in the spec will cause an
AttributeError to be raised, but there is no guarantee that the spec's
methods signatures are respected in any way. This creates the possibility
to have faulty code with passing unittests and assertions.

Example:

from unittest import mock

class Something(object):
    def foo(self, a, b, c, d):
        pass

m = mock.Mock(spec=Something)
m.foo()

Adds the autospec argument to Mock, and its mock_add_spec method.

Passes the spec's attribute with the same name to the child mock (spec-ing
the child), if the mock's autospec is True.

Sets _mock_check_sig if the given spec is callable.

Adds unit tests to validate the fact that the autospec method signatures are
respected.
Currently, when patching methods with autospec, their self / cls
arguments are not consumed, causing call asserts to fail (they
expect an instance / class reference as the first argument).

Example:

from unittest import mock

class Something(object):
    def foo(self, a, b, c, d):
        pass

patcher = mock.patch.object(Something, 'foo', autospec=True)
patcher.start()

s = Something()
s.foo()
@csabella
Copy link
Contributor

@claudiubelu, please resolve the merge conflicts. Thank you!

@cjw296
Copy link
Contributor

cjw296 commented Jan 24, 2020

@claudiubelu - it looks like #1982 is with @voidspace and this PR depends on it? Would it be okay to close that one until #1982 is resolved?

@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Aug 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review stale Stale PR or inactive for long period of time.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants