[TIP] Dilemma with using mock objects
Tom Davis
tom at recursivedream.com
Tue Apr 24 11:46:54 PDT 2012
On Tue, 24 Apr 2012 12:24:24 -0400, John Wong wrote:
> Dear Tom and Herman,
>
> Thank you guys for the genuine responses. They are helpful.
>
> First, Herman:
>
> >In a script like the one you describe, I would try to group all the
> > side-effecty IO related code into a single place and isolate it as
> > much as possible from the logic of the rest of the program. That way,
> > if you provide a single entry point or a wrapper for the IO code, you
> > will be able to easily test the flow of the program and not have to
> > worry about complicated mocking of built-in functions.
>
> I am guessing you want me to create a small function that handles the
> logics inside the for loop (or iteritems())?
>
>
> > for src, dest in src_dest:
> > if os.path.exists(src):
> > if os.path.exists(dest):
> > shutil.rmtree(dest)
> > shutil.move(src, dest) # either way we will proceed to move
> > else:
> > return (False, '%s does not exist!' % src)
> > return (True, 'Success!')
>
> By skipping, yes, I want the "else" clause of `if os.path.exists(dest)`
> (meaning, it doesn't exist yet). By mocking "os.path.exists" to be True, it
> probably will make it True throughout. In that one special test, how can I
> separate the patch? How can I stop the patch at a particular point?
Oh, I see. Like you want three to be True then the fourth False. You can just
use side_effect for this and implement an inner function, say, that increments a
counter. Here's an example:
@mock.patch('time.time')
def test_something(self, mock_time):
def side_effect():
# Setup a side effect to let the first call to time.time() be 0 and
# the next be larger, tripping the break point
if mock_time.call_count == 1:
return 0
return sys.maxint
mock_time.side_effect = side_effect
self.assertFalse(my_thing())
Now, in the real code for this, my_thing() calls time.time() and does something
based on its value in a loop. Basically, when enough time passes, it breaks out
and informs the caller all work wasn't completed. Here I'm giving it one pass of
normal operation then making it think we're a zillion years in the future.
>
> > Finally, instead of using the exception system you are
> > bulk-catching exceptions and returning error indicators instead. As you
> > mentioned above, this gives you the annoying requirement of figuring out
> every
> > possible exception instead of leaving it to the caller to properly handle
> them.
>
> I don't want to raise any exception, that's why I returns a tuple (False,
> e). As a web app, rasie exception is bad. I figure it's a good idea to just
> return False, along with the exception indicator. It's up to the user (in
> this case, at the moment, me) to log that exception, and return nice error
> message.
There's nothing wrong with raising exceptions at all. Presumably, some view code
or something calls your function, right? That's where these exceptions should be
caught, logged, etc. At least in my opinion. Returning tuples of style
`(result, errno)` is more of a C-ism than a Python-ism.
For instance, what happens if you add another error condition to this? Right now
the only time False is returned is when the path doesn't exist, but that
certainly won't be the only error condition (permissions are another common
one). Now "False" is no longer sufficient and you need to start parsing error
message strings! That's bad. Or you invent error codes, in which case you might
as well just raise exceptions to begin with.
>
> I think you are referring to `side_effect` which I think I need to
> explicitly supply the actual exception, right? Is there a more general way
> to do the side effect (I wouldn't know all the exceptions that the method
> is capable of generating)?
>
> I think I can do this
>
> import unittest
> from shutil import Error
> import shutil
> from mock import patch
>
> class TestSequenceFunctions(unittest.TestCase):
> @patch.object(shutil, 'move')
> def test2(self, mocker):
> import shutil
> mocker.side_effect = Error
> # mocker.side_effect = Exception <--- this is fine too
> src = '/home/ted/tester/1'
> dest = '/home/ted/tester/2'
> #result = shutil.move(src, dest)
> #mocker.assert_called_with(src, dest)
> self.assertRaises(Error, mocker, src, dest) # we can assert
> Exception instead of Error as well
>
> So what's the downside of always using `Exception` over explicit exception
> class such as shutill.Error? My implementation catches `Exception` so I
> feel like I've generalised it (Error will be caught as well).
There isn't a downside, really. Unless you handle different exceptions
differently internally. But this test doesn't really test your code: it just
tests the mock library and Python's ability to propagate an exception up the
stack.
This is a very common misstep when one's new to testing. You end up testing a
bunch of stuff that validates language constructs rather than personal
code. This test is particularly strange in that you are calling `shutil.move()`
directly via `self.assertRaises()`, meaning you are strictly testing the mock
library and nothing else.
A test such as this:
with patch('mymodule.shutil.move') as shutil:
shutil.side_effect = Error
self.assertRaises(Error, my_func, src, dest)
Actually runs your function, but again, is misleading: it simply asserts you
aren't catching Error in your code (which you shouldn't be, but needn't test for
either, unless it's uniquely important to the understanding of the API or is
protecting against some unfortunate future where you naively catch Error).
>
>
> Last, but not the least, mock objects supposed to mimic all the real thing,
> but it doesn't actually catch exception unless we supply a side-effect? I
> tried to minix `shutil.move(src, dest)` with dest being a directory that
> already exists. The real object will throw exception because dest must not
> already exists.
A mock is a replacement for the thing you're patching that looks like it
(optionally enforcing its API, though not by default); it does not actually
execute any of the same code. If you need to simulate a situation where `dest`
already exists, you would need to patch `shutil.move` and give its mock the
appropriate `side_effect`.
>
>
> Thanks.
>
> Ted
>
>
>
>
> On Mon, Apr 23, 2012 at 10:47 PM, Tom Davis <tom at recursivedream.com> wrote:
>
> > On Mon, 23 Apr 2012 18:24:49 -0400, Tu Ted wrote:
> > > Please bear with me. Suppose I want to assert that result is in a
> > > particular format, I might set the return value to something as (True,
> > > 'Success')
> > >
> > > Sample code: http://pastebin.com/VraxaKjL
> > >
> > > def batch_move(self, *args, **kwargs):
> > > '''
> > > Move a batch of files to its respective destinations.
> > > Return type: tuple (boolean, message)
> > > T/F , 'string' / str(exception)
> > > '''
> > >
> > > srcs= kwargs.get('srcs', None)
> > > dests = kwargs.get('dests', None)
> > >
> > > try:
> > > if srcs and dests:
> > > # map srcs and dests into dictionary (srcs --> keys, dests -->
> > > values)
> > > src_dest= dict(zip(srcs, dests))
> > >
> > > for src, dest in src_dest:
> > > if os.path.exists(src):
> > > if os.path.exists(dest):
> > > shutil.rmtree(dest)
> > > shutil.move(src, dest) # either way we will proceed to move
> > > else:
> > > return (False, '%s does not exist!' % src)
> > > return (True, 'Success!')
> > >
> > > else:
> > > return (False, 'Something gone wrong with those kwargs...')
> > > except Exception as e:
> > > return (False, e)
> >
> > I think you probably need src_dest.iteritems(), for starters...
> >
> > >
> > > In order to get to return (True, 'Success!'), I have
> > >
> > > 1. patch os.path.exists with True as return value. But in one unittest I
> > > want to skip this, how do I patch that os.path.exists?
> > > if os.path.exists(dest): # I want to skip this
> > > shutil.rmtree(dest)
> > >
> >
> > If by "skip" you mean go to the "else" block, you could just configure the
> > mock
> > to return False instead. If by "skip" you mean you want the actual
> > function to
> > be called, just don't mock that test (if using a decorator) or stop the
> > patch
> > (if patching manually).
> >
> > >
> > >
> > > 2. How do I patch shutil.move(src, dest)? Do I just give True so it
> > doesn't
> > > generate error? What if I want the case it fails and caught an exception?
> > > How do I simulate that? (I wouldn't always know which exception to catch,
> > > the primarily reason to use Exception as e).
> >
> > You should test every case, both success and failure. You can consult the
> > documentation or the source for `shutil.move` in order to write tests for
> > each
> > failure case. In the case that neither of these are completely helpful,
> > just try
> > different scenarios and see what happens. Why would moves fail? Destination
> > exists, insufficient permissions, etc.
> >
> > >
> > > 3. If I actually pass the function, does it really mean no exception
> > caught
> > > and it went through every single line? Or is it because I set
> > > `mock_object.return_value = (True, 'Success!')?
> >
> > A library like coverage.py will tell you that more easily, but if all you
> > did
> > was mock exists() and move(), then you could get every other line to run
> > (presuming you had True/False cases that ultimately cover every code path).
> >
> > Now, there are a number of ways you're making your life more difficult by
> > the
> > way this function is written. First, it has to validate its arguments
> > manually;
> > that adds another code path (bad kwargs). Second, it couples the success
> > of the
> > function with the presentation of its failure; this could lead you to test
> > oft-altered string values (which probably have no place in this function to
> > begin with). Finally, instead of using the exception system you are
> > bulk-catching exceptions and returning error indicators instead. As you
> > mentioned above, this gives you the annoying requirement of figuring out
> > every
> > possible exception instead of leaving it to the caller to properly handle
> > them.
> >
> > >
> > > 4. I am only using two dependencies here, do I need to patch out all the
> > > external dependencies such as (os, sys, math, datetime) all in one? Or if
> > > my function is using other functions (which are refactored)
> > >
> > >
> > > def f1(*args, **kwargs):
> > > f2(..) # use math, plot, datetime
> > > f3(..) # use math and datetime
> > > f4(..) # use datetime
> >
> > You only need to patch that which you do not want to run as normal. In the
> > example you gave, you should technically mock calls to each of f2(), f3(),
> > and
> > f4(), simply asserting on the values passed to them. This, of course,
> > presumes
> > you have written tests for each of these functions elsewhere, thus do not
> > need
> > to test them indirectly a second time.
> >
> > >
> > >
> > > Thanks. Sorry for the long questions. I really want to be good at writing
> > > unittests.
> >
> > An excellent goal to those on this list, I am sure ;)
> >
> > Hopefully my answers were somewhat useful...
> >
> > >
> > >
> > > --
> > > Ted
> >
> > > _______________________________________________
> > > testing-in-python mailing list
> > > testing-in-python at lists.idyll.org
> > > http://lists.idyll.org/listinfo/testing-in-python
> >
> >
> > _______________________________________________
> > testing-in-python mailing list
> > testing-in-python at lists.idyll.org
> > http://lists.idyll.org/listinfo/testing-in-python
> >
More information about the testing-in-python
mailing list