diff --git a/tests/unit/test_live_server_asset_diff.py b/tests/unit/test_live_server_asset_diff.py new file mode 100644 index 0000000..d7c6807 --- /dev/null +++ b/tests/unit/test_live_server_asset_diff.py @@ -0,0 +1,240 @@ +"""Acceptance tests for the live-server ``asset_diff`` op (intentumdiff-vscode#25). + +The Rust core owns this behaviour and pins it with its own ``#[cfg(test)]`` tests; these +prove the guarantee survives the binding — that an editor sending a repo-relative path and a +ref really does get the engine's artifact manifest back over the protocol, rather than a +plausible-looking summary assembled on this side of the boundary. +""" +from __future__ import annotations + +import shutil +import struct +import subprocess +import zlib +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +# Reason string matches the skip ratchet's existing "platform" class (skip_reasons_baseline.json) +# rather than minting a new one — a new phrasing for an already-classified condition is exactly +# what that gate exists to stop. +pytestmark = pytest.mark.skipif(shutil.which("git") is None, reason="git not installed") + +# Every layer the perceptual viewer can render. The panel reported "perceptual diff pending" +# for as long as nothing asked the engine for these, so the test asserts all of them. +_ARTIFACT_LAYERS = ("before", "after", "diff", "heatmap", "mask", "overlay", "contact_sheet") + + +def _png_bytes(pixels: list[tuple[int, int, int]], width: int, height: int) -> bytes: + """A minimal RGB PNG, so the fixture needs no image library on the Python side.""" + raw = b"".join( + b"\x00" + b"".join(struct.pack("BBB", *pixels[y * width + x]) for x in range(width)) + for y in range(height) + ) + + def chunk(tag: bytes, data: bytes) -> bytes: + crc = zlib.crc32(tag + data) & 0xFFFFFFFF + return struct.pack(">I", len(data)) + tag + data + struct.pack(">I", crc) + + return ( + b"\x89PNG\r\n\x1a\n" + + chunk(b"IHDR", struct.pack(">IIBBBBB", width, height, 8, 2, 0, 0, 0)) + + chunk(b"IDAT", zlib.compress(raw)) + + chunk(b"IEND", b"") + ) + + +def _checkerboard(width: int, height: int) -> list[tuple[int, int, int]]: + return [ + (200, 40, 90) if (x // 4 + y // 4) % 2 else (30, 60, 140) + for y in range(height) + for x in range(width) + ] + + +def _git(repo: Path, *args: str) -> None: + subprocess.run( + ["git", "-C", str(repo), *args], + check=True, + capture_output=True, + ) + + +@pytest.fixture() +def image_repo(tmp_path: Path) -> Path: + """A git repo whose committed image differs from its working-tree copy.""" + repo = tmp_path / "repo" + (repo / "assets").mkdir(parents=True) + card = repo / "assets" / "card.png" + width, height = 32, 32 + card.write_bytes(_png_bytes(_checkerboard(width, height), width, height)) + _git(repo, "init") + _git(repo, "config", "user.email", "intentumdiff@example.test") + _git(repo, "config", "user.name", "IntentumDiff Test") + _git(repo, "add", ".") + _git(repo, "commit", "-m", "base") + + changed = _checkerboard(width, height) + for y in range(8, 22): + for x in range(6, 20): + changed[y * width + x] = (250, 250, 40) + card.write_bytes(_png_bytes(changed, width, height)) + return repo + + +def _server(repo: Path, ref: str = "HEAD"): + from intentumdiff.live_server import LiveServer + + differ = MagicMock() + differ._config = MagicMock() + differ._registry = MagicMock() + return LiveServer(differ, repo_path=str(repo), ref=ref) + + +def _asset_diff(repo: Path, request: dict, ref: str = "HEAD") -> dict: + sent: list[dict] = [] + _server(repo, ref)._process_request({"op": "asset_diff", "seq": 7, **request}, sent.append) + assert len(sent) == 1, sent + return sent[0] + + +class TestAssetDiffOp: + def test_tracked_image_returns_the_engines_artifact_manifest(self, image_repo: Path) -> None: + response = _asset_diff(image_repo, {"path": "assets/card.png"}) + + assert response["ok"] is True + assert response["op"] == "asset_diff" + assert response["seq"] == 7 + result = response["result"] + assert result["status"] == "compared" + assert result["file_path"] == "assets/card.png" + + for layer in _ARTIFACT_LAYERS: + artifact = result["artifacts"].get(layer) + assert artifact, f"engine did not report the {layer} artifact" + assert Path(artifact).is_file(), f"{layer} announced at {artifact} but not written" + + # The overlay geometry the viewer draws is only meaningful with these three together. + assert result["comparison_dimensions"] == {"width": 32, "height": 32} + assert result["hotspots"], "a 14x14 block change should surface at least one hotspot" + assert result["histograms"]["red_delta"] + assert result["changed_pixel_percentage"] > 0 + + def test_artifacts_land_in_a_cache_that_ignores_itself(self, image_repo: Path) -> None: + response = _asset_diff(image_repo, {"path": "assets/card.png"}) + + cache = image_repo / ".intentumdiff-cache" + artifact = Path(response["result"]["artifacts"]["heatmap"]).resolve() + assert cache.resolve() in artifact.parents, "artifacts must not land in the work tree" + assert "*" in (cache / ".gitignore").read_text(encoding="utf-8") + + def test_added_image_is_reported_as_skipped_with_a_reason(self, image_repo: Path) -> None: + (image_repo / "assets" / "new.png").write_bytes(_png_bytes(_checkerboard(8, 8), 8, 8)) + + result = _asset_diff(image_repo, {"path": "assets/new.png"})["result"] + + # An added image has nothing to compare against. Saying so is the point: the state this + # replaced claimed a comparison was merely "pending" and never produced one. + assert result["status"] == "skipped" + assert result["change_type"] == "A" + assert "no before" in result["reason"] + assert not result.get("artifacts") + + def test_deleted_image_is_reported_as_skipped(self, image_repo: Path) -> None: + (image_repo / "assets" / "card.png").unlink() + + result = _asset_diff(image_repo, {"path": "assets/card.png"})["result"] + + assert result["status"] == "skipped" + assert result["change_type"] == "D" + + def test_unresolvable_ref_is_an_error_not_an_added_image(self, image_repo: Path) -> None: + response = _asset_diff(image_repo, {"path": "assets/card.png", "ref": "no-such-ref"}) + + assert response["ok"] is False + assert "git rev not found" in response["error"]["message"] + + @pytest.mark.parametrize( + "path", + ["../outside.png", "..\\outside.png", "assets/../../outside.png"], + ) + def test_path_escaping_the_served_repo_is_refused(self, image_repo: Path, path: str) -> None: + response = _asset_diff(image_repo, {"path": path}) + + assert response["ok"] is False + assert response["error"]["code"] == "invalid_request" + + def test_request_without_a_path_or_pair_is_refused(self, image_repo: Path) -> None: + response = _asset_diff(image_repo, {}) + + assert response["ok"] is False + assert response["error"]["code"] == "invalid_request" + + def test_explicit_before_after_paths_still_work(self, image_repo: Path) -> None: + before = image_repo / "assets" / "before.png" + after = image_repo / "assets" / "after.png" + pixels = _checkerboard(16, 16) + before.write_bytes(_png_bytes(pixels, 16, 16)) + changed = list(pixels) + for index in range(20, 60): + changed[index] = (255, 255, 255) + after.write_bytes(_png_bytes(changed, 16, 16)) + + result = _asset_diff( + image_repo, + {"before_path": "assets/before.png", "after_path": "assets/after.png"}, + )["result"] + + assert result["status"] == "compared" + assert result["artifacts"]["overlay"] + + def test_serving_a_subdirectory_still_resolves_the_base_blob(self, image_repo: Path) -> None: + # A monorepo subfolder opened as a workspace addresses files relative to itself; git + # addresses blobs from the repository root. + result = _asset_diff(image_repo / "assets", {"path": "card.png"})["result"] + + assert result["status"] == "compared" + assert result["file_path"] == "card.png" + + def test_a_users_own_cache_gitignore_is_left_alone(self, image_repo: Path) -> None: + _asset_diff(image_repo, {"path": "assets/card.png"}) + marker = image_repo / ".intentumdiff-cache" / ".gitignore" + marker.write_text("mine\n", encoding="utf-8") + + _asset_diff(image_repo, {"path": "assets/card.png"}) + + assert marker.read_text(encoding="utf-8") == "mine\n" + + def test_a_non_image_path_is_an_error_not_a_comparison(self, image_repo: Path) -> None: + (image_repo / "notes.txt").write_text("not an image", encoding="utf-8") + + response = _asset_diff(image_repo, {"path": "notes.txt"}) + + assert response["ok"] is False + assert "asset diff failed" in response["error"]["message"] + + +class TestAssetDiffContract: + """The op is served by BOTH live-servers from one core implementation. + + The Python server and the native binary (``crates/live-server``) call the same + ``live_handle_asset_diff``, so what they advertise and what they answer cannot drift apart — + which matters because the extension prefers the native binary whenever one is bundled, and + for a while that binary had no arm for this op at all. + """ + + def test_capabilities_advertise_the_asset_diff_op(self, image_repo: Path) -> None: + capabilities = _server(image_repo)._capabilities() + + assert "asset_diff" in capabilities["operations"] + + def test_the_response_comes_from_the_core_not_this_layer(self, image_repo: Path) -> None: + from intentumdiff import rust_core + + request = {"op": "asset_diff", "seq": 7, "path": "assets/card.png"} + expected = rust_core.live_handle_asset_diff(str(image_repo), "HEAD", request, 7) + served = _asset_diff(image_repo, {"path": "assets/card.png"}) + + # Same request, same engine, same content-addressed output -> identical answer. + assert served == expected diff --git a/tests/unit/test_rename_does_not_hide_behaviour.py b/tests/unit/test_rename_does_not_hide_behaviour.py new file mode 100644 index 0000000..075852b --- /dev/null +++ b/tests/unit/test_rename_does_not_hide_behaviour.py @@ -0,0 +1,79 @@ +"""A rename must never hide a behavioural change in the same function. + +# Why this exists + +Renaming a function while editing its body is one of the commonest shapes in real review — +extract, rename, adjust. The engine collapsed that into a single `REFACTORING` change and +discarded the body edits with the DELETION/ADDITION pair it replaced. + +`REFACTORING` means "structure changed, behaviour did not". So the tool was not merely missing +a change; it was *asserting the change was safe* in its own vocabulary, and a reviewer acting +on that label would skim past it. + +The guard that allowed it asked "does the function start on the same line?" and, if so, treated +the rename as compatible without ever comparing the bodies. Same start line is evidence of +nothing. + +See intentumdiff-core#18. The Rust half lives in `draft_suppressors.rs`; this is the +acceptance half proving it survives the binding. +""" + +from __future__ import annotations + +from intentumdiff import SemanticDiffer + +BEFORE = "def total(cart):\n return sum(i.price for i in cart)\n" + +RENAMED_ONLY = "def calculate_total(cart):\n return sum(i.price for i in cart)\n" + +# Totals over 100 now get 10% off. A reviewer MUST see this. +BODY_CHANGED = ( + "def total(cart):\n" + " subtotal = sum(i.price for i in cart)\n" + " if subtotal > 100:\n" + " return subtotal * 0.9\n" + " return subtotal\n" +) + +RENAMED_AND_BODY_CHANGED = ( + "def calculate_total(cart):\n" + " subtotal = sum(i.price for i in cart)\n" + " if subtotal > 100:\n" + " return subtotal * 0.9\n" + " return subtotal\n" +) + + +def _kinds(old: str, new: str) -> list[str]: + diff = SemanticDiffer().diff_strings(old, new, "billing.py") + return [str(c.change_type).rsplit(".", 1)[-1] for c in diff.changes] + + +def test_a_rename_with_a_changed_body_is_not_reported_as_refactoring() -> None: + """THE regression. + + Not "reports more changes" — the load-bearing assertion is that it does NOT claim + REFACTORING, because that label tells a reviewer the behaviour is unchanged. + """ + kinds = _kinds(BEFORE, RENAMED_AND_BODY_CHANGED) + assert "REFACTORING" not in kinds, ( + "A rename that also changed the body was labelled REFACTORING — which asserts " + f"behaviour did not change. The discount logic would be skimmed. Got: {kinds}" + ) + assert kinds, "a rename plus a behavioural change must report something" + + +def test_a_pure_rename_is_still_one_refactoring() -> None: + """The fix must not be 'disable rename detection'. + + A rename with an untouched body is exactly what REFACTORING is for, and collapsing it to + one change is the value the feature adds. + """ + assert _kinds(BEFORE, RENAMED_ONLY) == ["REFACTORING"] + + +def test_a_body_change_without_a_rename_is_unaffected() -> None: + """Control: the path that always worked must keep working.""" + kinds = _kinds(BEFORE, BODY_CHANGED) + assert "REFACTORING" not in kinds + assert len(kinds) >= 3, f"expected the inserted statements to surface, got {kinds}"