diff --git a/docs/developers-guide.md b/docs/developers-guide.md index e6957c5d..807ed9b9 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -88,6 +88,33 @@ 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. 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, 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 +`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 @@ -153,6 +180,21 @@ 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 @@ -162,11 +204,55 @@ 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 `RuntimeError` 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 | | --- | --- | @@ -310,6 +396,18 @@ 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`) + +`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 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`) `lading.commands.publish_preflight` performs workspace validation before @@ -321,7 +419,7 @@ _run_preflight_checks( *, allow_dirty: bool, configuration: LadingConfig, - runner: _CommandRunner | None = None, + runner: CommandRunner | None = None, ) -> None ``` 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/__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/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/bump.py b/lading/commands/bump.py index 7cf365d2..15572195 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.runtime import CommandRunner, subprocess_runner 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,10 +80,12 @@ 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) -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 ( @@ -86,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}:" @@ -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) if changed_manifests else () + 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,41 @@ 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. + + 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, 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. + """ + 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: + LOGGER.info("Dry run; would refresh %s", lockfile_path) + return lockfiles + + 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 def _update_crate_manifest( @@ -257,11 +305,11 @@ def _format_result_message( workspace_root: Path, ) -> str: """Summarise the bump outcome for CLI presentation.""" - if not changes.manifests and not changes.documents: - return _format_no_changes_message(target_version, dry_run) + if not any((changes.manifests, changes.documents, changes.lockfiles)): + 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 @@ -270,6 +318,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..bef04f59 --- /dev/null +++ b/lading/commands/lockfile.py @@ -0,0 +1,224 @@ +"""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` +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`` +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: + +```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 + +import collections.abc as cabc +import logging +import typing as typ +from pathlib import Path + +from lading.exceptions import LadingError + +if typ.TYPE_CHECKING: + from lading.runtime import CommandRunner + +LOGGER = logging.getLogger(__name__) +_ManifestExists = cabc.Callable[[Path], bool] + + +class LockfileRefreshError(LadingError): + """Raised when Cargo cannot regenerate a lockfile.""" + + +def _handle_git_ls_files_failure( + exit_code: int, + stdout: str, + stderr: str, + workspace_root: Path, +) -> tuple[Path, ...] | None: + """Return ``None`` for git success, or an empty result for git failure.""" + if exit_code == 0: + return None + 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( + stdout: str, + workspace_root: Path, + manifest_exists: _ManifestExists, +) -> tuple[Path, ...]: + """Return lockfile paths from ``git ls-files`` with adjacent manifests.""" + lockfiles: list[Path] = [] + for line in stdout.splitlines(): + relative_path = line.strip() + if not relative_path: + continue + lockfile_path = workspace_root / relative_path + if "target" in lockfile_path.relative_to(workspace_root).parts: + continue + 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. + + 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)``. + manifest_exists + Callable used to decide whether a candidate lockfile has an adjacent + manifest. The default adapter checks the filesystem. + + 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. + """ + exit_code, stdout, stderr = runner( + ("git", "ls-files", "**/Cargo.lock", "Cargo.lock"), + cwd=workspace_root, + ) + error_result = _handle_git_ls_files_failure( + exit_code, stdout, stderr, workspace_root + ) + if error_result is not None: + return error_result + 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( + manifest_path: Path, + runner: CommandRunner, +) -> 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. + """ + 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 {lockfile_path}" + if detail: + message = f"{message}: {detail}" + raise LockfileRefreshError(message) + LOGGER.info("Refreshed %s", lockfile_path) + return lockfile_path + + +def validate_lockfile_freshness( + manifest_path: Path, + runner: CommandRunner, +) -> bool: + """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", + "metadata", + "--locked", + "--manifest-path", + str(manifest_path), + "--format-version=1", + ), + cwd=manifest_path.parent, + ) + 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/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_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/commands/publish_preflight.py b/lading/commands/publish_preflight.py index 5b39fad7..9dde4198 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 @@ -24,10 +28,15 @@ import dataclasses as dc import logging import os +import shlex import tempfile 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 +143,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) @@ -180,6 +194,58 @@ 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, + 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: list[Path] = [] + 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 = [ + "Tracked Cargo.lock files are stale after manifest version changes.", + ( + "This commonly happens after running `lading bump`; repair each " + "stale lockfile directly:" + ), + ] + for lockfile_path in stale_lockfiles: + manifest_path = lockfile_path.parent / "Cargo.toml" + lines.append(f"- {lockfile_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) + raise PublishPreflightError(message) + + def _preflight_argument_sets( target_dir: Path, *, unit_tests_only: bool ) -> tuple[tuple[str, ...], tuple[str, ...]]: 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..770ec400 --- /dev/null +++ b/lading/exceptions.py @@ -0,0 +1,20 @@ +"""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 + + +class LadingError(Exception): + """Base class for all lading exceptions.""" diff --git a/lading/workspace/metadata.py b/lading/workspace/metadata.py index c3a7ade8..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, ) @@ -21,7 +21,7 @@ from pathlib import Path -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.""" 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/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/metadata_fixtures.py b/tests/bdd/steps/metadata_fixtures.py index 9655926d..98189989 100644 --- a/tests/bdd/steps/metadata_fixtures.py +++ b/tests/bdd/steps/metadata_fixtures.py @@ -91,11 +91,14 @@ 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="", ).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_bump_steps.py b/tests/bdd/steps/test_bump_steps.py index 9c4b3571..287c5d96 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="cargo generate-lockfile\n", stderr="") + + @when( parsers.parse("I invoke lading bump {version} with that workspace"), target_fixture="cli_run", @@ -47,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"] @@ -117,6 +137,33 @@ 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 + 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}"')) 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..d2ec9cea 100644 --- a/tests/bdd/steps/test_publish_given_steps.py +++ b/tests/bdd/steps/test_publish_given_steps.py @@ -67,6 +67,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 c744bf3a..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 ----- @@ -185,6 +251,9 @@ def _register_preflight_commands( """ defaults: dict[tuple[str, ...], ResponseProvider] = { ("git", "status", "--porcelain"): _CommandResponse(exit_code=0), + ("git", "ls-files", "**/Cargo.lock", "Cargo.lock"): _CommandResponse( + exit_code=0 + ), ( "cargo", "check", @@ -234,15 +303,74 @@ 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() + 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) + ) + 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, +) -> 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/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/__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_lockfile_validation.ambr b/tests/unit/publish/__snapshots__/test_preflight_lockfile_validation.ambr new file mode 100644 index 00000000..7536f7b0 --- /dev/null +++ b/tests/unit/publish/__snapshots__/test_preflight_lockfile_validation.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_command_helpers.py b/tests/unit/publish/test_command_helpers.py index d8e36d3b..4b0107e2 100644 --- a/tests/unit/publish/test_command_helpers.py +++ b/tests/unit/publish/test_command_helpers.py @@ -4,7 +4,13 @@ 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") @@ -66,3 +72,57 @@ 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_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 + + +@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(cli._CMD_MOX_STUB_ENV, value) + + assert cli._select_runner() is cmd_mox_runner.cmd_mox_runner + + +def test_should_use_cmd_mox_stub_returns_false_by_default( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Missing environment values disable cmd-mox stubbing.""" + monkeypatch.delenv(cli._CMD_MOX_STUB_ENV, raising=False) + + assert cli._select_runner() is subprocess_runner 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..18482eeb --- /dev/null +++ b/tests/unit/publish/test_preflight_cargo_runner.py @@ -0,0 +1,177 @@ +"""Unit tests for the low-level _run_cargo_preflight helper.""" + +from __future__ import annotations + +import collections.abc as cabc +import dataclasses as dc +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 + + +@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"], + 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() + + +@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", + ), + pytest.param( + _RunCargoPreflightCase( + options=publish._CargoPreflightOptions( + extra_args=("--workspace", "--all-targets"), + test_excludes=["", " ", "\t", "\n"], + ), + expected_tail=(), + ), + id="blank_test_excludes", + ), + pytest.param( + _RunCargoPreflightCase( + options=publish._CargoPreflightOptions( + extra_args=("--workspace", "--all-targets"), + unit_tests_only=True, + ), + expected_tail=("--lib", "--bins"), + ), + id="unit_tests_only", + ), + 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}" + ) + + +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 c455ed62..569bfaf3 100644 --- a/tests/unit/publish/test_preflight_checks.py +++ b/tests/unit/publish/test_preflight_checks.py @@ -4,185 +4,16 @@ 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 .conftest import ORIGINAL_PREFLIGHT, make_config, make_preflight_config if typ.TYPE_CHECKING: - 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 + from lading.runtime import CommandRunner def test_preflight_checks_remove_all_targets_for_unit_only( @@ -437,41 +268,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..01832df5 --- /dev/null +++ b/tests/unit/publish/test_preflight_lockfile_validation.py @@ -0,0 +1,124 @@ +"""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, publish_preflight + +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_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_preflight._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_preflight, + "discover_tracked_lockfiles", + lambda _root, _runner: (root_lockfile, nested_lockfile), + ) + monkeypatch.setattr( + 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_preflight._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_preflight, + "discover_tracked_lockfiles", + lambda _root, _runner: (root_lockfile, nested_lockfile), + ) + monkeypatch.setattr( + 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_preflight._validate_lockfile_freshness( + workspace_root, runner=runner, env={} + ) + + assert str(excinfo.value) == snapshot() diff --git a/tests/unit/publish/test_run_integration.py b/tests/unit/publish/test_run_integration.py index a24d1d87..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[0] == "git": + 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_bump_command_integration.py b/tests/unit/test_bump_command_integration.py index 64646b33..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, @@ -25,6 +26,9 @@ ) if typ.TYPE_CHECKING: + import collections.abc as cabc + from pathlib import Path + from _pytest.monkeypatch import MonkeyPatch @@ -65,6 +69,136 @@ 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, ( + "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( + 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(), ( + f"expected '- Cargo.lock (lockfile)' in bump output:\n{message}" + ) + + +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) @@ -249,11 +383,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_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_cargo_metadata_loading.py b/tests/unit/test_cargo_metadata_loading.py index 34578c58..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, @@ -125,6 +125,16 @@ 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 coerce_text(encoded) == alpha + + binary = b"foo\xff" + 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 96a42f8e..2cd6a615 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 cli.subprocess_runner @pytest.mark.parametrize( diff --git a/tests/unit/test_lockfile.py b/tests/unit/test_lockfile.py new file mode 100644 index 00000000..75af33cc --- /dev/null +++ b/tests/unit/test_lockfile.py @@ -0,0 +1,294 @@ +"""Unit tests for Cargo lockfile helper functions.""" + +from __future__ import annotations + +import collections.abc as cabc +import string +from pathlib import Path + +import hypothesis.strategies as st +import pytest +from hypothesis import given + +from lading.commands import lockfile + +# --------------------------------------------------------------------------- +# 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: + """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]: + """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, "", "" + + 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: + """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.toml").write_text("[package]\n", encoding="utf-8") + (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]: + assert command == ("git", "ls-files", "**/Cargo.lock", "Cargo.lock") + assert cwd == tmp_path + assert env is None + return ( + 0, + ( + "Cargo.lock\n" + "tests/ui_lints/Cargo.lock\n" + "target/debug/Cargo.lock\n" + "orphan/Cargo.lock\n" + ), + "", + ) + + 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_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: + """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" + + 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: + """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, "", "" + + 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: + """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) + + +@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 + 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}" + ) + + +@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"