Thursday 16 December 2010

unittest.py quirk

Now I am using Python 2.7, and whatever form of unittest module that comes with it. Maybe this is fixed over in the (still not relevant to me) 3.x branches. Anyway, oftentimes I will create some simple mocking in the setUp method of my test case, and then the test method will error in a way that is unrelated to my mocking. But error handling of tests happens before tearDown is called and it will collide with my mocking, so instead of finding out about what test failed I find out about this collision.

It goes something like this.

class MyTESTSSSS(unittest.TestCase):
def setUp(self):
self.oldOpen = open
__builtin__.open = ReplacementOpenFunc

def testSomething(self):
1/0
Of course, that may not reproduce the problem, it's just for the purpose of illustration. Anyway, what's my point.. oh, in order to work around this, I now have to structure my tests in a way that enforces pre-error teardown.
class MyTESTSSSS(unittest.TestCase):
def setUp(self):
self.oldOpen = open
__builtin__.open = ReplacementOpenFunc

def preErrorTearDown(self):
__builtin__.open = self.oldOpen

def testSomething(self):
try:
self._testSomething()
finally:
self.preErrorTearDown()

def _testSomething(self):
1/0
Anyone else encounter this problem and take any different approaches to recovering from it?

If the answer involves a mocking framework, decorators, or context managers, then it is the wrong answer for me :-)

14 comments:

  1. I am confused; you do not have a tearDown() method of any sort in the first example, so I am not sure what you mean by saying that it collides with your test. Could you add a tearDown() method that "collides" so that we can see it?

    ReplyDelete
  2. Monkey-patching __builtin__.open is not a good idea; most people would patch yourcodemodule.open instead, or have an explicit hook for tests.

    Not cleaning it up at all (there's no tearDown in your first example) is a *terrible* idea.

    I must be missing the point of your post. Did you mean to say that unittest itself calls open() if a test fails, and does that before running your tearDown, that you accidentally omitted in your first example?

    (That would be one of the reasons why monkey-patching __builtins__ isn't a good idea.)

    ReplyDelete
  3. The error handling collides with the mocking, the tearDown method never gets reached. To be specific, the construction of the traceback does file access in the linecache module. This finds my open() function and ... there are errors in my errors :-)

    ReplyDelete
  4. In a sufficiently complicated framework, monkey-patching __builtins__ is a necessity. There is simply no way around it. And to restructure the framework to fit a unit test oriented notion of rightness, is not a reasonable option.

    ReplyDelete
  5. You can actually avoid patching __builtin__.open - simply patch open in the namespaces it is used rather than globally.

    I *know* you said you didn't want a mocking framework, but your use case would be so much easier with 'patch' from the mock library (even if you stick with patching __builtin__). I posted a couple of examples of mocking open to my blog:

    http://www.voidspace.org.uk/python/weblog/arch_d7_2010_10_02.shtml#e1188

    Note that patching in a setUp function isn't entirely safe because if an error is thrown in a setUp it is never undone. (Even if you had a tearDown it isn't called if there is an error during a setUp.)

    As far as I can tell, what you are saying is that your monkey patching is getting in the way of error / failure handling. This is a problem with the way you do your patching obviously - ("doctor it hurts when I globally patch __builtin__.open") but (speaking as maintainer of unittest in the Python standard library) there is really very little that happens in between a test failing and the tearDown being called so it is hard to tell exactly what the problem is in your case. (Hmmm... on re-reading you do mention that it is the construction of the traceback that causes the problem. So yup - you can't globally patch open like that without undoing it before a stacktrace has to be created.)

    Really, using the patch decorator would solve that in a lot neater way than the workarounds you're putting in place. It would undo the patching automatically for you immediately the test exits, even if it is due to failure or error. mock puts very little requirements on you as to how you write your tests - it doesn't have record, replay and verify or any of that - so it is really a library rather than a framework.

    ReplyDelete
  6. Richard, why should it be necessary? I think you ought to reconsider your design decisions.

    If you're using proper dependency injection techniques, it should not, and you could just feed a mock file object to the instance you're testing.

    open() monkeypatching could be a workaround for 3rd party libraries you don't control, but you shouldn't rely on them for testing your code - it's a smell!

    ReplyDelete
  7. Sorry to Hans, whose comment I deleted because I am clumsy. But he said: "I would suggest that enhancing a project's testability is exactly the sort of thing a framework should do."

    ReplyDelete
  8. Michael, your comment provides lots of interesting detail. I've written mocking libraries before, and they're not that hard to write. My current preference now is to write as simple and straightforward code as possible - and to me enciphering my logic with the arbitrary structure of other libraries and frameworks no matter how well designed or sensible, moves away from that goal.

    If the price to pay, is the odd clash of the unit test error mechanic with the mocked built-ins, then that is barely an inconvenience.

    ReplyDelete
  9. Hans and Alan, once I would have agreed with you both. But nowadays I find allusions to helpful sounding notions like "enhancing my project's testability" and "smells" and "reconsidering design decisions" to be more idealistic than realistic. Different people like different smells. If the cost of a valuable and interesting design decision is that standard approaches to testing cannot be taken - are the non-standard approaches actually worse for simply being so?

    ReplyDelete
  10. Would you provide a real-world example of a class using open that you need to test? Then I could either find out a solution, or say you're right :-)

    ReplyDelete
  11. I don't understand how dependency injection is appropriate. 'Open' is a stdlib built-in, that's available everywhere. Am I misunderstanding, or is Alan advocating that things like this (and, presumably, 'int', etc) should be passed in to every function that uses them? Or that relying on anything from the built-in top level namespace is to be avoided?

    Btw, for what it's worth, I wholeheartedly second Michael's recommendation to use his 'mock' library. It's very lightweight and unintrusive and extremely handy for situations like this. I couldn't bear writing tests without it.

    ReplyDelete
  12. Well, the point of mock (and patch) is that it makes writing code like you show above much *simpler to write*. With the added benefit that it doesn't have the painful drawbacks. Equivalent code using mock.path would be:

    from mock import patch
    ...
    @patch('__builtin__.open')
    def test_something(self, mock_open):
    ...

    No fragility and the mock is created for you! (If you're using Python 2.6+ then you can use patch as a class decorator to automatically decorate all test methods in a class if that is what you want.)

    In an effort to make things simple you seem determined to make them more complex... :-)


    Note that even if you decide not to use mock you can still make your code much nicer to read (and solve your problem) if you put your patching and unpatching in a decorator and decorate your tests.

    def patch_open(func):
    def inner(self):
    self.origOpen = open
    __builtin__.open = MockOpen
    try:
    return func(self)
    finally:
    __builtin__.open = MockOpen
    return inner

    This is effectively what mock.patch does for you; with additional smarts and it will create a mock object for you that you can configure and make assertions on.

    ReplyDelete
  13. Hmm... all whitespace gone and of course undoing the patch should be:

    __builtin__.open = self.origOpen

    ReplyDelete
  14. Tartley:
    I'm not advocating anything like that, but I need an example with *your* to show you what's the problem with that.

    I suppose the code is violating the SRP, mixing too many concerns in a single class; this forces the monkeypatching to create a test.

    ReplyDelete