[RFC] assert: keep "where … and …" output in verbose mode #5933

blueyed · 2019-10-09T02:52:35Z

Ref: pytest-dev#5932 Fixes: pytest-dev#5192

blueyed · 2019-10-09T02:54:43Z

This is quite hacky/involved.
I think it might be better to replace something like "~%(default)" instead, which would avoid having to pass through the original repl.
OTOH it might be useful in custom hooks.

blueyed · 2019-10-09T02:58:52Z

/cc @darrenburns via https://github.com/darrenburns/pytest-clarity (#5931)
I guess your feedback would be helpful here.. :)

JBKahn · 2019-10-09T13:30:40Z

Changing pytest-clarity to use return expl + "\n~" + "\n~".join([utf8_replace(line) for line in output]) like in your example is a huge improvement.

======================================================================= FAILURES =======================================================================
________________________________________________________________________ test_a ________________________________________________________________________

    def test_a():
>       assert len(a) == len(b)
E       assert 3 == 2
E        +  where 3 = len([1, 2, 3])
E        +  and   2 = len([1, 2])
E         left == right failed.
E         Showing split diff:
E
E         left:  3
E         right: 2

test_file.py:5: AssertionError
________________________________________________________________ test_bigger_assertion _________________________________________________________________

    def test_bigger_assertion():
        first_dict = {a: a for a in range(100)}
        second_dict = {b: b for b in range(1, 101)}
>       assert first_dict == second_dict
E       assert {0: 0, 1: 1, 2: 2, 3: 3, ...} == {1: 1, 2: 2, 3: 3, 4: 4, ...}
E         left == right failed.
E         Showing unified diff (L=left, R=right):
E
E          L {0: 0,
E          L  1: 1,
E          R {1: 1,
E            2: 2,
…
E            97: 97,
E            98: 98,
E          L  99: 99}
E          R  99: 99,
E          R  100: 100}
E
E         1 items in left, but not right:
E         + (0, 0)
E         1 items in right, but not left:
E         - (100, 100)

test_file.py:10: AssertionError
================================================================== 2 failed in 0.05s ===================================================================

darrenburns · 2019-10-09T19:08:55Z

This would be really cool! I personally prefer this approach over the templating suggestion, but I guess it may break backwards compatibility since the hook interface will change?

nicoddemus

I much prefer this approach over templating... hooks are kept backward compatible if we introduce new parameters, so let's go for it. 👍

nicoddemus · 2019-10-09T20:11:37Z

src/_pytest/hookspec.py

@@ -472,7 +472,7 @@ def pytest_unconfigure(config):
 # -------------------------------------------------------------------------


-def pytest_assertrepr_compare(config, op, left, right):
+def pytest_assertrepr_compare(config, op, left, right, expl):


Please update the docstring with expl as well. 😁

Would need more updating in general then, especially for when we go with the return-string-to-skip special postprocessing approach.

nicoddemus · 2019-10-09T20:11:58Z

src/_pytest/assertion/util.py

@@ -146,7 +148,7 @@ def assertrepr_compare(config, op, left, right):
                    type_fn = (isdatacls, isattrs)
                    explanation = _compare_eq_cls(left, right, verbose, type_fn)
                elif verbose > 0:
-                    explanation = _compare_eq_verbose(left, right)
+                    return expl + "\n~" + "\n~".join(_compare_eq_verbose(left, right))


Sorry, I don't get the "\n~" part...? 🤔

It mimics https://github.com/blueyed/pytest/blob/1543a16330a5be10022505429b66608e7ee0ed19/src/_pytest/assertion/__init__.py#L139 (which is skipped for strings).
It's the "mini language" used with https://github.com/blueyed/pytest/blob/1543a16330a5be10022505429b66608e7ee0ed19/src/_pytest/assertion/util.py#L59-L91.

nicoddemus · 2019-10-09T20:13:26Z

but I guess it may break backwards compatibility since the hook interface will change?

No, pluggy is smart and discards parameters that an implementation doesn't declare. 😉

blueyed · 2019-10-09T23:41:38Z

Why do you both prefer this over a special template var (keep in mind that you have to use the "mini template language" here already).
It requires you to be aware of internals more, including that you have to prefix things with "~"/">".
It also clearly is a hack to handle strings in a special way (to skip post-processing).

