diff options
author | Michael Foord <michael@voidspace.org.uk> | 2012-03-25 17:39:04 +0100 |
---|---|---|
committer | Michael Foord <michael@voidspace.org.uk> | 2012-03-25 17:39:04 +0100 |
commit | 7427b82ea700015d9305670425dd23989004498a (patch) | |
tree | 90c3371591ea5fd03e87d7d2ed7143ca5c91c39b | |
parent | 044b4079fe673e31a81ef9e18470bf30026f01cc (diff) | |
download | mock-7427b82ea700015d9305670425dd23989004498a.tar.gz |
Fix various bugs around spec arguments to patchers
-rw-r--r-- | docs/changelog.txt | 4 | ||||
-rw-r--r-- | docs/patch.txt | 6 | ||||
-rw-r--r-- | mock.py | 55 | ||||
-rw-r--r-- | tests/testpatch.py | 63 |
4 files changed, 102 insertions, 26 deletions
diff --git a/docs/changelog.txt b/docs/changelog.txt index 8e12639..4eb3fa0 100644 --- a/docs/changelog.txt +++ b/docs/changelog.txt @@ -20,8 +20,12 @@ The standard library version! methods (other than equality and inequality) now return `NotImplemented` * Propagate traceback info to support subclassing of `_patch` by other libraries +* BUGFIX: passing multiple spec arguments to patchers (`spec` , `spec_set` and + `autospec`) had unpredictable results, now it is an error * BUGFIX: using `spec=True` *and* `create=True` as arguments to patchers could result in using `DEFAULT` as the spec. Now it is an error instead +* BUGFIX: using `spec` or `autospec` arguments to patchers, along with + `spec_set=True` did not work correctly 2012/02/13 Version 0.8.0 diff --git a/docs/patch.txt b/docs/patch.txt index 190fa13..d27dbc5 100644 --- a/docs/patch.txt +++ b/docs/patch.txt @@ -40,7 +40,7 @@ patch `patch` is straightforward to use. The key is to do the patching in the right namespace. See the section `where to patch`_. -.. function:: patch(target, new=DEFAULT, spec=None, create=False, spec_set=None, autospec=False, new_callable=None, **kwargs) +.. function:: patch(target, new=DEFAULT, spec=None, create=False, spec_set=None, autospec=None, new_callable=None, **kwargs) `patch` acts as a function decorator, class decorator or a context manager. Inside the body of the function or with statement, the `target` @@ -208,7 +208,7 @@ into a `patch` call using `**`: patch.object ============ -.. function:: patch.object(target, attribute, new=DEFAULT, spec=None, create=False, spec_set=None, autospec=False, new_callable=None, **kwargs) +.. function:: patch.object(target, attribute, new=DEFAULT, spec=None, create=False, spec_set=None, autospec=None, new_callable=None, **kwargs) patch the named member (`attribute`) on an object (`target`) with a mock object. @@ -336,7 +336,7 @@ magic methods `__getitem__`, `__setitem__`, `__delitem__` and either patch.multiple ============== -.. function:: patch.multiple(target, spec=None, create=False, spec_set=None, autospec=False, new_callable=None, **kwargs) +.. function:: patch.multiple(target, spec=None, create=False, spec_set=None, autospec=None, new_callable=None, **kwargs) Perform multiple patches in a single call. It takes the object to be patched (either as an object or a string to fetch the object by importing) @@ -1089,7 +1089,7 @@ class _patch(object): raise ValueError( "Cannot use 'new' and 'new_callable' together" ) - if autospec is not False: + if autospec is not None: raise ValueError( "Cannot use 'autospec' and 'new_callable' together" ) @@ -1218,17 +1218,38 @@ class _patch(object): new_callable = self.new_callable self.target = self.getter() + # normalise False to None + if spec is False: + spec = None + if spec_set is False: + spec_set = None + if autospec is False: + autospec = None + + if spec is not None and autospec is not None: + raise TypeError("Can't specify spec and autospec") + if ((spec is not None or autospec is not None) and + spec_set not in (True, None)): + raise TypeError("Can't provide explicit spec_set *and* spec or autospec") + original, local = self.get_original() - if new is DEFAULT and autospec is False: + if new is DEFAULT and autospec is None: inherit = False - if spec_set == True: - spec_set = original - elif spec == True: + if spec is True: # set spec to the object we are replacing spec = original + if spec_set is True: + spec_set = original + spec = None + elif spec is not None: + if spec_set is True: + spec_set = spec + spec = None + elif spec_set is True: + spec_set = original - if (spec or spec_set) is not None: + if spec is not None or spec_set is not None: if original is DEFAULT: raise TypeError("Can't use 'spec' with create=True") if isinstance(original, ClassTypes): @@ -1259,14 +1280,17 @@ class _patch(object): if inherit and _is_instance_mock(new): # we can only tell if the instance should be callable if the # spec is not a list - if (not _is_list(spec or spec_set) and not - _instance_callable(spec or spec_set)): + this_spec = spec + if spec_set is not None: + this_spec = spec_set + if (not _is_list(this_spec) and not + _instance_callable(this_spec)): Klass = NonCallableMagicMock _kwargs.pop('name') new.return_value = Klass(_new_parent=new, _new_name='()', **_kwargs) - elif autospec is not False: + elif autospec is not None: # spec is ignored, new *must* be default, spec_set is treated # as a boolean. Should we check spec is not None and that spec_set # is a bool? @@ -1343,13 +1367,12 @@ def _get_target(target): def _patch_object( target, attribute, new=DEFAULT, spec=None, - create=False, spec_set=None, autospec=False, + create=False, spec_set=None, autospec=None, new_callable=None, **kwargs ): """ patch.object(target, attribute, new=DEFAULT, spec=None, create=False, - spec_set=None, autospec=False, - new_callable=None, **kwargs) + spec_set=None, autospec=None, new_callable=None, **kwargs) patch the named member (`attribute`) on an object (`target`) with a mock object. @@ -1370,9 +1393,8 @@ def _patch_object( ) -def _patch_multiple(target, spec=None, create=False, - spec_set=None, autospec=False, - new_callable=None, **kwargs +def _patch_multiple(target, spec=None, create=False, spec_set=None, + autospec=None, new_callable=None, **kwargs ): """Perform multiple patches in a single call. It takes the object to be patched (either as an object or a string to fetch the object by importing) @@ -1423,8 +1445,7 @@ def _patch_multiple(target, spec=None, create=False, def patch( target, new=DEFAULT, spec=None, create=False, - spec_set=None, autospec=False, - new_callable=None, **kwargs + spec_set=None, autospec=None, new_callable=None, **kwargs ): """ `patch` acts as a function decorator, class decorator or a context diff --git a/tests/testpatch.py b/tests/testpatch.py index d6ebec3..5f6a1b1 100644 --- a/tests/testpatch.py +++ b/tests/testpatch.py @@ -20,6 +20,7 @@ if inPy3k: unicode = str PTModule = sys.modules[__name__] +MODNAME = '%s.PTModule' % __name__ def _get_proxy(obj, get_only=True): @@ -687,13 +688,13 @@ class PatchTest(unittest2.TestCase): def test_patch_spec_set(self): - @patch('%s.SomeClass' % __name__, spec=SomeClass, spec_set=True) + @patch('%s.SomeClass' % __name__, spec_set=SomeClass) def test(MockClass): MockClass.z = 'foo' self.assertRaises(AttributeError, test) - @patch.object(support, 'SomeClass', spec=SomeClass, spec_set=True) + @patch.object(support, 'SomeClass', spec_set=SomeClass) def test(MockClass): MockClass.z = 'foo' @@ -1653,7 +1654,6 @@ class PatchTest(unittest2.TestCase): def test_patch_propogrates_exc_on_exit(self): - class holder: exc_info = None, None, None @@ -1666,8 +1666,8 @@ class PatchTest(unittest2.TestCase): def with_custom_patch(target): getter, attribute = _get_target(target) return custom_patch( - getter, attribute, DEFAULT, None, False, False, - False, None, {} + getter, attribute, DEFAULT, None, False, None, + None, None, {} ) @with_custom_patch('squizz.squozz') @@ -1690,7 +1690,7 @@ class PatchTest(unittest2.TestCase): self.assertRaises(NameError, lambda: doesnotexist) # check that spec with create is innocuous if the original exists - p = patch('%s.PTModule' % __name__, create=True, + p = patch(MODNAME, create=True, **{kwarg: True}) try: p.start() @@ -1698,5 +1698,56 @@ class PatchTest(unittest2.TestCase): p.stop() + def test_multiple_specs(self): + original = PTModule + for kwarg in ('spec', 'spec_set'): + p = patch(MODNAME, autospec=0, **{kwarg: 0}) + self.assertRaises(TypeError, p.start) + self.assertIs(PTModule, original) + + for kwarg in ('spec', 'autospec'): + p = patch(MODNAME, spec_set=0, **{kwarg: 0}) + self.assertRaises(TypeError, p.start) + self.assertIs(PTModule, original) + + for kwarg in ('spec_set', 'autospec'): + p = patch(MODNAME, spec=0, **{kwarg: 0}) + self.assertRaises(TypeError, p.start) + self.assertIs(PTModule, original) + + + def test_specs_false_instead_of_none(self): + p = patch(MODNAME, spec=False, spec_set=False, autospec=False) + mock = p.start() + try: + # no spec should have been set, so attribute access should not fail + mock.does_not_exist + mock.does_not_exist = 3 + finally: + p.stop() + + + def test_falsey_spec(self): + for kwarg in ('spec', 'autospec', 'spec_set'): + p = patch(MODNAME, **{kwarg: 0}) + m = p.start() + try: + self.assertRaises(AttributeError, getattr, m, 'doesnotexit') + finally: + p.stop() + + + def test_spec_set_true(self): + for kwarg in ('spec', 'autospec'): + p = patch(MODNAME, spec_set=True, **{kwarg: True}) + m = p.start() + try: + self.assertRaises(AttributeError, setattr, m, + 'doesnotexist', 'something') + self.assertRaises(AttributeError, getattr, m, 'doesnotexist') + finally: + p.stop() + + if __name__ == '__main__': unittest2.main() |