Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Uses assert_called_with instead of called_with #2375

Merged
merged 1 commit into from
Nov 3, 2023

Conversation

hiwakaba
Copy link
Contributor

@hiwakaba hiwakaba commented Jul 13, 2023

Hello, Please check my patch. In my environment of Python3.12, an error occurs at the following line. Here is a part of the error message.

>       assert conn.send.called_with(request)

test/test_client_async.py:223: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <MagicMock name='BrokerConnection.send' id='281473155611728'>
name = 'called_with'

    def __getattr__(self, name):
        if name in {'_mock_methods', '_mock_unsafe'}:
            raise AttributeError(name)
        elif self._mock_methods is not None:
            if name not in self._mock_methods or name in _all_magics:
                raise AttributeError("Mock object has no attribute %r" % name)
        elif _is_magic(name):
            raise AttributeError(name)
        if not self._mock_unsafe and (not self._mock_methods or name not in self._mock_methods):
            if name.startswith(('assert', 'assret', 'asert', 'aseert', 'assrt')) or name in _ATTRIB_DENY_LIST:
>               raise AttributeError(
                    f"{name!r} is not a valid assertion. Use a spec "
                    f"for the mock if {name!r} is meant to be an attribute.")
E               AttributeError: 'called_with' is not a valid assertion. Use a spec for the mock if 'called_with' is meant to be an attribute.

assert conn.send.called_with(request)

Two assertions exists at the above line. The first assertion called_with is called and then assert is called. At the first assertion, we should seem to use assert_called_with instead of called_with. Please see:
python/cpython#100690

Then, the arguments of assert_called_with should be request, blocking=False because KafkaClient internally calls send with them.
https://github.com/dpkp/kafka-python/blob/master/kafka/client_async.py#L539

The second assertion should be removed because assert raises an error when an assert statement fails and do nothing when the statement succeeds.

$ python3
Python 3.12.0b3 (main, Jun 21 2023, 00:00:00) [GCC 13.1.1 20230614 (Red Hat 13.1.1-4)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from unittest.mock import Mock,MagicMock
>>> req = Mock
>>> conn = Mock
>>> conn.send = MagicMock(return_value=True)
>>> conn.send(req)
True
>>> conn.send.assert_called_with(req)     <--- return nothing when assertion is a success.
>>> conn.send.assert_not_called()    <--- raise AssertionError only when assertion fails.
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib64/python3.12/unittest/mock.py", line 905, in assert_not_called
    raise AssertionError(msg)
AssertionError: Expected 'mock' to not have been called. Called 1 times.
Calls: [call(<class 'unittest.mock.Mock'>)].

Thanks in advance,
Hirotaka


This change is Reviewable

@wbarnha wbarnha self-requested a review August 3, 2023 03:48
@wbarnha
Copy link
Collaborator

wbarnha commented Aug 3, 2023

I'll revisit this as soon as #2378 is merged.

@wbarnha wbarnha merged commit 4861bee into dpkp:master Nov 3, 2023
@hiwakaba
Copy link
Contributor Author

hiwakaba commented Nov 3, 2023

@wbarnha Thanks for your works!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants