Skip to content

Commit 66350d8

Browse files
authored
Merge pull request #760 from mrava87/test-picov
Agent: added pi coverage checker
2 parents dac9174 + 343d2e9 commit 66350d8

3 files changed

Lines changed: 153 additions & 0 deletions

File tree

.pi/prompts/optest.md

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
---
2+
description: Raise an operator's test coverage above 90% by adding/modifying tests
3+
argument-hint: "<OperatorName> [threshold]"
4+
---
5+
Goal: bring test coverage for the PyLops operator `$1` to at least **${2:-90}%**.
6+
7+
Follow this workflow precisely:
8+
9+
1. **Locate the operator.** Find the source module that defines `class $1`
10+
(e.g. `grep -rln "^class $1\b" pylops/`) and the test file(s) that already
11+
exercise it (`grep -rln "$1" pytests/`).
12+
13+
2. **Measure baseline coverage** for this operator only:
14+
```bash
15+
.pi/tools/operator_coverage.sh $1
16+
```
17+
Read the reported percentage and the list of *missing* line numbers.
18+
19+
3. **Inspect the uncovered lines** in the source module. For each missing line,
20+
identify what behaviour is untested: alternate dtypes, branches (e.g.
21+
`kind`/`edge`/`order` options), error paths (`raise`/`NotImplementedError`),
22+
adjoint vs forward, ND vs 1D, backend dispatch, etc.
23+
24+
4. **Add or modify tests** in the existing `pytests/test_*.py` file for this
25+
operator. Match the repo's conventions:
26+
- Parametrize with `@pytest.mark.parametrize` over `par` dicts and `dtype`.
27+
- Always include a `dottest(...)` adjoint check for new configurations.
28+
- Use `assert_array_almost_equal` for forward/inverse comparisons.
29+
- Keep the CuPy/`backend` guard pattern used at the top of the test file.
30+
Do NOT weaken assertions or add trivial no-op tests just to hit lines.
31+
32+
5. **Re-run** `.pi/tools/operator_coverage.sh $1` and iterate steps 3–4 until
33+
`COVERAGE_PCT >= ${2:-90}`. If some lines are genuinely untestable on the
34+
current backend (e.g. CUDA-only paths), say so explicitly and exclude them
35+
from the target with justification rather than faking coverage.
36+
37+
6. **Validate** the new tests actually pass and lint cleanly:
38+
```bash
39+
make lint
40+
```
41+
(run the relevant pytest file directly if a full `make tests` is too slow).
42+
43+
7. **Report** a short summary: starting %, final %, which test functions were
44+
added/changed, and any lines deliberately left uncovered with the reason.

