unittest.mock patch autospec doesn't work on staticmethods

[kevinbenton]

BPO	23078
Nosy	@cjw296, @voidspace, @PCManticore, @ambv, @berkerpeksag, @felipeochoa, @mariocj89, @tirkarthi, @tom-dalton-fanduel, @parejkoj, @atenni
PRs	bpo-23078: Add support for staticmethod and classmethod to mock created with autospec #11613 [3.7] bpo-23078: Add support for {class,static}method to mock.create_autospec() (GH-11613) #12901
Files	issue23078.patch issue23078.patch issue23078.patch: Update mock._callable to properly detect classmethod/staticmethod callables

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/berkerpeksag'
closed_at = <Date 2019-04-22.03:08:29.819>
created_at = <Date 2014-12-18.08:45:22.981>
labels = ['3.7', '3.8', 'type-bug', 'library']
title = "unittest.mock patch autospec doesn't work on staticmethods"
updated_at = <Date 2019-04-22.03:08:29.818>
user = 'https://bugs.python.org/kevinbenton'

bugs.python.org fields:

activity = <Date 2019-04-22.03:08:29.818>
actor = 'berker.peksag'
assignee = 'berker.peksag'
closed = True
closed_date = <Date 2019-04-22.03:08:29.819>
closer = 'berker.peksag'
components = ['Library (Lib)']
creation = <Date 2014-12-18.08:45:22.981>
creator = 'kevinbenton'
dependencies = []
files = ['37818', '37823', '40470']
hgrepos = []
issue_num = 23078
keywords = ['patch']
message_count = 19.0
messages = ['232864', '234486', '234501', '234502', '244677', '250716', '305823', '310728', '322849', '330433', '330434', '330569', '330570', '333734', '333836', '333892', '340633', '340635', '340636']
nosy_count = 18.0
nosy_names = ['kormat', 'cjw296', 'michael.foord', 'Claudiu.Popa', 'lukasz.langa', 'berker.peksag', 'epu', 'galuszkak', 'fov', 'kevinbenton', 'mariocj89', 'germanop', 'deanliao', 'xtreak', 'tom.dalton.fanduel', 'John Parejko2', 'atenni', 'parejkoj-3']
pr_nums = ['11613', '12901']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue23078'
versions = ['Python 3.7', 'Python 3.8']

[kevinbenton]

If one of the mock.patch methods is used with autospec=True on a staticmethod in an object, the mock library determines that it is not callable by checking for the __call__ attribute. This results in a NonCallableMagicMock being returned which of course dies with the following error when the mocked method is called:

TypeError: 'NonCallableMagicMock' object is not callable

It seems that the create_autospec needs to special case for classmethod and staticmethod.

The following change seems to fix it, however I am only vaguely familiar with the internals of mock so I'm not sure what this breaks.

diff -r d356250e275d mock.py
--- a/mock.py	Tue Apr 09 14:53:33 2013 +0100
+++ b/mock.py	Wed Dec 17 07:35:15 2014 -0800
@@ -2191,7 +2191,8 @@
         # descriptors don't have a spec
         # because we don't know what type they return
         _kwargs = {}
-    elif not _callable(spec):
+    elif not _callable(spec) and not isinstance(spec, (staticmethod,
+                                                       classmethod)):
         Klass = NonCallableMagicMock
     elif is_type and instance and not _instance_callable(spec):
         Klass = NonCallableMagicMock

[PCManticore]

Here's a patch which does this. One problem could be that staticmethod(some_callable) or classmethod(some_callable) aren't callable per se, but given the fact that users expects Foo.staticmethod or Foo.klassmethod to be callable when patching them, it's something we should consider for fixing.

[berkerpeksag]

Looks like b6ea3dc89a78 is not a valid changeset. Could you attach a patch from the Mercurial repo?

[PCManticore]

Ups, sorry about that, I'll update the patch.

[felipeochoa]

