Improve full diff output for lists

[blueyed]

Massage text input for difflib when comparing pformat output of
different line lengths.

Also do not strip ndiff output on the left, which currently already
removes indenting for lines with no differences.

Before:

E   AssertionError: assert ['version', '...version_info'] == ['version', '...version', ...]
E     Right contains 3 more items, first extra item: ' '
E     Full diff:
E     - ['version', 'version_info', 'sys.version', 'sys.version_info']
E     + ['version',
E     +  'version_info',
E     +  'sys.version',
E     +  'sys.version_info',
E     +  ' ',
E     +  'sys.version',
E     +  'sys.version_info']

After:

E   AssertionError: assert ['version', '...version_info'] == ['version', '...version', ...]
E     Right contains 3 more items, first extra item: ' '
E     Full diff:
E       ['version',
E        'version_info',
E        'sys.version',
E     +  'sys.version_info',
E     +  ' ',
E     +  'sys.version',
E        'sys.version_info'
E       ]

TODO:

• changelog
• "improve" / remove asserts
• more tests (?)

[nicoddemus]

Oh this is awesome, thanks for this!

I think this can go into features... to me that's a bug! 😁

About your other points:

1. Changelog: I would put your examples on it, even if it makes the entry large; it makes it very clear what the fix is doing.
2. asserts: I think they could be changed to at least including the offending character: assert closing in [")", "]", "}"], 'unexpected sequence closing {closing} in sequence {left} or something along the lines
3. Nothing comes to mind at the moment for more tests... you did test for both left/right sides, and I assume we have tests when lists have the same size already.

[blueyed]

Thanks for the review. (Amended since we cannot squash-merge)

Improved the separation of opening/closing parenthesis.
Removed the asserts, but protected with an if - wouldn't be cool to crash there.
Added one more test.

[blueyed]

I think this might be good then - assuming full coverage and no breaking existing tests.

[nicoddemus]

Great refactoring. 👍

How about targeting this on master? Also, only missing the CHANGELOG now, I would suggest in case you don't have it ready yet:

Fixed diff output of sequences of different lengths.

Before::


    E   AssertionError: assert ['version', '...version_info'] == ['version', '...version', ...]
    E     Right contains 3 more items, first extra item: ' '
    E     Full diff:
    E     - ['version', 'version_info', 'sys.version', 'sys.version_info']
    E     + ['version',
    E     +  'version_info',
    E     +  'sys.version',
    E     +  'sys.version_info',
    E     +  ' ',
    E     +  'sys.version',
    E     +  'sys.version_info']

After::

    E   AssertionError: assert ['version', '...version_info'] == ['version', '...version', ...]
    E     Right contains 3 more items, first extra item: ' '
    E     Full diff:
    E       ['version',
    E        'version_info',
    E        'sys.version',
    E     +  'sys.version_info',
    E     +  ' ',
    E     +  'sys.version',
    E        'sys.version_info'
    E       ]

[nicoddemus]

Ahh you got it already. 😁

[nicoddemus]

And how about targeting master?

[blueyed]

How about targeting this on master?

It's a change in behavior, would rather not do it.

[blueyed]

Can we check if the changelog renders properly (i.e. with code blocks)?

[nicoddemus]

It's a change in behavior, would rather not do it.

I disagree, but OK.

We can, you have to select the ... on the corner of the file and select "View file":

https://github.com/pytest-dev/pytest/blob/d4569b9e17cf7b8804930a1b91d0a7440f4e0ef3/changelog/5924.feature.rst

You are right, this is not rendering correctly... 🤔

Hmm, using .. code-block it renders nicely:

https://gist.github.com/nicoddemus/b61ada17545d59a315d6ac3a233c4c62

[blueyed]

btw: would be nice if it could "scroll" the -v output to the first diff, e.g. with:

E   AssertionError: assert ['version', '...version_info'] == ['version', '...version', ...]
E     Right contains 3 more items, first extra item: ' '
E     Full diff:
E       [
E        'version',
E        'version_info',
E        'sys.version',
E        'sys.version_info',...
E
E     ...Full output truncated (5 lines hidden), use '-vv' to show