diff --git a/docs/developers-guide.md b/docs/developers-guide.md index ffa2eff6..ec07c30e 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -210,11 +210,18 @@ only the workspace-root lockfile is processed, its lone failure is re-raised as the original cargo error rather than wrapped in the aggregate message. `BumpOptions` carries the dependency-injection points used by -`lading.commands.bump.run`. The `command_runner` field accepts an optional -`CommandRunner`, matching the command-runner protocol used by publish -execution. When `command_runner` is `None`, bump falls back to the default -subprocess runner. Tests pass a runner explicitly so lockfile commands can be -observed without invoking real Cargo processes. +`lading.commands.bump.run`. Lockfile operations are reached through the +`lockfile_repository` field, a `bump_lockfiles.LockfileRepository` port +introduced by issue #82: the bump domain never holds a raw command runner. When +the field is `None`, bump uses `bump_lockfiles.CargoLockfileRepository`, the +cargo-backed adapter bound to the default subprocess runner; the CLI binds the +adapter to its selected runner. Tests inject a repository (or bind the adapter +to a recording runner) so lockfile commands can be observed without invoking +real Cargo processes. The port's scope is bump-side lockfile projection and +regeneration; publish-side discovery and validation go through the sibling +`lockfile.LockfileInspectionRepository` port (see the Lockfile helpers section +below), so neither the bump nor the publish lockfile domain holds a raw +`CommandRunner` (issue #82). Bump-time crate-set derivation is centralized in the bump context: the `excluded` and `updated_crate_names` sets are computed exactly once in @@ -330,8 +337,22 @@ manifest rewrites, while publish only probes freshness read-only via `cargo metadata --locked --manifest-path ... --format-version=1`. It returns a `LockfileFreshness` result that distinguishes fresh lockfiles, lockfiles that Cargo says need updating under `--locked`, and unrelated Cargo failures. -`_validate_lockfile_freshness` in `publish_preflight.py` calls it before the -cargo check/test pre-flight. + +The publish pre-flight domain reaches both operations through the +`LockfileInspectionRepository` port (issue #82) rather than holding a command +runner: `_validate_lockfile_freshness` and `_collect_stale_lockfiles` in +`publish_preflight.py` depend only on the port, so VCS (git), filesystem, and +cargo execution concerns stay out of the freshness-classification logic. +`CargoLockfileInspectionRepository` is the git- and cargo-backed adapter; it +binds a `CommandRunner` and the optional pre-flight base environment, applying +that environment to any invocation that does not supply its own. +`publish_preflight._run_preflight_checks` is the composition root: it binds the +adapter to the selected command runner and pre-flight environment. Tests inject +a port double at the `_validate_lockfile_freshness` seam, observing discovery +and validation without invoking real git or cargo. This is the publish-side +counterpart to the bump-side `bump_lockfiles.LockfileRepository`; together they +complete issue #82's separation of lockfile VCS/filesystem concerns from the +command domain. `LockfileDiscoveryError` inherits `LadingError`; its messages include the git failure detail. diff --git a/docs/lading-design.md b/docs/lading-design.md index 182afeac..419bf8e0 100644 --- a/docs/lading-design.md +++ b/docs/lading-design.md @@ -376,6 +376,19 @@ lading bump [--dry-run] documentation counts, and documentation entries are suffixed with `(documentation)` for clarity. +### Lockfile repository port (bump side) + +The bump domain reaches Cargo lockfile projection and regeneration through +a `LockfileRepository` port defined in +`lading.commands.bump_lockfiles`. `CargoLockfileRepository` is the +cargo-backed adapter; it is constructed with an optional `CommandRunner` +and delegates to the module-level helpers. +`BumpOptions.lockfile_repository` is the injection point; when `None`, +bump substitutes `CargoLockfileRepository` bound to the default +subprocess runner, and the CLI binds the adapter at the composition +root. This keeps the bump domain free of a raw `CommandRunner` +(issue #82). + ## 4. `publish` Subcommand Design The `publish` command orchestrates the publication of crates to the designated @@ -477,6 +490,22 @@ names are listed before returning the user-specified order. compiletest `*.stderr` files when cargo test fails, exposing the debug diff directly in the CLI output. +### Lockfile inspection repository port (publish side) + +The publish pre-flight domain reaches tracked-lockfile discovery and +freshness validation through a `LockfileInspectionRepository` port +defined in `lading.commands.lockfile`. +`CargoLockfileInspectionRepository` is the git- and cargo-backed +adapter; it binds a `CommandRunner` and the optional pre-flight base +environment, applying that environment to invocations that do not +supply their own. `publish_preflight._run_preflight_checks` is the +composition root: it constructs the adapter and passes it to the +domain helpers `_collect_stale_lockfiles` and +`_validate_lockfile_freshness`, which depend only on the port. +Together with the bump-side `LockfileRepository`, the two ports keep +VCS, filesystem, and cargo execution concerns out of the lockfile +domain logic (issue #82). + ### Publish Preflight Sequence The preflight sequence diagram illustrates the pre-flight checks that run diff --git a/lading/cli.py b/lading/cli.py index 98398d1b..a1017258 100644 --- a/lading/cli.py +++ b/lading/cli.py @@ -341,7 +341,9 @@ def bump( rebuild_lockfiles=rebuild_lockfiles, configuration=configuration, workspace=workspace, - command_runner=command_runner, + lockfile_repository=commands.bump_lockfiles.CargoLockfileRepository( + runner=command_runner + ), ), ), ) diff --git a/lading/commands/__init__.py b/lading/commands/__init__.py index 31c21733..ae11b807 100644 --- a/lading/commands/__init__.py +++ b/lading/commands/__init__.py @@ -2,6 +2,6 @@ from __future__ import annotations -from . import bump, publish +from . import bump, bump_lockfiles, publish -__all__ = ["bump", "publish"] +__all__ = ["bump", "bump_lockfiles", "publish"] diff --git a/lading/commands/bump.py b/lading/commands/bump.py index 95800a95..eed78ef6 100644 --- a/lading/commands/bump.py +++ b/lading/commands/bump.py @@ -61,7 +61,6 @@ from lading.commands.bump_manifests import _BumpContext from lading.config import LadingConfig - from lading.runtime import CommandRunner from lading.workspace import WorkspaceCrate, WorkspaceGraph LOGGER = logging.getLogger(__name__) @@ -88,9 +87,11 @@ class BumpOptions: workspace : WorkspaceGraph | None, default None Loaded workspace graph. Programmatic callers may omit it only when they want ``run`` to inspect the workspace. - command_runner : CommandRunner | None, default None - Callable with the runtime command-runner interface. It is used for - lockfile rebuild commands; ``None`` uses the default subprocess runner. + lockfile_repository : bump_lockfiles.LockfileRepository | None, default None + Port used for lockfile projection and regeneration. ``None`` selects + the cargo-backed adapter with the default subprocess runner. Tests + inject a repository so lockfile behaviour can be observed without + invoking real Cargo processes. dependency_sections : Mapping[str, Collection[str]] Explicit dependency sections to rewrite by crate name. include_workspace_sections : bool, default False @@ -106,7 +107,7 @@ class BumpOptions: rebuild_lockfiles: bool | None = None configuration: LadingConfig | None = None workspace: WorkspaceGraph | None = None - command_runner: CommandRunner | None = None + lockfile_repository: bump_lockfiles.LockfileRepository | None = None dependency_sections: cabc.Mapping[str, cabc.Collection[str]] = dc.field( default_factory=lambda: types.MappingProxyType({}) ) @@ -192,7 +193,7 @@ def _initialize_bump_context( rebuild_lockfiles=rebuild_lockfiles, configuration=configuration, workspace=workspace, - command_runner=resolved_options.command_runner, + lockfile_repository=resolved_options.lockfile_repository, dependency_sections=resolved_options.dependency_sections, include_workspace_sections=resolved_options.include_workspace_sections, ) @@ -319,15 +320,13 @@ def _process_lockfiles( if context.base_options.rebuild_lockfiles is not True or not changed_manifests: return () lockfile_manifests = context.configuration.bump.lockfile_manifests - if context.base_options.dry_run: - return bump_lockfiles.resolve_lockfile_paths( - context.root_path, lockfile_manifests - ) - return bump_lockfiles.regenerate_lockfiles( - context.root_path, - lockfile_manifests, - runner=context.base_options.command_runner, + repository = ( + context.base_options.lockfile_repository + or bump_lockfiles.CargoLockfileRepository() ) + if context.base_options.dry_run: + return repository.resolve_lockfile_paths(context.root_path, lockfile_manifests) + return repository.regenerate_lockfiles(context.root_path, lockfile_manifests) def _prepare_sorted_changes( diff --git a/lading/commands/bump_lockfiles.py b/lading/commands/bump_lockfiles.py index 7109ae97..7f518df9 100644 --- a/lading/commands/bump_lockfiles.py +++ b/lading/commands/bump_lockfiles.py @@ -237,3 +237,149 @@ def _run_workspace_lockfile_update( stderr, ) raise LockfileRegenerationError(message) + + +@dc.dataclass(frozen=True, slots=True) +class CargoLockfileRepository: + """Cargo-backed :class:`LockfileRepository` bound to a command runner.""" + + runner: CommandRunner | None = None + + def resolve_lockfile_paths( + self, + workspace_root: Path, + lockfile_manifests: cabc.Sequence[str], + ) -> tuple[Path, ...]: + """Return the lockfile paths a regeneration run would touch. + + Parameters + ---------- + workspace_root : + Absolute path to the Cargo workspace root. + lockfile_manifests : + Configured manifest paths relative to *workspace_root*. The + workspace-root ``Cargo.toml`` is always prepended and + de-duplicated. + + Returns + ------- + tuple[Path, ...] + The lockfile paths that a regeneration run would touch, + in manifest execution order. + + Raises + ------ + LockfileRegenerationError + If any configured manifest path is invalid (outside the + workspace or not named ``Cargo.toml``), propagated from + :func:`_resolve_manifest_paths`. + """ + return resolve_lockfile_paths(workspace_root, lockfile_manifests) + + def regenerate_lockfiles( + self, + workspace_root: Path, + lockfile_manifests: cabc.Sequence[str], + ) -> tuple[Path, ...]: + """Regenerate lockfiles via ``cargo update --workspace``. + + Parameters + ---------- + workspace_root : + Absolute path to the Cargo workspace root. + lockfile_manifests : + Configured manifest paths relative to *workspace_root*. The + workspace-root ``Cargo.toml`` is always prepended and + de-duplicated. + + Returns + ------- + tuple[Path, ...] + Paths to every ``Cargo.lock`` regenerated, in manifest + execution order. + + Raises + ------ + LockfileRegenerationError + If any configured manifest path is invalid (outside the + workspace or not named ``Cargo.toml``), or — after every + manifest has been attempted — if ``cargo update --workspace`` + failed. A lone workspace-root failure re-raises the original + cargo error unchanged; multiple failures raise one aggregated + error listing each failed manifest with a repair command. + """ + return regenerate_lockfiles( + workspace_root, lockfile_manifests, runner=self.runner + ) + + +class LockfileRepository(typ.Protocol): + """Port for projecting and regenerating Cargo lockfiles after a bump. + + The bump domain depends on this protocol rather than on a command + runner, keeping execution infrastructure out of the public bump options + (issue #82). + """ + + def resolve_lockfile_paths( + self, + workspace_root: Path, + lockfile_manifests: cabc.Sequence[str], + ) -> tuple[Path, ...]: + """Return the lockfile paths a regeneration run would touch. + + Parameters + ---------- + workspace_root : + Absolute path to the Cargo workspace root. + lockfile_manifests : + Configured manifest paths relative to *workspace_root*. The + workspace-root ``Cargo.toml`` is always prepended and + de-duplicated. + + Returns + ------- + tuple[Path, ...] + The lockfile paths that a regeneration run would touch, + in manifest execution order. + + Raises + ------ + LockfileRegenerationError + If any configured manifest path is invalid (outside the + workspace or not named ``Cargo.toml``), propagated from + :func:`_resolve_manifest_paths`. + """ + + def regenerate_lockfiles( + self, + workspace_root: Path, + lockfile_manifests: cabc.Sequence[str], + ) -> tuple[Path, ...]: + """Regenerate lockfiles and return the rewritten paths. + + Parameters + ---------- + workspace_root : + Absolute path to the Cargo workspace root. + lockfile_manifests : + Configured manifest paths relative to *workspace_root*. The + workspace-root ``Cargo.toml`` is always prepended and + de-duplicated. + + Returns + ------- + tuple[Path, ...] + Paths to every ``Cargo.lock`` regenerated, in manifest + execution order. + + Raises + ------ + LockfileRegenerationError + If any configured manifest path is invalid (outside the + workspace or not named ``Cargo.toml``), or — after every + manifest has been attempted — if ``cargo update --workspace`` + failed. A lone workspace-root failure re-raises the original + cargo error unchanged; multiple failures raise one aggregated + error listing each failed manifest with a repair command. + """ diff --git a/lading/commands/lockfile.py b/lading/commands/lockfile.py index 9a2e5c0d..2b309018 100644 --- a/lading/commands/lockfile.py +++ b/lading/commands/lockfile.py @@ -19,12 +19,19 @@ refreshed in place after manifest rewrites, whereas validation here uses ``cargo metadata --locked`` purely as a read-only freshness probe. +The publish pre-flight domain reaches these operations through the +:class:`LockfileInspectionRepository` port (issue #82) rather than holding a +raw command runner. :class:`CargoLockfileInspectionRepository` is the +git- and cargo-backed adapter, bound to a runner (and optional environment +overrides) at the pre-flight composition root. + Typical publish-side usage: ```python -lockfiles = discover_tracked_lockfiles(workspace_root, runner) +repository = CargoLockfileInspectionRepository(runner=runner) +lockfiles = repository.discover_tracked_lockfiles(workspace_root) for lockfile_path in lockfiles: - validate_lockfile_freshness(lockfile_path.parent / "Cargo.toml", runner) + repository.validate_lockfile_freshness(lockfile_path.parent / "Cargo.toml") ``` """ @@ -226,3 +233,139 @@ def _is_lockfile_stale_detail(detail: str) -> bool: "needs to be updated" in normalized or "cannot update the lock file" in normalized ) + + +@dc.dataclass(frozen=True, slots=True) +class CargoLockfileInspectionRepository: + """Git- and cargo-backed adapter for publish-side lockfile inspection. + + Binds a :class:`~lading.runtime.CommandRunner` (and optional environment + overrides) so the publish pre-flight domain can discover tracked lockfiles + and probe their freshness without holding a raw command runner (issue #82). + The adapter applies ``env`` to any invocation that does not supply its own, + matching the behaviour the pre-flight base environment previously wired in + through an inline runner wrapper. + + Attributes + ---------- + runner : CommandRunner + Command runner used to execute the git discovery and cargo freshness + probes. + env : Mapping[str, str] | None, default None + Environment overrides applied to any invocation that does not supply + its own; ``None`` leaves each call's environment untouched. + manifest_exists : Callable[[Path], bool], default _manifest_exists + Predicate deciding whether a discovered lockfile has an adjacent + ``Cargo.toml`` manifest; the default checks the filesystem. + """ + + runner: CommandRunner + env: cabc.Mapping[str, str] | None = None + manifest_exists: _ManifestExists = _manifest_exists + + def discover_tracked_lockfiles(self, workspace_root: Path) -> tuple[Path, ...]: + """Return tracked Cargo.lock files with adjacent manifests. + + Parameters + ---------- + workspace_root + Path to the repository root searched for tracked lockfiles. + + Returns + ------- + tuple[Path, ...] + Git-tracked ``Cargo.lock`` files outside any ``target`` directory + that have an adjacent ``Cargo.toml`` manifest. + + """ + return discover_tracked_lockfiles( + workspace_root, + self._bound_runner(), + manifest_exists=self.manifest_exists, + ) + + def validate_lockfile_freshness(self, manifest_path: Path) -> LockfileFreshness: + """Return Cargo's locked-mode freshness result for ``manifest_path``. + + Parameters + ---------- + manifest_path + Path to the Cargo manifest to validate under ``--locked``. + + Returns + ------- + LockfileFreshness + Structured result describing whether the lockfile is fresh, stale + (Cargo says it needs updating under ``--locked``), or failed for + another reason. + + """ + return validate_lockfile_freshness(manifest_path, self._bound_runner()) + + def _bound_runner(self) -> CommandRunner: + """Return ``runner`` with ``env`` applied when a call omits its own.""" + if self.env is None: + return self.runner + base_env = self.env + base_runner = self.runner + + def runner_with_env( + command: cabc.Sequence[str], + *, + cwd: Path | None = None, + env: cabc.Mapping[str, str] | None = None, + **runner_kwargs: bool, + ) -> tuple[int, str, str]: + """Invoke ``base_runner`` with ``base_env`` as the default env. + + Any extra keyword (notably ``echo_stdout``) is forwarded to + ``base_runner`` unchanged; only ``env`` is defaulted (to the bound + ``base_env``) when a call omits it. + """ + effective_env = base_env if env is None else env + return base_runner(command, cwd=cwd, env=effective_env, **runner_kwargs) + + return runner_with_env + + +class LockfileInspectionRepository(typ.Protocol): + """Port for discovering tracked lockfiles and probing their freshness. + + The publish pre-flight domain depends on this protocol rather than on a + command runner, keeping VCS, filesystem, and cargo execution concerns out + of the freshness-classification logic (issue #82). This is the publish-side + counterpart to :class:`lading.commands.bump_lockfiles.LockfileRepository`, + which owns bump-side lockfile projection and regeneration. + """ + + def discover_tracked_lockfiles(self, workspace_root: Path) -> tuple[Path, ...]: + """Return tracked Cargo.lock files with adjacent manifests. + + Parameters + ---------- + workspace_root + Path to the repository root searched for tracked lockfiles. + + Returns + ------- + tuple[Path, ...] + Git-tracked ``Cargo.lock`` files outside any ``target`` directory + that have an adjacent ``Cargo.toml`` manifest. + + """ + + def validate_lockfile_freshness(self, manifest_path: Path) -> LockfileFreshness: + """Return the freshness result for ``manifest_path``. + + Parameters + ---------- + manifest_path + Path to the Cargo manifest to validate. + + Returns + ------- + LockfileFreshness + The freshness result, distinguishing fresh, stale, and failed + states. + + """ diff --git a/lading/commands/publish_preflight.py b/lading/commands/publish_preflight.py index d70d4619..f7ea5334 100644 --- a/lading/commands/publish_preflight.py +++ b/lading/commands/publish_preflight.py @@ -33,16 +33,14 @@ import typing as typ from pathlib import Path -from lading.commands.lockfile import ( - discover_tracked_lockfiles, - validate_lockfile_freshness, -) +from lading.commands.lockfile import CargoLockfileInspectionRepository from lading.commands.publish_diagnostics import _append_compiletest_diagnostics from lading.commands.publish_errors import PublishPreflightError from lading.commands.publish_execution import _invoke from lading.utils.process import append_detail, command_detail, with_detail if typ.TYPE_CHECKING: + from lading.commands.lockfile import LockfileInspectionRepository from lading.config import CompiletestExtern, LadingConfig from lading.runtime import CommandRunner @@ -129,7 +127,14 @@ def _run_preflight_checks( configuration: LadingConfig, runner: CommandRunner | None = None, ) -> None: - """Execute publish pre-flight checks for ``workspace_root``.""" + """Execute publish pre-flight checks for ``workspace_root``. + + This is the composition root for lockfile inspection: it binds + :class:`CargoLockfileInspectionRepository` to the selected command runner + and the pre-flight base environment (issue #82), so the freshness domain + step runs through the port without holding a raw runner. Tests inject a + port double at the :func:`_validate_lockfile_freshness` seam instead. + """ command_runner = runner or _invoke preflight_config = configuration.preflight base_env = _build_preflight_environment(preflight_config.env_overrides) @@ -145,11 +150,11 @@ def _run_preflight_checks( runner=command_runner, env=base_env, ) - _validate_lockfile_freshness( - workspace_root, + repository = CargoLockfileInspectionRepository( runner=command_runner, env=base_env, ) + _validate_lockfile_freshness(workspace_root, repository=repository) with tempfile.TemporaryDirectory(prefix="lading-preflight-target-") as target: target_path = Path(target) @@ -198,7 +203,7 @@ def _compose_preflight_arguments( def _collect_stale_lockfiles( tracked: cabc.Iterable[Path], - runner: CommandRunner, + repository: LockfileInspectionRepository, ) -> list[Path]: """Classify tracked lockfiles; raise immediately on error, return stale paths. @@ -210,7 +215,7 @@ def _collect_stale_lockfiles( stale: list[Path] = [] for lockfile_path in tracked: manifest_path = lockfile_path.parent / "Cargo.toml" - freshness = validate_lockfile_freshness(manifest_path, runner) + freshness = repository.validate_lockfile_freshness(manifest_path) if freshness.is_fresh: continue if freshness.is_stale: @@ -245,26 +250,17 @@ def _build_stale_lockfile_message(stale_lockfiles: list[Path]) -> str: def _validate_lockfile_freshness( workspace_root: Path, *, - runner: CommandRunner, - env: cabc.Mapping[str, str] | None = None, + repository: LockfileInspectionRepository, ) -> 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, - 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) - - tracked = discover_tracked_lockfiles(workspace_root, runner_with_env) - stale_lockfiles = _collect_stale_lockfiles(tracked, runner_with_env) + """Fail early when tracked Cargo.lock files are stale. + + Discovery and freshness probing run through ``repository`` (the + :class:`LockfileInspectionRepository` port), so this domain step never + holds a command runner or knows how lockfiles are located and validated + (issue #82). + """ + tracked = repository.discover_tracked_lockfiles(workspace_root) + stale_lockfiles = _collect_stale_lockfiles(tracked, repository) if not stale_lockfiles: LOGGER.info("All %d tracked lockfile(s) are fresh under --locked", len(tracked)) diff --git a/tests/unit/publish/test_preflight_lockfile_validation.py b/tests/unit/publish/test_preflight_lockfile_validation.py index 5c106f2e..e7e3ff53 100644 --- a/tests/unit/publish/test_preflight_lockfile_validation.py +++ b/tests/unit/publish/test_preflight_lockfile_validation.py @@ -1,8 +1,15 @@ -"""Unit tests for _validate_lockfile_freshness pre-flight helper.""" +"""Unit tests for the _validate_lockfile_freshness pre-flight helper. + +These exercise the pre-flight freshness domain step through the +:class:`lading.commands.lockfile.LockfileInspectionRepository` port (issue +#82): tests inject a recording repository double instead of a command runner, +so discovery, classification, and remediation messaging are verified without +touching git or cargo. +""" from __future__ import annotations -import collections.abc as cabc +import dataclasses as dc import typing as typ from pathlib import Path @@ -11,78 +18,74 @@ from lading.commands import lockfile, publish, publish_preflight if typ.TYPE_CHECKING: + import collections.abc as cabc + from syrupy.assertion import SnapshotAssertion +@dc.dataclass +class _RecordingLockfileRepository: + """In-memory ``LockfileInspectionRepository`` double for pre-flight tests.""" + + tracked: tuple[Path, ...] + freshness: cabc.Mapping[Path, lockfile.LockfileFreshness] | None = None + default_freshness: lockfile.LockfileFreshness = dc.field( + default_factory=lambda: lockfile.LockfileFreshness(is_fresh=True) + ) + discovered_roots: list[Path] = dc.field(default_factory=list) + validated_manifests: list[Path] = dc.field(default_factory=list) + + def discover_tracked_lockfiles(self, workspace_root: Path) -> tuple[Path, ...]: + """Record the discovery call and return the configured lockfiles.""" + self.discovered_roots.append(workspace_root) + return self.tracked + + def validate_lockfile_freshness( + self, manifest_path: Path + ) -> lockfile.LockfileFreshness: + """Record the validation call and return the configured freshness.""" + self.validated_manifests.append(manifest_path) + if self.freshness is not None and manifest_path in self.freshness: + return self.freshness[manifest_path] + return self.default_freshness + + +_STALE_DETAIL = "the lock file Cargo.lock needs to be updated but --locked was passed" + + def test_validate_lockfile_freshness_passes_when_all_lockfiles_are_fresh( - monkeypatch: pytest.MonkeyPatch, tmp_path: Path + tmp_path: Path, ) -> None: - """Fresh tracked lockfiles allow preflight to continue.""" + """Fresh tracked lockfiles allow preflight to continue via the port.""" 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_preflight, - "discover_tracked_lockfiles", - lambda _root, _runner: (root_lockfile, nested_lockfile), - ) + repository = _RecordingLockfileRepository(tracked=(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_preflight._validate_lockfile_freshness( - tmp_path, - runner=runner, - env={"CARGO_TERM_COLOR": "never"}, - ) + publish_preflight._validate_lockfile_freshness(tmp_path, repository=repository) - assert recorded_env == [{"CARGO_TERM_COLOR": "never"}] * 2 + assert repository.discovered_roots == [tmp_path] + assert repository.validated_manifests == [ + root_lockfile.parent / "Cargo.toml", + nested_lockfile.parent / "Cargo.toml", + ] -def test_validate_lockfile_freshness_reports_stale_lockfiles( - monkeypatch: pytest.MonkeyPatch, tmp_path: Path -) -> None: +def test_validate_lockfile_freshness_reports_stale_lockfiles(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_preflight, - "discover_tracked_lockfiles", - lambda _root, _runner: (root_lockfile, nested_lockfile), - ) - monkeypatch.setattr( - publish_preflight, - "validate_lockfile_freshness", - lambda _manifest, _runner: lockfile.LockfileFreshness( - is_fresh=False, - is_stale=True, - detail=( - "the lock file Cargo.lock needs to be updated but --locked was passed" - ), + repository = _RecordingLockfileRepository( + tracked=(root_lockfile, nested_lockfile), + default_freshness=lockfile.LockfileFreshness( + is_fresh=False, is_stale=True, detail=_STALE_DETAIL ), ) - 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, match="Tracked Cargo\\.lock files are stale", ) as excinfo: - publish_preflight._validate_lockfile_freshness(tmp_path, runner=runner, env={}) + publish_preflight._validate_lockfile_freshness(tmp_path, repository=repository) message = str(excinfo.value) assert str(root_lockfile) in message @@ -98,80 +101,42 @@ def runner( 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_preflight, - "discover_tracked_lockfiles", - lambda _root, _runner: (root_lockfile, nested_lockfile), - ) - monkeypatch.setattr( - publish_preflight, - "validate_lockfile_freshness", - lambda _manifest, _runner: lockfile.LockfileFreshness( - is_fresh=False, - is_stale=True, - detail=( - "the lock file Cargo.lock needs to be updated but --locked was passed" - ), + repository = _RecordingLockfileRepository( + tracked=(root_lockfile, nested_lockfile), + default_freshness=lockfile.LockfileFreshness( + is_fresh=False, is_stale=True, detail=_STALE_DETAIL ), ) - 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, match="Tracked Cargo\\.lock files are stale", ) as excinfo: publish_preflight._validate_lockfile_freshness( - workspace_root, runner=runner, env={} + workspace_root, repository=repository ) assert str(excinfo.value) == snapshot() -def test_validate_lockfile_freshness_surfaces_cargo_failures( - monkeypatch: pytest.MonkeyPatch, tmp_path: Path -) -> None: +def test_validate_lockfile_freshness_surfaces_cargo_failures(tmp_path: Path) -> None: """Cargo failures unrelated to stale lockfiles abort with cargo details.""" root_lockfile = tmp_path / "Cargo.lock" - - monkeypatch.setattr( - publish_preflight, - "discover_tracked_lockfiles", - lambda _root, _runner: (root_lockfile,), - ) - monkeypatch.setattr( - publish_preflight, - "validate_lockfile_freshness", - lambda _manifest, _runner: lockfile.LockfileFreshness( - is_fresh=False, - detail="failed to download registry index", + repository = _RecordingLockfileRepository( + tracked=(root_lockfile,), + default_freshness=lockfile.LockfileFreshness( + is_fresh=False, detail="failed to download registry index" ), ) - 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, match="failed to download registry index", ): - publish_preflight._validate_lockfile_freshness(tmp_path, runner=runner, env={}) + publish_preflight._validate_lockfile_freshness(tmp_path, repository=repository) diff --git a/tests/unit/test_bump_lockfile_repository.py b/tests/unit/test_bump_lockfile_repository.py new file mode 100644 index 00000000..e79fafea --- /dev/null +++ b/tests/unit/test_bump_lockfile_repository.py @@ -0,0 +1,75 @@ +"""Integration tests for the injected LockfileRepository port in :mod:`lading.commands.bump`.""" # noqa: E501 fixed one-line module docstring; wrapping it trips pydocstyle D205/D209 + +from __future__ import annotations + +import collections.abc as cabc +import pathlib + +from lading.commands import bump +from tests.helpers.workspace_builders import _make_config, _make_workspace + + +class _RecordingLockfileRepository: + """LockfileRepository double recording calls without touching Cargo.""" + + def __init__(self) -> None: + self.resolved: list[tuple[pathlib.Path, tuple[str, ...]]] = [] + self.regenerated: list[tuple[pathlib.Path, tuple[str, ...]]] = [] + + def resolve_lockfile_paths( + self, + workspace_root: pathlib.Path, + lockfile_manifests: cabc.Sequence[str], + ) -> tuple[pathlib.Path, ...]: + self.resolved.append((workspace_root, tuple(lockfile_manifests))) + return (workspace_root / "Cargo.lock",) + + def regenerate_lockfiles( + self, + workspace_root: pathlib.Path, + lockfile_manifests: cabc.Sequence[str], + ) -> tuple[pathlib.Path, ...]: + self.regenerated.append((workspace_root, tuple(lockfile_manifests))) + return (workspace_root / "Cargo.lock",) + + +def test_run_uses_injected_lockfile_repository(tmp_path: pathlib.Path) -> None: + """Bump reaches lockfile operations only through the repository port.""" + workspace = _make_workspace(tmp_path) + repository = _RecordingLockfileRepository() + + message = bump.run( + tmp_path, + "1.2.3", + options=bump.BumpOptions( + configuration=_make_config(), + workspace=workspace, + lockfile_repository=repository, + ), + ) + + assert repository.regenerated == [(tmp_path.resolve(), ())] + assert repository.resolved == [] + assert "Cargo.lock (lockfile)" in message + + +def test_dry_run_projects_through_lockfile_repository( + tmp_path: pathlib.Path, +) -> None: + """Dry runs project lockfile paths without regenerating.""" + workspace = _make_workspace(tmp_path) + repository = _RecordingLockfileRepository() + + bump.run( + tmp_path, + "1.2.3", + options=bump.BumpOptions( + dry_run=True, + configuration=_make_config(), + workspace=workspace, + lockfile_repository=repository, + ), + ) + + assert repository.resolved == [(tmp_path.resolve(), ())] + assert repository.regenerated == [] diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index af253b3e..ad5e427d 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -21,6 +21,7 @@ from lading import cli from lading import config as config_module from lading.commands import bump as bump_command +from lading.commands import bump_lockfiles from lading.commands import publish as publish_command from lading.utils import normalise_workspace_root from lading.workspace import WorkspaceCrate, WorkspaceGraph @@ -398,7 +399,9 @@ 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.command_runner is cli.subprocess_runner + repository = options.lockfile_repository + assert isinstance(repository, bump_lockfiles.CargoLockfileRepository) + assert repository.runner is cli.subprocess_runner def test_publish_cli_logs_dry_run_default_flag_resolution( diff --git a/tests/unit/test_lockfile.py b/tests/unit/test_lockfile.py index 9aba79ef..5f5e9f38 100644 --- a/tests/unit/test_lockfile.py +++ b/tests/unit/test_lockfile.py @@ -432,6 +432,150 @@ def runner( return runner +# --------------------------------------------------------------------------- +# CargoLockfileInspectionRepository adapter (issue #82) +# --------------------------------------------------------------------------- + + +# Each recorded call captures (command, cwd, env, echo_stdout). +type _RecordedCall = tuple[ + tuple[str, ...], Path | None, cabc.Mapping[str, str] | None, bool +] + + +def _recording_runner( + calls: list[_RecordedCall], + *, + exit_code: int = 0, + stdout: str = "", + stderr: str = "", +) -> cabc.Callable[..., tuple[int, str, str]]: + """Return a runner recording each invocation's command, cwd, env, echo_stdout.""" + + def runner( + command: cabc.Sequence[str], + *, + cwd: Path | None = None, + env: cabc.Mapping[str, str] | None = None, + echo_stdout: bool = True, + ) -> tuple[int, str, str]: + calls.append((tuple(command), cwd, env, echo_stdout)) + return exit_code, stdout, stderr + + return runner + + +@pytest.fixture +def _cargo_workspace(tmp_path: Path) -> None: + """Write a minimal root Cargo workspace manifest and empty lockfile.""" + (tmp_path / "Cargo.toml").write_text("[workspace]\n", encoding="utf-8") + (tmp_path / "Cargo.lock").write_text("", encoding="utf-8") + + +class TestCargoLockfileInspectionRepositoryAdapter: + """Tests for the CargoLockfileInspectionRepository adapter (issue #82).""" + + @pytest.mark.usefixtures("_cargo_workspace") + def test_adapter_discovers_lockfiles_binding_env(self, tmp_path: Path) -> None: + """The adapter discovers tracked lockfiles through its bound runner and env.""" + calls: list[_RecordedCall] = [] + base_env = {"CARGO_TERM_COLOR": "never"} + repository = lockfile.CargoLockfileInspectionRepository( + runner=_recording_runner(calls, stdout="Cargo.lock\n"), + env=base_env, + ) + + result = repository.discover_tracked_lockfiles(tmp_path) + + assert result == (tmp_path / "Cargo.lock",), "discovers the tracked lockfile" + assert calls == [ + ( + ("git", "ls-files", "**/Cargo.lock", "Cargo.lock"), + tmp_path, + base_env, + True, + ) + ], "git ls-files should receive the bound env" + + def test_adapter_validates_freshness_binding_env(self, tmp_path: Path) -> None: + """The adapter probes freshness through its bound runner, applying env.""" + manifest_path = tmp_path / "Cargo.toml" + calls: list[_RecordedCall] = [] + base_env = {"CARGO_TERM_COLOR": "never"} + repository = lockfile.CargoLockfileInspectionRepository( + runner=_recording_runner(calls), + env=base_env, + ) + + result = repository.validate_lockfile_freshness(manifest_path) + + assert result.is_fresh, "probe should report the lockfile fresh" + assert len(calls) == 1, "one cargo call expected" + command, cwd, env, echo_stdout = calls[0] + assert command[:3] == ("cargo", "metadata", "--locked"), "cargo metadata probe" + assert cwd == manifest_path.parent, "cargo runs in the manifest directory" + assert env == base_env, "cargo call should receive the bound env" + assert echo_stdout is True, "echo_stdout defaults to True" + + @pytest.mark.usefixtures("_cargo_workspace") + def test_adapter_without_env_leaves_runner_env_untouched( + self, tmp_path: Path + ) -> None: + """With no bound env the adapter forwards calls without injecting one.""" + calls: list[_RecordedCall] = [] + repository = lockfile.CargoLockfileInspectionRepository( + runner=_recording_runner(calls, stdout="Cargo.lock\n"), + ) + + repository.discover_tracked_lockfiles(tmp_path) + + assert calls[0][2] is None, "no env should be injected without a bound env" + + @pytest.mark.usefixtures("_cargo_workspace") + def test_adapter_honours_injected_manifest_exists(self, tmp_path: Path) -> None: + """A custom ``manifest_exists`` predicate overrides the filesystem probe.""" + # The _cargo_workspace fixture writes a real manifest/lockfile pair, so the + # default filesystem probe would include this lockfile. The injected + # predicate must be what excludes it, so the test fails if the adapter + # ignores ``manifest_exists``. + calls: list[_RecordedCall] = [] + probed: list[Path] = [] + + def manifest_exists(manifest_path: Path) -> bool: + probed.append(manifest_path) + return False + + repository = lockfile.CargoLockfileInspectionRepository( + runner=_recording_runner(calls, stdout="Cargo.lock\n"), + manifest_exists=manifest_exists, + ) + + result = repository.discover_tracked_lockfiles(tmp_path) + + assert result == (), "injected predicate should exclude the lockfile" + assert probed == [tmp_path / "Cargo.toml"], "predicate probed the manifest" + + def test_adapter_bound_runner_forwards_echo_stdout(self, tmp_path: Path) -> None: + """The env-bound runner forwards ``echo_stdout`` unchanged to the runner.""" + calls: list[_RecordedCall] = [] + base_env = {"CARGO_TERM_COLOR": "never"} + repository = lockfile.CargoLockfileInspectionRepository( + runner=_recording_runner(calls), + env=base_env, + ) + + bound_runner = repository._bound_runner() + bound_runner(("git", "status"), cwd=tmp_path, echo_stdout=False) + + assert len(calls) == 1, "one forwarded call expected" + command, cwd, env, echo_stdout = calls[0] + assert command == ("git", "status"), "command forwarded unchanged" + assert cwd == tmp_path, "cwd forwarded unchanged" + # env is still defaulted from the bound base_env when the call omits it. + assert env == base_env, "env defaulted from the bound base_env" + assert echo_stdout is False, "echo_stdout forwarded unchanged" + + @pytest.mark.usefixtures("_metrics_registry") def test_discovery_records_lockfile_count(tmp_path: Path) -> None: """Discovery increments the discovered-lockfiles counter by the count."""