Skip to content
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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better diff for multiline strings #14

Open
patrick91 opened this issue Jan 26, 2020 · 4 comments
Open

Better diff for multiline strings #14

patrick91 opened this issue Jan 26, 2020 · 4 comments

Comments

@patrick91
Copy link

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

@hjwp
Copy link
Owner

hjwp commented Feb 3, 2020

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

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

@hjwp
Copy link
Owner

hjwp commented Feb 3, 2020

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

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

@patrick91
Copy link
Author

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 :)

@PeterNerlich
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants