Skip to content

Commit 0c1e79b

Browse files
Check requires-mark coverage across CI runs
1 parent c68b25b commit 0c1e79b

File tree

4 files changed

+266
-1
lines changed

4 files changed

+266
-1
lines changed

.github/workflows/ci.yaml

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,8 @@ jobs:
169169
--timeout 180 \
170170
--cov=xarray \
171171
--cov-report=xml \
172-
--junitxml=pytest.xml
172+
--junitxml=pytest.xml \
173+
--report-log=reportlog.jsonl
173174
174175
- name: Upload test results
175176
if: always()
@@ -178,6 +179,13 @@ jobs:
178179
name: Test results for OS ${{ runner.os }} pixi-env ${{ matrix.pixi-env }} pytest-addopts ${{ matrix.pytest-addopts }}
179180
path: pytest.xml
180181

182+
- name: Upload report log
183+
if: always()
184+
uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0
185+
with:
186+
name: report-log-${{ runner.os }}-${{ matrix.pixi-env }}-${{ matrix.pytest-addopts || 'default' }}
187+
path: reportlog.jsonl
188+
181189
- name: Upload code coverage to Codecov
182190
uses: codecov/codecov-action@57e3a136b779b570ffcdbf80b3bdc90e7fab3de2 # v6.0.0
183191
env:
@@ -189,6 +197,29 @@ jobs:
189197
name: codecov-umbrella
190198
fail_ci_if_error: false
191199

200+
requires-coverage:
201+
name: Requires marker coverage
202+
runs-on: ubuntu-latest
203+
needs: test
204+
if: needs.test.result == 'success'
205+
206+
steps:
207+
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
208+
with:
209+
persist-credentials: false
210+
211+
- name: Download report logs
212+
uses: actions/download-artifact@95815c38cf2ff2164869cbab79da8d1f422bc89e # v4.2.1
213+
with:
214+
pattern: report-log-*
215+
path: report-logs
216+
217+
- name: Check requires marker coverage
218+
run: |
219+
python3 ci/check_requires_coverage.py \
220+
report-logs \
221+
--allowlist ci/requires_coverage_allowlist.txt
222+
192223
event_file:
193224
name: "Event File"
194225
runs-on: ubuntu-slim

ci/check_requires_coverage.py

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
#!/usr/bin/env python
2+
"""Check that requires_* tests are exercised in CI.
3+
4+
The CI test matrix should run tests guarded by ``requires_*`` markers in at
5+
least one environment whenever possible. This script inspects pytest report-log
6+
files from the test matrix and fails if a test is only ever skipped for a
7+
``requires`` reason, unless that skip reason is explicitly allowlisted.
8+
"""
9+
10+
from __future__ import annotations
11+
12+
import argparse
13+
import json
14+
import sys
15+
from collections import defaultdict
16+
from pathlib import Path
17+
from typing import Iterable
18+
19+
_SKIP_PREFIX = "Skipped: "
20+
21+
22+
def iter_reportlog_paths(paths: Iterable[Path]) -> list[Path]:
23+
reportlogs: list[Path] = []
24+
for path in paths:
25+
if path.is_dir():
26+
reportlogs.extend(sorted(path.rglob("*.jsonl")))
27+
elif path.suffix == ".jsonl":
28+
reportlogs.append(path)
29+
return reportlogs
30+
31+
32+
def load_allowlist(path: Path | None) -> set[str]:
33+
if path is None:
34+
return set()
35+
36+
allowlist = set()
37+
for line in path.read_text().splitlines():
38+
stripped = line.strip()
39+
if stripped and not stripped.startswith("#"):
40+
allowlist.add(stripped)
41+
return allowlist
42+
43+
44+
def _skip_reason(longrepr: object) -> str | None:
45+
if isinstance(longrepr, str):
46+
reason = longrepr
47+
elif isinstance(longrepr, list) and len(longrepr) >= 3:
48+
reason = str(longrepr[2])
49+
else:
50+
return None
51+
52+
if reason.startswith(_SKIP_PREFIX):
53+
return reason.removeprefix(_SKIP_PREFIX)
54+
return reason
55+
56+
57+
def collect_reportlog_data(reportlogs: Iterable[Path]) -> dict[str, dict[str, set[str]]]:
58+
"""Return per-nodeid execution and skip information."""
59+
data: dict[str, dict[str, set[str]]] = defaultdict(
60+
lambda: {"call_outcomes": set(), "skip_reasons": set()}
61+
)
62+
63+
for reportlog in reportlogs:
64+
for line in reportlog.read_text().splitlines():
65+
record = json.loads(line)
66+
if record.get("$report_type") != "TestReport":
67+
continue
68+
69+
nodeid = record.get("nodeid")
70+
if not nodeid:
71+
continue
72+
73+
when = record.get("when")
74+
outcome = record.get("outcome")
75+
if when == "call":
76+
data[nodeid]["call_outcomes"].add(str(outcome))
77+
elif when == "setup" and outcome == "skipped":
78+
reason = _skip_reason(record.get("longrepr"))
79+
if reason is not None:
80+
data[nodeid]["skip_reasons"].add(reason)
81+
82+
return data
83+
84+
85+
def uncovered_requires_tests(
86+
reportlogs: Iterable[Path],
87+
allowlist: set[str],
88+
) -> dict[str, set[str]]:
89+
"""Return tests that only ever skipped for a requires reason."""
90+
uncovered: dict[str, set[str]] = {}
91+
for nodeid, info in collect_reportlog_data(reportlogs).items():
92+
if info["call_outcomes"]:
93+
continue
94+
95+
skip_reasons = info["skip_reasons"]
96+
if not skip_reasons:
97+
continue
98+
99+
if all(reason.startswith("requires ") for reason in skip_reasons):
100+
missing_reasons = skip_reasons - allowlist
101+
if missing_reasons:
102+
uncovered[nodeid] = missing_reasons
103+
104+
return uncovered
105+
106+
107+
def main(argv: list[str] | None = None) -> int:
108+
parser = argparse.ArgumentParser(
109+
description="Validate that requires_* tests are exercised somewhere in CI."
110+
)
111+
parser.add_argument(
112+
"paths",
113+
nargs="+",
114+
type=Path,
115+
help="Report-log files or directories containing report-log files.",
116+
)
117+
parser.add_argument(
118+
"--allowlist",
119+
type=Path,
120+
default=None,
121+
help="Optional allowlist of skip reasons that are intentionally uncovered in CI.",
122+
)
123+
args = parser.parse_args(argv)
124+
125+
reportlogs = iter_reportlog_paths(args.paths)
126+
if not reportlogs:
127+
raise SystemExit("No report-log files were found.")
128+
129+
uncovered = uncovered_requires_tests(
130+
reportlogs, allowlist=load_allowlist(args.allowlist)
131+
)
132+
133+
if uncovered:
134+
print("The following tests are only ever skipped for a requires reason:")
135+
for nodeid, reasons in sorted(uncovered.items()):
136+
print(f"- {nodeid}: {', '.join(sorted(reasons))}")
137+
return 1
138+
139+
print(f"Checked {len(reportlogs)} report-log file(s); no uncovered requires tests found.")
140+
return 0
141+
142+
143+
if __name__ == "__main__":
144+
raise SystemExit(main())

ci/requires_coverage_allowlist.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
# Dependencies intentionally not exercised in GitHub Actions.
2+
requires cupy
3+
requires h5netcdf>=1.3.0 and h5py with ros3 support

ci/test_check_requires_coverage.py

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
from __future__ import annotations
2+
3+
import json
4+
from pathlib import Path
5+
6+
from ci.check_requires_coverage import uncovered_requires_tests
7+
8+
9+
def _write_reportlog(path: Path, records: list[dict[str, object]]) -> Path:
10+
path.write_text("\n".join(json.dumps(record) for record in records) + "\n")
11+
return path
12+
13+
14+
def test_uncovered_requires_test_is_reported(tmp_path: Path) -> None:
15+
reportlog = _write_reportlog(
16+
tmp_path / "reportlog.jsonl",
17+
[
18+
{"$report_type": "TestReport", "nodeid": "test_mod.py::test_gpu", "when": "setup", "outcome": "skipped", "longrepr": ["test_mod.py", 12, "Skipped: requires foo"]},
19+
{"$report_type": "TestReport", "nodeid": "test_mod.py::test_gpu", "when": "teardown", "outcome": "passed", "longrepr": None},
20+
],
21+
)
22+
23+
uncovered = uncovered_requires_tests([reportlog], allowlist=set())
24+
25+
assert uncovered == {"test_mod.py::test_gpu": {"requires foo"}}
26+
27+
28+
def test_requires_test_is_covered_if_it_runs_elsewhere(tmp_path: Path) -> None:
29+
reportlog_a = _write_reportlog(
30+
tmp_path / "a.jsonl",
31+
[
32+
{"$report_type": "TestReport", "nodeid": "test_mod.py::test_optional", "when": "setup", "outcome": "skipped", "longrepr": ["test_mod.py", 12, "Skipped: requires foo"]},
33+
],
34+
)
35+
reportlog_b = _write_reportlog(
36+
tmp_path / "b.jsonl",
37+
[
38+
{"$report_type": "TestReport", "nodeid": "test_mod.py::test_optional", "when": "setup", "outcome": "passed", "longrepr": None},
39+
{"$report_type": "TestReport", "nodeid": "test_mod.py::test_optional", "when": "call", "outcome": "passed", "longrepr": None},
40+
{"$report_type": "TestReport", "nodeid": "test_mod.py::test_optional", "when": "teardown", "outcome": "passed", "longrepr": None},
41+
],
42+
)
43+
44+
assert uncovered_requires_tests([reportlog_a, reportlog_b], allowlist=set()) == {}
45+
46+
47+
def test_requires_skip_reason_allowlist_is_respected(tmp_path: Path) -> None:
48+
reportlog = _write_reportlog(
49+
tmp_path / "reportlog.jsonl",
50+
[
51+
{"$report_type": "TestReport", "nodeid": "test_mod.py::test_ros3", "when": "setup", "outcome": "skipped", "longrepr": ["test_mod.py", 12, "Skipped: requires h5netcdf>=1.3.0 and h5py with ros3 support"]},
52+
],
53+
)
54+
55+
assert (
56+
uncovered_requires_tests(
57+
[reportlog],
58+
allowlist={"requires h5netcdf>=1.3.0 and h5py with ros3 support"},
59+
)
60+
== {}
61+
)
62+
63+
64+
def test_requires_skip_with_non_requires_skip_is_considered_covered(
65+
tmp_path: Path,
66+
) -> None:
67+
reportlog = _write_reportlog(
68+
tmp_path / "reportlog.jsonl",
69+
[
70+
{
71+
"$report_type": "TestReport",
72+
"nodeid": "test_mod.py::test_optional",
73+
"when": "setup",
74+
"outcome": "skipped",
75+
"longrepr": ["test_mod.py", 12, "Skipped: requires foo"],
76+
},
77+
{
78+
"$report_type": "TestReport",
79+
"nodeid": "test_mod.py::test_optional",
80+
"when": "setup",
81+
"outcome": "skipped",
82+
"longrepr": ["test_mod.py", 12, "Skipped: some other reason"],
83+
},
84+
],
85+
)
86+
87+
assert uncovered_requires_tests([reportlog], allowlist=set()) == {}

0 commit comments

Comments
 (0)