.pi/tools/operator_coverage.sh

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
#!/usr/bin/env bash
2+
#
3+
# operator_coverage.sh -- measure test coverage for a single PyLops operator.
4+
#
5+
# Usage:
6+
# .pi/tools/operator_coverage.sh <OperatorName> [extra pytest args...]
7+
#
8+
# Example:
9+
# .pi/tools/operator_coverage.sh FirstDerivative
10+
# .pi/tools/operator_coverage.sh FFT -k fft
11+
#
12+
# It locates the source module that defines `class <OperatorName>`, runs the
13+
# test suite with coverage scoped to that single module, then prints the
14+
# coverage percentage and the list of uncovered (missing) line numbers.
15+
#
16+
# Runner selection (first available wins, override with RUNNER env var):
17+
# 1. $RUNNER (e.g. RUNNER="uv run")
18+
# 2. uv run (if `uv` is on PATH)
19+
# 3. python3 -m (if the `coverage` module is importable)
20+
#
21+
set -euo pipefail
22+
23+
OP="${1:-}"
24+
if [[ -z "$OP" ]]; then
25+
echo "usage: $0 <OperatorName> [extra pytest args...]" >&2
26+
exit 2
27+
fi
28+
shift || true
29+
30+
# Script lives in <repo>/.pi/tools/, so the repo root is two levels up.
31+
REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
32+
cd "$REPO_ROOT"
33+
34+
# --- locate the source module defining the operator -------------------------
35+
SRC="$(grep -rln "^class ${OP}\b" pylops/ || true)"
36+
if [[ -z "$SRC" ]]; then
37+
echo "error: could not find 'class ${OP}' under pylops/" >&2
38+
exit 1
39+
fi
40+
if [[ "$(echo "$SRC" | wc -l)" -gt 1 ]]; then
41+
echo "warning: multiple modules define '${OP}':" >&2
42+
echo "$SRC" >&2
43+
SRC="$(echo "$SRC" | head -1)"
44+
echo "using: $SRC" >&2
45+
fi
46+
47+
# --- locate the test file(s) that reference this operator -------------------
48+
# Used as the default pytest target so we don't run (and break on) the whole
49+
# suite, which may fail to collect due to optional deps (torch, cupy, ...).
50+
# Pick the test file with the most references to the operator (the dedicated
51+
# one), avoiding files that merely use it as a building block.
52+
TESTS="$(grep -rcl --include='*.py' "\b${OP}\b" pytests/ 2>/dev/null \
53+
| xargs -r grep -rc "\b${OP}\b" 2>/dev/null \
54+
| sort -t: -k2 -nr | head -1 | cut -d: -f1 || true)"
55+
if [[ $# -gt 0 ]]; then
56+
PYTEST_ARGS=("$@") # caller-provided args win
57+
elif [[ -n "$TESTS" ]]; then
58+
PYTEST_ARGS=("$TESTS")
59+
else
60+
PYTEST_ARGS=() # fall back to full suite
61+
fi
62+
63+
# --- pick a runner ----------------------------------------------------------
64+
if [[ -n "${RUNNER:-}" ]]; then
65+
RUN=($RUNNER)
66+
elif command -v uv >/dev/null 2>&1; then
67+
RUN=(uv run)
68+
elif python3 -c "import coverage" >/dev/null 2>&1; then
69+
RUN=(python3 -m)
70+
# python3 -m coverage ... -> prepend nothing, handled below
71+
RUN=()
72+
else
73+
echo "error: no runner found. Install 'uv' or 'coverage' (pip install coverage pytest)." >&2
74+
exit 1
75+
fi
76+
77+
cov() { "${RUN[@]}" coverage "$@"; }
78+
79+
echo "==> operator : ${OP}"
80+
echo "==> source : ${SRC}"
81+
echo "==> runner : ${RUN[*]:-python3 -m} coverage"
82+
echo "==> pytest : ${PYTEST_ARGS[*]:-<full suite>}"
83+
echo
84+
85+
# --- run coverage scoped to that single source file -------------------------
86+
cov run --source=pylops -m pytest "${PYTEST_ARGS[@]}" >/tmp/optest_pytest.log 2>&1 || {
87+
echo "pytest run failed; tail of log:" >&2
88+
tail -40 /tmp/optest_pytest.log >&2
89+
exit 1
90+
}
91+
92+
echo "===================== COVERAGE (missing lines) ====================="
93+
cov report -m --include="$SRC"
94+
echo "===================================================================="
95+
96+
# --- extract percentage for scripting / threshold checks --------------------
97+
PCT="$(cov report --include="$SRC" | awk '/TOTAL|'"$(basename "$SRC")"'/{gsub("%","",$NF); print $NF}' | tail -1)"
98+
echo
99+
echo "COVERAGE_PCT=${PCT}"

pytests/test_derivative.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,16 @@
9898
np.random.seed(10)
9999

100100

101+
def test_FirstDerivative_invalid():
102+
"""FirstDerivative raises NotImplementedError for invalid kind/order"""
103+
# invalid centered order
104+
with pytest.raises(NotImplementedError, match="order must be"):
105+
FirstDerivative(10, kind="centered", order=4)
106+
# invalid kind
107+
with pytest.raises(NotImplementedError, match="kind must be"):
108+
FirstDerivative(10, kind="invalid")
109+
110+
101111
@pytest.mark.parametrize(
102112
"par", [(par1), (par2), (par3), (par4), (par1e), (par2e), (par3e), (par4e)]
103113
)

0 commit comments

Comments
 (0)