nicoddemus · 2019-10-10T10:47:25Z

Oh I didn't realize we had a template language in place, sorry for having given my opinion so soon. I will have to take a deeper look later then.

TODO: - [ ] real plugin hook? - [ ] should handle other places where safeformat is used also, via args/options to the hook then likely. Ref: pytest-dev#3962 Ref: pytest-dev#5933

blueyed · 2019-10-21T21:02:29Z

@nicoddemus
Could you make up your mind?

(it is a long-standing regression, so it would be great to make some progress here)

blueyed · 2019-10-21T21:03:39Z

Who can we ping about this - being more into the assertion rewriting part of pytest?

nicoddemus · 2019-10-23T23:15:36Z

Could you make up your mind?

Sorry about the delay, I've been busy with other things.

The problem that I have with the expl parameter is that the user can't really do anything with it in practice:

def y():
    return [0, 1]

def test():
    assert y() == [1, 2, 3, 4]

For this test failure, the expl parameter is:

"%(py2)s\n{%(py2)s = %(py0)s()\n} == %(py5)s"

So the expl parameter of pytest_assertrepr_compare is only really useful for pytest's own internal implementation, assertrepr_compare.

I see two options:

We have to rethink how pytest_assertrepr_compare actually works so users can reuse the variables saved by pytest's rewriting mechanism.
We need to somehow pass expl from the rewriting mechanism directly to assertrepr_compare somehow.

I wouldn't even know where to start with 1), and 2) would require some hacking, like setting expl to a global.

At first look 2) is ugly, but we might evaluate it under the light that we already are doing something similar by installing util._reprcompare during pytest_runtest_setup.

What if we set util._reprcompare_expls during _call_reprcompare?

    if util._reprcompare is not None:
        util._reprcompare_expl = expl
        try:
            custom = util._reprcompare(ops[i], each_obj[i], each_obj[i + 1])
        finally:
            util._reprcompare_expl = None
        if custom is not None:
            return custom

Again, while this is ugly, at least it is kept internal and won't introduce a parameter which we might regret later.

What do you think?

blueyed · 2019-10-31T03:05:43Z

The problem that I have with the expl parameter is that the user can't really do anything with it in practice:

Yes.

What if we set util._reprcompare_expls during _call_reprcompare?

How does that help plugins with using the default?

nicoddemus · 2019-11-06T18:33:47Z

How does that help plugins with using the default?

Unless I'm misunderstanding, I thought the patch was about to fix "keep "where … and …" output in verbose mode" in the pytest core, not necessarily expose to plugins the new "expls" parameter.

In my previous post, assertrepr_compare would access the global util._reprcompare_expl instead of receiving it by argument... again a hack, but we can avoid exposing a expl parameter to users, which would require us to carry this over many releases if we ever find a better way to represent the explanation obtained by the rewriter.

