diff --git a/CHANGES.md b/CHANGES.md index 72c1fb6..4916967 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -3,7 +3,20 @@ ## 7.0.0b16 (unreleased) -- Nothing changed yet. +- Honour `auto_commit` in `plonecli setup` and add its `--no-git` flag, so the + command no longer commits (or initialises) a repository the user opted out of. + It now reports the commit like `create` and `add`. + [MrTango] + +- Commit the skills installed by `plonecli skill install/update --scope project`, + so the project is not left dirty for the next `plonecli add`. Opt out with + `--no-git` or `auto_commit = false`. + [MrTango] + +- Locate the copier-templates checkout for the integration tests via the repo's + own `develop/plone/src/copier-templates` and the configured clone, so the + suite no longer silently skips 23 of its 24 cases. + [MrTango] ## 7.0.0b15 (2026-08-15) diff --git a/evals/scaffolding/.gitignore b/evals/scaffolding/.gitignore index 294788e..5d63ea1 100644 --- a/evals/scaffolding/.gitignore +++ b/evals/scaffolding/.gitignore @@ -1,4 +1,6 @@ workspaces/ +workspaces-git/ results/ +results-git/ __pycache__/ *.py[cod] diff --git a/evals/scaffolding/EVALUATION.md b/evals/scaffolding/EVALUATION.md index 1830ff8..619b046 100644 --- a/evals/scaffolding/EVALUATION.md +++ b/evals/scaffolding/EVALUATION.md @@ -1,89 +1,204 @@ # Scaffolding evaluation findings -> Historical baseline from before the fixes. A full verification run passed -> 245/245 cases with no warnings on 2026-08-13. The ignored -> `results/report.md` is mutable and may instead contain the latest quick or -> CI-validation run. +Run on 2026-08-15 against plonecli `7.0.0b16.dev0` (`c8ebe13`) and +copier-templates `2705bf3`. The ignored `results/` and `results-git/` trees hold +the runtime reports; they are replaced on every run and may contain a later +quick or CI-validation run instead. + +The three Medium problems were fixed in plonecli after the run; each carries its +fix and its regression test below. The three Low problems live in +copier-templates and are still open. ## Scope -The full run executed 245 cases against the development templates checkout: +Two harnesses were executed. + +`run_evals.py` (full mode, 245 cases) covers: - all 3 project templates; - all 20 feature templates individually; -- explicit finite high-interaction matrices for backend/Svelte booleans, behavior booleans, REST booleans crossed with target mode, reachable content-type states, view choices/booleans, vocabulary types, all viewlet managers and template states, and Zope distribution/storage choices; -- combined, reversed-order, repeated-application, hostile-input, and command-chain cases; +- explicit finite high-interaction matrices for backend/Svelte booleans, + behavior booleans, REST booleans crossed with target mode, reachable + content-type states, view choices/booleans, vocabulary types, all viewlet + managers and template states, and Zope distribution/storage choices; +- combined, reversed-order, repeated-application, hostile-input, and + command-chain cases; - harmless root commands and the CLI command unit suite. -Open-ended strings, integers, and environment-discovered choices cannot have a literal exhaustive Cartesian product. They are covered with defaults, non-default valid values, manual-choice paths, and hostile quote/newline/backslash partitions. Finite domains are exhaustive only in the explicitly named high-interaction matrices; other templates receive individual non-default cases plus their focused unit tests. - -Result: **237 passed, 8 failed**. See the ignored runtime report at `results/report.md` and per-case logs under `results/logs/`. - -## Problems - -### High: free text can generate invalid TOML - -`backend_addon` and `zope-setup` interpolate title, description, and author values directly into quoted TOML. Quotes, newlines, and backslashes can produce an invalid `pyproject.toml`. - -Evidence: +`run_git_evals.py` (25 cases, new in this round) covers the auto-commit path +that `run_evals.py` deliberately disables — see "Auto-commit verification". -- `hostile-backend-toml-strings`: generation returned success, but TOML validation failed. -- `hostile-zope-toml-strings`: the generated TOML was invalid and the post-copy hook aborted while parsing it. +Open-ended strings, integers, and environment-discovered choices cannot have a +literal exhaustive Cartesian product. They are covered with defaults, +non-default valid values, manual-choice paths, and hostile +quote/newline/backslash partitions. Finite domains are exhaustive only in the +explicitly named high-interaction matrices; other templates receive individual +non-default cases plus their focused unit tests. -Use a TOML-safe Jinja filter or generate these values through `tomlkit` instead of interpolating raw strings. +## Result -### High: `zope_instance` cannot be added through plonecli +**245 passed, 0 failed** in the scaffolding matrix (1050s), and **25 passed, 0 +failed** in the auto-commit harness (40s). -All four `zope_instance` CLI cases failed because `plonecli add` did not list `zope_instance` in a standalone `zope-setup` project. The generated project contains both `[tool.plone.project.settings]` and `[tool.plone.backend_addon.settings]`. Project detection checks backend settings first, classifies the project as `backend_addon`, and exposes the wrong subtemplate set. +Every problem in the previous round is fixed and stayed fixed: free-text TOML +values serialize safely, standalone Zope projects are detected as such, +`create` → `setup` chains refresh the project context, conflicting theme +overlays are rejected, and the Barceloneta integration test targets the +generated package path. The copier-template suite now emits **zero** warnings, +so the deprecated `ContextHook.update` and `click.__version__` opportunities are +also closed. -Either avoid writing backend-addon settings for standalone Zope projects or make project detection prefer the substantive project settings in this mixed layout. +## Auto-commit verification -### High: chained `create` then `setup` fails +`plonecli` promises that every generated or modified file is committed, so a +package always has reviewable history. `run_evals.py` cannot check that: it +passes `--no-git` on every case to keep generated trees disposable, which left +the default user-facing path with no coverage at all. -The CLI declares `chain=True`, but `chain-create-then-setup` failed after successfully creating the backend add-on. The group retains the project context detected before `create`, so `setup` still reports that it is outside a package. +`run_git_evals.py` closes that gap. It runs the same commands with git enabled +and, after each one, asserts the project is a git repository, that +`git status --porcelain` is empty, and that the run produced the expected new +commit. -Refresh project context after creation, or remove command chaining if cross-context chains are not supported. +| Command | Cases | Result | +|---|---|---| +| `create backend_addon` / `zope-setup` / `addon` | 3 | clean tree, one commit per template step | +| `skill install --scope project` | 1 | clean tree, `Add plonecli skills` | +| `setup` | 1 | clean tree, `Add zope-setup template` | +| `add ` (all 19) + `add zope_instance` | 20 | clean tree, `Add subtemplate` | -### Medium: theme variants conflict without non-interactive resolution +`serve`, `debug`, and `test` delegate to invoke and write only into gitignored +paths (`runtime/*/var/`, `.pytest_cache/`, `.coverage`); they are not expected +to commit and are covered by the root CLI command unit suite. `config`, +`update` and `completion` do not touch a project tree. -The all-feature sequence failed when `theme_barceloneta` followed `theme`: both own `profiles/default/theme.xml` and related theme paths. Copier requested an interactive overwrite despite `--defaults`, then aborted in the non-TTY evaluation. +Two commands did **not** hold the contract — both are fixed, see problems 1 and +2 below. -Treat theme templates as explicit alternatives and reject a second theme with a clear message, or add a documented overwrite/replacement flow. - -### Medium: Barceloneta integration test uses a stale path - -The root integration suite generated the test at `src/collective/mythemetest/tests/test_theme_my_test_theme.py`, but `tests/test_theme_barceloneta_integration.py` expects it under top-level `tests/`. Result: 23 integration cases passed and 1 failed. +## Problems -Update the assertion and pytest target to the generated `src//tests/` layout. +### Fixed, was Medium: `plonecli setup` ignored `auto_commit = false` and had no `--no-git` + +`cli.py` called `run_create()` without a `git_commit` argument, and +`templates.py:139` defaults it to `True`. `create` and `add` both compute +`config.auto_commit and not no_git`; `setup` computed nothing and always +committed. It was also the only file-writing command with no `--no-git` flag, +and it discarded `run_create`'s return value, so it never printed the +`Committed: ...` line the other two print. + +Evidence: with `auto_commit = false` in `~/.plonecli/config.toml`, `create` +correctly produced a package with no repository, and the following `setup` ran +`git init` and committed it anyway — as `Create package with zope-setup +template`, for a package it did not create. + +**Fixed.** `setup` now passes `git_commit=config.auto_commit and not no_git`, +accepts `--no-git`, carries a chained `--no-git` across `create ... setup`, and +echoes the returned commit message. Re-verified end to end: with +`auto_commit = false` the package still has no repository after `setup`; with +the default it commits `Add zope-setup template` and leaves a clean tree. +Covered by `tests/test_setup_command.py` and `tests/test_plonecli.py`. + +### Fixed, was Medium: `plonecli skill install --scope project` left the repo dirty + +The installer writes `.agents/skills/` and `.claude/skills/` into +the project root and never committed. Neither path is in the generated +`.gitignore`, so the working tree was left with two untracked directories. + +That broke the next command: `plonecli add --defaults` aborted with +"Refusing to run on a git repository with uncommitted changes", so plonecli's +own command put a project into a state plonecli refuses to work in. + +**Fixed.** A project-scope `install`/`update` now commits the installed skills +under the same `auto_commit` contract, as `Add plonecli skills` / +`Update plonecli skills`, with `--no-git` to opt out. It uses the new +`git.commit_paths()`, which commits only the skill directories: it never runs +`git init` and never sweeps unrelated user changes into the commit. User-scope +installs are untouched. Covered by `tests/test_skill.py` and +`tests/test_git.py`. + +### Fixed, was Medium: the root integration suite silently skipped 23 of its 24 cases + +`_templates_dir()` in `tests/test_theme_barceloneta_integration.py` and +`tests/test_all_templates_data.py` resolved the template checkout from +`PLONECLI_TEMPLATES_DIR` or two hard-coded `/home/node/...` paths. This +repository keeps its development checkout at `develop/plone/src/copier-templates` +(per `AGENTS.md`), which is neither. + +So a plain `uv run pytest -m integration` reported "1 passed, 2 skipped" — +green, and almost entirely empty, because the parametrized template sweep +collapsed to an empty parameter set. + +**Fixed.** All three integration modules now share +`tests/helpers.find_templates_checkout()`, which tries `PLONECLI_TEMPLATES_DIR`, +the repo-relative `develop/plone/src/copier-templates`, the configured +`PlonecliConfig().templates_dir`, then the devcontainer paths. A plain +`uv run pytest -m integration --collect-only` now collects 24 cases with no +environment variable set. Covered by `tests/test_templates_checkout.py`, whose +last test asserts the sweep is populated whenever a checkout exists — the +regression itself. + +### Low: the template git warning prints a mangled filename + +`shared/hooks/git_check.py` calls `.strip()` on the whole `git status +--porcelain` output before splitting it into lines. Porcelain lines are +`XY`, so stripping removes the leading status space of the *first* +line and shifts every subsequent slice by one: ` M .gitignore` is reported as +`gitignore`. + +Observed in the clean-tree `setup` run: + +```text +Modified files: + - gitignore +``` + +Split with `splitlines()` on the unstripped output. `plonecli/git.py` already +does this correctly; only the template hook is affected. + +### Low: the git warning fires on changes plonecli itself just made + +The same warning appears during `setup` and `add zope_instance` even when the +tree was clean before the command, because `zope-setup` invokes the +`zope_instance` template from a post-copy hook after the current copy has +already written files. The user is told to commit or stash changes that +plonecli made moments earlier and is about to commit itself. + +Scope the check to the state before the copy operation, or suppress it for +nested template invocations. + +### Low: `plonecli setup` replaces the backend addon's `.gitignore` wholesale + +`backend_addon` and `zope-setup` each ship a full `.gitignore.jinja`, and +`setup` overwrites the first with the second. Today the loss is only +`Thumbs.db` — `dist/` and `.cache` are still matched by the remaining bare +patterns, verified with `git check-ignore` — so there is no functional impact. +But the two files are independently maintained copies, so any backend-only +ignore rule added later disappears on `setup` without a signal. + +Share one base fragment between the two templates, or append instead of +overwrite. ## Optimization opportunities -- Copier template extensions emitted hundreds of deprecation warnings because `ContextHook.update` is deprecated. Migrate hooks to modify context in `hook`. -- `click_aliases` reads deprecated `click.__version__`; update or replace the dependency before Click 9.1. -- Feature generation inside these nested, `--no-git` workspaces reports the outer plonecli repository as dirty. Git cleanliness checks should be scoped to the detected generated project rather than walking into an unrelated parent repository. -- Keep the generated TOML/XML/Python validators as CI checks. They found failures that successful Copier exit codes did not detect. - -## Resolution - -All findings above have been addressed: - -- free-text TOML values use serialization filters; -- standalone Zope projects are detected correctly; -- chained `create` → `setup` refreshes project context; -- theme variants reject conflicting overlays; -- the Barceloneta integration test uses the generated package test path; -- context hooks use the current in-place API; -- the deprecated command-alias dependency was removed; -- Git checks are scoped to the generated project; -- subtemplate validation tasks use Copier's `_copier_operation` value and no - longer report files generated earlier in the same copy as pre-existing - changes; -- generated TOML/XML/Python validation runs in CI. - -## Baseline test receipts - -- Root unit suite: **209 passed, 16 skipped**. -- Copier-template unit suite: **386 passed, 2 integration tests deselected**. +- CI runs only `run_evals.py --ci-validation`. Add `run_git_evals.py --quick` + so the auto-commit contract cannot regress unnoticed. The integration suite no + longer needs `PLONECLI_TEMPLATES_DIR` to find the checkout, but setting it in + CI still pins which checkout is used. +- The five `WARNING: Git repository has uncommitted changes!` entries in the + matrix report are the two Low findings above, not template failures; fixing + them removes the noise from the report's warning section. +- Keep the generated TOML/XML/Python validators as CI checks. They caught + failures in the previous round that successful Copier exit codes did not. + +## Test receipts + +- Root unit suite: **280 passed, 12 skipped** (the skips are the model-billing + skill evals; 216 passed / 1 skipped before the third fix, which restored the + template sweep and added the new tests). +- Root integration suite: **24 collected and passed**, with or without + `PLONECLI_TEMPLATES_DIR`. +- Copier-template unit suite: **391 passed**, 2 integration tests deselected, + **0 warnings**. - Copier-template integration suite: **2 passed**. -- Root integration suite: **23 passed, 1 failed** (stale Barceloneta test path above). -- Full scaffolding matrix: **199 passed, 8 failed** (the eight cases map to four product problems above). +- Full scaffolding matrix: **245 passed, 0 failed**. +- Auto-commit harness: **25 passed, 0 failed**. diff --git a/evals/scaffolding/README.md b/evals/scaffolding/README.md index d0a0317..1106eb5 100644 --- a/evals/scaffolding/README.md +++ b/evals/scaffolding/README.md @@ -24,6 +24,22 @@ repository template inventory and fails when a new template has no lane. The run PLONECLI_TEMPLATES_DIR=/workspaces/plonecli/develop/plone/src/copier-templates ``` +`run_git_evals.py` is the complement. Because the matrix above passes +`--no-git` everywhere, it never exercises auto-commit — the behaviour users get +by default. This harness runs the same commands with git enabled and asserts, +after each one, that the project is a git repository, that `git status +--porcelain` is empty, and that the expected commit was created: + +```sh +uv run python evals/scaffolding/run_git_evals.py --quick +uv run python evals/scaffolding/run_git_evals.py +``` + +`--quick` runs only the project-level cases (`create` × 3, `skill install +--scope project`, `setup`); the full run adds every subtemplate plus +`zope_instance`. It writes to `workspaces-git/` and `results-git/`, so the two +harnesses never share state. + Generated trees are disposable and always live beneath `workspaces/`. Reports and per-case command logs are written beneath `results/`: diff --git a/evals/scaffolding/run_git_evals.py b/evals/scaffolding/run_git_evals.py new file mode 100644 index 0000000..ceaab0d --- /dev/null +++ b/evals/scaffolding/run_git_evals.py @@ -0,0 +1,375 @@ +#!/usr/bin/env python3 +"""Verify plonecli auto-commit: every command leaves the project repo clean. + +The scaffolding matrix in ``run_evals.py`` runs everything with ``--no-git`` so +that generated trees stay disposable. That leaves the auto-commit path — the +behaviour users actually get by default — untested. This harness is the +complement: it runs the same commands *with* git enabled and asserts, after +every command, that + +- the generated project is a git repository, +- ``git status --porcelain`` is empty, and +- the run produced the expected new commit. + +Reports land in ``results-git/report.json`` and ``results-git/report.md``. +""" + +from __future__ import annotations + +import argparse +import json +import os +import shutil +import subprocess +import sys +import time +from pathlib import Path +from typing import Any + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from run_evals import ( # noqa: E402 + BACKEND_SUBTEMPLATES, + CLI, + ROOT, + TEMPLATES, + backend_data, + data_args, + individual_data, + provenance, + zope_data, +) + +HERE = Path(__file__).resolve().parent +WORKSPACES = HERE / "workspaces-git" +RESULTS = HERE / "results-git" +LOGS = RESULTS / "logs" + + +def git(repo: Path, *args: str) -> subprocess.CompletedProcess: + return subprocess.run( + ["git", "-C", str(repo), *args], + capture_output=True, + text=True, + ) + + +def porcelain(repo: Path) -> list[str]: + return [line for line in git(repo, "status", "--porcelain").stdout.splitlines()] + + +def commit_subjects(repo: Path) -> list[str]: + result = git(repo, "log", "--format=%s") + if result.returncode != 0: + return [] + return result.stdout.splitlines() + + +def run(cmd: list[str], cwd: Path, env: dict[str, str], log: Path, timeout: int): + started = time.monotonic() + completed = subprocess.run( + cmd, + cwd=str(cwd), + env=env, + stdin=subprocess.DEVNULL, + capture_output=True, + text=True, + timeout=timeout, + ) + log.write_text( + f"$ {' '.join(cmd)}\n(cwd={cwd})\n\n--- stdout ---\n{completed.stdout}\n" + f"--- stderr ---\n{completed.stderr}\n", + encoding="utf-8", + ) + return completed, time.monotonic() - started + + +class Recorder: + def __init__(self) -> None: + self.results: list[dict[str, Any]] = [] + + def record(self, case_id: str, problems: list[str], detail: dict[str, Any]) -> None: + status = "passed" if not problems else "failed" + reason = f" -- {'; '.join(problems)}" if problems else "" + print(f" {status}: {case_id}{reason}") + self.results.append( + { + "id": case_id, + "status": status, + "problems": problems, + **detail, + } + ) + + +def exit_problems(completed: subprocess.CompletedProcess) -> list[str]: + """A one-entry problem list when the command itself failed.""" + if completed.returncode == 0: + return [] + return [f"exit={completed.returncode}"] + + +def check_repo( + repo: Path, expect_commit: str | None, before: list[str] +) -> tuple[list[str], dict[str, Any]]: + """Return (problems, detail) for the repo state after a command.""" + problems: list[str] = [] + if not (repo / ".git").exists(): + return ["no git repository was initialised"], {"dirty": [], "commits": []} + + dirty = porcelain(repo) + after = commit_subjects(repo) + if dirty: + problems.append(f"working tree dirty after the command: {dirty[:8]}") + if expect_commit is not None: + new = after[: len(after) - len(before)] + if not new: + problems.append("no new commit was created") + elif expect_commit not in new[0]: + problems.append( + f"commit subject {new[0]!r} does not mention {expect_commit!r}" + ) + return problems, {"dirty": dirty, "commits": after} + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--timeout", type=int, default=900) + parser.add_argument( + "--quick", + action="store_true", + help="Only the project-level cases, skipping the per-subtemplate sweep.", + ) + args = parser.parse_args() + + shutil.rmtree(WORKSPACES, ignore_errors=True) + shutil.rmtree(RESULTS, ignore_errors=True) + WORKSPACES.mkdir(parents=True) + LOGS.mkdir(parents=True) + + env = dict(os.environ) + env["PLONECLI_TEMPLATES_DIR"] = str(TEMPLATES) + env["PYTHONUNBUFFERED"] = "1" + # A deterministic identity so the commit path never depends on the machine. + env.setdefault("GIT_AUTHOR_NAME", "Eval Runner") + env.setdefault("GIT_AUTHOR_EMAIL", "eval@example.invalid") + env.setdefault("GIT_COMMITTER_NAME", "Eval Runner") + env.setdefault("GIT_COMMITTER_EMAIL", "eval@example.invalid") + + rec = Recorder() + started = time.monotonic() + + # --- create backend_addon ------------------------------------------------- + print("create cases") + backend_root = WORKSPACES / "create-backend" + backend_root.mkdir() + target = backend_root / "collective.gitbackend" + cmd = [ + *CLI, + "create", + "backend_addon", + str(target), + "--defaults", + "--allow-dirty", + *data_args(backend_data("collective.gitbackend")), + ] + completed, _ = run(cmd, ROOT, env, LOGS / "create-backend_addon.log", args.timeout) + problems = exit_problems(completed) + repo_problems, detail = check_repo(target, "backend_addon", []) + rec.record("create-backend_addon", problems + repo_problems, detail) + + # --- skill install --scope project ---------------------------------------- + # Writes into the project tree, so it falls under the same contract. + if (target / ".git").exists(): + before = commit_subjects(target) + cmd = [*CLI, "skill", "install", "--scope", "project"] + completed, _ = run(cmd, target, env, LOGS / "skill-install.log", args.timeout) + problems = exit_problems(completed) + repo_problems, detail = check_repo(target, "plonecli skills", before) + rec.record("skill-install", problems + repo_problems, detail) + + # --- create zope-setup ---------------------------------------------------- + zope_root = WORKSPACES / "create-zope" + zope_root.mkdir() + zope_target = zope_root / "gitzope" + cmd = [ + *CLI, + "create", + "zope-setup", + str(zope_target), + "--defaults", + "--allow-dirty", + *data_args(zope_data("gitzope")), + ] + completed, _ = run(cmd, ROOT, env, LOGS / "create-zope-setup.log", args.timeout) + problems = exit_problems(completed) + repo_problems, detail = check_repo(zope_target, "zope-setup", []) + rec.record("create-zope-setup", problems + repo_problems, detail) + + # --- create addon composite ---------------------------------------------- + composite_root = WORKSPACES / "create-composite" + composite_root.mkdir() + composite_target = composite_root / "collective.gitcomposite" + # plone_version is omitted: the backend template asks for a minor version and + # zope-setup for a full version, so one value cannot satisfy both choices. + merged = { + **backend_data("collective.gitcomposite"), + **zope_data("collective.gitcomposite"), + } + data = {key: value for key, value in merged.items() if key != "plone_version"} + cmd = [ + *CLI, + "create", + "addon", + str(composite_target), + "--defaults", + "--allow-dirty", + *data_args(data), + ] + completed, _ = run(cmd, ROOT, env, LOGS / "create-addon.log", args.timeout) + problems = exit_problems(completed) + # The composite applies backend_addon then zope-setup, so the newest commit + # names the last step and the history must hold one commit per step. + repo_problems, detail = check_repo(composite_target, "zope-setup", []) + if len(detail["commits"]) < 2: + repo_problems.append( + f"expected one commit per composite step, got {detail['commits']}" + ) + rec.record("create-addon-composite", problems + repo_problems, detail) + + # --- setup ---------------------------------------------------------------- + print("setup case") + setup_root = WORKSPACES / "setup" + setup_root.mkdir() + setup_target = setup_root / "collective.gitsetup" + cmd = [ + *CLI, + "create", + "backend_addon", + str(setup_target), + "--defaults", + "--allow-dirty", + *data_args(backend_data("collective.gitsetup")), + ] + completed, _ = run(cmd, ROOT, env, LOGS / "setup-create.log", args.timeout) + if completed.returncode != 0: + rec.record("setup", [f"parent create failed exit={completed.returncode}"], {}) + else: + before = commit_subjects(setup_target) + cmd = [ + *CLI, + "setup", + "--defaults", + "--allow-dirty", + *data_args(zope_data("collective.gitsetup")), + ] + completed, _ = run(cmd, setup_target, env, LOGS / "setup.log", args.timeout) + problems = exit_problems(completed) + repo_problems, detail = check_repo(setup_target, "zope-setup", before) + rec.record("setup", problems + repo_problems, detail) + + # --- add zope_instance into the standalone Zope project ------------------- + if not args.quick and (zope_target / ".git").exists(): + before = commit_subjects(zope_target) + cmd = [ + *CLI, + "add", + "zope_instance", + "--defaults", + "--allow-dirty", + *data_args( + { + "instance_name": "git-extra", + "port": 8188, + "base_path": "runtime-extra", + "db_storage": "instance", + "initial_zope_username": "runner", + "initial_user_password": "not-a-real-password", + } + ), + ] + completed, _ = run( + cmd, zope_target, env, LOGS / "add-zope_instance.log", args.timeout + ) + problems = exit_problems(completed) + repo_problems, detail = check_repo(zope_target, "zope_instance", before) + rec.record("add-zope_instance", problems + repo_problems, detail) + + # --- add ---------------------------------------------------- + if not args.quick: + print("add cases") + parent_root = WORKSPACES / "add-parent" + parent_root.mkdir() + parent = parent_root / "collective.gitparent" + cmd = [ + *CLI, + "create", + "backend_addon", + str(parent), + "--defaults", + "--allow-dirty", + *data_args(backend_data("collective.gitparent")), + ] + completed, _ = run(cmd, ROOT, env, LOGS / "add-parent.log", args.timeout) + if completed.returncode != 0: + rec.record( + "add-parent", [f"parent create failed exit={completed.returncode}"], {} + ) + else: + for index, template in enumerate(BACKEND_SUBTEMPLATES): + case_root = WORKSPACES / "add" / template + case_root.mkdir(parents=True) + project = case_root / parent.name + shutil.copytree(parent, project, symlinks=True) + before = commit_subjects(project) + cmd = [ + *CLI, + "add", + template, + "--defaults", + "--allow-dirty", + *data_args(individual_data(template, index)), + ] + completed, _ = run( + cmd, project, env, LOGS / f"add-{template}.log", args.timeout + ) + problems = exit_problems(completed) + repo_problems, detail = check_repo(project, template, before) + rec.record(f"add-{template}", problems + repo_problems, detail) + + elapsed = time.monotonic() - started + failures = sum(r["status"] != "passed" for r in rec.results) + report = { + "elapsed_seconds": round(elapsed, 1), + "provenance": provenance(), + "counts": { + "total": len(rec.results), + "passed": len(rec.results) - failures, + "failed": failures, + }, + "cases": rec.results, + } + RESULTS.mkdir(parents=True, exist_ok=True) + (RESULTS / "report.json").write_text( + json.dumps(report, indent=2, sort_keys=True) + "\n", encoding="utf-8" + ) + lines = [ + "# plonecli auto-commit evaluation", + "", + f"{report['counts']['passed']} passed, {failures} failed " + f"in {report['elapsed_seconds']}s.", + "", + "| case | status | problems |", + "| --- | --- | --- |", + ] + for case in rec.results: + problems = "; ".join(case["problems"]).replace("|", "\\|") or "-" + lines.append(f"| `{case['id']}` | {case['status']} | {problems} |") + (RESULTS / "report.md").write_text("\n".join(lines) + "\n", encoding="utf-8") + + print(f"Wrote {RESULTS / 'report.md'}; failures={failures}") + return 1 if failures else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/plonecli/cli.py b/plonecli/cli.py index bb53390..5b552b9 100644 --- a/plonecli/cli.py +++ b/plonecli/cli.py @@ -14,7 +14,7 @@ from plonecli.config import load_config, save_config from plonecli.exceptions import NoSuchValue, NotInPackageError -from plonecli.git import dirty_files +from plonecli.git import commit_paths, dirty_files from plonecli.output import echo from plonecli.project import find_project_root from plonecli.registry import TemplateRegistry @@ -352,6 +352,7 @@ def create(context, template, name, data, data_file, defaults, no_git, allow_dir context.obj["target_dir"] = name context.obj["chain_defaults"] = defaults context.obj["chain_allow_dirty"] = allow_dirty + context.obj["chain_no_git"] = no_git # Chained commands share the group context created before generation. Refresh # it now so ``create ... setup`` and similar cross-context chains work. context.obj["project"] = find_project_root(Path(name)) @@ -406,13 +407,20 @@ def add(context, template, data, data_file, defaults, no_git, allow_dirty): @cli.command(cls=InterspersedCommand) @template_run_options +@click.option( + "--no-git", + "no_git", + is_flag=True, + help="Do not auto-commit the changes made by zope-setup.", +) @click.pass_context -def setup(context, data, data_file, defaults, allow_dirty): +def setup(context, data, data_file, defaults, no_git, allow_dirty): """Run zope-setup inside an existing backend_addon""" # Click's chained parser may bind shared trailing flags to the preceding # create command. Carry those execution flags across the chain. defaults = defaults or context.obj.get("chain_defaults", False) allow_dirty = allow_dirty or context.obj.get("chain_allow_dirty", False) + no_git = no_git or context.obj.get("chain_no_git", False) project = context.obj.get("project") if project is None: raise NotInPackageError(context.command.name) @@ -428,14 +436,17 @@ def setup(context, data, data_file, defaults, allow_dirty): config = context.obj["config"] answers = _collect_data(data_file, data) echo("\nRunning zope-setup...", fg="green", reverse=True) - run_create( + committed = run_create( "zope-setup", str(project.root_folder), config, data=answers, defaults=defaults, + git_commit=config.auto_commit and not no_git, overwrite=True, ) + if committed: + echo(f" Committed: {committed}", fg="green") TASKS_FILE = "tasks.py" @@ -697,8 +708,14 @@ def update(context): help="Copy files for the .claude alias instead of symlinking.", ) @click.option("--force", is_flag=True, help="Overwrite an existing installation.") +@click.option( + "--no-git", + "no_git", + is_flag=True, + help="Do not auto-commit the skills installed with --scope project.", +) @click.pass_context -def skill(context, action, scope, copy_only, force): +def skill(context, action, scope, copy_only, force, no_git): """Install/update the bundled Agent Skills for AI coding agents. Drops each bundled skill (Agent Skills open standard) into @@ -736,7 +753,9 @@ def skill(context, action, scope, copy_only, force): except FileNotFoundError as e: # pragma: no cover - packaging guard raise click.ClickException(str(e)) from e - verb = "Updated" if action == "update" else "Installed" + verb, commit_verb = ( + ("Updated", "Update") if action == "update" else ("Installed", "Add") + ) echo(f"\n{verb} plonecli skills under {base}", fg="green", reverse=True) for act in actions: if act.kind == "symlink": @@ -744,6 +763,20 @@ def skill(context, action, scope, copy_only, force): else: echo(f" copied {act.target}") + # Project-scope installs write into the project tree, so they fall under the + # same auto-commit contract as create/add: leaving them untracked makes the + # next ``plonecli add`` refuse to run on a dirty repository. + config = context.obj["config"] + if scope == "project" and config.auto_commit and not no_git: + committed = commit_paths( + base, + f"{commit_verb} plonecli skills", + config, + [str(act.target.relative_to(base)) for act in actions], + ) + if committed: + echo(f" Committed: {committed}", fg="green") + @cli.command(cls=InterspersedCommand) @click.argument( diff --git a/plonecli/git.py b/plonecli/git.py index 11acb66..0b64698 100644 --- a/plonecli/git.py +++ b/plonecli/git.py @@ -10,6 +10,7 @@ from __future__ import annotations import subprocess +from collections.abc import Sequence from pathlib import Path from plonecli.config import PlonecliConfig @@ -75,15 +76,80 @@ def _has_identity(path: Path) -> bool: return True -def _nothing_staged(path: Path) -> bool: - """Return True if the index has no staged changes to commit.""" - result = subprocess.run( - ["git", "diff", "--cached", "--quiet"], - cwd=str(path), - ) +def _nothing_staged(path: Path, paths: Sequence[str] = ()) -> bool: + """Return True if the index has no staged changes to commit. + + ``paths`` narrows the check to those pathspecs. + """ + cmd = ["git", "diff", "--cached", "--quiet"] + if paths: + cmd += ["--", *paths] + result = subprocess.run(cmd, cwd=str(path)) return result.returncode == 0 +def _commit_command(path: Path, message: str, config: PlonecliConfig) -> list[str]: + """The ``git commit`` argv, with an identity fallback when git has none.""" + cmd = ["git"] + if not _has_identity(path): + cmd += [ + "-c", + f"user.name={config.author_name}", + "-c", + f"user.email={config.author_email}", + ] + return cmd + ["commit", "-m", message] + + +def commit_paths( + target_dir: str | Path, + message: str, + config: PlonecliConfig, + paths: Sequence[str], +) -> str | None: + """Commit only ``paths`` (relative to ``target_dir``) in an existing repo. + + Unlike :func:`commit_template_changes` this never runs ``git init`` and + never stages anything outside ``paths``, so a command that writes a few + known files leaves the rest of the working tree untouched. + + Returns: + The commit message if a commit was made, otherwise ``None`` (no + repository, nothing written, or nothing changed). + """ + target = Path(target_dir) + if not is_git_repo(target): + return None + + existing = [p for p in paths if (target / p).exists()] + if not existing: + return None + + try: + subprocess.run( + ["git", "add", "--", *existing], + cwd=str(target), + check=True, + capture_output=True, + ) + if _nothing_staged(target, existing): + return None + subprocess.run( + [*_commit_command(target, message, config), "--", *existing], + cwd=str(target), + check=True, + capture_output=True, + ) + return message + except (subprocess.CalledProcessError, FileNotFoundError) as exc: + error( + f"ERROR: git auto-commit failed ({exc}).\n" + f"The files written in {target} are uncommitted - commit them " + f"yourself." + ) + return None + + def commit_template_changes( target_dir: str | Path, template_name: str, @@ -136,17 +202,8 @@ def commit_template_changes( else: message = f"Add {template_name} template" - commit_cmd = ["git"] - if not _has_identity(target): - commit_cmd += [ - "-c", - f"user.name={config.author_name}", - "-c", - f"user.email={config.author_email}", - ] - commit_cmd += ["commit", "-m", message] subprocess.run( - commit_cmd, + _commit_command(target, message, config), cwd=str(target), check=True, capture_output=True, diff --git a/tests/helpers.py b/tests/helpers.py index 082e380..027e585 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -1,7 +1,24 @@ """Shared test helpers.""" +import os +from pathlib import Path from unittest.mock import MagicMock +import pytest + +from plonecli.config import PlonecliConfig + +REPO_ROOT = Path(__file__).resolve().parent.parent + +# Where this repository keeps its development checkout of copier-templates. +DEV_TEMPLATES_DIR = REPO_ROOT / "develop" / "plone" / "src" / "copier-templates" + +# Devcontainer locations kept as a last resort. +LEGACY_TEMPLATES_DIRS = [ + "/home/node/develop/plone/src/copier-templates", + "/home/node/.copier-templates/plone-copier-templates", +] + def project_at(path, project_type="backend_addon"): """A stand-in ProjectContext rooted at ``path``.""" @@ -12,3 +29,32 @@ def project_at(path, project_type="backend_addon"): package_folder="test/addon", settings={}, ) + + +def _templates_candidates(): + """Ordered candidate locations for a copier-templates checkout.""" + return [ + os.environ.get("PLONECLI_TEMPLATES_DIR"), + DEV_TEMPLATES_DIR, + PlonecliConfig().templates_dir, + *LEGACY_TEMPLATES_DIRS, + ] + + +def find_templates_checkout(marker="backend_addon/copier.yml"): + """First candidate checkout containing ``marker``, or None. + + Safe to call at collection time: it never skips. + """ + for candidate in _templates_candidates(): + if candidate and (Path(candidate) / marker).exists(): + return Path(candidate) + return None + + +def templates_checkout(marker="backend_addon/copier.yml"): + """Like :func:`find_templates_checkout`, but skips the test when missing.""" + path = find_templates_checkout(marker) + if path is None: + pytest.skip(f"No copier-templates checkout with {marker} available") + return path diff --git a/tests/test_addon_composite_integration.py b/tests/test_addon_composite_integration.py index e45b22b..212f0cf 100644 --- a/tests/test_addon_composite_integration.py +++ b/tests/test_addon_composite_integration.py @@ -17,7 +17,6 @@ from __future__ import annotations -import os import shutil from pathlib import Path from unittest.mock import patch @@ -27,21 +26,7 @@ from plonecli.cli import cli from plonecli.config import PlonecliConfig - - -def _templates_dir() -> Path: - """Locate a usable copier-templates clone or skip.""" - env_dir = os.environ.get("PLONECLI_TEMPLATES_DIR") - candidates = [env_dir] if env_dir else [] - candidates += [ - PlonecliConfig().templates_dir, - "/home/node/develop/plone/src/copier-templates", - "/home/node/.copier-templates/plone-copier-templates", - ] - for c in candidates: - if c and (Path(c) / "addon" / "copier.yml").exists(): - return Path(c) - pytest.skip("No copier-templates clone with the addon composite available") +from tests.helpers import templates_checkout @pytest.mark.integration @@ -49,7 +34,7 @@ def test_create_addon_generates_initial_instance(tmp_path: Path) -> None: if shutil.which("uv") is None: pytest.skip("uv is required for the integration test") - templates_dir = _templates_dir() + templates_dir = templates_checkout("addon/copier.yml") config = PlonecliConfig(templates_dir=str(templates_dir)) project_dir = tmp_path / "my.tool" diff --git a/tests/test_all_templates_data.py b/tests/test_all_templates_data.py index 455a011..21b37ba 100644 --- a/tests/test_all_templates_data.py +++ b/tests/test_all_templates_data.py @@ -19,7 +19,6 @@ from __future__ import annotations -import os import shutil from pathlib import Path @@ -29,9 +28,7 @@ from plonecli.config import PlonecliConfig from plonecli.project import find_project_root from plonecli.templates import run_add, run_create - -DEV_TEMPLATES_DIR = Path("/home/node/develop/plone/src/copier-templates") -FALLBACK_TEMPLATES_DIR = Path("/home/node/.copier-templates/plone-copier-templates") +from tests.helpers import find_templates_checkout, templates_checkout # Required-but-defaultless questions, plus anything whose default is a Jinja # expression we cannot render here, get a concrete, validator-passing value. @@ -45,24 +42,6 @@ } -def _find_templates_dir() -> Path | None: - env_dir = os.environ.get("PLONECLI_TEMPLATES_DIR") - if env_dir and Path(env_dir).exists(): - return Path(env_dir) - if DEV_TEMPLATES_DIR.exists(): - return DEV_TEMPLATES_DIR - if FALLBACK_TEMPLATES_DIR.exists(): - return FALLBACK_TEMPLATES_DIR - return None - - -def _templates_dir() -> Path: - templates_dir = _find_templates_dir() - if templates_dir is None: - pytest.skip("No copier-templates checkout available") - return templates_dir - - def _all_templates(): """Yield (name, copier.yml-dict) for every real (non-composite) template. @@ -70,7 +49,7 @@ def _all_templates(): only valid inside a test). When no templates checkout is available it yields nothing, leaving the parametrized tests with an empty parameter set. """ - templates_dir = _find_templates_dir() + templates_dir = find_templates_checkout() if templates_dir is None: return for cfg in sorted(templates_dir.glob("*/copier.yml")): @@ -198,10 +177,10 @@ def _generate_main(name, template_data, config, tmp_path) -> Path: @pytest.fixture(scope="module") def _parents(tmp_path_factory): """Generate one backend_addon and one zope-setup parent to add subs into.""" - config = PlonecliConfig(templates_dir=str(_templates_dir())) + config = PlonecliConfig(templates_dir=str(templates_checkout())) parents = {} for main in ("backend_addon", "zope-setup"): - cfg = _templates_dir() / main / "copier.yml" + cfg = templates_checkout() / main / "copier.yml" if not cfg.exists(): continue data = yaml.safe_load(cfg.read_text()) @@ -228,7 +207,7 @@ def test_template_generates_non_interactively(name, template_data, _parents, tmp if shutil.which("uv") is None: pytest.skip("uv is required to run template post-copy hooks") - config = PlonecliConfig(templates_dir=str(_templates_dir())) + config = PlonecliConfig(templates_dir=str(templates_checkout())) meta = template_data.get("_plonecli", {}) if meta.get("type") == "main": diff --git a/tests/test_git.py b/tests/test_git.py index e84a0bd..f4ccfbb 100644 --- a/tests/test_git.py +++ b/tests/test_git.py @@ -4,7 +4,12 @@ from unittest.mock import patch from plonecli.config import PlonecliConfig -from plonecli.git import commit_template_changes, dirty_files, is_git_repo +from plonecli.git import ( + commit_paths, + commit_template_changes, + dirty_files, + is_git_repo, +) def _log(path): @@ -137,7 +142,7 @@ def test_parent_repository_is_not_used_for_generated_project(tmp_path): (tmp_path / "outer.py").write_text("dirty outer\n") project = tmp_path / "generated" project.mkdir() - (project / "pyproject.toml").write_text("[project]\nname = \"generated\"\n") + (project / "pyproject.toml").write_text('[project]\nname = "generated"\n') assert is_git_repo(project) is False assert dirty_files(project) == ([], []) @@ -165,3 +170,38 @@ def test_dirty_files_reports_modified_and_untracked(tmp_path): modified, untracked = dirty_files(tmp_path) assert "a.py" in modified assert "new.py" in untracked + + +def test_commit_paths_commits_only_the_given_paths(tmp_path): + config = PlonecliConfig() + (tmp_path / "a.py").write_text("a\n") + commit_template_changes(tmp_path, "backend_addon", config, is_subtemplate=False) + + (tmp_path / "skills").mkdir() + (tmp_path / "skills" / "SKILL.md").write_text("skill\n") + (tmp_path / "unrelated.py").write_text("mine\n") + + msg = commit_paths(tmp_path, "Add plonecli skills", config, ["skills"]) + + assert msg == "Add plonecli skills" + assert _log(tmp_path)[0] == "Add plonecli skills" + # The user's own work is left alone, not swept into the commit. + assert dirty_files(tmp_path) == ([], ["unrelated.py"]) + + +def test_commit_paths_noop_without_repo_or_changes(tmp_path): + config = PlonecliConfig() + (tmp_path / "skills").mkdir() + + # Never initialises a repository of its own. + assert commit_paths(tmp_path, "Add plonecli skills", config, ["skills"]) is None + assert is_git_repo(tmp_path) is False + + (tmp_path / "a.py").write_text("a\n") + commit_template_changes(tmp_path, "backend_addon", config, is_subtemplate=False) + + # Nothing written under the paths, and nothing changed afterwards. + assert commit_paths(tmp_path, "Add plonecli skills", config, ["missing"]) is None + (tmp_path / "skills" / "SKILL.md").write_text("skill\n") + assert commit_paths(tmp_path, "Add plonecli skills", config, ["skills"]) is not None + assert commit_paths(tmp_path, "Add plonecli skills", config, ["skills"]) is None diff --git a/tests/test_plonecli.py b/tests/test_plonecli.py index 102f362..74d7410 100644 --- a/tests/test_plonecli.py +++ b/tests/test_plonecli.py @@ -204,6 +204,38 @@ def test_create_then_setup_chain_refreshes_project( assert mock_run_create.call_args_list[1].args[1] == str(target) +@patch("plonecli.cli.find_project_root") +@patch("plonecli.cli.load_config") +@patch("plonecli.cli.run_create") +@patch("plonecli.cli.ensure_templates_cloned") +def test_create_then_setup_chain_carries_no_git( + mock_ensure, + mock_run_create, + mock_config, + mock_project, + runner, + tmp_path, +): + """Click binds a trailing ``--no-git`` to create; setup must honour it too.""" + _make_template(tmp_path, "backend_addon", {"type": "main"}) + target = tmp_path / "my.addon" + mock_config.return_value = MagicMock(templates_dir=str(tmp_path), auto_commit=True) + mock_project.side_effect = lambda start=None: ( + project_at(target) if start is not None else None + ) + + result = runner.invoke( + cli, + ["create", "backend_addon", str(target), "--defaults", "--no-git", "setup"], + ) + + assert result.exit_code == 0, result.output + assert [c.kwargs["git_commit"] for c in mock_run_create.call_args_list] == [ + False, + False, + ] + + @patch("plonecli.cli.find_project_root", return_value=None) @patch("plonecli.cli.load_config") def test_create_unknown_template(mock_config, mock_project, runner, tmp_path): diff --git a/tests/test_setup_command.py b/tests/test_setup_command.py index a43153a..a9a4027 100644 --- a/tests/test_setup_command.py +++ b/tests/test_setup_command.py @@ -155,3 +155,51 @@ def test_setup_allow_dirty_proceeds( assert result.exit_code == 0, result.output mock_run_create.assert_called_once() + + +@patch("plonecli.cli.find_project_root") +@patch("plonecli.cli.load_config") +@patch("plonecli.cli.run_create") +def test_setup_commits_by_default( + mock_run_create, mock_config, mock_project, runner, tmp_path +): + mock_config.return_value = MagicMock(templates_dir=str(tmp_path), auto_commit=True) + mock_project.return_value = project_at(tmp_path) + mock_run_create.return_value = "Add zope-setup template" + + result = runner.invoke(cli, ["setup"]) + + assert result.exit_code == 0, result.output + assert mock_run_create.call_args.kwargs["git_commit"] is True + assert "Committed: Add zope-setup template" in result.output + + +@patch("plonecli.cli.find_project_root") +@patch("plonecli.cli.load_config") +@patch("plonecli.cli.run_create") +def test_setup_no_git_skips_the_commit( + mock_run_create, mock_config, mock_project, runner, tmp_path +): + mock_config.return_value = MagicMock(templates_dir=str(tmp_path), auto_commit=True) + mock_project.return_value = project_at(tmp_path) + + result = runner.invoke(cli, ["setup", "--no-git"]) + + assert result.exit_code == 0, result.output + assert mock_run_create.call_args.kwargs["git_commit"] is False + + +@patch("plonecli.cli.find_project_root") +@patch("plonecli.cli.load_config") +@patch("plonecli.cli.run_create") +def test_setup_respects_auto_commit_false( + mock_run_create, mock_config, mock_project, runner, tmp_path +): + """``auto_commit = false`` must silence setup like it does create and add.""" + mock_config.return_value = MagicMock(templates_dir=str(tmp_path), auto_commit=False) + mock_project.return_value = project_at(tmp_path) + + result = runner.invoke(cli, ["setup"]) + + assert result.exit_code == 0, result.output + assert mock_run_create.call_args.kwargs["git_commit"] is False diff --git a/tests/test_skill.py b/tests/test_skill.py index c76fbad..6ebafd2 100644 --- a/tests/test_skill.py +++ b/tests/test_skill.py @@ -1,11 +1,32 @@ """Tests for the `plonecli skill` command and the skill installer.""" +import subprocess +from pathlib import Path from unittest.mock import MagicMock, patch import pytest from plonecli import skill_installer from plonecli.cli import cli +from plonecli.git import dirty_files + + +def _git(path, *args): + subprocess.run(["git", "-C", str(path), *args], check=True, capture_output=True) + + +def _init_repo(path): + subprocess.run(["git", "init", "-q", str(path)], check=True) + _git(path, "config", "user.name", "Test") + _git(path, "config", "user.email", "test@example.org") + + +def _log(path): + return subprocess.run( + ["git", "-C", str(path), "log", "--pretty=%s"], + capture_output=True, + text=True, + ).stdout.splitlines() def test_bundled_skills_present(): @@ -131,3 +152,94 @@ def test_cli_skill_install_twice_errors(mock_config, mock_project, runner, tmp_p result = runner.invoke(cli, ["skill", "install", "--scope", "project"]) assert result.exit_code != 0 assert "already installed" in result.output + + +def _project_config(**kwargs): + return MagicMock( + templates_dir="/tmp/nonexistent", + author_name="Test", + author_email="test@example.org", + **kwargs, + ) + + +@patch("plonecli.cli.find_project_root", return_value=None) +@patch("plonecli.cli.load_config") +def test_cli_skill_install_project_scope_commits( + mock_config, mock_project, runner, tmp_path +): + """A project install must leave the repo clean, not blocking the next add.""" + mock_config.return_value = _project_config(auto_commit=True) + with runner.isolated_filesystem(temp_dir=tmp_path) as fs: + _init_repo(fs) + result = runner.invoke(cli, ["skill", "install", "--scope", "project"]) + + assert result.exit_code == 0, result.output + assert "Committed: Add plonecli skills" in result.output + assert _log(fs) == ["Add plonecli skills"] + assert dirty_files(Path(fs)) == ([], []) + + +@patch("plonecli.cli.find_project_root", return_value=None) +@patch("plonecli.cli.load_config") +def test_cli_skill_update_project_scope_commits( + mock_config, mock_project, runner, tmp_path +): + mock_config.return_value = _project_config(auto_commit=True) + with runner.isolated_filesystem(temp_dir=tmp_path) as fs: + _init_repo(fs) + assert ( + runner.invoke(cli, ["skill", "install", "--scope", "project"]).exit_code + == 0 + ) + # A drifted installed copy, committed as it stands: the update has + # something real to restore and therefore something to commit. + (Path(fs) / ".agents" / "skills" / "plonecli" / "SKILL.md").write_text("stale") + _git(fs, "commit", "-qam", "Local edit") + + result = runner.invoke(cli, ["skill", "update", "--scope", "project"]) + + assert result.exit_code == 0, result.output + assert _log(fs)[0] == "Update plonecli skills" + assert dirty_files(Path(fs)) == ([], []) + + +@pytest.mark.parametrize( + "args,config_kwargs", + [ + (["--no-git"], {"auto_commit": True}), + ([], {"auto_commit": False}), + ], +) +@patch("plonecli.cli.find_project_root", return_value=None) +@patch("plonecli.cli.load_config") +def test_cli_skill_install_can_opt_out_of_the_commit( + mock_config, mock_project, runner, tmp_path, args, config_kwargs +): + mock_config.return_value = _project_config(**config_kwargs) + with runner.isolated_filesystem(temp_dir=tmp_path) as fs: + _init_repo(fs) + result = runner.invoke(cli, ["skill", "install", "--scope", "project", *args]) + + assert result.exit_code == 0, result.output + assert _log(fs) == [] + assert dirty_files(Path(fs)) == ([], [".agents/", ".claude/"]) + + +@patch("plonecli.cli.find_project_root", return_value=None) +@patch("plonecli.cli.load_config") +def test_cli_skill_install_user_scope_does_not_commit( + mock_config, mock_project, runner, tmp_path, monkeypatch +): + """User scope writes into ``$HOME``; that is not a plonecli project.""" + mock_config.return_value = _project_config(auto_commit=True) + home = tmp_path / "home" + home.mkdir() + _init_repo(home) + monkeypatch.setattr(Path, "home", classmethod(lambda cls: home)) + + with runner.isolated_filesystem(temp_dir=tmp_path): + result = runner.invoke(cli, ["skill", "install"]) + + assert result.exit_code == 0, result.output + assert _log(home) == [] diff --git a/tests/test_templates_checkout.py b/tests/test_templates_checkout.py new file mode 100644 index 0000000..30fba96 --- /dev/null +++ b/tests/test_templates_checkout.py @@ -0,0 +1,101 @@ +"""Tests for locating a copier-templates checkout in the test suite. + +The integration tests are worthless if they cannot find the checkout: pytest +reports them as passed/skipped either way, so a wrong lookup silently empties +the sweep instead of failing. +""" + +from pathlib import Path + +import pytest + +from tests import helpers +from tests.helpers import find_templates_checkout, templates_checkout + +MARKER = "backend_addon/copier.yml" + + +def _fake_checkout(path: Path, marker: str = MARKER) -> Path: + (path / marker).parent.mkdir(parents=True, exist_ok=True) + (path / marker).write_text("{}\n") + return path + + +def _isolate(monkeypatch, tmp_path): + """Point every non-explicit candidate at somewhere that does not exist.""" + monkeypatch.delenv("PLONECLI_TEMPLATES_DIR", raising=False) + monkeypatch.setattr(helpers, "DEV_TEMPLATES_DIR", tmp_path / "missing-dev") + monkeypatch.setattr(helpers, "LEGACY_TEMPLATES_DIRS", []) + monkeypatch.setattr( + helpers, + "PlonecliConfig", + lambda: type("C", (), {"templates_dir": str(tmp_path / "missing-config")})(), + ) + + +def test_env_var_wins(monkeypatch, tmp_path): + _isolate(monkeypatch, tmp_path) + env_dir = _fake_checkout(tmp_path / "env") + _fake_checkout(tmp_path / "dev") + monkeypatch.setattr(helpers, "DEV_TEMPLATES_DIR", tmp_path / "dev") + monkeypatch.setenv("PLONECLI_TEMPLATES_DIR", str(env_dir)) + + assert find_templates_checkout() == env_dir + + +def test_repo_development_checkout_is_a_candidate(monkeypatch, tmp_path): + """AGENTS.md puts the checkout at develop/plone/src/copier-templates.""" + _isolate(monkeypatch, tmp_path) + dev_dir = _fake_checkout(tmp_path / "dev") + monkeypatch.setattr(helpers, "DEV_TEMPLATES_DIR", dev_dir) + + assert find_templates_checkout() == dev_dir + + +def test_repo_development_path_matches_agents_md(): + assert ( + helpers.DEV_TEMPLATES_DIR + == helpers.REPO_ROOT / "develop" / "plone" / "src" / "copier-templates" + ) + + +def test_configured_clone_is_a_candidate(monkeypatch, tmp_path): + _isolate(monkeypatch, tmp_path) + clone = _fake_checkout(tmp_path / "clone") + monkeypatch.setattr( + helpers, + "PlonecliConfig", + lambda: type("C", (), {"templates_dir": str(clone)})(), + ) + + assert find_templates_checkout() == clone + + +def test_marker_must_be_present(monkeypatch, tmp_path): + """A directory without the requested template is not a usable checkout.""" + _isolate(monkeypatch, tmp_path) + checkout = _fake_checkout(tmp_path / "backend-only") + monkeypatch.setenv("PLONECLI_TEMPLATES_DIR", str(checkout)) + + assert find_templates_checkout() == checkout + assert find_templates_checkout("addon/copier.yml") is None + + +def test_returns_none_and_skips_when_nothing_is_available(monkeypatch, tmp_path): + _isolate(monkeypatch, tmp_path) + + assert find_templates_checkout() is None + with pytest.raises(pytest.skip.Exception): + templates_checkout() + + +def test_template_sweep_is_populated_when_a_checkout_exists(): + """The regression itself: a found checkout must yield real parameters.""" + from tests.test_all_templates_data import _all_templates + + if find_templates_checkout() is None: + pytest.skip("No copier-templates checkout available") + + names = [name for name, _ in _all_templates()] + assert "backend_addon" in names + assert len(names) > 5 diff --git a/tests/test_theme_barceloneta_integration.py b/tests/test_theme_barceloneta_integration.py index 593701d..00733c1 100644 --- a/tests/test_theme_barceloneta_integration.py +++ b/tests/test_theme_barceloneta_integration.py @@ -24,20 +24,7 @@ from plonecli.config import PlonecliConfig from plonecli.project import find_project_root from plonecli.templates import run_add, run_create - -DEV_TEMPLATES_DIR = Path("/home/node/develop/plone/src/copier-templates") -FALLBACK_TEMPLATES_DIR = Path("/home/node/.copier-templates/plone-copier-templates") - - -def _templates_dir() -> Path: - env_dir = os.environ.get("PLONECLI_TEMPLATES_DIR") - if env_dir and Path(env_dir).exists(): - return Path(env_dir) - if DEV_TEMPLATES_DIR.exists(): - return DEV_TEMPLATES_DIR - if FALLBACK_TEMPLATES_DIR.exists(): - return FALLBACK_TEMPLATES_DIR - pytest.skip("No copier-templates checkout available") +from tests.helpers import templates_checkout @pytest.mark.integration @@ -45,7 +32,7 @@ def test_theme_barceloneta_generates_and_tests_pass(tmp_path: Path) -> None: if shutil.which("uv") is None: pytest.skip("uv is required for the integration test") - templates_dir = _templates_dir() + templates_dir = templates_checkout() config = PlonecliConfig(templates_dir=str(templates_dir)) package_name = "collective.mythemetest" diff --git a/uv.lock b/uv.lock index 7ed6ecc..8e3ff8d 100644 --- a/uv.lock +++ b/uv.lock @@ -567,7 +567,7 @@ wheels = [ [[package]] name = "plonecli" -version = "7.0.0b15.dev0" +version = "7.0.0b16.dev0" source = { editable = "." } dependencies = [ { name = "click" },