From 7082c5c53c8fbc695801b9cdef474d4d2574beeb Mon Sep 17 00:00:00 2001 From: Dani Pinyol Date: Thu, 17 Sep 2026 14:04:45 +0200 Subject: [PATCH] fix: Wrong message when comparing two sequences with alignment Fixes #41 When comparing 2 sequences that are the same except for an extra item at the beginning of one of them, there was a bug when reporting a compressed version of the sequence. It contained <[.., first item]>, where it displays the head item at the end rather than at the beginning. --- assertpy2/helpers.py | 78 +++++++++++++++++++++++------------ docs/guides/errors.md | 8 ++-- tests/test_dict_compare.py | 13 +++--- tests/test_equals.py | 10 ++--- tests/test_message_elision.py | 71 ++++++++++++++++++++++++++----- tests/test_rich_diff.py | 8 ++-- 6 files changed, 133 insertions(+), 55 deletions(-) diff --git a/assertpy2/helpers.py b/assertpy2/helpers.py index cf87e53c..dbbccdad 100644 --- a/assertpy2/helpers.py +++ b/assertpy2/helpers.py @@ -45,17 +45,50 @@ def _both_list_like(left: object, right: object) -> bool: ) -def _joined_parts(parts: list[str], *, elided: bool, opener: str = "", closer: str = "") -> str: +class _Elided: + """Marker standing in a parts list for a run of elements equal to their counterpart. + + Carried along with the parts rather than reduced to a flag because a flag can only put the ``..`` + in front: a value differing from its counterpart by one extra leading element then printed as + ``[.., 0]``, which is the shape of a changed *tail*. + """ + + __slots__ = () + + def __repr__(self) -> str: + return ".." + + +_ELIDED = _Elided() +_Part = str | _Elided + + +def _joined_parts(parts: list[_Part], *, opener: str = "", closer: str = "") -> str: """Assemble a collapsed repr, capping how many differing parts are spelled out. Collapsing only removes what matched, so a value where nearly everything differs still prints in - full. The cap is what keeps that case from becoming a wall of text on one line. + full. The cap is what keeps that case from becoming a wall of text on one line. `_ELIDED` entries + mark where the matched runs were and never count against the cap. """ - hidden = len(parts) - 5 - if hidden > 0: - parts = [*parts[:5], f"... and {hidden} more"] - prefix = ".." if elided and not parts else ".., " if elided else "" - return f"{opener}{prefix}{', '.join(parts)}{closer}" + kept: list[_Part] = [] + spelled = hidden = 0 + for part in parts: + if isinstance(part, _Elided): + # one marker per run, however many elements the run swallowed + if not (kept and isinstance(kept[-1], _Elided)): + kept.append(part) + continue + spelled += 1 + if spelled <= 5: + kept.append(part) + else: + hidden += 1 + if hidden: + # the count stands for everything past the cap, so a marker it displaced says nothing more + while kept and isinstance(kept[-1], _Elided): + kept.pop() + kept.append(f"... and {hidden} more") + return f"{opener}{', '.join(str(part) for part in kept)}{closer}" def _elided_text_repr(text: str, counterpart: str) -> str: @@ -70,14 +103,13 @@ def _elided_text_repr(text: str, counterpart: str) -> str: # string with none of it near the change return _windowed(text, counterpart, width=320)[0] if len(text) > 320 else text other_lines = counterpart.splitlines() - parts = [] - elided = False + parts: list[_Part] = [] for index, line in enumerate(text.splitlines()): if index < len(other_lines) and line == other_lines[index]: - elided = True + parts.append(_ELIDED) continue parts.append(f"line {index + 1}: {line}") - return _joined_parts(parts, elided=elided) + return _joined_parts(parts) def _elided_seq_repr(seq, counterpart) -> str: @@ -98,20 +130,16 @@ def _elided_seq_repr(seq, counterpart) -> str: # on a two-element list the ".." form is the longer of the two return rendered aligned = _aligned_match_indices(seq, counterpart) - parts = [] - elided = False + parts: list[_Part] = [] for index, value in enumerate(seq): # two loops rather than a per-element branch: this runs once per element of every rendered sequence if aligned is not None: matched = index in aligned else: matched = index < len(counterpart) and not _guarded_not_equal(value, counterpart[index]) - if matched: - elided = True - continue - parts.append(_safe_repr(value)) + parts.append(_ELIDED if matched else _safe_repr(value)) opener, closer = ("(", ")") if isinstance(seq, tuple) else ("[", "]") - return _joined_parts(parts, elided=elided, opener=opener, closer=closer) + return _joined_parts(parts, opener=opener, closer=closer) def _keyed_pair(value: object, other: object) -> tuple[MappingLike, MappingLike] | None: @@ -364,8 +392,7 @@ def _dict_repr(mapping, counterpart, _seen=None): if id(mapping) in _seen: return "{}" _seen = _seen | {id(mapping)} - parts = [] - ellip = False + parts: list[_Part] = [] # left in the mapping's order, which the diff prints: sorting here made the two halves disagree for key, value in ((key, mapping[key]) for key in mapping): if key not in counterpart: @@ -373,7 +400,7 @@ def _dict_repr(mapping, counterpart, _seen=None): else: decision = _node_decision(value, counterpart[key], config, field=key) if decision == "equal": - ellip = True + parts.append(_ELIDED) elif decision == "leaf": parts.append(f"{_safe_repr(key)}: {_safe_repr(value)}") else: # recurse @@ -385,7 +412,7 @@ def _dict_repr(mapping, counterpart, _seen=None): else: value_repr = _safe_repr(value) parts.append(f"{_safe_repr(key)}: {value_repr}") - return _joined_parts(parts, elided=ellip, opener="{", closer="}") + return _joined_parts(parts, opener="{", closer="}") def _list_repr(seq, counterpart, _seen): """List counterpart of ``_dict_repr``: collapse equal elements to ``..`` and drill only into @@ -395,8 +422,7 @@ def _list_repr(seq, counterpart, _seen): if id(seq) in _seen: return "[]" _seen = _seen | {id(seq)} - parts = [] - ellip = False + parts: list[_Part] = [] for index, value in enumerate(seq): if index >= len(counterpart): parts.append(_safe_repr(value)) # extra element beyond the counterpart's length @@ -404,7 +430,7 @@ def _list_repr(seq, counterpart, _seen): other_value = counterpart[index] decision = _node_decision(value, other_value, config, field=None) if decision == "equal": - ellip = True + parts.append(_ELIDED) elif decision == "leaf": parts.append(_safe_repr(value)) elif (keyed := _keyed_pair(value, other_value)) is not None: @@ -414,7 +440,7 @@ def _list_repr(seq, counterpart, _seen): else: parts.append(_safe_repr(value)) opener, closer = ("(", ")") if isinstance(seq, tuple) else ("[", "]") # keep tuples looking like tuples - return _joined_parts(parts, elided=ellip, opener=opener, closer=closer) + return _joined_parts(parts, opener=opener, closer=closer) if (keyed := _keyed_pair(val, other)) is not None: reported_val = self._selected_keys_only(keyed[0], ignore, include) diff --git a/docs/guides/errors.md b/docs/guides/errors.md index fc45cdae..4ff9272a 100644 --- a/docs/guides/errors.md +++ b/docs/guides/errors.md @@ -82,8 +82,8 @@ except AssertionError as e: # + 99 ``` -The `..` in that message stands for the parts that matched. Only what differs is spelled out, so a -one-field change in a wide object reads as `{.., 'b': 2}` rather than as both objects in full. +Each `..` in that message stands for a run of parts that matched, and stands where that run was, so a +one-field change in a wide object reads as `{.., 'b': 2, ..}` rather than as both objects in full. Sequences collapse the same way once they grow past a line or so, which keeps a single changed element out of a forty-item dump: @@ -93,7 +93,7 @@ try: assert_that(list(range(40))).is_equal_to([*range(27), 999, *range(28, 40)]) except AssertionError as e: print(e) - # Expected <[.., 27]> to be equal to <[.., 999]>, but was not. + # Expected <[.., 27, ..]> to be equal to <[.., 999, ..]>, but was not. # diff (sequence): # [27]: # - 27 @@ -108,7 +108,7 @@ try: assert_that([0, *range(1, 40)]).is_equal_to(list(range(1, 40))) except AssertionError as e: print(e) - # Expected <[.., 0]> to be equal to <[..]>, but was not. + # Expected <[0, ..]> to be equal to <[..]>, but was not. # diff (sequence): # actual[0]: - 0 ``` diff --git a/tests/test_dict_compare.py b/tests/test_dict_compare.py index ed62a239..78746637 100644 --- a/tests/test_dict_compare.py +++ b/tests/test_dict_compare.py @@ -83,7 +83,10 @@ def test_failure_single_entry(): def test_failure_multi_entry(): with pytest.raises(AssertionError) as exc_info: assert_that({"a": 1, "b": 2, "c": 3}).is_equal_to({"a": 1, "b": 3, "c": 3}) - assert_that(str(exc_info.value)).is_equal_to("Expected <{.., 'b': 2}> to be equal to <{.., 'b': 3}>, but was not.") + # 'a' matched ahead of the changed key and 'c' matched behind it, so 'b' is marked on both sides + assert_that(str(exc_info.value)).is_equal_to( + "Expected <{.., 'b': 2, ..}> to be equal to <{.., 'b': 3, ..}>, but was not." + ) def test_failure_multi_entry_failure(): @@ -375,7 +378,7 @@ def test_failure_top_mismatch_when_ignoring_single_nested_key(): with pytest.raises(AssertionError) as exc_info: assert_that(actual).is_equal_to(expected, ignore=("b", "c")) assert_that(str(exc_info.value)).is_equal_to( - "Expected <{.., 'a': 1}> to be equal to <{.., 'a': 2}> ignoring keys , but was not." + "Expected <{'a': 1, ..}> to be equal to <{'a': 2, ..}> ignoring keys , but was not." ) @@ -385,7 +388,7 @@ def test_failure_top_mismatch_when_ignoring_single_nested_sibling_key(): with pytest.raises(AssertionError) as exc_info: assert_that(actual).is_equal_to(expected, ignore=("b", "d")) assert_that(str(exc_info.value)).is_equal_to( - "Expected <{.., 'a': 1}> to be equal to <{.., 'a': 2}> ignoring keys , but was not." + "Expected <{'a': 1, ..}> to be equal to <{'a': 2, ..}> ignoring keys , but was not." ) @@ -395,8 +398,8 @@ def test_failure_deep_mismatch_when_ignoring_double_nested_sibling_key(): with pytest.raises(AssertionError) as exc_info: assert_that(actual).is_equal_to(expected, ignore=("b", "f", "g")) assert_that(str(exc_info.value)).is_equal_to( - "Expected <{.., 'b': {.., 'd': {'e': 3}}}> to be equal to " - "<{.., 'b': {.., 'd': {'e': 4}}}> ignoring keys , but was not." + "Expected <{.., 'b': {.., 'd': {'e': 3}, ..}}> to be equal to " + "<{.., 'b': {.., 'd': {'e': 4}, ..}}> ignoring keys , but was not." ) diff --git a/tests/test_equals.py b/tests/test_equals.py index c9bbbf13..add68f24 100644 --- a/tests/test_equals.py +++ b/tests/test_equals.py @@ -42,7 +42,7 @@ def test_is_equal_long_list_failure_elides_the_matching_elements(): expected[27] = 999 with pytest.raises(AssertionError) as exc_info: assert_that(actual).is_equal_to(expected) - assert_that(str(exc_info.value)).is_equal_to("Expected <[.., 27]> to be equal to <[.., 999]>, but was not.") + assert_that(str(exc_info.value)).is_equal_to("Expected <[.., 27, ..]> to be equal to <[.., 999, ..]>, but was not.") def test_is_equal_few_but_long_elements_still_elides(): @@ -56,7 +56,7 @@ def test_is_equal_long_tuple_failure_keeps_tuple_brackets(): expected[27] = 999 with pytest.raises(AssertionError) as exc_info: assert_that(actual).is_equal_to(tuple(expected)) - assert_that(str(exc_info.value)).is_equal_to("Expected <(.., 27)> to be equal to <(.., 999)>, but was not.") + assert_that(str(exc_info.value)).is_equal_to("Expected <(.., 27, ..)> to be equal to <(.., 999, ..)>, but was not.") def test_is_equal_multiline_failure_elides_the_matching_lines(): @@ -65,7 +65,7 @@ def test_is_equal_multiline_failure_elides_the_matching_lines(): with pytest.raises(AssertionError) as exc_info: assert_that(actual).is_equal_to(expected) assert_that(str(exc_info.value)).is_equal_to( - "Expected <.., line 6: line 5> to be equal to <.., line 6: line five>, but was not." + "Expected <.., line 6: line 5, ..> to be equal to <.., line 6: line five, ..>, but was not." ) @@ -74,7 +74,7 @@ def test_is_equal_multiline_failure_lists_every_changed_line(): expected = actual.replace("line 1", "L1").replace("line 6", "L6") with pytest.raises(AssertionError) as exc_info: assert_that(actual).is_equal_to(expected) - assert_that(str(exc_info.value)).contains("line 2: line 1, line 7: line 6") + assert_that(str(exc_info.value)).contains("line 2: line 1, .., line 7: line 6") def test_is_equal_short_multiline_failure_is_printed_whole(): @@ -413,7 +413,7 @@ def test_is_equal_shifted_list_failure_elides_the_aligned_run(): # elides on the alignment the diff pairs on: by position, every later element shifts out and both dump whole with pytest.raises(AssertionError) as exc_info: assert_that([0, *range(1, 40)]).is_equal_to(list(range(1, 40))) - assert_that(str(exc_info.value)).contains("Expected <[.., 0]> to be equal to <[..]>") + assert_that(str(exc_info.value)).contains("Expected <[0, ..]> to be equal to <[..]>") class TestFindAmbiguousOperandOnMismatchedShapes: diff --git a/tests/test_message_elision.py b/tests/test_message_elision.py index 2e7fbe9e..1d742fed 100644 --- a/tests/test_message_elision.py +++ b/tests/test_message_elision.py @@ -11,7 +11,7 @@ import pytest from assertpy2 import assert_that -from assertpy2.helpers import _both_list_like, _elided_seq_repr, _elided_text_repr, _joined_parts +from assertpy2.helpers import _ELIDED, _both_list_like, _elided_seq_repr, _elided_text_repr, _joined_parts class TestSequenceElisionBoundaries: @@ -54,7 +54,9 @@ def test_three_lines_are_printed_whole(self): def test_four_lines_are_collapsed_to_the_changed_ones(self): # the cost of a multi-line value is vertical, and the message prints the value twice collapsed = _elided_text_repr("a\nb\nc\nd", "a\nZ\nc\nd") - assert_that(collapsed).is_equal_to(".., line 2: b") + # line 1 matched ahead of the change and lines 3-4 matched behind it, so the change is marked + # on both sides + assert_that(collapsed).is_equal_to(".., line 2: b, ..") class TestJoinedPartsCap: @@ -62,23 +64,35 @@ class TestJoinedPartsCap: in full. The cap on spelled-out parts is what keeps that from becoming a wall of text.""" def test_five_parts_are_all_spelled_out(self): - assert_that(_joined_parts([str(index) for index in range(5)], elided=False)).is_equal_to("0, 1, 2, 3, 4") + assert_that(_joined_parts([str(index) for index in range(5)])).is_equal_to("0, 1, 2, 3, 4") def test_the_sixth_part_turns_into_a_count(self): - assert_that(_joined_parts([str(index) for index in range(6)], elided=False)).is_equal_to( - "0, 1, 2, 3, 4, ... and 1 more" - ) + assert_that(_joined_parts([str(index) for index in range(6)])).is_equal_to("0, 1, 2, 3, 4, ... and 1 more") def test_the_count_names_how_many_were_dropped(self): - assert_that(_joined_parts([str(index) for index in range(9)], elided=False)).is_equal_to( - "0, 1, 2, 3, 4, ... and 4 more" + assert_that(_joined_parts([str(index) for index in range(9)])).is_equal_to("0, 1, 2, 3, 4, ... and 4 more") + + def test_markers_do_not_count_against_the_cap(self): + # a marker stands for what was dropped for being equal; spending the cap on those would push + # the differing parts the message exists to show out of it + parts = [item for index in range(5) for item in (_ELIDED, str(index))] + assert_that(_joined_parts(parts)).is_equal_to(".., 0, .., 1, .., 2, .., 3, .., 4") + + def test_a_marker_the_count_displaced_is_dropped(self): + # "... and N more" already stands for everything past the cap, marker included + assert_that(_joined_parts([*[str(index) for index in range(6)], _ELIDED])).is_equal_to( + "0, 1, 2, 3, 4, ... and 1 more" ) - def test_an_elided_prefix_marks_what_matched(self): - assert_that(_joined_parts(["x"], elided=True, opener="[", closer="]")).is_equal_to("[.., x]") + def test_a_run_of_matches_collapses_to_one_marker(self): + assert_that(_joined_parts([_ELIDED, _ELIDED, _ELIDED, "x"], opener="[", closer="]")).is_equal_to("[.., x]") + + def test_the_marker_stands_where_the_matched_run_was(self): + assert_that(_joined_parts([_ELIDED, "x"], opener="[", closer="]")).is_equal_to("[.., x]") + assert_that(_joined_parts(["x", _ELIDED], opener="[", closer="]")).is_equal_to("[x, ..]") def test_an_all_matching_value_is_just_the_marker(self): - assert_that(_joined_parts([], elided=True, opener="{", closer="}")).is_equal_to("{..}") + assert_that(_joined_parts([_ELIDED], opener="{", closer="}")).is_equal_to("{..}") class TestElisionReachesTheMessage: @@ -115,3 +129,38 @@ def test_the_failure_names_the_field_not_the_index(self): with pytest.raises(AssertionError) as exc_info: assert_that(self._Point(1, 2)).is_equal_to(self._Point(1, 3)) assert_that(str(exc_info.value)).contains("y=2").contains("y=3") + + +class TestElisionMarkerPlacement: + """``..`` stands where the collapsed elements were, so a changed head reads differently from a + changed tail. + + A single leading marker said only *that* something matched, never where: a sequence differing + from its counterpart by one extra element at the front printed that element behind the marker, + as ``[.., 0]``, which reads as a changed tail. + """ + + def test_a_changed_head_keeps_the_marker_behind_it(self): + seq = [0, *range(1, 40)] + assert_that(_elided_seq_repr(seq, list(range(1, 40)))).is_equal_to("[0, ..]") + + def test_a_changed_tail_keeps_the_marker_in_front(self): + assert_that(_elided_seq_repr([*[1] * 20, 2], [1] * 21)).is_equal_to("[.., 2]") + + def test_a_changed_middle_is_marked_on_both_sides(self): + assert_that(_elided_seq_repr([*[1] * 10, 2, *[1] * 10], [1] * 21)).is_equal_to("[.., 2, ..]") + + def test_a_changed_head_reaches_the_failure_message(self): + with pytest.raises(AssertionError) as exc_info: + assert_that([0, *range(1, 40)]).is_equal_to(list(range(1, 40))) + assert_that(str(exc_info.value)).contains("<[0, ..]>") + + def test_a_changed_first_line_keeps_the_marker_behind_it(self): + # the text path collapses by line and marks the run the same way + assert_that(_elided_text_repr("X\nb\nc\nd", "a\nb\nc\nd")).is_equal_to("line 1: X, ..") + + def test_a_changed_first_key_keeps_the_marker_behind_it(self): + # and so does the mapping path, where the run is the keys that matched + with pytest.raises(AssertionError) as exc_info: + assert_that({"a": 1, "b": 2}).is_equal_to({"a": 9, "b": 2}) + assert_that(str(exc_info.value)).contains("<{'a': 1, ..}>") diff --git a/tests/test_rich_diff.py b/tests/test_rich_diff.py index 92537000..ee961c8a 100644 --- a/tests/test_rich_diff.py +++ b/tests/test_rich_diff.py @@ -460,18 +460,18 @@ def test_list_in_dict_collapses_to_changed_element(self): {"rows": [{"id": 1, "v": "x"}, {"id": 2, "v": "CHANGED"}, {"id": 3, "v": "z"}]} ) msg = str(exc.value) - assert_that(msg).contains("[.., {.., 'v': 'y'}]") + assert_that(msg).contains("[.., {.., 'v': 'y'}, ..]") assert_that(msg).does_not_contain("'id': 1").does_not_contain("'id': 3") def test_scalar_list_collapses(self): with pytest.raises(AssertionError) as exc: assert_that({"a": [1, 2, 3, 4, 5]}).is_equal_to({"a": [1, 2, 999, 4, 5]}) - assert_that(str(exc.value)).contains("'a': [.., 3]") + assert_that(str(exc.value)).contains("'a': [.., 3, ..]") def test_tuple_renders_with_parens(self): with pytest.raises(AssertionError) as exc: assert_that({"t": (1, 2, 3)}).is_equal_to({"t": (1, 9, 3)}) - assert_that(str(exc.value)).contains("'t': (.., 2)") + assert_that(str(exc.value)).contains("'t': (.., 2, ..)") def test_nested_list_of_lists(self): with pytest.raises(AssertionError) as exc: @@ -486,7 +486,7 @@ def test_extra_element_shown(self): def test_tolerance_mismatch_shows_element_as_leaf(self): with pytest.raises(AssertionError) as exc: assert_that({"a": [1.0, 2.0, 3.0]}).is_equal_to({"a": [1.0, 2.5, 3.0]}, tolerance=0.1) - assert_that(str(exc.value)).contains("'a': [.., 2.0]") + assert_that(str(exc.value)).contains("'a': [.., 2.0, ..]") def test_pure_dict_message_unchanged(self): with pytest.raises(AssertionError) as exc: