From 42e0150d3477f6ed4941632455ecdbd70a184469 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 1 Jun 2026 23:20:48 +0200 Subject: [PATCH 01/24] Refresh stale Cargo lockfiles during bump (#61) Add tracked `Cargo.lock` discovery and refresh support after manifest version rewrites so nested workspace lockfiles stay aligned with bumped path dependency versions. Validate tracked lockfiles before publish preflight cargo checks and report stale files with the exact `cargo generate-lockfile` repair command. Cover the behaviour with unit and BDD tests for live bump, dry-run bump, and publish preflight failures. --- lading/commands/bump.py | 46 ++++- lading/commands/lockfile.py | 95 +++++++++++ lading/commands/publish.py | 6 + lading/commands/publish_preflight.py | 50 ++++++ tests/bdd/features/cli.feature | 32 ++++ tests/bdd/steps/test_bump_steps.py | 46 ++++- tests/bdd/steps/test_publish_given_steps.py | 23 ++- .../bdd/steps/test_publish_infrastructure.py | 43 +++++ tests/unit/publish/test_preflight_checks.py | 75 +++++++++ tests/unit/test_bump_command_integration.py | 85 ++++++++++ tests/unit/test_lockfile.py | 157 ++++++++++++++++++ 11 files changed, 653 insertions(+), 5 deletions(-) create mode 100644 lading/commands/lockfile.py create mode 100644 tests/unit/test_lockfile.py diff --git a/lading/commands/bump.py b/lading/commands/bump.py index 7cf365d2..176614b9 100644 --- a/lading/commands/bump.py +++ b/lading/commands/bump.py @@ -4,11 +4,14 @@ import collections.abc as cabc import dataclasses as dc +import logging import types import typing as typ from lading import config as config_module from lading.commands import bump_docs, bump_toml +from lading.commands.lockfile import discover_tracked_lockfiles, refresh_lockfile +from lading.commands.publish_execution import _CommandRunner, _invoke from lading.utils import normalise_workspace_root if typ.TYPE_CHECKING: @@ -31,6 +34,8 @@ "build": "build-dependencies", } +LOGGER = logging.getLogger(__name__) + @dc.dataclass(frozen=True, slots=True) class BumpOptions: @@ -43,6 +48,7 @@ class BumpOptions: default_factory=lambda: types.MappingProxyType({}) ) include_workspace_sections: bool = False + runner: _CommandRunner | None = None @dc.dataclass(frozen=True, slots=True) @@ -51,6 +57,7 @@ class BumpChanges: manifests: cabc.Sequence[Path] = () documents: cabc.Sequence[Path] = () + lockfiles: cabc.Sequence[Path] = () @dc.dataclass(frozen=True, slots=True) @@ -73,6 +80,8 @@ def _build_changes_description(changes: BumpChanges) -> str: parts.append(f"{len(changes.manifests)} manifest(s)") if changes.documents: parts.append(f"{len(changes.documents)} documentation file(s)") + if changes.lockfiles: + parts.append(f"{len(changes.lockfiles)} lockfile(s)") return parts[0] if len(parts) == 1 else " and ".join(parts) @@ -105,7 +114,10 @@ def run( _process_workspace_manifest(context, target_version, changed_manifests) _process_crate_manifests(context, target_version, changed_manifests) changed_documents = _process_documentation_files(context, target_version) - changes = _prepare_sorted_changes(context, changed_manifests, changed_documents) + refreshed_lockfiles = _refresh_lockfiles(context) + changes = _prepare_sorted_changes( + context, changed_manifests, changed_documents, refreshed_lockfiles + ) return _format_result_message( changes, target_version, @@ -135,6 +147,7 @@ def _initialize_bump_context( dry_run=resolved_options.dry_run, configuration=configuration, workspace=workspace, + runner=resolved_options.runner, ) excluded = frozenset(configuration.bump.exclude) updated_crate_names = frozenset( @@ -204,6 +217,7 @@ def _prepare_sorted_changes( context: _BumpContext, changed_manifests: set[Path], changed_documents: set[Path], + refreshed_lockfiles: cabc.Sequence[Path] = (), ) -> BumpChanges: """Return ordered :class:`BumpChanges` suitable for result rendering.""" ordered_manifests = tuple( @@ -215,7 +229,29 @@ def _prepare_sorted_changes( ordered_documents: tuple[Path, ...] = tuple( sorted(changed_documents, key=lambda path: str(path)) ) - return BumpChanges(manifests=ordered_manifests, documents=ordered_documents) + ordered_lockfiles: tuple[Path, ...] = tuple( + sorted(refreshed_lockfiles, key=lambda path: str(path)) + ) + return BumpChanges( + manifests=ordered_manifests, + documents=ordered_documents, + lockfiles=ordered_lockfiles, + ) + + +def _refresh_lockfiles(context: _BumpContext) -> tuple[Path, ...]: + """Refresh tracked lockfiles after manifest rewrites.""" + runner = context.base_options.runner or _invoke + lockfiles = discover_tracked_lockfiles(context.root_path, runner) + if context.base_options.dry_run: + for lockfile_path in lockfiles: + LOGGER.info("Dry run; would refresh %s", lockfile_path) + return lockfiles + + return tuple( + refresh_lockfile(lockfile_path.parent / "Cargo.toml", runner) + for lockfile_path in lockfiles + ) def _update_crate_manifest( @@ -257,7 +293,7 @@ def _format_result_message( workspace_root: Path, ) -> str: """Summarise the bump outcome for CLI presentation.""" - if not changes.manifests and not changes.documents: + if not any((changes.manifests, changes.documents, changes.lockfiles)): return _format_no_changes_message(target_version, dry_run) description = _build_changes_description(changes) @@ -270,6 +306,10 @@ def _format_result_message( f"- {_format_manifest_path(document_path, workspace_root)} (documentation)" for document_path in changes.documents ) + formatted_paths.extend( + f"- {_format_manifest_path(lockfile_path, workspace_root)} (lockfile)" + for lockfile_path in changes.lockfiles + ) return "\n".join([header, *formatted_paths]) diff --git a/lading/commands/lockfile.py b/lading/commands/lockfile.py new file mode 100644 index 00000000..ba88a64a --- /dev/null +++ b/lading/commands/lockfile.py @@ -0,0 +1,95 @@ +"""Cargo lockfile discovery, refresh, and freshness validation helpers.""" + +from __future__ import annotations + +import logging +import typing as typ +from pathlib import Path + +if typ.TYPE_CHECKING: + from lading.commands.publish_execution import _CommandRunner + +LOGGER = logging.getLogger(__name__) + + +class LockfileRefreshError(RuntimeError): + """Raised when Cargo cannot regenerate a lockfile.""" + + +def discover_tracked_lockfiles( + workspace_root: Path, + runner: _CommandRunner, +) -> tuple[Path, ...]: + """Return tracked Cargo.lock files with adjacent manifests.""" + candidate_lockfiles = tuple( + path + for path in workspace_root.rglob("Cargo.lock") + if "target" not in path.relative_to(workspace_root).parts + ) + if not candidate_lockfiles: + return () + + exit_code, stdout, stderr = runner( + ("git", "ls-files", "*/Cargo.lock", "Cargo.lock"), + cwd=workspace_root, + ) + if exit_code != 0: + detail = (stderr or stdout).strip() + if "not a git repository" in detail.lower(): + LOGGER.warning( + "Skipping Cargo.lock discovery because %s is not a git repository", + workspace_root, + ) + return () + LOGGER.warning( + "Skipping Cargo.lock discovery after git ls-files failed: %s", detail + ) + return () + + lockfiles: list[Path] = [] + for line in stdout.splitlines(): + relative_path = line.strip() + if not relative_path: + continue + lockfile_path = workspace_root / relative_path + manifest_path = lockfile_path.parent / "Cargo.toml" + if manifest_path.exists(): + lockfiles.append(lockfile_path) + return tuple(lockfiles) + + +def refresh_lockfile( + manifest_path: Path, + runner: _CommandRunner, +) -> Path: + """Regenerate the lockfile for ``manifest_path`` and return its path.""" + exit_code, stdout, stderr = runner( + ("cargo", "generate-lockfile", "--manifest-path", str(manifest_path)), + cwd=manifest_path.parent, + ) + if exit_code != 0: + detail = (stderr or stdout).strip() + message = f"Failed to refresh {manifest_path.parent / 'Cargo.lock'}" + if detail: + message = f"{message}: {detail}" + raise LockfileRefreshError(message) + return manifest_path.parent / "Cargo.lock" + + +def validate_lockfile_freshness( + manifest_path: Path, + runner: _CommandRunner, +) -> bool: + """Return whether Cargo accepts ``manifest_path`` under ``--locked``.""" + exit_code, _stdout, _stderr = runner( + ( + "cargo", + "metadata", + "--locked", + "--manifest-path", + str(manifest_path), + "--format-version=1", + ), + cwd=manifest_path.parent, + ) + return exit_code == 0 diff --git a/lading/commands/publish.py b/lading/commands/publish.py index fca64656..f30aa9ef 100644 --- a/lading/commands/publish.py +++ b/lading/commands/publish.py @@ -89,6 +89,7 @@ _compose_preflight_arguments, _run_aux_build_commands, _run_cargo_preflight, + _validate_lockfile_freshness, _verify_clean_working_tree, ) from lading.utils.path import normalise_workspace_root @@ -743,6 +744,11 @@ def _run_preflight_checks( runner=command_runner, env=base_env, ) + _validate_lockfile_freshness( + workspace_root, + runner=command_runner, + env=base_env, + ) with tempfile.TemporaryDirectory(prefix="lading-preflight-target-") as target: target_path = Path(target) diff --git a/lading/commands/publish_preflight.py b/lading/commands/publish_preflight.py index 5b39fad7..440204c5 100644 --- a/lading/commands/publish_preflight.py +++ b/lading/commands/publish_preflight.py @@ -28,6 +28,10 @@ import typing as typ from pathlib import Path +from lading.commands.lockfile import ( + discover_tracked_lockfiles, + validate_lockfile_freshness, +) from lading.commands.publish_diagnostics import _append_compiletest_diagnostics from lading.commands.publish_errors import PublishPreflightError from lading.commands.publish_execution import _invoke @@ -134,6 +138,11 @@ def _run_preflight_checks( runner=command_runner, env=base_env, ) + _validate_lockfile_freshness( + workspace_root, + runner=command_runner, + env=base_env, + ) with tempfile.TemporaryDirectory(prefix="lading-preflight-target-") as target: target_path = Path(target) @@ -179,7 +188,48 @@ def _compose_preflight_arguments( arguments.append(f"--target-dir={target_dir}") return tuple(arguments) +def _validate_lockfile_freshness( + workspace_root: Path, + *, + runner: _CommandRunner, + env: cabc.Mapping[str, str] | None = None, +) -> None: + """Fail early when tracked Cargo.lock files are stale.""" + base_env = env + + def runner_with_env( + command: cabc.Sequence[str], + *, + cwd: Path | None = None, + env: cabc.Mapping[str, str] | None = None, + ) -> tuple[int, str, str]: + effective_env = base_env if env is None else env + return runner(command, cwd=cwd, env=effective_env) + + stale_lockfiles: list[Path] = [] + for lockfile_path in discover_tracked_lockfiles(workspace_root, runner_with_env): + manifest_path = lockfile_path.parent / "Cargo.toml" + if not validate_lockfile_freshness(manifest_path, runner_with_env): + stale_lockfiles.append(lockfile_path) + + if not stale_lockfiles: + return + lines = [ + "Tracked Cargo.lock files are stale after manifest version changes.", + ( + "This commonly happens after running `lading bump`; re-run it to " + "refresh tracked lockfiles, or repair each stale lockfile manually:" + ), + ] + for lockfile_path in stale_lockfiles: + manifest_path = lockfile_path.parent / "Cargo.toml" + lines.append(f"- {lockfile_path}") + lines.append(f" cargo generate-lockfile --manifest-path {manifest_path}") + + message = "\n".join(lines) + LOGGER.error(message) + raise PublishPreflightError(message) def _preflight_argument_sets( target_dir: Path, *, unit_tests_only: bool ) -> tuple[tuple[str, ...], tuple[str, ...]]: diff --git a/tests/bdd/features/cli.feature b/tests/bdd/features/cli.feature index 3d9ffff2..47f8d064 100644 --- a/tests/bdd/features/cli.feature +++ b/tests/bdd/features/cli.feature @@ -17,6 +17,22 @@ Feature: Lading CLI scaffolding And the workspace manifest version is "0.1.0" And the crate "alpha" manifest version is "0.1.0" + Scenario: Bump refreshes tracked Cargo.lock files + Given a workspace directory with configuration + And cargo metadata describes a sample workspace + And the workspace has tracked Cargo.lock files + When I invoke lading bump 1.2.3 with that workspace + Then the CLI output lists lockfile path "- Cargo.lock (lockfile)" + And the bump command refreshed tracked lockfiles + + Scenario: Bump dry-run does not modify lockfiles + Given a workspace directory with configuration + And cargo metadata describes a sample workspace + And the workspace has tracked Cargo.lock files + When I invoke lading bump 1.2.3 with that workspace using --dry-run + Then the CLI output lists lockfile path "- Cargo.lock (lockfile)" + And the bump command did not refresh tracked lockfiles + Scenario: Bumping with an invalid version fails fast Given a workspace directory with configuration When I invoke lading bump 1.2 with that workspace @@ -182,6 +198,22 @@ Feature: Lading CLI scaffolding Then the CLI exits with code 1 And the stderr contains "Pre-flight cargo test failed with exit code 1: cargo test failed" + Scenario: Publish pre-flight aborts when lockfile is stale + Given a workspace directory with configuration + And cargo metadata describes a sample workspace + And publish pre-flight finds a stale tracked Cargo.lock + When I invoke lading publish with that workspace + Then the CLI exits with code 1 + And the stderr contains "Tracked Cargo.lock files are stale after manifest version changes." + + Scenario: Publish pre-flight error includes repair command + Given a workspace directory with configuration + And cargo metadata describes a sample workspace + And publish pre-flight finds a stale tracked Cargo.lock + When I invoke lading publish with that workspace + Then the CLI exits with code 1 + And the stderr contains "cargo generate-lockfile --manifest-path" + Scenario: Publish pre-flight skips configured cargo test crates Given a workspace directory with configuration And cargo metadata describes a sample workspace diff --git a/tests/bdd/steps/test_bump_steps.py b/tests/bdd/steps/test_bump_steps.py index 9c4b3571..579e14f1 100644 --- a/tests/bdd/steps/test_bump_steps.py +++ b/tests/bdd/steps/test_bump_steps.py @@ -4,7 +4,7 @@ import typing as typ -from pytest_bdd import parsers, then, when +from pytest_bdd import given, parsers, then, when from . import config_fixtures as _config_fixtures # noqa: F401 from . import manifest_fixtures as _manifest_fixtures # noqa: F401 @@ -13,9 +13,31 @@ if typ.TYPE_CHECKING: from pathlib import Path + import pytest + from cmd_mox import CmdMox + from .test_common_steps import _run_cli # noqa: F401 +@given("the workspace has tracked Cargo.lock files") +def given_workspace_has_tracked_lockfiles( + cmd_mox: CmdMox, + monkeypatch: pytest.MonkeyPatch, + workspace_directory: Path, +) -> None: + """Stub tracked Cargo.lock discovery and refresh commands for bump.""" + from tests.helpers.workspace_helpers import install_cargo_stub + + install_cargo_stub(cmd_mox, monkeypatch) + (workspace_directory / "Cargo.lock").write_text("# root lock\n", encoding="utf-8") + cmd_mox.stub("git").with_args( + "ls-files", "*/Cargo.lock", "Cargo.lock" + ).returns(exit_code=0, stdout="Cargo.lock\n", stderr="") + cmd_mox.stub("cargo::generate-lockfile").with_args( + "--manifest-path", str(workspace_directory / "Cargo.toml") + ).returns(exit_code=0, stdout="", stderr="") + + @when( parsers.parse("I invoke lading bump {version} with that workspace"), target_fixture="cli_run", @@ -117,6 +139,28 @@ def then_cli_output_lists_documentation_path( assert expected in stdout_lines +@then(parsers.parse('the CLI output lists lockfile path "{expected}"')) +def then_cli_output_lists_lockfile_path( + cli_run: dict[str, typ.Any], expected: str +) -> None: + """Assert that the CLI output includes ``expected`` as a lockfile line.""" + assert cli_run["returncode"] == 0 + stdout_lines = [line.strip() for line in cli_run["stdout"].splitlines()] + assert expected in stdout_lines + + +@then("the bump command refreshed tracked lockfiles") +def then_bump_refreshed_lockfiles(cli_run: dict[str, typ.Any]) -> None: + """Assert the live bump lockfile scenario completed successfully.""" + assert cli_run["returncode"] == 0 + + +@then("the bump command did not refresh tracked lockfiles") +def then_bump_did_not_refresh_lockfiles(cli_run: dict[str, typ.Any]) -> None: + """Assert the dry-run lockfile scenario completed without refresh.""" + assert cli_run["returncode"] == 0 + + @then(parsers.parse('the documentation file "{relative_path}" contains "{expected}"')) def then_documentation_contains( cli_run: dict[str, typ.Any], relative_path: str, expected: str diff --git a/tests/bdd/steps/test_publish_given_steps.py b/tests/bdd/steps/test_publish_given_steps.py index 431d27c8..1cc81e19 100644 --- a/tests/bdd/steps/test_publish_given_steps.py +++ b/tests/bdd/steps/test_publish_given_steps.py @@ -66,7 +66,28 @@ def given_cargo_test_fails( exit_code=1, stderr="cargo test failed" ) - +@given("publish pre-flight finds a stale tracked Cargo.lock") +def given_publish_preflight_finds_stale_lockfile( + workspace_directory: Path, + preflight_overrides: dict[tuple[str, ...], ResponseProvider], +) -> None: + """Simulate a tracked lockfile that fails locked metadata validation.""" + (workspace_directory / "Cargo.lock").write_text("# stale lock\n", encoding="utf-8") + preflight_overrides["git", "ls-files", "*/Cargo.lock", "Cargo.lock"] = ( + _CommandResponse(exit_code=0, stdout="Cargo.lock\n") + ) + preflight_overrides[ + "cargo", + "metadata", + "--locked", + "--manifest-path", + ] = _CommandResponse( + exit_code=101, + stderr=( + f"error: cannot update the lock file {workspace_directory / 'Cargo.lock'} " + "because --locked was passed to prevent this" + ), + ) @given(parsers.parse('cargo test fails with compiletest artifact "{relative_path}"')) def given_cargo_test_fails_with_artifact( workspace_directory: Path, diff --git a/tests/bdd/steps/test_publish_infrastructure.py b/tests/bdd/steps/test_publish_infrastructure.py index c744bf3a..b04accbe 100644 --- a/tests/bdd/steps/test_publish_infrastructure.py +++ b/tests/bdd/steps/test_publish_infrastructure.py @@ -185,6 +185,12 @@ def _register_preflight_commands( """ defaults: dict[tuple[str, ...], ResponseProvider] = { ("git", "status", "--porcelain"): _CommandResponse(exit_code=0), + ( + "cargo", + "metadata", + "--locked", + "--manifest-path", + ): _CommandResponse(exit_code=0), ( "cargo", "check", @@ -234,6 +240,20 @@ def _register_preflight_commands( defaults |= normalized_overrides defaults[publish_command] = publish_response + git_responses = { + command[1:]: response + for command, response in defaults.items() + if command[0] == "git" + } + defaults = { + command: response + for command, response in defaults.items() + if command[0] != "git" + } + if git_responses: + config.cmd_mox.stub("git").runs( + _make_git_handler(git_responses, config.recorder) + ) for command, response in defaults.items(): expectation_program, expectation_args = _resolve_preflight_expectation(command) config.cmd_mox.stub(expectation_program).runs( @@ -242,7 +262,30 @@ def _register_preflight_commands( ) ) +def _make_git_handler( + responses: dict[tuple[str, ...], ResponseProvider], + recorder: _PreflightInvocationRecorder | None, +) -> cabc.Callable[[_CmdInvocation], tuple[str, str, int]]: + """Build a git handler that can serve multiple git subcommands.""" + def _handler(invocation: _CmdInvocation) -> tuple[str, str, int]: + args = tuple(invocation.args) + try: + response = responses[args] + except KeyError as exc: + message = f"Unexpected git invocation arguments: {args!r}" + raise AssertionError(message) from exc + active_response = response(invocation) if callable(response) else response + if recorder is not None: + env_mapping = dict(getattr(invocation, "env", {})) + recorder.record("git", args, env_mapping) + return ( + active_response.stdout, + active_response.stderr, + active_response.exit_code, + ) + + return _handler def _build_env_restore_dict(var_name: str) -> dict[str, str]: """Build a dictionary for restoring an environment variable.""" previous = os.environ.get(var_name) diff --git a/tests/unit/publish/test_preflight_checks.py b/tests/unit/publish/test_preflight_checks.py index c455ed62..f8fc116d 100644 --- a/tests/unit/publish/test_preflight_checks.py +++ b/tests/unit/publish/test_preflight_checks.py @@ -349,7 +349,82 @@ def runner( assert "cargo build --package lint" in str(excinfo.value) +def test_validate_lockfile_freshness_passes_when_all_lockfiles_are_fresh( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """Fresh tracked lockfiles allow preflight to continue.""" + root_lockfile = tmp_path / "Cargo.lock" + nested_lockfile = tmp_path / "tests" / "ui_lints" / "Cargo.lock" + recorded_env: list[cabc.Mapping[str, str] | None] = [] + + monkeypatch.setattr( + publish._publish_preflight, + "discover_tracked_lockfiles", + lambda _root, _runner: (root_lockfile, nested_lockfile), + ) + monkeypatch.setattr( + publish._publish_preflight, + "validate_lockfile_freshness", + lambda _manifest, runner: runner(("cargo", "metadata"), cwd=tmp_path)[0] == 0, + ) + + def runner( + command: tuple[str, ...], + *, + cwd: Path | None = None, + env: cabc.Mapping[str, str] | None = None, + ) -> tuple[int, str, str]: + recorded_env.append(env) + return 0, "", "" + publish._validate_lockfile_freshness( + tmp_path, + runner=runner, + env={"CARGO_TERM_COLOR": "never"}, + ) + + assert recorded_env == [{"CARGO_TERM_COLOR": "never"}] * 2 + +def test_validate_lockfile_freshness_reports_stale_lockfiles( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """Stale lockfiles are collected and reported with repair commands.""" + root_lockfile = tmp_path / "Cargo.lock" + nested_lockfile = tmp_path / "tests" / "ui_lints" / "Cargo.lock" + + monkeypatch.setattr( + publish._publish_preflight, + "discover_tracked_lockfiles", + lambda _root, _runner: (root_lockfile, nested_lockfile), + ) + monkeypatch.setattr( + publish._publish_preflight, + "validate_lockfile_freshness", + lambda _manifest, _runner: False, + ) + + def runner( + command: tuple[str, ...], + *, + cwd: Path | None = None, + env: cabc.Mapping[str, str] | None = None, + ) -> tuple[int, str, str]: + return 0, "", "" + + with pytest.raises(publish.PublishPreflightError) as excinfo: + publish._validate_lockfile_freshness(tmp_path, runner=runner, env={}) + + message = str(excinfo.value) + assert str(root_lockfile) in message + assert str(nested_lockfile) in message + assert "lading bump" in message + assert ( + f"cargo generate-lockfile --manifest-path {tmp_path / 'Cargo.toml'}" in message + ) + assert ( + "cargo generate-lockfile --manifest-path " + f"{tmp_path / 'tests' / 'ui_lints' / 'Cargo.toml'}" + ) in message def test_preflight_env_overrides_forwarded( monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: diff --git a/tests/unit/test_bump_command_integration.py b/tests/unit/test_bump_command_integration.py index 64646b33..65fb6913 100644 --- a/tests/unit/test_bump_command_integration.py +++ b/tests/unit/test_bump_command_integration.py @@ -25,6 +25,9 @@ ) if typ.TYPE_CHECKING: + import collections.abc as cabc + from pathlib import Path + from _pytest.monkeypatch import MonkeyPatch @@ -65,6 +68,88 @@ def test_run_updates_workspace_and_members(tmp_path: pathlib.Path) -> None: assert _load_version(crate.manifest_path, ("package",)) == "1.2.3" +def test_run_refreshes_tracked_lockfiles( + tmp_path: pathlib.Path, monkeypatch: MonkeyPatch +) -> None: + """`bump.run` refreshes every tracked lockfile after manifest updates.""" + workspace = _make_workspace(tmp_path) + lockfiles = ( + tmp_path / "Cargo.lock", + tmp_path / "tests" / "ui_lints" / "Cargo.lock", + ) + refreshed: list[pathlib.Path] = [] + + def discover( + workspace_root: pathlib.Path, + runner: bump._CommandRunner, + ) -> tuple[pathlib.Path, ...]: + assert workspace_root == tmp_path + return lockfiles + + def refresh( + manifest_path: pathlib.Path, + runner: bump._CommandRunner, + ) -> pathlib.Path: + refreshed.append(manifest_path.parent / "Cargo.lock") + return manifest_path.parent / "Cargo.lock" + + def runner( + command: cabc.Sequence[str], + *, + cwd: Path | None = None, + env: cabc.Mapping[str, str] | None = None, + ) -> tuple[int, str, str]: + return 0, "", "" + + monkeypatch.setattr(bump, "discover_tracked_lockfiles", discover) + monkeypatch.setattr(bump, "refresh_lockfile", refresh) + + message = bump.run( + tmp_path, + "1.2.3", + options=bump.BumpOptions( + configuration=_make_config(), workspace=workspace, runner=runner + ), + ) + + assert tuple(refreshed) == lockfiles + assert "- Cargo.lock (lockfile)" in message.splitlines() + assert "- tests/ui_lints/Cargo.lock (lockfile)" in message.splitlines() + + +def test_run_dry_run_reports_lockfiles_without_refreshing( + tmp_path: pathlib.Path, monkeypatch: MonkeyPatch +) -> None: + """Dry-run bump lists tracked lockfiles but does not regenerate them.""" + workspace = _make_workspace(tmp_path) + lockfiles = (tmp_path / "Cargo.lock",) + + def discover( + workspace_root: pathlib.Path, + runner: bump._CommandRunner, + ) -> tuple[pathlib.Path, ...]: + return lockfiles + + def refresh( + manifest_path: pathlib.Path, + runner: bump._CommandRunner, + ) -> pathlib.Path: + pytest.fail("dry-run must not refresh lockfiles") + + monkeypatch.setattr(bump, "discover_tracked_lockfiles", discover) + monkeypatch.setattr(bump, "refresh_lockfile", refresh) + + message = bump.run( + tmp_path, + "1.2.3", + options=bump.BumpOptions( + dry_run=True, configuration=_make_config(), workspace=workspace + ), + ) + + assert "- Cargo.lock (lockfile)" in message.splitlines() + + def test_run_updates_root_package_section(tmp_path: pathlib.Path) -> None: """The workspace manifest `[package]` section also receives the new version.""" workspace = _make_workspace(tmp_path) diff --git a/tests/unit/test_lockfile.py b/tests/unit/test_lockfile.py new file mode 100644 index 00000000..cdc423b5 --- /dev/null +++ b/tests/unit/test_lockfile.py @@ -0,0 +1,157 @@ +"""Unit tests for Cargo lockfile helper functions.""" + +from __future__ import annotations + +import collections.abc as cabc +import typing as typ + +import pytest + +from lading.commands import lockfile + +if typ.TYPE_CHECKING: + from pathlib import Path + + +def test_discover_tracked_lockfiles_returns_empty_result(tmp_path: Path) -> None: + """Empty git output produces no lockfiles.""" + (tmp_path / "Cargo.lock").write_text("", encoding="utf-8") + + def runner( + command: cabc.Sequence[str], + *, + cwd: Path | None = None, + env: cabc.Mapping[str, str] | None = None, + ) -> tuple[int, str, str]: + assert command == ("git", "ls-files", "*/Cargo.lock", "Cargo.lock") + assert cwd == tmp_path + return 0, "", "" + + assert lockfile.discover_tracked_lockfiles(tmp_path, runner) == () + + +def test_discover_tracked_lockfiles_filters_missing_manifests(tmp_path: Path) -> None: + """Only tracked lockfiles next to Cargo.toml files are returned.""" + root_manifest = tmp_path / "Cargo.toml" + root_manifest.write_text("[workspace]\n", encoding="utf-8") + (tmp_path / "Cargo.lock").write_text("", encoding="utf-8") + nested = tmp_path / "tests" / "ui_lints" + nested.mkdir(parents=True) + (nested / "Cargo.toml").write_text("[package]\n", encoding="utf-8") + (nested / "Cargo.lock").write_text("", encoding="utf-8") + target = tmp_path / "target" / "debug" + target.mkdir(parents=True) + (target / "Cargo.lock").write_text("", encoding="utf-8") + + def runner( + command: cabc.Sequence[str], + *, + cwd: Path | None = None, + env: cabc.Mapping[str, str] | None = None, + ) -> tuple[int, str, str]: + return ( + 0, + ( + "Cargo.lock\n" + "tests/ui_lints/Cargo.lock\n" + "target/debug/Cargo.lock\n" + "orphan/Cargo.lock\n" + ), + "", + ) + + assert lockfile.discover_tracked_lockfiles(tmp_path, runner) == ( + tmp_path / "Cargo.lock", + nested / "Cargo.lock", + ) + + +def test_discover_tracked_lockfiles_handles_non_git_directory( + tmp_path: Path, +) -> None: + """Non-git workspaces do not abort lockfile discovery.""" + (tmp_path / "Cargo.lock").write_text("", encoding="utf-8") + + def runner( + command: cabc.Sequence[str], + *, + cwd: Path | None = None, + env: cabc.Mapping[str, str] | None = None, + ) -> tuple[int, str, str]: + return 128, "", "fatal: not a git repository" + + assert lockfile.discover_tracked_lockfiles(tmp_path, runner) == () + + +def test_refresh_lockfile_returns_lockfile_path(tmp_path: Path) -> None: + """Successful lockfile refresh returns the expected Cargo.lock path.""" + manifest = tmp_path / "Cargo.toml" + + def runner( + command: cabc.Sequence[str], + *, + cwd: Path | None = None, + env: cabc.Mapping[str, str] | None = None, + ) -> tuple[int, str, str]: + assert command == ( + "cargo", + "generate-lockfile", + "--manifest-path", + str(manifest), + ) + assert cwd == manifest.parent + return 0, "", "" + + assert lockfile.refresh_lockfile(manifest, runner) == tmp_path / "Cargo.lock" + + +def test_refresh_lockfile_raises_on_failure(tmp_path: Path) -> None: + """Refresh failures include cargo stderr in the raised error.""" + manifest = tmp_path / "Cargo.toml" + + def runner( + command: cabc.Sequence[str], + *, + cwd: Path | None = None, + env: cabc.Mapping[str, str] | None = None, + ) -> tuple[int, str, str]: + return 101, "", "failed to resolve" + + with pytest.raises(lockfile.LockfileRefreshError, match="failed to resolve"): + lockfile.refresh_lockfile(manifest, runner) + + +def _validate_lockfile_freshness_for_exit_code( + tmp_path: Path, exit_code: int +) -> bool: + """Run lockfile freshness validation with a fake cargo exit code.""" + manifest = tmp_path / "Cargo.toml" + + def runner( + command: cabc.Sequence[str], + *, + cwd: Path | None = None, + env: cabc.Mapping[str, str] | None = None, + ) -> tuple[int, str, str]: + assert command == ( + "cargo", + "metadata", + "--locked", + "--manifest-path", + str(manifest), + "--format-version=1", + ) + assert cwd == manifest.parent + return exit_code, "", "stale" + + return lockfile.validate_lockfile_freshness(manifest, runner) + + +def test_validate_lockfile_freshness_returns_true_for_success(tmp_path: Path) -> None: + """Cargo metadata success means the lockfile is fresh.""" + assert _validate_lockfile_freshness_for_exit_code(tmp_path, 0) is True + + +def test_validate_lockfile_freshness_returns_false_for_failure(tmp_path: Path) -> None: + """Cargo metadata failure means the lockfile is stale.""" + assert _validate_lockfile_freshness_for_exit_code(tmp_path, 101) is False From b8c4f76a12c976cbcf0a0a5b3b73ef2815a0c860 Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 6 Jun 2026 09:54:23 +0200 Subject: [PATCH 02/24] Extract Cargo lockfile discovery helpers (#61) Split `discover_tracked_lockfiles` into private helpers for the `git ls-files` call and manifest-adjacent filtering. Preserve the existing behaviour, warning messages and public API while reducing the function's complexity. --- lading/commands/lockfile.py | 53 +++++++++++++++++++++++++------------ 1 file changed, 36 insertions(+), 17 deletions(-) diff --git a/lading/commands/lockfile.py b/lading/commands/lockfile.py index ba88a64a..59b1302c 100644 --- a/lading/commands/lockfile.py +++ b/lading/commands/lockfile.py @@ -16,19 +16,14 @@ class LockfileRefreshError(RuntimeError): """Raised when Cargo cannot regenerate a lockfile.""" -def discover_tracked_lockfiles( +def _git_ls_lockfiles( workspace_root: Path, runner: _CommandRunner, -) -> tuple[Path, ...]: - """Return tracked Cargo.lock files with adjacent manifests.""" - candidate_lockfiles = tuple( - path - for path in workspace_root.rglob("Cargo.lock") - if "target" not in path.relative_to(workspace_root).parts - ) - if not candidate_lockfiles: - return () +) -> str | None: + """Run ``git ls-files`` for Cargo.lock paths. + Return stdout or ``None`` on failure. + """ exit_code, stdout, stderr = runner( ("git", "ls-files", "*/Cargo.lock", "Cargo.lock"), cwd=workspace_root, @@ -40,24 +35,48 @@ def discover_tracked_lockfiles( "Skipping Cargo.lock discovery because %s is not a git repository", workspace_root, ) - return () - LOGGER.warning( - "Skipping Cargo.lock discovery after git ls-files failed: %s", detail - ) - return () + else: + LOGGER.warning( + "Skipping Cargo.lock discovery after git ls-files failed: %s", detail + ) + return None + return stdout + +def _lockfiles_with_manifests( + workspace_root: Path, + stdout: str, +) -> tuple[Path, ...]: + """Return ``git ls-files`` lockfile paths with an adjacent ``Cargo.toml``.""" lockfiles: list[Path] = [] for line in stdout.splitlines(): relative_path = line.strip() if not relative_path: continue lockfile_path = workspace_root / relative_path - manifest_path = lockfile_path.parent / "Cargo.toml" - if manifest_path.exists(): + if (lockfile_path.parent / "Cargo.toml").exists(): lockfiles.append(lockfile_path) return tuple(lockfiles) +def discover_tracked_lockfiles( + workspace_root: Path, + runner: _CommandRunner, +) -> tuple[Path, ...]: + """Return tracked Cargo.lock files with adjacent manifests.""" + candidate_lockfiles = tuple( + path + for path in workspace_root.rglob("Cargo.lock") + if "target" not in path.relative_to(workspace_root).parts + ) + if not candidate_lockfiles: + return () + stdout = _git_ls_lockfiles(workspace_root, runner) + if stdout is None: + return () + return _lockfiles_with_manifests(workspace_root, stdout) + + def refresh_lockfile( manifest_path: Path, runner: _CommandRunner, From 4df983b3cd1392b524616ca66bee3cb875982c6f Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 6 Jun 2026 09:57:09 +0200 Subject: [PATCH 03/24] Flatten Cargo lockfile discovery flow (#61) Move `git ls-files` error handling and manifest-adjacent filtering into private helpers so `discover_tracked_lockfiles` delegates after the git call. Preserve the public API, warning text and existing behaviour. --- lading/commands/lockfile.py | 57 ++++++++++++++++++------------------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/lading/commands/lockfile.py b/lading/commands/lockfile.py index 59b1302c..7e43ff5c 100644 --- a/lading/commands/lockfile.py +++ b/lading/commands/lockfile.py @@ -16,38 +16,33 @@ class LockfileRefreshError(RuntimeError): """Raised when Cargo cannot regenerate a lockfile.""" -def _git_ls_lockfiles( +def _git_ls_files_error_result( + exit_code: int, + stdout: str, + stderr: str, workspace_root: Path, - runner: _CommandRunner, -) -> str | None: - """Run ``git ls-files`` for Cargo.lock paths. - - Return stdout or ``None`` on failure. - """ - exit_code, stdout, stderr = runner( - ("git", "ls-files", "*/Cargo.lock", "Cargo.lock"), - cwd=workspace_root, - ) - if exit_code != 0: - detail = (stderr or stdout).strip() - if "not a git repository" in detail.lower(): - LOGGER.warning( - "Skipping Cargo.lock discovery because %s is not a git repository", - workspace_root, - ) - else: - LOGGER.warning( - "Skipping Cargo.lock discovery after git ls-files failed: %s", detail - ) +) -> tuple[Path, ...] | None: + """Return an empty result for failed ``git ls-files`` invocations.""" + if exit_code == 0: return None - return stdout + detail = (stderr or stdout).strip() + if "not a git repository" in detail.lower(): + LOGGER.warning( + "Skipping Cargo.lock discovery because %s is not a git repository", + workspace_root, + ) + else: + LOGGER.warning( + "Skipping Cargo.lock discovery after git ls-files failed: %s", detail + ) + return () def _lockfiles_with_manifests( - workspace_root: Path, stdout: str, + workspace_root: Path, ) -> tuple[Path, ...]: - """Return ``git ls-files`` lockfile paths with an adjacent ``Cargo.toml``.""" + """Return lockfile paths from ``git ls-files`` with adjacent manifests.""" lockfiles: list[Path] = [] for line in stdout.splitlines(): relative_path = line.strip() @@ -71,10 +66,14 @@ def discover_tracked_lockfiles( ) if not candidate_lockfiles: return () - stdout = _git_ls_lockfiles(workspace_root, runner) - if stdout is None: - return () - return _lockfiles_with_manifests(workspace_root, stdout) + exit_code, stdout, stderr = runner( + ("git", "ls-files", "*/Cargo.lock", "Cargo.lock"), + cwd=workspace_root, + ) + error_result = _git_ls_files_error_result(exit_code, stdout, stderr, workspace_root) + if error_result is not None: + return error_result + return _lockfiles_with_manifests(stdout, workspace_root) def refresh_lockfile( From 9bfdbb8effba1537cb4a727c0f2a0424891dc656 Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 7 Jun 2026 01:11:35 +0200 Subject: [PATCH 04/24] Format lockfile unit helper signature (#61) Apply Ruff's preferred one-line signature for the lockfile freshness test helper so `make check-fmt` passes in CI. --- tests/unit/test_lockfile.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/unit/test_lockfile.py b/tests/unit/test_lockfile.py index cdc423b5..4e262325 100644 --- a/tests/unit/test_lockfile.py +++ b/tests/unit/test_lockfile.py @@ -121,9 +121,7 @@ def runner( lockfile.refresh_lockfile(manifest, runner) -def _validate_lockfile_freshness_for_exit_code( - tmp_path: Path, exit_code: int -) -> bool: +def _validate_lockfile_freshness_for_exit_code(tmp_path: Path, exit_code: int) -> bool: """Run lockfile freshness validation with a fake cargo exit code.""" manifest = tmp_path / "Cargo.toml" From 56fa799487ed5ee1d0ce061e843035c32a7db228 Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 7 Jun 2026 04:52:19 +0200 Subject: [PATCH 05/24] Filter git-discovered target lockfiles (#61) Apply the same `target` path exclusion to lockfiles returned by `git ls-files` so tracked fixture or build-output lockfiles under target directories are not refreshed or validated. --- lading/commands/lockfile.py | 2 ++ tests/unit/test_lockfile.py | 1 + 2 files changed, 3 insertions(+) diff --git a/lading/commands/lockfile.py b/lading/commands/lockfile.py index 7e43ff5c..b055b982 100644 --- a/lading/commands/lockfile.py +++ b/lading/commands/lockfile.py @@ -49,6 +49,8 @@ def _lockfiles_with_manifests( if not relative_path: continue lockfile_path = workspace_root / relative_path + if "target" in lockfile_path.relative_to(workspace_root).parts: + continue if (lockfile_path.parent / "Cargo.toml").exists(): lockfiles.append(lockfile_path) return tuple(lockfiles) diff --git a/tests/unit/test_lockfile.py b/tests/unit/test_lockfile.py index 4e262325..90f13d28 100644 --- a/tests/unit/test_lockfile.py +++ b/tests/unit/test_lockfile.py @@ -41,6 +41,7 @@ def test_discover_tracked_lockfiles_filters_missing_manifests(tmp_path: Path) -> (nested / "Cargo.lock").write_text("", encoding="utf-8") target = tmp_path / "target" / "debug" target.mkdir(parents=True) + (target / "Cargo.toml").write_text("[package]\n", encoding="utf-8") (target / "Cargo.lock").write_text("", encoding="utf-8") def runner( From c9132042e7c8eb8c7bf3ca82b8faf4c25ec0e948 Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 7 Jun 2026 13:32:38 +0200 Subject: [PATCH 06/24] Address lockfile review findings (#61) Keep lockfile refresh side effects while returning the discovered paths so bump output reflects the paths selected by discovery. Clarify lockfile helper naming and public docstrings, and tighten tests around refresh command invocation, discovery runner arguments, and freshness validation parametrisation. --- lading/commands/bump.py | 5 +- lading/commands/lockfile.py | 99 +++++++++++++++++++-- tests/bdd/steps/test_bump_steps.py | 5 ++ tests/unit/test_bump_command_integration.py | 16 +++- tests/unit/test_lockfile.py | 21 +++-- 5 files changed, 124 insertions(+), 22 deletions(-) diff --git a/lading/commands/bump.py b/lading/commands/bump.py index 176614b9..75e997b0 100644 --- a/lading/commands/bump.py +++ b/lading/commands/bump.py @@ -248,10 +248,9 @@ def _refresh_lockfiles(context: _BumpContext) -> tuple[Path, ...]: LOGGER.info("Dry run; would refresh %s", lockfile_path) return lockfiles - return tuple( + for lockfile_path in lockfiles: refresh_lockfile(lockfile_path.parent / "Cargo.toml", runner) - for lockfile_path in lockfiles - ) + return lockfiles def _update_crate_manifest( diff --git a/lading/commands/lockfile.py b/lading/commands/lockfile.py index b055b982..e1407da0 100644 --- a/lading/commands/lockfile.py +++ b/lading/commands/lockfile.py @@ -1,4 +1,29 @@ -"""Cargo lockfile discovery, refresh, and freshness validation helpers.""" +"""Cargo lockfile discovery, refresh, and freshness validation helpers. + +This module centralises the Cargo lockfile operations shared by release +workflows. It discovers lockfiles that belong to the source workspace, +regenerates them after manifest rewrites, and validates that Cargo can read +them under ``--locked`` before expensive publish pre-flight commands run. + +Discovery is intentionally conservative. :func:`discover_tracked_lockfiles` +first checks for non-``target`` lockfile candidates in the workspace, then +narrows the result to git-tracked ``Cargo.lock`` files that are not under a +``target`` directory and have an adjacent ``Cargo.toml`` manifest. + +``lading bump`` calls :func:`discover_tracked_lockfiles` followed by +:func:`refresh_lockfile` after it updates manifest versions. ``lading publish`` +uses :func:`discover_tracked_lockfiles` and +:func:`validate_lockfile_freshness` before the cargo check/test pre-flight, so +stale lockfiles fail early with an actionable repair command. + +Typical local or CI usage follows the same sequence: + +>>> lockfiles = discover_tracked_lockfiles(workspace_root, runner) +>>> for lockfile_path in lockfiles: +... refresh_lockfile(lockfile_path.parent / "Cargo.toml", runner) +... validate_lockfile_freshness(lockfile_path.parent / "Cargo.toml", runner) +True +""" from __future__ import annotations @@ -16,13 +41,13 @@ class LockfileRefreshError(RuntimeError): """Raised when Cargo cannot regenerate a lockfile.""" -def _git_ls_files_error_result( +def _handle_git_ls_files_failure( exit_code: int, stdout: str, stderr: str, workspace_root: Path, ) -> tuple[Path, ...] | None: - """Return an empty result for failed ``git ls-files`` invocations.""" + """Return ``None`` for git success, or an empty result for git failure.""" if exit_code == 0: return None detail = (stderr or stdout).strip() @@ -60,7 +85,29 @@ def discover_tracked_lockfiles( workspace_root: Path, runner: _CommandRunner, ) -> tuple[Path, ...]: - """Return tracked Cargo.lock files with adjacent manifests.""" + """Return tracked Cargo.lock files with adjacent manifests. + + Parameters + ---------- + workspace_root + Path to the repository root that should be searched for lockfiles. + runner + Callable used to execute shell commands. It receives a command + sequence and returns ``(exit_code, stdout, stderr)``. + + Returns + ------- + tuple[Path, ...] + Git-tracked ``Cargo.lock`` files outside any ``target`` directory and + with an adjacent ``Cargo.toml`` manifest. The helper + :func:`_handle_git_ls_files_failure` handles git failures, and + :func:`_lockfiles_with_manifests` applies manifest and path filtering. + + Notes + ----- + If ``workspace_root`` is not a git repository, discovery logs a warning + through :func:`_handle_git_ls_files_failure` and returns an empty tuple. + """ candidate_lockfiles = tuple( path for path in workspace_root.rglob("Cargo.lock") @@ -72,7 +119,9 @@ def discover_tracked_lockfiles( ("git", "ls-files", "*/Cargo.lock", "Cargo.lock"), cwd=workspace_root, ) - error_result = _git_ls_files_error_result(exit_code, stdout, stderr, workspace_root) + error_result = _handle_git_ls_files_failure( + exit_code, stdout, stderr, workspace_root + ) if error_result is not None: return error_result return _lockfiles_with_manifests(stdout, workspace_root) @@ -82,7 +131,28 @@ def refresh_lockfile( manifest_path: Path, runner: _CommandRunner, ) -> Path: - """Regenerate the lockfile for ``manifest_path`` and return its path.""" + """Regenerate the lockfile for ``manifest_path`` and return its path. + + Parameters + ---------- + manifest_path + Path to the ``Cargo.toml`` manifest to generate a lockfile for. + runner + Callable used to run external commands. It receives a command sequence + and returns ``(exit_code, stdout, stderr)``. + + Returns + ------- + Path + Path to the generated ``Cargo.lock`` in ``manifest_path.parent``. + + Raises + ------ + LockfileRefreshError + Raised when ``cargo generate-lockfile`` returns a non-zero exit code. + The error message includes Cargo's stderr, or stdout when stderr is + empty, so callers can report why regeneration failed. + """ exit_code, stdout, stderr = runner( ("cargo", "generate-lockfile", "--manifest-path", str(manifest_path)), cwd=manifest_path.parent, @@ -100,7 +170,22 @@ def validate_lockfile_freshness( manifest_path: Path, runner: _CommandRunner, ) -> bool: - """Return whether Cargo accepts ``manifest_path`` under ``--locked``.""" + """Return whether Cargo accepts ``manifest_path`` under ``--locked``. + + Parameters + ---------- + manifest_path + Path to the Cargo manifest file to validate. + runner + Callable used to execute the cargo command. It receives a command + sequence and returns ``(exit_code, stdout, stderr)``. + + Returns + ------- + bool + ``True`` if ``cargo metadata --locked`` succeeds with exit code 0; + ``False`` otherwise. + """ exit_code, _stdout, _stderr = runner( ( "cargo", diff --git a/tests/bdd/steps/test_bump_steps.py b/tests/bdd/steps/test_bump_steps.py index 579e14f1..3a0c7c20 100644 --- a/tests/bdd/steps/test_bump_steps.py +++ b/tests/bdd/steps/test_bump_steps.py @@ -153,12 +153,17 @@ def then_cli_output_lists_lockfile_path( def then_bump_refreshed_lockfiles(cli_run: dict[str, typ.Any]) -> None: """Assert the live bump lockfile scenario completed successfully.""" assert cli_run["returncode"] == 0 + output = f"{cli_run['stdout']}\n{cli_run['stderr']}" + assert "cargo generate-lockfile" in output @then("the bump command did not refresh tracked lockfiles") def then_bump_did_not_refresh_lockfiles(cli_run: dict[str, typ.Any]) -> None: """Assert the dry-run lockfile scenario completed without refresh.""" assert cli_run["returncode"] == 0 + output = f"{cli_run['stdout']}\n{cli_run['stderr']}" + assert "cargo::generate-lockfile" not in output + assert "cargo generate-lockfile" not in output @then(parsers.parse('the documentation file "{relative_path}" contains "{expected}"')) diff --git a/tests/unit/test_bump_command_integration.py b/tests/unit/test_bump_command_integration.py index 65fb6913..6753b349 100644 --- a/tests/unit/test_bump_command_integration.py +++ b/tests/unit/test_bump_command_integration.py @@ -112,9 +112,15 @@ def runner( ), ) - assert tuple(refreshed) == lockfiles - assert "- Cargo.lock (lockfile)" in message.splitlines() - assert "- tests/ui_lints/Cargo.lock (lockfile)" in message.splitlines() + assert tuple(refreshed) == lockfiles, ( + "refreshed lockfiles do not match expected lockfiles" + ) + assert "- Cargo.lock (lockfile)" in message.splitlines(), ( + f"expected top-level Cargo.lock entry missing from message:\n{message}" + ) + assert "- tests/ui_lints/Cargo.lock (lockfile)" in message.splitlines(), ( + f"expected tests/ui_lints/Cargo.lock entry missing from message:\n{message}" + ) def test_run_dry_run_reports_lockfiles_without_refreshing( @@ -147,7 +153,9 @@ def refresh( ), ) - assert "- Cargo.lock (lockfile)" in message.splitlines() + assert "- Cargo.lock (lockfile)" in message.splitlines(), ( + f"expected '- Cargo.lock (lockfile)' in bump output:\n{message}" + ) def test_run_updates_root_package_section(tmp_path: pathlib.Path) -> None: diff --git a/tests/unit/test_lockfile.py b/tests/unit/test_lockfile.py index 90f13d28..5043891b 100644 --- a/tests/unit/test_lockfile.py +++ b/tests/unit/test_lockfile.py @@ -50,6 +50,9 @@ def runner( cwd: Path | None = None, env: cabc.Mapping[str, str] | None = None, ) -> tuple[int, str, str]: + assert command == ("git", "ls-files", "*/Cargo.lock", "Cargo.lock") + assert cwd == tmp_path + assert env is None return ( 0, ( @@ -146,11 +149,13 @@ def runner( return lockfile.validate_lockfile_freshness(manifest, runner) -def test_validate_lockfile_freshness_returns_true_for_success(tmp_path: Path) -> None: - """Cargo metadata success means the lockfile is fresh.""" - assert _validate_lockfile_freshness_for_exit_code(tmp_path, 0) is True - - -def test_validate_lockfile_freshness_returns_false_for_failure(tmp_path: Path) -> None: - """Cargo metadata failure means the lockfile is stale.""" - assert _validate_lockfile_freshness_for_exit_code(tmp_path, 101) is False +@pytest.mark.parametrize("case", [(0, True), (101, False)]) +def test_validate_lockfile_freshness_parametrized( + tmp_path: Path, + case: tuple[int, bool], +) -> None: + """Cargo metadata exit status determines whether the lockfile is fresh.""" + exit_code, expected_bool = case + assert ( + _validate_lockfile_freshness_for_exit_code(tmp_path, exit_code) is expected_bool + ) From 76802a948990994f32bdc9bf8ac1c679cfefcbea Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 7 Jun 2026 16:21:42 +0200 Subject: [PATCH 07/24] Add package base exception (#61) Introduce `LadingError` as the package-level base exception and export it from `lading`. Move existing direct domain exception roots from `RuntimeError` to `LadingError` while preserving their intermediate hierarchies and public behaviour. --- lading/__init__.py | 3 ++- lading/commands/lockfile.py | 4 +++- lading/commands/publish_errors.py | 21 ++++++++++++--------- lading/commands/publish_manifest.py | 3 ++- lading/commands/publish_plan.py | 3 ++- lading/config.py | 3 ++- lading/exceptions.py | 7 +++++++ lading/workspace/metadata.py | 3 ++- lading/workspace/models.py | 4 +++- 9 files changed, 35 insertions(+), 16 deletions(-) create mode 100644 lading/exceptions.py diff --git a/lading/__init__.py b/lading/__init__.py index bd40a0a9..24550954 100644 --- a/lading/__init__.py +++ b/lading/__init__.py @@ -7,5 +7,6 @@ from __future__ import annotations from .cli import app, main +from .exceptions import LadingError -__all__ = ["app", "main"] +__all__ = ["LadingError", "app", "main"] diff --git a/lading/commands/lockfile.py b/lading/commands/lockfile.py index e1407da0..deb7e7bf 100644 --- a/lading/commands/lockfile.py +++ b/lading/commands/lockfile.py @@ -31,13 +31,15 @@ import typing as typ from pathlib import Path +from lading.exceptions import LadingError + if typ.TYPE_CHECKING: from lading.commands.publish_execution import _CommandRunner LOGGER = logging.getLogger(__name__) -class LockfileRefreshError(RuntimeError): +class LockfileRefreshError(LadingError): """Raised when Cargo cannot regenerate a lockfile.""" diff --git a/lading/commands/publish_errors.py b/lading/commands/publish_errors.py index 588bcbf1..ea3b6557 100644 --- a/lading/commands/publish_errors.py +++ b/lading/commands/publish_errors.py @@ -6,11 +6,12 @@ pipelines raise :class:`PublishError` when cargo publication work fails after those checks have completed. -Both classes inherit from :class:`RuntimeError`, carry their message through the -standard exception ``args`` tuple, and avoid additional mutable state. Callers of -``lading.commands.publish.run`` can catch :class:`PublishPreflightError` to -handle both validation and publish failures through one path, or catch -:class:`PublishError` first when publish-phase failures need distinct handling. +Both classes inherit from :class:`lading.exceptions.LadingError`, carry their +message through the standard exception ``args`` tuple, and avoid additional +mutable state. Callers of ``lading.commands.publish.run`` can catch +:class:`PublishPreflightError` to handle both validation and publish failures +through one path, or catch :class:`PublishError` first when publish-phase +failures need distinct handling. Examples -------- @@ -24,20 +25,22 @@ from __future__ import annotations +from lading.exceptions import LadingError -class PublishPreflightError(RuntimeError): + +class PublishPreflightError(LadingError): """Raise when required pre-publication checks fail. Parameters ---------- *args - Positional arguments passed to :class:`RuntimeError`; the first + Positional arguments passed to :class:`LadingError`; the first argument is conventionally the human-readable failure message. Attributes ---------- args - The message arguments stored by :class:`RuntimeError`. + The message arguments stored by :class:`LadingError`. Notes ----- @@ -59,7 +62,7 @@ class PublishError(PublishPreflightError): Attributes ---------- args - The message arguments stored by :class:`RuntimeError`. + The message arguments stored by :class:`PublishPreflightError`. Notes ----- diff --git a/lading/commands/publish_manifest.py b/lading/commands/publish_manifest.py index baccc25e..a3d722f7 100644 --- a/lading/commands/publish_manifest.py +++ b/lading/commands/publish_manifest.py @@ -38,6 +38,7 @@ from tomlkit.toml_document import TOMLDocument from lading import config as config_module +from lading.exceptions import LadingError if typ.TYPE_CHECKING: # pragma: no cover - typing helpers only from pathlib import Path @@ -55,7 +56,7 @@ ) -class PublishPreparationError(RuntimeError): +class PublishPreparationError(LadingError): """Publish staging failed to prepare required assets. Raised when: diff --git a/lading/commands/publish_plan.py b/lading/commands/publish_plan.py index 4ebcbb79..59bc807f 100644 --- a/lading/commands/publish_plan.py +++ b/lading/commands/publish_plan.py @@ -6,6 +6,7 @@ import dataclasses as dc import typing as typ +from lading.exceptions import LadingError from lading.workspace import WorkspaceDependencyCycleError if typ.TYPE_CHECKING: # pragma: no cover - typing helper @@ -15,7 +16,7 @@ from lading.workspace import WorkspaceCrate, WorkspaceGraph -class PublishPlanError(RuntimeError): +class PublishPlanError(LadingError): """Raised when the publish plan cannot be constructed.""" diff --git a/lading/config.py b/lading/config.py index f1497cac..a0697d5b 100644 --- a/lading/config.py +++ b/lading/config.py @@ -10,6 +10,7 @@ from cyclopts.config import Toml +from lading.exceptions import LadingError from lading.utils import normalise_workspace_root if typ.TYPE_CHECKING: # pragma: no cover - type checking only @@ -41,7 +42,7 @@ }) -class ConfigurationError(RuntimeError): +class ConfigurationError(LadingError): """Raised when the :mod:`lading` configuration is invalid.""" diff --git a/lading/exceptions.py b/lading/exceptions.py new file mode 100644 index 00000000..b1a67c91 --- /dev/null +++ b/lading/exceptions.py @@ -0,0 +1,7 @@ +"""Package-level base exception for the lading toolkit.""" + +from __future__ import annotations + + +class LadingError(Exception): + """Base class for all lading exceptions.""" diff --git a/lading/workspace/metadata.py b/lading/workspace/metadata.py index c3a7ade8..b5eb2251 100644 --- a/lading/workspace/metadata.py +++ b/lading/workspace/metadata.py @@ -19,9 +19,10 @@ if typ.TYPE_CHECKING: # pragma: no cover - import-time typing aids only from pathlib import Path +from lading.exceptions import LadingError -class CargoMetadataError(RuntimeError): +class CargoMetadataError(LadingError): """Raised when ``cargo metadata`` cannot be executed successfully.""" @classmethod diff --git a/lading/workspace/models.py b/lading/workspace/models.py index 191d845f..76feb376 100644 --- a/lading/workspace/models.py +++ b/lading/workspace/models.py @@ -13,6 +13,8 @@ from tomlkit import parse from tomlkit.exceptions import TOMLKitError +from lading.exceptions import LadingError + WORKSPACE_ROOT_MISSING_MSG = "cargo metadata missing 'workspace_root'" ALLOWED_DEP_KINDS: typ.Final[set[str]] = {"normal", "dev", "build"} @@ -31,7 +33,7 @@ def _is_ordering_dependency( return dependency.kind in ORDER_DEPENDENCY_KINDS -class WorkspaceModelError(RuntimeError): +class WorkspaceModelError(LadingError): """Raised when the workspace model cannot be constructed.""" From bac4059ee3bdb57310c8646dff9ab3eef99c6fd0 Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 7 Jun 2026 16:48:48 +0200 Subject: [PATCH 08/24] Document lockfile refresh workflow (#61) Log successful lockfile refreshes during bump so operators can see both each refreshed path and the workspace-level summary. Document bump refresh behaviour, publish freshness validation, and the shared lockfile helpers in the user and developer guides. Record the completed lockfile work in the roadmap. --- docs/developers-guide.md | 22 ++++++++++++++++++++++ docs/roadmap.md | 7 ++++++- docs/users-guide.md | 20 ++++++++++++++++++++ lading/commands/bump.py | 5 +++++ lading/commands/lockfile.py | 1 + 5 files changed, 54 insertions(+), 1 deletion(-) diff --git a/docs/developers-guide.md b/docs/developers-guide.md index e6957c5d..a122bb19 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -88,6 +88,28 @@ publish runs with stub mode enabled. ## Workspace discovery helpers + +### Lockfile helpers (`lading/commands/lockfile.py`) + +`discover_tracked_lockfiles(workspace_root, runner)` filters git-tracked +`Cargo.lock` files outside `target/` with adjacent `Cargo.toml` manifests. +Private helpers `_handle_git_ls_files_failure` and +`_lockfiles_with_manifests` perform the error-handling and path-filtering +passes respectively. + +`refresh_lockfile(manifest_path, runner)` runs +`cargo generate-lockfile --manifest-path` for the supplied manifest and raises +`LockfileRefreshError` on non-zero exit. `_refresh_lockfiles` in `bump.py` +calls it after manifest rewrites. + +`validate_lockfile_freshness(manifest_path, runner)` runs +`cargo metadata --locked --manifest-path ... --format-version=1`. It returns +`True` on success and `False` otherwise. `_validate_lockfile_freshness` in +`publish_preflight.py` calls it before the cargo check/test pre-flight. + +`LockfileRefreshError` inherits `LadingError`; its message includes Cargo +stderr. + ### `load_cargo_metadata` Import `lading.workspace.load_cargo_metadata` to execute `cargo metadata` with diff --git a/docs/roadmap.md b/docs/roadmap.md index d683bef7..7bd52c26 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -106,7 +106,9 @@ workspace and member crate manifests. - [x] **Implement `--dry-run` Mode for Bumping:** - **Outcome:** The `--dry-run` flag prevents any file modifications and - instead prints a summary of intended changes. + instead prints a summary of intended changes. Tracked `Cargo.lock` files + are refreshed after manifest rewrites in live mode; in dry-run mode they + are reported but not modified (closes `#61`). - **Completion Criteria:** Running `lading bump 1.2.3 --dry-run` produces a report of all files that would be changed, and a subsequent check confirms no files were actually modified. @@ -177,6 +179,9 @@ occur. - **Completion Criteria:** A test confirms that the publish command fails if either of the pre-flight checks fails. The `--forbid-dirty` flag restores the cleanliness guard when operators need to enforce it. + - Tracked `Cargo.lock` files are validated for freshness under `--locked` + before the `cargo check`/`cargo test` pre-flight; stale lockfiles surface + an actionable `cargo generate-lockfile` repair command (closes `#61`). ______________________________________________________________________ diff --git a/docs/users-guide.md b/docs/users-guide.md index b44a07d8..bd0dfc38 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -73,6 +73,12 @@ To preview changes without writing any files: lading bump 1.2.3 --dry-run ``` +After updating manifest versions, `lading bump` automatically refreshes any +git-tracked `Cargo.lock` files (excluding those under `target/`) that have an +adjacent `Cargo.toml`. Refreshed lockfiles are listed in the command output +with a `(lockfile)` suffix. In dry-run mode the lockfiles are listed but not +modified. + If `bump.documentation.globs` is configured, `lading` also searches those Markdown files for TOML code fences and updates version values that refer to workspace crates. @@ -86,6 +92,20 @@ be validated without uploading crates. lading publish ``` +Before running `cargo check` and `cargo test`, `lading publish` validates that +all git-tracked `Cargo.lock` files are fresh under `--locked` mode. If any +lockfile is stale — for example after a `lading bump` that regenerated a nested +workspace lockfile — the command exits with code 1 and prints a repair command: + +```text +Tracked Cargo.lock files are stale after manifest version changes. +Run the following to repair: + cargo generate-lockfile --manifest-path /Cargo.toml +``` + +Run the repair command, commit the updated lockfile, then re-run +`lading publish`. + To require a clean working tree before running the pre-flight checks, pass `--forbid-dirty`: diff --git a/lading/commands/bump.py b/lading/commands/bump.py index 75e997b0..e1a99c43 100644 --- a/lading/commands/bump.py +++ b/lading/commands/bump.py @@ -250,6 +250,11 @@ def _refresh_lockfiles(context: _BumpContext) -> tuple[Path, ...]: for lockfile_path in lockfiles: refresh_lockfile(lockfile_path.parent / "Cargo.toml", runner) + LOGGER.info( + "Refreshed %d tracked lockfile(s) in %s", + len(lockfiles), + context.root_path, + ) return lockfiles diff --git a/lading/commands/lockfile.py b/lading/commands/lockfile.py index deb7e7bf..fed18cd6 100644 --- a/lading/commands/lockfile.py +++ b/lading/commands/lockfile.py @@ -165,6 +165,7 @@ def refresh_lockfile( if detail: message = f"{message}: {detail}" raise LockfileRefreshError(message) + LOGGER.info("Refreshed %s", manifest_path.parent / "Cargo.lock") return manifest_path.parent / "Cargo.lock" From fc3ec4fd77c0f571351ff7b60b3ab3f6ff6610dd Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 7 Jun 2026 20:13:03 +0200 Subject: [PATCH 09/24] Tighten lockfile follow-up handling (#61) Avoid lockfile discovery and refresh work when a bump run does not change any Cargo manifests. Quote stale-lockfile repair manifest paths for shell-safe publish diagnostics and keep the publish pre-flight stubs aligned with lockfile discovery. Improve lockfile unit assertions so failures report the expected and actual values directly. --- lading/commands/bump.py | 2 +- lading/commands/publish_preflight.py | 6 +++- .../bdd/steps/test_publish_infrastructure.py | 1 + tests/unit/test_bump_command_integration.py | 10 +++++- tests/unit/test_lockfile.py | 32 +++++++++++++++---- 5 files changed, 42 insertions(+), 9 deletions(-) diff --git a/lading/commands/bump.py b/lading/commands/bump.py index e1a99c43..4b269c41 100644 --- a/lading/commands/bump.py +++ b/lading/commands/bump.py @@ -114,7 +114,7 @@ def run( _process_workspace_manifest(context, target_version, changed_manifests) _process_crate_manifests(context, target_version, changed_manifests) changed_documents = _process_documentation_files(context, target_version) - refreshed_lockfiles = _refresh_lockfiles(context) + refreshed_lockfiles = _refresh_lockfiles(context) if changed_manifests else () changes = _prepare_sorted_changes( context, changed_manifests, changed_documents, refreshed_lockfiles ) diff --git a/lading/commands/publish_preflight.py b/lading/commands/publish_preflight.py index 440204c5..b297d468 100644 --- a/lading/commands/publish_preflight.py +++ b/lading/commands/publish_preflight.py @@ -24,6 +24,7 @@ import dataclasses as dc import logging import os +import shlex import tempfile import typing as typ from pathlib import Path @@ -225,7 +226,10 @@ def runner_with_env( for lockfile_path in stale_lockfiles: manifest_path = lockfile_path.parent / "Cargo.toml" lines.append(f"- {lockfile_path}") - lines.append(f" cargo generate-lockfile --manifest-path {manifest_path}") + quoted_manifest_path = shlex.quote(str(manifest_path)) + lines.append( + f" cargo generate-lockfile --manifest-path {quoted_manifest_path}" + ) message = "\n".join(lines) LOGGER.error(message) diff --git a/tests/bdd/steps/test_publish_infrastructure.py b/tests/bdd/steps/test_publish_infrastructure.py index b04accbe..3fedc560 100644 --- a/tests/bdd/steps/test_publish_infrastructure.py +++ b/tests/bdd/steps/test_publish_infrastructure.py @@ -185,6 +185,7 @@ def _register_preflight_commands( """ defaults: dict[tuple[str, ...], ResponseProvider] = { ("git", "status", "--porcelain"): _CommandResponse(exit_code=0), + ("git", "ls-files"): _CommandResponse(exit_code=0), ( "cargo", "metadata", diff --git a/tests/unit/test_bump_command_integration.py b/tests/unit/test_bump_command_integration.py index 6753b349..1ae95838 100644 --- a/tests/unit/test_bump_command_integration.py +++ b/tests/unit/test_bump_command_integration.py @@ -342,11 +342,19 @@ def test_run_uses_loaded_configuration_and_workspace( ids=lambda scenario: scenario.test_id, ) def test_run_reports_when_versions_already_match( - tmp_path: pathlib.Path, scenario: _NoChangeScenario + tmp_path: pathlib.Path, scenario: _NoChangeScenario, monkeypatch: MonkeyPatch ) -> None: """Report the no-op message for both live and dry-run invocations.""" workspace = _make_workspace(tmp_path) configuration = _make_config() + + def fail_discovery( + workspace_root: pathlib.Path, + runner: bump._CommandRunner, + ) -> tuple[pathlib.Path, ...]: + pytest.fail("no-op bumps must not discover or refresh lockfiles") + + monkeypatch.setattr(bump, "discover_tracked_lockfiles", fail_discovery) message = bump.run( tmp_path, "0.1.0", diff --git a/tests/unit/test_lockfile.py b/tests/unit/test_lockfile.py index 5043891b..58aaf853 100644 --- a/tests/unit/test_lockfile.py +++ b/tests/unit/test_lockfile.py @@ -27,7 +27,11 @@ def runner( assert cwd == tmp_path return 0, "", "" - assert lockfile.discover_tracked_lockfiles(tmp_path, runner) == () + result = lockfile.discover_tracked_lockfiles(tmp_path, runner) + assert result == (), ( + "git repo with no tracked lockfiles should return an empty tuple; " + f"got {result!r}" + ) def test_discover_tracked_lockfiles_filters_missing_manifests(tmp_path: Path) -> None: @@ -64,10 +68,15 @@ def runner( "", ) - assert lockfile.discover_tracked_lockfiles(tmp_path, runner) == ( + result = lockfile.discover_tracked_lockfiles(tmp_path, runner) + expected = ( tmp_path / "Cargo.lock", nested / "Cargo.lock", ) + assert result == expected, ( + "only manifest-adjacent, non-target lockfiles should be returned; " + f"expected {expected!r}, got {result!r}" + ) def test_discover_tracked_lockfiles_handles_non_git_directory( @@ -84,7 +93,11 @@ def runner( ) -> tuple[int, str, str]: return 128, "", "fatal: not a git repository" - assert lockfile.discover_tracked_lockfiles(tmp_path, runner) == () + result = lockfile.discover_tracked_lockfiles(tmp_path, runner) + assert result == (), ( + "discovery should not abort on non-git errors; " + f"expected empty tuple, got {result!r}" + ) def test_refresh_lockfile_returns_lockfile_path(tmp_path: Path) -> None: @@ -106,7 +119,12 @@ def runner( assert cwd == manifest.parent return 0, "", "" - assert lockfile.refresh_lockfile(manifest, runner) == tmp_path / "Cargo.lock" + expected = tmp_path / "Cargo.lock" + result = lockfile.refresh_lockfile(manifest, runner) + assert result == expected, ( + "refresh helper returned unexpected lockfile path; " + f"expected {expected!r}, got {result!r}" + ) def test_refresh_lockfile_raises_on_failure(tmp_path: Path) -> None: @@ -156,6 +174,8 @@ def test_validate_lockfile_freshness_parametrized( ) -> None: """Cargo metadata exit status determines whether the lockfile is fresh.""" exit_code, expected_bool = case - assert ( - _validate_lockfile_freshness_for_exit_code(tmp_path, exit_code) is expected_bool + actual = _validate_lockfile_freshness_for_exit_code(tmp_path, exit_code) + assert actual is expected_bool, ( + "freshness result did not match cargo metadata exit code; " + f"exit_code={exit_code}, expected {expected_bool}, got {actual}" ) From a59fe9589f2e206b5a1489324940b9e594ce0ece Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 7 Jun 2026 20:41:34 +0200 Subject: [PATCH 10/24] Address lockfile review follow-ups (#61) Avoid walking the workspace during tracked lockfile discovery by relying on git ls-files directly, while preserving target and manifest filtering. Update documentation and test stubs for the reviewed lockfile flow. --- docs/developers-guide.md | 10 +++++--- lading/commands/bump.py | 7 +++++- lading/commands/lockfile.py | 24 +++++++------------ lading/commands/publish_preflight.py | 10 +++++--- tests/bdd/steps/metadata_fixtures.py | 3 +++ .../bdd/steps/test_publish_infrastructure.py | 4 +++- tests/e2e/steps/test_e2e_steps.py | 1 + tests/unit/publish/test_run_integration.py | 2 +- 8 files changed, 37 insertions(+), 24 deletions(-) diff --git a/docs/developers-guide.md b/docs/developers-guide.md index a122bb19..260fd92e 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -100,7 +100,10 @@ passes respectively. `refresh_lockfile(manifest_path, runner)` runs `cargo generate-lockfile --manifest-path` for the supplied manifest and raises `LockfileRefreshError` on non-zero exit. `_refresh_lockfiles` in `bump.py` -calls it after manifest rewrites. +calls it after manifest rewrites. The refresh loop is intentionally not +transactional: if a later lockfile refresh fails, previously rewritten +manifests and refreshed lockfiles remain on disk, and the operator should fix +the Cargo error and rerun `lading bump` with the same version. `validate_lockfile_freshness(manifest_path, runner)` runs `cargo metadata --locked --manifest-path ... --format-version=1`. It returns @@ -187,8 +190,9 @@ failure to a warning and continues. The option is rejected at runtime when ### Exception hierarchy (`publish_errors`) `lading.commands.publish_errors` defines the public error boundary for -publish orchestration. Both classes inherit from `RuntimeError` and carry -their message through the standard `args` tuple. +publish orchestration. Both classes inherit from the package-level +`LadingError` base, which itself extends `Exception`, and carry their message +through the standard `args` tuple. | Exception | Raised when | | --- | --- | diff --git a/lading/commands/bump.py b/lading/commands/bump.py index 4b269c41..f64ac168 100644 --- a/lading/commands/bump.py +++ b/lading/commands/bump.py @@ -240,7 +240,12 @@ def _prepare_sorted_changes( def _refresh_lockfiles(context: _BumpContext) -> tuple[Path, ...]: - """Refresh tracked lockfiles after manifest rewrites.""" + """Refresh tracked lockfiles after manifest rewrites. + + Refresh is intentionally not transactional. If one lockfile refresh fails, + manifests already rewritten to the target version remain on disk; after + fixing the Cargo error, rerun ``lading bump`` with the same version. + """ runner = context.base_options.runner or _invoke lockfiles = discover_tracked_lockfiles(context.root_path, runner) if context.base_options.dry_run: diff --git a/lading/commands/lockfile.py b/lading/commands/lockfile.py index fed18cd6..80538cff 100644 --- a/lading/commands/lockfile.py +++ b/lading/commands/lockfile.py @@ -6,9 +6,9 @@ them under ``--locked`` before expensive publish pre-flight commands run. Discovery is intentionally conservative. :func:`discover_tracked_lockfiles` -first checks for non-``target`` lockfile candidates in the workspace, then -narrows the result to git-tracked ``Cargo.lock`` files that are not under a -``target`` directory and have an adjacent ``Cargo.toml`` manifest. +queries the git index for tracked ``Cargo.lock`` files, then narrows the +result to paths that are not under a ``target`` directory and have an adjacent +``Cargo.toml`` manifest. ``lading bump`` calls :func:`discover_tracked_lockfiles` followed by :func:`refresh_lockfile` after it updates manifest versions. ``lading publish`` @@ -18,11 +18,12 @@ Typical local or CI usage follows the same sequence: ->>> lockfiles = discover_tracked_lockfiles(workspace_root, runner) ->>> for lockfile_path in lockfiles: -... refresh_lockfile(lockfile_path.parent / "Cargo.toml", runner) -... validate_lockfile_freshness(lockfile_path.parent / "Cargo.toml", runner) -True +```python +lockfiles = discover_tracked_lockfiles(workspace_root, runner) +for lockfile_path in lockfiles: + refresh_lockfile(lockfile_path.parent / "Cargo.toml", runner) + validate_lockfile_freshness(lockfile_path.parent / "Cargo.toml", runner) +``` """ from __future__ import annotations @@ -110,13 +111,6 @@ def discover_tracked_lockfiles( If ``workspace_root`` is not a git repository, discovery logs a warning through :func:`_handle_git_ls_files_failure` and returns an empty tuple. """ - candidate_lockfiles = tuple( - path - for path in workspace_root.rglob("Cargo.lock") - if "target" not in path.relative_to(workspace_root).parts - ) - if not candidate_lockfiles: - return () exit_code, stdout, stderr = runner( ("git", "ls-files", "*/Cargo.lock", "Cargo.lock"), cwd=workspace_root, diff --git a/lading/commands/publish_preflight.py b/lading/commands/publish_preflight.py index b297d468..e67702c5 100644 --- a/lading/commands/publish_preflight.py +++ b/lading/commands/publish_preflight.py @@ -13,9 +13,13 @@ Examples -------- ->>> from pathlib import Path ->>> from lading.commands.publish_preflight import _run_preflight_checks ->>> _run_preflight_checks(Path("."), allow_dirty=False, configuration=config) +```python +from pathlib import Path + +from lading.commands.publish_preflight import _run_preflight_checks + +_run_preflight_checks(Path("."), allow_dirty=False, configuration=config) +``` """ from __future__ import annotations diff --git a/tests/bdd/steps/metadata_fixtures.py b/tests/bdd/steps/metadata_fixtures.py index 9655926d..cec35c30 100644 --- a/tests/bdd/steps/metadata_fixtures.py +++ b/tests/bdd/steps/metadata_fixtures.py @@ -96,6 +96,9 @@ def _mock_cargo_metadata( stdout=json.dumps(payload), stderr="", ).any_order() + cmd_mox.stub("git").with_args("ls-files", "*/Cargo.lock", "Cargo.lock").returns( + exit_code=0, stdout="", stderr="" + ).any_order() @given("cargo metadata describes a sample workspace") diff --git a/tests/bdd/steps/test_publish_infrastructure.py b/tests/bdd/steps/test_publish_infrastructure.py index 3fedc560..2a9159cd 100644 --- a/tests/bdd/steps/test_publish_infrastructure.py +++ b/tests/bdd/steps/test_publish_infrastructure.py @@ -185,7 +185,9 @@ def _register_preflight_commands( """ defaults: dict[tuple[str, ...], ResponseProvider] = { ("git", "status", "--porcelain"): _CommandResponse(exit_code=0), - ("git", "ls-files"): _CommandResponse(exit_code=0), + ("git", "ls-files", "*/Cargo.lock", "Cargo.lock"): _CommandResponse( + exit_code=0 + ), ( "cargo", "metadata", diff --git a/tests/e2e/steps/test_e2e_steps.py b/tests/e2e/steps/test_e2e_steps.py index 53094d3e..43f5046c 100644 --- a/tests/e2e/steps/test_e2e_steps.py +++ b/tests/e2e/steps/test_e2e_steps.py @@ -80,6 +80,7 @@ def given_nontrivial_workspace_in_git_repo( raise E2EExpectationError.unsupported_fixture_version(version) e2e_workspace, e2e_git_repo = e2e_workspace_with_git monkeypatch.setenv("LADING_USE_CMD_MOX_STUB", "1") + cmd_mox.spy("git").passthrough() stub_cargo_metadata(cmd_mox, e2e_workspace) return {"workspace": e2e_workspace, "git_repo": e2e_git_repo} diff --git a/tests/unit/publish/test_run_integration.py b/tests/unit/publish/test_run_integration.py index a24d1d87..39bf1e1e 100644 --- a/tests/unit/publish/test_run_integration.py +++ b/tests/unit/publish/test_run_integration.py @@ -574,7 +574,7 @@ def skip_git_invoke( cwd: Path | None = None, env: cabc.Mapping[str, str] | None = None, ) -> tuple[int, str, str]: - if command[0] == "git": + if command == ("git", "status", "--porcelain"): message = "git status should be skipped by default" raise AssertionError(message) return 0, "", "" From d0da65a570e6919a737b4c1f2db92f223fcaafe8 Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 7 Jun 2026 23:41:49 +0200 Subject: [PATCH 11/24] Fix lockfile preflight review items (#61) Update partial-refresh recovery guidance, log successful lockfile freshness validation, and exercise the real validation helper in preflight tests. Keep the runner environment propagation assertion intact. --- lading/commands/bump.py | 4 +++- lading/commands/publish_preflight.py | 5 ++++- tests/unit/publish/test_preflight_checks.py | 7 +------ 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lading/commands/bump.py b/lading/commands/bump.py index f64ac168..52f1d892 100644 --- a/lading/commands/bump.py +++ b/lading/commands/bump.py @@ -244,7 +244,9 @@ def _refresh_lockfiles(context: _BumpContext) -> tuple[Path, ...]: Refresh is intentionally not transactional. If one lockfile refresh fails, manifests already rewritten to the target version remain on disk; after - fixing the Cargo error, rerun ``lading bump`` with the same version. + fixing the Cargo error, run + ``cargo generate-lockfile --manifest-path /Cargo.toml`` + for each stale lockfile. """ runner = context.base_options.runner or _invoke lockfiles = discover_tracked_lockfiles(context.root_path, runner) diff --git a/lading/commands/publish_preflight.py b/lading/commands/publish_preflight.py index e67702c5..d48603fb 100644 --- a/lading/commands/publish_preflight.py +++ b/lading/commands/publish_preflight.py @@ -208,16 +208,19 @@ def runner_with_env( cwd: Path | None = None, env: cabc.Mapping[str, str] | None = None, ) -> tuple[int, str, str]: + """Invoke ``runner`` with ``base_env`` applied when no env is supplied.""" effective_env = base_env if env is None else env return runner(command, cwd=cwd, env=effective_env) + tracked = discover_tracked_lockfiles(workspace_root, runner_with_env) stale_lockfiles: list[Path] = [] - for lockfile_path in discover_tracked_lockfiles(workspace_root, runner_with_env): + for lockfile_path in tracked: manifest_path = lockfile_path.parent / "Cargo.toml" if not validate_lockfile_freshness(manifest_path, runner_with_env): stale_lockfiles.append(lockfile_path) if not stale_lockfiles: + LOGGER.info("All %d tracked lockfile(s) are fresh under --locked", len(tracked)) return lines = [ diff --git a/tests/unit/publish/test_preflight_checks.py b/tests/unit/publish/test_preflight_checks.py index f8fc116d..b7d9faf6 100644 --- a/tests/unit/publish/test_preflight_checks.py +++ b/tests/unit/publish/test_preflight_checks.py @@ -362,14 +362,9 @@ def test_validate_lockfile_freshness_passes_when_all_lockfiles_are_fresh( "discover_tracked_lockfiles", lambda _root, _runner: (root_lockfile, nested_lockfile), ) - monkeypatch.setattr( - publish._publish_preflight, - "validate_lockfile_freshness", - lambda _manifest, runner: runner(("cargo", "metadata"), cwd=tmp_path)[0] == 0, - ) def runner( - command: tuple[str, ...], + command: cabc.Sequence[str], *, cwd: Path | None = None, env: cabc.Mapping[str, str] | None = None, From 0b50f42281eeae1f93e12a3e4c835e101154c180 Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 7 Jun 2026 23:43:00 +0200 Subject: [PATCH 12/24] Clarify lockfile refresh recovery (#61) Document that rerunning lading bump after a partial lockfile refresh failure will not refresh lockfiles unless manifests are rewritten, and point users to cargo generate-lockfile for repair. --- lading/commands/bump.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lading/commands/bump.py b/lading/commands/bump.py index 52f1d892..7dfe004f 100644 --- a/lading/commands/bump.py +++ b/lading/commands/bump.py @@ -244,7 +244,8 @@ def _refresh_lockfiles(context: _BumpContext) -> tuple[Path, ...]: Refresh is intentionally not transactional. If one lockfile refresh fails, manifests already rewritten to the target version remain on disk; after - fixing the Cargo error, run + fixing the Cargo error, rerunning ``lading bump`` will not refresh + lockfiles because refresh only runs when manifests are rewritten. Run ``cargo generate-lockfile --manifest-path /Cargo.toml`` for each stale lockfile. """ From c7a4ddea3834751a677cfcdca710ead231d02694 Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 7 Jun 2026 23:44:47 +0200 Subject: [PATCH 13/24] Make bump output dry-run explicit (#61) Require dry_run to be passed by keyword in bump output formatting helpers and remove the positional-boolean lint suppressions. --- lading/commands/bump.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lading/commands/bump.py b/lading/commands/bump.py index 7dfe004f..2d32f8b8 100644 --- a/lading/commands/bump.py +++ b/lading/commands/bump.py @@ -85,7 +85,7 @@ def _build_changes_description(changes: BumpChanges) -> str: return parts[0] if len(parts) == 1 else " and ".join(parts) -def _format_no_changes_message(target_version: str, dry_run: bool) -> str: # noqa: FBT001 +def _format_no_changes_message(target_version: str, *, dry_run: bool) -> str: """Format message when no changes are required.""" if dry_run: return ( @@ -95,7 +95,7 @@ def _format_no_changes_message(target_version: str, dry_run: bool) -> str: # no return f"No manifest changes required; all versions already {target_version}." -def _format_header(description: str, target_version: str, dry_run: bool) -> str: # noqa: FBT001 +def _format_header(description: str, target_version: str, *, dry_run: bool) -> str: """Format the summary header line.""" if dry_run: return f"Dry run; would update version to {target_version} in {description}:" @@ -306,10 +306,10 @@ def _format_result_message( ) -> str: """Summarise the bump outcome for CLI presentation.""" if not any((changes.manifests, changes.documents, changes.lockfiles)): - return _format_no_changes_message(target_version, dry_run) + return _format_no_changes_message(target_version, dry_run=dry_run) description = _build_changes_description(changes) - header = _format_header(description, target_version, dry_run) + header = _format_header(description, target_version, dry_run=dry_run) formatted_paths = [ f"- {_format_manifest_path(manifest_path, workspace_root)}" for manifest_path in changes.manifests From 5f74e99fbdf44c4c5a825f1e1205a3061d62526d Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 8 Jun 2026 12:56:27 +0200 Subject: [PATCH 14/24] Cover bump CLI runner default (#61) Assert the bump CLI leaves the injectable command runner unset so the command layer uses its production default while lower-level tests can still inject stubs. --- tests/unit/test_cli.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index 96a42f8e..20cd90e0 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -390,6 +390,7 @@ def fake_run(*args: object, **kwargs: object) -> str: options = captured_kwargs["options"] assert isinstance(options, bump_command.BumpOptions) assert options.dry_run is True + assert options.runner is None @pytest.mark.parametrize( From 2f9712d4cb77f75504c759bd34449a49675fc99b Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 8 Jun 2026 13:35:02 +0200 Subject: [PATCH 15/24] Fix nested lockfile discovery (#61) Use a recursive git pathspec for tracked Cargo.lock discovery and align test stubs with that command. Update stale-lockfile recovery text to point at direct cargo generate-lockfile repair. --- docs/developers-guide.md | 5 ++++- lading/commands/lockfile.py | 2 +- lading/commands/publish_preflight.py | 4 ++-- tests/bdd/steps/metadata_fixtures.py | 2 +- tests/bdd/steps/test_bump_steps.py | 2 +- tests/bdd/steps/test_publish_given_steps.py | 2 +- tests/bdd/steps/test_publish_infrastructure.py | 2 +- tests/unit/publish/test_run_integration.py | 3 ++- tests/unit/test_lockfile.py | 4 ++-- 9 files changed, 15 insertions(+), 11 deletions(-) diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 260fd92e..0abe0327 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -103,7 +103,10 @@ passes respectively. calls it after manifest rewrites. The refresh loop is intentionally not transactional: if a later lockfile refresh fails, previously rewritten manifests and refreshed lockfiles remain on disk, and the operator should fix -the Cargo error and rerun `lading bump` with the same version. +the Cargo error, then run +`cargo generate-lockfile --manifest-path /Cargo.toml` for each affected +crate manifest. Rerunning `lading bump` will not refresh lockfiles once the +manifests are already rewritten. `validate_lockfile_freshness(manifest_path, runner)` runs `cargo metadata --locked --manifest-path ... --format-version=1`. It returns diff --git a/lading/commands/lockfile.py b/lading/commands/lockfile.py index 80538cff..0f7c1e6a 100644 --- a/lading/commands/lockfile.py +++ b/lading/commands/lockfile.py @@ -112,7 +112,7 @@ def discover_tracked_lockfiles( through :func:`_handle_git_ls_files_failure` and returns an empty tuple. """ exit_code, stdout, stderr = runner( - ("git", "ls-files", "*/Cargo.lock", "Cargo.lock"), + ("git", "ls-files", "**/Cargo.lock", "Cargo.lock"), cwd=workspace_root, ) error_result = _handle_git_ls_files_failure( diff --git a/lading/commands/publish_preflight.py b/lading/commands/publish_preflight.py index d48603fb..0d42883c 100644 --- a/lading/commands/publish_preflight.py +++ b/lading/commands/publish_preflight.py @@ -226,8 +226,8 @@ def runner_with_env( lines = [ "Tracked Cargo.lock files are stale after manifest version changes.", ( - "This commonly happens after running `lading bump`; re-run it to " - "refresh tracked lockfiles, or repair each stale lockfile manually:" + "This commonly happens after running `lading bump`; repair each " + "stale lockfile directly:" ), ] for lockfile_path in stale_lockfiles: diff --git a/tests/bdd/steps/metadata_fixtures.py b/tests/bdd/steps/metadata_fixtures.py index cec35c30..5d5415d4 100644 --- a/tests/bdd/steps/metadata_fixtures.py +++ b/tests/bdd/steps/metadata_fixtures.py @@ -96,7 +96,7 @@ def _mock_cargo_metadata( stdout=json.dumps(payload), stderr="", ).any_order() - cmd_mox.stub("git").with_args("ls-files", "*/Cargo.lock", "Cargo.lock").returns( + cmd_mox.stub("git").with_args("ls-files", "**/Cargo.lock", "Cargo.lock").returns( exit_code=0, stdout="", stderr="" ).any_order() diff --git a/tests/bdd/steps/test_bump_steps.py b/tests/bdd/steps/test_bump_steps.py index 3a0c7c20..0bdc9675 100644 --- a/tests/bdd/steps/test_bump_steps.py +++ b/tests/bdd/steps/test_bump_steps.py @@ -31,7 +31,7 @@ def given_workspace_has_tracked_lockfiles( install_cargo_stub(cmd_mox, monkeypatch) (workspace_directory / "Cargo.lock").write_text("# root lock\n", encoding="utf-8") cmd_mox.stub("git").with_args( - "ls-files", "*/Cargo.lock", "Cargo.lock" + "ls-files", "**/Cargo.lock", "Cargo.lock" ).returns(exit_code=0, stdout="Cargo.lock\n", stderr="") cmd_mox.stub("cargo::generate-lockfile").with_args( "--manifest-path", str(workspace_directory / "Cargo.toml") diff --git a/tests/bdd/steps/test_publish_given_steps.py b/tests/bdd/steps/test_publish_given_steps.py index 1cc81e19..133a10e4 100644 --- a/tests/bdd/steps/test_publish_given_steps.py +++ b/tests/bdd/steps/test_publish_given_steps.py @@ -73,7 +73,7 @@ def given_publish_preflight_finds_stale_lockfile( ) -> None: """Simulate a tracked lockfile that fails locked metadata validation.""" (workspace_directory / "Cargo.lock").write_text("# stale lock\n", encoding="utf-8") - preflight_overrides["git", "ls-files", "*/Cargo.lock", "Cargo.lock"] = ( + preflight_overrides["git", "ls-files", "**/Cargo.lock", "Cargo.lock"] = ( _CommandResponse(exit_code=0, stdout="Cargo.lock\n") ) preflight_overrides[ diff --git a/tests/bdd/steps/test_publish_infrastructure.py b/tests/bdd/steps/test_publish_infrastructure.py index 2a9159cd..4d7c301c 100644 --- a/tests/bdd/steps/test_publish_infrastructure.py +++ b/tests/bdd/steps/test_publish_infrastructure.py @@ -185,7 +185,7 @@ def _register_preflight_commands( """ defaults: dict[tuple[str, ...], ResponseProvider] = { ("git", "status", "--porcelain"): _CommandResponse(exit_code=0), - ("git", "ls-files", "*/Cargo.lock", "Cargo.lock"): _CommandResponse( + ("git", "ls-files", "**/Cargo.lock", "Cargo.lock"): _CommandResponse( exit_code=0 ), ( diff --git a/tests/unit/publish/test_run_integration.py b/tests/unit/publish/test_run_integration.py index 39bf1e1e..fc01dd91 100644 --- a/tests/unit/publish/test_run_integration.py +++ b/tests/unit/publish/test_run_integration.py @@ -574,7 +574,8 @@ def skip_git_invoke( cwd: Path | None = None, env: cabc.Mapping[str, str] | None = None, ) -> tuple[int, str, str]: - if command == ("git", "status", "--porcelain"): + normalized_cmd = tuple(command) + if normalized_cmd == ("git", "status", "--porcelain"): message = "git status should be skipped by default" raise AssertionError(message) return 0, "", "" diff --git a/tests/unit/test_lockfile.py b/tests/unit/test_lockfile.py index 58aaf853..6b5b2396 100644 --- a/tests/unit/test_lockfile.py +++ b/tests/unit/test_lockfile.py @@ -23,7 +23,7 @@ def runner( cwd: Path | None = None, env: cabc.Mapping[str, str] | None = None, ) -> tuple[int, str, str]: - assert command == ("git", "ls-files", "*/Cargo.lock", "Cargo.lock") + assert command == ("git", "ls-files", "**/Cargo.lock", "Cargo.lock") assert cwd == tmp_path return 0, "", "" @@ -54,7 +54,7 @@ def runner( cwd: Path | None = None, env: cabc.Mapping[str, str] | None = None, ) -> tuple[int, str, str]: - assert command == ("git", "ls-files", "*/Cargo.lock", "Cargo.lock") + assert command == ("git", "ls-files", "**/Cargo.lock", "Cargo.lock") assert cwd == tmp_path assert env is None return ( From f35aff9d6b46a1d715b7b32d762d3a07dd167629 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 8 Jun 2026 13:41:55 +0200 Subject: [PATCH 16/24] Address lockfile review checks (#61) Inject the manifest-existence probe used during lockfile discovery so filesystem checks are explicit and replaceable. Add observability for lockfile discovery, refresh, and validation, plus regression coverage for partial refresh failures. Lock bump and publish remediation formatting with syrupy snapshots. --- lading/commands/lockfile.py | 39 +++++++++++++++--- .../test_bump_command_internals.ambr | 11 +++++ .../__snapshots__/test_preflight_checks.ambr | 11 +++++ tests/unit/publish/test_preflight_checks.py | 34 +++++++++++++++ tests/unit/test_bump_command_integration.py | 41 +++++++++++++++++++ tests/unit/test_bump_command_internals.py | 28 +++++++++++++ tests/unit/test_lockfile.py | 30 ++++++++++++++ 7 files changed, 188 insertions(+), 6 deletions(-) create mode 100644 tests/unit/__snapshots__/test_bump_command_internals.ambr create mode 100644 tests/unit/publish/__snapshots__/test_preflight_checks.ambr diff --git a/lading/commands/lockfile.py b/lading/commands/lockfile.py index 0f7c1e6a..7a09d10e 100644 --- a/lading/commands/lockfile.py +++ b/lading/commands/lockfile.py @@ -28,6 +28,7 @@ from __future__ import annotations +import collections.abc as cabc import logging import typing as typ from pathlib import Path @@ -38,6 +39,7 @@ from lading.commands.publish_execution import _CommandRunner LOGGER = logging.getLogger(__name__) +_ManifestExists = cabc.Callable[[Path], bool] class LockfileRefreshError(LadingError): @@ -69,6 +71,7 @@ def _handle_git_ls_files_failure( def _lockfiles_with_manifests( stdout: str, workspace_root: Path, + manifest_exists: _ManifestExists, ) -> tuple[Path, ...]: """Return lockfile paths from ``git ls-files`` with adjacent manifests.""" lockfiles: list[Path] = [] @@ -79,14 +82,21 @@ def _lockfiles_with_manifests( lockfile_path = workspace_root / relative_path if "target" in lockfile_path.relative_to(workspace_root).parts: continue - if (lockfile_path.parent / "Cargo.toml").exists(): + if manifest_exists(lockfile_path.parent / "Cargo.toml"): lockfiles.append(lockfile_path) return tuple(lockfiles) +def _manifest_exists(manifest_path: Path) -> bool: + """Return whether ``manifest_path`` exists on disk.""" + return manifest_path.exists() + + def discover_tracked_lockfiles( workspace_root: Path, runner: _CommandRunner, + *, + manifest_exists: _ManifestExists = _manifest_exists, ) -> tuple[Path, ...]: """Return tracked Cargo.lock files with adjacent manifests. @@ -97,6 +107,9 @@ def discover_tracked_lockfiles( runner Callable used to execute shell commands. It receives a command sequence and returns ``(exit_code, stdout, stderr)``. + manifest_exists + Callable used to decide whether a candidate lockfile has an adjacent + manifest. The default adapter checks the filesystem. Returns ------- @@ -120,7 +133,13 @@ def discover_tracked_lockfiles( ) if error_result is not None: return error_result - return _lockfiles_with_manifests(stdout, workspace_root) + lockfiles = _lockfiles_with_manifests(stdout, workspace_root, manifest_exists) + LOGGER.info( + "Discovered %d tracked lockfile(s) with adjacent manifests in %s", + len(lockfiles), + workspace_root, + ) + return lockfiles def refresh_lockfile( @@ -149,18 +168,20 @@ def refresh_lockfile( The error message includes Cargo's stderr, or stdout when stderr is empty, so callers can report why regeneration failed. """ + lockfile_path = manifest_path.parent / "Cargo.lock" + LOGGER.info("Refreshing %s", lockfile_path) exit_code, stdout, stderr = runner( ("cargo", "generate-lockfile", "--manifest-path", str(manifest_path)), cwd=manifest_path.parent, ) if exit_code != 0: detail = (stderr or stdout).strip() - message = f"Failed to refresh {manifest_path.parent / 'Cargo.lock'}" + message = f"Failed to refresh {lockfile_path}" if detail: message = f"{message}: {detail}" raise LockfileRefreshError(message) - LOGGER.info("Refreshed %s", manifest_path.parent / "Cargo.lock") - return manifest_path.parent / "Cargo.lock" + LOGGER.info("Refreshed %s", lockfile_path) + return lockfile_path def validate_lockfile_freshness( @@ -194,4 +215,10 @@ def validate_lockfile_freshness( ), cwd=manifest_path.parent, ) - return exit_code == 0 + is_fresh = exit_code == 0 + LOGGER.info( + "Validated lockfile freshness for %s: %s", + manifest_path, + "fresh" if is_fresh else "stale", + ) + return is_fresh diff --git a/tests/unit/__snapshots__/test_bump_command_internals.ambr b/tests/unit/__snapshots__/test_bump_command_internals.ambr new file mode 100644 index 00000000..6d2936fb --- /dev/null +++ b/tests/unit/__snapshots__/test_bump_command_internals.ambr @@ -0,0 +1,11 @@ +# serializer version: 1 +# name: test_format_result_message_snapshot + ''' + Dry run; would update version to 4.5.6 in 2 manifest(s) and 1 documentation file(s) and 2 lockfile(s): + - Cargo.toml + - crates/alpha/Cargo.toml + - README.md (documentation) + - Cargo.lock (lockfile) + - tests/ui_lints/Cargo.lock (lockfile) + ''' +# --- diff --git a/tests/unit/publish/__snapshots__/test_preflight_checks.ambr b/tests/unit/publish/__snapshots__/test_preflight_checks.ambr new file mode 100644 index 00000000..7536f7b0 --- /dev/null +++ b/tests/unit/publish/__snapshots__/test_preflight_checks.ambr @@ -0,0 +1,11 @@ +# serializer version: 1 +# name: test_validate_lockfile_freshness_error_snapshot + ''' + Tracked Cargo.lock files are stale after manifest version changes. + This commonly happens after running `lading bump`; repair each stale lockfile directly: + - /workspace root/Cargo.lock + cargo generate-lockfile --manifest-path '/workspace root/Cargo.toml' + - /workspace root/tests/ui_lints/Cargo.lock + cargo generate-lockfile --manifest-path '/workspace root/tests/ui_lints/Cargo.toml' + ''' +# --- diff --git a/tests/unit/publish/test_preflight_checks.py b/tests/unit/publish/test_preflight_checks.py index b7d9faf6..1240351f 100644 --- a/tests/unit/publish/test_preflight_checks.py +++ b/tests/unit/publish/test_preflight_checks.py @@ -15,6 +15,7 @@ if typ.TYPE_CHECKING: from pathlib import Path +from pathlib import Path def test_split_command_rejects_empty_sequence() -> None: @@ -420,6 +421,39 @@ def runner( "cargo generate-lockfile --manifest-path " f"{tmp_path / 'tests' / 'ui_lints' / 'Cargo.toml'}" ) in message + +def test_validate_lockfile_freshness_error_snapshot( + monkeypatch: pytest.MonkeyPatch, + snapshot: SnapshotAssertion, +) -> None: + """Stale lockfile remediation output is locked by snapshot.""" + workspace_root = Path("/workspace root") + root_lockfile = workspace_root / "Cargo.lock" + nested_lockfile = workspace_root / "tests" / "ui_lints" / "Cargo.lock" + + monkeypatch.setattr( + publish._publish_preflight, + "discover_tracked_lockfiles", + lambda _root, _runner: (root_lockfile, nested_lockfile), + ) + monkeypatch.setattr( + publish._publish_preflight, + "validate_lockfile_freshness", + lambda _manifest, _runner: False, + ) + + def runner( + command: cabc.Sequence[str], + *, + cwd: Path | None = None, + env: cabc.Mapping[str, str] | None = None, + ) -> tuple[int, str, str]: + return 0, "", "" + + with pytest.raises(publish.PublishPreflightError) as excinfo: + publish._validate_lockfile_freshness(workspace_root, runner=runner, env={}) + + assert str(excinfo.value) == snapshot() def test_preflight_env_overrides_forwarded( monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: diff --git a/tests/unit/test_bump_command_integration.py b/tests/unit/test_bump_command_integration.py index 1ae95838..78b8f69f 100644 --- a/tests/unit/test_bump_command_integration.py +++ b/tests/unit/test_bump_command_integration.py @@ -12,6 +12,7 @@ from lading import config as config_module from lading.commands import bump +from lading.commands.lockfile import LockfileRefreshError from lading.workspace import WorkspaceDependency, WorkspaceGraph from tests.helpers.workspace_builders import ( _build_workspace_with_internal_deps, @@ -158,6 +159,46 @@ def refresh( ) +def test_run_propagates_partial_lockfile_refresh_failure( + tmp_path: pathlib.Path, monkeypatch: MonkeyPatch +) -> None: + """Refresh failures leave prior refreshes observable before propagating.""" + workspace = _make_workspace(tmp_path) + first = tmp_path / "Cargo.lock" + second = tmp_path / "tests" / "ui_lints" / "Cargo.lock" + lockfiles = (first, second) + refreshed: list[pathlib.Path] = [] + + def discover( + workspace_root: pathlib.Path, + runner: bump._CommandRunner, + ) -> tuple[pathlib.Path, ...]: + return lockfiles + + def refresh( + manifest_path: pathlib.Path, + runner: bump._CommandRunner, + ) -> pathlib.Path: + lockfile_path = manifest_path.parent / "Cargo.lock" + refreshed.append(lockfile_path) + if lockfile_path == second: + msg = "failed after first refresh" + raise LockfileRefreshError(msg) + return lockfile_path + + monkeypatch.setattr(bump, "discover_tracked_lockfiles", discover) + monkeypatch.setattr(bump, "refresh_lockfile", refresh) + + with pytest.raises(LockfileRefreshError, match="failed after first refresh"): + bump.run( + tmp_path, + "1.2.3", + options=bump.BumpOptions(configuration=_make_config(), workspace=workspace), + ) + + assert refreshed == [first, second] + + def test_run_updates_root_package_section(tmp_path: pathlib.Path) -> None: """The workspace manifest `[package]` section also receives the new version.""" workspace = _make_workspace(tmp_path) diff --git a/tests/unit/test_bump_command_internals.py b/tests/unit/test_bump_command_internals.py index b6909bda..abc5363c 100644 --- a/tests/unit/test_bump_command_internals.py +++ b/tests/unit/test_bump_command_internals.py @@ -19,6 +19,8 @@ if typ.TYPE_CHECKING: from pathlib import Path + from syrupy.assertion import SnapshotAssertion + @dc.dataclass(frozen=True, slots=True) class UpdateCrateTestParams: @@ -327,6 +329,32 @@ def test_format_result_message_handles_changes(tmp_path: Path) -> None: ] +def test_format_result_message_snapshot( + tmp_path: Path, snapshot: SnapshotAssertion +) -> None: + """Bump output formatting is locked for manifest, docs, and lockfiles.""" + workspace_root = tmp_path + + message = bump._format_result_message( + bump.BumpChanges( + manifests=( + workspace_root / "Cargo.toml", + workspace_root / "crates" / "alpha" / "Cargo.toml", + ), + documents=(workspace_root / "README.md",), + lockfiles=( + workspace_root / "Cargo.lock", + workspace_root / "tests" / "ui_lints" / "Cargo.lock", + ), + ), + "4.5.6", + dry_run=True, + workspace_root=workspace_root, + ) + + assert message == snapshot() + + def test_update_manifest_writes_when_changed(tmp_path: Path) -> None: """Applying a new version persists changes to disk.""" manifest_path = tmp_path / "Cargo.toml" diff --git a/tests/unit/test_lockfile.py b/tests/unit/test_lockfile.py index 6b5b2396..dfe19a86 100644 --- a/tests/unit/test_lockfile.py +++ b/tests/unit/test_lockfile.py @@ -79,6 +79,36 @@ def runner( ) +def test_discover_tracked_lockfiles_accepts_manifest_probe( + tmp_path: Path, +) -> None: + """Manifest filtering is delegated to the injected probe.""" + probed: list[Path] = [] + + def runner( + command: cabc.Sequence[str], + *, + cwd: Path | None = None, + env: cabc.Mapping[str, str] | None = None, + ) -> tuple[int, str, str]: + return 0, "Cargo.lock\nnested/Cargo.lock\n", "" + + def manifest_exists(manifest_path: Path) -> bool: + probed.append(manifest_path) + return ( + manifest_path.name == "Cargo.toml" and manifest_path.parent.name != "nested" + ) + + result = lockfile.discover_tracked_lockfiles( + tmp_path, + runner, + manifest_exists=manifest_exists, + ) + + assert result == (tmp_path / "Cargo.lock",) + assert probed == [tmp_path / "Cargo.toml", tmp_path / "nested" / "Cargo.toml"] + + def test_discover_tracked_lockfiles_handles_non_git_directory( tmp_path: Path, ) -> None: From ad565e979ed92406746e392ee8814d9e382d9532 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 8 Jun 2026 13:43:41 +0200 Subject: [PATCH 17/24] Document LadingError reuse guidance (#61) Expand the developer guide exception hierarchy section with package-level LadingError guidance, a reuse plan for new domain exceptions, and caller/test usage rules. --- docs/developers-guide.md | 54 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 49 insertions(+), 5 deletions(-) diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 0abe0327..b2799317 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -190,12 +190,56 @@ yet. When enabled, `lading publish` downgrades that specific index-lookup failure to a warning and continues. The option is rejected at runtime when `live=True`, so it cannot mask a real upload failure. -### Exception hierarchy (`publish_errors`) -`lading.commands.publish_errors` defines the public error boundary for -publish orchestration. Both classes inherit from the package-level -`LadingError` base, which itself extends `Exception`, and carry their message -through the standard `args` tuple. +### Exception hierarchy (`lading.exceptions`) + +`lading.exceptions.LadingError` is the package-level base class for domain +failures raised by lading itself. It extends `Exception` directly and gives +callers one stable type to catch when they want to handle expected lading +failures without also catching unrelated runtime errors from Python, Cargo, git +wrappers, or test doubles. + +Every root domain exception should inherit from `LadingError`. More specific +exceptions should continue to inherit from the local root for their feature +area so existing handling remains precise: + +| Root exception | Module | Notes | +| --- | --- | --- | +| `ConfigurationError` | `lading.config` | Base for configuration loading and validation failures. | +| `WorkspaceModelError` | `lading.workspace.models` | Base for workspace graph/model validation failures. | +| `CargoMetadataError` | `lading.workspace.metadata` | Base for cargo metadata execution and parsing failures. | +| `LockfileRefreshError` | `lading.commands.lockfile` | Raised when `cargo generate-lockfile` fails. | +| `PublishPlanError` | `lading.commands.publish_plan` | Raised when a publish plan cannot be constructed. | +| `PublishPreparationError` | `lading.commands.publish_manifest` | Raised when staged publish manifests or workspace assets cannot be prepared. | +| `PublishPreflightError` | `lading.commands.publish_errors` | Raised for local publish validation and pre-flight failures. | + +Reuse plan: + +- New command, workspace, and configuration modules should define exactly one + local root exception that subclasses `LadingError` when they introduce a new + failure family. +- Subclasses should inherit from that local root, not from `LadingError` + directly, unless they are themselves the root of a new family. +- Do not inherit lading domain exceptions from `RuntimeError`; reserve + `RuntimeError` for unexpected programming errors or third-party APIs that + already expose it. +- Keep messages useful at the boundary where they are raised. Include the + relevant path, crate name, command, or configuration key so CLI handlers can + report the exception without reconstructing context. + +Usage guidance: + +- CLI and integration boundaries may catch `LadingError` to render expected + lading failures as user-facing diagnostics. +- Feature code should catch the narrowest local exception it can handle, such + as `PublishPreflightError` or `LockfileRefreshError`, and let unrelated + `LadingError` subclasses propagate. +- Tests should assert the specific exception type for the behaviour under test, + then use `LadingError` only when verifying common boundary handling. + +`lading.commands.publish_errors` defines the public error boundary for publish +orchestration. Both publish exceptions inherit from the package-level +`LadingError` base and carry their message through the standard `args` tuple. | Exception | Raised when | | --- | --- | From 4466c9cac393bdb4189709d702a53e9ea7f5a022 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 8 Jun 2026 14:58:26 +0200 Subject: [PATCH 18/24] Split publish preflight tests (#61) Move command utility, cargo runner, metadata decoding, and lockfile validation tests out of `test_preflight_checks.py` so the remaining file covers only pre-flight orchestration. Relocate the syrupy snapshot with the moved lockfile-validation test module while preserving the snapshot key and content. --- ...> test_preflight_lockfile_validation.ambr} | 0 tests/unit/publish/test_command_helpers.py | 56 +++- .../publish/test_preflight_cargo_runner.py | 169 ++++++++++ tests/unit/publish/test_preflight_checks.py | 316 +----------------- .../test_preflight_lockfile_validation.py | 122 +++++++ tests/unit/test_cargo_metadata_loading.py | 9 +- 6 files changed, 357 insertions(+), 315 deletions(-) rename tests/unit/publish/__snapshots__/{test_preflight_checks.ambr => test_preflight_lockfile_validation.ambr} (100%) create mode 100644 tests/unit/publish/test_preflight_cargo_runner.py create mode 100644 tests/unit/publish/test_preflight_lockfile_validation.py diff --git a/tests/unit/publish/__snapshots__/test_preflight_checks.ambr b/tests/unit/publish/__snapshots__/test_preflight_lockfile_validation.ambr similarity index 100% rename from tests/unit/publish/__snapshots__/test_preflight_checks.ambr rename to tests/unit/publish/__snapshots__/test_preflight_lockfile_validation.ambr diff --git a/tests/unit/publish/test_command_helpers.py b/tests/unit/publish/test_command_helpers.py index d8e36d3b..8c3730fd 100644 --- a/tests/unit/publish/test_command_helpers.py +++ b/tests/unit/publish/test_command_helpers.py @@ -1,12 +1,14 @@ """Unit tests for publish command execution helpers.""" +from pathlib import Path import importlib import io -from pathlib import Path from lading.testing import cmd_mox_runner execution = importlib.import_module("lading.runtime.subprocess_runner") +from lading.commands import publish +import pytest def test_normalise_environment_handles_none_and_values() -> None: @@ -66,3 +68,55 @@ def test_echo_buffered_output_skips_empty_payloads() -> None: sink = io.StringIO() cmd_mox_runner._echo_buffered_output("", sink) assert sink.getvalue() == "" + +def test_split_command_rejects_empty_sequence() -> None: + """Splitting an empty command raises a descriptive error.""" + with pytest.raises(publish.PublishPreflightError) as excinfo: + publish._split_command(()) + + assert "Command sequence must contain" in str(excinfo.value) + +@pytest.mark.parametrize( + "command", + [ + ("cargo", "check"), + ("cargo", "test", "--workspace"), + ("git", "status", "--porcelain"), + ], +) +def test_normalise_cmd_mox_command_forwards_non_cargo_commands( + command: tuple[str, ...], +) -> None: + """cmd-mox normalisation preserves non-cargo commands and arguments.""" + program, args = command[0], tuple(command[1:]) + + rewritten_program, rewritten_args = publish._normalise_cmd_mox_command( + program, args + ) + + if program == "cargo" and args: + expected_program = f"cargo::{args[0]}" + expected_args = list(args[1:]) + else: + expected_program = program + expected_args = list(args) + + assert rewritten_program == expected_program + assert rewritten_args == expected_args + +@pytest.mark.parametrize("value", ["1", "true", "TRUE", "Yes", "on"]) +def test_should_use_cmd_mox_stub_honours_truthy_values( + value: str, monkeypatch: pytest.MonkeyPatch +) -> None: + """Environment values recognised as truthy enable cmd-mox stubbing.""" + monkeypatch.setenv(publish.metadata_module.CMD_MOX_STUB_ENV_VAR, value) + + assert publish._should_use_cmd_mox_stub() is True + +def test_should_use_cmd_mox_stub_returns_false_by_default( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Missing environment values disable cmd-mox stubbing.""" + monkeypatch.delenv(publish.metadata_module.CMD_MOX_STUB_ENV_VAR, raising=False) + + assert publish._should_use_cmd_mox_stub() is False diff --git a/tests/unit/publish/test_preflight_cargo_runner.py b/tests/unit/publish/test_preflight_cargo_runner.py new file mode 100644 index 00000000..78182917 --- /dev/null +++ b/tests/unit/publish/test_preflight_cargo_runner.py @@ -0,0 +1,169 @@ +"""Unit tests for the low-level _run_cargo_preflight helper.""" + +from __future__ import annotations + +import collections.abc as cabc +import typing as typ +from pathlib import Path + +import pytest + +from lading.commands import publish + +from .conftest import ORIGINAL_PREFLIGHT + + +def test_run_cargo_preflight_raises_on_failure( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """Non-zero command results are converted into preflight errors.""" + + def failing_runner( + command: tuple[str, ...], + *, + cwd: Path | None = None, + env: cabc.Mapping[str, str] | None = None, + ) -> tuple[int, str, str]: + assert cwd == tmp_path + assert command[0] == "cargo" + return 1, "", "boom" + + with pytest.raises(publish.PublishPreflightError) as excinfo: + publish._run_cargo_preflight( + tmp_path, + "check", + runner=failing_runner, + options=publish._CargoPreflightOptions(extra_args=("--workspace",)), + ) + + message = str(excinfo.value) + assert "cargo check" in message + assert "boom" in message + + +def _run_and_record_cargo_preflight( + workspace_root: Path, + subcommand: typ.Literal["check", "test"], + options: publish._CargoPreflightOptions, +) -> tuple[str, ...]: + """Run cargo preflight with a recording runner and return the command. + + Returns + ------- + The recorded cargo command as a tuple of strings. + + """ + recorded: list[tuple[str, ...]] = [] + + def recording_runner( + command: tuple[str, ...], + *, + cwd: Path | None = None, + env: cabc.Mapping[str, str] | None = None, + ) -> tuple[int, str, str]: + recorded.append(command) + return 0, "", "" + + publish._run_cargo_preflight( + workspace_root, + subcommand, + runner=recording_runner, + options=options, + ) + + assert len(recorded) == 1, f"Expected 1 recorded command, got {len(recorded)}" + return recorded.pop() + + +def test_run_cargo_preflight_honours_test_excludes(tmp_path: Path) -> None: + """Configured test exclusions append ``--exclude`` arguments.""" + command = _run_and_record_cargo_preflight( + tmp_path, + "test", + publish._CargoPreflightOptions( + extra_args=("--workspace", "--all-targets"), + test_excludes=(" alpha ", "", "beta"), + ), + ) + assert command[:2] == ("cargo", "test") + assert command[2:4] == ("--workspace", "--all-targets") + assert command[4:] == ("--exclude", "alpha", "--exclude", "beta") + + +def test_run_cargo_preflight_excludes_blank_entries(tmp_path: Path) -> None: + """Blank test exclude entries do not emit ``--exclude`` arguments.""" + command = _run_and_record_cargo_preflight( + tmp_path, + "test", + publish._CargoPreflightOptions( + extra_args=("--workspace", "--all-targets"), + test_excludes=["", " ", "\t", "\n"], + ), + ) + assert "--exclude" not in command + + +def test_run_cargo_preflight_honours_unit_tests_only(tmp_path: Path) -> None: + """The unit test flag narrows cargo test targets to lib and bins.""" + command = _run_and_record_cargo_preflight( + tmp_path, + "test", + publish._CargoPreflightOptions( + extra_args=("--workspace", "--all-targets"), unit_tests_only=True + ), + ) + assert command[:2] == ("cargo", "test") + assert command[2:4] == ("--workspace", "--all-targets") + assert command[4:6] == ("--lib", "--bins") + + +def test_run_cargo_preflight_defaults_when_unit_tests_only_false( + tmp_path: Path, +) -> None: + """When unit-tests-only is disabled, no target narrowing arguments are added.""" + command = _run_and_record_cargo_preflight( + tmp_path, + "test", + publish._CargoPreflightOptions( + extra_args=("--workspace", "--all-targets"), unit_tests_only=False + ), + ) + assert command[:2] == ("cargo", "test") + assert command[2:4] == ("--workspace", "--all-targets") + assert "--lib" not in command + assert "--bins" not in command + + +def test_compiletest_diagnostic_details( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """Failing cargo test pre-flight lists stderr artifacts with tail output.""" + monkeypatch.setattr(publish, "_run_preflight_checks", ORIGINAL_PREFLIGHT) + artifact = tmp_path / "ui.stderr" + artifact.write_text("line1\nline2\nline3\n", encoding="utf-8") + + def failing_runner( + command: tuple[str, ...], + *, + cwd: Path | None = None, + env: cabc.Mapping[str, str] | None = None, + ) -> tuple[int, str, str]: + return 1, f"diff at {artifact}", "" + + options = publish._CargoPreflightOptions( + extra_args=("--workspace",), + env={}, + diagnostics_tail_lines=2, + ) + with pytest.raises(publish.PublishPreflightError) as excinfo: + publish._run_cargo_preflight( + tmp_path, + "test", + runner=failing_runner, + options=options, + ) + + message = str(excinfo.value) + assert str(artifact) in message + assert "line2" in message + assert "line3" in message diff --git a/tests/unit/publish/test_preflight_checks.py b/tests/unit/publish/test_preflight_checks.py index 1240351f..28d46d01 100644 --- a/tests/unit/publish/test_preflight_checks.py +++ b/tests/unit/publish/test_preflight_checks.py @@ -4,187 +4,15 @@ import collections.abc as cabc import typing as typ +from pathlib import Path import pytest -from lading.commands import publish, publish_execution, publish_preflight -from lading.runtime import CommandRunner, coerce_text -from lading.testing.cmd_mox_runner import normalise_cmd_mox_command +from lading.commands import publish, publish_preflight +from lading.runtime import CommandRunner from .conftest import ORIGINAL_PREFLIGHT, make_config, make_preflight_config -if typ.TYPE_CHECKING: - from pathlib import Path -from pathlib import Path - - -def test_split_command_rejects_empty_sequence() -> None: - """Splitting an empty command raises a descriptive error.""" - with pytest.raises(publish.PublishPreflightError) as excinfo: - publish_execution.split_command(()) - - assert "Command sequence must contain" in str(excinfo.value) - - -@pytest.mark.parametrize( - "command", - [ - ("cargo", "check"), - ("cargo", "test", "--workspace"), - ("git", "status", "--porcelain"), - ], -) -def test_normalise_cmd_mox_command_forwards_non_cargo_commands( - command: tuple[str, ...], -) -> None: - """cmd-mox normalisation preserves non-cargo commands and arguments.""" - program, args = command[0], tuple(command[1:]) - - rewritten_program, rewritten_args = normalise_cmd_mox_command(program, args) - - if program == "cargo" and args: - expected_program = f"cargo::{args[0]}" - expected_args = list(args[1:]) - else: - expected_program = program - expected_args = list(args) - - assert rewritten_program == expected_program - assert rewritten_args == expected_args - - -def test_metadata_coerce_text_decodes_bytes() -> None: - """Binary output is decoded using UTF-8 with replacement semantics.""" - alpha = "\N{GREEK SMALL LETTER ALPHA}" - encoded = alpha.encode() - assert coerce_text(encoded) == alpha - - binary = b"foo\xff" - assert coerce_text(binary) == "foo\ufffd" - - -def test_run_cargo_preflight_raises_on_failure( - monkeypatch: pytest.MonkeyPatch, tmp_path: Path -) -> None: - """Non-zero command results are converted into preflight errors.""" - - def failing_runner( - command: tuple[str, ...], - *, - cwd: Path | None = None, - env: cabc.Mapping[str, str] | None = None, - ) -> tuple[int, str, str]: - assert cwd == tmp_path - assert command[0] == "cargo" - return 1, "", "boom" - - with pytest.raises(publish.PublishPreflightError) as excinfo: - publish_preflight._run_cargo_preflight( - tmp_path, - "check", - runner=failing_runner, - options=publish_preflight._CargoPreflightOptions( - extra_args=("--workspace",) - ), - ) - - message = str(excinfo.value) - assert "cargo check" in message - assert "boom" in message - - -def _run_and_record_cargo_preflight( - workspace_root: Path, - subcommand: typ.Literal["check", "test"], - options: publish_preflight._CargoPreflightOptions, -) -> tuple[str, ...]: - """Run cargo preflight with a recording runner and return the command. - - Returns - ------- - The recorded cargo command as a tuple of strings. - - """ - recorded: list[tuple[str, ...]] = [] - - def recording_runner( - command: tuple[str, ...], - *, - cwd: Path | None = None, - env: cabc.Mapping[str, str] | None = None, - ) -> tuple[int, str, str]: - recorded.append(command) - return 0, "", "" - - publish_preflight._run_cargo_preflight( - workspace_root, - subcommand, - runner=recording_runner, - options=options, - ) - - assert len(recorded) == 1, f"Expected 1 recorded command, got {len(recorded)}" - return recorded.pop() - - -def test_run_cargo_preflight_honours_test_excludes(tmp_path: Path) -> None: - """Configured test exclusions append ``--exclude`` arguments.""" - command = _run_and_record_cargo_preflight( - tmp_path, - "test", - publish_preflight._CargoPreflightOptions( - extra_args=("--workspace", "--all-targets"), - test_excludes=(" alpha ", "", "beta"), - ), - ) - assert command[:2] == ("cargo", "test") - assert command[2:4] == ("--workspace", "--all-targets") - assert command[4:] == ("--exclude", "alpha", "--exclude", "beta") - - -def test_run_cargo_preflight_excludes_blank_entries(tmp_path: Path) -> None: - """Blank test exclude entries do not emit ``--exclude`` arguments.""" - command = _run_and_record_cargo_preflight( - tmp_path, - "test", - publish_preflight._CargoPreflightOptions( - extra_args=("--workspace", "--all-targets"), - test_excludes=["", " ", "\t", "\n"], - ), - ) - assert "--exclude" not in command - - -def test_run_cargo_preflight_honours_unit_tests_only(tmp_path: Path) -> None: - """The unit test flag narrows cargo test targets to lib and bins.""" - command = _run_and_record_cargo_preflight( - tmp_path, - "test", - publish_preflight._CargoPreflightOptions( - extra_args=("--workspace", "--all-targets"), unit_tests_only=True - ), - ) - assert command[:2] == ("cargo", "test") - assert command[2:4] == ("--workspace", "--all-targets") - assert command[4:6] == ("--lib", "--bins") - - -def test_run_cargo_preflight_defaults_when_unit_tests_only_false( - tmp_path: Path, -) -> None: - """When unit-tests-only is disabled, no target narrowing arguments are added.""" - command = _run_and_record_cargo_preflight( - tmp_path, - "test", - publish_preflight._CargoPreflightOptions( - extra_args=("--workspace", "--all-targets"), unit_tests_only=False - ), - ) - assert command[:2] == ("cargo", "test") - assert command[2:4] == ("--workspace", "--all-targets") - assert "--lib" not in command - assert "--bins" not in command - def test_preflight_checks_remove_all_targets_for_unit_only( monkeypatch: pytest.MonkeyPatch, tmp_path: Path @@ -350,110 +178,7 @@ def runner( assert "cargo build --package lint" in str(excinfo.value) -def test_validate_lockfile_freshness_passes_when_all_lockfiles_are_fresh( - monkeypatch: pytest.MonkeyPatch, tmp_path: Path -) -> None: - """Fresh tracked lockfiles allow preflight to continue.""" - root_lockfile = tmp_path / "Cargo.lock" - nested_lockfile = tmp_path / "tests" / "ui_lints" / "Cargo.lock" - recorded_env: list[cabc.Mapping[str, str] | None] = [] - - monkeypatch.setattr( - publish._publish_preflight, - "discover_tracked_lockfiles", - lambda _root, _runner: (root_lockfile, nested_lockfile), - ) - - def runner( - command: cabc.Sequence[str], - *, - cwd: Path | None = None, - env: cabc.Mapping[str, str] | None = None, - ) -> tuple[int, str, str]: - recorded_env.append(env) - return 0, "", "" - - publish._validate_lockfile_freshness( - tmp_path, - runner=runner, - env={"CARGO_TERM_COLOR": "never"}, - ) - - assert recorded_env == [{"CARGO_TERM_COLOR": "never"}] * 2 - -def test_validate_lockfile_freshness_reports_stale_lockfiles( - monkeypatch: pytest.MonkeyPatch, tmp_path: Path -) -> None: - """Stale lockfiles are collected and reported with repair commands.""" - root_lockfile = tmp_path / "Cargo.lock" - nested_lockfile = tmp_path / "tests" / "ui_lints" / "Cargo.lock" - - monkeypatch.setattr( - publish._publish_preflight, - "discover_tracked_lockfiles", - lambda _root, _runner: (root_lockfile, nested_lockfile), - ) - monkeypatch.setattr( - publish._publish_preflight, - "validate_lockfile_freshness", - lambda _manifest, _runner: False, - ) - - def runner( - command: tuple[str, ...], - *, - cwd: Path | None = None, - env: cabc.Mapping[str, str] | None = None, - ) -> tuple[int, str, str]: - return 0, "", "" - - with pytest.raises(publish.PublishPreflightError) as excinfo: - publish._validate_lockfile_freshness(tmp_path, runner=runner, env={}) - - message = str(excinfo.value) - assert str(root_lockfile) in message - assert str(nested_lockfile) in message - assert "lading bump" in message - assert ( - f"cargo generate-lockfile --manifest-path {tmp_path / 'Cargo.toml'}" in message - ) - assert ( - "cargo generate-lockfile --manifest-path " - f"{tmp_path / 'tests' / 'ui_lints' / 'Cargo.toml'}" - ) in message - -def test_validate_lockfile_freshness_error_snapshot( - monkeypatch: pytest.MonkeyPatch, - snapshot: SnapshotAssertion, -) -> None: - """Stale lockfile remediation output is locked by snapshot.""" - workspace_root = Path("/workspace root") - root_lockfile = workspace_root / "Cargo.lock" - nested_lockfile = workspace_root / "tests" / "ui_lints" / "Cargo.lock" - - monkeypatch.setattr( - publish._publish_preflight, - "discover_tracked_lockfiles", - lambda _root, _runner: (root_lockfile, nested_lockfile), - ) - monkeypatch.setattr( - publish._publish_preflight, - "validate_lockfile_freshness", - lambda _manifest, _runner: False, - ) - - def runner( - command: cabc.Sequence[str], - *, - cwd: Path | None = None, - env: cabc.Mapping[str, str] | None = None, - ) -> tuple[int, str, str]: - return 0, "", "" - - with pytest.raises(publish.PublishPreflightError) as excinfo: - publish._validate_lockfile_freshness(workspace_root, runner=runner, env={}) - assert str(excinfo.value) == snapshot() def test_preflight_env_overrides_forwarded( monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: @@ -541,41 +266,6 @@ def recording_runner( assert str(artifact) in last_flags -def test_compiletest_diagnostic_details( - monkeypatch: pytest.MonkeyPatch, tmp_path: Path -) -> None: - """Failing cargo test pre-flight lists stderr artifacts with tail output.""" - monkeypatch.setattr(publish, "_run_preflight_checks", ORIGINAL_PREFLIGHT) - artifact = tmp_path / "ui.stderr" - artifact.write_text("line1\nline2\nline3\n", encoding="utf-8") - - def failing_runner( - command: tuple[str, ...], - *, - cwd: Path | None = None, - env: cabc.Mapping[str, str] | None = None, - ) -> tuple[int, str, str]: - return 1, f"diff at {artifact}", "" - - options = publish_preflight._CargoPreflightOptions( - extra_args=("--workspace",), - env={}, - diagnostics_tail_lines=2, - ) - with pytest.raises(publish.PublishPreflightError) as excinfo: - publish_preflight._run_cargo_preflight( - tmp_path, - "test", - runner=failing_runner, - options=options, - ) - - message = str(excinfo.value) - assert str(artifact) in message - assert "line2" in message - assert "line3" in message - - def test_verify_clean_working_tree_detects_dirty_state( monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: diff --git a/tests/unit/publish/test_preflight_lockfile_validation.py b/tests/unit/publish/test_preflight_lockfile_validation.py new file mode 100644 index 00000000..39227e53 --- /dev/null +++ b/tests/unit/publish/test_preflight_lockfile_validation.py @@ -0,0 +1,122 @@ +"""Unit tests for _validate_lockfile_freshness pre-flight helper.""" + +from __future__ import annotations + +import collections.abc as cabc +import typing as typ +from pathlib import Path + +import pytest + +from lading.commands import publish + +if typ.TYPE_CHECKING: + from syrupy.assertion import SnapshotAssertion + + +def test_validate_lockfile_freshness_passes_when_all_lockfiles_are_fresh( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """Fresh tracked lockfiles allow preflight to continue.""" + root_lockfile = tmp_path / "Cargo.lock" + nested_lockfile = tmp_path / "tests" / "ui_lints" / "Cargo.lock" + recorded_env: list[cabc.Mapping[str, str] | None] = [] + + monkeypatch.setattr( + publish._publish_preflight, + "discover_tracked_lockfiles", + lambda _root, _runner: (root_lockfile, nested_lockfile), + ) + + def runner( + command: cabc.Sequence[str], + *, + cwd: Path | None = None, + env: cabc.Mapping[str, str] | None = None, + ) -> tuple[int, str, str]: + recorded_env.append(env) + return 0, "", "" + + publish._validate_lockfile_freshness( + tmp_path, + runner=runner, + env={"CARGO_TERM_COLOR": "never"}, + ) + + assert recorded_env == [{"CARGO_TERM_COLOR": "never"}] * 2 + + +def test_validate_lockfile_freshness_reports_stale_lockfiles( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """Stale lockfiles are collected and reported with repair commands.""" + root_lockfile = tmp_path / "Cargo.lock" + nested_lockfile = tmp_path / "tests" / "ui_lints" / "Cargo.lock" + + monkeypatch.setattr( + publish._publish_preflight, + "discover_tracked_lockfiles", + lambda _root, _runner: (root_lockfile, nested_lockfile), + ) + monkeypatch.setattr( + publish._publish_preflight, + "validate_lockfile_freshness", + lambda _manifest, _runner: False, + ) + + def runner( + command: tuple[str, ...], + *, + cwd: Path | None = None, + env: cabc.Mapping[str, str] | None = None, + ) -> tuple[int, str, str]: + return 0, "", "" + + with pytest.raises(publish.PublishPreflightError) as excinfo: + publish._validate_lockfile_freshness(tmp_path, runner=runner, env={}) + + message = str(excinfo.value) + assert str(root_lockfile) in message + assert str(nested_lockfile) in message + assert "lading bump" in message + assert ( + f"cargo generate-lockfile --manifest-path {tmp_path / 'Cargo.toml'}" in message + ) + assert ( + "cargo generate-lockfile --manifest-path " + f"{tmp_path / 'tests' / 'ui_lints' / 'Cargo.toml'}" + ) in message + + +def test_validate_lockfile_freshness_error_snapshot( + monkeypatch: pytest.MonkeyPatch, + snapshot: SnapshotAssertion, +) -> None: + """Stale lockfile remediation output is locked by snapshot.""" + workspace_root = Path("/workspace root") + root_lockfile = workspace_root / "Cargo.lock" + nested_lockfile = workspace_root / "tests" / "ui_lints" / "Cargo.lock" + + monkeypatch.setattr( + publish._publish_preflight, + "discover_tracked_lockfiles", + lambda _root, _runner: (root_lockfile, nested_lockfile), + ) + monkeypatch.setattr( + publish._publish_preflight, + "validate_lockfile_freshness", + lambda _manifest, _runner: False, + ) + + def runner( + command: cabc.Sequence[str], + *, + cwd: Path | None = None, + env: cabc.Mapping[str, str] | None = None, + ) -> tuple[int, str, str]: + return 0, "", "" + + with pytest.raises(publish.PublishPreflightError) as excinfo: + publish._validate_lockfile_freshness(workspace_root, runner=runner, env={}) + + assert str(excinfo.value) == snapshot() diff --git a/tests/unit/test_cargo_metadata_loading.py b/tests/unit/test_cargo_metadata_loading.py index 34578c58..5ec1acda 100644 --- a/tests/unit/test_cargo_metadata_loading.py +++ b/tests/unit/test_cargo_metadata_loading.py @@ -124,7 +124,14 @@ def test_load_cargo_metadata_error_decodes_byte_streams( assert "manifest missing" in str(excinfo.value) - +def test_metadata_coerce_text_decodes_bytes() -> None: + """Binary output is decoded using UTF-8 with replacement semantics.""" + alpha = "\N{GREEK SMALL LETTER ALPHA}" + encoded = alpha.encode() + assert metadata_module._coerce_text(encoded) == alpha + + binary = b"foo\xff" + assert metadata_module._coerce_text(binary) == "foo\ufffd" @pytest.mark.parametrize( "scenario", [ From 411d6330ff1e56a6110f342db8760d7e14b5ce64 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 8 Jun 2026 18:07:14 +0200 Subject: [PATCH 19/24] Parametrize cargo preflight argument tests (#61) Replace four structurally identical cargo preflight command tests with a single parametrized scenario table. Keep the failure and diagnostic tests unchanged while preserving the same option combinations and expected command arguments. --- .../publish/test_preflight_cargo_runner.py | 112 ++++++++++-------- 1 file changed, 60 insertions(+), 52 deletions(-) diff --git a/tests/unit/publish/test_preflight_cargo_runner.py b/tests/unit/publish/test_preflight_cargo_runner.py index 78182917..18482eeb 100644 --- a/tests/unit/publish/test_preflight_cargo_runner.py +++ b/tests/unit/publish/test_preflight_cargo_runner.py @@ -3,6 +3,7 @@ from __future__ import annotations import collections.abc as cabc +import dataclasses as dc import typing as typ from pathlib import Path @@ -41,6 +42,14 @@ def failing_runner( assert "boom" in message +@dc.dataclass(frozen=True) +class _RunCargoPreflightCase: + """Parameters for a single cargo-preflight argument-construction scenario.""" + + options: publish._CargoPreflightOptions + expected_tail: tuple[str, ...] + + def _run_and_record_cargo_preflight( workspace_root: Path, subcommand: typ.Literal["check", "test"], @@ -75,63 +84,62 @@ def recording_runner( return recorded.pop() -def test_run_cargo_preflight_honours_test_excludes(tmp_path: Path) -> None: - """Configured test exclusions append ``--exclude`` arguments.""" - command = _run_and_record_cargo_preflight( - tmp_path, - "test", - publish._CargoPreflightOptions( - extra_args=("--workspace", "--all-targets"), - test_excludes=(" alpha ", "", "beta"), +@pytest.mark.parametrize( + "scenario", + [ + pytest.param( + _RunCargoPreflightCase( + options=publish._CargoPreflightOptions( + extra_args=("--workspace", "--all-targets"), + test_excludes=(" alpha ", "", "beta"), + ), + expected_tail=("--exclude", "alpha", "--exclude", "beta"), + ), + id="test_excludes", ), - ) - assert command[:2] == ("cargo", "test") - assert command[2:4] == ("--workspace", "--all-targets") - assert command[4:] == ("--exclude", "alpha", "--exclude", "beta") - - -def test_run_cargo_preflight_excludes_blank_entries(tmp_path: Path) -> None: - """Blank test exclude entries do not emit ``--exclude`` arguments.""" - command = _run_and_record_cargo_preflight( - tmp_path, - "test", - publish._CargoPreflightOptions( - extra_args=("--workspace", "--all-targets"), - test_excludes=["", " ", "\t", "\n"], + pytest.param( + _RunCargoPreflightCase( + options=publish._CargoPreflightOptions( + extra_args=("--workspace", "--all-targets"), + test_excludes=["", " ", "\t", "\n"], + ), + expected_tail=(), + ), + id="blank_test_excludes", ), - ) - assert "--exclude" not in command - - -def test_run_cargo_preflight_honours_unit_tests_only(tmp_path: Path) -> None: - """The unit test flag narrows cargo test targets to lib and bins.""" - command = _run_and_record_cargo_preflight( - tmp_path, - "test", - publish._CargoPreflightOptions( - extra_args=("--workspace", "--all-targets"), unit_tests_only=True + pytest.param( + _RunCargoPreflightCase( + options=publish._CargoPreflightOptions( + extra_args=("--workspace", "--all-targets"), + unit_tests_only=True, + ), + expected_tail=("--lib", "--bins"), + ), + id="unit_tests_only", ), - ) - assert command[:2] == ("cargo", "test") - assert command[2:4] == ("--workspace", "--all-targets") - assert command[4:6] == ("--lib", "--bins") - - -def test_run_cargo_preflight_defaults_when_unit_tests_only_false( - tmp_path: Path, -) -> None: - """When unit-tests-only is disabled, no target narrowing arguments are added.""" - command = _run_and_record_cargo_preflight( - tmp_path, - "test", - publish._CargoPreflightOptions( - extra_args=("--workspace", "--all-targets"), unit_tests_only=False + pytest.param( + _RunCargoPreflightCase( + options=publish._CargoPreflightOptions( + extra_args=("--workspace", "--all-targets"), + unit_tests_only=False, + ), + expected_tail=(), + ), + id="unit_tests_only_false", ), + ], +) +def test_run_cargo_preflight_command_arguments( + tmp_path: Path, scenario: _RunCargoPreflightCase +) -> None: + """Cargo preflight constructs correct arguments for each option combination.""" + command = _run_and_record_cargo_preflight(tmp_path, "test", scenario.options) + assert command[:4] == ("cargo", "test", "--workspace", "--all-targets"), ( + f"Unexpected command prefix: {command[:4]}" + ) + assert command[4:] == scenario.expected_tail, ( + f"Unexpected command tail: {command[4:]!r}, expected {scenario.expected_tail!r}" ) - assert command[:2] == ("cargo", "test") - assert command[2:4] == ("--workspace", "--all-targets") - assert "--lib" not in command - assert "--bins" not in command def test_compiletest_diagnostic_details( From 62558d6eb34aa3ba52e5e29372495380e85ff77f Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 8 Jun 2026 18:19:30 +0200 Subject: [PATCH 20/24] Document exception and bump internals (#61) Expand the package exception module documentation so `LadingError` is clearly identified as the common root for lading domain failures. Add bump command internals guidance covering the `BumpOptions.runner` injection point and the `BumpChanges.lockfiles` summary field. --- docs/developers-guide.md | 16 ++++++++++++++++ lading/exceptions.py | 15 ++++++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/docs/developers-guide.md b/docs/developers-guide.md index b2799317..4d1366ee 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -181,6 +181,22 @@ Examples: - `PublishOptions(allow_dirty=False)` — require a clean git working tree before proceeding with publish preparation. + +## Bump command internals + +`BumpOptions` carries the dependency-injection points used by +`lading.commands.bump.run`. The `runner` field accepts an optional +`_CommandRunner`, matching the command-runner protocol used by publish +execution. When `runner` is `None`, bump falls back to the default subprocess +runner. Tests pass a runner explicitly so lockfile refresh commands can be +observed without invoking real Cargo processes. + +`BumpChanges` records the user-visible files touched by a bump run. Its +`lockfiles` field contains the git-tracked `Cargo.lock` files refreshed after +manifest rewrites. The output formatter treats these paths like manifests and +documentation files, listing each refreshed lockfile with a `(lockfile)` suffix +so operators can see which generated files need review and commit. + ## Publish command internals `PublishOptions.allow_unpublished_workspace_deps` is a dry-run-only override diff --git a/lading/exceptions.py b/lading/exceptions.py index b1a67c91..770ec400 100644 --- a/lading/exceptions.py +++ b/lading/exceptions.py @@ -1,4 +1,17 @@ -"""Package-level base exception for the lading toolkit.""" +"""Package-level base exception for the lading toolkit. + +`LadingError` is the common root for expected domain failures raised by +`lading` itself. Configuration, workspace metadata, lockfile, and publish +modules define their local root exceptions by inheriting from this class, so +callers can catch a single package-level type without also catching unrelated +Python runtime failures. + +Examples include `ConfigurationError`, `PublishPreflightError`, +`CargoMetadataError`, `WorkspaceModelError`, and `LockfileRefreshError`. +Feature-specific subclasses should continue to inherit from their local root +exception, preserving precise handling within each component while keeping a +consistent package-wide exception hierarchy. +""" from __future__ import annotations From 100bd3d2ad9d16cfd8c11013fe31cf2cb4997ba5 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 8 Jun 2026 18:27:49 +0200 Subject: [PATCH 21/24] Document lockfile test runner stub (#61) Add the missing docstring to the lockfile discovery runner closure so the project docstring coverage check stays above the required threshold. --- tests/unit/test_lockfile.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/test_lockfile.py b/tests/unit/test_lockfile.py index dfe19a86..046284d0 100644 --- a/tests/unit/test_lockfile.py +++ b/tests/unit/test_lockfile.py @@ -23,6 +23,7 @@ def runner( cwd: Path | None = None, env: cabc.Mapping[str, str] | None = None, ) -> tuple[int, str, str]: + """Stub runner returning a successful git result with empty stdout.""" assert command == ("git", "ls-files", "**/Cargo.lock", "Cargo.lock") assert cwd == tmp_path return 0, "", "" From 328c24851c7149212592dcb279aa4d22bc4fa3d8 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 8 Jun 2026 19:01:01 +0200 Subject: [PATCH 22/24] Extract shared command runner boundary (#61) Move the shared `_CommandRunner` protocol behind a neutral `_runners` module and point bump, publish, preflight, and lockfile typing at that boundary. Leave the concrete subprocess and cmd-mox implementation in `publish_execution`, with `_runners._invoke` delegating there at call time. Document the boundary in the developer guide so bump no longer appears to depend on publish-specific execution internals for lockfile refreshes. --- docs/developers-guide.md | 13 +++++++++++++ lading/commands/bump.py | 6 +++--- lading/commands/lockfile.py | 8 ++++---- lading/commands/publish.py | 1 + lading/commands/publish_execution.py | 2 +- lading/commands/publish_preflight.py | 2 +- 6 files changed, 23 insertions(+), 9 deletions(-) diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 4d1366ee..dd58e545 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -399,6 +399,19 @@ includes all four values. Using a single function for message construction keeps the error format consistent across the packaging and publish phases and makes snapshot testing straightforward. + +### Command runners (`lading/commands/_runners.py`) + +`lading.commands._runners` owns the shared `_CommandRunner` protocol and the +default `_invoke` adapter used by both bump and publish command workflows. +Keeping this small boundary module neutral prevents `lading bump` from +depending on publish-specific infrastructure just to run +`cargo generate-lockfile`. + +`lading.commands.publish_execution` still owns the concrete subprocess and +cmd-mox implementation. The neutral `_invoke` adapter delegates there at call +time, while command modules type against `_CommandRunner` from `_runners`. + ### Pre-flight validation (`publish_preflight`) `lading.commands.publish_preflight` performs workspace validation before diff --git a/lading/commands/bump.py b/lading/commands/bump.py index 2d32f8b8..15572195 100644 --- a/lading/commands/bump.py +++ b/lading/commands/bump.py @@ -11,7 +11,7 @@ from lading import config as config_module from lading.commands import bump_docs, bump_toml from lading.commands.lockfile import discover_tracked_lockfiles, refresh_lockfile -from lading.commands.publish_execution import _CommandRunner, _invoke +from lading.runtime import CommandRunner, subprocess_runner from lading.utils import normalise_workspace_root if typ.TYPE_CHECKING: @@ -48,7 +48,7 @@ class BumpOptions: default_factory=lambda: types.MappingProxyType({}) ) include_workspace_sections: bool = False - runner: _CommandRunner | None = None + runner: CommandRunner | None = None @dc.dataclass(frozen=True, slots=True) @@ -249,7 +249,7 @@ def _refresh_lockfiles(context: _BumpContext) -> tuple[Path, ...]: ``cargo generate-lockfile --manifest-path /Cargo.toml`` for each stale lockfile. """ - runner = context.base_options.runner or _invoke + runner = context.base_options.runner or subprocess_runner lockfiles = discover_tracked_lockfiles(context.root_path, runner) if context.base_options.dry_run: for lockfile_path in lockfiles: diff --git a/lading/commands/lockfile.py b/lading/commands/lockfile.py index 7a09d10e..bef04f59 100644 --- a/lading/commands/lockfile.py +++ b/lading/commands/lockfile.py @@ -36,7 +36,7 @@ from lading.exceptions import LadingError if typ.TYPE_CHECKING: - from lading.commands.publish_execution import _CommandRunner + from lading.runtime import CommandRunner LOGGER = logging.getLogger(__name__) _ManifestExists = cabc.Callable[[Path], bool] @@ -94,7 +94,7 @@ def _manifest_exists(manifest_path: Path) -> bool: def discover_tracked_lockfiles( workspace_root: Path, - runner: _CommandRunner, + runner: CommandRunner, *, manifest_exists: _ManifestExists = _manifest_exists, ) -> tuple[Path, ...]: @@ -144,7 +144,7 @@ def discover_tracked_lockfiles( def refresh_lockfile( manifest_path: Path, - runner: _CommandRunner, + runner: CommandRunner, ) -> Path: """Regenerate the lockfile for ``manifest_path`` and return its path. @@ -186,7 +186,7 @@ def refresh_lockfile( def validate_lockfile_freshness( manifest_path: Path, - runner: _CommandRunner, + runner: CommandRunner, ) -> bool: """Return whether Cargo accepts ``manifest_path`` under ``--locked``. diff --git a/lading/commands/publish.py b/lading/commands/publish.py index f30aa9ef..a917c1ca 100644 --- a/lading/commands/publish.py +++ b/lading/commands/publish.py @@ -55,6 +55,7 @@ from pathlib import Path from lading import config as config_module +from lading.commands import publish_preflight as _publish_preflight from lading.commands.publish_errors import PublishError, PublishPreflightError from lading.commands.publish_execution import ( _invoke, diff --git a/lading/commands/publish_execution.py b/lading/commands/publish_execution.py index 31a4703f..e1fc5394 100644 --- a/lading/commands/publish_execution.py +++ b/lading/commands/publish_execution.py @@ -7,10 +7,10 @@ from lading.commands.publish_errors import PublishPreflightError from lading.runtime import CommandSpawnError -from lading.runtime.subprocess_runner import split_command as _runtime_split_command from lading.runtime.subprocess_runner import ( subprocess_runner as _default_subprocess_runner, ) +from lading.runtime.subprocess_runner import split_command as _runtime_split_command def _publish_error(message: str) -> PublishPreflightError: diff --git a/lading/commands/publish_preflight.py b/lading/commands/publish_preflight.py index 0d42883c..4ce023b1 100644 --- a/lading/commands/publish_preflight.py +++ b/lading/commands/publish_preflight.py @@ -196,7 +196,7 @@ def _compose_preflight_arguments( def _validate_lockfile_freshness( workspace_root: Path, *, - runner: _CommandRunner, + runner: CommandRunner, env: cabc.Mapping[str, str] | None = None, ) -> None: """Fail early when tracked Cargo.lock files are stale.""" From 03efe7cd7fa5182dd2a21320fefc338430484757 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 8 Jun 2026 19:25:12 +0200 Subject: [PATCH 23/24] Add lockfile filtering property tests (#61) Add Hypothesis as a development dependency and cover the lockfile filtering helper with generated git stdout cases. Check that returned lockfiles never include `target` components, only pass through approved adjacent manifests, and always originate from non-empty git stdout lines. --- pyproject.toml | 1 + tests/unit/test_lockfile.py | 88 +++++++++++++++++++++++++++++++++++-- uv.lock | 23 ++++++++++ 3 files changed, 109 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 933d39fc..cb248dcc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -37,6 +37,7 @@ dev = [ "pyright", "pytest-timeout", "cmd-mox@git+https://github.com/leynos/cmd-mox@f583c279a15760aba5cfd9bddf1fbbe9b1f8c429", + "hypothesis>=6", "interrogate>=1.7.0", "syrupy>=5.1.0", ] diff --git a/tests/unit/test_lockfile.py b/tests/unit/test_lockfile.py index 046284d0..75af33cc 100644 --- a/tests/unit/test_lockfile.py +++ b/tests/unit/test_lockfile.py @@ -3,14 +3,38 @@ from __future__ import annotations import collections.abc as cabc -import typing as typ +import string +from pathlib import Path +import hypothesis.strategies as st import pytest +from hypothesis import given from lading.commands import lockfile -if typ.TYPE_CHECKING: - from pathlib import Path +# --------------------------------------------------------------------------- +# Hypothesis strategies for _lockfiles_with_manifests property tests +# --------------------------------------------------------------------------- + +_safe_component: st.SearchStrategy[str] = st.text( + alphabet=string.ascii_lowercase + string.digits + "_-", + min_size=1, + max_size=16, +).filter(lambda s: s != "target") + +_path_component: st.SearchStrategy[str] = st.one_of(st.just("target"), _safe_component) + +_lockfile_line: st.SearchStrategy[str] = st.lists( + _path_component, min_size=1, max_size=4 +).map(lambda parts: "/".join(parts) + "/Cargo.lock") + +_hypothesis_stdout: st.SearchStrategy[str] = st.lists( + st.one_of(_lockfile_line, st.just(""), st.just(" ")), + min_size=0, + max_size=20, +).map("\n".join) + +_HYPOTHESIS_WORKSPACE = Path("/repo") def test_discover_tracked_lockfiles_returns_empty_result(tmp_path: Path) -> None: @@ -210,3 +234,61 @@ def test_validate_lockfile_freshness_parametrized( "freshness result did not match cargo metadata exit code; " f"exit_code={exit_code}, expected {expected_bool}, got {actual}" ) + + +@given(stdout=_hypothesis_stdout) +def test_no_returned_path_contains_target_component(stdout: str) -> None: + """No returned lockfile path has 'target' as a relative path component.""" + result = lockfile._lockfiles_with_manifests( + stdout, + _HYPOTHESIS_WORKSPACE, + manifest_exists=lambda _: True, + ) + for path in result: + relative_parts = path.relative_to(_HYPOTHESIS_WORKSPACE).parts + assert "target" not in relative_parts, ( + f"Returned path {path} contains a 'target' component; " + f"relative parts: {relative_parts}" + ) + + +@given(stdout=_hypothesis_stdout) +def test_all_returned_paths_have_adjacent_manifest(stdout: str) -> None: + """Every returned path had manifest_exists approve its adjacent Cargo.toml.""" + approved: set[Path] = set() + + def manifest_exists(manifest_path: Path) -> bool: + """Approve candidates whose path hash is even.""" + approved_result = hash(manifest_path) % 2 == 0 + if approved_result: + approved.add(manifest_path) + return approved_result + + returned = lockfile._lockfiles_with_manifests( + stdout, + _HYPOTHESIS_WORKSPACE, + manifest_exists=manifest_exists, + ) + for path in returned: + expected_manifest = path.parent / "Cargo.toml" + assert expected_manifest in approved, ( + f"Returned path {path} was not approved by manifest_exists; " + f"adjacent manifest {expected_manifest} not in approved set" + ) + + +@given(stdout=_hypothesis_stdout) +def test_returned_paths_are_subset_of_git_stdout(stdout: str) -> None: + """Every returned path corresponds to a non-empty git stdout line.""" + tracked_lines = {line.strip() for line in stdout.splitlines() if line.strip()} + result = lockfile._lockfiles_with_manifests( + stdout, + _HYPOTHESIS_WORKSPACE, + manifest_exists=lambda _: True, + ) + for path in result: + relative = str(path.relative_to(_HYPOTHESIS_WORKSPACE)) + assert relative in tracked_lines, ( + f"Returned path {path} (relative: {relative!r}) " + f"does not appear in git stdout lines: {tracked_lines!r}" + ) diff --git a/uv.lock b/uv.lock index f2646a5c..037f0fd6 100644 --- a/uv.lock +++ b/uv.lock @@ -163,6 +163,18 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/f8/fc/b86c22ad3b18d8324a9d6fe5a3b55403291d2bf7572ba6a16efa5aa88059/gherkin_official-29.0.0-py3-none-any.whl", hash = "sha256:26967b0d537a302119066742669e0e8b663e632769330be675457ae993e1d1bc", size = 37085, upload-time = "2024-08-12T09:41:07.954Z" }, ] +[[package]] +name = "hypothesis" +version = "6.155.2" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "sortedcontainers" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/f5/04/64032a1dccd2233615c8a3f701bbb563558575ed017496a24b6d81762c91/hypothesis-6.155.2.tar.gz", hash = "sha256:ae36880287c9c5defe9f199d3d2b67d9947a4da2a46e6c57373cbdf2345b20e1", size = 477765, upload-time = "2026-06-05T16:32:23.63Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/ec/6e/e735f27ac1a530a4cd0a31cd970ec495a3a11830fdc5d281cc292593b330/hypothesis-6.155.2-py3-none-any.whl", hash = "sha256:c85ce6dcd630a90ce501f1d1dd1bc84b97f5649ca8a27e134c8cbf5aa480b1a5", size = 544213, upload-time = "2026-06-05T16:32:21.15Z" }, +] + [[package]] name = "iniconfig" version = "2.1.0" @@ -205,6 +217,7 @@ dependencies = [ dev = [ { name = "build" }, { name = "cmd-mox" }, + { name = "hypothesis" }, { name = "interrogate" }, { name = "pyright" }, { name = "pytest" }, @@ -229,6 +242,7 @@ requires-dist = [ dev = [ { name = "build" }, { name = "cmd-mox", git = "https://github.com/leynos/cmd-mox?rev=f583c279a15760aba5cfd9bddf1fbbe9b1f8c429" }, + { name = "hypothesis", specifier = ">=6" }, { name = "interrogate", specifier = ">=1.7.0" }, { name = "pyright" }, { name = "pytest" }, @@ -574,6 +588,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/b7/ce/149a00dd41f10bc29e5921b496af8b574d8413afcd5e30dfa0ed46c2cc5e/six-1.17.0-py2.py3-none-any.whl", hash = "sha256:4721f391ed90541fddacab5acf947aa0d3dc7d27b2e1e8eda2be8970586c3274", size = 11050, upload-time = "2024-12-04T17:35:26.475Z" }, ] +[[package]] +name = "sortedcontainers" +version = "2.4.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/e8/c4/ba2f8066cceb6f23394729afe52f3bf7adec04bf9ed2c820b39e19299111/sortedcontainers-2.4.0.tar.gz", hash = "sha256:25caa5a06cc30b6b83d11423433f65d1f9d76c4c6a0c90e3379eaa43b9bfdb88", size = 30594, upload-time = "2021-05-16T22:03:42.897Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/32/46/9cb0e58b2deb7f82b84065f37f3bffeb12413f947f9388e4cac22c4621ce/sortedcontainers-2.4.0-py2.py3-none-any.whl", hash = "sha256:a163dcaede0f1c021485e957a39245190e74249897e2ae4b2aa38595db237ee0", size = 29575, upload-time = "2021-05-16T22:03:41.177Z" }, +] + [[package]] name = "syrupy" version = "5.1.0" From 527c7a809e291a7cd54326196ecbccb51f44afad Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 8 Jun 2026 19:56:18 +0200 Subject: [PATCH 24/24] Resolve runner wiring after rebase (#61) Keep the issue 61 lockfile behaviour aligned with the runtime runner boundary now present on `origin/main`. Pass the selected CLI runner into `lading bump`, keep publish preflight runner wrappers compatible with the shared protocol, and update tests to use the current helper modules. Adjust BDD cmd-mox preflight registration so metadata, lockfile, package, and publish command stubs can coexist after the rebase. Update developer documentation to describe `lading.runtime.CommandRunner` instead of the removed command-local runner module. --- docs/developers-guide.md | 24 ++-- lading/cli.py | 1 + lading/commands/publish.py | 1 - lading/commands/publish_execution.py | 2 +- lading/commands/publish_preflight.py | 5 + lading/workspace/metadata.py | 3 +- tests/bdd/steps/metadata_fixtures.py | 2 +- tests/bdd/steps/test_bump_steps.py | 12 +- tests/bdd/steps/test_publish_given_steps.py | 3 + .../bdd/steps/test_publish_infrastructure.py | 104 ++++++++++++++++-- tests/unit/publish/test_command_helpers.py | 28 +++-- tests/unit/publish/test_preflight_checks.py | 4 +- .../test_preflight_lockfile_validation.py | 20 ++-- tests/unit/test_cargo_metadata_loading.py | 9 +- tests/unit/test_cli.py | 2 +- 15 files changed, 158 insertions(+), 62 deletions(-) diff --git a/docs/developers-guide.md b/docs/developers-guide.md index dd58e545..807ed9b9 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -88,7 +88,6 @@ publish runs with stub mode enabled. ## Workspace discovery helpers - ### Lockfile helpers (`lading/commands/lockfile.py`) `discover_tracked_lockfiles(workspace_root, runner)` filters git-tracked @@ -181,7 +180,6 @@ Examples: - `PublishOptions(allow_dirty=False)` — require a clean git working tree before proceeding with publish preparation. - ## Bump command internals `BumpOptions` carries the dependency-injection points used by @@ -206,7 +204,6 @@ yet. When enabled, `lading publish` downgrades that specific index-lookup failure to a warning and continues. The option is rejected at runtime when `live=True`, so it cannot mask a real upload failure. - ### Exception hierarchy (`lading.exceptions`) `lading.exceptions.LadingError` is the package-level base class for domain @@ -399,18 +396,17 @@ includes all four values. Using a single function for message construction keeps the error format consistent across the packaging and publish phases and makes snapshot testing straightforward. +### Command runners (`lading.runtime`) -### Command runners (`lading/commands/_runners.py`) - -`lading.commands._runners` owns the shared `_CommandRunner` protocol and the -default `_invoke` adapter used by both bump and publish command workflows. -Keeping this small boundary module neutral prevents `lading bump` from -depending on publish-specific infrastructure just to run -`cargo generate-lockfile`. +`lading.runtime` owns the shared `CommandRunner` protocol and the production +`subprocess_runner` adapter. Command modules type against this protocol so tests +can inject cmd-mox or recording runners without depending on publish-specific +infrastructure. -`lading.commands.publish_execution` still owns the concrete subprocess and -cmd-mox implementation. The neutral `_invoke` adapter delegates there at call -time, while command modules type against `_CommandRunner` from `_runners`. +`lading.commands.publish_execution` still owns publish-specific error mapping +around command execution. `lading bump` uses the runtime runner directly for +lockfile refreshes, while `lading publish` uses `_invoke` where failures should +surface as `PublishPreflightError`. ### Pre-flight validation (`publish_preflight`) @@ -423,7 +419,7 @@ _run_preflight_checks( *, allow_dirty: bool, configuration: LadingConfig, - runner: _CommandRunner | None = None, + runner: CommandRunner | None = None, ) -> None ``` diff --git a/lading/cli.py b/lading/cli.py index 36a8bc9f..6ccf266b 100644 --- a/lading/cli.py +++ b/lading/cli.py @@ -318,6 +318,7 @@ def bump( dry_run=dry_run, configuration=configuration, workspace=workspace, + runner=command_runner, ), ), ) diff --git a/lading/commands/publish.py b/lading/commands/publish.py index a917c1ca..f30aa9ef 100644 --- a/lading/commands/publish.py +++ b/lading/commands/publish.py @@ -55,7 +55,6 @@ from pathlib import Path from lading import config as config_module -from lading.commands import publish_preflight as _publish_preflight from lading.commands.publish_errors import PublishError, PublishPreflightError from lading.commands.publish_execution import ( _invoke, diff --git a/lading/commands/publish_execution.py b/lading/commands/publish_execution.py index e1fc5394..31a4703f 100644 --- a/lading/commands/publish_execution.py +++ b/lading/commands/publish_execution.py @@ -7,10 +7,10 @@ from lading.commands.publish_errors import PublishPreflightError from lading.runtime import CommandSpawnError +from lading.runtime.subprocess_runner import split_command as _runtime_split_command from lading.runtime.subprocess_runner import ( subprocess_runner as _default_subprocess_runner, ) -from lading.runtime.subprocess_runner import split_command as _runtime_split_command def _publish_error(message: str) -> PublishPreflightError: diff --git a/lading/commands/publish_preflight.py b/lading/commands/publish_preflight.py index 4ce023b1..9dde4198 100644 --- a/lading/commands/publish_preflight.py +++ b/lading/commands/publish_preflight.py @@ -193,6 +193,7 @@ def _compose_preflight_arguments( arguments.append(f"--target-dir={target_dir}") return tuple(arguments) + def _validate_lockfile_freshness( workspace_root: Path, *, @@ -207,8 +208,10 @@ def runner_with_env( *, cwd: Path | None = None, env: cabc.Mapping[str, str] | None = None, + echo_stdout: bool = True, ) -> tuple[int, str, str]: """Invoke ``runner`` with ``base_env`` applied when no env is supplied.""" + del echo_stdout effective_env = base_env if env is None else env return runner(command, cwd=cwd, env=effective_env) @@ -241,6 +244,8 @@ def runner_with_env( message = "\n".join(lines) LOGGER.error(message) raise PublishPreflightError(message) + + def _preflight_argument_sets( target_dir: Path, *, unit_tests_only: bool ) -> tuple[tuple[str, ...], tuple[str, ...]]: diff --git a/lading/workspace/metadata.py b/lading/workspace/metadata.py index b5eb2251..0389caeb 100644 --- a/lading/workspace/metadata.py +++ b/lading/workspace/metadata.py @@ -8,10 +8,10 @@ import json import typing as typ +from lading.exceptions import LadingError from lading.runtime import ( CommandRunner, CommandSpawnError, - LadingError, coerce_text, subprocess_runner, ) @@ -19,7 +19,6 @@ if typ.TYPE_CHECKING: # pragma: no cover - import-time typing aids only from pathlib import Path -from lading.exceptions import LadingError class CargoMetadataError(LadingError): diff --git a/tests/bdd/steps/metadata_fixtures.py b/tests/bdd/steps/metadata_fixtures.py index 5d5415d4..98189989 100644 --- a/tests/bdd/steps/metadata_fixtures.py +++ b/tests/bdd/steps/metadata_fixtures.py @@ -91,7 +91,7 @@ def _mock_cargo_metadata( "packages": list(packages), "workspace_members": list(member_ids), } - cmd_mox.mock("cargo").with_args("metadata", "--format-version", "1").returns( + cmd_mox.stub("cargo").with_args("metadata", "--format-version", "1").returns( exit_code=0, stdout=json.dumps(payload), stderr="", diff --git a/tests/bdd/steps/test_bump_steps.py b/tests/bdd/steps/test_bump_steps.py index 0bdc9675..287c5d96 100644 --- a/tests/bdd/steps/test_bump_steps.py +++ b/tests/bdd/steps/test_bump_steps.py @@ -30,12 +30,12 @@ def given_workspace_has_tracked_lockfiles( install_cargo_stub(cmd_mox, monkeypatch) (workspace_directory / "Cargo.lock").write_text("# root lock\n", encoding="utf-8") - cmd_mox.stub("git").with_args( - "ls-files", "**/Cargo.lock", "Cargo.lock" - ).returns(exit_code=0, stdout="Cargo.lock\n", stderr="") + cmd_mox.stub("git").with_args("ls-files", "**/Cargo.lock", "Cargo.lock").returns( + exit_code=0, stdout="Cargo.lock\n", stderr="" + ) cmd_mox.stub("cargo::generate-lockfile").with_args( "--manifest-path", str(workspace_directory / "Cargo.toml") - ).returns(exit_code=0, stdout="", stderr="") + ).returns(exit_code=0, stdout="cargo generate-lockfile\n", stderr="") @when( @@ -69,9 +69,7 @@ def when_invoke_lading_bump_dry_run( @then(parsers.parse('the bump command reports manifest updates for "{version}"')) -def then_command_reports_workspace( - cli_run: dict[str, typ.Any], version: str -) -> None: +def then_command_reports_workspace(cli_run: dict[str, typ.Any], version: str) -> None: """Assert that the bump command reports the updated manifests.""" assert cli_run["returncode"] == 0 stdout = cli_run["stdout"] diff --git a/tests/bdd/steps/test_publish_given_steps.py b/tests/bdd/steps/test_publish_given_steps.py index 133a10e4..d2ec9cea 100644 --- a/tests/bdd/steps/test_publish_given_steps.py +++ b/tests/bdd/steps/test_publish_given_steps.py @@ -66,6 +66,7 @@ def given_cargo_test_fails( exit_code=1, stderr="cargo test failed" ) + @given("publish pre-flight finds a stale tracked Cargo.lock") def given_publish_preflight_finds_stale_lockfile( workspace_directory: Path, @@ -88,6 +89,8 @@ def given_publish_preflight_finds_stale_lockfile( "because --locked was passed to prevent this" ), ) + + @given(parsers.parse('cargo test fails with compiletest artifact "{relative_path}"')) def given_cargo_test_fails_with_artifact( workspace_directory: Path, diff --git a/tests/bdd/steps/test_publish_infrastructure.py b/tests/bdd/steps/test_publish_infrastructure.py index 4d7c301c..979311e9 100644 --- a/tests/bdd/steps/test_publish_infrastructure.py +++ b/tests/bdd/steps/test_publish_infrastructure.py @@ -155,6 +155,72 @@ def _handler(invocation: _CmdInvocation) -> tuple[str, str, int]: return _handler +def _matches_expected_prefix( + expected: tuple[str, ...], + received: tuple[str, ...], +) -> bool: + """Return whether ``received`` begins with the expected argument tuple.""" + if len(received) < len(expected): + return False + return all( + expected_arg == received[index] for index, expected_arg in enumerate(expected) + ) + + +def _make_preflight_dispatch_handler( + entries: cabc.Sequence[tuple[tuple[str, ...], ResponseProvider]], + recorder: _PreflightInvocationRecorder | None, + label: str, +) -> cabc.Callable[[_CmdInvocation], tuple[str, str, int]]: + """Build a handler that dispatches several prefixes for one command.""" + + def _handler(invocation: _CmdInvocation) -> tuple[str, str, int]: + received = tuple(invocation.args) + for expected_arguments, response in entries: + if _matches_expected_prefix(expected_arguments, received): + active_response = ( + response(invocation) if callable(response) else response + ) + if recorder is not None: + env_mapping = dict(getattr(invocation, "env", {})) + recorder.record(label, received, env_mapping) + return ( + active_response.stdout, + active_response.stderr, + active_response.exit_code, + ) + expected = ", ".join(str(arguments) for arguments, _response in entries) + message = ( + f"Unexpected {label} invocation arguments: {received!r}; " + f"expected one of {expected}" + ) + raise AssertionError(message) + + return _handler + + +def _existing_static_stub_response( + cmd_mox: CmdMox, + program: str, +) -> tuple[tuple[str, ...], ResponseProvider] | None: + """Return an existing static stub response for ``program`` if present.""" + double = getattr(cmd_mox, "_doubles", {}).get(program) + if double is None or getattr(double, "kind", None) != "stub": + return None + response = getattr(double, "response", None) + if response is None or getattr(double, "handler", None) is not None: + return None + expected_arguments = tuple(getattr(double.expectation, "args", ())) + return ( + expected_arguments, + _CommandResponse( + exit_code=response.exit_code, + stdout=response.stdout, + stderr=response.stderr, + ), + ) + + def _create_stub_config( cmd_mox: CmdMox, preflight_overrides: dict[tuple[str, ...], ResponseProvider], @@ -171,10 +237,10 @@ def _create_stub_config( ) -def _register_preflight_commands( +def _normalise_preflight_responses( config: _PreflightStubConfig, -) -> None: - """Install cmd-mox doubles for publish pre-flight commands. +) -> dict[tuple[str, ...], ResponseProvider]: + """Return default and override responses keyed by command tuple. Notes ----- @@ -188,12 +254,6 @@ def _register_preflight_commands( ("git", "ls-files", "**/Cargo.lock", "Cargo.lock"): _CommandResponse( exit_code=0 ), - ( - "cargo", - "metadata", - "--locked", - "--manifest-path", - ): _CommandResponse(exit_code=0), ( "cargo", "check", @@ -243,6 +303,14 @@ def _register_preflight_commands( defaults |= normalized_overrides defaults[publish_command] = publish_response + return defaults + + +def _register_preflight_commands( + config: _PreflightStubConfig, +) -> None: + """Install cmd-mox doubles for publish pre-flight commands.""" + defaults = _normalise_preflight_responses(config) git_responses = { command[1:]: response for command, response in defaults.items() @@ -257,14 +325,26 @@ def _register_preflight_commands( config.cmd_mox.stub("git").runs( _make_git_handler(git_responses, config.recorder) ) + grouped_responses: dict[str, list[tuple[tuple[str, ...], ResponseProvider]]] = {} for command, response in defaults.items(): expectation_program, expectation_args = _resolve_preflight_expectation(command) + grouped_responses.setdefault(expectation_program, []).append(( + expectation_args, + response, + )) + for expectation_program, entries in grouped_responses.items(): + existing = _existing_static_stub_response(config.cmd_mox, expectation_program) + if existing is not None: + entries.insert(0, existing) config.cmd_mox.stub(expectation_program).runs( - _make_preflight_handler( - response, expectation_args, config.recorder, expectation_program + _make_preflight_dispatch_handler( + entries, + config.recorder, + expectation_program, ) ) + def _make_git_handler( responses: dict[tuple[str, ...], ResponseProvider], recorder: _PreflightInvocationRecorder | None, @@ -289,6 +369,8 @@ def _handler(invocation: _CmdInvocation) -> tuple[str, str, int]: ) return _handler + + def _build_env_restore_dict(var_name: str) -> dict[str, str]: """Build a dictionary for restoring an environment variable.""" previous = os.environ.get(var_name) diff --git a/tests/unit/publish/test_command_helpers.py b/tests/unit/publish/test_command_helpers.py index 8c3730fd..4b0107e2 100644 --- a/tests/unit/publish/test_command_helpers.py +++ b/tests/unit/publish/test_command_helpers.py @@ -1,14 +1,18 @@ """Unit tests for publish command execution helpers.""" -from pathlib import Path import importlib import io +from pathlib import Path +import pytest + +from lading import cli +from lading.commands import publish, publish_execution +from lading.runtime import subprocess_runner from lading.testing import cmd_mox_runner +from lading.testing.cmd_mox_runner import normalise_cmd_mox_command execution = importlib.import_module("lading.runtime.subprocess_runner") -from lading.commands import publish -import pytest def test_normalise_environment_handles_none_and_values() -> None: @@ -69,13 +73,15 @@ def test_echo_buffered_output_skips_empty_payloads() -> None: cmd_mox_runner._echo_buffered_output("", sink) assert sink.getvalue() == "" + def test_split_command_rejects_empty_sequence() -> None: """Splitting an empty command raises a descriptive error.""" with pytest.raises(publish.PublishPreflightError) as excinfo: - publish._split_command(()) + publish_execution.split_command(()) assert "Command sequence must contain" in str(excinfo.value) + @pytest.mark.parametrize( "command", [ @@ -90,9 +96,7 @@ def test_normalise_cmd_mox_command_forwards_non_cargo_commands( """cmd-mox normalisation preserves non-cargo commands and arguments.""" program, args = command[0], tuple(command[1:]) - rewritten_program, rewritten_args = publish._normalise_cmd_mox_command( - program, args - ) + rewritten_program, rewritten_args = normalise_cmd_mox_command(program, args) if program == "cargo" and args: expected_program = f"cargo::{args[0]}" @@ -104,19 +108,21 @@ def test_normalise_cmd_mox_command_forwards_non_cargo_commands( assert rewritten_program == expected_program assert rewritten_args == expected_args + @pytest.mark.parametrize("value", ["1", "true", "TRUE", "Yes", "on"]) def test_should_use_cmd_mox_stub_honours_truthy_values( value: str, monkeypatch: pytest.MonkeyPatch ) -> None: """Environment values recognised as truthy enable cmd-mox stubbing.""" - monkeypatch.setenv(publish.metadata_module.CMD_MOX_STUB_ENV_VAR, value) + monkeypatch.setenv(cli._CMD_MOX_STUB_ENV, value) + + assert cli._select_runner() is cmd_mox_runner.cmd_mox_runner - assert publish._should_use_cmd_mox_stub() is True def test_should_use_cmd_mox_stub_returns_false_by_default( monkeypatch: pytest.MonkeyPatch, ) -> None: """Missing environment values disable cmd-mox stubbing.""" - monkeypatch.delenv(publish.metadata_module.CMD_MOX_STUB_ENV_VAR, raising=False) + monkeypatch.delenv(cli._CMD_MOX_STUB_ENV, raising=False) - assert publish._should_use_cmd_mox_stub() is False + assert cli._select_runner() is subprocess_runner diff --git a/tests/unit/publish/test_preflight_checks.py b/tests/unit/publish/test_preflight_checks.py index 28d46d01..569bfaf3 100644 --- a/tests/unit/publish/test_preflight_checks.py +++ b/tests/unit/publish/test_preflight_checks.py @@ -9,10 +9,12 @@ import pytest from lading.commands import publish, publish_preflight -from lading.runtime import CommandRunner from .conftest import ORIGINAL_PREFLIGHT, make_config, make_preflight_config +if typ.TYPE_CHECKING: + from lading.runtime import CommandRunner + def test_preflight_checks_remove_all_targets_for_unit_only( monkeypatch: pytest.MonkeyPatch, tmp_path: Path diff --git a/tests/unit/publish/test_preflight_lockfile_validation.py b/tests/unit/publish/test_preflight_lockfile_validation.py index 39227e53..01832df5 100644 --- a/tests/unit/publish/test_preflight_lockfile_validation.py +++ b/tests/unit/publish/test_preflight_lockfile_validation.py @@ -8,7 +8,7 @@ import pytest -from lading.commands import publish +from lading.commands import publish, publish_preflight if typ.TYPE_CHECKING: from syrupy.assertion import SnapshotAssertion @@ -23,7 +23,7 @@ def test_validate_lockfile_freshness_passes_when_all_lockfiles_are_fresh( recorded_env: list[cabc.Mapping[str, str] | None] = [] monkeypatch.setattr( - publish._publish_preflight, + publish_preflight, "discover_tracked_lockfiles", lambda _root, _runner: (root_lockfile, nested_lockfile), ) @@ -37,7 +37,7 @@ def runner( recorded_env.append(env) return 0, "", "" - publish._validate_lockfile_freshness( + publish_preflight._validate_lockfile_freshness( tmp_path, runner=runner, env={"CARGO_TERM_COLOR": "never"}, @@ -54,12 +54,12 @@ def test_validate_lockfile_freshness_reports_stale_lockfiles( nested_lockfile = tmp_path / "tests" / "ui_lints" / "Cargo.lock" monkeypatch.setattr( - publish._publish_preflight, + publish_preflight, "discover_tracked_lockfiles", lambda _root, _runner: (root_lockfile, nested_lockfile), ) monkeypatch.setattr( - publish._publish_preflight, + publish_preflight, "validate_lockfile_freshness", lambda _manifest, _runner: False, ) @@ -73,7 +73,7 @@ def runner( return 0, "", "" with pytest.raises(publish.PublishPreflightError) as excinfo: - publish._validate_lockfile_freshness(tmp_path, runner=runner, env={}) + publish_preflight._validate_lockfile_freshness(tmp_path, runner=runner, env={}) message = str(excinfo.value) assert str(root_lockfile) in message @@ -98,12 +98,12 @@ def test_validate_lockfile_freshness_error_snapshot( nested_lockfile = workspace_root / "tests" / "ui_lints" / "Cargo.lock" monkeypatch.setattr( - publish._publish_preflight, + publish_preflight, "discover_tracked_lockfiles", lambda _root, _runner: (root_lockfile, nested_lockfile), ) monkeypatch.setattr( - publish._publish_preflight, + publish_preflight, "validate_lockfile_freshness", lambda _manifest, _runner: False, ) @@ -117,6 +117,8 @@ def runner( return 0, "", "" with pytest.raises(publish.PublishPreflightError) as excinfo: - publish._validate_lockfile_freshness(workspace_root, runner=runner, env={}) + publish_preflight._validate_lockfile_freshness( + workspace_root, runner=runner, env={} + ) assert str(excinfo.value) == snapshot() diff --git a/tests/unit/test_cargo_metadata_loading.py b/tests/unit/test_cargo_metadata_loading.py index 5ec1acda..64fcf554 100644 --- a/tests/unit/test_cargo_metadata_loading.py +++ b/tests/unit/test_cargo_metadata_loading.py @@ -9,7 +9,7 @@ import pytest -from lading.runtime import CommandSpawnError +from lading.runtime import CommandSpawnError, coerce_text from lading.workspace import ( CargoExecutableNotFoundError, CargoMetadataError, @@ -124,14 +124,17 @@ def test_load_cargo_metadata_error_decodes_byte_streams( assert "manifest missing" in str(excinfo.value) + def test_metadata_coerce_text_decodes_bytes() -> None: """Binary output is decoded using UTF-8 with replacement semantics.""" alpha = "\N{GREEK SMALL LETTER ALPHA}" encoded = alpha.encode() - assert metadata_module._coerce_text(encoded) == alpha + assert coerce_text(encoded) == alpha binary = b"foo\xff" - assert metadata_module._coerce_text(binary) == "foo\ufffd" + assert coerce_text(binary) == "foo\ufffd" + + @pytest.mark.parametrize( "scenario", [ diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index 20cd90e0..2cd6a615 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -390,7 +390,7 @@ def fake_run(*args: object, **kwargs: object) -> str: options = captured_kwargs["options"] assert isinstance(options, bump_command.BumpOptions) assert options.dry_run is True - assert options.runner is None + assert options.runner is cli.subprocess_runner @pytest.mark.parametrize(