Show detailed py.test assert introspect for call arguments

[asfaltboy]

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?

[nicoddemus]

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

[nicoddemus]

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?

[asfaltboy]

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.

[nicoddemus]

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.

[asfaltboy]

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

[nicoddemus]

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.

[asfaltboy]

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.

[blueyed]

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?!

[asfaltboy]

@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.

[blueyed]

adding -v where I want to have a deeper look is easy

Maybe -vv should also include the original exception's message?

[nicoddemus]

Just merged this, will deal with #44 in a separate PR. 😁

Thanks again @asfaltboy! 😄