From 9fcd1cdd67da63382016a25891c9a5b6049fa84c Mon Sep 17 00:00:00 2001 From: Esteban Zimanyi Date: Tue, 8 Sep 2026 00:09:14 +0200 Subject: [PATCH] Remove the flag that lets a run report skips as acceptable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `--allow-skips` made the rule conditional: a repository passing it reported success over a suite that asserted nothing, and the summary line said so in words that read like a pass. No workflow in the ecosystem passes it — a code search over MEOS-API, GoMEOS, JMEOS, MobilityAPI, MobilityFlink, MobilitySpark, MobilityKafka, MobilityDuck and PyMEOS finds the flag only in this file's own definition, and the composite action never exposed it — so it stands as an escape hatch and nothing else. The parser now rejects it: `unrecognized arguments: --allow-skips`. The command line moves into `build_parser` so the suite can read its options, and two tests keep the hatch shut. One asserts that no option carries `skip` or `ignore` in its name, against a positive control that `--min-tests` is there, so the assertion cannot pass over a parser it failed to build. The other reads a vstest summary carrying one skip and holds that the count reaches the caller whatever the floor says. 18 tests pass. Against a real `dotnet test` log the tool exits 0 on a clean run, 1 on a summary carrying `Skipped: 1`, 1 on a floor above the total, and 2 on the removed flag. --- tests/test_test_outcome.py | 27 +++++++++++++++++++++++++++ tools/check-test-outcome.py | 27 ++++++++++++++------------- 2 files changed, 41 insertions(+), 13 deletions(-) diff --git a/tests/test_test_outcome.py b/tests/test_test_outcome.py index 10f6b40..9c65473 100644 --- a/tests/test_test_outcome.py +++ b/tests/test_test_outcome.py @@ -141,5 +141,32 @@ def test_a_vstest_run_over_an_empty_assembly_names_no_dialect(self): self.assertIsNone(outcome.read_summaries(log)[2]) +class NoFlagPermitsASkipTests(unittest.TestCase): + """No option tolerates a skip, and none may be added back. + + A flag that makes skips acceptable is the whole rule's escape hatch: the + repository holding it can pass the flag and report success over a suite that + covers nothing. Refusing the flag at the parser is what makes the rule + unconditional, and this test is what keeps it refused.""" + + def _parser_options(self): + parser = outcome.build_parser() + return {opt for action in parser._actions for opt in action.option_strings} + + def test_the_parser_offers_no_option_tolerating_a_skip(self): + options = self._parser_options() + # A positive control: the parser is real and does carry its own options. + self.assertIn("--min-tests", options) + permissive = {o for o in options + if "skip" in o.lower() or "ignore" in o.lower()} + self.assertEqual(set(), permissive) + + def test_a_skip_fails_whatever_the_floor(self): + log = ("Passed! - Failed: 0, Passed: 66, Skipped: 1, " + "Total: 67, Duration: 1 s - Suite.dll (net8.0)\n") + total, skipped, dialect, _ = outcome.read_summaries(log) + self.assertEqual(("vstest", 67, 1), (dialect, total, skipped)) + + if __name__ == "__main__": unittest.main() diff --git a/tools/check-test-outcome.py b/tools/check-test-outcome.py index 1a64e5a..ab31f59 100755 --- a/tools/check-test-outcome.py +++ b/tools/check-test-outcome.py @@ -44,7 +44,7 @@ # consumer tees must come from `go test -v`. # # Usage: -# tools/check-test-outcome.py [--min-tests N] [--allow-skips] +# tools/check-test-outcome.py [--min-tests N] # # Exit status is 0 when the log satisfies both rules and 1 otherwise. @@ -176,14 +176,22 @@ def read_summaries(text: str): return 0, 0, None, [] -def main() -> int: +def build_parser() -> argparse.ArgumentParser: + """The command line, as its own function so a test can read the options. + + There is deliberately no option that tolerates a skip. A flag permitting one + is the rule's escape hatch — the repository holding it passes the flag and + reports success over a suite that covers nothing — so the absence is a + property the suite asserts rather than a convention.""" 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() + return ap + + +def main() -> int: + args = build_parser().parse_args() path = Path(args.log) if not path.exists(): @@ -209,7 +217,7 @@ def main() -> int: failed = False - if skipped and not args.allow_skips: + if skipped: for line in text.splitlines(): s = line.strip() if (s.startswith("SKIPPED") or " SKIPPED " in s @@ -230,13 +238,6 @@ def main() -> int: if failed: return 1 - if skipped: - # Only --allow-skips reaches here with a non-zero count, and saying - # "nothing skipped" over it would misreport the one run that tolerates - # them. - print(f"check-test-outcome: {skipped} skipped, tolerated by " - f"--allow-skips; the suite did not shrink") - return 0 print("check-test-outcome: nothing skipped, and the suite did not shrink") return 0