diff --git a/docs/changelog.txt b/docs/changelog.txt
index 8e12639d8f7e80eac5b2dc8e69ad5835f82c7c54..4eb3fa05f8439a8a134c277d023433d38711ce45 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 190fa13a6150837c70cd32aef482d620865fd155..d27dbc54167861ecb95ad0c823fb0e4af477dea4 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)
diff --git a/mock.py b/mock.py
index 75aaf736f2c57ad14437353cf6b47172afddfabc..e852bcfeafeb4ac14355fdea291c664e41150eae 100644
--- a/mock.py
+++ b/mock.py
@@ -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 d6ebec3aa1d8712c41a932a1fc23de8fcd9b0732..5f6a1b1cdff66d125196749fd4e89d0d5bc4697b 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()