diff --git a/.supertool.example.json b/.supertool.example.json index 7117cfe..b6e5689 100644 --- a/.supertool.example.json +++ b/.supertool.example.json @@ -2,7 +2,16 @@ "compact": false, "rtk": false, "parallel": 0, - "adviseForNewTest": false, + "_comment_advice": "Non-blocking [advice] lines emitted after a mutating op. Each rule: hooks_into (ops, default all mutating), match (path glob), when (new-file|existing-file|always), contains (regex over ADDED content), resolve/resolveFromValidator (subprocess emitting a would-be target via exit 3 + stderr), message ({target}/{path}/{op} interpolate). See docs/validators.md#advice.", + "advice": { + "newTest": { + "hooks_into": ["paste"], + "match": "*.php", + "when": "new-file", + "resolveFromValidator": true, + "message": "new class without test" + } + }, "validator_cache": true, "validator_cache_ttl_hours": 24, "_comment_defaults": "Default project/repo for payload ops that would otherwise require one. gl-issue-create reads 'gitlab_project' when the payload omits 'project'; gh-issue-create reads 'github_repo' when it omits 'repo'. Resolution order: explicit payload field > this config > the 'origin' git remote (host-matched). Omit this block to rely on git-remote auto-detect alone.", diff --git a/CHANGELOG.md b/CHANGELOG.md index 5415471..312db30 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **`gl-job` / `gh-job` gained a `:fail` suffix** — `gl-job:ID:fail` (alias `:errors`) prints only the matched failure blocks with no tail, the tight triage view. It applies the same per-job pattern table as the default mode (rector → diff lines, phpstan → 🪪/type markers, unit → `FAILURES!`/`Failed asserting`), so a red job shows just its actionable failures instead of the default's blocks-plus-80-line-tail. This names the discoverable front door for a behavior that previously only existed as the undocumented `:errors` mode on `gl-job` (and was entirely absent on `gh-job`); `:errors` stays as a back-compat alias. The default (no suffix), `:grep:PATTERN`, and `:raw` are unchanged. Closes [#326](https://github.com/Digital-Process-Tools/claude-supertool/issues/326). - **`grep` gained a `:no-auto-read` flag** — `grep:PATTERN:PATH:no-auto-read` suppresses the single-small-file auto-read so only the matching line(s) are emitted, mirroring `glob`'s existing flag. Default behavior (auto-read a concrete file < 20KB on a match) is unchanged. The flag is order-independent with `:count` and any `LIMIT`/`CONTEXT` args. Avoids silently dumping 10-18KB of unrequested file content when the caller only wants the hit. Closes [#320](https://github.com/Digital-Process-Tools/claude-supertool/issues/320). - **`git-commit` gained an `@file` payload route for multi-line messages** — `git-commit:@-` (stdin) or `git-commit:@msg.toml` reads a `message` field (subject + blank line + body) and an optional `paths` list, so a real subject+body commit no longer forces a drop to raw `git commit -F file` — which skipped the op's auto `Co-Authored-By` trailer and needed a follow-up `--amend`. The trailer is still appended on the payload route. The colon-CLI one-liner (`git-commit:::MESSAGE`) is unchanged. Mechanically this generalizes the `@file` field parser: a syntax token in a trailing `[...]` group is now optional, and a `...` token is variadic (a payload list expands into multiple positional args) — previously the parser leaked the `[`/`...]` brackets into the field names (`msg[`, `path...]`), so the route 404'd with `missing required field 'msg['`. Closes [#340](https://github.com/Digital-Process-Tools/claude-supertool/issues/340). +- **Generalized the new-class-without-test advisory into a config-driven `advice` block** — the hardcoded `adviseForNewTest` bool is replaced by a top-level `advice` map of named rules, each gated (all optional) by `hooks_into` (ops, default all mutating), `match` (path glob), `when` (`new-file`|`existing-file`|`always`), `contains` (regex over the content the op *added* — lines present after but not before, so it fires only when the op introduces the pattern), and `resolve`/`resolveFromValidator` (a subprocess emitting a would-be target via the same exit-3 + stderr contract). `message` is the rendered line, with `{target}`/`{path}`/`{op}` interpolation. The old single-purpose advisory was a dead end the moment a second nudge was wanted (e.g. "regenerate the XSD/cache after adding a component class or public attribute" — exactly the easy-to-forget step the XML editor depends on); the generic block lets a project add that as data instead of code. **Breaking:** the `adviseForNewTest: true` bool is removed — express it as an `advice.newTest` rule (`{"hooks_into": ["paste"], "match": "*.php", "when": "new-file", "resolveFromValidator": true, "message": "new class without test"}`); see `.supertool.example.json`. Closes [#347](https://github.com/Digital-Process-Tools/claude-supertool/issues/347). ### Fixed diff --git a/docs/configuration.md b/docs/configuration.md index 58e94ce..c48331d 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -176,13 +176,25 @@ Set `"compact": true` in `.supertool.json` to enable compact reads. When enabled Compact is disabled when using `grep=` filter or `offset` (editing needs exact lines). -## Advise on new class without test +## Advice — config-driven post-edit hints ```json -{ "adviseForNewTest": true } +{ + "advice": { + "newTest": { + "hooks_into": ["paste"], "match": "*.php", "when": "new-file", + "resolveFromValidator": true, "message": "new class without test" + }, + "newComponent": { + "hooks_into": ["edit", "paste"], "match": "*.php", + "contains": "extends \\w*ComponentBase|implements \\w*IComponent", + "message": "XSD/cache regen likely (dvsi_xsd + dvsi_clearcache)" + } + } +} ``` -Opt-in (default false). When set, a `paste` that creates a new `*.php` file with no resolvable sibling test gets a non-blocking `[advice]` line appended to the op output. It reuses a validator's `resolve` cmd to find the would-be test path. Full contract in [validators.md → resolve](validators.md#resolve--map-a-source-file-to-its-real-target). +Each rule appends a non-blocking `[advice]` line after a mutating op when it matches. Gates: `hooks_into` (ops, default all mutating), `match` (path glob), `when` (`new-file`|`existing-file`|`always`), `contains` (regex over the content the op *added*), and `resolve`/`resolveFromValidator` (a subprocess emitting a would-be target). Full field reference and the resolve contract in [validators.md → advice](validators.md#advice--config-driven-post-op-hints). ## Parallel execution diff --git a/docs/validators.md b/docs/validators.md index a4f890f..dd07a2a 100644 --- a/docs/validators.md +++ b/docs/validators.md @@ -241,30 +241,60 @@ By default a validator runs against the file the op touched. The optional `resol The exit code is **ignored** by the validator path. That is deliberate, and it's what makes the advisory below possible. -### adviseForNewTest — nag on a new class with no test +### advice — config-driven post-op hints -Autonomous flows that write only through supertool bypass the editor/git-hook reminders to write a test. With the top-level flag enabled: +Autonomous flows that write only through supertool bypass the editor/git-hook reminders to do follow-up work (write a test, regenerate the XSD, …). The top-level `advice` block emits non-blocking `[advice]` lines after a mutating op when a rule matches: ```json -// .supertool.json — opt-in, default false -{ "adviseForNewTest": true } +// .supertool.json +{ + "advice": { + "newTest": { + "hooks_into": ["paste"], + "match": "*.php", + "when": "new-file", + "resolveFromValidator": true, + "message": "new class without test" + }, + "newComponent": { + "hooks_into": ["edit", "paste"], + "match": "*.php", + "contains": "extends \\w*ComponentBase|implements \\w*IComponent", + "message": "XSD/cache regen likely (dvsi_xsd + dvsi_clearcache)" + } + } +} ``` -a `paste` that **creates a new `*.php` file** (it did not exist before) with no resolvable sibling test gets a non-blocking line appended to the op output: +Each rule is gated by (all optional): + +| field | meaning | default | +|-------|---------|---------| +| `hooks_into` | ops that trigger the rule | all mutating (`edit`, `paste`, `replace`, `replace_lines`, `vim`) | +| `match` | path glob | `*` | +| `when` | `new-file` \\| `existing-file` \\| `always` — gated on whether the file existed before the op | `always` | +| `contains` | regex tested against the **content the op added** (lines present after but not before) — fires only when the op *introduces* the pattern, not when the file already held it | — (no content gate) | +| `resolve` | a subprocess (a source→target resolver) emitting a would-be target | — | +| `resolveFromValidator` | reuse the first `resolve` cmd declared on a validator instead of duplicating it | `false` | +| `message` | the line shown; `{target}`/`{path}`/`{op}` interpolate. A message with no `{target}` gets ` — consider ` appended when a resolver produced one | `""` | + +A `paste` that **creates a new `*.php` file** with no resolvable sibling test (the `newTest` rule above) appends: ``` [advice] ℹ new class without test — consider tests/unit/SiX/FooTest.php ``` -It reuses the same `resolve` cmd declared on a validator, so the source→test path logic lives in exactly one place. Because the advisory needs the *would-be* target (which doesn't exist yet) while the validator path needs stdout to stay empty on a miss, the resolver signals the two cases on different channels: +#### The resolve contract + +A `resolve`/`resolveFromValidator` rule reuses the same `resolve` cmd declared on a validator, so the source→target path logic lives in exactly one place. Because the advisory needs the *would-be* target (which doesn't exist yet) while the validator path needs stdout to stay empty on a miss, the resolver signals the two cases on different channels: | case | stdout | stderr | exit | |------|--------|--------|------| -| test exists | test path | — | 0 | -| no test | *(empty)* | would-be target | 3 | +| target exists | target path | — | 0 | +| no target | *(empty)* | would-be target | 3 | -stdout stays empty on a miss → the validator still skips, unchanged. The miss target rides on **stderr**, flagged by **exit 3** → only the advisory reads it. Advisory only: it never blocks the write, and never fires on a rewrite of an existing file. +stdout stays empty on a miss → the validator still skips, unchanged. The miss target rides on **stderr**, flagged by **exit 3** → only the advisory reads it. Advisory only: it never blocks the write. ## Format-on-save diff --git a/supertool.py b/supertool.py index a60c57b..0af58f1 100755 --- a/supertool.py +++ b/supertool.py @@ -9319,31 +9319,43 @@ def _formatters_run_batch( return [_formatter_run_one(name, spec, path) for name, spec in applicable.items()] -def _advise_new_test(op: str, path: str, pre_existed: bool) -> str: - """Advisory (never blocks): nag when a `paste` creates a new source file - that has no sibling test. - - Opt-in via top-level config ``adviseForNewTest: true``. Reuses a validator's - ``resolve`` cmd (the source→test resolver) to find the would-be test target: - the resolver exits 3 and prints the target on stderr when no test exists - (stdout stays empty so the phpunit validator still skips). Returns an - ``[advice]`` block, or "" when not applicable. - """ - if pre_existed or op != "paste": - return "" - cfg = _load_config() - if not cfg.get("adviseForNewTest"): - return "" - if not path.endswith(".php"): - return "" - # Reuse the source→test resolver declared on a validator (e.g. phpunit). - resolve_cmd = None - for spec in (cfg.get("validators") or {}).values(): - if isinstance(spec, dict) and spec.get("resolve"): - resolve_cmd = spec["resolve"] - break - if not resolve_cmd: +_ADVICE_DEFAULT_OPS = ("edit", "paste", "replace", "replace_lines", "vim") + + +def _advice_added_text(path: str, pre_content: Optional[bytes]) -> str: + """Text the op introduced: lines in the current file absent from + ``pre_content``. When ``pre_content`` is None (no snapshot taken) the whole + current file is returned — correct for a freshly created file, slightly + broad for an in-place edit. Gates ``contains`` rules on what the op *added*, + not on what the file already held.""" + try: + with open(path, "rb") as f: + post = f.read() + except OSError: return "" + if pre_content is None: + return post.decode("utf-8", "replace") + # Multiset diff (not set): a line duplicated by the op counts as added even + # when an identical line already existed. Each post line consumes one pre + # occurrence; the leftovers are what the op introduced. + pre_counts: Dict[bytes, int] = {} + for ln in pre_content.splitlines(): + pre_counts[ln] = pre_counts.get(ln, 0) + 1 + added = [] + for ln in post.splitlines(): + if pre_counts.get(ln, 0) > 0: + pre_counts[ln] -= 1 + else: + added.append(ln) + return b"\n".join(added).decode("utf-8", "replace") + + +def _advice_resolve(resolve_cmd: str, path: str) -> Optional[str]: + """Run a rule's ``resolve`` subprocess (a source→target resolver). Returns + the target string (possibly empty) when the resolver signals "advice + applies" via exit 3 — the would-be target rides on stderr while stdout stays + empty so a validator reusing the same cmd still skips. Returns None to + suppress (exit 0 = target already exists, or any error).""" cmd = (resolve_cmd .replace("{supertool_dir}", _INSTALL_DIR) .replace("{python}", _python_token()) @@ -9356,16 +9368,117 @@ def _advise_new_test(op: str, path: str, pre_existed: bool) -> str: text=True, timeout=30, env=(_merged_env if _prefix_env else None)) except (subprocess.TimeoutExpired, OSError): - return "" + return None if r.returncode != 3: + return None + return r.stderr.strip().splitlines()[-1] if r.stderr.strip() else "" + + +def _resolve_cmd_from_validators(cfg: Dict[str, Any], + name: Optional[str] = None) -> Optional[str]: + """A ``resolve`` cmd declared on a validator — lets an advice rule reuse the + source→target resolver instead of duplicating it. ``name`` picks a specific + validator (unambiguous when several declare a resolver); without it, the + first validator that declares one wins.""" + validators = cfg.get("validators") or {} + if name: + spec = validators.get(name) + return spec.get("resolve") if isinstance(spec, dict) else None + for spec in validators.values(): + if isinstance(spec, dict) and spec.get("resolve"): + return spec["resolve"] + return None + + +def _eval_advice_rule(spec: Dict[str, Any], op: str, path: str, + pre_existed: bool, pre_content: Optional[bytes], + cfg: Dict[str, Any]) -> str: + """Evaluate one advice rule. Returns the rendered advice line, or "" when + the rule does not apply to this op/path/state.""" + if op not in (spec.get("hooks_into") or _ADVICE_DEFAULT_OPS): + return "" + glob = spec.get("match", "*") + if glob and not _match_glob(path, glob): + return "" + when = spec.get("when", "always") + if when == "new-file" and pre_existed: + return "" + if when == "existing-file" and not pre_existed: return "" - target = "" - if r.stderr.strip(): - target = r.stderr.strip().splitlines()[-1] - msg = f"{mark('ℹ')} new class without test" - if target: - msg += f" — consider {target}" - return "\n[advice]\n" + msg + "\n" + contains = spec.get("contains") + if contains: + try: + if not re.search(contains, _advice_added_text(path, pre_content)): + return "" + except re.error: + return "" + target = None + rfv = spec.get("resolveFromValidator") + if spec.get("resolve") or rfv: + resolve_cmd = spec.get("resolve") + if not resolve_cmd and rfv: + resolve_cmd = _resolve_cmd_from_validators( + cfg, rfv if isinstance(rfv, str) else None) + if not resolve_cmd: + return "" + target = _advice_resolve(resolve_cmd, path) + if target is None: + return "" + # Interpolate {path}/{op} first so a resolver-produced target can never be + # re-scanned for placeholders. strip() on the append branch drops the leading + # space left when the configured message is empty. + message = spec.get("message", "").replace("{path}", path).replace("{op}", op) + if "{target}" in message: + message = message.replace("{target}", target or "") + elif target: + message = f"{message} — consider {target}".strip() + return f"{mark('ℹ')} {message}".rstrip() + + +def _run_advice(op: str, path: str, pre_existed: bool, + pre_content: Optional[bytes] = None) -> str: + """Advisory (never blocks): emit config-driven hints after a mutating op. + + Rules live under the top-level ``advice`` config block. Each rule may gate + on ``hooks_into`` (ops, default all mutating), ``match`` (path glob), + ``when`` (new-file|existing-file|always), ``contains`` (regex over the + content the op *added*) and ``resolve``/``resolveFromValidator`` (a + subprocess emitting a would-be target via exit 3 + stderr). ``message`` is + the line shown; ``{target}``/``{path}``/``{op}`` interpolate, and a bare + ``{target}``-less message gets " — consider " appended when a + resolver produced one. Returns an ``[advice]`` block, or "" when nothing + applies.""" + cfg = _load_config() + rules = {name: spec for name, spec in (cfg.get("advice") or {}).items() + if isinstance(spec, dict)} + if not rules: + return "" + lines = [] + for spec in rules.values(): + line = _eval_advice_rule(spec, op, path, pre_existed, pre_content, cfg) + if line: + lines.append(line) + if not lines: + return "" + return "\n[advice]\n" + "\n".join(lines) + "\n" + + +def _advice_wants_pre(op: str, path: str) -> bool: + """True when a configured advice rule with a ``contains`` gate applies to + this op/path. The caller snapshots pre-edit bytes so the added-content diff + is exact even when no rollback/notifier would otherwise capture them — + without this, ``contains`` silently falls back to whole-file matching and + fires on content the op did not introduce.""" + for spec in (_load_config().get("advice") or {}).values(): + if not isinstance(spec, dict) or not spec.get("contains"): + continue + if op not in (spec.get("hooks_into") or _ADVICE_DEFAULT_OPS): + continue + glob = spec.get("match", "*") + if glob and not _match_glob(path, glob): + continue + return True + return False def _run_with_validators(op: str, parts: Any, do_op: Any) -> str: @@ -9425,7 +9538,7 @@ def _run_with_validators(op: str, parts: Any, do_op: Any) -> str: if not applicable_fmt and not applicable: # No validators/formatters — still need pre_content for notifier diff view pre_for_notif = None - if applicable_notif and os.path.isfile(path): + if (applicable_notif or _advice_wants_pre(op, path)) and os.path.isfile(path): try: with open(path, "rb") as f: pre_for_notif = f.read() @@ -9437,14 +9550,15 @@ def _run_with_validators(op: str, parts: Any, do_op: Any) -> str: return body for _srv in _new_file_servers: _mcp_stop_server(_srv) - return body + _advise_new_test(op, path, _pre_existed) + return body + _run_advice(op, path, _pre_existed, pre_for_notif) needs_rollback = any(v.get("rollback_on_fail") for v in applicable.values()) needs_fmt_rollback = any(v.get("rollback_on_fail") for v in applicable_fmt.values()) # Capture pre_content for rollback AND/OR notifier diff view pre_content: Optional[bytes] = None - needs_pre = needs_rollback or needs_fmt_rollback or bool(applicable_notif) + needs_pre = (needs_rollback or needs_fmt_rollback or bool(applicable_notif) + or _advice_wants_pre(op, path)) if needs_pre and os.path.isfile(path): try: with open(path, "rb") as f: @@ -9524,7 +9638,7 @@ def _run_with_validators(op: str, parts: Any, do_op: Any) -> str: if applicable: suffix += "\n[validators]\n" + diff_out - return body + suffix + _advise_new_test(op, path, _pre_existed) + return body + suffix + _run_advice(op, path, _pre_existed, pre_content) # Filter sentinel: `@syntax` selects validators that declare `"syntax": true` diff --git a/tests/test_validators.py b/tests/test_validators.py index b0d3b11..55948b0 100644 --- a/tests/test_validators.py +++ b/tests/test_validators.py @@ -746,7 +746,7 @@ def test_phplint_adapter_no_arg_returns_schema_error() -> None: # --------------------------------------------------------------------------- -# _advise_new_test — paste-time "write a test" advisory +# _run_advice — config-driven post-op advisories (the `advice` block) # --------------------------------------------------------------------------- # resolve cmd that signals a miss: exit 3 + would-be target on stderr. @@ -757,55 +757,198 @@ def test_phplint_adapter_no_arg_returns_schema_error() -> None: # resolve cmd that signals a hit: exit 0 + test path on stdout. _RESOLVE_HIT = '{python} -c "import sys; sys.stdout.write(\'tests/unit/SiX/FooTest.php\')"' +# The newTest rule, expressed purely as config (the old adviseForNewTest bool). +_NEW_TEST_RULE = { + "hooks_into": ["paste"], + "match": "*.php", + "when": "new-file", + "resolveFromValidator": True, + "message": "new class without test", +} -def _set_advice_config(enabled: bool, resolve: str | None = _RESOLVE_MISS) -> None: + +def _set_advice(rules: dict, resolve: str | None = _RESOLVE_MISS) -> None: validators: dict = {} if resolve is not None: validators["phpunit"] = {"cmd": "noop", "resolve": resolve} - supertool._CONFIG = {"adviseForNewTest": enabled, "validators": validators} + supertool._CONFIG = {"advice": rules, "validators": validators} supertool._CONFIG_CHECKED = True -def test_advise_emitted_for_new_class_without_test() -> None: - _set_advice_config(True) - out = supertool._advise_new_test("paste", "Dvsi/.../Foo.class.php", pre_existed=False) +# --- newTest rule (resolveFromValidator) ------------------------------------ + +def test_newtest_rule_emits_for_new_class_without_test() -> None: + _set_advice({"newTest": _NEW_TEST_RULE}) + out = supertool._run_advice("paste", "Dvsi/.../Foo.class.php", pre_existed=False) assert "[advice]" in out assert "new class without test" in out assert "tests/unit/SiX/FooTest.php" in out -def test_advise_silent_when_flag_disabled() -> None: - _set_advice_config(False) - assert supertool._advise_new_test("paste", "Foo.class.php", pre_existed=False) == "" +def test_advice_silent_when_no_advice_block() -> None: + _set_advice({}) + assert supertool._run_advice("paste", "Foo.class.php", pre_existed=False) == "" + + +def test_advice_block_uses_real_newlines_not_literal() -> None: + _set_advice({"n": {"hooks_into": ["paste"], "message": "hi"}}, resolve=None) + out = supertool._run_advice("paste", "Foo.class.php", pre_existed=False) + assert out.startswith("\n[advice]\n") + assert out.endswith("\n") + assert "\\n" not in out # literal backslash-n must never appear + + +def test_newtest_silent_when_file_already_existed() -> None: + _set_advice({"newTest": _NEW_TEST_RULE}) + assert supertool._run_advice("paste", "Foo.class.php", pre_existed=True) == "" + + +def test_newtest_silent_for_non_paste_op() -> None: + _set_advice({"newTest": _NEW_TEST_RULE}) + assert supertool._run_advice("edit", "Foo.class.php", pre_existed=False) == "" + + +def test_newtest_silent_for_non_php_file() -> None: + _set_advice({"newTest": _NEW_TEST_RULE}) + assert supertool._run_advice("paste", "styles.scss", pre_existed=False) == "" + + +def test_newtest_silent_when_test_exists() -> None: + _set_advice({"newTest": _NEW_TEST_RULE}, resolve=_RESOLVE_HIT) + assert supertool._run_advice("paste", "Foo.class.php", pre_existed=False) == "" + +def test_newtest_silent_when_no_resolve_cmd() -> None: + _set_advice({"newTest": _NEW_TEST_RULE}, resolve=None) + assert supertool._run_advice("paste", "Foo.class.php", pre_existed=False) == "" -def test_advise_silent_when_file_already_existed() -> None: - _set_advice_config(True) - assert supertool._advise_new_test("paste", "Foo.class.php", pre_existed=True) == "" +def test_resolvefromvalidator_by_name_picks_the_right_resolver() -> None: + # A wrong resolver is declared first; the name disambiguates (the real + # ordering trap: phpstan-component's resolve precedes phpunit's in DVSI). + supertool._CONFIG = { + "advice": {"newTest": {**_NEW_TEST_RULE, "resolveFromValidator": "phpunit"}}, + "validators": { + "phpstan-component": {"cmd": "noop", "resolve": _RESOLVE_HIT}, + "phpunit": {"cmd": "noop", "resolve": _RESOLVE_MISS}, + }, + } + supertool._CONFIG_CHECKED = True + out = supertool._run_advice("paste", "Foo.class.php", pre_existed=False) + assert "new class without test" in out + assert "tests/unit/SiX/FooTest.php" in out -def test_advise_silent_for_non_paste_op() -> None: - _set_advice_config(True) - assert supertool._advise_new_test("edit", "Foo.class.php", pre_existed=False) == "" +# --- contains gate (matches added content, not the whole file) -------------- + +def test_contains_rule_emits_when_added_text_matches(tmp_path: Path) -> None: + f = tmp_path / "Widget.php" + f.write_text(" None: + f = tmp_path / "Plain.php" + f.write_text(" None: + # Pattern already present in pre_content → not "added" → no advice. + f = tmp_path / "Widget.php" + f.write_text(" None: + # A second copy of an existing line is "added" — set-diff would lose it. + f = tmp_path / "f.txt" + f.write_text("dup\ndup\nother\n") + added = supertool._advice_added_text(str(f), b"dup\n") + assert added.count("dup") == 1 # one consumed by pre, one remains + assert "other" in added + + +def test_advice_wants_pre_gates_on_contains_rules() -> None: + # The fix for the pre_content=None over-fire: a contains rule that applies + # to this op/path makes the caller snapshot pre-edit bytes. + _set_advice({"comp": { + "hooks_into": ["edit"], + "match": "*/Components/*.php", + "contains": r"extends ComponentBase", + "message": "regen", + }}, resolve=None) + assert supertool._advice_wants_pre("edit", "SiX/Components/Foo.php") is True + assert supertool._advice_wants_pre("edit", "SiX/Other/Foo.php") is False # match + assert supertool._advice_wants_pre("paste", "SiX/Components/Foo.php") is False # hooks_into + # A rule without `contains` never needs pre. + _set_advice({"n": {"hooks_into": ["edit"], "message": "hi"}}, resolve=None) + assert supertool._advice_wants_pre("edit", "SiX/Components/Foo.php") is False + + +# --- when gate + default ops ------------------------------------------------ + +def test_when_existing_file_fires_only_on_existing(tmp_path: Path) -> None: + f = tmp_path / "X.php" + f.write_text(" None: - _set_advice_config(True) - assert supertool._advise_new_test("paste", "styles.scss", pre_existed=False) == "" +def test_default_ops_cover_all_mutating(tmp_path: Path) -> None: + f = tmp_path / "X.php" + f.write_text(" None: - _set_advice_config(True, resolve=_RESOLVE_HIT) - assert supertool._advise_new_test("paste", "Foo.class.php", pre_existed=False) == "" +# --- multiple rules + interpolation ----------------------------------------- + +def test_multiple_rules_each_emit_a_line(tmp_path: Path) -> None: + f = tmp_path / "Widget.php" + f.write_text(" None: - _set_advice_config(True, resolve=None) - assert supertool._advise_new_test("paste", "Foo.class.php", pre_existed=False) == "" + +def test_target_template_interpolation(tmp_path: Path) -> None: + f = tmp_path / "Foo.php" + f.write_text(" Path: mirror test exists on disk -> echo it on stdout, exit 0 no test -> mirror target on stderr, exit 3 Mirror rule: /src/ -> /tests/, Foo.php -> FooTest.php. - - argv path is normalized to forward slashes so the /src/ -> /tests/ - mirror rule works on Windows, where the path arrives with backslashes. """ script = tmp_path / "resolve_test_equiv.py" script.write_text( @@ -838,10 +978,10 @@ def _write_resolver_script(tmp_path: Path) -> Path: return script -def _set_advice_config_with_script(script: Path) -> None: +def _set_advice_with_script(script: Path) -> None: resolve = f'{{python}} {script.as_posix()} {{file}}' supertool._CONFIG = { - "adviseForNewTest": True, + "advice": {"newTest": dict(_NEW_TEST_RULE)}, "validators": {"phpunit": {"cmd": "noop", "resolve": resolve}}, } supertool._CONFIG_CHECKED = True @@ -852,11 +992,10 @@ def test_advise_real_script_emits_when_no_test_on_disk(tmp_path: Path) -> None: src_dir.mkdir(parents=True) src = src_dir / "Foo.php" src.write_text(" None: src_dir.mkdir(parents=True) src = src_dir / "Foo.php" src.write_text("