classification
Title: assert_called_with could be more powerful if it allowed placeholders
Type: enhancement Stage:
Components: Versions: Python 3.4
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: michael.foord Nosy List: ezio.melotti, michael.foord, pitrou
Priority: normal Keywords:

Created on 2013-01-28 12:50 by pitrou, last changed 2013-02-28 13:13 by michael.foord.

Messages (12)
msg180852 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-01-28 12:50
assert_called_with currently compares every argument for equality, which is not very practical when one of the arguments is a complex object, of which you only want to check certain properties.

It could be very nice if you could write e.g.:

from mock import Mock, PLACEHOLDER

...
my_mock(1, someobj(), bar=someotherobj()):
foo, bar = my_mock.assert_called_with(
    1, PLACEHOLDER, bar=PLACEHOLDER)
self.assertIsInstance(bar, someobj)
self.assertIsInstance(foo, someotherobj)


(another name for PLACEHOLDER could be CAPTURE, regex-style :-))
msg180853 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2013-01-28 13:19
You mean like mock.ANY ?
msg180854 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2013-01-28 13:21
Oh, you want the assert_called_with call to *return* the objects compared with the placeholder? 

Well, mock.ANY already exists and you can pull the arguments out for individual assertions using some_mock.call_args.

args, kwargs = some_mock.call_args
msg180855 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-01-28 13:57
Ah, well... I agree mock.ANY sounds cool :-)
Perhaps it could be mentioned in the docs for assert_etc.? Otherwise you only learn about it if you are masochistic enough to read the doc till the end :-)

> you can pull the arguments out for individual assertions using
> some_mock.call_args.

Yeah, but it's cumbersome and it boils down to doing the matching by hand, while assert_called_something already does it.
msg180856 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-01-28 14:03
I'm noticing that with multiple ANY keyword arguments, the order in the result tuple is undefined. So perhaps ANY could be instantiable in those cases where disambiguation is required:

foo, bar = my_mock.assert_called_with(1, foo=ANY(0), bar=ANY(1))
self.assertIsInstance(bar, someobj)
self.assertIsInstance(foo, someotherobj)
msg180867 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2013-01-28 17:51
Yes there are definitely room for documentation improvements.

And, yes - pulling the args out from some_mock.call_args "boils down to doing the matching by hand". You only do it when you *want* to do the matching by hand.

Your use case I would write:

from mock import Mock, ANY

...
my_mock(1, someobj(), bar=someotherobj())
my_mock.assert_called_with(1, ANY, bar=ANY)

args, kwargs = my_mock.call_args
foo = args[1]
bar = kwargs['bar']

self.assertIsInstance(bar, someobj)
self.assertIsInstance(foo, someotherobj)


It's a *little* cumbersome, but not something I do very often and not much extra code. I'm not sure that having "assert_called_with" return objects is an intuitive API either. (And having to specify tuple indices in a call to ANY is a bit odd too.)

Custom matchers that do the comparison in their __eq__ method would be another possibility:

class IsInstance(object):
  def __init__(self, Type):
    self.Type = Type
  def __eq__(self, other):
   return isinstance(other, self.Type)


my_mock.assert_called_with(1, IsInstance(someobj), bar=IsInstance(someotherobj))
msg181013 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-01-31 10:00
I think a better possibility would be to return an indexable "match" object (as bit like re match objects):

  my_mock(1, someobj(), bar=someotherobj()):
  match = my_mock.assert_called_with(
      1, ANY, bar=ANY)
  self.assertIsInstance(match[0], someobj)
  self.assertIsInstance(match['bar'], someotherobj)

> It's a *little* cumbersome, but not something I do very often and not
> much extra code. I'm not sure that having "assert_called_with" return
> objects is an intuitive API either. 

It has happened to me quite a bit in the latest days :-)
"assert_called_with" returning something may fall in the "not very pure" category, but we already have "assertRaises" and friends returning a context manager, and it has proved quite practical.

Also, your proposed workaround wouldn't work for assert_any_call().

Perhaps we should wait for other people to chime in.
msg181895 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2013-02-11 14:03
I still don't particularly like the idea of the assert_* methods returning something.

If the call args tuples had args and kwargs attributes, for which there are outstanding feature requests, then you could simply do:

my_mock(1, someobj(), bar=someotherobj())

foo = my_mock.call_args.args[1]
bar = my_mock.call_args.kwargs['bar']

By avoiding the extra step of tuple unpacking this is still nice and readable (IMO).
msg183215 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-02-28 12:06
Just got bitten by this again. Context: I have a protocol which is sending JSON datagrams over the wire and I'm checking the sent data. I can't exactly check the JSON-encoded content since dict ordering is undefined. So right now I have to write:

    self.assertEqual(q.transport.send.call_count, 1)
    (dgram, target), _ = q.transport.send.call_args
    self.assertEqual(target, (PEER, PORT))
    self.assertEqual(json.loads(dgram), { ... })

Clumsy, clumsy (note that I am too lazy to add a check for the kwargs).

Would be much better if I could simply write:

    dgram, = q.tranport.assert_called_once_with(mock.ANY, (PEER, PORT))
    self.assertEqual(json.loads(dgram), { ... })
msg183216 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2013-02-28 12:14
Note that there is nothing stopping you using the mock.ANY and assert_called_once_with style assert currently. You're making your assert more clumsy than it needs to be.

With my proposal you could write:

    q.tranport.assert_called_once_with(mock.ANY, (PEER, PORT))
    dgram = q.tranport.call_args.args[0]
    self.assertEqual(json.loads(dgram),expected)

You could currently be doing:

    q.tranport.assert_called_once_with(mock.ANY, (PEER, PORT))
    dgram = q.tranport.call_args[0][0]
    self.assertEqual(json.loads(dgram), expected)
msg183217 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-02-28 13:07
I still have to do some tuple unpacking myself while assert_* already did it first. It can be tedious and error-prone (especially if there are several arguments to get).

If the assert methods returning something bothers you, how about introducing a new method match_call_with(...)?
msg183218 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2013-02-28 13:13
Or create a JsonMatcher class that does it for you.


class JsonMatcher(object):
   def __init__(self, expected):
       self.expected = expected

   def __eq__(self, other):
       return json.loads(other) == self.expected

q.tranport.assert_called_once_with(JsonMatcher(expected), (PEER, PORT))
History
Date User Action Args
2013-02-28 13:13:38michael.foordsetmessages: + msg183218
2013-02-28 13:07:48pitrousetmessages: + msg183217
2013-02-28 12:14:02michael.foordsetassignee: michael.foord
messages: + msg183216
2013-02-28 12:06:19pitrousetmessages: + msg183215
2013-02-11 14:03:01michael.foordsetmessages: + msg181895
2013-01-31 10:00:38pitrousetmessages: + msg181013
2013-01-28 21:22:28ezio.melottisetnosy: + ezio.melotti
2013-01-28 17:51:11michael.foordsetmessages: + msg180867
2013-01-28 14:03:14pitrousetmessages: + msg180856
2013-01-28 13:57:34pitrousetmessages: + msg180855
2013-01-28 13:21:14michael.foordsetmessages: + msg180854
2013-01-28 13:19:41michael.foordsetmessages: + msg180853
2013-01-28 12:50:20pitroucreate