mirror of
				https://github.com/python/cpython.git
				synced 2025-10-31 13:41:24 +00:00 
			
		
		
		
	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.
This commit is contained in:
		
							parent
							
								
									42c9f0fd0a
								
							
						
					
					
						commit
						fdb9efce6a
					
				
					 3 changed files with 84 additions and 8 deletions
				
			
		|  | @ -633,8 +633,8 @@ def __getattr__(self, name): | ||||||
|         if not self._mock_unsafe: |         if not self._mock_unsafe: | ||||||
|             if name.startswith(('assert', 'assret', 'asert', 'aseert', 'assrt')): |             if name.startswith(('assert', 'assret', 'asert', 'aseert', 'assrt')): | ||||||
|                 raise AttributeError( |                 raise AttributeError( | ||||||
|                     f"{name} is not a valid assertion. Use a spec " |                     f"{name!r} is not a valid assertion. Use a spec " | ||||||
|                     f"for the mock if {name} is meant to be an attribute.") |                     f"for the mock if {name!r} is meant to be an attribute.") | ||||||
| 
 | 
 | ||||||
|         result = self._mock_children.get(name) |         result = self._mock_children.get(name) | ||||||
|         if result is _deleted: |         if result is _deleted: | ||||||
|  | @ -1242,6 +1242,17 @@ def _importer(target): | ||||||
|     return thing |     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): | class _patch(object): | ||||||
| 
 | 
 | ||||||
|     attribute_name = None |     attribute_name = None | ||||||
|  | @ -1249,7 +1260,7 @@ class _patch(object): | ||||||
| 
 | 
 | ||||||
|     def __init__( |     def __init__( | ||||||
|             self, getter, attribute, new, spec, create, |             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_callable is not None: | ||||||
|             if new is not DEFAULT: |             if new is not DEFAULT: | ||||||
|  | @ -1260,6 +1271,8 @@ def __init__( | ||||||
|                 raise ValueError( |                 raise ValueError( | ||||||
|                     "Cannot use 'autospec' and 'new_callable' together" |                     "Cannot use 'autospec' and 'new_callable' together" | ||||||
|                 ) |                 ) | ||||||
|  |         if not unsafe: | ||||||
|  |             _check_spec_arg_typos(kwargs) | ||||||
| 
 | 
 | ||||||
|         self.getter = getter |         self.getter = getter | ||||||
|         self.attribute = attribute |         self.attribute = attribute | ||||||
|  | @ -1569,7 +1582,7 @@ def _get_target(target): | ||||||
| def _patch_object( | def _patch_object( | ||||||
|         target, attribute, new=DEFAULT, spec=None, |         target, attribute, new=DEFAULT, spec=None, | ||||||
|         create=False, spec_set=None, autospec=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 |     patch the named member (`attribute`) on an object (`target`) with a mock | ||||||
|  | @ -1591,7 +1604,7 @@ def _patch_object( | ||||||
|     getter = lambda: target |     getter = lambda: target | ||||||
|     return _patch( |     return _patch( | ||||||
|         getter, attribute, new, spec, create, |         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( | def patch( | ||||||
|         target, new=DEFAULT, spec=None, create=False, |         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 |     `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 |     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. |     "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 |     `patch` takes arbitrary keyword arguments. These will be passed to | ||||||
|     `AsyncMock` if the patched object is asynchronous, to `MagicMock` |     `AsyncMock` if the patched object is asynchronous, to `MagicMock` | ||||||
|     otherwise or to `new_callable` if specified. |     otherwise or to `new_callable` if specified. | ||||||
|  | @ -1718,7 +1735,7 @@ def patch( | ||||||
|     getter, attribute = _get_target(target) |     getter, attribute = _get_target(target) | ||||||
|     return _patch( |     return _patch( | ||||||
|         getter, attribute, new, spec, create, |         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, | 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 |     """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 |     mock will use the corresponding attribute on the `spec` object as their | ||||||
|     spec. |     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 |     spec for an instance object by passing `instance=True`. The returned mock | ||||||
|     will only be callable if instances of the mock are callable. |     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 |     `create_autospec` also takes arbitrary keyword arguments that are passed to | ||||||
|     the constructor of the created mock.""" |     the constructor of the created mock.""" | ||||||
|     if _is_list(spec): |     if _is_list(spec): | ||||||
|  | @ -2601,6 +2622,8 @@ def create_autospec(spec, spec_set=False, instance=False, _parent=None, | ||||||
|         _kwargs = {} |         _kwargs = {} | ||||||
|     if _kwargs and instance: |     if _kwargs and instance: | ||||||
|         _kwargs['_spec_as_instance'] = True |         _kwargs['_spec_as_instance'] = True | ||||||
|  |     if not unsafe: | ||||||
|  |         _check_spec_arg_typos(kwargs) | ||||||
| 
 | 
 | ||||||
|     _kwargs.update(kwargs) |     _kwargs.update(kwargs) | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -38,6 +38,12 @@ def cmeth(cls, a, b, c, d=None): pass | ||||||
|     def smeth(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 | def something(a): pass | ||||||
| 
 | 
 | ||||||
| 
 | 
 | ||||||
|  | @ -2175,6 +2181,52 @@ def __init__(self): | ||||||
| 
 | 
 | ||||||
|         self.assertEqual(obj.obj_with_bool_func.__bool__.call_count, 0) |         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__': | if __name__ == '__main__': | ||||||
|     unittest.main() |     unittest.main() | ||||||
|  |  | ||||||
|  | @ -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. | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 vabr-g
						vabr-g