Public Access
174 lines
7.4 KiB
Markdown
174 lines
7.4 KiB
Markdown
# ADR-0001: Test Isolation Pytest Plugin and Shift-Left Quality Gates
|
|
|
|
Date: 2026-07-13
|
|
Status: Accepted
|
|
|
|
## Context
|
|
|
|
Unit tests in devx were slow (10s+) and getting slower. Investigation
|
|
revealed two root causes:
|
|
|
|
1. **Unpatched subprocess calls** — test functions calling
|
|
`subprocess.run`, `update_doc_versions`, or `run_cmd` without
|
|
`@patch` decorators, causing real subprocess execution during tests.
|
|
2. **Excessive iterations** — statistical tests with 1000-iteration
|
|
loops that should use property-based testing or smaller samples.
|
|
|
|
These issues were discovered manually by profiling with
|
|
`pytest --durations=0`. There was no automated check to prevent
|
|
regressions — new tests could introduce the same patterns and slow
|
|
down the suite again.
|
|
|
|
Additionally, translation completeness checks
|
|
(`devx.ci.check_translations`) only ran in CI, not locally. Developers
|
|
discovered missing translations at CI time, wasting round-trips.
|
|
|
|
## Decision
|
|
|
|
### 1. Test Isolation as a Pytest Plugin (pytest11 entry point)
|
|
|
|
Implement the test isolation check as a **pytest plugin** registered
|
|
via the `pytest11` entry point in `pyproject.toml`:
|
|
|
|
```toml
|
|
[project.entry-points.pytest11]
|
|
devx_test_isolation = "devx.tools.check_test_isolation"
|
|
```
|
|
|
|
This makes the check **transparent and always-on** — every `pytest`
|
|
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 **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` 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
|
|
|
|
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.
|
|
|
|
### 2. Shift-Left Quality Gates in `make lint`
|
|
|
|
Add `devx-check-translations` and `devx-check-test-isolation` to the
|
|
`devx-lint` target in `devx.mak`. This means `make lint` now runs:
|
|
|
|
- ruff check + format
|
|
- pyright typecheck
|
|
- bandit security scan
|
|
- **translation completeness** (missing keys, dead keys, missing languages)
|
|
- **test isolation** (unpatched subprocess, time.sleep, excessive loops)
|
|
|
|
These were previously CI-only checks. Running them in `make lint`
|
|
catches issues at the developer's machine, not in CI.
|
|
|
|
### 3. Pre-commit Hook Coverage
|
|
|
|
Update the pre-commit hook to run all three shift-left checks:
|
|
test speed, translation completeness, and test isolation. This
|
|
catches issues even earlier than `make lint` — before the commit
|
|
is even created.
|
|
|
|
## Consequences
|
|
|
|
### Positive
|
|
|
|
- **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 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; 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
|
|
|
|
- **Coverage instrumentation gap**: The pytest plugin module is loaded
|
|
before coverage starts, so module-level code (decorators, class
|
|
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 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.
|
|
|
|
## Implementation Details
|
|
|
|
### Pytest Plugin Discovery
|
|
|
|
The `pytest11` entry point is the standard mechanism for pytest
|
|
plugins. When devx is installed (via pip), pytest auto-discovers
|
|
the plugin. No `conftest.py` or `pytest_plugins` declaration needed
|
|
in consumer repos.
|
|
|
|
### Disabling the Plugin
|
|
|
|
- `--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)
|
|
|
|
### Call-Graph Analysis
|
|
|
|
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 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.
|