search

derwiki-adroll / mock

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

create_autospec'd method does not pass default arguments #148

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
In our project, we used mocksignature pretty extensively for testing.  We also 
nearly exclusively use named keyword arguments with defaults.  Over this past 
weekend, our buildout env pulled down the mock 1.0.0 dev release, which removed 
mocksignature, and I've been investigating how to make our project work against 
1.0.0 (currently, it got locked to 0.8).

Commits/issues indicated that create_autospec is the replacement for 
mocksignature, but I've found it does not support defaults for named keyword 
arguments.  Specifically, it makes a method with *args, **kwargs, which cannot 
support defaults (as far as I can tell).  This breaks a ton of tests which 
expect calls to be made with the defaults inserted.

I've written a patch but have just realized it breaks things by always 
converting kwargs to positional args, which is not ideal. I'll attach anyway 
for comment.

As it stands right now:

def testmeth(one="one", two=False):
    print one, two

from mock import create_autospec
m2=create_autospec(testmeth)
m2('hi')

m2.call_args
# => call('hi')

When really I'd expect:

# => call('hi', False)

or maybe

# => call('hi', two=False)

The patch I wrote here will do the ('hi', False) thing, but it also mutates 
named kwargs into positional args ala:

m2(two='second')
# => call('one', 'second')

And I'd expect the two= to be preserved in there somewhere.  Thoughts?

Original issue reported on code.google.com by dave.fos...@gmail.com on 28 Mar 2012 at 6:03

Attachments:

GoogleCodeExporter commented 9 years ago 
Is this only a backwards compatibility issue? (For which I apologise. In part 
you can blame easy_install which decides to install the dev release instead of 
the latest release on pypi which pip does - switching to pip instead of 
easy_install would at least help here.)

In general I think it is more desirable that the call is only recorded with the 
arguments actually used. I always considered it a big wart with mocksignature 
that arguments not in the original call were recorded. So whilst I could 
consider adding back a compatibility shim for mock 1.0, going forward I think 
the "current" situation is only *better*.

In other words, do you have a use case for wanting to record arguments that 
aren't actually used in the call - or is it just the pain of upgrading?

Original comment by fuzzyman on 28 Mar 2012 at 6:11

GoogleCodeExporter commented 9 years ago 
(No worries on the auto-upgrade, we had for some reason missed locking the 
version like we do _every other package_, heh - we use buildout so I'm not sure 
what that uses under the hood to get things from pypi)

I'll try to summarize the use case - the methods we currently create via 
mocksignature are called from the code we're actually testing.  So if we're 
testing a certain service's method, and that method eventually calls a 
dependent service (mocksignature'd), we check the call made to that dependent 
service for ALL the parameters we'd expect to see there.  The method we're 
testing is in charge of building that call and using it however it would 
normally be used in production, which means we're depending on the default 
values to exist in the call.  I think that is logical from a testing scenario.

In your opinion, are we maybe testing the wrong thing here, aka reaching too 
far into the expected implementation of the dependent method?

(while upgrading would be slightly painful, it wouldn't take more than an 
afternoon.  I just want to make sure what we're testing is accurate!)

Original comment by dave.fos...@gmail.com on 28 Mar 2012 at 6:23

GoogleCodeExporter commented 9 years ago 
So it seems to me you're using your "mocksignature'd" functions to test two 
things - that they are called in the right way *and* that the functions you're 
mocking out have the right defaults. mocksignature was nice for you because it 
combined both of those tests in one.

I'll see if there's an easy way to add a backwards compatibility mode to have 
create_autospec behave like mocksignature. A possibility for you guys is to 
have another set of tests that either mock at a lower level, or use 
inspect.getargspec, to check that your service functions have the right 
defaults.

Original comment by fuzzyman on 28 Mar 2012 at 6:42

GoogleCodeExporter commented 9 years ago 
Since it IS unit testing, it is likely fine for the test developer to 
understand what the mocked call will look like, and remove the default params.  
I'm in discussions with co-workers to see how they feel.

You stated before that you liked how create_autospec is functioning so don't 
worry about breaking it if it ends up too difficult. 

thanks!

Original comment by dave.fos...@gmail.com on 28 Mar 2012 at 7:03

GoogleCodeExporter commented 9 years ago 
Thanks for the discussion Dave. I'm closing this as won't fix. If it turns out 
you really do want a way of "normalising" call signatures, including defaults, 
I'd be open to adding this to autospec as an option.

Original comment by fuzzyman on 1 Apr 2012 at 11:36

GoogleCodeExporter commented 9 years ago

Original comment by fuzzyman on 1 Apr 2012 at 11:37