[TIP] Is this a bug in unittest.mock, or am I missing something?

[John W]

Thank you James for the additional input. I've confirmed the same
behavior over here.
The fact that the "==" check you concocted behaves differently from
"assert_has_calls" for this case does push me towards thinking this is
a bug rather than a feature.
I too would be interested to hear the opinions of others on this list
re: that point.


Now to the other topic: the "why are you even doing that?" question... (:
I've been mostly trying to figure out how this library is supposed to
work (since I'm new to it) rather than try to perfect my testing
methodology. Call me a bottom-up learner (:
But since both you and that other fellow on SO seem to find my example
unconvincing, I'll expand upon it a bit and you can tell me what you
think:

Let's say I have a little "Downloader" class (this is "Foo" in my
original example):

    ### download.py
    class Downloader:
        def __init__(self, temp_dir):
            self.temp_dir = temp_dir

        def download_item(self, item):
            '''Downloads the item to self.temp_dir'''
            # code omitted...
            pass

Assume that that class is well-tested on its own — we know it connects
to the right servers, downloads the right file to the right place,
etc.

The Downloader is used somewhere else, as part of a larger program.

    ### main.py
    from download import Downloader
    def process_many_things():
        '''This function downloads various files, processes them,
        and outputs the result'''
        # code omitted...
        pass

I'd like to test `process_many_things`. I suppose this not really a
unit test, more like an end-to-end test with mocked-out dependenceies
— let's not get into a discussion of nuanced testing terminology (:
I just want to make sure that process_many_things downloads some
items, amongst other things.

So, in my test code, I might have:

    ### test_end_to_end.py
    from unittest.mock import patch, call
    import main

    @patch('main.Downloader' autospec=True)
    def test_happy_path(mockDownloader):
        # Do a bunch of stuff
        main.process_many_things()

        # Now confirm that it performed appropriate actions.
    	mockDownloader.assert_has_calls([
            call('expected/temp/dir').download_item('expected-item')
            # You can imagine other stuff here...
        ])

Now, I'm sure if you ask 10 developers how to write this test you
could get 10 different answers; there are many approaches.
But is this particular one flat-out wrong, somehow?
Ignoring the fact that this bug we've discussed prevents the above
from working as expected, is this somehow a cuckoobananas example
altogether?
I admit it's not perfect, but it tests some real business logic worth
testing, I'd say.

> The style of testing that I use at work and advocate is one that
> generally avoids `assert_has_calls`, and tends to check
> `call_count` and `call_args_list` and make use
> of `assert_called_once_with`.

Yes, I find myself leaning that way, too. While I was trying out
various approaches, I stumbled upon the weird behavior of
assert_has_calls, so went down a bit of a rabbit hole trying to
understand that (hence this thread, etc).
The code I have today does not use it, and I'll be sure to warn others, too (:

Thanks again
-John


On 2/17/17, James Cooke <me at jamescooke.info> wrote:
> Hi John,
>
> OK great - your `test_calls_match_2` works for me too. Sorry for
> injecting any confusion into the thread.
>
> I've had another look at your `MockFoo` example and I think that the
> "problem" is the way that calls are matched by `assert_has_calls`. That
> function is working on the `mock_calls` list
> (https://docs.python.org/3/library/unittest.mock.html#unittest.mock.Mock.mock_calls),
> and using that directly appears to work for me:
>
>     from unittest import mock
>
>     ...
>
>     def test_foo():
>         with mock.patch('test.Foo', autospec=True) as MockFoo:
>             m = Foo()
>             m.my_method(122)
>             expected_calls = [mock.call(), mock.call().my_method(122)]
>             assert MockFoo.mock_calls == expected_calls
>
> So, yes, it looks to me that there is a bug in the `assert_has_calls`
> matching, but there's probably a "good" reason that it works this way -
> maybe someone on this list can check this / validate my thinking?
>
> Meanwhile, I would argue that this example is attempting to mock out too
> much and it is not clear the context which would require this approach
> (IMO you can see this in the Stack Overflow comment thread on your
> original question). If this is *really* what you want to do, I would
> inject an instance and test the calls on the constructor and instance
> separately (explicit better than implicit) - something like this,
> although it's pretty dirty because the whole of `Foo` is mocked, when
> really it appears the goal is to test the call on `my_method`.
>
>
>     @mock.patch('test.Foo', autospec=True)
>     def test_foo(MockFoo):
>         instance = mock.Mock(spec=Foo)
>         instance.my_method.return_value = '__MY_METHOD__'
>         MockFoo.return_value = instance
>         m = Foo()
>
>         result = m.my_method(123)
>
>         assert result == '__MY_METHOD__'
>         MockFoo.assert_called_once_with()
>         instance.my_method.assert_called_once_with(123)
>
> The style of testing that I use at work and advocate is one that
> generally avoids `assert_has_calls`, and tends to check `call_count` and
> `call_args_list` and make use of `assert_called_once_with`. I've found
> these much more stable and provide expected results compared to
> `assert_has_calls`.
>
> Hope that helps.
>
> Cheers,
>
> James
>
>
> On Fri, 17 Feb 2017, at 12:38 AM, John W wrote:
>> To take your approach, and do what (I think) I'm trying to do, I'd write:
>>
>>     from unittest import mock
>>     def test_calls_match_2():
>>         """
>>         Calls on a Mock match expectation
>>         """
>>         constructor = mock.Mock()
>>         instance = constructor()
>>         instance.my_method(123)
>>
>>         constructor.assert_has_calls([mock.call(),
>>         mock.call().my_method(123)])
>>
>> That passes just fine for me, and seems reasonable.
>>
>> I think the reason you had "call()" vs "call" was that "m" in your
>> example had "my_method" called on it directly, rather than calling it
>> on the result of the initial call to "m()". That is - calling
>> my_method on the constructor, rather than the instance.
>>
>> So that all seems fine to me, and seems to work as expected.
>>
>> But then, when mock.patch and autospec get involved, as in my original
>> example, it all seems to go south...
>>
>> -John
>>
>> On 2/16/17, James Cooke <me at jamescooke.info> wrote:
>> > Hi John,
>> >
>> >
>> >
>> > This is interesting and thanks for sharing it. Running python 3.5.2, I
>> > firstly simplified your example and got a slightly different exception:
>> >
>> >
>> > from unittest import mock
>> >
>> >
>> >
>> >
>> >
>> > def test_calls_match():
>> >
>> >     """
>> >
>> >     Calls on a Mock match expectation
>> >
>> >     """
>> >
>> >     m = mock.Mock()
>> >
>> >     m()
>> >
>> >     m.my_method(123)
>> >
>> >
>> >
>> >     m.assert_has_calls([mock.call(), mock.call().my_method(123)])
>> >
>> >
>> >
>> > The exception is slightly different for me:
>> >
>> >
>> >
>> > E               AssertionError: Calls not found.
>> >
>> > E               Expected: [call(), call().my_method(123)]
>> >
>> > E               Actual: [call(), call.my_method(123)]
>> >
>> >
>> >
>> > When I remove the parentheses after the second `call`, then everything
>> > works for me, and the test passes.
>> >
>> >
>> >     m.assert_has_calls([mock.call(), mock.call.my_method(123)])
>> >
>> >
>> >
>> > My assumption is that MagicMock will operate like Mock and that your
>> > MockFoo is a MagicMock, so could you try the `mock.call.my_method` on
>> > Python 3.4?
>> >
>> >
>> > I'm interested to hear how that works for you.
>> >
>> >
>> >
>> > Cheers,
>> >
>> >
>> >
>> > James
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> > On Thu, 16 Feb 2017, at 07:06 PM, John W wrote:
>> >
>> >> They seem to be the same types.
>> >
>> >>
>> >
>> >> I added these prints:
>> >
>> >>
>> >
>> >>         print("type of call():", type(call()))
>> >
>> >>         print("type of call().my_method(123):",
>> >
>> >>         type(call().my_method(123)))
>> >
>> >>         for c in MockFoo.mock_calls:
>> >
>> >>             print("type of mock call '{}': {}".format(c, type(c)))
>> >
>> >>
>> >
>> >> And output is:
>> >
>> >>
>> >
>> >>     type of call(): <class 'unittest.mock._Call'>
>> >
>> >>     type of call().my_method(123): <class 'unittest.mock._Call'>
>> >
>> >>     type of mock call 'call()': <class 'unittest.mock._Call'>
>> >
>> >>     type of mock call 'call().my_method(123)': <class
>> >
>> >>     'unittest.mock._Call'>
>> >
>> >>
>> >
>> >>
>> >
>> >> Thanks for the suggestion though.
>> >
>> >> I've been getting crickets from every place I've asked this
>> >> question (:
>> >>
>> >
>> >> -John
>> >
>> >>
>> >
>> >> _______________________________________________
>> >
>> >> testing-in-python mailing list
>> >
>> >> testing-in-python at lists.idyll.org
>> >
>> >> http://lists.idyll.org/listinfo/testing-in-python
>> >
>> >
>> >
>> >
>> >
>> > --
>> >
>> > James Cooke
>> >
>> > Backend software developer
>> >
>> > CV PDF: http://jamescooke.info/docs/james_cooke_cv.pdf
>> >
>> > Website: http://jamescooke.info/
>> >
>
>
> --
> James Cooke
> Backend software developer
> CV PDF: http://jamescooke.info/docs/james_cooke_cv.pdf
> Website: http://jamescooke.info/
>
> _______________________________________________
> testing-in-python mailing list
> testing-in-python at lists.idyll.org
> http://lists.idyll.org/listinfo/testing-in-python
>