diff --git a/docs/decisions/0001-test-isolation-pytest-plugin-and-shift-left-quality-gates.md b/docs/decisions/0001-test-isolation-pytest-plugin-and-shift-left-quality-gates.md index ad897c7..7a50c05 100644 --- a/docs/decisions/0001-test-isolation-pytest-plugin-and-shift-left-quality-gates.md +++ b/docs/decisions/0001-test-isolation-pytest-plugin-and-shift-left-quality-gates.md @@ -40,20 +40,30 @@ invocation in any repo with devx installed automatically runs the static analysis. No extra Makefile target or CI step needed. The plugin (`devx.tools.check_test_isolation`) statically analyzes -test files during `pytest_collection_finish` and emits -`UserWarning` for violations: +test files during `pytest_collection_finish` and **fails the test run** +on any hard violation: - **unpatched-subprocess**: `subprocess.run/call/Popen/check_call/check_output` - called in a test function without `@patch` + called in a test function without `@patch` or `with patch(...)` - **unpatched-sleep**: `time.sleep` called without `@patch` - **unpatched-helper**: known subprocess-spawning helpers (`update_doc_versions`, `run_cmd`, `run_tests`) called without `@patch` (and without patching their internal dependencies) - **excessive-iterations**: `for _ in range(N)` where N > 100 +- **heavy-module-import**: `httpx`, `ansible`, etc. imported at module + level in test files, slowing collection for all tests +- **reload-without-cleanup**: `importlib.reload()` called an odd number + of times, leaving module state modified -The plugin recognizes transitive safety: if `run_cmd` is patched, -`run_tests` (which calls `run_cmd`) is safe. This is tracked via -`HELPER_INTERNAL_CALLS`. +Transitive-subprocess findings (via call-graph analysis) are reported +as **advisories** — the static analysis can't predict early exits or +runtime branch conditions, so the runtime audit is authoritative. + +The plugin also wraps `subprocess.run` at runtime to catch real +subprocess calls that leak through transitive call paths (for example +`CliRunner.invoke(main)` → `main()` → `update_doc_versions()` → +`subprocess.run()`). If a test spawns a real subprocess without +`@patch`, the test fails. A standalone CLI (`python -m devx.tools.check_test_isolation`) is also provided for CI gates and pre-commit hooks where pytest isn't run. @@ -85,16 +95,20 @@ is even created. - **Automatic enforcement**: The pytest plugin runs on every `pytest` invocation across devx, grm, and infra — no per-repo configuration - needed. New tests with unpatched subprocess calls emit warnings - immediately. + needed. New tests with unpatched subprocess calls fail immediately. - **Shift-left**: Translation gaps and test isolation violations are caught locally (pre-commit / `make lint`) instead of in CI. -- **Fast feedback**: Static analysis adds <0.1s to test runs — no - runtime overhead. -- **No false positives**: The transitive dependency tracking - (`HELPER_INTERNAL_CALLS`) correctly recognizes that patching - `run_cmd` makes `run_tests` safe, and patching `subprocess.run` - makes all helpers safe. +- **Fast feedback**: Static analysis adds <0.1s to test runs; runtime + subprocess audit adds negligible overhead (wrapper checks a + thread-local flag). +- **Transitive detection**: The call-graph BFS traces + `CliRunner.invoke(main)` → `main()` → `update_doc_versions()` → + `subprocess.run()`, catching indirect subprocess leaks that direct + analysis misses. The runtime audit provides authoritative enforcement. +- **No false positives**: The call graph correctly recognizes that + patching `run_cmd` makes `run_tests` (which calls `run_cmd`) safe, + and class methods are excluded to avoid false positives when classes + like `TeaCLI` are patched. ### Negative @@ -103,10 +117,12 @@ is even created. definitions) appears uncovered. Mitigated by `-p no:devx_test_isolation` in devx's own `pyproject.toml` `addopts` and `# pragma: no cover` on plugin hook functions. -- **Static analysis limitations**: The plugin only sees direct calls - in test function bodies, not indirect calls through `main()` or - other wrappers. This is acceptable — the `check_test_speed` tool - catches the symptom (slow tests) for indirect cases. +- **Static analysis limitations**: The call-graph BFS can't predict + runtime branch conditions or early exits — a test that patches + `shutil.which` to return `None` may skip the subprocess path + entirely, but the static analysis still reports it. Transitive + findings are advisories (exit 0) for this reason; the runtime audit + is authoritative. - **Translation burden**: Every new `_()` call in source requires adding 6 language translations. This is by design (all supported languages must be complete) but adds friction for quick prototypes. @@ -122,21 +138,36 @@ in consumer repos. ### Disabling the Plugin -- `--no-test-isolation` flag: disables analysis for a single run +- `--no-test-isolation` flag: disables static analysis and runtime + subprocess audit for a single run - `-p no:devx_test_isolation` in `addopts`: disables for a repo (used in devx's own `pyproject.toml` for coverage reasons) -### Strict Mode +### Call-Graph Analysis -- `--strict-test-isolation` flag: promotes warnings to errors and - prints a summary to stderr -- `filterwarnings = ["error:Test isolation:UserWarning"]` in - `pyproject.toml`: same effect via pytest's warning filter system +The `CallGraph` class parses all `.py` files under `src/` and builds +a map of function → called functions. When a test calls +`CliRunner.invoke(target)`, a BFS traces the call graph from `target` +to find all reachable functions. Class methods are excluded from the +call graph to avoid false positives when classes are patched (for example +`@patch("...TeaCLI")` mocks all methods). The BFS respects `@patch` +decorators — if a function is patched, traversal stops at that node. + +### Runtime Subprocess Audit + +The `_SubprocessAudit` singleton wraps `subprocess.run`, `call`, +`check_call`, `check_output`, and `Popen` with thread-local +recording wrappers. During each non-integration test, the wrapper +records calls; if any are recorded (that is the test didn't `@patch` +subprocess), the test fails. The wrappers check a thread-local flag, +so inactive audits have zero overhead beyond the flag check. ### Known Subprocess Helpers The `KNOWN_SUBPROCESS_HELPERS` dict maps function names to descriptions. `HELPER_INTERNAL_CALLS` maps each helper to the function names it internally calls, enabling transitive safety -checks. Both are defined in `check_test_isolation.py` and can be -extended as new subprocess-spawning helpers are added to devx. +checks for direct calls in test functions. The call-graph BFS +handles transitive detection for `CliRunner.invoke` targets. Both +are defined in `check_test_isolation.py` and can be extended as +new subprocess-spawning helpers are added to devx. diff --git a/docs/user/cli-commands.md b/docs/user/cli-commands.md index 69e6ce6..453a39c 100644 --- a/docs/user/cli-commands.md +++ b/docs/user/cli-commands.md @@ -339,28 +339,37 @@ devx tools check-test-speed --max-seconds 4 --max-single-seconds 0.5 Statically analyze test files for un-hermetic patterns that cause slow or flaky tests. Also available as a **pytest plugin** (auto-discovered via the `pytest11` entry point when devx is installed — runs -automatically on every `pytest` invocation). +automatically on every `pytest` invocation and **fails on violations**). -Detected patterns: +Detected patterns (hard errors — exit non-zero): - **unpatched-subprocess**: `subprocess.run/call/Popen/check_call/check_output` - called in a test function without `@patch` + called in a test function without `@patch` or `with patch(...)` - **unpatched-sleep**: `time.sleep` called without `@patch` - **unpatched-helper**: known subprocess-spawning helpers (`update_doc_versions`, `run_cmd`, `run_tests`) called without `@patch` or patching their internal deps - **excessive-iterations**: `for _ in range(N)` where N > 100 +- **heavy-module-import**: `httpx`, `ansible`, etc. imported at module level +- **reload-without-cleanup**: `importlib.reload()` called an odd number of times + +Advisory patterns (exit 0 — runtime audit is authoritative): + +- **transitive-subprocess**: `CliRunner.invoke(target)` where `target` + transitively calls `subprocess.run` without being patched. Detected via + static call-graph analysis. The runtime subprocess audit catches actual + leaks — if a real subprocess runs without `@patch`, the test fails. ```bash devx tools check-test-isolation -devx tools check-test-isolation --test-path tests/ --strict -devx tools check-test-isolation --categories unpatched-subprocess,unpatched-sleep +devx tools check-test-isolation --test-path tests/ +devx tools check-test-isolation --categories unpatched-subprocess,transitive-subprocess devx tools check-test-isolation --max-loop-iterations 50 +devx tools check-test-isolation --src-dir src/ ``` Pytest plugin options (automatic when devx is installed): -- `--strict-test-isolation` — fail the test run on violations -- `--no-test-isolation` — disable analysis for this run +- `--no-test-isolation` — disable static analysis and runtime subprocess audit - `--test-isolation-max-loop N` — max iterations per loop (default: 100) ### `devx tools configure-repo` diff --git a/src/devx/api_clients.py b/src/devx/api_clients.py index 301bc72..5b595a9 100644 --- a/src/devx/api_clients.py +++ b/src/devx/api_clients.py @@ -224,6 +224,16 @@ class GiteaClient: r = self._request("GET", f"/pulls/{pr_number}") return r.json() + def update_pr(self, pr_number: str | int, fields: dict[str, Any]) -> dict[str, Any]: + """Update a pull request (e.g. title, body, state). + + Args: + pr_number: PR number. + fields: Dict of fields to update (e.g. {"title": "new title"}). + """ + r = self._request("PATCH", f"/pulls/{pr_number}", json=fields) + return r.json() + def create_pr(self, title: str, head: str, base: str = "master", body: str = "") -> dict[str, Any]: """Create a pull request and return the PR dict. diff --git a/src/devx/ci/check_auto_merge_ready.py b/src/devx/ci/check_auto_merge_ready.py index 0838283..8202856 100644 --- a/src/devx/ci/check_auto_merge_ready.py +++ b/src/devx/ci/check_auto_merge_ready.py @@ -285,6 +285,26 @@ def cli( click.echo("=" * 60, err=True) for e in errors: click.echo(f" - {e}", err=True) + + # Remediation hints for the most common failure: PR title format + title_errors = [ + e for e in errors if "PR title must follow format" in str(e) or "PR title task ID mismatch" in str(e) + ] + if title_errors and pr_number is not None and repo is not None: + click.echo("", err=True) + click.echo("REMEDIATION:", err=True) + click.echo( + _( + " Fix the PR title with:\n" + " python3 -m devx.ci.fix_pr_title --repo {repo} --pr-number {pr}\n" + " Or manually set the PR title to: '{expected}'", + repo=repo, + pr=pr_number, + expected=f"{task_id}: ", + ), + err=True, + ) + raise click.ClickException(_("Pre-merge validation failed.")) click.echo("[pre-merge-check] All auto-merge preconditions satisfied.") diff --git a/src/devx/ci/fix_pr_title.py b/src/devx/ci/fix_pr_title.py new file mode 100644 index 0000000..c08af1d --- /dev/null +++ b/src/devx/ci/fix_pr_title.py @@ -0,0 +1,127 @@ +#!/usr/bin/env python3 +"""Auto-fix PR title to follow the ``{PREFIX}-N: `` convention. + +Reads the task ID from the branch name, fetches the Vikunja task title, +and updates the PR title via the Gitea API. + +Exit codes: + 0 = PR title updated (or already correct) + 1 = Error (missing token, PR not found, etc.) + +Usage:: + + python3 -m devx.ci.fix_pr_title --repo owner/repo --pr-number 123 + python3 -m devx.ci.fix_pr_title --repo owner/repo --branch DEVX-256-fix-foo --pr-number 123 +""" + +from __future__ import annotations + +import click +from dotenv import load_dotenv # pyright: ignore[reportMissingImports,reportUnknownVariableType] + +from devx.api_clients import GiteaClient +from devx.ci.auto_merge import extract_task_id +from devx.ci.check_auto_merge_ready import get_vikunja_title_optional +from devx.config import ( + GITEA_API_URL, + TASK_PREFIX, +) +from devx.exceptions import APIError +from devx.i18n import _ +from devx.tokens import get_ci_token + +load_dotenv() + + +@click.command() +@click.option("--repo", required=True, help=_("Repository in owner/name format")) +@click.option("--pr-number", type=int, required=True, help=_("PR number to fix")) +@click.option("--branch", default=None, help=_("Branch name (auto-fetched from PR if not given)")) +@click.option("--dry-run", is_flag=True, help=_("Show what would change without updating")) +def cli(repo: str, pr_number: int, branch: str | None, dry_run: bool) -> None: + """Fix PR title to follow the ``{PREFIX}-N: <title>`` convention.""" + if "/" not in repo: + raise click.ClickException(_("Repo must be in 'owner/name' format, got: {repo}", repo=repo)) + owner, repo_name = repo.split("/", 1) + + # 1. Get CI token + try: + token = get_ci_token() + except click.ClickException as exc: + raise click.ClickException(_("CI_GITEA_API_TOKEN not set: {error}", error=str(exc))) from exc + + client = GiteaClient(GITEA_API_URL, token, owner, repo_name) + + # 2. Fetch PR + try: + pr = client.get_pr(pr_number) + except APIError as exc: + raise click.ClickException(_("Failed to fetch PR #{pr}: {error}", pr=pr_number, error=str(exc))) from exc + + current_title = str(pr.get("title", "")) + if not branch: + branch = str(pr.get("head", {}).get("ref", "")) + if not branch: + raise click.ClickException(_("Could not determine branch name from PR #{pr}", pr=pr_number)) + + click.echo(f"[fix-pr-title] Branch: {branch}") + click.echo(f"[fix-pr-title] Current PR title: {current_title}") + + # 3. Extract task ID from branch + task_id = extract_task_id(branch) + if not task_id: + raise click.ClickException( + _( + "No task ID found in branch '{branch}'. Expected format: {prefix}-N-description.", + branch=branch, + prefix=TASK_PREFIX, + ) + ) + + click.echo(f"[fix-pr-title] Task ID: {task_id}") + + # 4. Get Vikunja task title + vikunja_title = get_vikunja_title_optional(task_id) + if vikunja_title is None: + # Fallback: strip common prefixes from current title + # (e.g. "fix: ...", "feat: ...", "refactor: ...") + import re + + stripped = re.sub( + r"^(fix|feat|refactor|chore|docs|test|ci|build|perf|style|revert)(\(.+?\))?!?:\s*", "", current_title + ) + # Also strip any leading task ID prefix + stripped = re.sub(rf"^{TASK_PREFIX}-\d+:\s*", "", stripped) + vikunja_title = stripped if stripped else current_title + click.echo(f"[fix-pr-title] WARNING: Vikunja task not found — using stripped title: {vikunja_title}") + else: + click.echo(f"[fix-pr-title] Vikunja title: {vikunja_title}") + + # 5. Build new title + # Defensive: strip task ID prefix from Vikunja title if present + if vikunja_title.startswith(f"{task_id}:"): + vikunja_title = vikunja_title[len(f"{task_id}:") :].strip() + + new_title = f"{task_id}: {vikunja_title}" + + if current_title == new_title: + click.echo(f"[fix-pr-title] PR title already correct: {new_title}") + return + + click.echo(f"[fix-pr-title] New PR title: {new_title}") + + if dry_run: + click.echo("[fix-pr-title] Dry run — not updating PR.") + return + + # 6. Update PR title + try: + client.update_pr(pr_number, {"title": new_title}) + except APIError as exc: + raise click.ClickException(_("Failed to update PR #{pr}: {error}", pr=pr_number, error=str(exc))) from exc + + click.echo(f"[fix-pr-title] PR #{pr_number} title updated to: {new_title}") + + +if __name__ == "__main__": # pragma: no cover + cli() # pragma: no cover diff --git a/src/devx/tools/check_test_isolation.py b/src/devx/tools/check_test_isolation.py index 8a9abbd..5f13a85 100644 --- a/src/devx/tools/check_test_isolation.py +++ b/src/devx/tools/check_test_isolation.py @@ -7,25 +7,29 @@ This module is used in two ways: When devx is installed, pytest auto-discovers this plugin via the ``pytest11`` entry point. Every ``pytest`` run statically analyzes test files for patterns that cause slow, non-deterministic, or - non-hermetic tests and reports violations as warnings. + non-hermetic tests and **fails the test run** if any violations are found. - To promote warnings to errors (fail the test run), add to pyproject.toml:: + The plugin also wraps ``subprocess.run`` at runtime to catch real + subprocess calls that leak through transitive call paths (e.g. + ``CliRunner.invoke(main)`` → ``main()`` → ``update_doc_versions()`` + → ``subprocess.run()``). If a test spawns a real subprocess without + ``@patch``, the test fails. - [tool.pytest.ini_options] - filterwarnings = ["error:Test isolation:UserWarning"] - - Or use the ``--strict-test-isolation`` flag on the command line. + To disable for a specific run: ``--no-test-isolation``. 2. **As a standalone CLI** (for CI gates):: python3 -m devx.tools.check_test_isolation [--test-path tests/] - python3 -m devx.tools.check_test_isolation --strict + + Always exits non-zero on any hard violation. Transitive-subprocess + findings are reported as advisories (exit 0) since static analysis + can't predict early exits — the runtime audit is authoritative. Patterns detected: 1. **Unpatched subprocess calls** — test functions that call ``subprocess.run/call/Popen/check_call/check_output`` without a - corresponding ``@patch`` decorator. + corresponding ``@patch`` decorator or ``with patch(...)`` context manager. 2. **Unpatched ``time.sleep``** — test functions that call ``time.sleep`` without patching it. 3. **Unpatched known-subprocess-helpers** — functions known to spawn @@ -34,12 +38,23 @@ Patterns detected: network I/O (e.g. ``get_pat``, ``load_secrets``, ``requests.get``) called without patching. 5. **Excessive iteration loops** — ``for _ in range(N)`` where N > 100. +6. **Module-level heavy imports** — importing ``httpx``, ``ansible``, + etc. at module level in test files slows collection for all tests. +7. **``importlib.reload`` without cleanup** — reloading a module in a + test mutates global state. Each reload must be paired with a + cleanup reload (or wrapped in try/finally) to restore defaults. +8. **Transitive subprocess leaks** — ``CliRunner.invoke(target)`` where + ``target`` transitively calls ``subprocess.run`` without being patched. + Detected via static call-graph analysis (warning) AND runtime audit + (authoritative — fails the test if a real subprocess runs). """ from __future__ import annotations import ast +import subprocess # nosec B404 import sys +import threading from dataclasses import dataclass, field from pathlib import Path @@ -51,6 +66,33 @@ from devx.i18n import _ DEFAULT_MAX_LOOP_ITERATIONS = 100 +# Heavy modules that are slow to import (>50ms). When imported at module +# level in a test file, they slow down test collection for ALL tests. +# Maps module name → approximate import time in milliseconds. +# NOTE: ``requests`` is excluded because it's a core devx dependency — +# it's loaded during collection regardless of whether test files import it. +HEAVY_MODULE_IMPORTS: dict[str, float] = { + "httpx": 80.0, + "aiohttp": 120.0, + "docker": 90.0, + "kubernetes": 200.0, + "boto3": 250.0, + "botocore": 200.0, + "ansible": 300.0, + "molecule": 150.0, + "cv2": 400.0, + "numpy": 100.0, + "pandas": 200.0, + "matplotlib": 300.0, + "PIL": 80.0, + "Pillow": 80.0, + "sqlalchemy": 150.0, + "django": 200.0, + "flask": 80.0, + "fastapi": 100.0, + "pydantic": 60.0, +} + # Functions known to spawn subprocesses. When a test calls any of these # without patching them, the real subprocess runs. # Maps function name → human-readable description. @@ -88,6 +130,76 @@ HELPER_INTERNAL_CALLS: dict[str, set[str]] = { "run_cmd": {"subprocess"}, } +# subprocess functions that the runtime audit wraps. +_SUBPROCESS_FUNCS = ("run", "call", "check_call", "check_output", "Popen") + + +# ── Runtime subprocess audit ────────────────────────────────────────────────── +# +# The static AST analyzer can only see direct calls in test functions. +# It cannot trace transitive calls through CliRunner.invoke(main, ...) +# → main() → update_doc_versions() → subprocess.run(). +# +# The runtime audit wraps subprocess functions during test execution. +# If a test does NOT @patch subprocess, the wrapper catches real calls. +# If a test DOES @patch subprocess, the patch overrides our wrapper +# (correct — the test is mocking it). + + +class _SubprocessAudit: + """Thread-local audit tracker for real subprocess calls during tests.""" + + def __init__(self) -> None: + self._local = threading.local() + self._installed = False + self._originals: dict[str, object] = {} + + def _ensure_installed(self) -> None: + """Install wrappers on subprocess module (once).""" + if self._installed: + return + for name in _SUBPROCESS_FUNCS: + original = getattr(subprocess, name, None) + if original is None: + continue + self._originals[name] = original + setattr(subprocess, name, self._make_wrapper(name, original)) + self._installed = True + + def _make_wrapper(self, name: str, original: object) -> object: + """Create a wrapper that records calls when auditing is active.""" + + def wrapper(*args: object, **kwargs: object) -> object: + calls = getattr(self._local, "calls", None) + if calls is not None: + # Extract command for diagnostics + cmd = args[0] if args else kwargs.get("args", "?") + if isinstance(cmd, (list, tuple)) and cmd: + cmd_str = " ".join(str(c) for c in cmd[:4]) + if len(cmd) > 4: + cmd_str += " ..." + else: + cmd_str = str(cmd) + calls.append((name, cmd_str)) + return original(*args, **kwargs) # type: ignore[misc] + + return wrapper + + def start_test(self) -> None: + """Begin auditing subprocess calls for the current test.""" + self._ensure_installed() + self._local.calls = [] + + def stop_test(self) -> list[tuple[str, str]]: + """Stop auditing and return recorded calls.""" + calls = getattr(self._local, "calls", []) + self._local.calls = None + return calls + + +# Singleton instance used by the pytest plugin +_audit = _SubprocessAudit() + # ── Data structures ─────────────────────────────────────────────────────────── @@ -125,22 +237,58 @@ class TestFunctionInfo: def _extract_patch_targets(node: ast.FunctionDef | ast.AsyncFunctionDef | ast.ClassDef) -> set[str]: - """Extract @patch targets from decorators on a function or class.""" + """Extract @patch targets from decorators AND ``with patch(...)`` statements. + + Detects: + - ``@patch("module.func")`` decorators + - ``with patch("module.func")`` context managers + - ``with patch.object(module, "func")`` context managers + - ``with patch("a"), patch("b")`` multiple patches + """ targets: set[str] = set() + + def _process_patch_call(call: ast.Call) -> None: + """Extract target from a patch() or patch.object() call.""" + func = call.func + # patch("module.func") — either bare `patch(...)` or `mock.patch(...)` + if (isinstance(func, ast.Name) and func.id == "patch") or ( + isinstance(func, ast.Attribute) and func.attr == "patch" + ): + if call.args and isinstance(call.args[0], ast.Constant) and isinstance(call.args[0].value, str): + target = call.args[0].value + targets.add(target) + targets.add(target.rsplit(".", 1)[-1]) + # patch.object(module, "func") — extract short name from 2nd arg + elif ( + isinstance(func, ast.Attribute) + and func.attr == "object" + and isinstance(func.value, ast.Name) + and func.value.id == "patch" + and len(call.args) >= 2 + and isinstance(call.args[1], ast.Constant) + and isinstance(call.args[1].value, str) + and call.args[0] + and isinstance(call.args[0], ast.Name) + ): + short = call.args[1].value + targets.add(short) + # We can't resolve the module alias here, but the short + # name is enough for patch matching in the call graph. + + # 1. Extract from decorators for decorator in node.decorator_list: if isinstance(decorator, ast.Call): - func = decorator.func - is_patch = ( - isinstance(func, ast.Name) - and func.id == "patch" - or isinstance(func, ast.Attribute) - and func.attr == "patch" - ) - if is_patch and decorator.args and isinstance(decorator.args[0], ast.Constant): - target = decorator.args[0].value - if isinstance(target, str): - targets.add(target) - targets.add(target.rsplit(".", 1)[-1]) + _process_patch_call(decorator) + + # 2. Extract from `with patch(...)` context managers in the body + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): + for child in ast.walk(node): + if isinstance(child, ast.With): + for item in child.items: + ctx = item.context_expr + if isinstance(ctx, ast.Call): + _process_patch_call(ctx) + return targets @@ -210,19 +358,303 @@ def _get_range_count(node: ast.Call) -> int | None: return None # pragma: no cover +# ── Call-graph builder ──────────────────────────────────────────────────────── +# +# The static AST analyzer can only see direct calls in test functions. +# It cannot trace transitive calls through CliRunner.invoke(main, ...) +# → main() → update_doc_versions() → subprocess.run(). +# +# The call-graph builder parses all source files in the package and builds +# a map: function_name → set of function_names it calls. +# When a test calls runner.invoke(target, ...), we trace the call graph +# from target to find all reachable functions, then check if any of them +# call subprocess.run (or other dangerous functions) without being patched. + + +# Dangerous functions that should never run in unit tests. +# Maps full call name → description. +_DANGEROUS_CALLS: dict[str, str] = { + "subprocess.run": "spawns a real subprocess", + "subprocess.call": "spawns a real subprocess", + "subprocess.check_call": "spawns a real subprocess", + "subprocess.check_output": "spawns a real subprocess", + "subprocess.Popen": "spawns a real subprocess", +} + + +@dataclass +class _FunctionNode: + """AST node for a function with its called names.""" + + name: str + module: str + calls: set[str] # short names of functions called + subprocess_calls: set[str] # dangerous subprocess calls made directly + io_calls: set[str] # known I/O function calls made directly + + +class CallGraph: + """Call graph built from source files in a package directory.""" + + def __init__(self, src_dir: Path) -> None: + self.src_dir = src_dir + # Maps "module.func" → _FunctionNode + self._nodes: dict[str, _FunctionNode] = {} + # Maps short name → list of full names (for resolution) + self._by_short: dict[str, list[str]] = {} + self._built = False + + def _ensure_built(self) -> None: + if self._built: + return + self._build() + self._built = True + + def _build(self) -> None: + """Parse all .py files under src_dir and build the call graph.""" + for py_file in sorted(self.src_dir.rglob("*.py")): + try: + source = py_file.read_text() + tree = ast.parse(source, filename=str(py_file)) + except (SyntaxError, UnicodeDecodeError): + continue + # Derive module name from path relative to src_dir + rel = py_file.relative_to(self.src_dir) + module_parts = list(rel.with_suffix("").parts) + if module_parts and module_parts[-1] == "__init__": + module_parts = module_parts[:-1] + module = ".".join(module_parts) + self._scan_module(tree, module) + + def _scan_module(self, tree: ast.Module, module: str) -> None: + """Scan a module AST and register all top-level functions. + + Methods defined inside classes are NOT registered — they are called + via objects (e.g. ``tea.create_issue()``) and resolving them by short + name alone causes false positives when the class is patched (e.g. + ``@patch("...TeaCLI")`` mocks all methods). + """ + for node in tree.body: + self._scan_node(node, module) + + def _scan_node(self, node: ast.AST, module: str) -> None: + """Recursively scan a node, registering non-method functions.""" + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): + self._register_function(node, module) + # Don't recurse into function bodies — nested functions are + # not callable by name from outside. + return + if isinstance(node, ast.ClassDef): + # Skip class body — methods are not registered. + return + # Recurse into other compound statements (if/for/try/with/etc.) + for child in ast.iter_child_nodes(node): + self._scan_node(child, module) + + def _register_function(self, node: ast.FunctionDef | ast.AsyncFunctionDef, module: str) -> None: + """Register a function and its direct calls in the call graph.""" + full_name = f"{module}.{node.name}" + calls: set[str] = set() + subprocess_calls: set[str] = set() + io_calls: set[str] = set() + + for child in ast.walk(node): + if isinstance(child, ast.Call): + full = _get_full_called_name(child) + short = _get_called_name(child) + if short: + calls.add(short) + if full and full in _DANGEROUS_CALLS: + subprocess_calls.add(full) + if short and short in KNOWN_IO_FUNCTIONS: + io_calls.add(short) + # KNOWN_SUBPROCESS_HELPERS are intermediate functions (e.g. + # run_tests → run_cmd → subprocess.run). They are already + # in *calls* so the BFS will traverse into them and find the + # actual subprocess call. Adding them to *subprocess_calls* + # here would cause false positives when the helper itself is + # transitively patched (e.g. run_cmd is patched → run_tests + # is safe, but would still be reported). + + fn_node = _FunctionNode( + name=node.name, + module=module, + calls=calls, + subprocess_calls=subprocess_calls, + io_calls=io_calls, + ) + self._nodes[full_name] = fn_node + self._by_short.setdefault(node.name, []).append(full_name) + + def find_reachable_dangerous( + self, + target_name: str, + patches: set[str], + max_depth: int = 10, + import_map: dict[str, str] | None = None, + ) -> list[tuple[str, str]]: + """Find all dangerous calls reachable from target_name that aren't patched. + + Returns a list of (function_name, description) tuples for each + unpatched dangerous call found in the transitive closure. + + If import_map is provided (mapping short names to fully-qualified + module paths), it's used to resolve the target precisely instead + of matching by short name alone. + """ + self._ensure_built() + + # Resolve target to full name(s) + # First try precise resolution via import_map + candidates: list[str] = [] + if import_map and target_name in import_map: + full = import_map[target_name] + candidates = [full] if full in self._nodes else self._by_short.get(target_name, []) + elif target_name in self._nodes: + # Already a fully-qualified name (e.g. devx.tools.build_image.main) + candidates = [target_name] + else: + # Fall back to short name resolution + short = target_name.rsplit(".", 1)[-1] + candidates = self._by_short.get(short, []) + + if not candidates: + return [] + + visited: set[str] = set() + dangerous: list[tuple[str, str]] = [] + queue: list[tuple[str, int]] = [(c, 0) for c in candidates] + + while queue: + full_name, depth = queue.pop(0) + if full_name in visited or depth > max_depth: + continue + visited.add(full_name) + + node = self._nodes.get(full_name) + if node is None: + continue + + # Check direct subprocess calls + for sc in node.subprocess_calls: + short = sc.rsplit(".", 1)[-1] + if not self._is_patched(sc, short, patches): + desc = _DANGEROUS_CALLS.get(sc, "") + dangerous.append((full_name, desc)) + + # Check direct IO calls + for io in node.io_calls: + if not self._is_patched(io, io, patches): + desc = KNOWN_IO_FUNCTIONS.get(io, "") + if desc: + dangerous.append((full_name, desc)) + + # Enqueue called functions — skip if the called function is patched + for called_short in node.calls: + if self._is_patched(called_short, called_short, patches): + continue + # Prefer same-module resolution, then fall back to short name + # only if there's a single global match (avoids false positives + # when multiple modules define functions with the same name). + same_module = f"{node.module}.{called_short}" + if same_module in self._nodes and same_module not in visited: + queue.append((same_module, depth + 1)) + else: + matches = self._by_short.get(called_short, []) + if len(matches) == 1 and matches[0] not in visited: + queue.append((matches[0], depth + 1)) + + return dangerous + + @staticmethod + def _is_patched(full: str, short: str, patches: set[str]) -> bool: + """Check if a function is covered by the test's @patch set.""" + if short in patches or full in patches: + return True + # Check if any patch entry ends with ".short" (e.g. "subprocess.run" + # is patched by "devx.ci.release.subprocess.run"). Use exact + # endswith, not substring, to avoid "run" matching "run_cmd". + return any(p.endswith(f".{short}") or p == full for p in patches) + + # ── Analyzers ───────────────────────────────────────────────────────────────── class TestIsolationVisitor(ast.NodeVisitor): """AST visitor that detects un-hermetic test patterns.""" - def __init__(self, file_path: Path, max_loop_iterations: int = DEFAULT_MAX_LOOP_ITERATIONS): + def __init__( + self, + file_path: Path, + max_loop_iterations: int = DEFAULT_MAX_LOOP_ITERATIONS, + call_graph: CallGraph | None = None, + ): self.file_path = file_path self.max_loop_iterations = max_loop_iterations + self.call_graph = call_graph self.violations: list[Violation] = [] self._current_function: TestFunctionInfo | None = None self._current_class_patches: set[str] = set() self._in_test_class = False + self._reload_calls: list[tuple[int, str | None]] = [] + # Import map: short name → fully-qualified module.func + # e.g. {"main": "devx.ci.release.main"} for `from devx.ci.release import main` + self._import_map: dict[str, str] = {} + + def visit_Import(self, node: ast.Import) -> None: + # Track imports for call-graph resolution + if self._current_function is None: + for alias in node.names: + name = alias.asname or alias.name + self._import_map[name] = alias.name + # Check for heavy module imports + if self._current_function is None: + for alias in node.names: + mod = alias.name.split(".")[0] + if mod in HEAVY_MODULE_IMPORTS: + self.violations.append( + Violation( + file=self.file_path, + line=node.lineno, + col=node.col_offset, + category="heavy-module-import", + message=_( + "Heavy import '{mod}' (~{ms:.0f}ms) at module level — " + "this slows test collection for all tests. " + "Move inside test functions or use lazy import.", + mod=alias.name, + ms=HEAVY_MODULE_IMPORTS[mod], + ), + ) + ) + self.generic_visit(node) + + def visit_ImportFrom(self, node: ast.ImportFrom) -> None: + # Track imports for call-graph resolution + if self._current_function is None and node.module: + for alias in node.names: + name = alias.asname or alias.name + self._import_map[name] = f"{node.module}.{alias.name}" + # Check for heavy module imports + if self._current_function is None and node.module: + mod = node.module.split(".")[0] + if mod in HEAVY_MODULE_IMPORTS: + self.violations.append( + Violation( + file=self.file_path, + line=node.lineno, + col=node.col_offset, + category="heavy-module-import", + message=_( + "Heavy import '{mod}' (~{ms:.0f}ms) at module level — " + "this slows test collection for all tests. " + "Move inside test functions or use lazy import.", + mod=node.module, + ms=HEAVY_MODULE_IMPORTS[mod], + ), + ) + ) + self.generic_visit(node) def visit_ClassDef(self, node: ast.ClassDef) -> None: old_class_patches = self._current_class_patches @@ -258,9 +690,33 @@ class TestIsolationVisitor(ast.NodeVisitor): is_test=True, ) old_func = self._current_function + old_reloads = self._reload_calls self._current_function = info + self._reload_calls = [] self.generic_visit(node) + # Check 7: importlib.reload without cleanup + # Each reload mutates global module state. An odd number of + # reloads means the module is left in a modified state. + if len(self._reload_calls) % 2 != 0: + first_line, mod_name = self._reload_calls[0] + self.violations.append( + Violation( + file=self.file_path, + line=first_line, + col=0, + category="reload-without-cleanup", + message=_( + "importlib.reload({mod}) called {n} time(s) in test '{test}' — " + "odd count leaves module in modified state. " + "Add a final reload to restore defaults or wrap in try/finally.", + mod=mod_name or "module", + n=len(self._reload_calls), + test=info.name, + ), + ) + ) self._current_function = old_func + self._reload_calls = old_reloads def visit_Call(self, node: ast.Call) -> None: if self._current_function is None: @@ -271,6 +727,16 @@ class TestIsolationVisitor(ast.NodeVisitor): short_name = _get_called_name(node) all_patches = self._current_function.patches | self._current_function.class_patches + # Track importlib.reload calls for cleanup check + if full_name == "importlib.reload" or (short_name == "reload" and "reload" in all_patches): + mod_arg = node.args[0] if node.args else None + mod_name = None + if isinstance(mod_arg, ast.Name): + mod_name = mod_arg.id + elif isinstance(mod_arg, ast.Attribute): + mod_name = mod_arg.attr + self._reload_calls.append((node.lineno, mod_name)) + # Check 1: subprocess.run / subprocess.call / subprocess.Popen etc. if full_name and full_name.startswith("subprocess."): method = full_name.split(".", 1)[1] @@ -368,6 +834,52 @@ class TestIsolationVisitor(ast.NodeVisitor): ) ) + # Check 8: CliRunner.invoke / runner.invoke — trace call graph + # Detect runner.invoke(target, ...) or CliRunner().invoke(target, ...) + if short_name == "invoke" and self.call_graph is not None and node.args: + target = node.args[0] + target_name: str | None = None + if isinstance(target, ast.Name): + target_name = target.id + elif isinstance(target, ast.Attribute): + # Handle module.func pattern (e.g. build_image.main) + # Resolve module prefix via import_map + if isinstance(target.value, ast.Name): + mod_short = target.value.id + mod_full = self._import_map.get(mod_short) + target_name = f"{mod_full}.{target.attr}" if mod_full else target.attr + else: + target_name = target.attr + if target_name: + dangerous = self.call_graph.find_reachable_dangerous( + target_name, all_patches, import_map=self._import_map + ) + if dangerous: + # Deduplicate by function name + seen: set[str] = set() + unique: list[tuple[str, str]] = [] + for func, desc in dangerous: + if func not in seen: + seen.add(func) + unique.append((func, desc)) + funcs_desc = "; ".join(f"{f} ({d})" for f, d in unique[:3]) + self.violations.append( + Violation( + file=self.file_path, + line=node.lineno, + col=node.col_offset, + category="transitive-subprocess", + message=_( + "CliRunner.invoke({target}) in test '{test}' reaches " + "unpatched dangerous functions: {funcs}. " + "Add @patch for each or patch the calling function.", + target=target_name, + test=self._current_function.name, + funcs=funcs_desc, + ), + ) + ) + self.generic_visit(node) def visit_For(self, node: ast.For) -> None: @@ -402,7 +914,11 @@ def find_test_files(test_path: Path) -> list[Path]: return sorted(test_path.rglob("test_*.py")) -def analyze_file(file_path: Path, max_loop_iterations: int = DEFAULT_MAX_LOOP_ITERATIONS) -> list[Violation]: +def analyze_file( + file_path: Path, + max_loop_iterations: int = DEFAULT_MAX_LOOP_ITERATIONS, + call_graph: CallGraph | None = None, +) -> list[Violation]: """Analyze a single test file for isolation violations. Files in ``integration/`` directories are skipped — integration tests @@ -424,7 +940,7 @@ def analyze_file(file_path: Path, max_loop_iterations: int = DEFAULT_MAX_LOOP_IT ) ] - visitor = TestIsolationVisitor(file_path, max_loop_iterations) + visitor = TestIsolationVisitor(file_path, max_loop_iterations, call_graph) visitor.visit(tree) return visitor.violations @@ -433,12 +949,13 @@ def analyze_test_files( test_path: Path, max_loop_iterations: int = DEFAULT_MAX_LOOP_ITERATIONS, categories: set[str] | None = None, + call_graph: CallGraph | None = None, ) -> list[Violation]: """Analyze all test files under test_path. Returns list of violations.""" test_files = find_test_files(test_path) all_violations: list[Violation] = [] for file_path in test_files: - violations = analyze_file(file_path, max_loop_iterations) + violations = analyze_file(file_path, max_loop_iterations, call_graph) if categories: violations = [v for v in violations if v.category in categories] all_violations.extend(violations) @@ -449,23 +966,17 @@ def analyze_test_files( # # When devx is installed, pytest auto-discovers this plugin via the # `pytest11` entry point. The plugin runs static analysis on every -# test file during collection and emits warnings for violations. -# Use --strict-test-isolation to promote warnings to errors. +# test file during collection and **fails** on any violation. +# It also wraps subprocess at runtime to catch transitive leaks. def pytest_addoption(parser): # type: ignore[no-untyped-def] # pragma: no cover """Register pytest command-line options.""" - parser.addoption( - "--strict-test-isolation", - action="store_true", - default=False, - help="Fail the test run if any test isolation violations are found.", - ) parser.addoption( "--no-test-isolation", action="store_true", default=False, - help="Disable test isolation static analysis.", + help="Disable test isolation static analysis and runtime subprocess audit.", ) parser.addoption( "--test-isolation-max-loop", @@ -476,46 +987,129 @@ def pytest_addoption(parser): # type: ignore[no-untyped-def] # pragma: no cove def pytest_collection_finish(session): # type: ignore[no-untyped-def] # pragma: no cover - """Run static analysis after all test files are collected.""" + """Run static analysis after all test files are collected. Always strict.""" if session.config.getoption("--no-test-isolation"): return - strict = session.config.getoption("--strict-test-isolation") max_loop = session.config.getoption("--test-isolation-max-loop") - # Analyze all collected test files + # Build call graph from source directory for transitive analysis + call_graph: CallGraph | None = None + for item in session.items: + fspath = Path(str(item.fspath)) + for parent in fspath.parents: + src_dir = parent / "src" + if src_dir.is_dir(): + call_graph = CallGraph(src_dir) + break + if call_graph is not None: + break + test_files: set[Path] = set() for item in session.items: test_files.add(Path(str(item.fspath))) all_violations: list[Violation] = [] for file_path in sorted(test_files): - violations = analyze_file(file_path, max_loop) + violations = analyze_file(file_path, max_loop, call_graph) all_violations.extend(violations) if not all_violations: return - # Emit warnings - import warnings + # transitive-subprocess is advisory (static can't predict early exits). + # All other categories are hard errors. + errors = [v for v in all_violations if v.category != "transitive-subprocess"] + transitive = [v for v in all_violations if v.category == "transitive-subprocess"] - for v in sorted(all_violations, key=lambda x: (str(x.file), x.line)): - msg = f"Test isolation violation: {v.format()}" - warnings.warn(msg, UserWarning, stacklevel=2) - - if strict: - count = len(all_violations) - files = len({v.file for v in all_violations}) + if errors: + count = len(errors) + files = len({v.file for v in errors}) click.echo( _( - "\nTest isolation check FAILED: {count} violation(s) in {files} file(s).\n" - "Fix: add @patch decorators for subprocess/time.sleep calls, " - "or patch the calling function.\n", + "\nTest isolation check FAILED: {count} violation(s) in {files} file(s).\n", count=count, files=files, ), err=True, ) + for v in sorted(errors, key=lambda x: (str(x.file), x.line)): + click.echo(f" {v.format()}", err=True) + click.echo( + _( + "Fix: add @patch decorators or with patch() context managers " + "for subprocess/time.sleep calls, or patch the calling function.\n" + ), + err=True, + ) + import pytest + + pytest.fail( + f"Test isolation: {count} violation(s) found. See output above.", + pytrace=False, + ) + + # transitive-subprocess warnings are advisory — runtime audit is authoritative + if transitive: + import warnings + + for v in sorted(transitive, key=lambda x: (str(x.file), x.line)): + msg = f"Test isolation advisory: {v.format()}" + warnings.warn(msg, UserWarning, stacklevel=2) + + +# ── Runtime subprocess audit hooks ──────────────────────────────────────────── + + +def _is_integration_test(item: object) -> bool: + """Check if a test item is an integration test.""" + markers = getattr(item, "keywords", {}) + if "integration" in markers: + return True + fspath = str(getattr(item, "fspath", "")) + return "integration" in fspath + + +def pytest_runtest_setup(item: object) -> None: # type: ignore[no-untyped-def] # pragma: no cover + """Start subprocess audit for non-integration tests.""" + config = getattr(item, "config", None) + if config is None: + return + if config.getoption("--no-test-isolation"): + return + if _is_integration_test(item): + return + _audit.start_test() + + +def pytest_runtest_teardown(item: object, nextitem: object) -> None: # type: ignore[no-untyped-def] # pragma: no cover + """Fail test if real subprocess calls were made without @patch.""" + config = getattr(item, "config", None) + if config is None: + return + if config.getoption("--no-test-isolation"): + return + if _is_integration_test(item): + return + calls = _audit.stop_test() + if not calls: + return + + test_name = getattr(item, "name", str(item)) + lines = [ + _( + "Real subprocess call(s) detected in test '{test}' without @patch:", + test=test_name, + ) + ] + for func_name, cmd in calls: + lines.append(f" {func_name}({cmd})") + lines.append(_('Add @patch("subprocess.run") or patch the calling function to fix this.')) + msg = "\n".join(lines) + + import pytest + + pytest.fail(msg, pytrace=False) # ── Standalone CLI ──────────────────────────────────────────────────────────── @@ -538,59 +1132,103 @@ def pytest_collection_finish(session): # type: ignore[no-untyped-def] # pragma show_default=True, help="Maximum allowed iterations in a single test loop.", ) -@click.option( - "--strict", - is_flag=True, - default=False, - help="Treat warnings as errors (non-zero exit on any violation).", -) @click.option( "--categories", type=str, default="", help="Comma-separated list of categories to check (default: all). " - "Available: unpatched-subprocess, unpatched-sleep, unpatched-helper, excessive-iterations", + "Available: unpatched-subprocess, unpatched-sleep, unpatched-helper, " + "excessive-iterations, heavy-module-import, reload-without-cleanup, " + "transitive-subprocess", ) -def cli(test_paths: tuple[Path, ...], max_loop_iterations: int, strict: bool, categories: str) -> None: - """Check test files for un-hermetic patterns that cause slow or flaky tests.""" +@click.option( + "--src-dir", + type=click.Path(exists=True, file_okay=False, path_type=Path), + default=None, + help="Source directory for call-graph analysis (auto-detected if omitted).", +) +def cli( + test_paths: tuple[Path, ...], + max_loop_iterations: int, + categories: str, + src_dir: Path | None, +) -> None: + """Check test files for un-hermetic patterns that cause slow or flaky tests. + + Always exits non-zero on any hard violation. Transitive-subprocess + findings are reported as advisories (exit 0) since static analysis + can't predict early exits — the runtime audit is authoritative. + """ allowed: set[str] | None = None if categories: allowed = {c.strip() for c in categories.split(",")} + # Build call graph for transitive subprocess detection + call_graph: CallGraph | None = None + if src_dir is not None: + call_graph = CallGraph(src_dir) + else: + for tp in test_paths: + for parent in Path(tp).resolve().parents: + candidate = parent / "src" + if candidate.is_dir(): + call_graph = CallGraph(candidate) + break + if call_graph is not None: + break + all_violations: list[Violation] = [] total_files = 0 for test_path in test_paths: - violations = analyze_test_files(test_path, max_loop_iterations, allowed) + violations = analyze_test_files(test_path, max_loop_iterations, allowed, call_graph) all_violations.extend(violations) total_files += len(find_test_files(test_path)) - if not all_violations: + errors = [v for v in all_violations if v.category != "transitive-subprocess"] + advisories = [v for v in all_violations if v.category == "transitive-subprocess"] + + if not errors and not advisories: click.echo( _("Test isolation check passed: {count} test files analyzed, no violations found.", count=total_files) ) sys.exit(0) - click.echo( - _( - "Test isolation check FAILED: {count} violation(s) found in {files} test file(s).", - count=len(all_violations), - files=len({v.file for v in all_violations}), - ), - err=True, - ) - click.echo("") - for v in sorted(all_violations, key=lambda x: (str(x.file), x.line)): - click.echo(f" {v.format()}", err=True) + if errors: + click.echo( + _( + "Test isolation check FAILED: {count} violation(s) in {files} file(s).", + count=len(errors), + files=len({v.file for v in errors}), + ), + err=True, + ) + click.echo("") + for v in sorted(errors, key=lambda x: (str(x.file), x.line)): + click.echo(f" {v.format()}", err=True) + click.echo("") + click.echo( + _( + "Fix: add @patch decorators or with patch() context managers " + "for subprocess/time.sleep calls, or patch the calling function." + ), + err=True, + ) + sys.exit(1) - click.echo("") + # Advisories only — exit 0 but print them click.echo( _( - "Fix: add @patch decorators for subprocess/time.sleep calls, " - "or patch the calling function. Use property-based testing for statistical tests." - ), - err=True, + "Test isolation check passed with {count} advisory warning(s) in {files} file(s).", + count=len(advisories), + files=len({v.file for v in advisories}), + ) ) - sys.exit(1) + click.echo(_("Transitive-subprocess advisories (runtime audit is authoritative):")) + for v in sorted(advisories, key=lambda x: (str(x.file), x.line))[:10]: + click.echo(f" {v.format()}") + if len(advisories) > 10: + click.echo(f" ... and {len(advisories) - 10} more") + sys.exit(0) if __name__ == "__main__": # pragma: no cover diff --git a/src/devx/translations.json b/src/devx/translations.json index 83947fd..3aad660 100644 --- a/src/devx/translations.json +++ b/src/devx/translations.json @@ -183,14 +183,6 @@ "ru": "\nTag → Commit alignment:", "zh": "\nTag → Commit alignment:" }, - "\nTest isolation check FAILED: {count} violation(s) in {files} file(s).\nFix: add @patch decorators for subprocess/time.sleep calls, or patch the calling function.\n": { - "bg": "\nПроверката за изолация на тестове НЕ ПРЕМИНА: {count} нарушения в {files} файла.\nРешение: добавете @patch декоратори за subprocess/time.sleep извиквания или patch-нете извикващата функция.\n", - "de": "\nTestisolationsprüfung FEHLGESCHLAGEN: {count} Verstoß/Verstöße in {files} Datei(en).\nBehebung: @patch-Dekoratoren für subprocess/time.sleep-Aufrufe hinzufügen oder die aufrufende Funktion patchen.\n", - "en": "\nTest isolation check FAILED: {count} violation(s) in {files} file(s).\nFix: add @patch decorators for subprocess/time.sleep calls, or patch the calling function.\n", - "pl": "\nSprawdzenie izolacji testów NIE ZALICZONE: {count} naruszeń w {files} plikach.\nNaprawa: dodaj dekoratory @patch dla wywołań subprocess/time.sleep lub patchuj wywołującą funkcję.\n", - "ru": "\nПроверка изоляции тестов НЕ ПРОЙДЕНА: {count} нарушений в {files} файлах.\nИсправление: добавьте декораторы @patch для вызовов subprocess/time.sleep или patch вызывающую функцию.\n", - "zh": "\n测试隔离检查失败：在 {files} 个文件中有 {count} 个违规。\n修复：为 subprocess/time.sleep 调用添加 @patch 装饰器，或 patch 调用函数。\n" - }, "\nUntagged release commits:": { "bg": "\nUntagged release commits:", "de": "\nUntagged release commits:", @@ -1583,14 +1575,6 @@ "ru": "Fetching origin/master...", "zh": "Fetching origin/master..." }, - "Fix: add @patch decorators for subprocess/time.sleep calls, or patch the calling function. Use property-based testing for statistical tests.": { - "bg": "Решение: добавете @patch декоратори за subprocess/time.sleep извиквания или patch-нете извикващата функция. Използвайте property-based тестове за статистически тестове.", - "de": "Behebung: @patch-Dekoratoren für subprocess/time.sleep-Aufrufe hinzufügen oder die aufrufende Funktion patchen. Property-based testing für statistische Tests verwenden.", - "en": "Fix: add @patch decorators for subprocess/time.sleep calls, or patch the calling function. Use property-based testing for statistical tests.", - "pl": "Naprawa: dodaj dekoratory @patch dla wywołań subprocess/time.sleep lub patchuj wywołującą funkcję. Użyj testów opartych na właściwościach dla testów statystycznych.", - "ru": "Исправление: добавьте декораторы @patch для вызовов subprocess/time.sleep или patch вызывающую функцию. Используйте property-based тестирование для статистических тестов.", - "zh": "修复：为 subprocess/time.sleep 调用添加 @patch 装饰器，或 patch 调用函数。对统计测试使用基于属性的测试。" - }, "Force-push failed:\n{error}\nThe remote may have unexpected commits. Fetch and try again.": { "bg": "Force-push failed:\n{error}\nThe remote may have unexpected commits. Fetch and try again.", "de": "Force-push failed:\n{error}\nThe remote may have unexpected commits. Fetch and try again.", @@ -2903,14 +2887,6 @@ "ru": "Test '{name}' took {elapsed:.2f}s (limit: {limit}s). Optimise: use lighter fixtures, reduce I/O, or mock external calls.", "zh": "Test '{name}' took {elapsed:.2f}s (limit: {limit}s). Optimise: use lighter fixtures, reduce I/O, or mock external calls." }, - "Test isolation check FAILED: {count} violation(s) found in {files} test file(s).": { - "bg": "Проверката за изолация на тестове НЕ ПРЕМИНА: открити са {count} нарушения в {files} тестови файла.", - "de": "Testisolationsprüfung FEHLGESCHLAGEN: {count} Verstoß/Verstöße in {files} Testdatei(en) gefunden.", - "en": "Test isolation check FAILED: {count} violation(s) found in {files} test file(s).", - "pl": "Sprawdzenie izolacji testów NIE ZALICZONE: znaleziono {count} naruszeń w {files} plikach testowych.", - "ru": "Проверка изоляции тестов НЕ ПРОЙДЕНА: найдено {count} нарушений в {files} тестовых файлах.", - "zh": "测试隔离检查失败：在 {files} 个测试文件中发现 {count} 个违规。" - }, "Test isolation check passed: {count} test files analyzed, no violations found.": { "bg": "Проверката за изолация на тестове премина: анализирани са {count} тестови файла, няма нарушения.", "de": "Testisolationsprüfung bestanden: {count} Testdateien analysiert, keine Verstöße gefunden.", @@ -3686,5 +3662,165 @@ "pl": "{separator}", "ru": "{separator}", "zh": "{separator}" + }, + "\nTest isolation check FAILED: {count} violation(s) in {files} file(s).\n": { + "bg": "\nTest isolation check FAILED: {count} violation(s) in {files} file(s).\n", + "de": "\nTest isolation check FAILED: {count} violation(s) in {files} file(s).\n", + "en": "\nTest isolation check FAILED: {count} violation(s) in {files} file(s).\n", + "pl": "\nTest isolation check FAILED: {count} violation(s) in {files} file(s).\n", + "ru": "\nTest isolation check FAILED: {count} violation(s) in {files} file(s).\n", + "zh": "\nTest isolation check FAILED: {count} violation(s) in {files} file(s).\n" + }, + " Fix the PR title with:\n python3 -m devx.ci.fix_pr_title --repo {repo} --pr-number {pr}\n Or manually set the PR title to: '{expected}'": { + "bg": " Fix the PR title with:\n python3 -m devx.ci.fix_pr_title --repo {repo} --pr-number {pr}\n Or manually set the PR title to: '{expected}'", + "de": " Fix the PR title with:\n python3 -m devx.ci.fix_pr_title --repo {repo} --pr-number {pr}\n Or manually set the PR title to: '{expected}'", + "en": " Fix the PR title with:\n python3 -m devx.ci.fix_pr_title --repo {repo} --pr-number {pr}\n Or manually set the PR title to: '{expected}'", + "pl": " Fix the PR title with:\n python3 -m devx.ci.fix_pr_title --repo {repo} --pr-number {pr}\n Or manually set the PR title to: '{expected}'", + "ru": " Fix the PR title with:\n python3 -m devx.ci.fix_pr_title --repo {repo} --pr-number {pr}\n Or manually set the PR title to: '{expected}'", + "zh": " Fix the PR title with:\n python3 -m devx.ci.fix_pr_title --repo {repo} --pr-number {pr}\n Or manually set the PR title to: '{expected}'" + }, + "Add @patch(\"subprocess.run\") or patch the calling function to fix this.": { + "bg": "Add @patch(\"subprocess.run\") or patch the calling function to fix this.", + "de": "Add @patch(\"subprocess.run\") or patch the calling function to fix this.", + "en": "Add @patch(\"subprocess.run\") or patch the calling function to fix this.", + "pl": "Add @patch(\"subprocess.run\") or patch the calling function to fix this.", + "ru": "Add @patch(\"subprocess.run\") or patch the calling function to fix this.", + "zh": "Add @patch(\"subprocess.run\") or patch the calling function to fix this." + }, + "Branch name (auto-fetched from PR if not given)": { + "bg": "Branch name (auto-fetched from PR if not given)", + "de": "Branch name (auto-fetched from PR if not given)", + "en": "Branch name (auto-fetched from PR if not given)", + "pl": "Branch name (auto-fetched from PR if not given)", + "ru": "Branch name (auto-fetched from PR if not given)", + "zh": "Branch name (auto-fetched from PR if not given)" + }, + "CI_GITEA_API_TOKEN not set: {error}": { + "bg": "CI_GITEA_API_TOKEN not set: {error}", + "de": "CI_GITEA_API_TOKEN not set: {error}", + "en": "CI_GITEA_API_TOKEN not set: {error}", + "pl": "CI_GITEA_API_TOKEN not set: {error}", + "ru": "CI_GITEA_API_TOKEN not set: {error}", + "zh": "CI_GITEA_API_TOKEN not set: {error}" + }, + "CliRunner.invoke({target}) in test '{test}' reaches unpatched dangerous functions: {funcs}. Add @patch for each or patch the calling function.": { + "bg": "CliRunner.invoke({target}) in test '{test}' reaches unpatched dangerous functions: {funcs}. Add @patch for each or patch the calling function.", + "de": "CliRunner.invoke({target}) in test '{test}' reaches unpatched dangerous functions: {funcs}. Add @patch for each or patch the calling function.", + "en": "CliRunner.invoke({target}) in test '{test}' reaches unpatched dangerous functions: {funcs}. Add @patch for each or patch the calling function.", + "pl": "CliRunner.invoke({target}) in test '{test}' reaches unpatched dangerous functions: {funcs}. Add @patch for each or patch the calling function.", + "ru": "CliRunner.invoke({target}) in test '{test}' reaches unpatched dangerous functions: {funcs}. Add @patch for each or patch the calling function.", + "zh": "CliRunner.invoke({target}) in test '{test}' reaches unpatched dangerous functions: {funcs}. Add @patch for each or patch the calling function." + }, + "Could not determine branch name from PR #{pr}": { + "bg": "Could not determine branch name from PR #{pr}", + "de": "Could not determine branch name from PR #{pr}", + "en": "Could not determine branch name from PR #{pr}", + "pl": "Could not determine branch name from PR #{pr}", + "ru": "Could not determine branch name from PR #{pr}", + "zh": "Could not determine branch name from PR #{pr}" + }, + "Failed to fetch PR #{pr}: {error}": { + "bg": "Failed to fetch PR #{pr}: {error}", + "de": "Failed to fetch PR #{pr}: {error}", + "en": "Failed to fetch PR #{pr}: {error}", + "pl": "Failed to fetch PR #{pr}: {error}", + "ru": "Failed to fetch PR #{pr}: {error}", + "zh": "Failed to fetch PR #{pr}: {error}" + }, + "Failed to update PR #{pr}: {error}": { + "bg": "Failed to update PR #{pr}: {error}", + "de": "Failed to update PR #{pr}: {error}", + "en": "Failed to update PR #{pr}: {error}", + "pl": "Failed to update PR #{pr}: {error}", + "ru": "Failed to update PR #{pr}: {error}", + "zh": "Failed to update PR #{pr}: {error}" + }, + "Fix: add @patch decorators or with patch() context managers for subprocess/time.sleep calls, or patch the calling function.": { + "bg": "Fix: add @patch decorators or with patch() context managers for subprocess/time.sleep calls, or patch the calling function.", + "de": "Fix: add @patch decorators or with patch() context managers for subprocess/time.sleep calls, or patch the calling function.", + "en": "Fix: add @patch decorators or with patch() context managers for subprocess/time.sleep calls, or patch the calling function.", + "pl": "Fix: add @patch decorators or with patch() context managers for subprocess/time.sleep calls, or patch the calling function.", + "ru": "Fix: add @patch decorators or with patch() context managers for subprocess/time.sleep calls, or patch the calling function.", + "zh": "Fix: add @patch decorators or with patch() context managers for subprocess/time.sleep calls, or patch the calling function." + }, + "Fix: add @patch decorators or with patch() context managers for subprocess/time.sleep calls, or patch the calling function.\n": { + "bg": "Fix: add @patch decorators or with patch() context managers for subprocess/time.sleep calls, or patch the calling function.\n", + "de": "Fix: add @patch decorators or with patch() context managers for subprocess/time.sleep calls, or patch the calling function.\n", + "en": "Fix: add @patch decorators or with patch() context managers for subprocess/time.sleep calls, or patch the calling function.\n", + "pl": "Fix: add @patch decorators or with patch() context managers for subprocess/time.sleep calls, or patch the calling function.\n", + "ru": "Fix: add @patch decorators or with patch() context managers for subprocess/time.sleep calls, or patch the calling function.\n", + "zh": "Fix: add @patch decorators or with patch() context managers for subprocess/time.sleep calls, or patch the calling function.\n" + }, + "Heavy import '{mod}' (~{ms:.0f}ms) at module level — this slows test collection for all tests. Move inside test functions or use lazy import.": { + "bg": "Heavy import '{mod}' (~{ms:.0f}ms) at module level — this slows test collection for all tests. Move inside test functions or use lazy import.", + "de": "Heavy import '{mod}' (~{ms:.0f}ms) at module level — this slows test collection for all tests. Move inside test functions or use lazy import.", + "en": "Heavy import '{mod}' (~{ms:.0f}ms) at module level — this slows test collection for all tests. Move inside test functions or use lazy import.", + "pl": "Heavy import '{mod}' (~{ms:.0f}ms) at module level — this slows test collection for all tests. Move inside test functions or use lazy import.", + "ru": "Heavy import '{mod}' (~{ms:.0f}ms) at module level — this slows test collection for all tests. Move inside test functions or use lazy import.", + "zh": "Heavy import '{mod}' (~{ms:.0f}ms) at module level — this slows test collection for all tests. Move inside test functions or use lazy import." + }, + "No task ID found in branch '{branch}'. Expected format: {prefix}-N-description.": { + "bg": "No task ID found in branch '{branch}'. Expected format: {prefix}-N-description.", + "de": "No task ID found in branch '{branch}'. Expected format: {prefix}-N-description.", + "en": "No task ID found in branch '{branch}'. Expected format: {prefix}-N-description.", + "pl": "No task ID found in branch '{branch}'. Expected format: {prefix}-N-description.", + "ru": "No task ID found in branch '{branch}'. Expected format: {prefix}-N-description.", + "zh": "No task ID found in branch '{branch}'. Expected format: {prefix}-N-description." + }, + "PR number to fix": { + "bg": "PR number to fix", + "de": "PR number to fix", + "en": "PR number to fix", + "pl": "PR number to fix", + "ru": "PR number to fix", + "zh": "PR number to fix" + }, + "Real subprocess call(s) detected in test '{test}' without @patch:": { + "bg": "Real subprocess call(s) detected in test '{test}' without @patch:", + "de": "Real subprocess call(s) detected in test '{test}' without @patch:", + "en": "Real subprocess call(s) detected in test '{test}' without @patch:", + "pl": "Real subprocess call(s) detected in test '{test}' without @patch:", + "ru": "Real subprocess call(s) detected in test '{test}' without @patch:", + "zh": "Real subprocess call(s) detected in test '{test}' without @patch:" + }, + "Show what would change without updating": { + "bg": "Show what would change without updating", + "de": "Show what would change without updating", + "en": "Show what would change without updating", + "pl": "Show what would change without updating", + "ru": "Show what would change without updating", + "zh": "Show what would change without updating" + }, + "Test isolation check FAILED: {count} violation(s) in {files} file(s).": { + "bg": "Test isolation check FAILED: {count} violation(s) in {files} file(s).", + "de": "Test isolation check FAILED: {count} violation(s) in {files} file(s).", + "en": "Test isolation check FAILED: {count} violation(s) in {files} file(s).", + "pl": "Test isolation check FAILED: {count} violation(s) in {files} file(s).", + "ru": "Test isolation check FAILED: {count} violation(s) in {files} file(s).", + "zh": "Test isolation check FAILED: {count} violation(s) in {files} file(s)." + }, + "Test isolation check passed with {count} advisory warning(s) in {files} file(s).": { + "bg": "Test isolation check passed with {count} advisory warning(s) in {files} file(s).", + "de": "Test isolation check passed with {count} advisory warning(s) in {files} file(s).", + "en": "Test isolation check passed with {count} advisory warning(s) in {files} file(s).", + "pl": "Test isolation check passed with {count} advisory warning(s) in {files} file(s).", + "ru": "Test isolation check passed with {count} advisory warning(s) in {files} file(s).", + "zh": "Test isolation check passed with {count} advisory warning(s) in {files} file(s)." + }, + "Transitive-subprocess advisories (runtime audit is authoritative):": { + "bg": "Transitive-subprocess advisories (runtime audit is authoritative):", + "de": "Transitive-subprocess advisories (runtime audit is authoritative):", + "en": "Transitive-subprocess advisories (runtime audit is authoritative):", + "pl": "Transitive-subprocess advisories (runtime audit is authoritative):", + "ru": "Transitive-subprocess advisories (runtime audit is authoritative):", + "zh": "Transitive-subprocess advisories (runtime audit is authoritative):" + }, + "importlib.reload({mod}) called {n} time(s) in test '{test}' — odd count leaves module in modified state. Add a final reload to restore defaults or wrap in try/finally.": { + "bg": "importlib.reload({mod}) called {n} time(s) in test '{test}' — odd count leaves module in modified state. Add a final reload to restore defaults or wrap in try/finally.", + "de": "importlib.reload({mod}) called {n} time(s) in test '{test}' — odd count leaves module in modified state. Add a final reload to restore defaults or wrap in try/finally.", + "en": "importlib.reload({mod}) called {n} time(s) in test '{test}' — odd count leaves module in modified state. Add a final reload to restore defaults or wrap in try/finally.", + "pl": "importlib.reload({mod}) called {n} time(s) in test '{test}' — odd count leaves module in modified state. Add a final reload to restore defaults or wrap in try/finally.", + "ru": "importlib.reload({mod}) called {n} time(s) in test '{test}' — odd count leaves module in modified state. Add a final reload to restore defaults or wrap in try/finally.", + "zh": "importlib.reload({mod}) called {n} time(s) in test '{test}' — odd count leaves module in modified state. Add a final reload to restore defaults or wrap in try/finally." } } diff --git a/tests/unit/test_api_clients.py b/tests/unit/test_api_clients.py index 6738ee8..136b483 100644 --- a/tests/unit/test_api_clients.py +++ b/tests/unit/test_api_clients.py @@ -316,6 +316,18 @@ class TestGiteaClient: call_kwargs = client._session.request.call_args.kwargs assert call_kwargs["json"]["base"] == "develop" + def test_update_pr(self) -> None: + client = GiteaClient("https://git.example.com", "tok", "owner", "repo") + client._session.request = MagicMock(return_value=_mock_response({"number": 42, "title": "DEVX-99: New title"})) + result = client.update_pr(42, {"title": "DEVX-99: New title"}) + assert result["number"] == 42 + client._session.request.assert_called_once_with( + "PATCH", + "https://git.example.com/repos/owner/repo/pulls/42", + timeout=DEFAULT_TIMEOUT, + json={"title": "DEVX-99: New title"}, + ) + def test_get_pr_files(self) -> None: client = GiteaClient("https://git.example.com", "tok", "owner", "repo") client._session.request = MagicMock( diff --git a/tests/unit/test_build_image.py b/tests/unit/test_build_image.py index 20ac496..2624784 100644 --- a/tests/unit/test_build_image.py +++ b/tests/unit/test_build_image.py @@ -508,7 +508,8 @@ class TestCLIBuildImage: def test_missing_dockerfile_and_manifest(self) -> None: runner = CliRunner() - result = runner.invoke(build_image.main, []) + with patch("devx.tools.build_image.subprocess.run"): + result = runner.invoke(build_image.main, []) assert result.exit_code != 0 assert "manifest" in result.output.lower() or "dockerfile" in result.output.lower() @@ -516,10 +517,11 @@ class TestCLIBuildImage: dockerfile = tmp_path / "Dockerfile" dockerfile.touch() runner = CliRunner() - result = runner.invoke( - build_image.main, - ["--dockerfile", str(dockerfile), "--name", "ci-base", "--push"], - ) + with patch("devx.tools.build_image.subprocess.run"): + result = runner.invoke( + build_image.main, + ["--dockerfile", str(dockerfile), "--name", "ci-base", "--push"], + ) assert result.exit_code != 0 assert "registry" in result.output.lower() @@ -527,7 +529,7 @@ class TestCLIBuildImage: dockerfile = tmp_path / "Dockerfile" dockerfile.touch() runner = CliRunner() - with patch.dict("os.environ", {}, clear=True): + with patch("devx.tools.build_image.subprocess.run"), patch.dict("os.environ", {}, clear=True): result = runner.invoke( build_image.main, ["--dockerfile", str(dockerfile), "--name", "ci-base", "--push", "--registry", "git.example.com"], diff --git a/tests/unit/test_check_auto_merge_ready.py b/tests/unit/test_check_auto_merge_ready.py index 315544a..576cf86 100644 --- a/tests/unit/test_check_auto_merge_ready.py +++ b/tests/unit/test_check_auto_merge_ready.py @@ -152,7 +152,10 @@ class TestGetVikunjaTitleOptional: class TestCli: def test_fails_without_task_id(self) -> None: runner = CliRunner() - with patch.dict("os.environ", {"DEVX_TASK_PREFIX": "DEVX"}, clear=True): + with ( + patch.dict("os.environ", {"DEVX_TASK_PREFIX": "DEVX"}, clear=True), + patch("devx.ci.check_auto_merge_ready.is_branch_behind_master", return_value=False), + ): result = runner.invoke(cli, ["--branch", "no-task-id-here"]) assert result.exit_code != 0 @@ -242,6 +245,7 @@ class TestCli: runner = CliRunner() with ( patch.dict("os.environ", {"DEVX_TASK_PREFIX": "DEVX", "VIKUNJA_TOKEN": ""}, clear=True), + patch("devx.ci.check_auto_merge_ready.is_branch_behind_master", return_value=False), patch("devx.ci.check_auto_merge_ready.get_pr_title_from_gitea", return_value=None), ): result = runner.invoke( diff --git a/tests/unit/test_check_test_isolation.py b/tests/unit/test_check_test_isolation.py index 497d4de..98e35e5 100644 --- a/tests/unit/test_check_test_isolation.py +++ b/tests/unit/test_check_test_isolation.py @@ -2,14 +2,21 @@ from __future__ import annotations +import ast +import subprocess import textwrap from pathlib import Path +from unittest.mock import MagicMock from click.testing import CliRunner from devx.tools.check_test_isolation import ( HELPER_INTERNAL_CALLS, KNOWN_SUBPROCESS_HELPERS, + CallGraph, + _extract_patch_targets, + _is_integration_test, + _SubprocessAudit, analyze_file, analyze_test_files, cli, @@ -502,6 +509,97 @@ class TestAnalyzeFile: assert len(violations) == 1 assert violations[0].category == "syntax-error" + def test_heavy_module_import_at_module_level(self, tmp_path: Path) -> None: + file = _write_test_file( + tmp_path, + """ + import pandas + + def test_foo() -> None: + assert True + """, + ) + violations = analyze_file(file) + assert len(violations) == 1 + assert violations[0].category == "heavy-module-import" + assert "pandas" in violations[0].message + + def test_heavy_import_inside_function_ok(self, tmp_path: Path) -> None: + file = _write_test_file( + tmp_path, + """ + def test_foo() -> None: + import pandas + assert True + """, + ) + violations = analyze_file(file) + assert violations == [] + + def test_heavy_import_from_at_module_level(self, tmp_path: Path) -> None: + file = _write_test_file( + tmp_path, + """ + from matplotlib import pyplot as plt + + def test_foo() -> None: + assert True + """, + ) + violations = analyze_file(file) + assert len(violations) == 1 + assert violations[0].category == "heavy-module-import" + + def test_reload_without_cleanup_odd_count(self, tmp_path: Path) -> None: + file = _write_test_file( + tmp_path, + """ + import importlib + import devx.config as cfg + + def test_reload_no_cleanup() -> None: + importlib.reload(cfg) + assert cfg.TASK_PREFIX == "CUSTOM" + """, + ) + violations = analyze_file(file) + reload_violations = [v for v in violations if v.category == "reload-without-cleanup"] + assert len(reload_violations) == 1 + assert "1 time(s)" in reload_violations[0].message + + def test_reload_with_cleanup_even_count_ok(self, tmp_path: Path) -> None: + file = _write_test_file( + tmp_path, + """ + import importlib + import devx.config as cfg + + def test_reload_with_cleanup() -> None: + importlib.reload(cfg) + assert cfg.TASK_PREFIX == "CUSTOM" + importlib.reload(cfg) + """, + ) + violations = analyze_file(file) + reload_violations = [v for v in violations if v.category == "reload-without-cleanup"] + assert reload_violations == [] + + def test_reload_attribute_access_detected(self, tmp_path: Path) -> None: + file = _write_test_file( + tmp_path, + """ + import importlib + import devx.config + + def test_reload_attr() -> None: + importlib.reload(devx.config) + """, + ) + violations = analyze_file(file) + reload_violations = [v for v in violations if v.category == "reload-without-cleanup"] + assert len(reload_violations) == 1 + assert "config" in reload_violations[0].message + class TestAnalyzeTestFiles: def test_multiple_files(self, tmp_path: Path) -> None: @@ -742,7 +840,8 @@ class TestCli: assert "FAILED" in result.output assert "unpatched-subprocess" in result.output - def test_strict_flag(self, tmp_path: Path) -> None: + def test_always_strict(self, tmp_path: Path) -> None: + """CLI is always strict — no --strict flag needed.""" _write_test_file( tmp_path, """ @@ -753,7 +852,7 @@ class TestCli: """, ) runner = CliRunner() - result = runner.invoke(cli, ["--test-path", str(tmp_path), "--strict"]) + result = runner.invoke(cli, ["--test-path", str(tmp_path)]) assert result.exit_code == 1 def test_category_filter(self, tmp_path: Path) -> None: @@ -795,22 +894,6 @@ class TestCli: assert result.exit_code == 0 assert "no violations" in result.output - def test_strict_clean_directory_exits_zero(self, tmp_path: Path) -> None: - """Strict mode with no violations should still exit 0.""" - _write_test_file( - tmp_path, - """ - from unittest.mock import patch, MagicMock - class TestExample: - @patch("subprocess.run") - def test_ok(self, mock: MagicMock) -> None: - pass - """, - ) - runner = CliRunner() - result = runner.invoke(cli, ["--test-path", str(tmp_path), "--strict"]) - assert result.exit_code == 0 - class TestPytestPlugin: """Tests for the pytest plugin hooks. @@ -830,7 +913,7 @@ class TestPytestPlugin: pytest_addoption(parser) addoption_calls = parser.addoption.call_args_list - assert len(addoption_calls) >= 3 + assert len(addoption_calls) >= 2 def test_pytest_collection_finish_noop_when_disabled(self) -> None: """Plugin should skip analysis when --no-test-isolation is set.""" @@ -854,39 +937,10 @@ class TestPytestPlugin: pytest_collection_finish(session) def test_pytest_collection_finish_with_violation(self, tmp_path: Path) -> None: - """Plugin should emit warnings when violations are found.""" - import warnings + """Plugin should fail when hard violations are found (always strict).""" from unittest.mock import MagicMock - from devx.tools.check_test_isolation import pytest_collection_finish - - test_file = _write_test_file( - tmp_path, - """ - import subprocess - class TestExample: - def test_bad(self) -> None: - subprocess.run(["echo"]) - """, - ) - - session = MagicMock() - session.config.getoption.side_effect = lambda opt: False - item = MagicMock() - item.fspath = str(test_file) - session.items = [item] - - with warnings.catch_warnings(record=True) as w: - warnings.simplefilter("always") - pytest_collection_finish(session) - - assert len(w) >= 1 - assert any("Test isolation violation" in str(warning.message) for warning in w) - - def test_pytest_collection_finish_strict_mode(self, tmp_path: Path) -> None: - """Plugin should emit warnings and print summary in strict mode.""" - import warnings - from unittest.mock import MagicMock + import pytest from devx.tools.check_test_isolation import pytest_collection_finish @@ -903,7 +957,44 @@ class TestPytestPlugin: session = MagicMock() session.config.getoption.side_effect = lambda opt: { "--no-test-isolation": False, - "--strict-test-isolation": True, + "--test-isolation-max-loop": 100, + }.get(opt, False) + item = MagicMock() + item.fspath = str(test_file) + session.items = [item] + + with pytest.raises(pytest.fail.Exception, match="Test isolation"): + pytest_collection_finish(session) + + def test_pytest_collection_finish_advisory_only(self, tmp_path: Path) -> None: + """Transitive-subprocess advisories should warn, not fail.""" + import warnings + from unittest.mock import MagicMock + + from devx.tools.check_test_isolation import pytest_collection_finish + + # Create a src/ directory with a module that calls subprocess.run + # so the call graph can detect transitive subprocess calls. + src_dir = tmp_path / "src" / "mypkg" + src_dir.mkdir(parents=True) + (src_dir / "__init__.py").write_text("") + (src_dir / "cli.py").write_text("import subprocess\ndef main():\n subprocess.run(['echo'])\n") + + test_file = _write_test_file( + tmp_path, + """ + from click.testing import CliRunner + from mypkg.cli import main + class TestExample: + def test_advisory(self) -> None: + runner = CliRunner() + result = runner.invoke(main, []) + """, + ) + + session = MagicMock() + session.config.getoption.side_effect = lambda opt: { + "--no-test-isolation": False, "--test-isolation-max-loop": 100, }.get(opt, False) item = MagicMock() @@ -914,5 +1005,665 @@ class TestPytestPlugin: warnings.simplefilter("always") pytest_collection_finish(session) - assert len(w) >= 1 - assert any("Test isolation violation" in str(warning.message) for warning in w) + # Should only emit advisory warnings, not fail + assert any("advisory" in str(warning.message).lower() for warning in w) + + +class TestSubprocessAudit: + """Tests for the _SubprocessAudit runtime wrapper (lines 157-195).""" + + def test_ensure_installed_wraps_subprocess(self) -> None: + audit = _SubprocessAudit() + original_run = subprocess.run + try: + audit._ensure_installed() + assert audit._installed is True + assert "run" in audit._originals + # The subprocess.run should now be a wrapper, not the original + assert subprocess.run is not original_run + # Calling _ensure_installed again is a no-op (cached return) + audit._ensure_installed() + finally: + # Restore originals + for name, orig in audit._originals.items(): + setattr(subprocess, name, orig) + + def test_make_wrapper_records_calls_when_active(self) -> None: + audit = _SubprocessAudit() + mock_original = MagicMock(return_value="result") + wrapper = audit._make_wrapper("run", mock_original) + audit.start_test() + result = wrapper(["echo", "hi"], capture_output=True) + calls = audit.stop_test() + assert result == "result" + run_calls = [c for c in calls if c[0] == "run"] + assert len(run_calls) == 1 + assert "echo" in run_calls[0][1] + mock_original.assert_called_once_with(["echo", "hi"], capture_output=True) + + def test_make_wrapper_records_list_cmd_truncation(self) -> None: + """Long command lists should be truncated to first 4 elements.""" + audit = _SubprocessAudit() + mock_original = MagicMock(return_value="result") + wrapper = audit._make_wrapper("run", mock_original) + audit.start_test() + wrapper(["echo", "1", "2", "3", "4", "5", "6"], capture_output=True) + calls = audit.stop_test() + run_calls = [c for c in calls if c[0] == "run"] + assert len(run_calls) == 1 + assert "..." in run_calls[0][1] + + def test_make_wrapper_records_string_cmd(self) -> None: + """A string command (not list) should be recorded as-is.""" + audit = _SubprocessAudit() + mock_original = MagicMock(return_value="result") + wrapper = audit._make_wrapper("run", mock_original) + audit.start_test() + wrapper("echo hi", shell=True, capture_output=True) + calls = audit.stop_test() + run_calls = [c for c in calls if c[0] == "run"] + assert len(run_calls) == 1 + assert "echo hi" in run_calls[0][1] + + def test_calls_not_recorded_when_inactive(self) -> None: + """When audit is not active, calls should not be recorded.""" + audit = _SubprocessAudit() + mock_original = MagicMock(return_value="result") + wrapper = audit._make_wrapper("run", mock_original) + # Don't call start_test — audit inactive + wrapper(["echo", "hi"], capture_output=True) + # stop_test returns empty since no calls recorded + calls = audit.stop_test() + assert not calls + mock_original.assert_called_once_with(["echo", "hi"], capture_output=True) + + def test_start_then_stop_returns_calls(self) -> None: + """start_test initializes calls list, stop_test returns and clears it.""" + audit = _SubprocessAudit() + mock_original = MagicMock(return_value="result") + wrapper = audit._make_wrapper("run", mock_original) + audit.start_test() + wrapper(["echo"], capture_output=True) + calls = audit.stop_test() + assert len(calls) == 1 + # After stop, calls is cleared (None or empty) + calls2 = audit.stop_test() + assert not calls2 + + def test_ensure_installed_skips_missing_funcs(self) -> None: + """If a subprocess func is missing (None), it should be skipped (line 162).""" + audit = _SubprocessAudit() + saved = subprocess.check_output + try: + # Temporarily make check_output "missing" (None) + subprocess.check_output = None # type: ignore[assignment] + audit._ensure_installed() + # check_output should NOT be in originals (skipped) + assert "check_output" not in audit._originals + # run should still be wrapped + assert "run" in audit._originals + finally: + subprocess.check_output = saved # type: ignore[assignment] + for name, orig in audit._originals.items(): + setattr(subprocess, name, orig) + + +class TestExtractPatchTargets: + """Tests for _extract_patch_targets (lines 263-291).""" + + def _parse_func(self, source: str) -> ast.FunctionDef: + tree = ast.parse(textwrap.dedent(source)) + return tree.body[0] # type: ignore[return-value] + + def test_patch_object_extracted(self) -> None: + """patch.object(module, "name") should extract the short name.""" + node = self._parse_func( + """ + def test_foo(): + with patch.object(mymodule, "subprocess"): + mymodule.do_thing() + """ + ) + targets = _extract_patch_targets(node) + assert "subprocess" in targets + + def test_patch_object_with_module_alias(self) -> None: + """patch.object with a module alias Name as first arg.""" + node = self._parse_func( + """ + def test_foo(): + with patch.object(subprocess, "run"): + subprocess.run(["echo"]) + """ + ) + targets = _extract_patch_targets(node) + assert "run" in targets + + def test_with_patch_context_manager_extracted(self) -> None: + """with patch("module.func") in function body should be extracted.""" + node = self._parse_func( + """ + def test_foo(): + with patch("mymodule.subprocess.run"): + mymodule.do_thing() + """ + ) + targets = _extract_patch_targets(node) + assert "mymodule.subprocess.run" in targets + assert "run" in targets + + def test_with_multiple_patch_context_managers(self) -> None: + """with patch("a"), patch("b") should extract both.""" + node = self._parse_func( + """ + def test_foo(): + with patch("mod.a"), patch("mod.b"): + pass + """ + ) + targets = _extract_patch_targets(node) + assert "mod.a" in targets + assert "mod.b" in targets + assert "a" in targets + assert "b" in targets + + def test_patch_object_non_string_second_arg_ignored(self) -> None: + """patch.object with non-string 2nd arg should not crash.""" + node = self._parse_func( + """ + def test_foo(): + with patch.object(mymodule, some_var): + pass + """ + ) + targets = _extract_patch_targets(node) + assert targets == set() + + +class TestCallGraph: + """Tests for CallGraph building (lines 409, 419-420, 449, 470).""" + + def _make_src(self, tmp_path: Path, files: dict[str, str]) -> Path: + src = tmp_path / "src" + src.mkdir() + for rel, content in files.items(): + f = src / rel + f.parent.mkdir(parents=True, exist_ok=True) + f.write_text(textwrap.dedent(content)) + return src + + def test_ensure_built_cached(self, tmp_path: Path) -> None: + """_ensure_built should only build once (cached return).""" + src = self._make_src(tmp_path, {"pkg/__init__.py": "", "pkg/mod.py": "def foo():\n pass\n"}) + cg = CallGraph(src) + cg._ensure_built() + assert cg._built is True + nodes_before = dict(cg._nodes) + # Second call should be a no-op + cg._ensure_built() + assert cg._nodes == nodes_before + + def test_build_skips_syntax_error(self, tmp_path: Path) -> None: + """Files with syntax errors should be skipped, not crash.""" + src = self._make_src( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/broken.py": "def test(:\n pass\n", + "pkg/good.py": "def foo():\n pass\n", + }, + ) + cg = CallGraph(src) + cg._ensure_built() + # good.py's foo should be registered, broken.py skipped + assert any("foo" in k for k in cg._nodes) + + def test_build_skips_unicode_decode_error(self, tmp_path: Path) -> None: + """Files with invalid UTF-8 should be skipped.""" + src = tmp_path / "src" + src.mkdir() + (src / "pkg").mkdir() + (src / "pkg" / "__init__.py").write_text("") + (src / "pkg" / "binary.py").write_bytes(b"\xff\xfe\x00\xbad bytes") + (src / "pkg" / "good.py").write_text("def foo():\n pass\n") + cg = CallGraph(src) + cg._ensure_built() + assert any("foo" in k for k in cg._nodes) + + def test_scan_node_skips_classdef(self, tmp_path: Path) -> None: + """Methods inside classes should NOT be registered.""" + src = self._make_src( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/mod.py": """ + class MyClass: + def my_method(self): + subprocess.run(["echo"]) + def top_level(): + pass + """, + }, + ) + cg = CallGraph(src) + cg._ensure_built() + # top_level should be registered + assert "pkg.mod.top_level" in cg._nodes + # my_method should NOT be registered (class body skipped) + assert "pkg.mod.my_method" not in cg._nodes + assert "my_method" not in cg._by_short + + def test_register_function_records_io_calls(self, tmp_path: Path) -> None: + """KNOWN_IO_FUNCTIONS calls should be recorded in io_calls.""" + src = self._make_src( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/mod.py": """ + def foo(): + get_pat("staging") + load_secrets("prod") + """, + }, + ) + cg = CallGraph(src) + cg._ensure_built() + node = cg._nodes["pkg.mod.foo"] + assert "get_pat" in node.io_calls + assert "load_secrets" in node.io_calls + + def test_register_function_records_subprocess_calls(self, tmp_path: Path) -> None: + """subprocess.run calls should be recorded in subprocess_calls.""" + src = self._make_src( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/mod.py": """ + import subprocess + def foo(): + subprocess.run(["echo"]) + """, + }, + ) + cg = CallGraph(src) + cg._ensure_built() + node = cg._nodes["pkg.mod.foo"] + assert "subprocess.run" in node.subprocess_calls + + +class TestFindReachableDangerous: + """Tests for find_reachable_dangerous (lines 516-580).""" + + def _make_src(self, tmp_path: Path, files: dict[str, str]) -> Path: + src = tmp_path / "src" + src.mkdir() + for rel, content in files.items(): + f = src / rel + f.parent.mkdir(parents=True, exist_ok=True) + f.write_text(textwrap.dedent(content)) + return src + + def test_import_map_resolution(self, tmp_path: Path) -> None: + """import_map should resolve target to a precise full name.""" + src = self._make_src( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/mod.py": """ + import subprocess + def main(): + subprocess.run(["echo"]) + """, + }, + ) + cg = CallGraph(src) + dangerous = cg.find_reachable_dangerous("main", set(), import_map={"main": "pkg.mod.main"}) + assert len(dangerous) == 1 + assert "subprocess" in dangerous[0][1] + + def test_import_map_falls_back_to_short_name(self, tmp_path: Path) -> None: + """If import_map value not in nodes, fall back to short name (line 516).""" + src = self._make_src( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/mod.py": """ + import subprocess + def main(): + subprocess.run(["echo"]) + """, + }, + ) + cg = CallGraph(src) + # import_map points to a non-existent full name → fallback to by_short + dangerous = cg.find_reachable_dangerous("main", set(), import_map={"main": "nonexistent.pkg.main"}) + assert len(dangerous) == 1 + + def test_no_candidates_returns_empty(self, tmp_path: Path) -> None: + """If no candidates found, return empty list (line 526).""" + src = self._make_src(tmp_path, {"pkg/__init__.py": ""}) + cg = CallGraph(src) + dangerous = cg.find_reachable_dangerous("nonexistent", set()) + assert dangerous == [] + + def test_fully_qualified_name_candidate(self, tmp_path: Path) -> None: + """A fully-qualified target_name in nodes should be used directly (line 519).""" + src = self._make_src( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/mod.py": """ + import subprocess + def main(): + subprocess.run(["echo"]) + """, + }, + ) + cg = CallGraph(src) + dangerous = cg.find_reachable_dangerous("pkg.mod.main", set()) + assert len(dangerous) == 1 + + def test_short_name_fallback(self, tmp_path: Path) -> None: + """target_name not in nodes falls back to short name (line 522).""" + src = self._make_src( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/mod.py": """ + import subprocess + def main(): + subprocess.run(["echo"]) + """, + }, + ) + cg = CallGraph(src) + # "pkg.main" is not a full name in nodes, so it falls back to "main" + dangerous = cg.find_reachable_dangerous("pkg.main", set()) + assert len(dangerous) == 1 + + def test_visited_prevents_infinite_loop(self, tmp_path: Path) -> None: + """Visited set prevents infinite loops (line 535).""" + src = self._make_src( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/mod.py": """ + def a(): + b() + def b(): + a() + subprocess.run(["echo"]) + """, + }, + ) + cg = CallGraph(src) + dangerous = cg.find_reachable_dangerous("a", set()) + assert len(dangerous) == 1 + + def test_depth_limit_stops_traversal(self, tmp_path: Path) -> None: + """max_depth should stop traversal (line 534).""" + src = self._make_src( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/mod.py": """ + def a(): + b() + def b(): + c() + def c(): + subprocess.run(["echo"]) + """, + }, + ) + cg = CallGraph(src) + # With max_depth=0, only the direct node is visited + dangerous = cg.find_reachable_dangerous("a", set(), max_depth=0) + assert dangerous == [] + + def test_node_not_found_continues(self, tmp_path: Path) -> None: + """If a queued node isn't in _nodes, continue (line 540).""" + src = self._make_src(tmp_path, {"pkg/__init__.py": ""}) + cg = CallGraph(src) + # Manually inject a candidate that doesn't exist in nodes + cg._by_short["ghost"] = ["pkg.mod.ghost"] + dangerous = cg.find_reachable_dangerous("ghost", set()) + assert dangerous == [] + + def test_io_calls_checked(self, tmp_path: Path) -> None: + """IO calls should be reported as dangerous (lines 551-554).""" + src = self._make_src( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/mod.py": """ + def main(): + get_pat("staging") + """, + }, + ) + cg = CallGraph(src) + dangerous = cg.find_reachable_dangerous("main", set()) + assert len(dangerous) == 1 + assert "PAT" in dangerous[0][1] or "get_pat" in str(dangerous) + + def test_io_calls_patched_skipped(self, tmp_path: Path) -> None: + """Patched IO calls should not be reported.""" + src = self._make_src( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/mod.py": """ + def main(): + get_pat("staging") + """, + }, + ) + cg = CallGraph(src) + dangerous = cg.find_reachable_dangerous("main", {"get_pat"}) + assert dangerous == [] + + def test_patched_helper_skipped_in_enqueue(self, tmp_path: Path) -> None: + """A patched helper should not be enqueued (lines 559-560).""" + src = self._make_src( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/mod.py": """ + def main(): + run_cmd() + def run_cmd(): + subprocess.run(["echo"]) + """, + }, + ) + cg = CallGraph(src) + # run_cmd is patched → should not traverse into it + dangerous = cg.find_reachable_dangerous("main", {"run_cmd"}) + assert dangerous == [] + + def test_same_module_resolution(self, tmp_path: Path) -> None: + """Calls within the same module should prefer same-module resolution (lines 566-567).""" + src = self._make_src( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/mod.py": """ + def main(): + helper() + def helper(): + subprocess.run(["echo"]) + """, + }, + ) + cg = CallGraph(src) + dangerous = cg.find_reachable_dangerous("main", set()) + assert len(dangerous) == 1 + + def test_short_name_single_match_resolution(self, tmp_path: Path) -> None: + """A single global match by short name should be resolved (lines 571-572).""" + src = self._make_src( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/mod.py": """ + def main(): + helper() + """, + "pkg/other.py": """ + import subprocess + def helper(): + subprocess.run(["echo"]) + """, + }, + ) + cg = CallGraph(src) + dangerous = cg.find_reachable_dangerous("main", set()) + assert len(dangerous) == 1 + + def test_is_patched_endswith(self, tmp_path: Path) -> None: + """_is_patched should match patches ending with .short (line 580).""" + src = self._make_src( + tmp_path, + { + "pkg/__init__.py": "", + "pkg/mod.py": """ + import subprocess + def main(): + subprocess.run(["echo"]) + """, + }, + ) + cg = CallGraph(src) + # "devx.ci.release.subprocess.run" ends with ".run" + dangerous = cg.find_reachable_dangerous("main", {"devx.ci.release.subprocess.run"}) + assert dangerous == [] + + def test_is_patched_full_name_match(self) -> None: + """_is_patched should match exact full name.""" + assert CallGraph._is_patched("subprocess.run", "run", {"subprocess.run"}) is True + + def test_is_patched_short_name_match(self) -> None: + """_is_patched should match short name in patches.""" + assert CallGraph._is_patched("subprocess.run", "run", {"run"}) is True + + def test_is_patched_no_match(self) -> None: + """_is_patched should return False when not patched.""" + assert CallGraph._is_patched("subprocess.run", "run", {"other"}) is False + + def test_is_patched_endswith_no_false_positive(self) -> None: + """endswith should not match substrings (e.g. 'run' vs 'run_cmd').""" + assert CallGraph._is_patched("mod.run_cmd", "run_cmd", {"mod.run"}) is False + + +class TestVisitCallAttributeTarget: + """Tests for visit_Call with ast.Attribute target (lines 851-862).""" + + def test_invoke_with_module_func_attribute(self, tmp_path: Path) -> None: + """runner.invoke(module.func) should resolve via import_map.""" + src = tmp_path / "src" + src.mkdir() + (src / "pkg").mkdir() + (src / "pkg" / "__init__.py").write_text("") + (src / "pkg" / "cli.py").write_text("import subprocess\ndef main():\n subprocess.run(['echo'])\n") + + test_file = tmp_path / "test_example.py" + test_file.write_text( + textwrap.dedent( + """ + from click.testing import CliRunner + import pkg.cli as cli_mod + class TestExample: + def test_invoke(self) -> None: + runner = CliRunner() + result = runner.invoke(cli_mod.main, []) + """ + ) + ) + cg = CallGraph(src) + violations = analyze_file(test_file, call_graph=cg) + transitive = [v for v in violations if v.category == "transitive-subprocess"] + assert len(transitive) == 1 + + def test_invoke_with_attribute_no_import_map(self, tmp_path: Path) -> None: + """runner.invoke(mod.func) where mod not in import_map uses attr only (line 860).""" + src = tmp_path / "src" + src.mkdir() + (src / "pkg").mkdir() + (src / "pkg" / "__init__.py").write_text("") + (src / "pkg" / "cli.py").write_text("import subprocess\ndef main():\n subprocess.run(['echo'])\n") + + test_file = tmp_path / "test_example.py" + test_file.write_text( + textwrap.dedent( + """ + from click.testing import CliRunner + class TestExample: + def test_invoke(self) -> None: + runner = CliRunner() + # unknown_mod not imported, so falls back to attr name + result = runner.invoke(unknown_mod.main, []) + """ + ) + ) + cg = CallGraph(src) + violations = analyze_file(test_file, call_graph=cg) + transitive = [v for v in violations if v.category == "transitive-subprocess"] + assert len(transitive) == 1 + + def test_invoke_with_attribute_non_name_value(self, tmp_path: Path) -> None: + """runner.invoke(get_obj().func) — target.value is not a Name (line 862).""" + src = tmp_path / "src" + src.mkdir() + (src / "pkg").mkdir() + (src / "pkg" / "__init__.py").write_text("") + (src / "pkg" / "cli.py").write_text("import subprocess\ndef main():\n subprocess.run(['echo'])\n") + + test_file = tmp_path / "test_example.py" + test_file.write_text( + textwrap.dedent( + """ + from click.testing import CliRunner + class TestExample: + def test_invoke(self) -> None: + runner = CliRunner() + result = runner.invoke(CliRunner().main, []) + """ + ) + ) + cg = CallGraph(src) + violations = analyze_file(test_file, call_graph=cg) + transitive = [v for v in violations if v.category == "transitive-subprocess"] + assert len(transitive) == 1 + + +class TestIsIntegrationTest: + """Tests for _is_integration_test (lines 1076-1080).""" + + def test_marker_based_integration(self) -> None: + """A test item with 'integration' in keywords should be detected.""" + item = MagicMock() + item.keywords = {"integration", "test_foo"} + item.fspath = "tests/unit/test_foo.py" + assert _is_integration_test(item) is True + + def test_path_based_integration(self) -> None: + """A test item in an integration/ directory should be detected.""" + item = MagicMock() + item.keywords = {"test_foo"} + item.fspath = "tests/integration/test_foo.py" + assert _is_integration_test(item) is True + + def test_not_integration_test(self) -> None: + """A regular test item should not be detected as integration.""" + item = MagicMock() + item.keywords = {"test_foo"} + item.fspath = "tests/unit/test_foo.py" + assert _is_integration_test(item) is False + + def test_no_keywords_attr(self) -> None: + """An item without keywords attr should use fspath only.""" + item = MagicMock() + item.keywords = {} + item.fspath = "tests/unit/test_foo.py" + assert _is_integration_test(item) is False diff --git a/tests/unit/test_classify_changes.py b/tests/unit/test_classify_changes.py index 725b2de..f870cab 100644 --- a/tests/unit/test_classify_changes.py +++ b/tests/unit/test_classify_changes.py @@ -481,8 +481,9 @@ class TestRunGit: class TestMain: + @patch("devx.ci.classify_changes.get_changed_files", return_value=[]) @patch("devx.ci.classify_changes.get_latest_tag", return_value="") - def test_no_tags_outputs_true(self, mock_tag: MagicMock) -> None: + def test_no_tags_outputs_true(self, mock_tag: MagicMock, mock_changes: MagicMock) -> None: runner = CliRunner() result = runner.invoke(main, ["--quiet"]) assert result.exit_code == 0 @@ -555,8 +556,9 @@ class TestMain: # docs tag has no matching files — should not appear assert "Docs files" not in result.output + @patch("devx.ci.classify_changes.get_changed_files", return_value=[]) @patch("devx.ci.classify_changes.get_latest_tag", return_value="") - def test_no_tags_non_quiet(self, mock_tag: MagicMock) -> None: + def test_no_tags_non_quiet(self, mock_tag: MagicMock, mock_changes: MagicMock) -> None: runner = CliRunner() result = runner.invoke(main, []) assert result.exit_code == 0 @@ -731,7 +733,10 @@ class TestGithubOutput: mock_clf.return_value = self._make_classifier_with_ansible() gh_file = tmp_path / "output.txt" monkeypatch.setenv("GITHUB_OUTPUT", str(gh_file)) - with patch.object(classify_changes_mod, "get_latest_tag", return_value=""): + with ( + patch.object(classify_changes_mod, "get_latest_tag", return_value=""), + patch.object(classify_changes_mod, "get_changed_files", return_value=["src/cli.py"]), + ): runner = CliRunner() result = runner.invoke(main, ["--github-output"]) assert result.exit_code == 0 @@ -774,7 +779,10 @@ class TestGithubOutput: ) gh_file = tmp_path / "output.txt" monkeypatch.setenv("GITHUB_OUTPUT", str(gh_file)) - with patch.object(classify_changes_mod, "get_latest_tag", return_value=""): + with ( + patch.object(classify_changes_mod, "get_latest_tag", return_value=""), + patch.object(classify_changes_mod, "get_changed_files", return_value=["src/cli.py"]), + ): runner = CliRunner() result = runner.invoke(main, ["--github-output"]) assert result.exit_code == 0 @@ -789,8 +797,9 @@ class TestGithubOutput: mock_clf.return_value = self._make_classifier_with_ansible() gh_file = tmp_path / "output.txt" monkeypatch.setenv("GITHUB_OUTPUT", str(gh_file)) - runner = CliRunner() - result = runner.invoke(main, ["--github-output", "--force"]) + with patch.object(classify_changes_mod, "get_changed_files", return_value=["src/cli.py"]): + runner = CliRunner() + result = runner.invoke(main, ["--github-output", "--force"]) assert result.exit_code == 0 content = gh_file.read_text() assert "user-facing-changed=true" in content @@ -822,8 +831,9 @@ class TestGithubOutput: ) gh_file = tmp_path / "output.txt" monkeypatch.setenv("GITHUB_OUTPUT", str(gh_file)) - runner = CliRunner() - result = runner.invoke(main, ["--github-output", "--force"]) + with patch.object(classify_changes_mod, "get_changed_files", return_value=["src/cli.py"]): + runner = CliRunner() + result = runner.invoke(main, ["--github-output", "--force"]) assert result.exit_code == 0 content = gh_file.read_text() assert "user-facing-changed=true" in content @@ -836,8 +846,9 @@ class TestGithubOutput: gh_file = tmp_path / "output.txt" monkeypatch.setenv("GITHUB_OUTPUT", str(gh_file)) monkeypatch.setenv("FORCE_DEPLOY", "true") - runner = CliRunner() - result = runner.invoke(main, ["--github-output"]) + with patch.object(classify_changes_mod, "get_changed_files", return_value=["src/cli.py"]): + runner = CliRunner() + result = runner.invoke(main, ["--github-output"]) assert result.exit_code == 0 content = gh_file.read_text() assert "user-facing-changed=true" in content @@ -869,8 +880,9 @@ class TestGithubOutput: gh_file = tmp_path / "output.txt" monkeypatch.setenv("GITHUB_OUTPUT", str(gh_file)) monkeypatch.setenv("FORCE_DEPLOY", "false") - runner = CliRunner() - result = runner.invoke(main, ["--github-output", "--force"]) + with patch.object(classify_changes_mod, "get_changed_files", return_value=["src/cli.py"]): + runner = CliRunner() + result = runner.invoke(main, ["--github-output", "--force"]) assert result.exit_code == 0 content = gh_file.read_text() assert "user-facing-changed=true" in content diff --git a/tests/unit/test_create_pr.py b/tests/unit/test_create_pr.py index e0606ce..8b5afd8 100644 --- a/tests/unit/test_create_pr.py +++ b/tests/unit/test_create_pr.py @@ -144,19 +144,21 @@ class TestCli: assert result.exit_code == 0 mock_create.assert_called_once_with("DEVX-42-fix", "master", "", "owner", "repo") + @patch("devx.tools.create_pr.subprocess.run") @patch("devx.tools.create_pr.create_pr") @patch("devx.tools.create_pr.REPO_OWNER", "owner") @patch("devx.tools.create_pr.get_repo_name", return_value="repo") - def test_explicit_branch(self, mock_repo: MagicMock, mock_create: MagicMock) -> None: + def test_explicit_branch(self, mock_repo: MagicMock, mock_create: MagicMock, mock_subproc: MagicMock) -> None: mock_create.return_value = {"number": 1} runner = CliRunner() result = runner.invoke(cli, ["--branch", "DEVX-42-fix"]) assert result.exit_code == 0 + @patch("devx.tools.create_pr.subprocess.run") @patch("devx.tools.create_pr.create_pr") @patch("devx.tools.create_pr.REPO_OWNER", "owner") @patch("devx.tools.create_pr.get_repo_name", return_value="repo") - def test_body_from_stdin(self, mock_repo: MagicMock, mock_create: MagicMock) -> None: + def test_body_from_stdin(self, mock_repo: MagicMock, mock_create: MagicMock, mock_subproc: MagicMock) -> None: mock_create.return_value = {"number": 1} runner = CliRunner() result = runner.invoke(cli, ["--branch", "DEVX-42-fix", "--body", "-"], input="PR body text") @@ -164,17 +166,19 @@ class TestCli: mock_create.assert_called_once() assert mock_create.call_args.args[2] == "PR body text" + @patch("devx.tools.create_pr.subprocess.run") @patch("devx.tools.create_pr.REPO_OWNER", "") @patch("devx.tools.create_pr.get_repo_name", return_value="repo") - def test_missing_owner(self, mock_repo: MagicMock) -> None: + def test_missing_owner(self, mock_repo: MagicMock, mock_subproc: MagicMock) -> None: runner = CliRunner() result = runner.invoke(cli, ["--branch", "DEVX-42-fix"]) assert result.exit_code != 0 assert "owner" in result.output.lower() + @patch("devx.tools.create_pr.subprocess.run") @patch("devx.tools.create_pr.create_pr") @patch("devx.tools.create_pr.get_repo_name", return_value="repo") - def test_explicit_owner(self, mock_repo: MagicMock, mock_create: MagicMock) -> None: + def test_explicit_owner(self, mock_repo: MagicMock, mock_create: MagicMock, mock_subproc: MagicMock) -> None: mock_create.return_value = {"number": 1} runner = CliRunner() result = runner.invoke(cli, ["--branch", "DEVX-42-fix", "--owner", "custom"]) diff --git a/tests/unit/test_docker_login.py b/tests/unit/test_docker_login.py index 24d004f..b36502c 100644 --- a/tests/unit/test_docker_login.py +++ b/tests/unit/test_docker_login.py @@ -90,8 +90,9 @@ class TestCli: assert result.exit_code == 0 mock_login.assert_called_once_with("reg.io", "emil", "tok", suppress_failure=False) + @patch("devx.tools.docker_login.docker_login") @patch.dict("os.environ", {}, clear=True) - def test_required_no_token_raises(self) -> None: + def test_required_no_token_raises(self, mock_login: MagicMock) -> None: runner = CliRunner() result = runner.invoke( cli, @@ -99,8 +100,9 @@ class TestCli: ) assert result.exit_code != 0 + @patch("devx.tools.docker_login.docker_login") @patch.dict("os.environ", {}, clear=True) - def test_optional_no_token_skips(self) -> None: + def test_optional_no_token_skips(self, mock_login: MagicMock) -> None: runner = CliRunner() result = runner.invoke( cli, diff --git a/tests/unit/test_fix_pr_title.py b/tests/unit/test_fix_pr_title.py new file mode 100644 index 0000000..6dfd043 --- /dev/null +++ b/tests/unit/test_fix_pr_title.py @@ -0,0 +1,195 @@ +"""Tests for devx.ci.fix_pr_title.""" + +from __future__ import annotations + +from unittest.mock import MagicMock, patch + +import pytest +from click.testing import CliRunner + +from devx.ci.fix_pr_title import cli + + +@pytest.fixture(autouse=True) +def _obl_infra_prefix(monkeypatch: pytest.MonkeyPatch) -> None: + """Use OBL-INFRA prefix to match infra repo conventions.""" + import re + + monkeypatch.setattr("devx.ci.fix_pr_title.TASK_PREFIX", "OBL-INFRA") + monkeypatch.setattr("devx.ci.auto_merge.TASK_PREFIX", "OBL-INFRA") + monkeypatch.setattr("devx.ci.auto_merge.PR_TITLE_RE", re.compile(r"^OBL-INFRA-\d+:\s+.+")) + monkeypatch.setattr("devx.ci.auto_merge._TASK_ID_PREFIX_RE", re.compile(r"^OBL-INFRA-\d+:\s*")) + monkeypatch.setattr("devx.ci._shared.TASK_ID_RE", re.compile(r"OBL-INFRA-\d+")) + + +class TestFixPrTitle: + @patch("devx.ci.fix_pr_title.get_ci_token") + @patch("devx.ci.fix_pr_title.GiteaClient") + @patch("devx.ci.fix_pr_title.get_vikunja_title_optional") + def test_fixes_title_with_vikunja( + self, + mock_vikunja: MagicMock, + mock_gitea_cls: MagicMock, + mock_ci_token: MagicMock, + ) -> None: + """PR title is updated to match task ID + Vikunja title.""" + mock_client = MagicMock() + mock_gitea_cls.return_value = mock_client + mock_client.get_pr.return_value = { + "number": 42, + "title": "Fix blackbox exporter", + "head": {"ref": "OBL-INFRA-458-blackbox-ipv4"}, + } + mock_vikunja.return_value = "Fix blackbox exporter IPv4 config" + mock_ci_token.return_value = "token" + + runner = CliRunner() + result = runner.invoke(cli, ["--repo", "oblachno/infra", "--pr-number", "42"]) + + assert result.exit_code == 0 + mock_client.update_pr.assert_called_once_with(42, {"title": "OBL-INFRA-458: Fix blackbox exporter IPv4 config"}) + + @patch("devx.ci.fix_pr_title.get_ci_token") + @patch("devx.ci.fix_pr_title.GiteaClient") + @patch("devx.ci.fix_pr_title.get_vikunja_title_optional") + def test_strips_conventional_commit_prefix_when_no_vikunja( + self, + mock_vikunja: MagicMock, + mock_gitea_cls: MagicMock, + mock_ci_token: MagicMock, + ) -> None: + """When Vikunja task not found, strips conventional-commit prefix from current title.""" + mock_client = MagicMock() + mock_gitea_cls.return_value = mock_client + mock_client.get_pr.return_value = { + "number": 10, + "title": "fix: platform self-monitoring and fixes", + "head": {"ref": "OBL-INFRA-456-platform-fixes"}, + } + mock_vikunja.return_value = None + mock_ci_token.return_value = "token" + + runner = CliRunner() + result = runner.invoke(cli, ["--repo", "oblachno/infra", "--pr-number", "10"]) + + assert result.exit_code == 0 + mock_client.update_pr.assert_called_once_with( + 10, {"title": "OBL-INFRA-456: platform self-monitoring and fixes"} + ) + + @patch("devx.ci.fix_pr_title.get_ci_token") + @patch("devx.ci.fix_pr_title.GiteaClient") + @patch("devx.ci.fix_pr_title.get_vikunja_title_optional") + def test_already_correct_title_no_update( + self, + mock_vikunja: MagicMock, + mock_gitea_cls: MagicMock, + mock_ci_token: MagicMock, + ) -> None: + """When PR title is already correct, no update is made.""" + mock_client = MagicMock() + mock_gitea_cls.return_value = mock_client + mock_client.get_pr.return_value = { + "number": 5, + "title": "OBL-INFRA-100: Fix bug", + "head": {"ref": "OBL-INFRA-100-fix-bug"}, + } + mock_vikunja.return_value = "Fix bug" + mock_ci_token.return_value = "token" + + runner = CliRunner() + result = runner.invoke(cli, ["--repo", "oblachno/infra", "--pr-number", "5"]) + + assert result.exit_code == 0 + mock_client.update_pr.assert_not_called() + + @patch("devx.ci.fix_pr_title.get_ci_token") + @patch("devx.ci.fix_pr_title.GiteaClient") + @patch("devx.ci.fix_pr_title.get_vikunja_title_optional") + def test_dry_run_no_update( + self, + mock_vikunja: MagicMock, + mock_gitea_cls: MagicMock, + mock_ci_token: MagicMock, + ) -> None: + """Dry run shows what would change without updating.""" + mock_client = MagicMock() + mock_gitea_cls.return_value = mock_client + mock_client.get_pr.return_value = { + "number": 7, + "title": "Fix thing", + "head": {"ref": "OBL-INFRA-7-fix-thing"}, + } + mock_vikunja.return_value = "Fix thing" + mock_ci_token.return_value = "token" + + runner = CliRunner() + result = runner.invoke(cli, ["--repo", "oblachno/infra", "--pr-number", "7", "--dry-run"]) + + assert result.exit_code == 0 + mock_client.update_pr.assert_not_called() + + @patch("devx.ci.fix_pr_title.get_ci_token") + @patch("devx.ci.fix_pr_title.GiteaClient") + def test_no_task_id_in_branch_exits_error( + self, + mock_gitea_cls: MagicMock, + mock_ci_token: MagicMock, + ) -> None: + """When branch has no task ID, exits with error.""" + mock_client = MagicMock() + mock_gitea_cls.return_value = mock_client + mock_client.get_pr.return_value = { + "number": 1, + "title": "Some title", + "head": {"ref": "just-a-branch"}, + } + mock_ci_token.return_value = "token" + + runner = CliRunner() + result = runner.invoke(cli, ["--repo", "oblachno/infra", "--pr-number", "1"]) + + assert result.exit_code != 0 + mock_client.update_pr.assert_not_called() + + @patch("devx.ci.fix_pr_title.get_ci_token") + def test_no_token_exits_error(self, mock_ci_token: MagicMock) -> None: + """When CI token is not set, exits with error.""" + mock_ci_token.side_effect = Exception("no token") + + runner = CliRunner() + result = runner.invoke(cli, ["--repo", "oblachno/infra", "--pr-number", "1"]) + + assert result.exit_code != 0 + + @patch("devx.ci.fix_pr_title.get_ci_token") + @patch("devx.ci.fix_pr_title.GiteaClient") + @patch("devx.ci.fix_pr_title.get_vikunja_title_optional") + def test_strips_task_id_prefix_from_vikunja_title( + self, + mock_vikunja: MagicMock, + mock_gitea_cls: MagicMock, + mock_ci_token: MagicMock, + ) -> None: + """When Vikunja title already has task ID prefix, it's stripped to avoid double prefix.""" + mock_client = MagicMock() + mock_gitea_cls.return_value = mock_client + mock_client.get_pr.return_value = { + "number": 99, + "title": "Fix thing", + "head": {"ref": "OBL-INFRA-99-fix-thing"}, + } + mock_vikunja.return_value = "OBL-INFRA-99: Fix thing" + mock_ci_token.return_value = "token" + + runner = CliRunner() + result = runner.invoke(cli, ["--repo", "oblachno/infra", "--pr-number", "99"]) + + assert result.exit_code == 0 + mock_client.update_pr.assert_called_once_with(99, {"title": "OBL-INFRA-99: Fix thing"}) + + def test_invalid_repo_format_exits_error(self) -> None: + """When repo is not in owner/name format, exits with error.""" + runner = CliRunner() + result = runner.invoke(cli, ["--repo", "invalid", "--pr-number", "1"]) + assert result.exit_code != 0 diff --git a/tests/unit/test_install_checkmake.py b/tests/unit/test_install_checkmake.py index 3abd06c..07ef6cd 100644 --- a/tests/unit/test_install_checkmake.py +++ b/tests/unit/test_install_checkmake.py @@ -66,7 +66,10 @@ class TestMain: def test_already_installed(self) -> None: from click.testing import CliRunner - with patch("shutil.which", return_value="/usr/bin/checkmake"): + with ( + patch("shutil.which", return_value="/usr/bin/checkmake"), + patch("devx.tools.install_checkmake._install_with_go"), + ): runner = CliRunner() runner.invoke(install_checkmake.cli, []) @@ -98,7 +101,10 @@ class TestMain: with patch.object(install_checkmake, "TARGET_PATH", target): with patch("shutil.which", side_effect=[None, None]): with patch.object(platform, "machine", return_value="x86_64"): - with patch("urllib.request.urlretrieve", side_effect=_write_file) as mock_retrieve: + with ( + patch("urllib.request.urlretrieve", side_effect=_write_file) as mock_retrieve, + patch("devx.tools.install_checkmake._install_with_go", return_value=False), + ): runner = CliRunner() runner.invoke(install_checkmake.cli, []) mock_retrieve.assert_called_once() diff --git a/tests/unit/test_molecule_all.py b/tests/unit/test_molecule_all.py index 1616446..d21c445 100644 --- a/tests/unit/test_molecule_all.py +++ b/tests/unit/test_molecule_all.py @@ -1,7 +1,7 @@ from __future__ import annotations from pathlib import Path -from unittest.mock import patch +from unittest.mock import MagicMock, patch import pytest from click.testing import CliRunner @@ -87,14 +87,16 @@ class TestRunPlatform: class TestMain: - def test_molecule_not_found(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + @patch("devx.molecule.molecule_all._run_molecule") + def test_molecule_not_found(self, mock_run: MagicMock, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.chdir(tmp_path) runner = CliRunner() result = runner.invoke(molecule_all.main, ["--bin", "nonexistent/bin"]) assert result.exit_code != 0 assert "molecule not found" in result.output - def test_role_dir_not_found(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + @patch("devx.molecule.molecule_all._run_molecule") + def test_role_dir_not_found(self, mock_run: MagicMock, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.chdir(tmp_path) bin_dir = tmp_path / ".venv" / "bin" bin_dir.mkdir(parents=True) @@ -104,7 +106,8 @@ class TestMain: assert result.exit_code != 0 assert "Role directory not found" in result.output - def test_role_dir_no_molecule(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + @patch("devx.molecule.molecule_all._run_molecule") + def test_role_dir_no_molecule(self, mock_run: MagicMock, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: """Roles dir exists but no role has molecule/ — should error.""" monkeypatch.chdir(tmp_path) bin_dir = tmp_path / ".venv" / "bin" diff --git a/tests/unit/test_molecule_ci_guard.py b/tests/unit/test_molecule_ci_guard.py index 55d1729..4fd4a9f 100644 --- a/tests/unit/test_molecule_ci_guard.py +++ b/tests/unit/test_molecule_ci_guard.py @@ -165,6 +165,7 @@ class TestCli: with ( patch("devx.molecule.molecule_ci_guard.subprocess.Popen") as mock_popen, + patch("devx.molecule.molecule_ci_guard.subprocess.run"), patch("devx.molecule.molecule_ci_guard.subprocess.run") as mock_run, patch("time.sleep"), ): @@ -193,6 +194,7 @@ class TestCli: with ( patch("devx.molecule.molecule_ci_guard.subprocess.Popen") as mock_popen, + patch("devx.molecule.molecule_ci_guard.subprocess.run"), patch("devx.molecule.molecule_ci_guard.subprocess.run") as mock_run, patch("devx.molecule.molecule_ci_guard.poll_for_other_failures") as mock_poll, patch("time.sleep"), @@ -212,8 +214,12 @@ class TestCli: """Pair with fewer than 2 parts should raise.""" from click.testing import CliRunner - runner = CliRunner() - result = runner.invoke(cli, ["invalid_no_pipe"]) + with ( + patch("devx.molecule.molecule_ci_guard.subprocess.Popen"), + patch("devx.molecule.molecule_ci_guard.subprocess.run"), + ): + runner = CliRunner() + result = runner.invoke(cli, ["invalid_no_pipe"]) assert result.exit_code != 0 assert "Invalid pair format" in result.output @@ -222,6 +228,8 @@ class TestCli: with ( patch("devx.molecule.molecule_ci_guard.subprocess.Popen") as mock_popen, + patch("devx.molecule.molecule_ci_guard.subprocess.run"), + patch("devx.molecule.molecule_ci_guard.subprocess.run"), patch("time.sleep"), ): proc = MagicMock() @@ -253,6 +261,8 @@ class TestCli: ), patch("devx.molecule.molecule_ci_guard.POLL_INTERVAL", 0.01), patch("devx.molecule.molecule_ci_guard.subprocess.Popen") as mock_popen, + patch("devx.molecule.molecule_ci_guard.subprocess.run"), + patch("devx.molecule.molecule_ci_guard.subprocess.run"), patch("devx.molecule.molecule_ci_guard.get_running_jobs") as mock_get_jobs, patch("time.sleep"), ): @@ -275,6 +285,8 @@ class TestCli: with ( patch("devx.molecule.molecule_ci_guard.subprocess.Popen") as mock_popen, + patch("devx.molecule.molecule_ci_guard.subprocess.run"), + patch("devx.molecule.molecule_ci_guard.subprocess.run"), patch("time.sleep", side_effect=KeyboardInterrupt), patch("os.killpg") as mock_killpg, patch("os.getpgid") as mock_getpgid, @@ -321,6 +333,7 @@ class TestCli: ), patch("devx.molecule.molecule_ci_guard.POLL_INTERVAL", 0.01), patch("devx.molecule.molecule_ci_guard.subprocess.Popen") as mock_popen, + patch("devx.molecule.molecule_ci_guard.subprocess.run"), patch("devx.molecule.molecule_ci_guard.get_running_jobs", side_effect=get_jobs_side_effect), patch("os.killpg") as mock_killpg, patch("os.getpgid") as mock_getpgid, @@ -358,6 +371,7 @@ class TestCli: ), patch("devx.molecule.molecule_ci_guard.POLL_INTERVAL", 0.01), patch("devx.molecule.molecule_ci_guard.subprocess.Popen") as mock_popen, + patch("devx.molecule.molecule_ci_guard.subprocess.run"), patch("devx.molecule.molecule_ci_guard.subprocess.run") as mock_run, patch("devx.molecule.molecule_ci_guard.get_running_jobs") as mock_get_jobs, patch("time.sleep", side_effect=lambda x: real_sleep(0)), @@ -404,6 +418,7 @@ class TestCli: ), patch("devx.molecule.molecule_ci_guard.POLL_INTERVAL", 0.01), patch("devx.molecule.molecule_ci_guard.subprocess.Popen") as mock_popen, + patch("devx.molecule.molecule_ci_guard.subprocess.run"), patch("devx.molecule.molecule_ci_guard.get_running_jobs", side_effect=get_jobs_side_effect), patch("os.killpg") as mock_killpg, patch("os.getpgid") as mock_getpgid, @@ -451,6 +466,7 @@ class TestCli: ), patch("devx.molecule.molecule_ci_guard.POLL_INTERVAL", 0.01), patch("devx.molecule.molecule_ci_guard.subprocess.Popen") as mock_popen, + patch("devx.molecule.molecule_ci_guard.subprocess.run"), patch("devx.molecule.molecule_ci_guard.get_running_jobs", side_effect=get_jobs_side_effect), patch("os.killpg") as mock_killpg, patch("os.getpgid") as mock_getpgid, @@ -546,6 +562,7 @@ class TestCliMultiRole: with ( patch("devx.molecule.molecule_ci_guard.subprocess.Popen") as mock_popen, + patch("devx.molecule.molecule_ci_guard.subprocess.run"), patch("devx.molecule.molecule_ci_guard.subprocess.run") as mock_run, patch("time.sleep"), ): diff --git a/tests/unit/test_notify_failure.py b/tests/unit/test_notify_failure.py index 39d1361..ea2fef1 100644 --- a/tests/unit/test_notify_failure.py +++ b/tests/unit/test_notify_failure.py @@ -9,9 +9,10 @@ from devx.gitea_cli import TeaCLIError, configure_tea_login class TestNotifyFailure: + @patch("devx.gitea_cli.configure_tea_login") @patch.dict("os.environ", {"CI_GITEA_TOKEN": "tok"}) @patch("devx.ci.notify_failure.TeaCLI") - def test_creates_issue_with_tea(self, mock_tea_cls: MagicMock) -> None: + def test_creates_issue_with_tea(self, mock_tea_cls: MagicMock, mock_login: MagicMock) -> None: mock_tea = MagicMock() mock_tea.list_labels.return_value = [{"id": 5, "name": "bug"}] mock_tea.create_issue.return_value = {"index": 42, "title": "test"} @@ -36,9 +37,10 @@ class TestNotifyFailure: mock_tea.create_issue.assert_called_once() mock_tea.add_label.assert_called_once_with("owner/repo", 42, ["bug"]) + @patch("devx.gitea_cli.configure_tea_login") @patch.dict("os.environ", {"CI_GITEA_TOKEN": "tok"}) @patch("devx.ci.notify_failure.TeaCLI") - def test_tea_creates_issue_without_bug_label(self, mock_tea_cls: MagicMock) -> None: + def test_tea_creates_issue_without_bug_label(self, mock_tea_cls: MagicMock, mock_login: MagicMock) -> None: mock_tea = MagicMock() mock_tea.list_labels.return_value = [{"id": 1, "name": "enhancement"}] mock_tea.create_issue.return_value = {"index": 43, "title": "test"} @@ -53,9 +55,10 @@ class TestNotifyFailure: assert "issue #43" in result.output mock_tea.add_label.assert_not_called() + @patch("devx.gitea_cli.configure_tea_login") @patch.dict("os.environ", {"CI_GITEA_TOKEN": "tok"}) @patch("devx.ci.notify_failure.TeaCLI") - def test_tea_error_raises(self, mock_tea_cls: MagicMock) -> None: + def test_tea_error_raises(self, mock_tea_cls: MagicMock, mock_login: MagicMock) -> None: """When tea fails, the workflow fails — no fallback.""" mock_tea = MagicMock() mock_tea.list_labels.side_effect = TeaCLIError("network error") @@ -70,9 +73,12 @@ class TestNotifyFailure: assert result.exit_code != 0 assert "tea" in result.output.lower() + @patch("devx.gitea_cli.configure_tea_login") @patch.dict("os.environ", {"CI_GITEA_TOKEN": "tok"}) @patch("devx.ci.notify_failure.TeaCLI") - def test_tea_list_labels_error_continues_without_labels(self, mock_tea_cls: MagicMock) -> None: + def test_tea_list_labels_error_continues_without_labels( + self, mock_tea_cls: MagicMock, mock_login: MagicMock + ) -> None: """If listing labels fails via tea, issue is still created without labels.""" mock_tea = MagicMock() mock_tea.list_labels.side_effect = TeaCLIError("network error") @@ -87,9 +93,10 @@ class TestNotifyFailure: assert result.exit_code == 0 assert "issue #50" in result.output + @patch("devx.gitea_cli.configure_tea_login") @patch.dict("os.environ", {"CI_GITEA_TOKEN": "tok"}) @patch("devx.ci.notify_failure.TeaCLI") - def test_tea_add_label_error_is_ignored(self, mock_tea_cls: MagicMock) -> None: + def test_tea_add_label_error_is_ignored(self, mock_tea_cls: MagicMock, mock_login: MagicMock) -> None: """If adding label fails via tea, issue is still reported as created.""" mock_tea = MagicMock() mock_tea.list_labels.return_value = [{"id": 5, "name": "bug"}] @@ -105,8 +112,9 @@ class TestNotifyFailure: assert result.exit_code == 0 assert "issue #51" in result.output + @patch("devx.gitea_cli.configure_tea_login") @patch.dict("os.environ", {"CI_GITEA_TOKEN": ""}, clear=True) - def test_missing_token_exits(self) -> None: + def test_missing_token_exits(self, mock_login: MagicMock) -> None: runner = CliRunner() result = runner.invoke( main, @@ -115,10 +123,13 @@ class TestNotifyFailure: assert result.exit_code != 0 assert "CI_GITEA_TOKEN" in result.output + @patch("devx.gitea_cli.configure_tea_login") @patch.dict("os.environ", {"CI_GITEA_TOKEN": "tok"}) @patch("devx.gitea_cli.shutil.which", return_value=None) @patch("devx.ci.notify_failure.TeaCLI") - def test_auto_login_no_tea_skips(self, mock_tea_cls: MagicMock, mock_which: MagicMock) -> None: + def test_auto_login_no_tea_skips( + self, mock_tea_cls: MagicMock, mock_which: MagicMock, mock_login: MagicMock + ) -> None: """--auto-login with tea not installed skips login and still creates issue.""" mock_tea = MagicMock() mock_tea.list_labels.return_value = [] @@ -133,10 +144,13 @@ class TestNotifyFailure: assert result.exit_code == 0 assert "issue #60" in result.output + @patch("devx.gitea_cli.configure_tea_login") @patch.dict("os.environ", {"CI_GITEA_TOKEN": ""}, clear=True) @patch("devx.gitea_cli.shutil.which", return_value="/usr/bin/tea") @patch("devx.ci.notify_failure.TeaCLI") - def test_auto_login_no_token_skips_login(self, mock_tea_cls: MagicMock, mock_which: MagicMock) -> None: + def test_auto_login_no_token_skips_login( + self, mock_tea_cls: MagicMock, mock_which: MagicMock, mock_login: MagicMock + ) -> None: """--auto-login with no CI_GITEA_TOKEN skips login but raises before creating issue.""" mock_tea = MagicMock() mock_tea_cls.return_value = mock_tea diff --git a/tests/unit/test_post_merge.py b/tests/unit/test_post_merge.py index 260b230..36d648a 100644 --- a/tests/unit/test_post_merge.py +++ b/tests/unit/test_post_merge.py @@ -91,9 +91,14 @@ class TestResolveTaskId: class TestMain: + @patch("devx.ci.post_merge.subprocess.run") + @patch("devx.ci.post_merge._get_git_commit_message", return_value="msg") + @patch("devx.ci.post_merge._get_git_commit_sha", return_value="sha") @patch.dict("os.environ", {"VIKUNJA_TOKEN": "tok"}) @patch("devx.ci.post_merge.VikunjaClient") - def test_full_flow(self, mock_client_cls: MagicMock) -> None: + def test_full_flow( + self, mock_client_cls: MagicMock, mock_msg: MagicMock, mock_sha: MagicMock, mock_subproc: MagicMock + ) -> None: mock_client = MagicMock() mock_client.list_project_tasks.return_value = [ {"id": 267, "identifier": "DEVX-20"}, @@ -109,9 +114,14 @@ class TestMain: mock_client.post_comment.assert_called_once() mock_client.update_task.assert_called_once_with(267, done=True) + @patch("devx.ci.post_merge.subprocess.run") + @patch("devx.ci.post_merge._get_git_commit_message", return_value="msg") + @patch("devx.ci.post_merge._get_git_commit_sha", return_value="sha") @patch.dict("os.environ", {"VIKUNJA_TOKEN": "tok"}) @patch("devx.ci.post_merge.VikunjaClient") - def test_no_commit_sha(self, mock_client_cls: MagicMock) -> None: + def test_no_commit_sha( + self, mock_client_cls: MagicMock, mock_msg: MagicMock, mock_sha: MagicMock, mock_subproc: MagicMock + ) -> None: mock_client = MagicMock() mock_client.list_project_tasks.return_value = [ {"id": 267, "identifier": "DEVX-20"}, @@ -124,31 +134,47 @@ class TestMain: args, _ = mock_client.post_comment.call_args assert "unknown" in args[1] + @patch("devx.ci.post_merge.subprocess.run") + @patch("devx.ci.post_merge._get_git_commit_message", return_value="msg") + @patch("devx.ci.post_merge._get_git_commit_sha", return_value="sha") @patch.dict("os.environ", {"VIKUNJA_TOKEN": ""}, clear=True) - def test_missing_token_exits(self) -> None: + def test_missing_token_exits(self, mock_msg: MagicMock, mock_sha: MagicMock, mock_subproc: MagicMock) -> None: runner = CliRunner() result = runner.invoke(main, ["DEVX-20: fix: bug"]) assert result.exit_code == 1 assert "VIKUNJA_TOKEN" in result.output + @patch("devx.ci.post_merge.subprocess.run") + @patch("devx.ci.post_merge._get_git_commit_message", return_value="msg") + @patch("devx.ci.post_merge._get_git_commit_sha", return_value="sha") @patch.dict("os.environ", {"VIKUNJA_TOKEN": "tok"}) - def test_no_task_id_non_release_fails(self) -> None: + def test_no_task_id_non_release_fails( + self, mock_msg: MagicMock, mock_sha: MagicMock, mock_subproc: MagicMock + ) -> None: """Non-release commits without DEVX-N prefix should fail.""" runner = CliRunner() result = runner.invoke(main, ["fix: resolve bug"]) assert result.exit_code != 0 assert "No task ID" in result.output + @patch("devx.ci.post_merge.subprocess.run") + @patch("devx.ci.post_merge._get_git_commit_message", return_value="msg") + @patch("devx.ci.post_merge._get_git_commit_sha", return_value="sha") @patch.dict("os.environ", {"VIKUNJA_TOKEN": "tok"}) - def test_release_commit_without_task_id_skips(self) -> None: + def test_release_commit_without_task_id_skips( + self, mock_msg: MagicMock, mock_sha: MagicMock, mock_subproc: MagicMock + ) -> None: """Release commits without DEVX-N prefix should skip gracefully.""" runner = CliRunner() result = runner.invoke(main, ["release: v0.3.2"]) assert result.exit_code == 0 assert "skipping" in result.output + @patch("devx.ci.post_merge.subprocess.run") + @patch("devx.ci.post_merge._get_git_commit_message", return_value="msg") + @patch("devx.ci.post_merge._get_git_commit_sha", return_value="sha") @patch.dict("os.environ", {"VIKUNJA_TOKEN": "tok"}) - def test_revert_commit_skips(self) -> None: + def test_revert_commit_skips(self, mock_msg: MagicMock, mock_sha: MagicMock, mock_subproc: MagicMock) -> None: """Revert commits without DEVX-N prefix should skip gracefully.""" runner = CliRunner() result = runner.invoke(main, ["revert: remove v0.6.0 release"]) @@ -156,17 +182,25 @@ class TestMain: assert "Infrastructure commit" in result.output assert "skipping" in result.output + @patch("devx.ci.post_merge.subprocess.run") + @patch("devx.ci.post_merge._get_git_commit_message", return_value="msg") + @patch("devx.ci.post_merge._get_git_commit_sha", return_value="sha") @patch.dict("os.environ", {"VIKUNJA_TOKEN": "tok"}) - def test_merge_commit_skips(self) -> None: + def test_merge_commit_skips(self, mock_msg: MagicMock, mock_sha: MagicMock, mock_subproc: MagicMock) -> None: """Merge commits without DEVX-N prefix should skip gracefully.""" runner = CliRunner() result = runner.invoke(main, ["Merge pull request #42"]) assert result.exit_code == 0 assert "Infrastructure commit" in result.output + @patch("devx.ci.post_merge.subprocess.run") + @patch("devx.ci.post_merge._get_git_commit_message", return_value="msg") + @patch("devx.ci.post_merge._get_git_commit_sha", return_value="sha") @patch.dict("os.environ", {"VIKUNJA_TOKEN": "tok"}) @patch("devx.ci.post_merge.VikunjaClient") - def test_resolve_failure_fails(self, mock_client_cls: MagicMock) -> None: + def test_resolve_failure_fails( + self, mock_client_cls: MagicMock, mock_msg: MagicMock, mock_sha: MagicMock, mock_subproc: MagicMock + ) -> None: """Missing Vikunja task is a fatal error — every PR must have a task.""" mock_client = MagicMock() mock_client.list_project_tasks.return_value = [] @@ -176,9 +210,14 @@ class TestMain: assert result.exit_code != 0 assert "Could not find" in result.output + @patch("devx.ci.post_merge.subprocess.run") + @patch("devx.ci.post_merge._get_git_commit_message", return_value="msg") + @patch("devx.ci.post_merge._get_git_commit_sha", return_value="sha") @patch.dict("os.environ", {"VIKUNJA_TOKEN": "tok"}) @patch("devx.ci.post_merge.VikunjaClient") - def test_post_comment_failure_fails(self, mock_client_cls: MagicMock) -> None: + def test_post_comment_failure_fails( + self, mock_client_cls: MagicMock, mock_msg: MagicMock, mock_sha: MagicMock, mock_subproc: MagicMock + ) -> None: """Vikunja API errors should fail — the task was not updated.""" mock_client = MagicMock() mock_client.list_project_tasks.return_value = [ @@ -191,9 +230,14 @@ class TestMain: assert result.exit_code != 0 assert "Vikunja API error" in result.output + @patch("devx.ci.post_merge.subprocess.run") + @patch("devx.ci.post_merge._get_git_commit_message", return_value="msg") + @patch("devx.ci.post_merge._get_git_commit_sha", return_value="sha") @patch.dict("os.environ", {"VIKUNJA_TOKEN": "tok"}) @patch("devx.ci.post_merge.VikunjaClient") - def test_mark_done_failure_fails(self, mock_client_cls: MagicMock) -> None: + def test_mark_done_failure_fails( + self, mock_client_cls: MagicMock, mock_msg: MagicMock, mock_sha: MagicMock, mock_subproc: MagicMock + ) -> None: """Vikunja API errors should fail — the task was not updated.""" mock_client = MagicMock() mock_client.list_project_tasks.return_value = [ @@ -235,11 +279,14 @@ class TestGetGitCommitSha: class TestFromGit: + @patch("devx.ci.post_merge.subprocess.run") @patch.dict("os.environ", {"VIKUNJA_TOKEN": "tok"}) @patch("devx.ci.post_merge.VikunjaClient") @patch("devx.ci.post_merge._get_git_commit_sha", return_value="abc123") @patch("devx.ci.post_merge._get_git_commit_message", return_value="DEVX-20: fix: bug") - def test_from_git(self, mock_msg: MagicMock, mock_sha: MagicMock, mock_client_cls: MagicMock) -> None: + def test_from_git( + self, mock_msg: MagicMock, mock_sha: MagicMock, mock_client_cls: MagicMock, mock_subproc: MagicMock + ) -> None: mock_client = MagicMock() mock_client.list_project_tasks.return_value = [ {"id": 267, "identifier": "DEVX-20"}, @@ -250,12 +297,13 @@ class TestFromGit: assert result.exit_code == 0 assert "updated and marked done" in result.output + @patch("devx.ci.post_merge.subprocess.run") @patch.dict("os.environ", {"VIKUNJA_TOKEN": "tok"}) @patch("devx.ci.post_merge.VikunjaClient") @patch("devx.ci.post_merge._get_git_commit_sha", return_value="abc123") @patch("devx.ci.post_merge._get_git_commit_message", return_value="DEVX-20: fix: bug") def test_from_git_with_explicit_sha( - self, mock_msg: MagicMock, mock_sha: MagicMock, mock_client_cls: MagicMock + self, mock_msg: MagicMock, mock_sha: MagicMock, mock_client_cls: MagicMock, mock_subproc: MagicMock ) -> None: mock_client = MagicMock() mock_client.list_project_tasks.return_value = [ @@ -266,8 +314,11 @@ class TestFromGit: result = runner.invoke(main, ["--from-git", "--commit-sha", "explicit_sha"]) assert result.exit_code == 0 + @patch("devx.ci.post_merge.subprocess.run") + @patch("devx.ci.post_merge._get_git_commit_message", return_value="msg") + @patch("devx.ci.post_merge._get_git_commit_sha", return_value="sha") @patch.dict("os.environ", {"VIKUNJA_TOKEN": "tok"}) - def test_no_msg_and_no_from_git(self) -> None: + def test_no_msg_and_no_from_git(self, mock_msg: MagicMock, mock_sha: MagicMock, mock_subproc: MagicMock) -> None: runner = CliRunner() result = runner.invoke(main, []) assert result.exit_code != 0 diff --git a/tests/unit/test_pr_label.py b/tests/unit/test_pr_label.py index 17ab212..9f891a6 100644 --- a/tests/unit/test_pr_label.py +++ b/tests/unit/test_pr_label.py @@ -11,7 +11,8 @@ from devx.tools.pr_label import cli class TestCli: - def test_no_token_raises(self, monkeypatch: pytest.MonkeyPatch) -> None: + @patch("devx.tools.pr_status.subprocess.run") + def test_no_token_raises(self, mock_subproc: MagicMock, monkeypatch: pytest.MonkeyPatch) -> None: for name in ("DEVELOPER_GITEA_API_TOKEN", "CI_GITEA_API_TOKEN", "CI_GITEA_TOKEN"): monkeypatch.delenv(name, raising=False) runner = CliRunner() @@ -23,16 +24,20 @@ class TestCli: assert result.exit_code != 0 assert "CI_GITEA_TOKEN" in result.output + @patch("devx.tools.pr_status.subprocess.run") @patch("devx.tools.pr_label.REPO_OWNER", "") - def test_no_owner_raises(self, monkeypatch: pytest.MonkeyPatch) -> None: + def test_no_owner_raises(self, mock_subproc: MagicMock, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("CI_GITEA_TOKEN", "tok") runner = CliRunner() result = runner.invoke(cli, ["--pr", "42", "--label", "ready-to-merge"]) assert result.exit_code != 0 assert "owner" in result.output.lower() + @patch("devx.tools.pr_status.subprocess.run") @patch("devx.tools.pr_label.GiteaClient") - def test_adds_new_label(self, mock_client_cls: MagicMock, monkeypatch: pytest.MonkeyPatch) -> None: + def test_adds_new_label( + self, mock_client_cls: MagicMock, mock_subproc: MagicMock, monkeypatch: pytest.MonkeyPatch + ) -> None: monkeypatch.setenv("CI_GITEA_TOKEN", "tok") monkeypatch.setenv("DEVX_REPO_OWNER", "owner") monkeypatch.setenv("DEVX_REPO_NAME", "repo") @@ -44,8 +49,11 @@ class TestCli: client.add_pr_label.assert_called_once_with(42, ["ready-to-merge"]) assert "Added label" in result.output + @patch("devx.tools.pr_status.subprocess.run") @patch("devx.tools.pr_label.GiteaClient") - def test_skips_existing_label(self, mock_client_cls: MagicMock, monkeypatch: pytest.MonkeyPatch) -> None: + def test_skips_existing_label( + self, mock_client_cls: MagicMock, mock_subproc: MagicMock, monkeypatch: pytest.MonkeyPatch + ) -> None: monkeypatch.setenv("CI_GITEA_TOKEN", "tok") monkeypatch.setenv("DEVX_REPO_OWNER", "owner") monkeypatch.setenv("DEVX_REPO_NAME", "repo") @@ -57,8 +65,11 @@ class TestCli: client.add_pr_label.assert_not_called() assert "already" in result.output + @patch("devx.tools.pr_status.subprocess.run") @patch("devx.tools.pr_label.GiteaClient") - def test_mixed_new_and_existing(self, mock_client_cls: MagicMock, monkeypatch: pytest.MonkeyPatch) -> None: + def test_mixed_new_and_existing( + self, mock_client_cls: MagicMock, mock_subproc: MagicMock, monkeypatch: pytest.MonkeyPatch + ) -> None: monkeypatch.setenv("CI_GITEA_TOKEN", "tok") monkeypatch.setenv("DEVX_REPO_OWNER", "owner") monkeypatch.setenv("DEVX_REPO_NAME", "repo") diff --git a/tests/unit/test_pr_logs.py b/tests/unit/test_pr_logs.py index 12f1a77..c49540d 100644 --- a/tests/unit/test_pr_logs.py +++ b/tests/unit/test_pr_logs.py @@ -152,7 +152,8 @@ class TestPrintLogs: class TestCli: - def test_no_token_raises(self, monkeypatch: pytest.MonkeyPatch) -> None: + @patch("devx.tools.pr_status.subprocess.run") + def test_no_token_raises(self, mock_subproc: MagicMock, monkeypatch: pytest.MonkeyPatch) -> None: for name in ("DEVELOPER_GITEA_API_TOKEN", "CI_GITEA_API_TOKEN", "CI_GITEA_TOKEN"): monkeypatch.delenv(name, raising=False) runner = CliRunner() @@ -164,8 +165,9 @@ class TestCli: assert result.exit_code != 0 assert "CI_GITEA_TOKEN" in result.output + @patch("devx.tools.pr_status.subprocess.run") @patch("devx.tools.pr_logs.REPO_OWNER", "") - def test_no_owner_raises(self, monkeypatch: pytest.MonkeyPatch) -> None: + def test_no_owner_raises(self, mock_subproc: MagicMock, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("CI_GITEA_TOKEN", "tok") runner = CliRunner() result = runner.invoke(cli, ["--pr", "42"]) @@ -190,8 +192,11 @@ class TestCli: assert result.exit_code != 0 assert "Fetching logs for PR #42" in result.output + @patch("devx.tools.pr_status.subprocess.run") @patch("devx.tools.pr_logs.GiteaClient") - def test_no_runs_found(self, mock_client_cls: MagicMock, monkeypatch: pytest.MonkeyPatch) -> None: + def test_no_runs_found( + self, mock_client_cls: MagicMock, mock_subproc: MagicMock, monkeypatch: pytest.MonkeyPatch + ) -> None: monkeypatch.setenv("CI_GITEA_TOKEN", "tok") monkeypatch.setenv("DEVX_REPO_OWNER", "owner") monkeypatch.setenv("DEVX_REPO_NAME", "repo") @@ -203,8 +208,11 @@ class TestCli: assert result.exit_code != 0 assert "No workflow runs" in result.output + @patch("devx.tools.pr_status.subprocess.run") @patch("devx.tools.pr_logs.GiteaClient") - def test_no_jobs(self, mock_client_cls: MagicMock, monkeypatch: pytest.MonkeyPatch) -> None: + def test_no_jobs( + self, mock_client_cls: MagicMock, mock_subproc: MagicMock, monkeypatch: pytest.MonkeyPatch + ) -> None: monkeypatch.setenv("CI_GITEA_TOKEN", "tok") monkeypatch.setenv("DEVX_REPO_OWNER", "owner") monkeypatch.setenv("DEVX_REPO_NAME", "repo") @@ -219,8 +227,11 @@ class TestCli: assert result.exit_code == 0 assert "No jobs" in result.output + @patch("devx.tools.pr_status.subprocess.run") @patch("devx.tools.pr_logs.GiteaClient") - def test_no_failed_jobs(self, mock_client_cls: MagicMock, monkeypatch: pytest.MonkeyPatch) -> None: + def test_no_failed_jobs( + self, mock_client_cls: MagicMock, mock_subproc: MagicMock, monkeypatch: pytest.MonkeyPatch + ) -> None: monkeypatch.setenv("CI_GITEA_TOKEN", "tok") monkeypatch.setenv("DEVX_REPO_OWNER", "owner") monkeypatch.setenv("DEVX_REPO_NAME", "repo") @@ -237,8 +248,11 @@ class TestCli: assert result.exit_code == 0 assert "No failed jobs" in result.output + @patch("devx.tools.pr_status.subprocess.run") @patch("devx.tools.pr_logs.GiteaClient") - def test_failed_job_logs(self, mock_client_cls: MagicMock, monkeypatch: pytest.MonkeyPatch) -> None: + def test_failed_job_logs( + self, mock_client_cls: MagicMock, mock_subproc: MagicMock, monkeypatch: pytest.MonkeyPatch + ) -> None: monkeypatch.setenv("CI_GITEA_TOKEN", "tok") monkeypatch.setenv("DEVX_REPO_OWNER", "owner") monkeypatch.setenv("DEVX_REPO_NAME", "repo") @@ -266,8 +280,11 @@ class TestCli: assert "FAILED step #3" in result.output assert "error: test failed" in result.output + @patch("devx.tools.pr_status.subprocess.run") @patch("devx.tools.pr_logs.GiteaClient") - def test_specific_job(self, mock_client_cls: MagicMock, monkeypatch: pytest.MonkeyPatch) -> None: + def test_specific_job( + self, mock_client_cls: MagicMock, mock_subproc: MagicMock, monkeypatch: pytest.MonkeyPatch + ) -> None: monkeypatch.setenv("CI_GITEA_TOKEN", "tok") monkeypatch.setenv("DEVX_REPO_OWNER", "owner") monkeypatch.setenv("DEVX_REPO_NAME", "repo") @@ -286,8 +303,11 @@ class TestCli: assert result.exit_code == 0 assert "lint output here" in result.output + @patch("devx.tools.pr_status.subprocess.run") @patch("devx.tools.pr_logs.GiteaClient") - def test_job_not_found(self, mock_client_cls: MagicMock, monkeypatch: pytest.MonkeyPatch) -> None: + def test_job_not_found( + self, mock_client_cls: MagicMock, mock_subproc: MagicMock, monkeypatch: pytest.MonkeyPatch + ) -> None: monkeypatch.setenv("CI_GITEA_TOKEN", "tok") monkeypatch.setenv("DEVX_REPO_OWNER", "owner") monkeypatch.setenv("DEVX_REPO_NAME", "repo") @@ -304,8 +324,11 @@ class TestCli: assert result.exit_code != 0 assert "No job matching" in result.output + @patch("devx.tools.pr_status.subprocess.run") @patch("devx.tools.pr_logs.GiteaClient") - def test_no_sha_raises(self, mock_client_cls: MagicMock, monkeypatch: pytest.MonkeyPatch) -> None: + def test_no_sha_raises( + self, mock_client_cls: MagicMock, mock_subproc: MagicMock, monkeypatch: pytest.MonkeyPatch + ) -> None: monkeypatch.setenv("CI_GITEA_TOKEN", "tok") monkeypatch.setenv("DEVX_REPO_OWNER", "owner") monkeypatch.setenv("DEVX_REPO_NAME", "repo") diff --git a/tests/unit/test_pr_status.py b/tests/unit/test_pr_status.py index 1e1d2d9..b26e547 100644 --- a/tests/unit/test_pr_status.py +++ b/tests/unit/test_pr_status.py @@ -121,7 +121,8 @@ class TestWaitForCompletion: class TestCli: - def test_no_token_raises(self, monkeypatch: pytest.MonkeyPatch) -> None: + @patch("devx.tools.pr_status.subprocess.run") + def test_no_token_raises(self, mock_subproc: MagicMock, monkeypatch: pytest.MonkeyPatch) -> None: for name in ("DEVELOPER_GITEA_API_TOKEN", "CI_GITEA_API_TOKEN", "CI_GITEA_TOKEN"): monkeypatch.delenv(name, raising=False) runner = CliRunner() @@ -133,17 +134,23 @@ class TestCli: assert result.exit_code != 0 assert "CI_GITEA_TOKEN" in result.output + @patch("devx.tools.pr_status.subprocess.run") @patch("devx.tools.pr_status.REPO_OWNER", "") @patch("devx.tools.pr_status.get_repo_name", side_effect=Exception("should not reach")) - def test_no_owner_raises(self, mock_repo_name: MagicMock, monkeypatch: pytest.MonkeyPatch) -> None: + def test_no_owner_raises( + self, mock_repo_name: MagicMock, mock_subproc: MagicMock, monkeypatch: pytest.MonkeyPatch + ) -> None: monkeypatch.setenv("CI_GITEA_TOKEN", "tok") runner = CliRunner() result = runner.invoke(cli, ["--pr", "42"]) assert result.exit_code != 0 assert "owner" in result.output.lower() + @patch("devx.tools.pr_status.subprocess.run") @patch("devx.tools.pr_status.GiteaClient") - def test_check_pr_status(self, mock_client_cls: MagicMock, monkeypatch: pytest.MonkeyPatch) -> None: + def test_check_pr_status( + self, mock_client_cls: MagicMock, mock_subproc: MagicMock, monkeypatch: pytest.MonkeyPatch + ) -> None: monkeypatch.setenv("CI_GITEA_TOKEN", "tok") monkeypatch.setenv("DEVX_REPO_OWNER", "owner") monkeypatch.setenv("DEVX_REPO_NAME", "repo") @@ -157,8 +164,11 @@ class TestCli: assert result.exit_code == 0 assert "[OK]" in result.output + @patch("devx.tools.pr_status.subprocess.run") @patch("devx.tools.pr_status.GiteaClient") - def test_check_sha_directly(self, mock_client_cls: MagicMock, monkeypatch: pytest.MonkeyPatch) -> None: + def test_check_sha_directly( + self, mock_client_cls: MagicMock, mock_subproc: MagicMock, monkeypatch: pytest.MonkeyPatch + ) -> None: monkeypatch.setenv("CI_GITEA_TOKEN", "tok") monkeypatch.setenv("DEVX_REPO_OWNER", "owner") monkeypatch.setenv("DEVX_REPO_NAME", "repo") @@ -171,8 +181,11 @@ class TestCli: assert result.exit_code == 0 assert "[OK]" in result.output + @patch("devx.tools.pr_status.subprocess.run") @patch("devx.tools.pr_status.GiteaClient") - def test_failure_raises_exception(self, mock_client_cls: MagicMock, monkeypatch: pytest.MonkeyPatch) -> None: + def test_failure_raises_exception( + self, mock_client_cls: MagicMock, mock_subproc: MagicMock, monkeypatch: pytest.MonkeyPatch + ) -> None: monkeypatch.setenv("CI_GITEA_TOKEN", "tok") monkeypatch.setenv("DEVX_REPO_OWNER", "owner") monkeypatch.setenv("DEVX_REPO_NAME", "repo") @@ -234,8 +247,11 @@ class TestCli: assert result.exit_code != 0 assert "Could not detect" in result.output + @patch("devx.tools.pr_status.subprocess.run") @patch("devx.tools.pr_status.GiteaClient") - def test_no_sha_raises(self, mock_client_cls: MagicMock, monkeypatch: pytest.MonkeyPatch) -> None: + def test_no_sha_raises( + self, mock_client_cls: MagicMock, mock_subproc: MagicMock, monkeypatch: pytest.MonkeyPatch + ) -> None: monkeypatch.setenv("CI_GITEA_TOKEN", "tok") monkeypatch.setenv("DEVX_REPO_OWNER", "owner") monkeypatch.setenv("DEVX_REPO_NAME", "repo") @@ -246,11 +262,17 @@ class TestCli: assert result.exit_code != 0 assert "SHA" in result.output + @patch("devx.tools.pr_status._get_current_branch_pr") @patch("devx.tools.pr_status.time.sleep") @patch("devx.tools.pr_status.time.time", side_effect=[0, 0, 100, 200]) @patch("devx.tools.pr_status.GiteaClient") def test_wait_success( - self, mock_client_cls: MagicMock, mock_time: MagicMock, mock_sleep: MagicMock, monkeypatch: pytest.MonkeyPatch + self, + mock_client_cls: MagicMock, + mock_time: MagicMock, + mock_sleep: MagicMock, + mock_branch_pr: MagicMock, + monkeypatch: pytest.MonkeyPatch, ) -> None: monkeypatch.setenv("CI_GITEA_TOKEN", "tok") monkeypatch.setenv("DEVX_REPO_OWNER", "owner") @@ -266,11 +288,17 @@ class TestCli: assert result.exit_code == 0 assert "[OK]" in result.output + @patch("devx.tools.pr_status._get_current_branch_pr") @patch("devx.tools.pr_status.time.sleep") @patch("devx.tools.pr_status.time.time", side_effect=[0, 700]) @patch("devx.tools.pr_status.GiteaClient") def test_wait_timeout( - self, mock_client_cls: MagicMock, mock_time: MagicMock, mock_sleep: MagicMock, monkeypatch: pytest.MonkeyPatch + self, + mock_client_cls: MagicMock, + mock_time: MagicMock, + mock_sleep: MagicMock, + mock_branch_pr: MagicMock, + monkeypatch: pytest.MonkeyPatch, ) -> None: monkeypatch.setenv("CI_GITEA_TOKEN", "tok") monkeypatch.setenv("DEVX_REPO_OWNER", "owner") diff --git a/tests/unit/test_pre_push_check.py b/tests/unit/test_pre_push_check.py index 4d48deb..0698a3e 100644 --- a/tests/unit/test_pre_push_check.py +++ b/tests/unit/test_pre_push_check.py @@ -97,9 +97,10 @@ class TestCli: result = runner.invoke(cli, []) assert result.exit_code == 0 + @patch("devx.tools.pre_push_check.get_current_branch") @patch("devx.tools.pre_push_check.task_exists", return_value=True) @patch.dict("os.environ", {"VIKUNJA_TOKEN": "tok"}) - def test_explicit_branch(self, mock_exists: MagicMock) -> None: + def test_explicit_branch(self, mock_exists: MagicMock, mock_branch: MagicMock) -> None: runner = CliRunner() result = runner.invoke(cli, ["--branch", "DEVX-42-fix"]) assert result.exit_code == 0 diff --git a/tests/unit/test_publish.py b/tests/unit/test_publish.py index 028a838..80c1ae8 100644 --- a/tests/unit/test_publish.py +++ b/tests/unit/test_publish.py @@ -165,6 +165,9 @@ class TestDefaultGiteaRegistryUrl: class TestMain: + @patch("devx.ci.publish.subprocess.run") + @patch("devx.ci.publish.get_latest_tag", return_value="v0.1.0") + @patch("devx.gitea_cli.configure_tea_login") @patch.dict("os.environ", {"CI_GITEA_TOKEN": "gitea-tok", "PYPI_TOKEN": "pypi-tok"}) @patch("devx.ci.publish.generate_release_notes", return_value="Release notes") @patch("devx.ci.publish.TeaCLI") @@ -176,6 +179,9 @@ class TestMain: mock_publish: MagicMock, mock_tea_cls: MagicMock, mock_notes: MagicMock, + mock_run: MagicMock, + mock_tag: MagicMock, + mock_login: MagicMock, ) -> None: mock_tea = MagicMock() mock_tea.list_releases.return_value = [] @@ -190,6 +196,9 @@ class TestMain: "owner/repo", tag="v1.0.0", title="v1.0.0", body="Release notes" ) + @patch("devx.ci.publish.subprocess.run") + @patch("devx.ci.publish.get_latest_tag", return_value="v0.1.0") + @patch("devx.gitea_cli.configure_tea_login") @patch.dict("os.environ", {"CI_GITEA_TOKEN": "gitea-tok"}, clear=True) @patch("devx.ci.publish.generate_release_notes", return_value="Release notes") @patch("devx.ci.publish.TeaCLI") @@ -201,6 +210,9 @@ class TestMain: mock_gitea_publish: MagicMock, mock_tea_cls: MagicMock, mock_notes: MagicMock, + mock_run: MagicMock, + mock_tag: MagicMock, + mock_login: MagicMock, ) -> None: """When no PYPI_TOKEN, publishes to Gitea PyPI registry.""" mock_tea = MagicMock() @@ -213,6 +225,9 @@ class TestMain: mock_gitea_publish.assert_called_once() mock_tea.create_release.assert_called_once() + @patch("devx.ci.publish.subprocess.run") + @patch("devx.ci.publish.get_latest_tag", return_value="v0.1.0") + @patch("devx.gitea_cli.configure_tea_login") @patch.dict("os.environ", {"CI_GITEA_TOKEN": "gitea-tok"}, clear=True) @patch("devx.ci.publish.generate_release_notes", return_value="Release notes") @patch("devx.ci.publish.TeaCLI") @@ -224,6 +239,9 @@ class TestMain: mock_gitea_publish: MagicMock, mock_tea_cls: MagicMock, mock_notes: MagicMock, + mock_run: MagicMock, + mock_tag: MagicMock, + mock_login: MagicMock, ) -> None: """--registry-url flag publishes to the specified Gitea registry.""" mock_tea = MagicMock() @@ -237,6 +255,9 @@ class TestMain: assert result.exit_code == 0 mock_gitea_publish.assert_called_once_with("https://custom.registry.com/pypi", "gitea-tok") + @patch("devx.ci.publish.subprocess.run") + @patch("devx.ci.publish.get_latest_tag", return_value="v0.1.0") + @patch("devx.gitea_cli.configure_tea_login") @patch.dict( "os.environ", {"CI_GITEA_TOKEN": "gitea-tok", "DEVX_PYPI_REGISTRY_URL": "https://env.registry.com/pypi"}, @@ -252,6 +273,9 @@ class TestMain: mock_gitea_publish: MagicMock, mock_tea_cls: MagicMock, mock_notes: MagicMock, + mock_run: MagicMock, + mock_tag: MagicMock, + mock_login: MagicMock, ) -> None: """DEVX_PYPI_REGISTRY_URL env var sets the registry URL.""" mock_tea = MagicMock() @@ -262,6 +286,9 @@ class TestMain: assert result.exit_code == 0 mock_gitea_publish.assert_called_once_with("https://env.registry.com/pypi", "gitea-tok") + @patch("devx.ci.publish.subprocess.run") + @patch("devx.ci.publish.get_latest_tag", return_value="v0.1.0") + @patch("devx.gitea_cli.configure_tea_login") @patch.dict("os.environ", {"CI_GITEA_TOKEN": "gitea-tok"}, clear=True) @patch("devx.ci.publish.generate_release_notes", return_value="Release notes") @patch("devx.ci.publish.TeaCLI") @@ -273,6 +300,9 @@ class TestMain: mock_build: MagicMock, mock_tea_cls: MagicMock, mock_notes: MagicMock, + mock_run: MagicMock, + mock_tag: MagicMock, + mock_login: MagicMock, ) -> None: """When no PYPI_TOKEN and no registry URL, skips publish and creates release only.""" mock_tea = MagicMock() @@ -284,20 +314,33 @@ class TestMain: assert "PYPI_TOKEN not set" in result.output mock_tea.create_release.assert_called_once() + @patch("devx.ci.publish.subprocess.run") + @patch("devx.ci.publish.get_latest_tag", return_value="v0.1.0") + @patch("devx.gitea_cli.configure_tea_login") @patch.dict("os.environ", {"CI_GITEA_TOKEN": ""}, clear=True) - def test_missing_repo_token_exits(self) -> None: + def test_missing_repo_token_exits(self, mock_run: MagicMock, mock_tag: MagicMock, mock_login: MagicMock) -> None: runner = CliRunner() result = runner.invoke(main, ["v1.0.0", "owner/repo"]) assert result.exit_code == 1 assert "CI_GITEA_TOKEN" in result.output + @patch("devx.ci.publish.subprocess.run") + @patch("devx.ci.publish.get_latest_tag", return_value="v0.1.0") + @patch("devx.gitea_cli.configure_tea_login") @patch.dict("os.environ", {"CI_GITEA_TOKEN": "gitea-tok", "PYPI_TOKEN": "pypi-tok"}) @patch("devx.ci.publish.generate_release_notes", return_value="Release notes") @patch("devx.ci.publish.TeaCLI") @patch("devx.ci.publish.publish_to_pypi") @patch("devx.ci.publish.build_package") def test_build_failure_raises_click( - self, mock_build: MagicMock, mock_publish: MagicMock, mock_tea_cls: MagicMock, mock_notes: MagicMock + self, + mock_build: MagicMock, + mock_publish: MagicMock, + mock_tea_cls: MagicMock, + mock_notes: MagicMock, + mock_run: MagicMock, + mock_tag: MagicMock, + mock_login: MagicMock, ) -> None: mock_build.side_effect = click.ClickException("build failed") runner = CliRunner() @@ -305,13 +348,23 @@ class TestMain: assert result.exit_code == 1 assert "build" in result.output + @patch("devx.ci.publish.subprocess.run") + @patch("devx.ci.publish.get_latest_tag", return_value="v0.1.0") + @patch("devx.gitea_cli.configure_tea_login") @patch.dict("os.environ", {"CI_GITEA_TOKEN": "gitea-tok", "PYPI_TOKEN": "pypi-tok"}) @patch("devx.ci.publish.generate_release_notes", return_value="Release notes") @patch("devx.ci.publish.TeaCLI") @patch("devx.ci.publish.publish_to_pypi") @patch("devx.ci.publish.build_package") def test_publish_failure_continues_to_gitea_release( - self, mock_build: MagicMock, mock_publish: MagicMock, mock_tea_cls: MagicMock, mock_notes: MagicMock + self, + mock_build: MagicMock, + mock_publish: MagicMock, + mock_tea_cls: MagicMock, + mock_notes: MagicMock, + mock_run: MagicMock, + mock_tag: MagicMock, + mock_login: MagicMock, ) -> None: """PyPI publish failure is non-fatal — Gitea release is still created.""" mock_tea = MagicMock() @@ -326,13 +379,23 @@ class TestMain: "owner/repo", tag="v1.0.0", title="v1.0.0", body="Release notes" ) + @patch("devx.ci.publish.subprocess.run") + @patch("devx.ci.publish.get_latest_tag", return_value="v0.1.0") + @patch("devx.gitea_cli.configure_tea_login") @patch.dict("os.environ", {"CI_GITEA_TOKEN": "gitea-tok", "PYPI_TOKEN": "pypi-tok"}) @patch("devx.ci.publish.generate_release_notes", return_value="Release notes") @patch("devx.ci.publish.TeaCLI") @patch("devx.ci.publish.publish_to_pypi") @patch("devx.ci.publish.build_package") def test_release_failure_raises_click( - self, mock_build: MagicMock, mock_publish: MagicMock, mock_tea_cls: MagicMock, mock_notes: MagicMock + self, + mock_build: MagicMock, + mock_publish: MagicMock, + mock_tea_cls: MagicMock, + mock_notes: MagicMock, + mock_run: MagicMock, + mock_tag: MagicMock, + mock_login: MagicMock, ) -> None: mock_tea = MagicMock() mock_tea.list_releases.return_value = [] @@ -343,12 +406,21 @@ class TestMain: assert result.exit_code == 1 assert "Release creation failed" in result.output + @patch("devx.ci.publish.subprocess.run") + @patch("devx.ci.publish.get_latest_tag", return_value="v0.1.0") + @patch("devx.gitea_cli.configure_tea_login") @patch.dict("os.environ", {"CI_GITEA_TOKEN": "gitea-tok"}) @patch("devx.ci.publish.generate_release_notes", return_value="Release notes") @patch("devx.ci.publish.TeaCLI") @patch("devx.ci.publish.build_package") def test_skip_build_skips_build_and_publish( - self, mock_build: MagicMock, mock_tea_cls: MagicMock, mock_notes: MagicMock + self, + mock_build: MagicMock, + mock_tea_cls: MagicMock, + mock_notes: MagicMock, + mock_run: MagicMock, + mock_tag: MagicMock, + mock_login: MagicMock, ) -> None: """--skip-build skips build_package and PyPI publish, only creates Gitea release.""" mock_tea = MagicMock() @@ -361,13 +433,23 @@ class TestMain: mock_build.assert_not_called() mock_tea.create_release.assert_called_once() + @patch("devx.ci.publish.subprocess.run") + @patch("devx.ci.publish.get_latest_tag", return_value="v0.1.0") + @patch("devx.gitea_cli.configure_tea_login") @patch.dict("os.environ", {"CI_GITEA_TOKEN": "gitea-tok", "PYPI_TOKEN": "pypi-tok"}) @patch("devx.ci.publish.generate_release_notes", return_value="Release notes") @patch("devx.ci.publish.TeaCLI") @patch("devx.ci.publish.publish_to_pypi") @patch("devx.ci.publish.build_package") def test_skips_release_creation_when_already_exists( - self, mock_build: MagicMock, mock_publish: MagicMock, mock_tea_cls: MagicMock, mock_notes: MagicMock + self, + mock_build: MagicMock, + mock_publish: MagicMock, + mock_tea_cls: MagicMock, + mock_notes: MagicMock, + mock_run: MagicMock, + mock_tag: MagicMock, + mock_login: MagicMock, ) -> None: """If the Gitea release already exists, skip creation (idempotent).""" mock_tea = MagicMock() @@ -379,13 +461,23 @@ class TestMain: assert "already exists" in result.output mock_tea.create_release.assert_not_called() + @patch("devx.ci.publish.subprocess.run") + @patch("devx.ci.publish.get_latest_tag", return_value="v0.1.0") + @patch("devx.gitea_cli.configure_tea_login") @patch.dict("os.environ", {"CI_GITEA_TOKEN": "gitea-tok", "PYPI_TOKEN": "pypi-tok"}) @patch("devx.ci.publish.generate_release_notes", return_value="Release notes") @patch("devx.ci.publish.TeaCLI") @patch("devx.ci.publish.publish_to_pypi") @patch("devx.ci.publish.build_package") def test_proceeds_to_create_when_list_releases_fails( - self, mock_build: MagicMock, mock_publish: MagicMock, mock_tea_cls: MagicMock, mock_notes: MagicMock + self, + mock_build: MagicMock, + mock_publish: MagicMock, + mock_tea_cls: MagicMock, + mock_notes: MagicMock, + mock_run: MagicMock, + mock_tag: MagicMock, + mock_login: MagicMock, ) -> None: """If list_releases raises TeaCLIError, proceed to create the release.""" mock_tea = MagicMock() @@ -395,6 +487,9 @@ class TestMain: result = runner.invoke(main, ["v1.0.0", "owner/repo"]) assert result.exit_code == 0 + @patch("devx.ci.publish.subprocess.run") + @patch("devx.ci.publish.get_latest_tag", return_value="v0.1.0") + @patch("devx.gitea_cli.configure_tea_login") @patch.dict("os.environ", {"CI_GITEA_TOKEN": "gitea-tok"}) @patch("devx.ci.publish.generate_release_notes", return_value="Release notes") @patch("devx.ci.publish.TeaCLI") @@ -408,6 +503,9 @@ class TestMain: mock_gitea_pub: MagicMock, mock_tea_cls: MagicMock, mock_notes: MagicMock, + mock_run: MagicMock, + mock_tag: MagicMock, + mock_login: MagicMock, ) -> None: """If create_release fails with 'already exists', treat as success.""" mock_tea = MagicMock() @@ -419,6 +517,9 @@ class TestMain: assert result.exit_code == 0 assert "already exists" in result.output + @patch("devx.ci.publish.subprocess.run") + @patch("devx.ci.publish.get_latest_tag", return_value="v0.1.0") + @patch("devx.gitea_cli.configure_tea_login") @patch.dict("os.environ", {"CI_GITEA_TOKEN": "gitea-tok"}) @patch("devx.ci.publish.generate_release_notes", return_value="Release notes") @patch("devx.ci.publish.TeaCLI") @@ -432,6 +533,9 @@ class TestMain: mock_gitea_pub: MagicMock, mock_tea_cls: MagicMock, mock_notes: MagicMock, + mock_run: MagicMock, + mock_tag: MagicMock, + mock_login: MagicMock, ) -> None: """If create_release fails with a non-'already exists' error, raise.""" mock_tea = MagicMock() @@ -487,40 +591,74 @@ class TestFromTag: result = is_release_commit("v1.2.3") assert result is False + @patch("devx.ci.publish.subprocess.run") + @patch("devx.gitea_cli.configure_tea_login") @patch("devx.ci.publish.get_latest_tag", return_value=None) - def test_from_tag_no_tag_skips(self, _mock: MagicMock) -> None: + def test_from_tag_no_tag_skips(self, _mock: MagicMock, mock_run: MagicMock, mock_login: MagicMock) -> None: runner = CliRunner() result = runner.invoke(main, ["--from-tag", "--skip-build", "", "owner/repo"]) assert result.exit_code == 0 assert "No tag found" in result.output + @patch("devx.ci.publish.subprocess.run") + @patch("devx.gitea_cli.configure_tea_login") @patch("devx.ci.publish.get_latest_tag", return_value=None) - def test_from_tag_no_repo_uses_env(self, _mock: MagicMock) -> None: + def test_from_tag_no_repo_uses_env(self, _mock: MagicMock, mock_run: MagicMock, mock_login: MagicMock) -> None: runner = CliRunner() with patch.dict("os.environ", {"GITHUB_REPOSITORY": "owner/repo"}): result = runner.invoke(main, ["--from-tag", "--skip-build"]) assert result.exit_code == 0 assert "No tag found" in result.output + @patch("devx.ci.publish.subprocess.run") + @patch("devx.gitea_cli.configure_tea_login") @patch("devx.ci.publish.get_latest_tag", return_value=None) - def test_from_tag_no_repo_no_env_raises(self, _mock: MagicMock) -> None: + def test_from_tag_no_repo_no_env_raises(self, _mock: MagicMock, mock_run: MagicMock, mock_login: MagicMock) -> None: runner = CliRunner() with patch.dict("os.environ", {}, clear=True): result = runner.invoke(main, ["--from-tag", "--skip-build"]) assert result.exit_code != 0 assert "REPO argument is required" in result.output + @patch("devx.ci.publish.subprocess.run") + @patch("devx.ci.publish.generate_release_notes", return_value="notes") + @patch("devx.ci.publish.publish_to_pypi") + @patch("devx.ci.publish.publish_to_gitea_registry") + @patch("devx.gitea_cli.configure_tea_login") @patch("devx.ci.publish.is_release_commit", return_value=False) @patch("devx.ci.publish.get_latest_tag", return_value="v1.0.0") - def test_from_tag_not_release_commit_skips(self, _mock_tag: MagicMock, _mock_rel: MagicMock) -> None: + def test_from_tag_not_release_commit_skips( + self, + _mock_tag: MagicMock, + _mock_rel: MagicMock, + mock_run: MagicMock, + mock_notes: MagicMock, + mock_pypi: MagicMock, + mock_gitea_reg: MagicMock, + mock_login: MagicMock, + ) -> None: runner = CliRunner() result = runner.invoke(main, ["--from-tag", "--skip-build", "", "owner/repo"]) assert result.exit_code == 0 assert "not a release commit" in result.output + @patch("devx.ci.publish.subprocess.run") + @patch("devx.ci.publish.generate_release_notes", return_value="notes") + @patch("devx.ci.publish.publish_to_pypi") + @patch("devx.ci.publish.publish_to_gitea_registry") + @patch("devx.gitea_cli.configure_tea_login") @patch("devx.ci.publish.is_release_commit", return_value=True) @patch("devx.ci.publish.get_latest_tag", return_value="v1.0.0") - def test_from_tag_publishes(self, _mock_tag: MagicMock, _mock_rel: MagicMock) -> None: + def test_from_tag_publishes( + self, + _mock_tag: MagicMock, + _mock_rel: MagicMock, + mock_run: MagicMock, + mock_notes: MagicMock, + mock_pypi: MagicMock, + mock_gitea_reg: MagicMock, + mock_login: MagicMock, + ) -> None: with patch.dict("os.environ", {"CI_GITEA_TOKEN": "fake"}): with patch("devx.ci.publish.TeaCLI") as mock_tea_cls: mock_tea = MagicMock() @@ -532,9 +670,23 @@ class TestFromTag: assert result.exit_code == 0 assert "Publishing release v1.0.0" in result.output + @patch("devx.ci.publish.subprocess.run") + @patch("devx.ci.publish.generate_release_notes", return_value="notes") + @patch("devx.ci.publish.publish_to_pypi") + @patch("devx.ci.publish.publish_to_gitea_registry") + @patch("devx.gitea_cli.configure_tea_login") @patch("devx.ci.publish.is_release_commit", return_value=True) @patch("devx.ci.publish.get_latest_tag", return_value="v1.0.0") - def test_from_tag_publishes_no_repo_arg(self, _mock_tag: MagicMock, _mock_rel: MagicMock) -> None: + def test_from_tag_publishes_no_repo_arg( + self, + _mock_tag: MagicMock, + _mock_rel: MagicMock, + mock_run: MagicMock, + mock_notes: MagicMock, + mock_pypi: MagicMock, + mock_gitea_reg: MagicMock, + mock_login: MagicMock, + ) -> None: with patch.dict("os.environ", {"CI_GITEA_TOKEN": "fake", "GITHUB_REPOSITORY": "owner/repo"}): with patch("devx.ci.publish.TeaCLI") as mock_tea_cls: mock_tea = MagicMock() @@ -546,7 +698,21 @@ class TestFromTag: assert result.exit_code == 0 assert "Publishing release v1.0.0" in result.output - def test_no_tag_no_from_tag_raises(self) -> None: + @patch("devx.ci.publish.subprocess.run") + @patch("devx.ci.publish.get_latest_tag", return_value="v0.1.0") + @patch("devx.ci.publish.generate_release_notes", return_value="notes") + @patch("devx.ci.publish.publish_to_pypi") + @patch("devx.ci.publish.publish_to_gitea_registry") + @patch("devx.gitea_cli.configure_tea_login") + def test_no_tag_no_from_tag_raises( + self, + mock_run: MagicMock, + mock_tag: MagicMock, + mock_notes: MagicMock, + mock_pypi: MagicMock, + mock_gitea_reg: MagicMock, + mock_login: MagicMock, + ) -> None: runner = CliRunner() result = runner.invoke(main, ["", "owner/repo", "--skip-build"]) assert result.exit_code != 0 @@ -556,10 +722,24 @@ class TestFromTag: class TestPublishAutoLogin: """Tests for --auto-login flag in publish.""" + @patch("devx.ci.publish.subprocess.run") + @patch("devx.ci.publish.get_latest_tag", return_value="v0.1.0") + @patch("devx.ci.publish.generate_release_notes", return_value="notes") + @patch("devx.ci.publish.publish_to_pypi") + @patch("devx.ci.publish.publish_to_gitea_registry") @patch.dict("os.environ", {"CI_GITEA_TOKEN": "tok"}) @patch("devx.ci.publish.configure_tea_login") @patch("devx.ci.publish.TeaCLI") - def test_auto_login_calls_configure(self, mock_tea_cls: MagicMock, mock_login: MagicMock) -> None: + def test_auto_login_calls_configure( + self, + mock_tea_cls: MagicMock, + mock_login: MagicMock, + mock_run: MagicMock, + mock_tag: MagicMock, + mock_notes: MagicMock, + mock_pypi: MagicMock, + mock_gitea_reg: MagicMock, + ) -> None: """--auto-login calls configure_tea_login before creating release.""" mock_tea = MagicMock() mock_tea.list_releases.return_value = [] @@ -571,10 +751,24 @@ class TestPublishAutoLogin: assert result.exit_code == 0 mock_login.assert_called_once() + @patch("devx.ci.publish.subprocess.run") + @patch("devx.ci.publish.get_latest_tag", return_value="v0.1.0") + @patch("devx.ci.publish.generate_release_notes", return_value="notes") + @patch("devx.ci.publish.publish_to_pypi") + @patch("devx.ci.publish.publish_to_gitea_registry") @patch.dict("os.environ", {"CI_GITEA_TOKEN": "tok"}) @patch("devx.ci.publish.configure_tea_login") @patch("devx.ci.publish.TeaCLI") - def test_no_auto_login_skips_configure(self, mock_tea_cls: MagicMock, mock_login: MagicMock) -> None: + def test_no_auto_login_skips_configure( + self, + mock_tea_cls: MagicMock, + mock_login: MagicMock, + mock_run: MagicMock, + mock_tag: MagicMock, + mock_notes: MagicMock, + mock_pypi: MagicMock, + mock_gitea_reg: MagicMock, + ) -> None: """Without --auto-login, configure_tea_login is not called.""" mock_tea = MagicMock() mock_tea.list_releases.return_value = [] diff --git a/tests/unit/test_rebase.py b/tests/unit/test_rebase.py index af004b7..b1f65b4 100644 --- a/tests/unit/test_rebase.py +++ b/tests/unit/test_rebase.py @@ -278,9 +278,10 @@ class TestRebaseTool: class TestPrRebaseTool: """Tests for the server-side PR rebase tool (devx.tools.pr_rebase).""" + @patch("devx.tools._shared.detect_pr_number") @patch.dict("os.environ", _FULL_ENV, clear=True) @patch("devx.tools.pr_rebase.GiteaClient") - def test_pr_rebase_success(self, mock_client_cls: MagicMock) -> None: + def test_pr_rebase_success(self, mock_client_cls: MagicMock, mock_detect: MagicMock) -> None: """Successful API rebase prints confirmation.""" mock_client = MagicMock() mock_client_cls.return_value = mock_client @@ -291,9 +292,10 @@ class TestPrRebaseTool: assert "rebased successfully" in result.output.lower() mock_client.update_pr_branch.assert_called_once_with(42, style="rebase") + @patch("devx.tools._shared.detect_pr_number") @patch.dict("os.environ", _FULL_ENV, clear=True) @patch("devx.tools.pr_rebase.GiteaClient") - def test_pr_rebase_api_error(self, mock_client_cls: MagicMock) -> None: + def test_pr_rebase_api_error(self, mock_client_cls: MagicMock, mock_detect: MagicMock) -> None: """API error during rebase exits with error.""" from devx.api_clients import APIError @@ -306,9 +308,10 @@ class TestPrRebaseTool: assert result.exit_code != 0 assert "rebase failed" in result.output.lower() + @patch("devx.tools._shared.detect_pr_number") @patch("devx.tools.pr_rebase.load_dotenv") @patch.dict("os.environ", {}, clear=True) - def test_pr_rebase_no_token(self, _mock_load: MagicMock) -> None: + def test_pr_rebase_no_token(self, _mock_load: MagicMock, mock_detect: MagicMock) -> None: """Missing CI_GITEA_TOKEN should fail.""" runner = CliRunner() result = runner.invoke(pr_rebase_main, ["--pr", "42"]) @@ -324,20 +327,26 @@ class TestPrRebaseTool: assert result.exit_code != 0 assert "could not detect" in result.output.lower() + @patch("devx.tools._shared.detect_pr_number") @patch("devx.tools.pr_rebase.load_dotenv") @patch.dict("os.environ", {"CI_GITEA_TOKEN": "tok"}, clear=True) @patch("devx.tools.pr_rebase.GiteaClient") - def test_pr_rebase_no_repo_env(self, _mock_client: MagicMock, _mock_load: MagicMock) -> None: + def test_pr_rebase_no_repo_env( + self, _mock_client: MagicMock, _mock_load: MagicMock, mock_detect: MagicMock + ) -> None: """Missing repo env vars should fail.""" runner = CliRunner() result = runner.invoke(pr_rebase_main, ["--pr", "42"]) assert result.exit_code != 0 assert "DEVX_REPO_OWNER" in result.output + @patch("devx.tools._shared.detect_pr_number") @patch("devx.tools.pr_rebase.load_dotenv") @patch.dict("os.environ", {"CI_GITEA_TOKEN": "tok", "GITHUB_REPOSITORY": "owner/repo"}, clear=True) @patch("devx.tools.pr_rebase.GiteaClient") - def test_pr_rebase_github_repo_fallback(self, mock_client_cls: MagicMock, _mock_load: MagicMock) -> None: + def test_pr_rebase_github_repo_fallback( + self, mock_client_cls: MagicMock, _mock_load: MagicMock, mock_detect: MagicMock + ) -> None: """GITHUB_REPOSITORY env var is used as fallback for owner/repo.""" mock_client = MagicMock() mock_client_cls.return_value = mock_client diff --git a/tests/unit/test_release.py b/tests/unit/test_release.py index dfffbd6..4799e98 100644 --- a/tests/unit/test_release.py +++ b/tests/unit/test_release.py @@ -952,22 +952,32 @@ class TestMain: verify specific git call sequences mock run_cmd with side_effect. """ + @patch("devx.ci.release.has_user_facing_changes", return_value=False) + @patch("devx.ci.release.update_doc_versions") @patch.dict("os.environ", {}) @patch("devx.ci.release.run_cmd") - def test_not_on_master_exits(self, mock_run_cmd: MagicMock) -> None: + def test_not_on_master_exits( + self, mock_run_cmd: MagicMock, mock_update_docs: MagicMock, mock_ufc: MagicMock + ) -> None: mock_run_cmd.return_value = MagicMock(returncode=0, stdout="feature-branch\n", stderr="") runner = CliRunner() result = runner.invoke(main, []) assert result.exit_code != 0 assert "master" in result.output + @patch("devx.ci.release.update_doc_versions") @patch.dict("os.environ", {}) @patch("devx.ci.release.get_latest_tag", return_value="v0.5.0") @patch("devx.ci.release.verify_tag_consistency", return_value=[]) @patch("devx.ci.release.has_user_facing_changes", return_value=False) @patch("devx.ci.release.run_cmd") def test_dry_run_on_non_master_warns( - self, mock_run_cmd: MagicMock, mock_uf: MagicMock, mock_vtc: MagicMock, mock_glt: MagicMock + self, + mock_run_cmd: MagicMock, + mock_uf: MagicMock, + mock_vtc: MagicMock, + mock_glt: MagicMock, + mock_update_docs: MagicMock, ) -> None: """Dry-run mode should not fail on non-master branches.""" mock_run_cmd.return_value = MagicMock(returncode=0, stdout="feature-branch\n", stderr="") @@ -976,6 +986,7 @@ class TestMain: assert result.exit_code == 0 assert "Dry-run mode" in result.output + @patch("devx.ci.release.update_doc_versions") @patch.dict("os.environ", {}) @patch("devx.ci.release.get_head_commit", return_value="abc123") @patch("devx.ci.release.get_tag_commit", return_value="abc123") @@ -991,6 +1002,7 @@ class TestMain: mock_vtc: MagicMock, mock_tc: MagicMock, mock_hc: MagicMock, + mock_update_docs: MagicMock, ) -> None: """If HEAD is a release commit and the tag exists, skip.""" mock_run_cmd.side_effect = [ @@ -1004,6 +1016,7 @@ class TestMain: assert "already a release commit" in result.output assert "Skipping" in result.output + @patch("devx.ci.release.update_doc_versions") @patch.dict("os.environ", {}) @patch("devx.ci.release.get_head_commit", return_value="def456") @patch("devx.ci.release.get_tag_commit", return_value="abc123") @@ -1019,6 +1032,7 @@ class TestMain: mock_vtc: MagicMock, mock_tc: MagicMock, mock_hc: MagicMock, + mock_update_docs: MagicMock, ) -> None: """If HEAD is a release commit but tag points elsewhere, error.""" mock_run_cmd.side_effect = [ @@ -1031,6 +1045,8 @@ class TestMain: assert result.exit_code != 0 assert "misalignment" in result.output + @patch("devx.ci.release.has_user_facing_changes", return_value=False) + @patch("devx.ci.release.update_doc_versions") @patch.dict("os.environ", {}) @patch("devx.ci.release.verify_tag_consistency", return_value=[]) @patch("devx.ci.release.fetch_tags") @@ -1044,6 +1060,8 @@ class TestMain: mock_changelog: MagicMock, mock_ft: MagicMock, mock_vtc: MagicMock, + mock_update_docs: MagicMock, + mock_ufc: MagicMock, ) -> None: """If HEAD is a release commit but the tag is missing, create the tag.""" mock_run_cmd.side_effect = [ @@ -1058,21 +1076,25 @@ class TestMain: assert "Recovering" in result.output mock_create_tag.assert_called_once_with("0.5.0", "## changelog", False) + @patch("devx.ci.release.update_doc_versions") @patch.dict("os.environ", {}) @patch("devx.ci.release.verify_tag_consistency", return_value=[]) @patch("devx.ci.release.fetch_tags") @patch("devx.ci.release.has_user_facing_changes", return_value=True) @patch("devx.ci.release.has_unreleased_changes", return_value=False) @patch("devx.ci.release.get_bumped_version", return_value="0.2.0") + @patch("devx.ci.release.get_latest_tag", return_value="v0.1.0") @patch("devx.ci.release.run_cmd") def test_no_unreleased_changes( self, mock_run_cmd: MagicMock, + mock_latest: MagicMock, mock_bumped: MagicMock, mock_has: MagicMock, mock_user: MagicMock, mock_ft: MagicMock, mock_vtc: MagicMock, + mock_update_docs: MagicMock, ) -> None: mock_run_cmd.return_value = MagicMock(returncode=0, stdout="master\n", stderr="") runner = CliRunner() @@ -1080,6 +1102,7 @@ class TestMain: assert result.exit_code == 0 assert "No unreleased changes" in result.output + @patch("devx.ci.release.update_doc_versions") @patch.dict("os.environ", {}) @patch("devx.ci.release.verify_tag_consistency", return_value=[]) @patch("devx.ci.release.has_user_facing_changes", return_value=True) @@ -1105,6 +1128,7 @@ class TestMain: mock_tag: MagicMock, mock_user: MagicMock, mock_vtc: MagicMock, + mock_update_docs: MagicMock, ) -> None: """Empty changelog should fail, not warn.""" mock_run_cmd.return_value = MagicMock(returncode=0, stdout="master\n", stderr="") @@ -1113,6 +1137,7 @@ class TestMain: assert result.exit_code != 0 assert "empty changelog" in result.output.lower() + @patch("devx.ci.release.update_doc_versions") @patch.dict("os.environ", {}) @patch("devx.ci.release.verify_tag_consistency", return_value=[]) @patch("devx.ci.release.has_user_facing_changes", return_value=True) @@ -1138,6 +1163,7 @@ class TestMain: mock_tag: MagicMock, mock_user: MagicMock, mock_vtc: MagicMock, + mock_update_docs: MagicMock, ) -> None: mock_run_cmd.return_value = MagicMock(returncode=0, stdout="master\n", stderr="") runner = CliRunner() @@ -1149,6 +1175,7 @@ class TestMain: mock_commit.assert_not_called() mock_tag.assert_not_called() + @patch("devx.ci.release.update_doc_versions") @patch.dict("os.environ", {}) @patch("devx.ci.release.verify_tag_consistency", return_value=[]) @patch("devx.ci.release.fetch_tags") @@ -1162,6 +1189,7 @@ class TestMain: mock_latest: MagicMock, mock_ft: MagicMock, mock_vtc: MagicMock, + mock_update_docs: MagicMock, ) -> None: """Release is skipped when only workflow/infrastructure files changed.""" mock_run_cmd.return_value = MagicMock(returncode=0, stdout="master\n", stderr="") @@ -1171,11 +1199,11 @@ class TestMain: assert "No user-facing changes" in result.output assert "Skipping release" in result.output + @patch("devx.ci.release.run_tests") @patch.dict("os.environ", {}) @patch("devx.ci.release.verify_tag_consistency", return_value=[]) @patch("devx.ci.release.fetch_tags") @patch("devx.ci.release.has_user_facing_changes", return_value=True) - @patch("devx.ci.release.run_tests") @patch("devx.ci.release.create_and_push_tag", return_value=True) @patch("devx.ci.release.commit_release_changes", return_value=True) @patch("devx.ci.release.update_changelog") @@ -1184,10 +1212,12 @@ class TestMain: @patch("devx.ci.release.get_latest_tag", return_value="v0.1.0") @patch("devx.ci.release.get_bumped_version", return_value="0.2.0") @patch("devx.ci.release.has_unreleased_changes", return_value=True) + @patch("devx.ci.release.update_doc_versions") @patch("devx.ci.release.run_cmd") def test_full_flow( self, mock_run_cmd: MagicMock, + mock_update_docs: MagicMock, mock_has: MagicMock, mock_bumped: MagicMock, mock_latest: MagicMock, @@ -1196,10 +1226,10 @@ class TestMain: mock_update_changelog: MagicMock, mock_commit: MagicMock, mock_tag: MagicMock, - mock_run_tests: MagicMock, mock_user: MagicMock, mock_ft: MagicMock, mock_vtc: MagicMock, + mock_run_tests: MagicMock, ) -> None: mock_run_cmd.return_value = MagicMock(returncode=0, stdout="master\n", stderr="") runner = CliRunner() @@ -1216,7 +1246,6 @@ class TestMain: @patch("devx.ci.release.verify_tag_consistency", return_value=[]) @patch("devx.ci.release.fetch_tags") @patch("devx.ci.release.has_user_facing_changes", return_value=True) - @patch("devx.ci.release.run_tests") @patch("devx.ci.release.create_and_push_tag", return_value=False) @patch("devx.ci.release.commit_release_changes", return_value=False) @patch("devx.ci.release.update_changelog") @@ -1225,10 +1254,12 @@ class TestMain: @patch("devx.ci.release.get_latest_tag", return_value="v0.1.0") @patch("devx.ci.release.get_bumped_version", return_value="0.2.0") @patch("devx.ci.release.has_unreleased_changes", return_value=True) + @patch("devx.ci.release.update_doc_versions") @patch("devx.ci.release.run_cmd") def test_full_flow_tag_exists( self, mock_run_cmd: MagicMock, + mock_update_docs: MagicMock, mock_has: MagicMock, mock_bumped: MagicMock, mock_latest: MagicMock, @@ -1237,7 +1268,6 @@ class TestMain: mock_update_changelog: MagicMock, mock_commit: MagicMock, mock_tag: MagicMock, - mock_run_tests: MagicMock, mock_user: MagicMock, mock_ft: MagicMock, mock_vtc: MagicMock, @@ -1250,11 +1280,11 @@ class TestMain: assert "already existed" in result.output mock_tag.assert_called_once_with("0.2.0", "changelog", False) + @patch("devx.ci.release.run_tests") @patch.dict("os.environ", {}) @patch("devx.ci.release.verify_tag_consistency", return_value=[]) @patch("devx.ci.release.fetch_tags") @patch("devx.ci.release.has_user_facing_changes", return_value=True) - @patch("devx.ci.release.run_tests") @patch("devx.ci.release.create_and_push_tag", return_value=True) @patch("devx.ci.release.commit_release_changes", return_value=True) @patch("devx.ci.release.update_changelog") @@ -1263,12 +1293,14 @@ class TestMain: @patch("devx.ci.release.get_latest_tag", return_value="v0.1.0") @patch("devx.ci.release.get_bumped_version", return_value="0.2.0") @patch("devx.ci.release.has_unreleased_changes", return_value=True) + @patch("devx.ci.release.update_doc_versions") @patch("devx.ci.release.time.sleep") @patch("devx.ci.release.run_cmd") def test_push_retry_succeeds_after_rebase_failure( self, mock_run_cmd: MagicMock, mock_sleep: MagicMock, + mock_update_docs: MagicMock, mock_has: MagicMock, mock_bumped: MagicMock, mock_latest: MagicMock, @@ -1277,10 +1309,10 @@ class TestMain: mock_update_changelog: MagicMock, mock_commit: MagicMock, mock_tag: MagicMock, - mock_run_tests: MagicMock, mock_user: MagicMock, mock_ft: MagicMock, mock_vtc: MagicMock, + mock_run_tests: MagicMock, ) -> None: """Push should retry after rebase failure and succeed on second attempt.""" ok = MagicMock(returncode=0, stdout="master\n", stderr="") @@ -1297,11 +1329,11 @@ class TestMain: assert "Rebase attempt 1/3 failed" in result.output assert "Pushed release commit to master" in result.output + @patch("devx.ci.release.run_tests") @patch.dict("os.environ", {}) @patch("devx.ci.release.verify_tag_consistency", return_value=[]) @patch("devx.ci.release.fetch_tags") @patch("devx.ci.release.has_user_facing_changes", return_value=True) - @patch("devx.ci.release.run_tests") @patch("devx.ci.release.create_and_push_tag", return_value=True) @patch("devx.ci.release.commit_release_changes", return_value=True) @patch("devx.ci.release.update_changelog") @@ -1310,12 +1342,14 @@ class TestMain: @patch("devx.ci.release.get_latest_tag", return_value="v0.1.0") @patch("devx.ci.release.get_bumped_version", return_value="0.2.0") @patch("devx.ci.release.has_unreleased_changes", return_value=True) + @patch("devx.ci.release.update_doc_versions") @patch("devx.ci.release.time.sleep") @patch("devx.ci.release.run_cmd") def test_push_fails_after_all_retries( self, mock_run_cmd: MagicMock, mock_sleep: MagicMock, + mock_update_docs: MagicMock, mock_has: MagicMock, mock_bumped: MagicMock, mock_latest: MagicMock, @@ -1324,10 +1358,10 @@ class TestMain: mock_update_changelog: MagicMock, mock_commit: MagicMock, mock_tag: MagicMock, - mock_run_tests: MagicMock, mock_user: MagicMock, mock_ft: MagicMock, mock_vtc: MagicMock, + mock_run_tests: MagicMock, ) -> None: """Push should fail after 3 unsuccessful rebase attempts.""" ok = MagicMock(returncode=0, stdout="master\n", stderr="") @@ -1350,11 +1384,11 @@ class TestMain: assert result.exit_code != 0 assert "Failed to push release commit after 3 attempts" in result.output + @patch("devx.ci.release.run_tests") @patch.dict("os.environ", {}) @patch("devx.ci.release.verify_tag_consistency", return_value=[]) @patch("devx.ci.release.fetch_tags") @patch("devx.ci.release.has_user_facing_changes", return_value=True) - @patch("devx.ci.release.run_tests") @patch("devx.ci.release.create_and_push_tag", return_value=True) @patch("devx.ci.release.commit_release_changes", return_value=True) @patch("devx.ci.release.update_changelog") @@ -1363,12 +1397,14 @@ class TestMain: @patch("devx.ci.release.get_latest_tag", return_value="v0.1.0") @patch("devx.ci.release.get_bumped_version", return_value="0.2.0") @patch("devx.ci.release.has_unreleased_changes", return_value=True) + @patch("devx.ci.release.update_doc_versions") @patch("devx.ci.release.time.sleep") @patch("devx.ci.release.run_cmd") def test_push_retry_succeeds_after_push_failure( self, mock_run_cmd: MagicMock, mock_sleep: MagicMock, + mock_update_docs: MagicMock, mock_has: MagicMock, mock_bumped: MagicMock, mock_latest: MagicMock, @@ -1377,10 +1413,10 @@ class TestMain: mock_update_changelog: MagicMock, mock_commit: MagicMock, mock_tag: MagicMock, - mock_run_tests: MagicMock, mock_user: MagicMock, mock_ft: MagicMock, mock_vtc: MagicMock, + mock_run_tests: MagicMock, ) -> None: """Push should retry after push rejection and succeed on second attempt.""" ok = MagicMock(returncode=0, stdout="master\n", stderr="") @@ -1397,6 +1433,7 @@ class TestMain: assert "Push attempt 1/3 failed" in result.output assert "Pushed release commit to master" in result.output + @patch("devx.ci.release.update_doc_versions") @patch.dict("os.environ", {}) @patch("devx.ci.release.verify_tag_consistency", return_value=[]) @patch("devx.ci.release.fetch_tags") @@ -1414,6 +1451,7 @@ class TestMain: mock_user: MagicMock, mock_ft: MagicMock, mock_vtc: MagicMock, + mock_update_docs: MagicMock, ) -> None: """Release is skipped when git-cliff doesn't bump the version.""" mock_run_cmd.return_value = MagicMock(returncode=0, stdout="master\n", stderr="") @@ -1435,10 +1473,12 @@ class TestMain: @patch("devx.ci.release.get_latest_tag", return_value="v0.1.0") @patch("devx.ci.release.get_bumped_version", return_value="0.2.0") @patch("devx.ci.release.has_unreleased_changes", return_value=True) + @patch("devx.ci.release.update_doc_versions") @patch("devx.ci.release.run_cmd") def test_full_flow_skip_tests( self, mock_run_cmd: MagicMock, + mock_update_docs: MagicMock, mock_has: MagicMock, mock_bumped: MagicMock, mock_latest: MagicMock, @@ -1473,10 +1513,12 @@ class TestMain: @patch("devx.ci.release.get_latest_tag", return_value="v0.1.0") @patch("devx.ci.release.get_bumped_version", return_value="0.2.0") @patch("devx.ci.release.has_unreleased_changes", return_value=True) + @patch("devx.ci.release.update_doc_versions") @patch("devx.ci.release.run_cmd") def test_tests_fail_aborts_before_tag( self, mock_run_cmd: MagicMock, + mock_update_docs: MagicMock, mock_has: MagicMock, mock_bumped: MagicMock, mock_latest: MagicMock, @@ -1517,10 +1559,12 @@ class TestMain: @patch("devx.ci.release.get_latest_tag", return_value="v0.1.0") @patch("devx.ci.release.get_bumped_version", return_value="0.2.0") @patch("devx.ci.release.has_unreleased_changes", return_value=True) + @patch("devx.ci.release.update_doc_versions") @patch("devx.ci.release.run_cmd") def test_lint_fail_aborts_before_tag( self, mock_run_cmd: MagicMock, + mock_update_docs: MagicMock, mock_has: MagicMock, mock_bumped: MagicMock, mock_latest: MagicMock, @@ -1548,6 +1592,8 @@ class TestMain: mock_commit.assert_not_called() mock_tag.assert_not_called() + @patch("devx.ci.release.has_user_facing_changes", return_value=False) + @patch("devx.ci.release.update_doc_versions") @patch.dict("os.environ", {}) @patch("devx.ci.release.get_changelog_versions", return_value=[]) @patch("devx.ci.release.get_init_version", return_value="0.1.0") @@ -1561,6 +1607,8 @@ class TestMain: mock_tags: MagicMock, mock_iv: MagicMock, mock_cv: MagicMock, + mock_update_docs: MagicMock, + mock_ufc: MagicMock, ) -> None: """--verify checks alignment and exits without releasing.""" mock_run_cmd.return_value = MagicMock(returncode=0, stdout="", stderr="") @@ -1569,12 +1617,19 @@ class TestMain: assert result.exit_code == 0 assert "Release Alignment Verification" in result.output + @patch("devx.ci.release.has_user_facing_changes", return_value=False) + @patch("devx.ci.release.update_doc_versions") @patch.dict("os.environ", {}) @patch("devx.ci.release.verify_tag_consistency", return_value=[" v0.1.0 → bad"]) @patch("devx.ci.release.fetch_tags") @patch("devx.ci.release.run_cmd") def test_preflight_tag_consistency_fails( - self, mock_run_cmd: MagicMock, mock_ft: MagicMock, mock_vtc: MagicMock + self, + mock_run_cmd: MagicMock, + mock_ft: MagicMock, + mock_vtc: MagicMock, + mock_update_docs: MagicMock, + mock_ufc: MagicMock, ) -> None: """Pre-flight tag consistency check aborts if tags are misaligned.""" mock_run_cmd.return_value = MagicMock(returncode=0, stdout="master\n", stderr="") diff --git a/tests/unit/test_setup.py b/tests/unit/test_setup.py index 8816bcd..08383d3 100644 --- a/tests/unit/test_setup.py +++ b/tests/unit/test_setup.py @@ -312,7 +312,13 @@ class TestMain: assert result.exit_code == 0 mock_tea.assert_not_called() - def test_main_missing_bin_dir(self) -> None: + @patch("devx.tools.setup._run") + @patch("devx.tools.setup._configure_tea_login") + @patch("devx.tools.setup._install_python_deps") + @patch("devx.tools.setup._verify") + def test_main_missing_bin_dir( + self, mock_tea: MagicMock, mock_deps: MagicMock, mock_verify: MagicMock, mock_run: MagicMock + ) -> None: runner = CliRunner() result = runner.invoke(main, ["--bin", "/nonexistent/path"]) assert result.exit_code != 0 diff --git a/tests/unit/test_setup_image.py b/tests/unit/test_setup_image.py index 0f1349e..213c692 100644 --- a/tests/unit/test_setup_image.py +++ b/tests/unit/test_setup_image.py @@ -185,12 +185,14 @@ class TestFallbackToSetupCi: class TestCli: + @patch("devx.tools.setup_image._fallback_to_setup_ci") @patch("devx.tools.setup_image._install_in_image") @patch("devx.tools.setup_image.Path") def test_cli_with_opt_venv_present( self, mock_path: MagicMock, mock_install: MagicMock, + mock_fallback: MagicMock, ) -> None: mock_path.return_value.is_dir.return_value = True runner = CliRunner() @@ -198,12 +200,14 @@ class TestCli: assert result.exit_code == 0 mock_install.assert_called_once() + @patch("devx.tools.setup_image._install_in_image") @patch("devx.tools.setup_image._fallback_to_setup_ci") @patch("devx.tools.setup_image.Path") def test_cli_falls_back_when_no_opt_venv( self, mock_path: MagicMock, mock_fallback: MagicMock, + mock_install: MagicMock, ) -> None: mock_path.return_value.is_dir.return_value = False runner = CliRunner() @@ -211,12 +215,14 @@ class TestCli: assert result.exit_code == 0 mock_fallback.assert_called_once() + @patch("devx.tools.setup_image._fallback_to_setup_ci") @patch("devx.tools.setup_image._install_in_image") @patch("devx.tools.setup_image.Path") def test_cli_default_values( self, mock_path: MagicMock, mock_install: MagicMock, + mock_fallback: MagicMock, ) -> None: mock_path.return_value.is_dir.return_value = True runner = CliRunner() @@ -229,12 +235,14 @@ class TestCli: assert call_args[3] == "git.oblachno.oblachno.fyi" assert call_args[4] == "oblachno-oss" + @patch("devx.tools.setup_image._fallback_to_setup_ci") @patch("devx.tools.setup_image._install_in_image") @patch("devx.tools.setup_image.Path") def test_cli_custom_venv_and_gitea( self, mock_path: MagicMock, mock_install: MagicMock, + mock_fallback: MagicMock, ) -> None: mock_path.return_value.is_dir.return_value = True runner = CliRunner() @@ -248,12 +256,14 @@ class TestCli: assert call_args[3] == "gitea.io" assert call_args[4] == "myorg" + @patch("devx.tools.setup_image._fallback_to_setup_ci") @patch("devx.tools.setup_image._install_in_image") @patch("devx.tools.setup_image.Path") def test_cli_with_extras( self, mock_path: MagicMock, mock_install: MagicMock, + mock_fallback: MagicMock, ) -> None: mock_path.return_value.is_dir.return_value = True runner = CliRunner() diff --git a/tests/unit/test_sync_wiki.py b/tests/unit/test_sync_wiki.py index efe6d4c..bc7202c 100644 --- a/tests/unit/test_sync_wiki.py +++ b/tests/unit/test_sync_wiki.py @@ -255,7 +255,17 @@ class TestCommitAndPush: class TestMain: - def test_no_token_raises(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + @patch("devx.ci.sync_wiki.commit_and_push") + @patch("devx.ci.sync_wiki.init_wiki") + @patch("devx.ci.sync_wiki.clone_wiki") + def test_no_token_raises( + self, + mock_clone: MagicMock, + mock_init: MagicMock, + mock_commit: MagicMock, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + ) -> None: for name in ("CI_GITEA_API_TOKEN", "CI_GITEA_TOKEN"): monkeypatch.delenv(name, raising=False) runner = CliRunner() @@ -263,7 +273,17 @@ class TestMain: assert result.exit_code != 0 assert "CI_GITEA_TOKEN" in result.output - def test_no_mapping_raises(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + @patch("devx.ci.sync_wiki.commit_and_push") + @patch("devx.ci.sync_wiki.init_wiki") + @patch("devx.ci.sync_wiki.clone_wiki") + def test_no_mapping_raises( + self, + mock_clone: MagicMock, + mock_init: MagicMock, + mock_commit: MagicMock, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + ) -> None: monkeypatch.setenv("CI_GITEA_TOKEN", "fake") monkeypatch.setattr("devx.ci.sync_wiki.MAPPING_FILE", tmp_path / "nonexistent.json") runner = CliRunner() @@ -271,6 +291,7 @@ class TestMain: assert result.exit_code != 0 assert "mapping.json" in result.output + @patch("devx.ci.sync_wiki.init_wiki") @patch("devx.ci.sync_wiki.clone_wiki", return_value=True) @patch("devx.ci.sync_wiki.commit_and_push", return_value=True) @patch("devx.ci.sync_wiki.sync_files", return_value=(1, 0)) @@ -279,6 +300,7 @@ class TestMain: mock_sync: MagicMock, mock_push: MagicMock, mock_clone: MagicMock, + mock_init: MagicMock, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -296,6 +318,7 @@ class TestMain: assert "dry-run" in result.output mock_push.assert_not_called() + @patch("devx.ci.sync_wiki.init_wiki") @patch("devx.ci.sync_wiki.clone_wiki", return_value=True) @patch("devx.ci.sync_wiki.commit_and_push", return_value=True) @patch("devx.ci.sync_wiki.sync_files", return_value=(2, 0)) @@ -304,6 +327,7 @@ class TestMain: mock_sync: MagicMock, mock_push: MagicMock, mock_clone: MagicMock, + mock_init: MagicMock, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -348,6 +372,7 @@ class TestMain: assert result.exit_code == 0 mock_init.assert_called_once() + @patch("devx.ci.sync_wiki.init_wiki") @patch("devx.ci.sync_wiki.time.sleep") @patch("devx.ci.sync_wiki.clone_wiki") @patch("devx.ci.sync_wiki.commit_and_push", return_value=True) @@ -358,6 +383,7 @@ class TestMain: mock_push: MagicMock, mock_clone: MagicMock, mock_sleep: MagicMock, + mock_init: MagicMock, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -383,6 +409,7 @@ class TestMain: assert result.exit_code == 0 assert "Verification" in result.output + @patch("devx.ci.sync_wiki.init_wiki") @patch("devx.ci.sync_wiki.clone_wiki", return_value=True) @patch("devx.ci.sync_wiki.commit_and_push", return_value=False) @patch("devx.ci.sync_wiki.sync_files", return_value=(1, 0)) @@ -391,6 +418,7 @@ class TestMain: mock_sync: MagicMock, mock_push: MagicMock, mock_clone: MagicMock, + mock_init: MagicMock, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -407,6 +435,7 @@ class TestMain: assert result.exit_code == 0 assert "No push needed" in result.output + @patch("devx.ci.sync_wiki.init_wiki") @patch("devx.ci.sync_wiki.time.sleep") @patch("devx.ci.sync_wiki.clone_wiki", side_effect=[True, False]) @patch("devx.ci.sync_wiki.commit_and_push", return_value=True) @@ -417,6 +446,7 @@ class TestMain: mock_push: MagicMock, mock_clone: MagicMock, mock_sleep: MagicMock, + mock_init: MagicMock, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -433,6 +463,7 @@ class TestMain: assert result.exit_code != 0 assert "could not clone" in result.output + @patch("devx.ci.sync_wiki.init_wiki") @patch("devx.ci.sync_wiki.time.sleep") @patch("devx.ci.sync_wiki.clone_wiki") @patch("devx.ci.sync_wiki.commit_and_push", return_value=True) @@ -443,6 +474,7 @@ class TestMain: mock_push: MagicMock, mock_clone: MagicMock, mock_sleep: MagicMock, + mock_init: MagicMock, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -467,6 +499,7 @@ class TestMain: assert result.exit_code != 0 assert "page(s) missing" in result.output + @patch("devx.ci.sync_wiki.init_wiki") @patch("devx.ci.sync_wiki.clone_wiki", return_value=True) @patch("devx.ci.sync_wiki.commit_and_push", return_value=True) @patch("devx.ci.sync_wiki.sync_files", return_value=(1, 0)) @@ -475,6 +508,7 @@ class TestMain: mock_sync: MagicMock, mock_push: MagicMock, mock_clone: MagicMock, + mock_init: MagicMock, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: diff --git a/tests/unit/test_validate_commit_msg.py b/tests/unit/test_validate_commit_msg.py index 34beaaf..cb251ee 100644 --- a/tests/unit/test_validate_commit_msg.py +++ b/tests/unit/test_validate_commit_msg.py @@ -3,7 +3,7 @@ import os import subprocess import tempfile -from unittest.mock import patch +from unittest.mock import MagicMock, patch from click.testing import CliRunner @@ -70,7 +70,8 @@ class TestMain: f.write(content) return path - def test_rejects_task_id_on_feature_branch(self) -> None: + @patch("devx.ci.validate_commit_msg.get_latest_commit_msg") + def test_rejects_task_id_on_feature_branch(self, mock_commit: MagicMock) -> None: msg_path = self._write_msg("DEVX-19: feat: add feature") with patch("devx.ci.validate_commit_msg.get_branch", return_value="DEVX-19"): runner = CliRunner() @@ -78,21 +79,24 @@ class TestMain: assert result.exit_code == 1 assert "task ID" in result.output - def test_accepts_conventional_on_feature_branch(self) -> None: + @patch("devx.ci.validate_commit_msg.get_latest_commit_msg") + def test_accepts_conventional_on_feature_branch(self, mock_commit: MagicMock) -> None: msg_path = self._write_msg("feat: add feature") with patch("devx.ci.validate_commit_msg.get_branch", return_value="DEVX-19"): runner = CliRunner() result = runner.invoke(main, [msg_path]) assert result.exit_code == 0 - def test_accepts_valid_master_commit(self) -> None: + @patch("devx.ci.validate_commit_msg.get_latest_commit_msg") + def test_accepts_valid_master_commit(self, mock_commit: MagicMock) -> None: msg_path = self._write_msg("DEVX-19: feat: add feature") with patch("devx.ci.validate_commit_msg.get_branch", return_value="master"): runner = CliRunner() result = runner.invoke(main, [msg_path]) assert result.exit_code == 0 - def test_rejects_master_without_task_id(self) -> None: + @patch("devx.ci.validate_commit_msg.get_latest_commit_msg") + def test_rejects_master_without_task_id(self, mock_commit: MagicMock) -> None: msg_path = self._write_msg("feat: add feature") with patch("devx.ci.validate_commit_msg.get_branch", return_value="master"): runner = CliRunner() @@ -100,7 +104,8 @@ class TestMain: assert result.exit_code == 1 assert "task ID" in result.output - def test_rejects_master_with_non_conventional_after_task_id(self) -> None: + @patch("devx.ci.validate_commit_msg.get_latest_commit_msg") + def test_rejects_master_with_non_conventional_after_task_id(self, mock_commit: MagicMock) -> None: msg_path = self._write_msg("DEVX-19: random message") with patch("devx.ci.validate_commit_msg.get_branch", return_value="master"): runner = CliRunner() @@ -108,7 +113,8 @@ class TestMain: assert result.exit_code == 1 assert "conventional" in result.output - def test_rejects_non_conventional_on_feature_branch(self) -> None: + @patch("devx.ci.validate_commit_msg.get_latest_commit_msg") + def test_rejects_non_conventional_on_feature_branch(self, mock_commit: MagicMock) -> None: msg_path = self._write_msg("random message") with patch("devx.ci.validate_commit_msg.get_branch", return_value="feature"): runner = CliRunner() @@ -116,26 +122,33 @@ class TestMain: assert result.exit_code == 1 assert "conventional" in result.output - def test_accepts_multiline_conventional(self) -> None: + @patch("devx.ci.validate_commit_msg.get_latest_commit_msg") + def test_accepts_multiline_conventional(self, mock_commit: MagicMock) -> None: msg_path = self._write_msg("feat: add feature\n\nBody text.\nMore text.") with patch("devx.ci.validate_commit_msg.get_branch", return_value="feature"): runner = CliRunner() result = runner.invoke(main, [msg_path]) assert result.exit_code == 0 - def test_usage_message_without_args(self) -> None: + @patch("devx.ci.validate_commit_msg.get_branch") + @patch("devx.ci.validate_commit_msg.get_latest_commit_msg") + def test_usage_message_without_args(self, mock_commit: MagicMock, mock_branch: MagicMock) -> None: runner = CliRunner() result = runner.invoke(main, []) assert result.exit_code != 0 - def test_branch_override_accepts_master_commit(self) -> None: + @patch("devx.ci.validate_commit_msg.get_branch") + @patch("devx.ci.validate_commit_msg.get_latest_commit_msg") + def test_branch_override_accepts_master_commit(self, mock_commit: MagicMock, mock_branch: MagicMock) -> None: """--branch master overrides branch detection (for CI use).""" msg_path = self._write_msg("DEVX-19: feat: add feature") runner = CliRunner() result = runner.invoke(main, [msg_path, "--branch", "master"]) assert result.exit_code == 0 - def test_branch_override_rejects_missing_task_id(self) -> None: + @patch("devx.ci.validate_commit_msg.get_branch") + @patch("devx.ci.validate_commit_msg.get_latest_commit_msg") + def test_branch_override_rejects_missing_task_id(self, mock_commit: MagicMock, mock_branch: MagicMock) -> None: """--branch master still enforces DEVX-N: prefix.""" msg_path = self._write_msg("feat: add feature") runner = CliRunner() @@ -143,7 +156,9 @@ class TestMain: assert result.exit_code == 1 assert "task ID" in result.output - def test_branch_override_feature_accepts_conventional(self) -> None: + @patch("devx.ci.validate_commit_msg.get_branch") + @patch("devx.ci.validate_commit_msg.get_latest_commit_msg") + def test_branch_override_feature_accepts_conventional(self, mock_commit: MagicMock, mock_branch: MagicMock) -> None: """--branch feature still rejects DEVX-N prefix.""" msg_path = self._write_msg("DEVX-19: feat: add feature") runner = CliRunner() @@ -166,8 +181,9 @@ class TestCustomPrefix: f.write(content) return path + @patch("devx.ci.validate_commit_msg.get_latest_commit_msg") @patch.dict("os.environ", {"DEVX_TASK_PREFIX": "PROJ"}) - def test_master_accepts_proj_prefix(self) -> None: + def test_master_accepts_proj_prefix(self, mock_commit: MagicMock) -> None: """Master branch accepts PROJ-N: prefix when DEVX_TASK_PREFIX=GRM.""" import importlib @@ -188,8 +204,9 @@ class TestCustomPrefix: importlib.reload(devx.config) importlib.reload(vcm) + @patch("devx.ci.validate_commit_msg.get_latest_commit_msg") @patch.dict("os.environ", {"DEVX_TASK_PREFIX": "PROJ"}) - def test_master_rejects_devx_prefix_when_proj_configured(self) -> None: + def test_master_rejects_devx_prefix_when_proj_configured(self, mock_commit: MagicMock) -> None: """Master branch rejects DEVX-N: prefix when DEVX_TASK_PREFIX=GRM.""" import importlib @@ -211,8 +228,9 @@ class TestCustomPrefix: importlib.reload(devx.config) importlib.reload(vcm) + @patch("devx.ci.validate_commit_msg.get_latest_commit_msg") @patch.dict("os.environ", {"DEVX_TASK_PREFIX": "PROJ"}) - def test_feature_branch_rejects_proj_prefix(self) -> None: + def test_feature_branch_rejects_proj_prefix(self, mock_commit: MagicMock) -> None: """Feature branch rejects PROJ-N: prefix when DEVX_TASK_PREFIX=GRM.""" import importlib @@ -283,7 +301,9 @@ class TestGitMode: result = runner.invoke(main, ["--git", "--branch", "master"]) assert result.exit_code != 0 - def test_no_file_no_git_raises(self) -> None: + @patch("devx.ci.validate_commit_msg.get_branch") + @patch("devx.ci.validate_commit_msg.get_latest_commit_msg") + def test_no_file_no_git_raises(self, mock_commit: MagicMock, mock_branch: MagicMock) -> None: runner = CliRunner() result = runner.invoke(main, ["--branch", "master"]) assert result.exit_code != 0 @@ -294,7 +314,8 @@ class TestGitMode: result = get_latest_commit_msg() assert result == "feat: test\n\nBody" - def test_stdin_input(self) -> None: + @patch("devx.ci.validate_commit_msg.get_latest_commit_msg") + def test_stdin_input(self, mock_commit: MagicMock) -> None: with patch("devx.ci.validate_commit_msg.get_branch", return_value="feature-branch"): runner = CliRunner() result = runner.invoke(main, input="feat: add feature\n", args=["-", "--branch", "feature-branch"]) diff --git a/tests/unit/test_validate_deploy_ref.py b/tests/unit/test_validate_deploy_ref.py index 1eb861c..01809cd 100644 --- a/tests/unit/test_validate_deploy_ref.py +++ b/tests/unit/test_validate_deploy_ref.py @@ -27,13 +27,15 @@ class TestValidateDeployRef: assert result.exit_code == 1 assert "does not exist" in result.output - def test_no_tag_without_allow_empty_exits_nonzero(self) -> None: + @patch("devx.ci.validate_deploy_ref.subprocess.run") + def test_no_tag_without_allow_empty_exits_nonzero(self, mock_subproc: MagicMock) -> None: runner = CliRunner() result = runner.invoke(main, []) assert result.exit_code == 1 assert "No tag specified" in result.output - def test_allow_empty_prints_pr_mode(self) -> None: + @patch("devx.ci.validate_deploy_ref.subprocess.run") + def test_allow_empty_prints_pr_mode(self, mock_subproc: MagicMock) -> None: runner = CliRunner() result = runner.invoke(main, ["--allow-empty"]) assert result.exit_code == 0 @@ -60,7 +62,8 @@ class TestValidateDeployRef: assert result.exit_code == 1 assert "GITHUB_OUTPUT" in result.output - def test_allow_empty_with_github_output(self, tmp_path: Path) -> None: + @patch("devx.ci.validate_deploy_ref.subprocess.run") + def test_allow_empty_with_github_output(self, mock_subproc: MagicMock, tmp_path: Path) -> None: runner = CliRunner() gh_output = tmp_path / "github_output" gh_output.write_text("")