Mock with spec object does not ensure method call signatures #74772

Activity

1. bclau commented on Jun 7, 2017

   

   bclaumannequin

   on Jun 7, 2017

   MannequinAuthor

   More actions

   Mock can accept a spec object / class as argument, making sure that accessing attributes that do not exist in the spec will cause an AttributeError to be raised, but there is no guarantee that the spec's methods signatures are respected in any way. This creates the possibility to have faulty code with passing unittests and assertions.

   Steps to reproduce:

   >>> import mock
   >>>
   >>> class Something(object):
   ...     def foo(self, a, b, c, d):
   ...         pass
   ...
   >>>
   >>> m = mock.Mock(spec=Something)
   >>> m.foo()
   <Mock name='mock.foo()' id='140286904787240'>
   >>> # TypeError should be raised, but it isn't.
   ...
   >>> m.foo.assert_called_once_with()

   Expected behaviour: It should have raised a TypeError, since the method signature is: def meth(self, a, b, c, d):

   Actual behaviour: No error.
2. bclau

   added

   3.7 (EOL)end of lifeend of life

   stdlibStandard Library Python modules in the Lib/ directoryStandard Library Python modules in the Lib/ directory

   type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

   on Jun 7, 2017
3. tirkarthi

   added

   3.8 (EOL)end of lifeend of life

   and removed

   3.7 (EOL)end of lifeend of life

   on Mar 31, 2019
4. tirkarthi commented on Mar 31, 2019

   

   tirkarthi

   on Mar 31, 2019

   Member

   More actions

   I am slightly concerned if spec should gain more responsibility than just validating attribute access which is mentioned in the docs. With the linked PR spec below would also validate the signature which is not done in Python 3.7 so this might break code for someone who only wants to validate access attribute access and not signature of the methods. It seems the PR also adds autospec argument to Mock that is currently not supported though spec and spec_set are supported.

   from unittest import mock
   
   def foo(lish):
       pass
   
   mock_foo = mock.Mock(spec=foo)
   mock_foo(1, 2)

   try:
   mock_foo.non_existent
   except AttributeError:
   print("raises AttributeError for non-existent attribute")

   # 3.7

   ➜ cpython git:(pr_1982) python3.7 /tmp/foo.py
   raises AttributeError for non-existent attribute

   # With PR

   ➜  cpython git:(pr_1982) ./python.exe /tmp/foo.py
   Traceback (most recent call last):
     File "/tmp/foo.py", line 7, in <module>
       mock_foo(1, 2)
     File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/unittest/mock.py", line 1009, in __call__
       _mock_self._mock_check_sig(*args, **kwargs)
     File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/unittest/mock.py", line 103, in checksig
       sig.bind(*args, **kwargs)
     File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/inspect.py", line 3016, in bind
       return args[0]._bind(args[1:], kwargs)
     File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/inspect.py", line 2937, in _bind
       raise TypeError('too many positional arguments') from None
   TypeError: too many positional arguments
5. voidspace commented on Apr 1, 2019

   

   voidspace

   on Apr 1, 2019

   Contributor

   More actions

   Spec objects are currently dumb. It would be a new feature to add signature validation to them.

   I think it would be a useful feature though as currently autospec sort of obsoletes spec objects whilst being more heavyweight and harder to use.

   I think it would appropriate to add to 3.8 but not to 3.7.

   Compatibility with existing tests is the issue. The obvious answer is a flag to turn it on/off but that adds complexity to the API.

   My gut feeling is that spec objects are far more commonly used in situations where validation would be useful than a detriment.

   > On 31 Mar 2019, at 18:07, Karthikeyan Singaravelan <report@bugs.python.org> wrote:
   > 
   > 
   > Karthikeyan Singaravelan <tir.karthi@gmail.com> added the comment:
   > 
   > I am slightly concerned if spec should gain more responsibility than just validating attribute access which is mentioned in the docs. With the linked PR spec below would also validate the signature which is not done in Python 3.7 so this might break code for someone who only wants to validate access attribute access and not signature of the methods. It seems the PR also adds autospec argument to Mock that is currently not supported though spec and spec_set are supported.
   > 
   > from unittest import mock
   > 
   > def foo(lish):
   >    pass
   > 
   > mock_foo = mock.Mock(spec=foo)
   > mock_foo(1, 2)
   > 
   > try:
   >    mock_foo.non_existent
   > except AttributeError:
   >    print("raises AttributeError for non-existent attribute")
   > 
   > # 3.7
   > 
   > ➜  cpython git:(pr_1982) python3.7 /tmp/foo.py
   > raises AttributeError for non-existent attribute
   > 
   > # With PR
   > 
   > ➜  cpython git:(pr_1982) ./python.exe /tmp/foo.py
   > Traceback (most recent call last):
   >  File "/tmp/foo.py", line 7, in <module>
   >    mock_foo(1, 2)
   >  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/unittest/mock.py", line 1009, in __call__
   >    _mock_self._mock_check_sig(*args, **kwargs)
   >  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/unittest/mock.py", line 103, in checksig
   >    sig.bind(*args, **kwargs)
   >  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/inspect.py", line 3016, in bind
   >    return args[0]._bind(args[1:], kwargs)
   >  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/inspect.py", line 2937, in _bind
   >    raise TypeError('too many positional arguments') from None
   > TypeError: too many positional arguments
   > 
   > 

   ---

   ---

   Python tracker <report@bugs.python.org>
   <https://bugs.python.org/issue30587\>

   ---
6. voidspace commented on Sep 10, 2019

   

   voidspace

   on Sep 10, 2019

   Contributor

   More actions

   I'd like spec to have signature validation. I don't think the use case for "attribute validation only" (current spec behaviour) is very strong and I'd rather not add new keywords. The mock API is already too complex.

   I'll take the existing PR and modify it to use spec instead of adding the new autospec.
7. voidspace commented on Sep 10, 2019

   

   voidspace

   on Sep 10, 2019

   Contributor

   More actions

   This will affect spec and spec_set.
8. ezio-melotti

   transferred this issue fromon Apr 10, 2022
9. serhiy-storchaka

   added this to Unittest & doctest issueson May 20, 2022
10. picnixz

    removed

    3.8 (EOL)end of lifeend of life

    on Jul 19, 2026