classification
Title: mock.attach_mock should work with any return value of patch()
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.9, Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder: mock calls don't propagate to parent (autospec)
View: 21478
Assigned To: Nosy List: anfedorov, cjw296, mariocj89, michael.foord, shmed, xtreak
Priority: normal Keywords:

Created on 2016-10-31 17:32 by anfedorov, last changed 2019-09-09 15:40 by xtreak. This issue is now closed.

Messages (13)
msg279812 - (view) Author: Andrey Fedorov (anfedorov) Date: 2016-10-31 17:32
The attach_mock in the following code sample fails silently:

    >>> from mock import patch, Mock
    >>> p = patch('requests.get', autospec=True)
    >>> manager = Mock()
    >>> manager.attach_mock(p.start(), 'requests_get')
    >>> import requests
    >>> requests.get('https://google.com')
    <MagicMock name='get()' id='4472381392'>
    >>> manager.mock_calls
    []
    >>> p.stop()
    >>> manager.mock_calls
    []

It seems this would be a useful use-case, especially given that this code would work as-expected if 'requests.get' in the second line were replaced with a path to a class.
msg280198 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2016-11-07 12:16
attach_mock should use function.mock when it is passed a function created by autospec. It should also *probably* fail if given a non-mock object (although that would prevent people duck-typing and attaching a mock-like object so I'm open to discussion on that).
msg280199 - (view) Author: Syed Suhail Ahmed (shmed) Date: 2016-11-07 13:29
I would like to work on this issue.
msg280319 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2016-11-08 17:04
Sure, go ahead Syed. Feel free to ask any questions you may have.
msg280599 - (view) Author: Syed Suhail Ahmed (shmed) Date: 2016-11-11 17:08
So from what I have understood, manager.attach_mock must raise an Exception when it is called with a wrong attribute, since the patch is called with autospec=True and you cannot call a mock with non existing attributes.Is that correct?
msg280618 - (view) Author: Andrey Fedorov (anfedorov) Date: 2016-11-11 22:16
There's some vagueness on how this is implemented, but I would prefer
manager.attach_mock to also work with whatever the return value of patch()
is.

On Fri, Nov 11, 2016 at 12:08 PM, Syed Suhail Ahmed <report@bugs.python.org>
wrote:

>
> Syed Suhail Ahmed added the comment:
>
> So from what I have understood, manager.attach_mock must raise an
> Exception when it is called with a wrong attribute, since the patch is
> called with autospec=True and you cannot call a mock with non existing
> attributes.Is that correct?
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue28569>
> _______________________________________
>
msg280628 - (view) Author: Andrey Fedorov (anfedorov) Date: 2016-11-12 03:22
To clarify, this is how I would expect these two functions to work together
"out of the box"

patches = { 'requests_get': 'requests.get', ... }

root_mock = mock.Mock()
for name, path in patches.items():
    m = mock.patch(path, auto_spec=True)
    root_mock.attach_mock(m, name)

This works when `path` is referring to a class, and it would be great if it
also worked with functions like `requests.get`, as well.

On Fri, Nov 11, 2016 at 5:16 PM, Andrey Fedorov <me@anfedorov.com> wrote:

> There's some vagueness on how this is implemented, but I would prefer
> manager.attach_mock to also work with whatever the return value of patch()
> is.
>
> On Fri, Nov 11, 2016 at 12:08 PM, Syed Suhail Ahmed <
> report@bugs.python.org> wrote:
>
>>
>> Syed Suhail Ahmed added the comment:
>>
>> So from what I have understood, manager.attach_mock must raise an
>> Exception when it is called with a wrong attribute, since the patch is
>> called with autospec=True and you cannot call a mock with non existing
>> attributes.Is that correct?
>>
>> ----------
>>
>> _______________________________________
>> Python tracker <report@bugs.python.org>
>> <http://bugs.python.org/issue28569>
>> _______________________________________
>>
>
>
msg280755 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2016-11-14 11:42
It should be perfectly valid to use attach_mock with an attribute that doesn't already exist. Part of it's purpose is to create new attributes.
msg280756 - (view) Author: Syed Suhail Ahmed (shmed) Date: 2016-11-14 11:45
But since autospec is set as True, then shouldnt attach_mock throw an
exception when called with an attribute that doesn't exist?

On Mon, Nov 14, 2016 at 5:12 PM, Michael Foord <report@bugs.python.org>
wrote:

>
> Michael Foord added the comment:
>
> It should be perfectly valid to use attach_mock with an attribute that
> doesn't already exist. Part of it's purpose is to create new attributes.
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue28569>
> _______________________________________
>
msg280757 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2016-11-14 11:47
Oh, I see what you mean - an attribute that doesn't exist on the original. With autospec that should throw an exception (AttributeError) I think, yes.
msg280795 - (view) Author: Andrey Fedorov (anfedorov) Date: 2016-11-14 17:04
Here's the full extension of the example from documentation that doesn't
seem to handle classes and functions consistently:

  import mock

  patches = {
      'requests_get': 'requests.get',
      'mymodule_Class1': 'mymodule.Class1'
  }

  root_mock = mock.Mock()
  for name, path in patches.items():
      m = mock.patch(path, autospec=True)
      root_mock.attach_mock(m.start(), name)

  import requests
  import mymodule

  mymodule.Class1('foo')
  requests.get('bar')

  print root_mock.mock_calls
  # [call.mymodule_Class1('foo')]

Does this working as expected make sense, or is there some reason this is
an undesirable API to behave consistently regardless of what's being
patched?
msg348056 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python committer) Date: 2019-07-17 11:50
Is this similar to issue21478? Michael, can you please confirm this behavior and the change proposed in issue21478?

I tried out the below program as per initial report

# foo.py

from unittest.mock import patch, Mock
p = patch('requests.get', autospec=True)
manager = Mock()
manager.attach_mock(p.start(), 'requests_get')

import requests
requests.get('https://google.com')
print(manager.mock_calls)
p.stop()
print(manager.mock_calls)

# Without patch

$ python.exe foo.py
[]
[]

# With PR 14688 in issue21478

$ python.exe foo.py
[call.requests_get('https://google.com')]
[call.requests_get('https://google.com')]
msg351503 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python committer) Date: 2019-09-09 15:40
I am closing this in favor of issue21478 as the original report is now fixed. Thanks.
History
Date User Action Args
2019-09-09 15:40:25xtreaksetstatus: open -> closed
superseder: mock calls don't propagate to parent (autospec)
messages: + msg351503

resolution: fixed
stage: needs patch -> resolved
2019-07-17 11:50:46xtreaksetnosy: + cjw296, mariocj89, xtreak

messages: + msg348056
versions: + Python 3.8, Python 3.9
2016-11-14 17:04:07anfedorovsetmessages: + msg280795
2016-11-14 11:47:19michael.foordsetmessages: + msg280757
2016-11-14 11:45:00shmedsetmessages: + msg280756
2016-11-14 11:42:31michael.foordsetmessages: + msg280755
2016-11-12 03:22:53anfedorovsetmessages: + msg280628
2016-11-11 22:16:47anfedorovsetmessages: + msg280618
2016-11-11 17:08:08shmedsetmessages: + msg280599
2016-11-08 17:04:49michael.foordsetmessages: + msg280319
2016-11-07 13:29:34shmedsetnosy: + shmed
messages: + msg280199
2016-11-07 12:16:00michael.foordsetstage: needs patch
messages: + msg280198
versions: - Python 2.7, Python 3.3, Python 3.4, Python 3.5, Python 3.6
2016-10-31 17:32:38anfedorovsettitle: mock.attach_mock should work with return value of patch() -> mock.attach_mock should work with any return value of patch()
2016-10-31 17:32:14anfedorovcreate