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.

Sign up for GitHub

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

Jump to bottom

patch.object doesn't restore function defaults #66336

Closed
chepner mannequin opened this issue Aug 5, 2014 · 11 comments
Closed

patch.object doesn't restore function defaults #66336

chepner mannequin opened this issue Aug 5, 2014 · 11 comments
Assignees
@orsenthil
Labels
easy stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@chepner
Copy link
Mannequin

chepner mannequin commented Aug 5, 2014

BPO 22138
Nosy @orsenthil, @vstinner, @ezio-melotti, @voidspace, @seanmccully
Files
  • issue22138.patch
    		•

    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/orsenthil'
closed_at = <Date 2016-01-09.07:45:37.636>
created_at = <Date 2014-08-05.04:14:00.659>
labels = ['easy', 'type-bug', 'library']
title = "patch.object doesn't restore function defaults"
updated_at = <Date 2016-01-09.07:45:37.592>
user = 'https://bugs.python.org/chepner'

    bugs.python.org fields:

    activity = <Date 2016-01-09.07:45:37.592>
actor = 'orsenthil'
assignee = 'orsenthil'
closed = True
closed_date = <Date 2016-01-09.07:45:37.636>
closer = 'orsenthil'
components = ['Library (Lib)']
creation = <Date 2014-08-05.04:14:00.659>
creator = 'chepner'
dependencies = []
files = ['36286']
hgrepos = []
issue_num = 22138
keywords = ['patch', 'easy']
message_count = 11.0
messages = ['224801', '224804', '224916', '224917', '224918', '224929', '225166', '255648', '255652', '257799', '257800']
nosy_count = 8.0
nosy_names = ['orsenthil', 'vstinner', 'ezio.melotti', 'michael.foord', 'python-dev', 'chepner', 'seanmccully', 'mailto1587']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue22138'
versions = ['Python 3.5', 'Python 3.6']
    @chepner
    Copy link
    Mannequin Author

    chepner mannequin commented Aug 5, 2014

    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

    @chepner chepner mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Aug 5, 2014
    @ezio-melotti
    Copy link
    Member 

    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

    @seanmccully
    Copy link
    Mannequin

    seanmccully mannequin commented Aug 6, 2014

    Is special casing the special attrs a permament enough solution?

    @ezio-melotti
    Copy link
    Member

    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).

    @voidspace
    Copy link
    Contributor

    It might have to be. There's no general purpose solution that will fit every possible behaviour for a Python descriptor I'm afraid.

    @voidspace
    Copy link
    Contributor

    And yes, there's deliberate proxy object support in mock.patch (django settings being one specific use-case).

    @seanmccully
    Copy link
    Mannequin

    seanmccully mannequin commented Aug 10, 2014

    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.

    @mailto1587
    Copy link
    Mannequin

    mailto1587 mannequin commented Dec 1, 2015

    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?

    @voidspace
    Copy link
    Contributor

    Sean's patch looks good to me.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jan 9, 2016

    New changeset b67ed559a7d3 by Senthil Kumaran in branch '3.5':
    Issue bpo-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

    @orsenthil
    Copy link
    Member

    This was an interesting issue. Thanks for the patch, Sean to fix this bug. I have committed it in 3.5 and 3.6.

    @orsenthil orsenthil self-assigned this Jan 9, 2016
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @orsenthil orsenthil

    Labels
    easy stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @voidspace @orsenthil @ezio-melotti