Show detailed py.test assert introspect for call arguments by asfaltboy · Pull Request #36 · pytest-dev/pytest-mock (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation11 Commits2 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Why do this
I really enjoy pytest's assertion introspection support, and find it invaluable when it comes to comparing large pieces of data. However, when asserting mock calls with large arguments, I found myself copy-pasting objects into deepdiff just to more easily see the difference.
Implementation
This may not be a complete solution, but it's the simplest I could think of to start discussing the point.
Among other improvements, we could also check --assert mode option, use a custom assertion comparison and raise_from original mock assertion.
Please let me know what you think about this feature?
Hey @asfaltboy thanks for the PR! I didn't have time yet to try it out in detail, I will do it until the end of the week
Hey @asfaltboy sorry for not replying earlier.
I like the idea in general, although I'm not sure I like the proposed UI. Perhaps we could just provide similar functions in the mocker fixture instead of a context manager?
def test_assert_called_args_with_introspection(mocker): stub = mocker.stub() mocker.assert_called_with(stub, *args, **kwargs) # or perhaps assert mocker.called_with(stub, *args, **kwargs)
What do you think?
The usage UI is basically unchanged; which is the whole idea of this patch. Instead of manually dissecting arguments and calling, for example, assert X == Y the users can now use mock's methods (namely assert_called_with and assert_called_once_with) and have the same nice diff as they expect when using py.test normally.
I'm sorry if my code is unclear, the context manager is used ONLY in own tests to test that the raised AssertionError includes a detailed diff (i.e introspection of objects). Honestly, I only used it because I saw the assert_traceback function, and wanted to conform to project style. The actual entry point of the feature is in the try/except clause that already happens in assert_wrapper.
If you prefer, we can probably as easily move the try/except clause around the yield in mocker fixture. Though, I rather like the idea of plugging in pytest-mock into an existing project (which already uses mock) and getting the same nice introspection with minimal work.
P.S: I was about to write this as a plugin for myself, but then I found your project and thought it fitting very much with the purpose of pytest-mock.
The actual entry point of the feature is in the try/except clause that already happens in assert_wrapper.
Oh of course, sorry about that... you are right, I misunderstood the patch.
I agree that it would be nice to have this in pytest-mock. Are you willing to continue working on the PR? From my point of view what's missing is docs and tests which cover all functions we are currently wrapping to make sure we are not breaking any of them by accident.
Sure, I can probably do some work during this weekend.
Current tests already call all of the mock helper functions (as part of the assert_traceback tests).
I can add the other calls in separate tests, but the feature doesn't add anything we can assert in these cases, so it feels a bit repetitive.
With regards to docs, I'm thinking to add a paragraph in (or under) the "improved reporting" section.
By the way, do you feel we should add an option to toggle off "advanced introspection"? Something like:
[pytest] mock_argument_introspection_monkeypatch = false
Current tests already call all of the mock helper functions (as part of the assert_traceback tests).
Oh you are right.
With regards to docs, I'm thinking to add a paragraph in (or under) the "improved reporting" section.
Sounds good.
By the way, do you feel we should add an option to toggle off "advanced introspection"?
Sounds good too. 😁
Thanks a lot for taking the time to work on this, I really appreciate it.
Sorry for the delay @nicoddemus. I've rebased over the current master and added docs. Unfortunately adding a toggle proved to be difficult to do cleanly and I keep running out of free time.
Please let me know if you want me to add any other changes.
I've just tried this, but am also seeing #44 with this (two tracebacks).
The diff with this is enhanced as expected though (with the second traceback).
However, the behavior changes: while you see the whole lists by default, you need to use -vv now to see the whole diff, which is still different from seeing the whole lists (which are easier to copy).
Given that it's actually helpful to see the original AssertionError with that information, but if I understand it correctly it should not be visible?!
@blueyed you are right of course, the behavior is different from original mock output. Personally, I find the enhanced output of py.test to better suit my needs, adding -v where I want to have a deeper look is easy. However, it would probably be best to make the feature toggleable for existing users, if only I had more time.
About #44, I'll leave a comment there.
adding -v where I want to have a deeper look is easy
Maybe -vv should also include the original exception's message?
nicoddemus added a commit that referenced this pull request
Just merged this, will deal with #44 in a separate PR. 😁
Thanks again @asfaltboy! 😄
asfaltboy added a commit to asfaltboy/pytest-mock that referenced this pull request
- assert both args and kwargs at the same time for a more detailed report
- update tests to assert the above
This improves on PR pytest-dev#36 according to the points raised in PR pytest-dev#57
This was referenced
Dec 13, 2016
This was referenced
Feb 12, 2018