Skip to content

fix: Wrong message when comparing two sequences with alignment #42

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.

Sign up for GitHub

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

Open
dpinol wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: Wrong message when comparing two sequences with alignment #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 52 additions & 26 deletions assertpy2/helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -364,16 +392,15 @@ def _dict_repr(mapping, counterpart, _seen=None):
if id(mapping) in _seen:
return "{<circular ref>}"
_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:
parts.append(f"{_safe_repr(key)}: {_safe_repr(value)}")
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
Expand All @@ -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
Expand All @@ -395,16 +422,15 @@ def _list_repr(seq, counterpart, _seen):
if id(seq) in _seen:
return "[<circular ref>]"
_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
continue
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:
Expand All @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions docs/guides/errors.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Expand All @@ -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
```
Expand Down
13 changes: 8 additions & 5 deletions tests/test_dict_compare.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down Expand Up @@ -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 <b.c>, but was not."
"Expected <{'a': 1, ..}> to be equal to <{'a': 2, ..}> ignoring keys <b.c>, but was not."
)


Expand All @@ -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 <b.d>, but was not."
"Expected <{'a': 1, ..}> to be equal to <{'a': 2, ..}> ignoring keys <b.d>, but was not."
)


Expand All @@ -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 <b.f.g>, but was not."
"Expected <{.., 'b': {.., 'd': {'e': 3}, ..}}> to be equal to "
"<{.., 'b': {.., 'd': {'e': 4}, ..}}> ignoring keys <b.f.g>, but was not."
)


Expand Down
10 changes: 5 additions & 5 deletions tests/test_equals.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand All @@ -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():
Expand All @@ -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."
)


Expand All @@ -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():
Expand Down Expand Up @@ -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:
Expand Down
71 changes: 60 additions & 11 deletions tests/test_message_elision.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -54,31 +54,45 @@ 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:
"""Collapsing only removes what matched, so a value where nearly everything differs still prints
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:
Expand Down Expand Up @@ -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, ..}>")
Loading