Refuse a skip, and a suite that shrank

.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 }}"

.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()

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 `<rule> <path-glob>  # 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 `<excludes>`, 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.

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 <excludes>, 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 <build.log> [--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 <class>`) 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())