Regarding Claudiu's comment about staticmethod(x) or classmethod(x) not being callable, would it suffice to add a specific check of the form (isinstance(x, (classmethod, staticmethod)) and _callable(x.__func__))?

Separately, would it be better to include the check for staticmethod and classmethod objects (with an underlying callable) inside the _callable function? Not sure if this would break anything, but it seems like conceptually the issue is with the definition of a callable object, not the selection of mock type to use.

[felipeochoa]

The attached patch implements these changes through _callable instead. This patch also ensures that the underlying object that staticmethod and classmethod wrap is a callable object as well.

[germanop]

Hi,

I hit this problem wile mocking one static method and found this fix.
Tested it and works for me.
However, I did not find it pushed anywhere: no Python 3.x and no mock for 2.7.

Is there any reason why it is not pushed anywhere, yet?

[deanliao]

I planned to upgrade Chromium OS's mock module to 2.0.0. I also encountered the issue that classmethod cannot be patched as callable mock.

Reviewers, can we start the review process?

[galuszkak]

This affects all versions from 3.4 up to 3.8-dev. Would be nice if someone could do the review of the supplied patch.

Thanks for awesome work on Python!

I'm here because it just hit me also and I was for 1 h thinking that I don't know how to use patch/mock. ;)

[tom-dalton-fanduel]

I've just come across this too, so would be great if the patch can be progressed.

[tom-dalton-fanduel]

Here's a minimal example so my comment is not totally vacuous:

import unittest
from unittest import mock

class Foo:
    @classmethod
    def bar(cls, baz):
        pass

class TestFoo(unittest.TestCase):
    def test_bar(self):
        with unittest.mock.patch.object(Foo, "bar", autospec=True):
            Foo.bar()

->

~/› python -m unittest example.py 
E
======================================================================
ERROR: test_bar (example.TestFoo)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "example.py", line 14, in test_bar
    Foo.bar()
TypeError: 'NonCallableMagicMock' object is not callable

----------------------------------------------------------------------
Ran 1 test in 0.001s

[parejkoj]

Adding to the list of "I just ran into this". The patch submitted by fov seems straightforward enough: what can we do to help shepherd it along?

[berkerpeksag]

Thanks for the pings. I will work on this issue this weekend. Note that 3.4 and 3.5 are in security-fix-only mode now, so I removed them from the versions field.

[parejkoj-3]

Were you able to make any progress on this? Do you need any help?

[tirkarthi]

@berker.peksag I have converted the patch at https://bugs.python.org/file40470/issue23078.patch and pushed it to a GitHub branch master...tirkarthi:bpo-23078 . I am willing to open a PR attributing to @fov in case you haven't had the time to look into this.

I am removing 3.6 since it's security fixes only mode.

Thanks

[felipeochoa]

Please go ahead with the PR. I can't push this one through, but would be
great to have this finally land!

On Thu, 17 Jan 2019 at 03:42, Karthikeyan Singaravelan <
report@bugs.python.org> wrote:

Karthikeyan Singaravelan <tir.karthi@gmail.com> added the comment:

@berker.peksag I have converted the patch at
https://bugs.python.org/file40470/issue23078.patch and pushed it to a
GitHub branch
master...tirkarthi:bpo-23078 . I
am willing to open a PR attributing to @fov in case you haven't had the
time to look into this.

I am removing 3.6 since it's security fixes only mode.

Thanks

----------
nosy: +cjw296, mariocj89, xtreak
versions: -Python 3.6

---

Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue23078\>

---

[berkerpeksag]

New changeset 9b21856 by Berker Peksag (Xtreak) in branch 'master':
bpo-23078: Add support for {class,static}method to mock.create_autospec() (GH-11613)
9b21856

[berkerpeksag]

New changeset 15a57a3 by Berker Peksag in branch '3.7':
bpo-23078: Add support for {class,static}method to mock.create_autospec() (GH-11613)
15a57a3

[berkerpeksag]

Thank you!