-
-
Notifications
You must be signed in to change notification settings - Fork 32.7k
bpo-35500: align expected and actual calls on mock.assert_called_with error message. #11804
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
Conversation
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. You can check yourself to see if the CLA has been received. Thanks again for your contribution, we look forward to reviewing it! |
Misc/NEWS.d/next/Library/2019-02-10-00-00-13.bpo-35500.1HOMmo.rst
Outdated
Show resolved
Hide resolved
Co-Authored-By: suhearsawho <susansu.software@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, but I'm still waiting for the issue creator to respond to the suggestions to replace the word "actual" with something longer to achieve vertical alignment between the "Expected" and "Actual" lines.
@taleinat @suhearsawho I just responded here : https://bugs.python.org/issue35500#msg335143. I don't really have a strong opinion on the wording and it's a matter of personal preference of mine on using "Expected" and "Actual". It would be a good improvement with the error message on separate lines itself. So I would be happy to see this landing even with a different wording. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM as per https://bugs.python.org/issue35500#msg335145. Please sign the CLA.
Minor nit : You might want to use the below format for hyperlinking to assert_called_with
method docs.
Write expected and actual call parameters on separate lines in :meth:`unittest.mock.Mock.assert_called_with` assertion errors. Contributed by Susan Su.
Thanks @suhearsawho for your contribution.
Changed format for hyperlinking to assert_called_with method (Suggestion from @tirkarthi).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Just waiting for approval of CLA signing.
CLA has been approved. |
… when self.call_args is None. This commit aims to create consistency with other case in mock.assert_called_with
Updated the code for the case when self.call_args is None. I declared an actual variable in the mock.py file and testmock.py file to maintain consistency with the rest of the code. Please let me know if there is anything I should revise! |
Looks great! Thank you @suhearsawho for your first contribution 😃 |
This patch adds an enhancement to the unittest module by adding a newline between the Assertion Error and Expected fields of the error messages generated by mock.assert_called_with. With this addition, the output is clearer as it is easier to read.
https://bugs.python.org/issue35500