[Python-checkins] bpo-41877: Check for misspelled speccing arguments (GH-23737)

gpshead webhook-mailer at python.org
Mon Dec 14 13:30:17 EST 2020


https://github.com/python/cpython/commit/fdb9efce6ac211f973088eef508740c3fa2bd182
commit: fdb9efce6ac211f973088eef508740c3fa2bd182
branch: master
author: vabr-g <vabr at google.com>
committer: gpshead <greg at krypto.org>
date: 2020-12-14T10:30:09-08:00
summary:

bpo-41877: Check for misspelled speccing arguments (GH-23737)

patch, patch.object and create_autospec silently ignore misspelled
arguments such as autospect, auto_spec and set_spec. This can lead
to tests failing to check what they are supposed to check.

This change adds a check causing a RuntimeError if the above
functions get any of the above misspellings as arguments. It also
adds a new argument, "unsafe", which can be set to True to disable
this check.

Also add "!r" to format specifiers in added error messages.

files:
A Misc/NEWS.d/next/Library/2020-12-10-19-49-52.bpo-41877.wiVlPc.rst
M Lib/unittest/mock.py
M Lib/unittest/test/testmock/testmock.py

diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py
index d43ea9e23c899..720f682efbb54 100644
--- a/Lib/unittest/mock.py
+++ b/Lib/unittest/mock.py
@@ -633,8 +633,8 @@ def __getattr__(self, name):
         if not self._mock_unsafe:
             if name.startswith(('assert', 'assret', 'asert', 'aseert', 'assrt')):
                 raise AttributeError(
-                    f"{name} is not a valid assertion. Use a spec "
-                    f"for the mock if {name} is meant to be an attribute.")
+                    f"{name!r} is not a valid assertion. Use a spec "
+                    f"for the mock if {name!r} is meant to be an attribute.")
 
         result = self._mock_children.get(name)
         if result is _deleted:
@@ -1242,6 +1242,17 @@ def _importer(target):
     return thing
 
 
+# _check_spec_arg_typos takes kwargs from commands like patch and checks that
+# they don't contain common misspellings of arguments related to autospeccing.
+def _check_spec_arg_typos(kwargs_to_check):
+    typos = ("autospect", "auto_spec", "set_spec")
+    for typo in typos:
+        if typo in kwargs_to_check:
+            raise RuntimeError(
+                f"{typo!r} might be a typo; use unsafe=True if this is intended"
+            )
+
+
 class _patch(object):
 
     attribute_name = None
@@ -1249,7 +1260,7 @@ class _patch(object):
 
     def __init__(
             self, getter, attribute, new, spec, create,
-            spec_set, autospec, new_callable, kwargs
+            spec_set, autospec, new_callable, kwargs, *, unsafe=False
         ):
         if new_callable is not None:
             if new is not DEFAULT:
@@ -1260,6 +1271,8 @@ def __init__(
                 raise ValueError(
                     "Cannot use 'autospec' and 'new_callable' together"
                 )
+        if not unsafe:
+            _check_spec_arg_typos(kwargs)
 
         self.getter = getter
         self.attribute = attribute
@@ -1569,7 +1582,7 @@ def _get_target(target):
 def _patch_object(
         target, attribute, new=DEFAULT, spec=None,
         create=False, spec_set=None, autospec=None,
-        new_callable=None, **kwargs
+        new_callable=None, *, unsafe=False, **kwargs
     ):
     """
     patch the named member (`attribute`) on an object (`target`) with a mock
@@ -1591,7 +1604,7 @@ def _patch_object(
     getter = lambda: target
     return _patch(
         getter, attribute, new, spec, create,
-        spec_set, autospec, new_callable, kwargs
+        spec_set, autospec, new_callable, kwargs, unsafe=unsafe
     )
 
 
@@ -1646,7 +1659,7 @@ def _patch_multiple(target, spec=None, create=False, spec_set=None,
 
 def patch(
         target, new=DEFAULT, spec=None, create=False,
-        spec_set=None, autospec=None, new_callable=None, **kwargs
+        spec_set=None, autospec=None, new_callable=None, *, unsafe=False, **kwargs
     ):
     """
     `patch` acts as a function decorator, class decorator or a context
@@ -1708,6 +1721,10 @@ def patch(
     use "as" then the patched object will be bound to the name after the
     "as"; very useful if `patch` is creating a mock object for you.
 
+    Patch will raise a `RuntimeError` if passed some common misspellings of
+    the arguments autospec and spec_set. Pass the argument `unsafe` with the
+    value True to disable that check.
+
     `patch` takes arbitrary keyword arguments. These will be passed to
     `AsyncMock` if the patched object is asynchronous, to `MagicMock`
     otherwise or to `new_callable` if specified.
@@ -1718,7 +1735,7 @@ def patch(
     getter, attribute = _get_target(target)
     return _patch(
         getter, attribute, new, spec, create,
-        spec_set, autospec, new_callable, kwargs
+        spec_set, autospec, new_callable, kwargs, unsafe=unsafe
     )
 
 
@@ -2568,7 +2585,7 @@ def call_list(self):
 
 
 def create_autospec(spec, spec_set=False, instance=False, _parent=None,
-                    _name=None, **kwargs):
+                    _name=None, *, unsafe=False, **kwargs):
     """Create a mock object using another object as a spec. Attributes on the
     mock will use the corresponding attribute on the `spec` object as their
     spec.
@@ -2584,6 +2601,10 @@ def create_autospec(spec, spec_set=False, instance=False, _parent=None,
     spec for an instance object by passing `instance=True`. The returned mock
     will only be callable if instances of the mock are callable.
 
+    `create_autospec` will raise a `RuntimeError` if passed some common
+    misspellings of the arguments autospec and spec_set. Pass the argument
+    `unsafe` with the value True to disable that check.
+
     `create_autospec` also takes arbitrary keyword arguments that are passed to
     the constructor of the created mock."""
     if _is_list(spec):
@@ -2601,6 +2622,8 @@ def create_autospec(spec, spec_set=False, instance=False, _parent=None,
         _kwargs = {}
     if _kwargs and instance:
         _kwargs['_spec_as_instance'] = True
+    if not unsafe:
+        _check_spec_arg_typos(kwargs)
 
     _kwargs.update(kwargs)
 
diff --git a/Lib/unittest/test/testmock/testmock.py b/Lib/unittest/test/testmock/testmock.py
index 016905c3b90e5..e38f41e1d2152 100644
--- a/Lib/unittest/test/testmock/testmock.py
+++ b/Lib/unittest/test/testmock/testmock.py
@@ -38,6 +38,12 @@ def cmeth(cls, a, b, c, d=None): pass
     def smeth(a, b, c, d=None): pass
 
 
+class Typos():
+    autospect = None
+    auto_spec = None
+    set_spec = None
+
+
 def something(a): pass
 
 
@@ -2175,6 +2181,52 @@ def __init__(self):
 
         self.assertEqual(obj.obj_with_bool_func.__bool__.call_count, 0)
 
+    def test_misspelled_arguments(self):
+        class Foo():
+            one = 'one'
+        # patch, patch.object and create_autospec need to check for misspelled
+        # arguments explicitly and throw a RuntimError if found.
+        with self.assertRaises(RuntimeError):
+            with patch(f'{__name__}.Something.meth', autospect=True): pass
+        with self.assertRaises(RuntimeError):
+            with patch.object(Foo, 'one', autospect=True): pass
+        with self.assertRaises(RuntimeError):
+            with patch(f'{__name__}.Something.meth', auto_spec=True): pass
+        with self.assertRaises(RuntimeError):
+            with patch.object(Foo, 'one', auto_spec=True): pass
+        with self.assertRaises(RuntimeError):
+            with patch(f'{__name__}.Something.meth', set_spec=True): pass
+        with self.assertRaises(RuntimeError):
+            with patch.object(Foo, 'one', set_spec=True): pass
+        with self.assertRaises(RuntimeError):
+            m = create_autospec(Foo, set_spec=True)
+        # patch.multiple, on the other hand, should flag misspelled arguments
+        # through an AttributeError, when trying to find the keys from kwargs
+        # as attributes on the target.
+        with self.assertRaises(AttributeError):
+            with patch.multiple(
+                f'{__name__}.Something', meth=DEFAULT, autospect=True): pass
+        with self.assertRaises(AttributeError):
+            with patch.multiple(
+                f'{__name__}.Something', meth=DEFAULT, auto_spec=True): pass
+        with self.assertRaises(AttributeError):
+            with patch.multiple(
+                f'{__name__}.Something', meth=DEFAULT, set_spec=True): pass
+
+        with patch(f'{__name__}.Something.meth', unsafe=True, autospect=True):
+            pass
+        with patch.object(Foo, 'one', unsafe=True, autospect=True): pass
+        with patch(f'{__name__}.Something.meth', unsafe=True, auto_spec=True):
+            pass
+        with patch.object(Foo, 'one', unsafe=True, auto_spec=True): pass
+        with patch(f'{__name__}.Something.meth', unsafe=True, set_spec=True):
+            pass
+        with patch.object(Foo, 'one', unsafe=True, set_spec=True): pass
+        m = create_autospec(Foo, set_spec=True, unsafe=True)
+        with patch.multiple(
+            f'{__name__}.Typos', autospect=True, set_spec=True, auto_spec=True):
+            pass
+
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/Misc/NEWS.d/next/Library/2020-12-10-19-49-52.bpo-41877.wiVlPc.rst b/Misc/NEWS.d/next/Library/2020-12-10-19-49-52.bpo-41877.wiVlPc.rst
new file mode 100644
index 0000000000000..d42200ecd03de
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2020-12-10-19-49-52.bpo-41877.wiVlPc.rst
@@ -0,0 +1 @@
+A check is added against misspellings of autospect, auto_spec and set_spec being passed as arguments to patch, patch.object and create_autospec.
\ No newline at end of file



More information about the Python-checkins mailing list