From f0c228c3d4902ea9597987a659d9065424fd800b Mon Sep 17 00:00:00 2001 From: Dani Pinyol Date: Sat, 25 Jul 2026 22:07:41 +0200 Subject: [PATCH] feat: align Sequences with difflib before being diffed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now [0, 1, 2, 3] vs [1, 2, 3] produces exactly one DiffEntry — [0]: - 0. --- assertpy2/_engine/_diff.py | 115 ++++++++++++++++++++++++++++++++----- assertpy2/helpers.py | 14 ++++- docs/guides/errors.md | 17 ++++++ tests/test_equals.py | 7 +++ tests/test_rich_diff.py | 98 ++++++++++++++++++++++++++++++- 5 files changed, 234 insertions(+), 17 deletions(-) diff --git a/assertpy2/_engine/_diff.py b/assertpy2/_engine/_diff.py index e5c5340..7e05ce7 100644 --- a/assertpy2/_engine/_diff.py +++ b/assertpy2/_engine/_diff.py @@ -13,6 +13,7 @@ from __future__ import annotations import dataclasses +import difflib from ..errors import DiffEntry, DiffResult, _safe_repr, _safe_str from ._compare import _node_decision @@ -28,13 +29,107 @@ def _field_dict(obj, is_model): return {field.name: getattr(obj, field.name) for field in obj.__attrs_attrs__} +_ALIGN_MAX_ELEMENTS = 1000 +"""Longest sequence `_alignment_opcodes()` will align. + +difflib's search is quadratic (~33ms at this cap, ~130ms at twice it), and a diff is only ever built +after an assertion has already failed, so the cap buys a bounded worst case at the price of an +unaligned - never wrong, only longer - diff for the rare huge sequence. +""" + + +def _alignment_opcodes(actual, expected): + """difflib opcodes aligning two sequences, or ``None`` when the caller must fall back to indices. + + Aligning first is what keeps an inserted or deleted element from re-reporting every element after + it: ``[0, 1, 2]`` against ``[1, 2]`` is one extra element, not three changed ones. ``None`` means + difflib cannot help - either sequence is over `_ALIGN_MAX_ELEMENTS`, or its elements are unhashable + (dicts, lists, numpy arrays), which difflib needs them to be to build its index. ``autojunk`` is + off because the heuristic treats any value filling more than 1% of a 200+ element sequence as junk, + which is exactly the repeated value an alignment has to match on. + """ + if max(len(actual), len(expected)) > _ALIGN_MAX_ELEMENTS: + return None + try: + return difflib.SequenceMatcher(None, actual, expected, autojunk=False).get_opcodes() + except (TypeError, ValueError): + return None + + +def _aligned_match_indices(seq, counterpart) -> set[int] | None: + """Indices of ``seq`` that align with an equal element of ``counterpart``, or ``None`` if unaligned. + + Lets the failure message elide a matched run that positional comparison would miss, so a shifted + sequence collapses in the message the same way it collapses in the diff + (`assertpy2.helpers._elided_seq_repr()`). + """ + opcodes = _alignment_opcodes(seq, counterpart) + if opcodes is None: + return None + matched: set[int] = set() + for tag, start, stop, _, _ in opcodes: + if tag == "equal": + matched.update(range(start, stop)) + return matched + + +def _element_entries(actual_item, expected_item, path, seen, config) -> list[DiffEntry]: + """Entries for one paired element: none when equal, one leaf, or the nested sub-diff.""" + decision = _node_decision(actual_item, expected_item, config) + if decision == "equal": + return [] + if decision == "recurse": + sub_entries = _sub_diff_entries(actual_item, expected_item, path, _seen=seen, config=config) + if sub_entries is not None: + return sub_entries + return [DiffEntry(path=path, actual=actual_item, expected=expected_item)] + + def _sequence_diff_entries(actual, expected, prefix, seen, config=None) -> list[DiffEntry]: - """Diff two sequences element-by-element, recursing into nested containers. + """Diff two sequences over their difflib alignment, recursing into nested containers. - ``seen`` must already include the ids of ``actual``/``expected`` so a self-referential element - is caught. Shared by the top-level (`_build_equality_diff()`) and nested - (`_sub_diff_entries()`) paths so both decompose sequences identically. Elements have no field - name, so a ``config`` applies only type comparators and tolerance to them. + Aligning first means an inserted or deleted element is reported as itself rather than as a + mismatch at every index after it. Each opcode block pairs its two ranges positionally, and + whatever one side runs out of becomes an actual-only / expected-only entry at its own index. + Unaligned pairs go through `_element_entries()` exactly as the positional walk does, so nested + containers still decompose and a ``config`` still owns the leaves it matches. + + An ``equal`` block is skipped only when there is no ``config``: difflib matched it on ``==``, + which is the whole test in that case, but a comparator is free to disagree and must still be + consulted. When alignment is unavailable, `_indexwise_diff_entries()` takes over. + + ``seen`` must already include the ids of ``actual``/``expected`` so a self-referential element is + caught. Shared by the top-level (`_build_equality_diff()`) and nested (`_sub_diff_entries()`) + paths so both decompose sequences identically. Elements have no field name, so a ``config`` + applies only type comparators and tolerance to them. + """ + opcodes = _alignment_opcodes(actual, expected) + if opcodes is None: + return _indexwise_diff_entries(actual, expected, prefix, seen, config) + entries: list[DiffEntry] = [] + for tag, actual_start, actual_stop, expected_start, expected_stop in opcodes: + if tag == "equal" and config is None: + continue + for offset in range(max(actual_stop - actual_start, expected_stop - expected_start)): + actual_index = actual_start + offset + expected_index = expected_start + offset + if actual_index >= actual_stop: + path = f"{prefix}[{expected_index}]" if prefix else f"[{expected_index}]" + entries.append(DiffEntry(path=path, actual=None, expected=expected[expected_index])) + continue + path = f"{prefix}[{actual_index}]" if prefix else f"[{actual_index}]" + if expected_index >= expected_stop: + entries.append(DiffEntry(path=path, actual=actual[actual_index], expected=None)) + else: + entries.extend(_element_entries(actual[actual_index], expected[expected_index], path, seen, config)) + return entries + + +def _indexwise_diff_entries(actual, expected, prefix, seen, config=None) -> list[DiffEntry]: + """Diff two sequences position-by-position, for the pairs difflib cannot align. + + The fallback of `_sequence_diff_entries()`: every index up to the longer length is compared + against the same index on the other side, and a trailing surplus is reported one element at a time. """ entries: list[DiffEntry] = [] max_len = max(len(actual), len(expected)) @@ -45,15 +140,7 @@ def _sequence_diff_entries(actual, expected, prefix, seen, config=None) -> list[ elif i >= len(expected): entries.append(DiffEntry(path=path, actual=actual[i], expected=None)) else: - decision = _node_decision(actual[i], expected[i], config) - if decision == "leaf": - entries.append(DiffEntry(path=path, actual=actual[i], expected=expected[i])) - elif decision == "recurse": - sub_entries = _sub_diff_entries(actual[i], expected[i], path, _seen=seen, config=config) - if sub_entries is not None: - entries.extend(sub_entries) - else: - entries.append(DiffEntry(path=path, actual=actual[i], expected=expected[i])) + entries.extend(_element_entries(actual[i], expected[i], path, seen, config)) return entries diff --git a/assertpy2/helpers.py b/assertpy2/helpers.py index 135f166..45d8467 100644 --- a/assertpy2/helpers.py +++ b/assertpy2/helpers.py @@ -13,7 +13,7 @@ from assertpy2.errors import DiffResult, _safe_repr, _truncated from ._engine._compare import _CompareConfig, _guarded_not_equal, _node_decision, _spec_matches -from ._engine._diff import _sub_diff_entries +from ._engine._diff import _aligned_match_indices, _sub_diff_entries from ._engine._introspection import is_attrs_instance, is_model_dump_object, is_namedtuple from ._engine._mixin_base import _MixinBase @@ -90,6 +90,11 @@ def _elided_seq_repr(seq, counterpart) -> str: A one-element change in a forty-element list reads as ``[.., 999]`` instead of dumping the list twice into a message the reader then has to diff by eye. + + Matched elements are found by the same alignment the diff uses + (`assertpy2._engine._diff._aligned_match_indices()`), so an element inserted at the head does not + shift every later element out of the elision; positional comparison is the fallback for the + sequences difflib cannot align. """ # past 20 elements the rendering is over budget by construction (one char each plus separators), so # the value is never rendered just to be measured: on the failure path that render is the whole value @@ -99,10 +104,15 @@ def _elided_seq_repr(seq, counterpart) -> str: # short enough to read whole: collapsing it would hide context to save a few characters, and # on a two-element list the ".." form is actually the longer of the two return rendered + aligned = _aligned_match_indices(seq, counterpart) parts = [] elided = False for index, value in enumerate(seq): - if index < len(counterpart) and not _guarded_not_equal(value, counterpart[index]): + if ( + index in aligned + if aligned is not None + else index < len(counterpart) and not _guarded_not_equal(value, counterpart[index]) + ): elided = True continue parts.append(_safe_repr(value)) diff --git a/docs/guides/errors.md b/docs/guides/errors.md index cd569aa..76a304b 100644 --- a/docs/guides/errors.md +++ b/docs/guides/errors.md @@ -58,6 +58,23 @@ except AssertionError as e: # + 999 ``` +Sequences are aligned before they are compared, so an inserted or deleted element is reported as +itself instead of as a mismatch at every index after it: + +```python +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. + # diff (sequence): + # [0]: - 0 +``` + +Comparing position by position would have called all forty elements different. Very long sequences +(over a thousand elements) and sequences of unhashable elements cannot be aligned, and fall back to +the positional comparison. + A multi-line value is collapsed by line, which matters most: every line of it costs a row of terminal, and the message carries the value twice. diff --git a/tests/test_equals.py b/tests/test_equals.py index fe8510a..f71001d 100644 --- a/tests/test_equals.py +++ b/tests/test_equals.py @@ -46,6 +46,13 @@ def test_is_equal_long_list_failure_elides_the_matching_elements(): assert_that(str(exc_info.value)).is_equal_to("Expected <[.., 27]> to be equal to <[.., 999]>, but was not.") +def test_is_equal_shifted_long_list_elides_the_matching_run(): + # one extra element at the head shifts every index: elided by position, nothing would collapse + with pytest.raises(AssertionError) as exc_info: + assert_that(list(range(40))).is_equal_to(list(range(1, 40))) + assert_that(str(exc_info.value)).is_equal_to("Expected <[.., 0]> to be equal to <[..]>, but was not.") + + def test_is_equal_few_but_long_elements_still_elides(): # the budget is the rendered length, not the element count: three long strings flood a message too with pytest.raises(AssertionError) as exc_info: diff --git a/tests/test_rich_diff.py b/tests/test_rich_diff.py index fb49da7..37aebf8 100644 --- a/tests/test_rich_diff.py +++ b/tests/test_rich_diff.py @@ -6,7 +6,8 @@ import pytest from assertpy2 import assert_that, match -from assertpy2._engine._diff import _build_equality_diff, _sub_diff_entries +from assertpy2._engine._compare import _build_compare_config +from assertpy2._engine._diff import _ALIGN_MAX_ELEMENTS, _build_equality_diff, _sub_diff_entries from assertpy2.errors import DiffEntry, DiffResult from assertpy2.helpers import HelpersMixin from assertpy2.pytest_plugin import _format_diff @@ -56,6 +57,101 @@ def test_empty_vs_nonempty(self): assert_that(result.entries[0].expected).is_equal_to(1) +class TestBuildEqualityDiffSequenceAlignment: + """A shifted sequence must report the inserted/deleted elements, not every index past the shift.""" + + def test_one_extra_element_at_the_head_is_one_entry(self): + result = _build_equality_diff([0, 1, 2, 3], [1, 2, 3]) + assert_that(result.kind).is_equal_to("sequence") + assert_that(result.entries).is_length(1) + assert_that(result.entries[0].path).is_equal_to("[0]") + assert_that(result.entries[0].actual).is_equal_to(0) + assert_that(result.entries[0].expected).is_none() + + def test_one_missing_element_at_the_head_is_one_entry(self): + result = _build_equality_diff([1, 2, 3], [0, 1, 2, 3]) + assert_that(result.entries).is_length(1) + assert_that(result.entries[0].path).is_equal_to("[0]") + assert_that(result.entries[0].actual).is_none() + assert_that(result.entries[0].expected).is_equal_to(0) + + def test_extra_element_in_the_middle_is_one_entry(self): + result = _build_equality_diff([1, 2, 9, 3], [1, 2, 3]) + assert_that(result.entries).is_length(1) + assert_that(result.entries[0].path).is_equal_to("[2]") + assert_that(result.entries[0].actual).is_equal_to(9) + assert_that(result.entries[0].expected).is_none() + + def test_missing_element_in_the_middle_is_one_entry(self): + result = _build_equality_diff([1, 2], [1, 5, 2]) + assert_that(result.entries).is_length(1) + assert_that(result.entries[0].path).is_equal_to("[1]") + assert_that(result.entries[0].actual).is_none() + assert_that(result.entries[0].expected).is_equal_to(5) + + def test_multi_element_run_at_the_head_reports_only_that_run(self): + result = _build_equality_diff([7, 8, 1, 2, 3], [1, 2, 3]) + assert_that(result.entries).is_length(2) + assert_that([entry.path for entry in result.entries]).is_equal_to(["[0]", "[1]"]) + assert_that([entry.actual for entry in result.entries]).is_equal_to([7, 8]) + assert_that([entry.expected for entry in result.entries]).each(match.is_none()) + + def test_long_shifted_list_stays_one_entry(self): + # the payoff case: without alignment every index past the head would be reported + result = _build_equality_diff(list(range(40)), list(range(1, 40))) + assert_that(result.entries).is_length(1) + assert_that(result.entries[0].path).is_equal_to("[0]") + assert_that(result.entries[0].actual).is_equal_to(0) + + def test_shift_and_substitution_report_separately(self): + result = _build_equality_diff([0, 1, 9, 3], [1, 2, 3]) + paths = [entry.path for entry in result.entries] + assert_that(paths).is_equal_to(["[0]", "[2]"]) + assert_that(result.entries[0].actual).is_equal_to(0) + assert_that(result.entries[0].expected).is_none() + assert_that(result.entries[1].actual).is_equal_to(9) + assert_that(result.entries[1].expected).is_equal_to(2) + + def test_nested_sequence_is_aligned_too(self): + # the nested walker shares the same sequence engine, so a list inside a dict collapses too + entries = _sub_diff_entries({"xs": [0, 1, 2]}, {"xs": [1, 2]}, "") + assert_that(entries).is_length(1) + assert_that(entries[0].path).is_equal_to("xs[0]") + assert_that(entries[0].actual).is_equal_to(0) + assert_that(entries[0].expected).is_none() + + def test_unhashable_elements_fall_back_to_the_index_wise_walk(self): + # difflib cannot align what it cannot hash; the index-wise diff still has to report the change + actual = [{"id": 0}, {"id": 1}] + expected = [{"id": 1}] + result = _build_equality_diff(actual, expected) + assert_that(result.kind).is_equal_to("sequence") + assert_that(result.entries).is_not_empty() + assert_that([entry.path for entry in result.entries]).contains("[0].id") + + def test_alignment_is_skipped_past_the_size_cap(self): + # above the cap the quadratic opcode search is not worth it on a failure path + size = _ALIGN_MAX_ELEMENTS + 1 + result = _build_equality_diff(list(range(size)), list(range(1, size))) + assert_that(len(result.entries)).is_greater_than(1) + + def test_comparator_disagreeing_with_eq_still_reports_the_element(self): + # difflib aligns on ==; a comparator that calls == equal elements different must not be hidden + config = _build_compare_config(None, {int: lambda actual, expected: False}) + result = _build_equality_diff([0, 1, 2], [1, 2], config=config) + paths = [entry.path for entry in result.entries] + assert_that(paths).is_equal_to(["[0]", "[1]", "[2]"]) + + def test_tolerance_still_tolerates_aligned_elements(self): + # the extra head element and the 2.2/2.0 pair, then only the head element once 2.2 is tolerated + assert_that(_build_equality_diff([0.0, 1.0, 2.2], [1.0, 2.0]).entries).is_length(2) + config = _build_compare_config(0.5, None) + result = _build_equality_diff([0.0, 1.0, 2.2], [1.0, 2.0], config=config) + assert_that(result.entries).is_length(1) + assert_that(result.entries[0].path).is_equal_to("[0]") + assert_that(result.entries[0].actual).is_equal_to(0.0) + + class TestBuildEqualityDiffSet: def test_extra_items(self): result = _build_equality_diff({1, 2, 3}, {1})