classification
Title: patch.object doesn't restore function defaults
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: orsenthil Nosy List: chepner, ezio.melotti, mailto1587, michael.foord, orsenthil, python-dev, seanmccully, vstinner
Priority: normal Keywords: easy, patch

Created on 2014-08-05 04:14 by chepner, last changed 2016-01-09 07:45 by orsenthil. This issue is now closed.

Files
File name Uploaded Description Edit
issue22138.patch seanmccully, 2014-08-06 08:47 review
Messages (11)
msg224801 - (view) Author: Clint Hepner (chepner) Date: 2014-08-05 04:14
Following a patch, a function's __defaults__ attribute is reset to None.

    def foo(x=5):
        return x

    assert foo() == 5  # As expected
    with unittest.mock.patch.object(foo, '__defaults__', (10,)):
        assert foo() == 10  # As expected

    assert foo() == 5  # Fails
    assert foo.__defaults__ is None  # Succeeds
msg224804 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2014-08-05 04:37
The issue seems to affect special attributes that can't be deleted.
In Lib/unittest/mock.py:1329, patch() tried to delete the attribute, and then, if it doesn't exist, it restores the previous value.  However some special attributes exist even after they are deleted, but their initial value is lost:
>>> def foo(x:int=5, y:int=3): return x + y
... 
>>> foo.__defaults__
(5, 3)
>>> del foo.__defaults__
>>> foo.__defaults__
>>> foo.__annotations__
{'y': <class 'int'>, 'x': <class 'int'>}
>>> del foo.__annotations__
>>> foo.__annotations__
{}
>>> foo.__qualname__
'foo'
>>> del foo.__qualname__
TypeError: __qualname__ must be set to a string object
msg224916 - (view) Author: Sean McCully (seanmccully) * Date: 2014-08-06 08:47
Is special casing the special attrs a permament enough solution?
msg224917 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2014-08-06 09:18
Thanks for the patch, however I don't think this is a robust solution.
Other objects might have undeletable attributes too.

The current code can delete an attribute without restoring it so an easy solution would be removing the hasattr() check, but that seems to be there to deal with proxy objects, so doing that will probably break them (Lib/unittest/test/testmock/testpatch.py:821 seems to test proxy object).
msg224918 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2014-08-06 09:18
It might have to be. There's no general purpose solution that will fit every possible behaviour for a Python descriptor I'm afraid.
msg224929 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2014-08-06 12:34
And yes, there's deliberate proxy object support in mock.patch (django settings being one specific use-case).
msg225166 - (view) Author: Sean McCully (seanmccully) * Date: 2014-08-10 21:45
So the changes submitted, take into the attributes that are part of the standard Python Data Model/Descriptors and defined as editable per documentation. 
https://docs.python.org/3/reference/datamodel.html

The thought is if a user is needing to support outside of Proxy Object (currently supported) and default descriptor attributes then mock should be special cased by user/developer. 

Please advise on current solution.
msg255648 - (view) Author: Xiadong Zhu (mailto1587) Date: 2015-12-01 11:40
How's the issue going on?

The situation to mock function's ``__defaults__`` attribute is general, as default argument is determinate after function definition, when we need to test a function such as:

    def access_db(statement, backend=default_db_backend):
        return default_db_backend.execute(statement)

that we must mock ``__defaults__`` attribute if we want to invoke it with default backend.

It has one year past, though I could patch the ``_patch`` class but it's dirty, is the issue a defect can be fixed or unsolvable?
msg255652 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2015-12-01 14:23
Sean's patch looks good to me.
msg257799 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-01-09 07:44
New changeset b67ed559a7d3 by Senthil Kumaran in branch '3.5':
Issue #22138: Fix mock.patch behavior when patching descriptors. Restore
https://hg.python.org/cpython/rev/b67ed559a7d3

New changeset 9b21dfd71561 by Senthil Kumaran in branch 'default':
merge from 3.5
https://hg.python.org/cpython/rev/9b21dfd71561
msg257800 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2016-01-09 07:45
This was an interesting issue. Thanks for the patch, Sean to fix this bug. I have committed it in 3.5 and 3.6.
History
Date User Action Args
2016-01-09 07:45:37orsenthilsetstatus: open -> closed

assignee: orsenthil
versions: + Python 3.5, Python 3.6, - Python 3.4
nosy: + orsenthil

messages: + msg257800
resolution: fixed
stage: commit review -> resolved
2016-01-09 07:44:20python-devsetnosy: + python-dev
messages: + msg257799
2015-12-01 16:10:43r.david.murraysetstage: needs patch -> commit review
2015-12-01 14:23:34michael.foordsetmessages: + msg255652
2015-12-01 11:40:40mailto1587setnosy: + mailto1587
messages: + msg255648
2014-08-16 12:44:47vstinnersetnosy: + vstinner
2014-08-10 21:45:53seanmccullysetmessages: + msg225166
2014-08-06 12:34:51michael.foordsetmessages: + msg224929
2014-08-06 09:18:51michael.foordsetmessages: + msg224918
2014-08-06 09:18:50ezio.melottisetmessages: + msg224917
2014-08-06 08:47:20seanmccullysetfiles: + issue22138.patch

nosy: + seanmccully
messages: + msg224916

keywords: + patch
2014-08-05 04:37:01ezio.melottisetnosy: + ezio.melotti, michael.foord
messages: + msg224804

keywords: + easy
stage: needs patch
2014-08-05 04:14:00chepnercreate