From 40ef293fba126850b5bb9d39d2256827cce71a36 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 1 Jun 2026 23:20:48 +0200 Subject: [PATCH 1/2] 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 | 12 +- lading/commands/publish_preflight.py | 53 ++++++ tests/bdd/features/cli.feature | 32 ++++ tests/bdd/steps/test_bump_steps.py | 46 ++++- tests/bdd/steps/test_publish_given_steps.py | 24 +++ .../bdd/steps/test_publish_infrastructure.py | 46 +++++ tests/unit/publish/test_preflight_checks.py | 79 +++++++++ tests/unit/test_bump_command_integration.py | 85 ++++++++++ tests/unit/test_lockfile.py | 157 ++++++++++++++++++ 11 files changed, 665 insertions(+), 10 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 e271c5d7..f88cae41 100644 --- a/lading/commands/publish.py +++ b/lading/commands/publish.py @@ -5,8 +5,6 @@ plans publication order, stages the workspace, and dispatches to one of two **publish pipelines** depending on the ``live`` flag. -**Pipeline dispatch** - *Dry-run* (``live=False``) keeps the historical batched two-phase pipeline: package every publishable crate, then ``cargo publish --dry-run`` every crate in plan order. @@ -16,8 +14,6 @@ then advance. This ordering lets dependent crates resolve newly uploaded in-plan dependencies during a single release train. -**Per-crate helpers** - :func:`_package_crate` and :func:`_publish_crate` each invoke ``cargo`` in the correct staged directory for a single crate. Both detect index-missing-version failures (:func:`_is_index_missing_version_error`) and @@ -33,8 +29,6 @@ failures through one catch boundary or distinguish the publish phase when needed. -**Related modules** - * :mod:`lading.commands.publish_plan` — plan construction and formatting * :mod:`lading.commands.publish_preflight` — ``cargo check`` / ``cargo test`` / git-status guards @@ -107,6 +101,7 @@ _normalise_test_excludes = _publish_preflight._normalise_test_excludes _run_aux_build_commands = _publish_preflight._run_aux_build_commands _run_cargo_preflight = _publish_preflight._run_cargo_preflight +_validate_lockfile_freshness = _publish_preflight._validate_lockfile_freshness _verify_clean_working_tree = _publish_preflight._verify_clean_working_tree LOGGER = logging.getLogger(__name__) @@ -753,6 +748,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 2b998ea4..6152b4cb 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 _CommandRunner, _invoke @@ -133,6 +137,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,6 +188,50 @@ def _compose_preflight_arguments( 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 79383532..09c20ffd 100644 --- a/tests/bdd/steps/test_publish_given_steps.py +++ b/tests/bdd/steps/test_publish_given_steps.py @@ -69,6 +69,30 @@ def given_cargo_test_fails( ) +@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 d298dc46..d0d84da6 100644 --- a/tests/bdd/steps/test_publish_infrastructure.py +++ b/tests/bdd/steps/test_publish_infrastructure.py @@ -186,6 +186,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", @@ -235,6 +241,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( @@ -244,6 +264,32 @@ 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 2f6802ab..53e38327 100644 --- a/tests/unit/publish/test_preflight_checks.py +++ b/tests/unit/publish/test_preflight_checks.py @@ -368,6 +368,85 @@ 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 2d2d70b03ccba4352bdcabd43315a2089d8d48f7 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Fri, 5 Jun 2026 20:37:16 +0000 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=93=9D=20CodeRabbit=20Chat:=20Impleme?= =?UTF-8?q?nt=20requested=20code=20changes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lading/commands/lockfile.py | 61 +++++++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 23 deletions(-) diff --git a/lading/commands/lockfile.py b/lading/commands/lockfile.py index ba88a64a..6f525019 100644 --- a/lading/commands/lockfile.py +++ b/lading/commands/lockfile.py @@ -16,52 +16,67 @@ class LockfileRefreshError(RuntimeError): """Raised when Cargo cannot regenerate a lockfile.""" -def discover_tracked_lockfiles( +def _query_git_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 () - +) -> str | None: + """Run ``git ls-files`` and 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, - ) - return () + if exit_code == 0: + 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 () + return None + +def _filter_lockfiles_with_manifests( + workspace_root: Path, + stdout: str, +) -> tuple[Path, ...]: + """Return lockfile paths from ``stdout`` that have 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.""" + has_lockfiles = any( + "target" not in path.relative_to(workspace_root).parts + for path in workspace_root.rglob("Cargo.lock") + ) + if not has_lockfiles: + return () + stdout = _query_git_tracked_lockfiles(workspace_root, runner) + if stdout is None: + return () + return _filter_lockfiles_with_manifests(workspace_root, stdout) + + 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)),