While your point about the risks of patching destructive calls is valid, if you're going to decry the practice of using mocks in tests, at least decry a version which uses them properly. In your first example you are patching the wrong module - you shouldn't patch os._exit (with potentially non-local effects), you should patch the module under test so that *in that module only*, the reference "os._exit" resolves to your patched function.The version of patching he suggests is definitely a lot better than my initial example, so let's take a look. First, the module we're going to test:
Most functions under test *aren't* destructive (so you'll get the expected test result failure), and by jumping straight to dependency injection in cases where you don't need it, you can end up adding a huge amount of complexity to your production code without adequate reason *and* give yourself additional code paths to test in the process. Dependency injection should be used only if there is a *production* related reason for adding it (and "this function is destructive, so we should use dependency injection rather than mocking to test it" is a valid reason).
For those non-destructive cases, you can avoid most of the non-local effects without adding complexity to the production code by localising your mock operation to as narrow a target as possible.
And now the patch-based tests, based on example code from Nick Coghlan (any mistakes were added by me):
import os def exit_with_result(function): result = function() if result: os._exit(0) else: os._exit(1)
This version is definitely a superior form of patching: only one module's view is impacted. Notice also the use of
import unittest # Note that we don't import os, because we're not touching it! import exitersketch class FakeOS: EXIT_NOT_CALLED = object() CALLED_WITH_DEFAULT = object() def __init__(self, module): self.module = module self.exit_code = self.EXIT_NOT_CALLED def _exit(self, code=CALLED_WITH_DEFAULT): self.exit_code = code def __getattr__(self, attr): return getattr(self.original_os, attr) def __enter__(self): self.original_os = self.module.os self.module.os = self return self def __exit__(self, *args): self.module.os = self.original_os class ExiterTests(unittest.TestCase): def test_exiter_success(self): with FakeOS(exitersketch) as fake: exitersketch.exit_with_result(lambda: True) self.assertEqual(fake.exit_code, 0) def test_exiter_failure(self): with FakeOS(exitersketch) as fake: exitersketch.exit_with_result(lambda: False) self.assertEqual(fake.exit_code, 1) if __name__ == '__main__': unittest.main()
__getattr__to ensure overriding
os._exitand not other parts of the
osmodule. Nonetheless, it still suffers from the inherent problem of patching: it's overriding more state than necessary. Thus it's still possible to call destructive functions by mistake if you rearrange your imports. For non-destructive functions it's still possible to have a test unexpectedly call a patched function, albeit only from code in the same module rather than globally. If you are going to use patching, though, making the patching as local as possible is definitely the way to go.