search

derwiki-adroll / mock

Automatically exported from code.google.com/p/mock
BSD 2-Clause "Simplified" License
0 stars 0 forks source link

Bug on call() comparison #233

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
This is a bug that has allowed to introduce failing unittests pass.

The oneliner is:

print 'Mock has bug on call comparison:', mock.call(0,False) == 
mock.call(False,0)

I have found it in the following way:

import mock

def function_badly_called(boolean, integer):
    return (boolean, integer)

def function_under_test(integer, boolean):
    a,b = (integer, boolean)
    function_badly_called(a,b)

function_badly_called = mock.MagicMock()
function_under_test(0,False)
function_badly_called.assert_called_once_with(False, 0)

What is the expected output? What do you see instead?
The oneliner should evaluate to false

What version of the product are you using? On what operating system?
1.0.1

Original issue reported on code.google.com by javier.d...@fon.com on 11 Jun 2014 at 8:16

GoogleCodeExporter commented 9 years ago 
That's because 0 and False compare equal.

Original comment by fuzzyman on 11 Jun 2014 at 9:38

GoogleCodeExporter commented 9 years ago 
It is indeed by that, but when asserting that a function has been called with 
some arguments, you should also check for at least the data types.

IMO, when doing a function check, you want to assert that a call was indeed 
done with such arguments, and no with others that evaluate to the same. Strict 
assertion should be possible.

Original comment by javier.d...@fon.com on 11 Jun 2014 at 9:41

GoogleCodeExporter commented 9 years ago 
I think I haven't given my thoughts too clear.

If I am asserting that a function has been called with something, I want that 
something to be asserted and not what it evaluates to. Else, the message would 
be

mock.assert_call_evaluates_to(....)
and not

mock.assert_called_with(...)

Original comment by javier.d...@fon.com on 11 Jun 2014 at 11:55

GoogleCodeExporter commented 9 years ago 
I will add an implementation of __eq__() that satisfies this needs, with the 
aim of having at least a new call like the following:

mock.assert_strict_called_with()

A fast development (with dirty code on it):
https://gist.github.com/txomon/4a592aaca14ff09c9b0e#file-gistfile1-txt-L47-l62

Original comment by javier.d...@fon.com on 11 Jun 2014 at 4:33

GoogleCodeExporter commented 9 years ago 
It is well documented that the mock assert methods use equality. You want type 
checking as well as equality (note that isinstance will return True for 
subclasses - and bool is a subclass of int so your implementation probably 
doesn't work for your use case).

Another use case is wanting an identity check instead of equality. There are 
documented ways of getting this (or other checks). I suggest you use one of 
those instead. See:

http://www.voidspace.org.uk/python/mock/examples.html#coping-with-mutable-argume
nts

Original comment by fuzzyman on 11 Jun 2014 at 4:40

GoogleCodeExporter commented 9 years ago 
Would it be possible to have any type of assertion that would check for types 
in calls? Argument swapping would be a good check, and even more if it would be 
inside some other method to assert calls.

What regards to my case, at the moment, my problem comes with that I had 
swapped two arguments, therefore, (False, 0) would not be equal to (0, False) 
thanks to 0 not being instance of False.

Original comment by javier.d...@fon.com on 12 Jun 2014 at 8:41

GoogleCodeExporter commented 9 years ago 
I haven't had any other requests for this, so I don't see it going in soon (I'm 
against proliferation of assert methods in general). Note that you can add it 
in a subclass and use that subclass yourself. (There is even an argument to 
patch allowing you to specify the type of mock objects it should create for 
you.)

It *certainly* can't be the default behaviour. Being able to use "matcher 
objects" to compare arguments against is very useful. See for example:

http://www.voidspace.org.uk/python/mock/examples.html#more-complex-argument-matc
hing

mock even provides one "matcher" that does this - the ANY object.

You could create a type checking matcher and use those in your assertions:

   mock.assert_called_with(Strict(0), Strict(False))

Original comment by fuzzyman on 12 Jun 2014 at 11:35