diff --git a/.github/actions/check-test-outcome/action.yml b/.github/actions/check-test-outcome/action.yml new file mode 100644 index 0000000..d87754d --- /dev/null +++ b/.github/actions/check-test-outcome/action.yml @@ -0,0 +1,40 @@ +name: Check test outcome +description: >- + Refuse the two ways a suite stops covering the code while the job still + reports success: a test that is SKIPPED, and a suite that SHRANK. + + A skipped test asserts nothing and reads as a pass in the conclusion. A test + the build never collects is reported nowhere at all, so the skip count cannot + see it — what moves is the total, which is why the total carries a floor. + + The rules themselves are tools/check-test-outcome.py in this repository, the + one runnable definition this action and a developer both call, so the CI + answer and the by-hand answer cannot differ. It reads surefire and pytest + summaries and sums every module's line. + +inputs: + log: + description: "Build log carrying the test summary." + required: true + min-tests: + description: >- + The floor the total may not fall below. Raise it when the suite grows; + lowering it is a deliberate act that belongs in the commit removing the + tests. + required: true + +runs: + using: composite + steps: + - name: Check out the rules + uses: actions/checkout@v4 + with: + repository: MobilityDB/MEOS-API + ref: master + path: .meos-api-outcome + + - name: Read the suite's own summary + shell: bash + run: | + python3 "$GITHUB_WORKSPACE/.meos-api-outcome/tools/check-test-outcome.py" \ + "${{ inputs.log }}" --min-tests "${{ inputs.min-tests }}" diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index 735c1ce..279d8a9 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -86,14 +86,42 @@ jobs: # conclusion, which is how the eleven went unnoticed. Every precondition # is supplied and asserted above, so a skip now means a guard reads a # condition this job no longer satisfies — a defect, not a circumstance. - - name: Refuse a silent skip + # + # The floor is the second half, and it guards what the skip count cannot + # see: a test the run never collects is reported nowhere, so removing one + # leaves the skip count at zero while coverage leaves. Raise it when the + # suite grows; lowering it belongs in the commit that removes the tests. + # Run the script from this checkout rather than through the action, which + # would fetch master: a pull request must be checked by the rules it + # carries, or a change to them is not exercised until after it merges. + # Consumers use the action; this repository owns the rules. + - name: Refuse a skip, and a suite that shrank + run: tools/check-test-outcome.py "$RUNNER_TEMP/pytest.log" --min-tests 268 + + # The rules earn their place by refusing a log that carries what they + # name. Both fixtures are written here rather than tracked, and the + # summary strings are assembled from parts so this file does not itself + # read as a skipping suite. + - name: Prove the check can fail run: | - if grep -qE '[0-9]+ skipped' "$RUNNER_TEMP/pytest.log"; then - grep -E '^SKIPPED' "$RUNNER_TEMP/pytest.log" || true - echo "::error::the suite skipped tests; every precondition is supplied, so a skip is a defect" + set -e + SK="skip""ped" + printf '===== 257 passed, 11 %s in 13.58s =====\n' "$SK" > "$RUNNER_TEMP/fx-skip.log" + if tools/check-test-outcome.py "$RUNNER_TEMP/fx-skip.log" --min-tests 268; then + echo "::error::the check passed a log reporting skips and a shrunken suite" + exit 1 + fi + printf '===== 267 passed in 13.58s =====\n' > "$RUNNER_TEMP/fx-shrank.log" + if tools/check-test-outcome.py "$RUNNER_TEMP/fx-shrank.log" --min-tests 268; then + echo "::error::the check passed a suite one test below its floor" + exit 1 + fi + printf 'nothing here\n' > "$RUNNER_TEMP/fx-empty.log" + if tools/check-test-outcome.py "$RUNNER_TEMP/fx-empty.log" --min-tests 268; then + echo "::error::the check passed a log carrying no summary at all" exit 1 fi - echo "no test skipped" + echo "the check refuses a skip, a shrunken suite, and a log with no summary" - name: Upload meos-idl.json as artefact if: always() diff --git a/GENERATION.md b/GENERATION.md index 9ab03a2..cbcb4a1 100644 --- a/GENERATION.md +++ b/GENERATION.md @@ -191,3 +191,34 @@ is what the action calls, so a local answer and a CI answer cannot differ. A deliberate exception is a recorded decision, so it lives in the repository as `tools/tree-hygiene-allow.txt` — one ` # why` per line — and carries its reason beside it. + +## Keeping a suite honest (check-test-outcome) + +Tree hygiene reads files; it cannot see what a run actually did. Two things a +suite does are invisible in a job's conclusion, and `check-test-outcome` refuses +both: + +| it refuses | why the job cannot see it otherwise | +| --- | --- | +| a test reported **skipped** | a skipped test carries no assertion, and the conclusion of a job with skips is `success`. `-Dmeos.enabled=false` is enough to disable a whole MEOS surface under a green build. | +| a suite that **shrank** | a test the run never collects appears in no count at all, so the skip number stays 0. A surefire ``, a `-Dtest=` filter, a class renamed out of `*Test` and a deleted file are all invisible to the first rule. The total is what moves, so the total carries a floor. | + +The second rule is the one that matters once the first exists: when skipping is +refused, deleting a test is the remaining way to stop running it. + +```yaml +- name: Refuse a skip, and a suite that shrank + uses: MobilityDB/MEOS-API/.github/actions/check-test-outcome@master + with: + log: ${{ runner.temp }}/build.log + min-tests: "268" +``` + +Pipe the build through `tee` to produce that log, and keep `set -o pipefail` so +the build's own failure is not masked by `tee` succeeding. The check reads both +the surefire and the pytest summary dialects, and sums every module's line: a +multi-module build prints one summary per module, so the last line alone +understates the total. + +Raise the floor when the suite grows. Lowering it is a deliberate act that +belongs in the same commit as the removal it accounts for. diff --git a/tools/check-test-outcome.py b/tools/check-test-outcome.py new file mode 100755 index 0000000..cb2ae52 --- /dev/null +++ b/tools/check-test-outcome.py @@ -0,0 +1,136 @@ +#!/usr/bin/env python3 +# check-test-outcome.py — read a build log's own test summary and refuse the two +# ways a suite stops covering the code while the job still reports success. +# +# This is the single, runnable definition of both rules: the check-test-outcome +# GitHub action calls it, and a developer runs it over a local build log, so the +# CI answer and the by-hand answer cannot differ. +# +# SKIPPED A skipped test asserts nothing and is indistinguishable from a +# passing one in the job's conclusion. Every construct that reaches +# it is reported in the summary: a JUnit @Disabled, an unmet +# @EnabledIf*, a failed assumeTrue, a Python skipUnless/skipTest. +# MEASURED: MobilityFlink's binding module under +# `-Dmeos.enabled=false` reports `Tests run: 7 ... Skipped: 7` and +# `BUILD SUCCESS` — the whole MEOS surface disabled by one flag, with +# a green build over it. +# +# SHRANK A test the build never collects is reported NOWHERE, so the skip +# rule above is blind to it: a surefire , a `-Dtest=` +# filter, a class renamed out of the `*Test` convention, or a file +# simply deleted. What moves is the TOTAL, so the total carries a +# floor that may rise and may not fall. Once skipping is refused, +# deleting a test is the remaining way to stop running it, and this +# is what stands in that path. +# +# Both summary dialects are read, and every module's line is summed rather than +# the last one taken — a multi-module build prints one summary per module, and +# reading only the last understates the total (measured: MobilityKafka prints 7 +# and 4, so its total is 11 rather than the 4 a tail would report). +# +# Usage: +# tools/check-test-outcome.py [--min-tests N] [--allow-skips] +# +# Exit status is 0 when the log satisfies both rules and 1 otherwise. + +import argparse +import re +import sys +from pathlib import Path + +# `Tests run: 12, Failures: 0, Errors: 0, Skipped: 0` — surefire prints this per +# CLASS (with a trailing `-- in `) and once per MODULE without it. Only +# the module lines are summed, or every test counts twice. +SUREFIRE = re.compile( + r"Tests run:\s*(\d+),\s*Failures:\s*(\d+),\s*Errors:\s*(\d+)," + r"\s*Skipped:\s*(\d+)\s*$") + +# `268 passed in 12.19s` / `257 passed, 11 skipped in 13.58s` — pytest's summary. +PYTEST = re.compile(r"(?:^|\s)(\d+)\s+passed(?:,\s*(\d+)\s+skipped)?") + + +def read_summaries(text: str): + """Return (total, skipped, dialect, lines) summed over every summary found.""" + total = skipped = 0 + lines = [] + for raw in text.splitlines(): + line = raw.rstrip() + m = SUREFIRE.search(line) + if m: + total += int(m.group(1)) + skipped += int(m.group(4)) + lines.append(line.strip()) + if lines: + return total, skipped, "surefire", lines + + for raw in text.splitlines(): + m = PYTEST.search(raw) + if m: + total += int(m.group(1)) + skipped += int(m.group(2) or 0) + lines.append(raw.strip()) + if lines: + return total, skipped, "pytest", lines + + return 0, 0, None, [] + + +def main() -> int: + ap = argparse.ArgumentParser(description=__doc__) + ap.add_argument("log", help="build log carrying the test summary") + ap.add_argument("--min-tests", type=int, default=0, + help="floor the total may not fall below") + ap.add_argument("--allow-skips", action="store_true", + help="report skips without failing (never in CI)") + args = ap.parse_args() + + path = Path(args.log) + if not path.exists(): + print(f"check-test-outcome: no log at {path}", file=sys.stderr) + return 1 + text = path.read_text(errors="replace") + + total, skipped, dialect, lines = read_summaries(text) + + # An extraction that finds nothing is a statement about the parser until it + # shows it found its input, so a log with no summary at all is a failure + # rather than a silent pass. + if dialect is None: + print(f"check-test-outcome: {path} ({len(text)} bytes) carries no test " + f"summary in either dialect — the suite did not run, or the log " + f"is not the one the run wrote", file=sys.stderr) + return 1 + + print(f"check-test-outcome: {dialect}, {len(lines)} summary line(s)") + for line in lines: + print(f" {line}") + print(f" total={total} skipped={skipped} floor={args.min_tests}") + + failed = False + + if skipped and not args.allow_skips: + for line in text.splitlines(): + s = line.strip() + if s.startswith("SKIPPED") or " SKIPPED " in s: + print(f" {s}") + print(f"::error::{skipped} test(s) skipped. A skipped test asserts " + f"nothing and reports as success. Supply the precondition its " + f"guard reads, or remove the guard.") + failed = True + + if total < args.min_tests: + print(f"::error::the suite ran {total} tests against a floor of " + f"{args.min_tests}. Tests that are not collected are reported " + f"nowhere, so a total that falls is how coverage leaves without " + f"a skip. Restore them, or lower the floor deliberately in the " + f"same commit that removes them.") + failed = True + + if failed: + return 1 + print("check-test-outcome: nothing skipped, and the suite did not shrink") + return 0 + + +if __name__ == "__main__": + sys.exit(main())