(Btw sorry for the delay, I am on vacation and now I'm trying to catch up with everything).

blueyed · 2019-11-09T20:20:51Z

@nicoddemus
The idea here is to also make this available for plugins / do it "properly". See previous comment: #6104

nicoddemus · 2019-11-22T21:12:27Z

The idea here is to also make this available for plugins / do it "properly". See previous comment: #6104

Is that the right issue number? 🤔

blueyed · 2019-11-22T21:29:57Z

The idea here is to also make this available for plugins / do it "properly". See previous comment: #6104

Is that the right issue number?

No, sorry. More likely #5932.

JBKahn · 2019-11-27T12:47:00Z

@blueyed does closing this mean no charge coming?

blueyed · 2019-11-28T00:56:33Z

@JBKahn I've closed all my open PRs for now. Feel free to pick it up, I might come back to it later myself, but just had to clean up things for myself.

JBKahn · 2019-11-28T02:54:17Z

@nicoddemus any more thoughts on this one? We'd really like to be able to use the explanation in the rewrite

[WIP/RFC] assert: keep "where … and …" output in verbose mode

1543a16

Ref: pytest-dev#5932 Fixes: pytest-dev#5192

blueyed mentioned this pull request Oct 9, 2019

Increasing verbosity turns off the assertion rewriting #5932

Closed

blueyed mentioned this pull request Oct 9, 2019

Failed test output is less verbose with -v than not using -v #5192

Closed

blueyed mentioned this pull request Oct 9, 2019

Help with plugin assertion rewriting #5931

Closed

JBKahn mentioned this pull request Oct 9, 2019

expression drill-down gone? darrenburns/pytest-clarity#3

Open

nicoddemus reviewed Oct 9, 2019

View reviewed changes

adjust existing test

5e7b060

blueyed changed the title ~~[WIP/RFC] assert: keep "where … and …" output in verbose mode~~ [RFC] assert: keep "where … and …" output in verbose mode Oct 16, 2019

blueyed added topic: rewrite related to the assertion rewrite mechanism type: bug problem that needs to be addressed labels Oct 16, 2019

blueyed mentioned this pull request Oct 20, 2019

Show full repr with assert a==b and -vv #4607

Merged

blueyed mentioned this pull request Oct 21, 2019

[RFC/WIP]: assert: use safeformat for display with -vv #6020

Closed

3 tasks

blueyed added 2 commits October 21, 2019 23:01

fix test_len

462296b

adjust signature in tests, do not prefer this

25c28c7

blueyed closed this Nov 27, 2019

blueyed mentioned this pull request Jan 13, 2020

Only activate if -vv used darrenburns/pytest-clarity#10

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFC] assert: keep "where … and …" output in verbose mode #5933

[RFC] assert: keep "where … and …" output in verbose mode #5933

blueyed commented Oct 9, 2019

blueyed commented Oct 9, 2019 •

edited

blueyed commented Oct 9, 2019

JBKahn commented Oct 9, 2019 •

edited by blueyed

darrenburns commented Oct 9, 2019

nicoddemus left a comment

nicoddemus Oct 9, 2019

blueyed Oct 9, 2019

nicoddemus Oct 9, 2019

blueyed Oct 9, 2019

nicoddemus commented Oct 9, 2019

blueyed commented Oct 9, 2019

nicoddemus commented Oct 10, 2019

blueyed commented Oct 21, 2019

blueyed commented Oct 21, 2019

nicoddemus commented Oct 23, 2019 •

edited

blueyed commented Oct 31, 2019

nicoddemus commented Nov 6, 2019 •

edited

blueyed commented Nov 9, 2019

nicoddemus commented Nov 22, 2019

blueyed commented Nov 22, 2019

JBKahn commented Nov 27, 2019

blueyed commented Nov 28, 2019

JBKahn commented Nov 28, 2019

[RFC] assert: keep "where … and …" output in verbose mode #5933

[RFC] assert: keep "where … and …" output in verbose mode #5933

Conversation

blueyed commented Oct 9, 2019

blueyed commented Oct 9, 2019 • edited

blueyed commented Oct 9, 2019

JBKahn commented Oct 9, 2019 • edited by blueyed

darrenburns commented Oct 9, 2019

nicoddemus left a comment

Choose a reason for hiding this comment

nicoddemus Oct 9, 2019

Choose a reason for hiding this comment

blueyed Oct 9, 2019

Choose a reason for hiding this comment

nicoddemus Oct 9, 2019

Choose a reason for hiding this comment

blueyed Oct 9, 2019

Choose a reason for hiding this comment

nicoddemus commented Oct 9, 2019

blueyed commented Oct 9, 2019

nicoddemus commented Oct 10, 2019

blueyed commented Oct 21, 2019

blueyed commented Oct 21, 2019

nicoddemus commented Oct 23, 2019 • edited

blueyed commented Oct 31, 2019

nicoddemus commented Nov 6, 2019 • edited

blueyed commented Nov 9, 2019

nicoddemus commented Nov 22, 2019

blueyed commented Nov 22, 2019

JBKahn commented Nov 27, 2019

blueyed commented Nov 28, 2019

JBKahn commented Nov 28, 2019

blueyed commented Oct 9, 2019 •

edited

JBKahn commented Oct 9, 2019 •

edited by blueyed

nicoddemus commented Oct 23, 2019 •

edited

nicoddemus commented Nov 6, 2019 •

edited