pytest-icdiff icon indicating copy to clipboard operation
pytest-icdiff copied to clipboard

Better diff for multiline strings

Open patrick91 opened this issue 6 years ago • 4 comments

Hi there, would be possible to get a better diff for multiline strings?

Right now it prints the "\n", which is not ideal, example here:

image

patrick91 avatar Jan 26 '20 11:01 patrick91

just building a minimal repro (sorry about the earlier reply, if you did see it)

Screenshot from 2020-02-03 14-53-13

hjwp avatar Feb 03 '20 14:02 hjwp

so, icdiff does do something better by default, so there must be a way, yes.

Screenshot from 2020-02-03 14-59-43

hjwp avatar Feb 03 '20 15:02 hjwp

I spent a bit of time this morning on this and looks like this is due to pytest's behaviour:

Return None for no custom explanation, otherwise return a list of strings. The strings will be joined by newlines but any newlines in a string will be escaped. Note that all but the first line will be indented slightly, the intention is for the first line to be a summary.

https://docs.pytest.org/en/6.2.x/assert.html#defining-your-own-explanation-for-failed-assertions

So we should be able to find a way to improve suppor for multiline diffs :)

I don't have a lot of time now, but I might try in future :)

patrick91 avatar Feb 24 '22 21:02 patrick91

Briefly looked into this, seems like this is something pprintpp should support, if anything. icdiff only has to worry about files, we have the additional step of converting arbitrary objects to a string representation – breaking at \\n after the fact could prove problematic (imagine a huge string with many line breaks inside a huge dict – currently it's at least clear that it's just a string in a dict).


Just tried to hack it for fun. Looks like we could cheese it by making a deep copy of the objects, replacing every str we come across with a special class with a representation that works for our case. I didn't want to re-implement deep copy, so I found a way to patch the function responsible for handling only str instances:

diff --git a/pytest_icdiff.py b/pytest_icdiff.py
index 16fbba5..4a7f654 100644
--- a/pytest_icdiff.py
+++ b/pytest_icdiff.py
@@ -2,6 +2,7 @@
 import shutil
 from pprintpp import pformat
 import icdiff
+from copy import deepcopy, _deepcopy_dispatch
 
 COLS = shutil.get_terminal_size().columns
 MARGIN_L = 10
@@ -26,6 +27,13 @@ def pytest_assertrepr_compare(config, op, left, right):
 
     half_cols = COLS / 2 - MARGINS
 
+    # Temporarily patch copy function for str
+    str_atomic = _deepcopy_dispatch[str]
+    _deepcopy_dispatch[str] = lambda x, memo: PrettyStr(x)
+    left = deepcopy(left)
+    right = deepcopy(right)
+    _deepcopy_dispatch[str] = str_atomic
+
     pretty_left = pformat(left, indent=2, width=half_cols).splitlines()
     pretty_right = pformat(right, indent=2, width=half_cols).splitlines()
     diff_cols = COLS - MARGINS
@@ -57,3 +65,15 @@ def pytest_assertrepr_compare(config, op, left, right):
         icdiff_lines = list(differ.make_table(pretty_left, pretty_right, context=True))
 
     return ["equals failed"] + [color_off + l for l in icdiff_lines]
+
+
+class PrettyStr(str):
+    def __repr__(self):
+        # Add a newline indication to all but the last line
+        lines = self.splitlines()
+        lines = list(map(self._pretty_line, lines[:-1])) + [repr(lines[-1])]
+        return '\n'.join(lines)
+    @staticmethod
+    def _pretty_line(x):
+        r = repr(x)
+        return r[:-1] + '\n' + r[-1:]
diff --git a/tests/test_pytest_icdiff.py b/tests/test_pytest_icdiff.py
index be5a5e1..81dff54 100644
--- a/tests/test_pytest_icdiff.py
+++ b/tests/test_pytest_icdiff.py
@@ -289,3 +289,16 @@ def test_really_long_diffs_use_context_mode(testdir):
     output = testdir.runpytest('-vv', '--color=yes').stdout.str()
     assert len(output.splitlines()) < 50
     assert "---" in output  # context split marker
+
+def test_pretty_strings(testdir):
+    testdir.makepyfile(
+        f"""
+        def test_one():
+            one = '\\n'.join(str(i) for i in range(30))
+            two = '\\n'.join(str(i) for i in range(1, 31))
+            assert one == two
+        """
+    )
+    output = testdir.runpytest('-vv', '--color=yes').stdout.str()
+    assert len(output.splitlines()) > 10
+    assert "---" in output

This seems to work, but no idea how efficiently. I definitely don't think this will work with multiple threads.

PeterNerlich avatar Feb 25 '23 20:02 PeterNerlich