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.

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

mock calls don't propagate to parent (autospec) #65677

Closed
and mannequin opened this issue May 12, 2014 · 22 comments
Closed

mock calls don't propagate to parent (autospec) #65677

and mannequin opened this issue May 12, 2014 · 22 comments
Assignees
Labels
3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@and
Copy link
Mannequin

and mannequin commented May 12, 2014

BPO 21478
Nosy @cjw296, @voidspace, @PCManticore, @kushaldas, @mariocj89, @tirkarthi, @iforapsy
PRs
  • bpo-21478: Autospec functions should propagate mock calls to parent #11273
  • [3.7] bpo-21478 - Autospec functions should propagate mock calls to parent GH-11273 #12039
  • bpo-21478: Record calls to parent when autospecced objects are used as child with attach_mock #14688
  • [3.8] bpo-21478: Record calls to parent when autospecced objects are used as child with attach_mock (GH 14688) #14902
  • [3.7] bpo-21478: Record calls to parent when autospecced objects are used as child with attach_mock (GH 14688) #14903
  • 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/voidspace'
    closed_at = <Date 2019-07-22.08:40:25.282>
    created_at = <Date 2014-05-12.12:56:34.786>
    labels = ['3.7', '3.8', 'type-bug', 'library', '3.9']
    title = "mock calls don't propagate to parent (autospec)"
    updated_at = <Date 2019-10-17.02:09:03.392>
    user = 'https://bugs.python.org/and'

    bugs.python.org fields:

    activity = <Date 2019-10-17.02:09:03.392>
    actor = 'xtreak'
    assignee = 'michael.foord'
    closed = True
    closed_date = <Date 2019-07-22.08:40:25.282>
    closer = 'xtreak'
    components = ['Library (Lib)']
    creation = <Date 2014-05-12.12:56:34.786>
    creator = 'and'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 21478
    keywords = ['patch']
    message_count = 22.0
    messages = ['218321', '218362', '332252', '347592', '347593', '347631', '347705', '348285', '348288', '348289', '348291', '354489', '354491', '354496', '354642', '354735', '354737', '354748', '354749', '354750', '354815', '354827']
    nosy_count = 9.0
    nosy_names = ['cjw296', 'michael.foord', 'Claudiu.Popa', 'kushal.das', 'and', 'mariocj89', 'xtreak', 'iforapsy', 'Caris Moses']
    pr_nums = ['11273', '12039', '14688', '14902', '14903']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue21478'
    versions = ['Python 3.7', 'Python 3.8', 'Python 3.9']

    @and
    Copy link
    Mannequin Author

    and mannequin commented May 12, 2014

    Calls to autospecced mock functions are not recorded to mock_calls list of parent mock. This only happens if autospec is used and the original object is a function.

    Example:

    import unittest.mock as mock
    
    def foo():
        pass
    
    parent = mock.Mock()
    parent.child = mock.create_autospec(foo)
    parent.child()
    print(parent.mock_calls)

    Output:
    []

    Expected output:
    [call.child()]

    It works fine if foo function is substituted with a class.

    Initially I came across this problem with patch() and attach_mock() but I simplified it for the demonstration.

    @and and mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels May 12, 2014
    @voidspace
    Copy link
    Contributor

    Mock objects detect when another mock is added as a "child", but they don't currently detect that a function created by autospec has been added. It should be a fairly easy fix.

    @voidspace voidspace self-assigned this May 12, 2014
    @tirkarthi
    Copy link
    Member

    I think this should be handled in _check_and_set_parent where if value's type is FunctionType then value.mock should be used against which parent and name should be set since create_autospec returns function with mock attached to 'mock' attribute and all helper methods attached. There could be a case where a function is directly attached to mock where value will be of FunctionType but value.mock will not be set since it's not created with create_autospec and the AttributeError has to silenced. I think the below patch will handle both scenarios. This doesn't cause any test failure and hence it will be good to convert the original report as a unit test. Feedback welcome on the approach. I will raise a PR with tests and I am updating the relevant versions where this fix can be applied.

    Sample program

    from unittest import mock
    
    def foo(a, b):
        pass
    
    parent = mock.Mock()
    a = mock.create_autospec(foo)
    parent.child = a # 'a' is FunctionType and has a.mock attribute set (create_autospec -> _set_signature -> _setup_func)
    
    parent.child_func = foo # 'foo' is FunctionType with no mock attribute set that could cause AttributeError
    
    parent.child(1, 2) # Recorded
    parent.child_func(2, 3) # Not recorded since it's actual call to child_func and has no parent set due to AttributeError
    print(parent.method_calls) # [call.child(1, 2)]

    Patch :

    diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py
    index 38189c9aec..143263722d 100644
    --- a/Lib/unittest/mock.py
    +++ b/Lib/unittest/mock.py
    @@ -321,6 +321,12 @@ class _CallList(list):

     def _check_and_set_parent(parent, value, name, new_name):
    +    if isinstance(value, FunctionTypes):
    +        try:
    +            value = value.mock
    +        except AttributeError:
    +            pass
    +
         if not _is_instance_mock(value):
             return False
         if ((value._mock_name or value._mock_new_name) or

    @tirkarthi tirkarthi added 3.7 (EOL) end of life 3.8 only security fixes labels Dec 20, 2018
    @iforapsy
    Copy link
    Mannequin

    iforapsy mannequin commented Jul 10, 2019

    Can we reopen this bug? Karthikeyan's PR works for Dmitry's toy example, but it does not work in the usual case where patch() and attach_mock() are used. I encountered this bug on Python 3.7.3, which includes the PR.

    Non-toy example:
    import unittest.mock as mock

        def foo():
            pass
    
        parent = mock.Mock()
    
        with mock.patch('__main__.foo', autospec=True) as mock_foo:
            parent.attach_mock(mock_foo, 'child')
            parent.child()
            print(parent.mock_calls)

    Actual output:
    []

    Expected output:
    [call.child()]

    The reason why Karthikeyan's PR works on the toy example is that that mock's name is not set. In the usual case, the function mock's name will be set so this "if" block in _check_and_set_parent will return immediately.
    if ((value._mock_name or value._mock_new_name) or
    (value._mock_parent is not None) or
    (value._mock_new_parent is not None)):
    return False

    I think a possible fix is to move the inner mock extraction out to the attach_mock function as that function contains code to clear the mock's parent and name attributes. Downside is that that would make it fail on Dmitry's toy example.

    @tirkarthi
    Copy link
    Member

    Thanks Jack for the report. I am reopening this issue. I will use your example as a unit test. I will try to look into it. If you have more cases or examples related to the issue feel free to add them.

    @tirkarthi tirkarthi added the 3.9 only security fixes label Jul 10, 2019
    @tirkarthi tirkarthi reopened this Jul 10, 2019
    @tirkarthi
    Copy link
    Member

    I think a possible fix is to move the inner mock extraction out to the attach_mock function as that function contains code to clear the mock's parent and name attributes. Downside is that that would make it fail on Dmitry's toy example.

    Moving the inner mock check would cause the original report to fail. But the same logic can be copied to attach_mock so that name and parent for inner mock set in "mock" attribute is cleared. So _check_and_set_parent code path is hit the mock attribute without name and parent would behave as expected. Jack, would appreciate your review of the PR. I have added your report as a unittest along with directly using create_autospec with attach_mock that behaves like patch and autospec=True. I have also asserted that name is reset to child as per the docs at https://docs.python.org/3/library/unittest.mock.html#attaching-mocks-as-attributes

    Thanks

    @iforapsy
    Copy link
    Mannequin

    iforapsy mannequin commented Jul 11, 2019

    Thanks, Karthikeyan! I applied your change to Lib/unittest/mock.py, and it fixed the bug for me. I confirmed that calls to the child mock now appear in the parent's mock_calls in my test suite. Your PR looks good to me.

    @cjw296
    Copy link
    Contributor

    cjw296 commented Jul 22, 2019

    New changeset 7397cda by Chris Withers (Xtreak) in branch 'master':
    bpo-21478: Record calls to parent when autospecced objects are used as child with attach_mock (GH 14688)
    7397cda

    @cjw296
    Copy link
    Contributor

    cjw296 commented Jul 22, 2019

    New changeset 22fd679 by Chris Withers (Miss Islington (bot)) in branch '3.8':
    bpo-21478: Record calls to parent when autospecced objects are used as child with attach_mock (GH 14688) (GH-14902)
    22fd679

    @cjw296
    Copy link
    Contributor

    cjw296 commented Jul 22, 2019

    New changeset e9b187a by Chris Withers (Miss Islington (bot)) in branch '3.7':
    bpo-21478: Record calls to parent when autospecced objects are used as child with attach_mock (GH 14688) (GH-14903)
    e9b187a

    @tirkarthi
    Copy link
    Member

    Thanks Jack for the report. Thanks Mario and Chris for reviews. I am closing this as resolved.

    @CarisMoses
    Copy link
    Mannequin

    CarisMoses mannequin commented Oct 11, 2019

    Hello,
    I am still running into this issue. I have tested the following code with Python 3.7.4, 3.7.5rc1 , and 3.8.0rc1.

    from unittest import TestCase
    from unittest.mock import patch, Mock, call
    
    class MyObject:
        def __init__(self):
            self.foo = 0
            self.bar = 0
    
        def set_foo(self, value):
            self.foo = value
    
        def set_bar(self, value):
            self.bar = value
    
    def do_something():
        o = MyObject()
        o.set_foo(3)
        o.set_bar(4)
        return 'something unrelated'
    
    class MyObjectTest(TestCase):
    @patch('test_mock.MyObject.set_bar', autospec=True)
    @patch('test_mock.MyObject.set_foo', autospec=True)
    def test_do_something(self, mock_set_foo, mock_set_bar):
        manager = Mock()
        manager.attach_mock(mock_set_foo, 'set_foo_func')
        manager.attach_mock(mock_set_bar, 'set_bar_func')
        do_something()
        assert manager.mock_calls == [call.set_foo_func(3), call.set_bar_func(4)]
    

    @tirkarthi
    Copy link
    Member

    Oh well :( My initial guess is that the report is for methods. The other reports were for functions. So I am wondering if the check for FunctionType is successful and if I need to handle something more. I haven't tried it yet.

    @CarisMoses
    Copy link
    Mannequin

    CarisMoses mannequin commented Oct 11, 2019

    I tried it with mocked functions instead of methods and got the same result, so I dug into this a little deeper. It seems as though the issue it how the mocked functions are called. If I replace the do_something() line with the following then it works.

    #do_something()
    manager.set_foo_func(3)
    manager.set_bar_func(4)

    I am a beginner with unittest so please let me know if I am just using this incorrectly. However in the original code I posted, if I print(manager.set_foo_func.mock_calls, manager.set_bar_func.mock_calls) I get the calls made in do_something(), however print(manager.mock_calls) returns an empty list which leads me to believe something else is wrong.

    @tirkarthi
    Copy link
    Member

    I tried your example as below using __name__. I received an AttributeError for which bpo-38473 in 3.7.5RC1 and 3.8.0RC1 and opened bpo-38473 and I am running my below code under that issue PR. For 3.7.4, I received manager.mock_calls to be an empty list since it doesn't contain the patch for this issue. Can you please confirm my results too.

    ➜ cpython git:(bpo-38473) cat ../backups/bpo21478.py
    import unittest
    from unittest import TestCase
    from unittest.mock import patch, Mock, call, ANY

    class MyObject:
        def __init__(self):
            self.foo = 0
            self.bar = 0
    
        def set_foo(self, value):
            self.foo = value
    
        def set_bar(self, value):
            self.bar = value
    
    
    def do_something():
        o = MyObject()
        o.set_foo(3)
        o.set_bar(4)
        return "something unrelated"
    
    
    class MyObjectTest(TestCase):
        @patch(f"{__name__}.MyObject.set_bar", autospec=True)
        @patch(f"{__name__}.MyObject.set_foo", autospec=True)
        def test_do_something(self, mock_set_foo, mock_set_bar):
            manager = Mock()
            manager.attach_mock(mock_set_foo, "set_foo_func")
            manager.attach_mock(mock_set_bar, "set_bar_func")
            do_something()
            assert manager.mock_calls == [
                call.set_foo_func(ANY, 3),
                call.set_bar_func(ANY, 4),
            ]
            manager.assert_has_calls([call.set_foo_func(ANY, 3), call.set_bar_func(ANY, 4)])
    
    
    if __name__ == "__main__":
        unittest.main()

    ➜ cpython git:(bpo-38473) ./python ../backups/bpo21478.py
    .
    ----------------------------------------------------------------------
    Ran 1 test in 0.006s

    OK

    @CarisMoses
    Copy link
    Mannequin

    CarisMoses mannequin commented Oct 15, 2019

    I am having some trouble figuring out how to use CPython to get the exact PR you tested on since I've never used CPython before. However, when I just install Python 3.7.5RC1 and 3.8.0RC1 from the binaries and run your code I do get the AttributeError. And when I run your code with Python3.7.4 from binaries I get an empty list.

    Can you point me to a good source that will tell me how to get the patch for bpo-38473? I built and ran cpython from source on my Mac successfully. However when I checkout a different branch or version, rebuild, and run ./python.exe I always get the same Python version.

    @tirkarthi
    Copy link
    Member

    Thanks for the confirmation. You can download the patch for the PR by appending .diff/.patch to the PR URL. The patch can be applied to your source locally with "git apply patch_file" to run my example. Reverting patch would cause AttributeError

    Diff URL : https://github.com/python/cpython/pull/16784.diff

    The PR also includes regression test so reverting the patch to mock.py and running below command would cause error in test.

    ./python -m test test_unittest

    Checking out a different branch and doing below command should do a clean rebuild with a different version of python. Also for reference : https://devguide.python.org/

    git clean -xdf && ./configure && make -j4

    Hope it helps

    @CarisMoses
    Copy link
    Mannequin

    CarisMoses mannequin commented Oct 15, 2019

    Great, thanks so much. It works with the patch. So will this patch be included in the next released version of Python?

    @tirkarthi
    Copy link
    Member

    It needs to approved and merged by a core dev so that it will be available by 3.7.6 and 3.8.1. To clarify the calls are recorded in 3.7.5 and 3.8.0 in mock_calls. It's a problem with assert_has_calls and autospec. As a workaround you can turn off autospec or use mock_calls to assert for calls instead of assert_has_calls. Sorry for the inconvenience.

    @CarisMoses
    Copy link
    Mannequin

    CarisMoses mannequin commented Oct 15, 2019

    I see. Thanks for your help!

    @CarisMoses
    Copy link
    Mannequin

    CarisMoses mannequin commented Oct 16, 2019

    I believe I have found another bug related to this issue. I can start a new issue if necessary. When I use some_mock.attach_mock(...) and make calls, the resulting some_mock.call_args is None while the some_mock.mock_calls list is not empty.

    The code below shows this in Python 3.7.5:

    from unittest import TestCase
    from unittest.mock import patch, Mock
    
    def foo(value):
        return value
    
    class MyObjectTest(TestCase):
    @patch(f'{__name__}.foo')
    def test_do_something(self, mock_foo):
        manager = Mock()
        manager.attach_mock(mock_foo, 'foo_func')
        foo(3)
        print(manager.mock_calls)
        print(manager.call_args)
    
    if __name__ == "__main__":
        unittest.main()

    The print statements return:
    [call.foo_func(3)]
    None

    While the code below (without attach_mock) works fine:
    from unittest import TestCase
    from unittest.mock import patch, Mock

    def foo(value):
        return value
    
    class MyObjectTest(TestCase):
    @patch(f'{__name__}.foo')
    def test_do_something(self, mock_foo):
        foo(3)
        print(mock_foo.mock_calls)
        print(mock_foo.call_args)
    
    if __name__ == "__main__":
        unittest.main()

    Print statements correctly return:
    [call(3)]
    call(3)

    for completeness the call_args_list also returns [] when using attach_mock. I also tested in Python 3.8.0 and got the same result.

    @tirkarthi
    Copy link
    Member

    Please open a new issue. It's getting little hard to track on this since it was closed and has 2 PRs merged.

    @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
    Labels
    3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants