From a488542f86c071774f4b3980b858477bf3fa8a8e Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 10 Jun 2026 01:25:21 +0200 Subject: [PATCH 1/3] Add Hypothesis tests for bump_lockfiles path resolution The path normalisation, deduplication, and ordering invariants in bump_lockfiles manifest resolution were exercised only by fixed parametrised cases. Add property tests covering the four invariants from issue #93: arbitrary in-workspace manifest strings (including redundant "." segments) always produce a normalised sibling Cargo.lock path; every spelling of the workspace root manifest deduplicates to exactly one root entry; the workspace root Cargo.lock is always the first element regardless of input order; and any manifest path escaping the workspace root raises LockfileRegenerationError. The escape test anchors the workspace in a subdirectory and excludes the degenerate suffix that would legitimately resolve back inside. Closes #93 --- tests/unit/test_bump_lockfiles.py | 99 ++++++++++++++++++++++++++++++- 1 file changed, 98 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_bump_lockfiles.py b/tests/unit/test_bump_lockfiles.py index 53998892..8e6ada15 100644 --- a/tests/unit/test_bump_lockfiles.py +++ b/tests/unit/test_bump_lockfiles.py @@ -7,12 +7,13 @@ import operator import pathlib import shlex +import string import tempfile import typing as typ from pathlib import Path import pytest -from hypothesis import given, settings +from hypothesis import assume, given, settings from hypothesis import strategies as st from lading.commands import bump_lockfiles @@ -275,6 +276,102 @@ def failing_runner( assert isinstance(exc_info.value.__cause__, OSError) +# --------------------------------------------------------------------------- +# Hypothesis property tests for manifest-path resolution (issue #93) +# --------------------------------------------------------------------------- + +_segment = st.text( + alphabet=string.ascii_lowercase + string.digits + "_-", + min_size=1, + max_size=10, +) + +# Relative directory paths that stay inside the workspace, optionally with +# redundant "." segments which normalise away. +_inside_dir = st.lists( + st.one_of(_segment, st.just(".")), + min_size=0, + max_size=4, +) + + +def _manifest_string(components: list[str]) -> str: + """Render a manifest path string from directory ``components``.""" + return "/".join((*components, "Cargo.toml")) + + +@given(dirs=st.lists(_inside_dir, max_size=6)) +@settings(max_examples=60, deadline=None) +def test_inside_manifests_resolve_to_sibling_lockfiles( + dirs: list[list[str]], +) -> None: + """Any in-workspace manifest string yields a sibling Cargo.lock path. + + Also pins the ordering and deduplication invariants: the workspace root + lockfile is always first, and resolved paths are unique. + """ + with tempfile.TemporaryDirectory(prefix="lading-bump-lockfiles-") as tmp: + workspace_root = Path(tmp) + manifests = [_manifest_string(components) for components in dirs] + + lockfiles = bump_lockfiles.resolve_lockfile_paths(workspace_root, manifests) + + resolved_root = workspace_root.resolve() + assert lockfiles[0] == resolved_root / "Cargo.lock" + assert len(set(lockfiles)) == len(lockfiles) + for lockfile_path in lockfiles: + assert lockfile_path.name == "Cargo.lock" + assert lockfile_path.parent == lockfile_path.parent.resolve() + lockfile_path.relative_to(resolved_root) + expected = {resolved_root / "Cargo.lock"} | { + (workspace_root.joinpath(*components, "Cargo.toml")).resolve().parent + / "Cargo.lock" + for components in dirs + } + assert set(lockfiles) == expected + + +@given(spellings=st.lists(st.sampled_from(["Cargo.toml", "./Cargo.toml"]), max_size=4)) +@settings(max_examples=20, deadline=None) +def test_root_manifest_spellings_deduplicate_to_one_invocation( + spellings: list[str], +) -> None: + """Every spelling of the root manifest produces exactly one root entry.""" + with tempfile.TemporaryDirectory(prefix="lading-bump-lockfiles-") as tmp: + workspace_root = Path(tmp) + + lockfiles = bump_lockfiles.resolve_lockfile_paths(workspace_root, spellings) + + assert lockfiles == (workspace_root.resolve() / "Cargo.lock",) + + +@given( + escape_depth=st.integers(min_value=1, max_value=3), + suffix=st.lists(_segment, max_size=2), +) +@settings(max_examples=30, deadline=None) +def test_escaping_manifests_are_rejected( + escape_depth: int, + suffix: list[str], +) -> None: + """Any manifest path escaping the workspace root raises an error.""" + with tempfile.TemporaryDirectory(prefix="lading-bump-lockfiles-") as tmp: + # Anchor inside a subdirectory so ".." segments cannot accidentally + # resolve back inside the temporary root. + workspace_root = Path(tmp) / "workspace" + workspace_root.mkdir() + # A suffix re-entering the workspace directory would resolve back + # inside and legitimately pass validation; exclude that case. + assume(suffix[:1] != ["workspace"]) + escaping = "/".join(([".."] * escape_depth) + [*suffix, "Cargo.toml"]) + + with pytest.raises( + bump_lockfiles.LockfileRegenerationError, + match="must stay within the workspace", + ): + bump_lockfiles.resolve_lockfile_paths(workspace_root, (escaping,)) + + # --------------------------------------------------------------------------- # Aggregated failure handling (issue #84) # --------------------------------------------------------------------------- From fb13d905d022f2d6fd37efe7055aaa09ef356b2e Mon Sep 17 00:00:00 2001 From: leynos Date: Tue, 14 Jul 2026 10:43:04 +0200 Subject: [PATCH 2/3] Address review feedback on bump_lockfiles property tests Respond to reviewer comments on the manifest-path resolution property tests: - Add diagnostic messages to every assertion in test_inside_manifests_resolve_to_sibling_lockfiles so a Hypothesis counter-example names the violated invariant. Convert the bare relative_to() containment call into an explicit is_relative_to assertion with a message. - Rename the module-level strategies to UPPER_SNAKE_CASE constants (_SEGMENT, _INSIDE_DIR) to match the naming convention for module-level values. - Strengthen the ordering invariant: compare the full resolved tuple against an ordered expected tuple built with dict.fromkeys (root Cargo.lock first, then per-manifest siblings in execution order, duplicates removed) instead of a set comparison, which permitted reordered nested lockfiles to pass. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/unit/test_bump_lockfiles.py | 53 +++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 16 deletions(-) diff --git a/tests/unit/test_bump_lockfiles.py b/tests/unit/test_bump_lockfiles.py index 8e6ada15..8e4f4616 100644 --- a/tests/unit/test_bump_lockfiles.py +++ b/tests/unit/test_bump_lockfiles.py @@ -280,7 +280,7 @@ def failing_runner( # Hypothesis property tests for manifest-path resolution (issue #93) # --------------------------------------------------------------------------- -_segment = st.text( +_SEGMENT = st.text( alphabet=string.ascii_lowercase + string.digits + "_-", min_size=1, max_size=10, @@ -288,8 +288,8 @@ def failing_runner( # Relative directory paths that stay inside the workspace, optionally with # redundant "." segments which normalise away. -_inside_dir = st.lists( - st.one_of(_segment, st.just(".")), +_INSIDE_DIR = st.lists( + st.one_of(_SEGMENT, st.just(".")), min_size=0, max_size=4, ) @@ -300,7 +300,7 @@ def _manifest_string(components: list[str]) -> str: return "/".join((*components, "Cargo.toml")) -@given(dirs=st.lists(_inside_dir, max_size=6)) +@given(dirs=st.lists(_INSIDE_DIR, max_size=6)) @settings(max_examples=60, deadline=None) def test_inside_manifests_resolve_to_sibling_lockfiles( dirs: list[list[str]], @@ -317,18 +317,39 @@ def test_inside_manifests_resolve_to_sibling_lockfiles( lockfiles = bump_lockfiles.resolve_lockfile_paths(workspace_root, manifests) resolved_root = workspace_root.resolve() - assert lockfiles[0] == resolved_root / "Cargo.lock" - assert len(set(lockfiles)) == len(lockfiles) + assert lockfiles[0] == resolved_root / "Cargo.lock", ( + "workspace root Cargo.lock must be the first resolved lockfile" + ) + assert len(set(lockfiles)) == len(lockfiles), ( + "resolved lockfiles must be unique" + ) for lockfile_path in lockfiles: - assert lockfile_path.name == "Cargo.lock" - assert lockfile_path.parent == lockfile_path.parent.resolve() - lockfile_path.relative_to(resolved_root) - expected = {resolved_root / "Cargo.lock"} | { - (workspace_root.joinpath(*components, "Cargo.toml")).resolve().parent - / "Cargo.lock" - for components in dirs - } - assert set(lockfiles) == expected + assert lockfile_path.name == "Cargo.lock", ( + f"resolved path must be a sibling Cargo.lock: {lockfile_path}" + ) + assert lockfile_path.parent == lockfile_path.parent.resolve(), ( + f"lockfile parent must be a normalised path: {lockfile_path}" + ) + assert lockfile_path.is_relative_to(resolved_root), ( + f"lockfile must stay within the workspace root: {lockfile_path}" + ) + # Build the expected ordered tuple: the workspace root Cargo.lock first, + # then the sibling lockfile for each manifest in execution order, with + # duplicates removed while preserving that order. + expected = tuple( + dict.fromkeys( + [resolved_root / "Cargo.lock"] + + [ + workspace_root.joinpath(*components, "Cargo.toml").resolve().parent + / "Cargo.lock" + for components in dirs + ] + ) + ) + assert lockfiles == expected, ( + "resolved lockfiles must preserve manifest execution order " + "without duplicates" + ) @given(spellings=st.lists(st.sampled_from(["Cargo.toml", "./Cargo.toml"]), max_size=4)) @@ -347,7 +368,7 @@ def test_root_manifest_spellings_deduplicate_to_one_invocation( @given( escape_depth=st.integers(min_value=1, max_value=3), - suffix=st.lists(_segment, max_size=2), + suffix=st.lists(_SEGMENT, max_size=2), ) @settings(max_examples=30, deadline=None) def test_escaping_manifests_are_rejected( From fa8c1b2bf66b78fff09da691548dca2233e4036f Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 19 Jul 2026 22:54:59 +0200 Subject: [PATCH 3/3] Exercise safe ".." traversal in bump_lockfiles path property test Extend the _INSIDE_DIR Hypothesis strategy so generated in-workspace manifest paths also include ".." parent-traversal segments alongside real and "." segments. A composite strategy tracks the running segment balance and only emits ".." while a prior real segment remains to cancel it, so paths such as crate/../Cargo.toml are exercised while never ascending above the workspace root after normalisation. The existing list bounds (0-4 segments) are preserved, and resolve_lockfile_paths continues to accept every generated manifest. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/unit/test_bump_lockfiles.py | 36 +++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/tests/unit/test_bump_lockfiles.py b/tests/unit/test_bump_lockfiles.py index 8e4f4616..894b9332 100644 --- a/tests/unit/test_bump_lockfiles.py +++ b/tests/unit/test_bump_lockfiles.py @@ -286,13 +286,37 @@ def failing_runner( max_size=10, ) + +@st.composite +def _inside_dir(draw: st.DrawFn) -> list[str]: + """Draw a relative directory path that stays within the workspace. + + Alongside real segments and redundant ``.`` segments, this emits safe + ``..`` parent-traversal segments that never ascend above the workspace + root after normalisation: a ``..`` is only produced while a prior real + segment remains to cancel it (the running segment balance never drops + below zero). Cases such as ``crate/../Cargo.toml`` are therefore + exercised without allowing traversal above the root. + """ + length = draw(st.integers(min_value=0, max_value=4)) + components: list[str] = [] + depth = 0 + for _ in range(length): + options = [_SEGMENT, st.just(".")] + if depth > 0: + options.append(st.just("..")) + segment = draw(st.one_of(*options)) + if segment == "..": + depth -= 1 + elif segment != ".": + depth += 1 + components.append(segment) + return components + + # Relative directory paths that stay inside the workspace, optionally with -# redundant "." segments which normalise away. -_INSIDE_DIR = st.lists( - st.one_of(_SEGMENT, st.just(".")), - min_size=0, - max_size=4, -) +# redundant "." segments and safe ".." traversals which normalise away. +_INSIDE_DIR = _inside_dir() def _manifest_string(components: list[str]) -> str: