Public Access
DEVX-136: feat: add fix_pr_title module and update_pr API method
This commit was merged in pull request #203.
This commit is contained in:
@@ -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.
|
||||
|
||||
|
||||
@@ -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}: <Vikunja task title>",
|
||||
),
|
||||
err=True,
|
||||
)
|
||||
|
||||
raise click.ClickException(_("Pre-merge validation failed."))
|
||||
|
||||
click.echo("[pre-merge-check] All auto-merge preconditions satisfied.")
|
||||
|
||||
@@ -0,0 +1,127 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Auto-fix PR title to follow the ``{PREFIX}-N: <title>`` 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
|
||||
@@ -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
|
||||
|
||||
+160
-24
@@ -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."
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user