search

derwiki-adroll / mock

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

Mocks/MagicMocks created with a spec should not always be callable #72

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
Currently if you create a mock object (Mock or MagicMock) with a spec then it 
will always be callable. 

In theory if the spec object is a class then it should be callable. If the spec 
object is an instance then the mock should only be callable if the instance has 
a __call__ method.

A problem is that people use the class as spec so that mocks can masquerade as 
instances of that class (the reason they are using a mock is so that they don't 
*have* to instantiate the class). So just because we are using a class as a 
mock doesn't mean we know that the mock *should* be callable. 

On the other hand if the spec *is* an instance then we may be safe in assuming 
that the mock will be representing an instance and not a class - and therefore 
if the instance *doesn't* have __call__ the mock shouldn't either. Setting an 
explicit 'return_value' could automatically override that.

If the mocking is put in place with `patch` then we *can* know whether or not 
the mock is replacing a class or an instance - so maybe a decision could be 
made there?

Note that this is a theoretical issue, no users have actually reported it as a 
problem (yet).

Original issue reported on code.google.com by fuzzyman on 7 Mar 2011 at 11:40

GoogleCodeExporter commented 9 years ago 
I've found this annoying when passing mocks into frameworks that check if an 
object is callable and, if so, call it.  (Examples I know of are both template 
languages: Django templates and ZPT.)  As a workaround I end up adding stuff 
like this to my tests:

   mock_foo.return_value = mock_foo

... which isn't hard, but I don't like that it's not self-explanatory to 
somebody reading my tests - too much black magic smell.

Original comment by sli...@gmail.com on 14 Jun 2011 at 6:38

GoogleCodeExporter commented 9 years ago 
The fix I was *going* to put in place was to have calling a mock that shouldn't 
be callable raise a TypeError exception. That wouldn't solve your use-case. The 
best way to solve that would be to have a new class NonCallableMock that 
doesn't have a __call__ method at all. That introduces some new complexity 
(attributes of non-callable mocks would still need to be callable) but would 
solve both use cases.

Original comment by fuzzyman on 15 Jun 2011 at 9:41

GoogleCodeExporter commented 9 years ago 
This also applies to autospec.

When patching a class it should be callable. The return value should have the 
same spec (inherit) but should only be callable if the class has a __call__ 
method.

If patching an instance (or builtin type) then the mock should only be callable 
if the original is callable (has a __call__ method).

Original comment by fuzzyman on 17 Jun 2011 at 4:50

GoogleCodeExporter commented 9 years ago

Original comment by fuzzyman on 18 Jun 2011 at 12:03

GoogleCodeExporter commented 9 years ago 
Committed in repo, will be in 0.8 alpha 2. The fix is the addition of two new 
mock classes NonCallableMock and NonCallableMagicMock.

Original comment by fuzzyman on 3 Jul 2011 at 11:18