From 5fbabb099f1839d6cd62363072adf9ac3309303d Mon Sep 17 00:00:00 2001 From: Joris Wouter Jonkers Date: Fri, 16 Oct 2026 15:14:42 +0200 Subject: [PATCH] feat: a quarantine check that fails the build once a quarantine runs out --- .coverage | Bin 53248 -> 53248 bytes .github/workflows/ci.yml | 7 ++ .github/workflows/quarantine-check.yml | 33 +++++++++ README.md | 1 + release-please-config.json | 4 + scripts/quarantine_check.py | 99 +++++++++++++++++++++++++ tests/test_quarantine_check.py | 98 ++++++++++++++++++++++++ 7 files changed, 242 insertions(+) create mode 100644 .github/workflows/quarantine-check.yml create mode 100644 scripts/quarantine_check.py create mode 100644 tests/test_quarantine_check.py diff --git a/.coverage b/.coverage index 6c3c1e30be477fb76808497321154f41f0c3201a..944624ea59806bd5d007e7fe08762ed26b913f7e 100644 GIT binary patch delta 1915 zcmZA1OH89x6bJA=fJ5QC_rjD?9xa6_Z3h_2bo!$2mco?M>Er8zDNtyDOw`OVPS}IVT^HM%ojrpF@_jpTo_|q7-L)zET@N!yI=n2=HBGo zbMNkL`( zsq07`-xqC<+L}vVudny~f638#9pc&w(;mR#hs_OTsR5OColdQ;u0R>FEJFzqQ=o{5 z#z02QDo{Z9XCaTM)gg!2x_~TVqYifw^<&5&);C}YQCo*JVzmZ~h_yO2QmtqW?jVX~ zSU^-)VIHwkg(N~Qz#O6?LjqB*K)jVKLkv+WLljXgK?ETeA&e-<5Ne?cK}5a)fd(p< zhgn262Yy5*3qHhB2E2&$5_k}cX>cQs7vVM{wFNFjQh{5Dxg^XW<`r-v5(-SWO7n0N z5l_HWd*ibC@i~~pC=!JUL^KXJ5a9?o5TP*GTZn=U5e&gNA`k>C!mGe_gg*dd2%jHD z5gspCTJXUL!tDWbyROkZUU7p7BUcKB5zZ4BLYz2Z5Han70mS4f7!jwF(2p3O1_NT^ z4Ehj`3D6_#4!DM}*`XI-*=nbGjX!M}IL_>X{=~Agw73qnJYxz(7Tnvj% z@sIdJ{33o3iug)=CO#4Gi?_vrcvUpSW3eOd37g1@qzH%^(JO%e#ed`9^Dp=}{5}4u z^fUiRI^}Qi*Z9l)75`(TqkNwELV_&n+*$3>1 zZL@dSn{0vYvlrR3Y)?9tp0GSyVIk&VE@qZ~XL{*^>7>7#Z}+^rq3gJup61)Fke1~# zDW;T2(O8i*tH>n(Y=Km(=Sf=^InqWwOR68=C9Q8{NVWAPQe(B2Ca%>NNvmrqQn7r8 zR9#<yJepav@2o$aAD}C2^H4$4RAfj8rT|NpdlAspjmc3v!skLN!Fn7lNc*K0wOm zW=WZ>pR|Qd_r4NySB)OWx8oTJ4`#W++T3&a3kLG%23Ad8N52 zQY1P_ipD2M;m8eADD1dW)J_V9Y@|SNoaAjNR-!*}o#gY6kv!hfEBP!Ww`YW;xXmP2 z%0zOW43kcrL!@cfAZhY+fOI-(L^bHQnCK(f9eR?@evM?c^^(r4Jy+Sd zAdOl%$ui1FBNmBd9_c2T%w4*UdpP|=CLM)?L!G4NS3oijbddU+-&X6w4E=2+y+KRT P=rx^K)m>=b-R=1gnbP-p delta 2328 zcmZA1YiL_#7zgmCw`W=3OVV!L(k5Hi<}_`yb#2mHwl?WCUApDcW@~y~o8Hrlt93&} z6ZSwyDxgUGaCt@uHPSWpo^tRhlHL_|asM@^se90bp&AOHW$IlM3L zd9Eh2t|qhI&TU=GjXWuHR}9%1*AlBY!z0$EE0^P!6PF$sTbW+Iv~sz6`H}q@85st7 zBSYR;Gn<}}IV;)M62`r?@_%dB?vFuh)(o3ph2)#1>~HoP`;pyaU$MJvligwOvRf?4 zUS!X*>+CW15SvfG?u9%RW^#;1gCQJ&X@qYRrVw5qOlsJG2@PI|Bit?+M<{j}LoDyY zMZ{7PMiGlkFoIZEgki+|0t_J%^Du~*OMp6nnw^6fVtN+(5mVD}0WmoR=MfW=5Jki% zpbwFZLoZ@{86t?$6oe5YqYy$2k3bMHGz{kugF|o@5gUXaRqIj^Kn$edjMfc6HzE}S zKcasex)7l-bRv2q(1D1qLp!1`3T=o;FSKgt10TX40xu#IffhtC1kE|>-r@K7;k34> z2TmaZe()f=1JI;Z-Oz~W@;5eeW9h_R?hAKpTBUB$aDzju8lW6eR}TupT?clA%ME3S zDg{cl!~@3=wJs=0Lp2y`YT+nGTQzV5QQZtSL}e9NH7HPwaBf2p!chqpM1=x}wWIC@6$nM1BF}Ak6uYjZjTy$U+GdWFj&J7!bRed;k6n%gJDO z`Du|Q4hg6DOvdDb{8RoSzn9<0&*dlbj(kVHBHxg!@@c8cD>5#FvO}Jf9@ZumSuD-s zz69};xF=o{zl$%$N8&y4l6Z?Xis!_2RXi$2#e*U$e4<{=ihwxA%Ge2JVN!7Z7yp6( z%D?4b@HPGn|CE2oZ}C@om_N^Q_{01fALL7Xl6Uc1UcepP$Zc$w{lR`_-?6XR$LxJ} zo4w9nW;fYW?D6!(Nw6_?ft^k7>Az>~h{>L}diw)V!;u+d?%v;E@NG_$yuK-28nDd@*YPz2^ zIdy?FF?pU8pNNu@@jlY{axZB#6(Nm`hDpOCA=1!r(5PyA9~?SIVQlcMUZ;9U1F3-C z4xAySV%?(?jrR@I!V#>4pLvVofPS9)1~&c68#|`DHQRNf}s{tkH1-8*K?W_ z@Sh@e2RwS)-9+m0HOjvguN+Be-< dYLhveBurVPOp&<{TjOpfPtSw3?5qFCe*s7DGXVeq diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 53b8e97..7d52f7c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -114,6 +114,13 @@ jobs: .venv-tests/bin/python -m coverage run --branch --source=scripts.mutation_report -m unittest tests.test_mutation_report .venv-tests/bin/python -m coverage report --show-missing --fail-under=100 + # A quarantine check that passes when it should fail lets a skipped test stay skipped forever. + - name: Run quarantine check tests with branch coverage + if: ${{ !cancelled() }} + run: | + .venv-tests/bin/python -m coverage run --branch --source=scripts.quarantine_check -m unittest tests.test_quarantine_check + .venv-tests/bin/python -m coverage report --show-missing --fail-under=100 + # --------------------------------------------------------------------------- # Terminal aggregator. This is the single required check in the ruleset. # It runs even when an upstream job fails/cancels (if: always()) and then diff --git a/.github/workflows/quarantine-check.yml b/.github/workflows/quarantine-check.yml new file mode 100644 index 0000000..171050a --- /dev/null +++ b/.github/workflows/quarantine-check.yml @@ -0,0 +1,33 @@ +name: Quarantine Check + +# Fails the build on a quarantined test whose quarantine has run out, names no issue, or lasts +# more than 14 days (workspace docs/testing-standards.md, section 7). A flaky test is fixed, or +# skipped for at most 14 days with the issue that tracks its fix; never skipped silently. + +on: + workflow_call: {} + +permissions: + contents: read + +jobs: + quarantine: + name: Quarantine check + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + persist-credentials: false + + # Checkout github-workflows at this workflow file's own released version; release-please + # keeps the literal in step with the tag this file ships in. + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + repository: JorisJonkers-dev/github-workflows + ref: v0.25.0 # x-release-please-version + path: .github-workflows + persist-credentials: false + + - name: Check every quarantine is live + run: python3 .github-workflows/scripts/quarantine_check.py diff --git a/README.md b/README.md index ebc53af..5ca9c7f 100644 --- a/README.md +++ b/README.md @@ -34,6 +34,7 @@ should call released tags instead of branches. | `python-ci.yml` | Python Ruff, typecheck, and test workflow with overrideable commands. | | `nix-ci.yml` | Nix formatter, Statix, Deadnix, flake check, and optional validation. | | `jvm-ci.yml` | Gradle static lint/test workflow that fails Gradle warnings and deprecations. | +| `quarantine-check.yml` | Fails the build on a quarantined test whose quarantine has expired, names no issue, or lasts more than 14 days. The marker is `quarantine: until YYYY-MM-DD`, in the reason of whatever skips the test. | | `mutation-report.yml` | Mutates the lines a pull request changes (gremlins for Go, Stryker for TypeScript and Vue) and keeps one comment listing the mutants that survived. Never fails the build. | | `docker-image-ci.yml` | Build a Docker image without publishing. | | `container-publish.yml` | Publish a GHCR image with release and sha tags. | diff --git a/release-please-config.json b/release-please-config.json index 5bb683e..e1673a7 100644 --- a/release-please-config.json +++ b/release-please-config.json @@ -42,6 +42,10 @@ "type": "generic", "path": ".github/workflows/path-buckets.yml" }, + { + "type": "generic", + "path": ".github/workflows/quarantine-check.yml" + }, { "type": "generic", "path": ".github/workflows/jvm-ci.yml" diff --git a/scripts/quarantine_check.py b/scripts/quarantine_check.py new file mode 100644 index 0000000..169af00 --- /dev/null +++ b/scripts/quarantine_check.py @@ -0,0 +1,99 @@ +#!/usr/bin/env python3 +"""Fail the build on a quarantined test whose quarantine has run out. + +A flaky test is fixed, or quarantined for at most 14 days with the issue that +tracks its fix (workspace docs/testing-standards.md, section 7). The marker is one +string, put in the reason of whatever skips the test in its language: + + t.Skip("quarantine: https://github.com///issues/ until 2026-10-30") + test.fixme(true, 'quarantine: https://github.com/... until 2026-10-30') + @Disabled("quarantine: https://github.com/... until 2026-10-30") + +Every tracked test file is read. A marker fails when its date has passed, when it +is more than 14 days away, when it names no issue, or when it has no date. So a +quarantine cannot become permanent: once its date passes, the build fails again. + + quarantine_check.py [--root DIR] [--today YYYY-MM-DD] +""" + +from __future__ import annotations + +import argparse +import re +import subprocess +from dataclasses import dataclass +from datetime import date, timedelta +from pathlib import Path + +LONGEST = timedelta(days=14) +MARKER = re.compile(r"quarantine:(?P[^\"'`)]*)") +ISSUE = re.compile(r"https://github\.com/[\w.-]+/[\w.-]+/issues/\d+") +UNTIL = re.compile(r"\buntil\s+(\d{4}-\d{2}-\d{2})\b") +TEST_FILE = re.compile(r"(_test\.go|\.(spec|test)\.[cm]?[jt]sx?|Test\.(kt|java))$") + + +@dataclass(frozen=True) +class Finding: + path: str + line: int + problem: str + + def annotation(self) -> str: + return f"::error file={self.path},line={self.line}::quarantine {self.problem}" + + +def is_test_file(path: str) -> bool: + return bool(TEST_FILE.search(path)) + + +def check_line(line: str, today: date) -> str | None: + """Why the quarantine on this line is not allowed, or None when it is, or there is none.""" + marker = MARKER.search(line) + if not marker: + return None + rest = marker.group("rest") + if not ISSUE.search(rest): + return "names no issue: give the GitHub issue that tracks the fix" + until = UNTIL.search(rest) + try: + expires = date.fromisoformat(until.group(1)) if until else None + except ValueError: + expires = None + if expires is None: + return "has no date: end it with `until YYYY-MM-DD`" + if expires < today: + return f"expired on {expires}: fix the test or remove it" + if expires > today + LONGEST: + return f"expires {expires}, more than 14 days away" + return None + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description="Fail on a quarantined test whose quarantine has run out.") + parser.add_argument("--root", default=".", help="the repository to read") + parser.add_argument("--today", type=date.fromisoformat, default=date.today(), help="the date to judge against") + args = parser.parse_args(argv) + + tracked = subprocess.run(["git", "ls-files"], cwd=args.root, capture_output=True, text=True, check=True).stdout.splitlines() + findings: list[Finding] = [] + quarantined = 0 + for path in filter(is_test_file, tracked): + for number, line in enumerate(Path(args.root, path).read_text(errors="replace").splitlines(), start=1): + if MARKER.search(line): + quarantined += 1 + problem = check_line(line, args.today) + if problem: + findings.append(Finding(path, number, problem)) + + for finding in findings: + print(finding.annotation()) + if not quarantined: + print("No quarantined tests.") + elif not findings: + noun = "test" if quarantined == 1 else "tests" + print(f"{quarantined} quarantined {noun}, every one live.") + return 1 if findings else 0 + + +if __name__ == "__main__": # pragma: no cover + raise SystemExit(main()) diff --git a/tests/test_quarantine_check.py b/tests/test_quarantine_check.py new file mode 100644 index 0000000..f50cd5d --- /dev/null +++ b/tests/test_quarantine_check.py @@ -0,0 +1,98 @@ +from __future__ import annotations + +import io +import subprocess +import tempfile +import unittest +from contextlib import redirect_stdout +from datetime import date +from pathlib import Path + +from scripts.quarantine_check import Finding, check_line, is_test_file, main + +TODAY = date(2026, 10, 16) +ISSUE = "https://github.com/JorisJonkers-dev/identity/issues/60" + + +class ReadingAMarker(unittest.TestCase): + def test_a_marker_with_an_issue_and_a_date_within_fourteen_days_is_accepted(self) -> None: + self.assertIsNone(check_line(f't.Skip("quarantine: {ISSUE} until 2026-10-30")', TODAY)) + + def test_a_marker_expiring_today_is_still_accepted(self) -> None: + self.assertIsNone(check_line(f"// quarantine: {ISSUE} until 2026-10-16", TODAY)) + + def test_a_marker_past_its_date_fails_the_build_again(self) -> None: + self.assertEqual(check_line(f"quarantine: {ISSUE} until 2026-10-15", TODAY), "expired on 2026-10-15: fix the test or remove it") + + def test_a_marker_more_than_fourteen_days_away_is_refused(self) -> None: + self.assertEqual(check_line(f"quarantine: {ISSUE} until 2026-10-31", TODAY), "expires 2026-10-31, more than 14 days away") + + def test_a_marker_without_an_issue_link_is_refused(self) -> None: + self.assertEqual(check_line("quarantine: flaky on CI until 2026-10-20", TODAY), "names no issue: give the GitHub issue that tracks the fix") + + def test_a_marker_without_a_date_is_refused(self) -> None: + self.assertEqual(check_line(f"quarantine: {ISSUE}", TODAY), "has no date: end it with `until YYYY-MM-DD`") + + def test_a_marker_with_an_impossible_date_is_refused(self) -> None: + self.assertEqual(check_line(f"quarantine: {ISSUE} until 2026-02-30", TODAY), "has no date: end it with `until YYYY-MM-DD`") + + def test_a_line_without_a_marker_is_ignored(self) -> None: + self.assertIsNone(check_line("func TestAnAdminAddsAPersonToAGroup(t *testing.T) {", TODAY)) + + +class WhichFilesAreTests(unittest.TestCase): + def test_go_typescript_kotlin_and_java_tests_are_scanned(self) -> None: + for path in ["internal/a/b_test.go", "web/src/x.spec.ts", "web/src/y.test.ts", "web/tests/e2e/gate.spec.ts", + "api/src/test/kotlin/FooTest.kt", "lib/src/integrationTest/java/BarTest.java"]: + with self.subTest(path=path): + self.assertTrue(is_test_file(path)) + + def test_documents_and_sources_are_not_scanned(self) -> None: + for path in ["docs/testing-standards.md", "internal/a/b.go", "web/src/x.ts", "README.md"]: + with self.subTest(path=path): + self.assertFalse(is_test_file(path)) + + +def repo(files: dict[str, str]) -> tempfile.TemporaryDirectory: + directory = tempfile.TemporaryDirectory() + root = Path(directory.name) + for path, text in files.items(): + (root / path).parent.mkdir(parents=True, exist_ok=True) + (root / path).write_text(text) + subprocess.run(["git", "init", "-q"], cwd=root, check=True) + subprocess.run(["git", "add", "-A"], cwd=root, check=True) + return directory + + +class CommandLine(unittest.TestCase): + def test_a_repository_with_live_quarantines_passes(self) -> None: + with repo({"a_test.go": f't.Skip("quarantine: {ISSUE} until 2026-10-20")\n'}) as root, redirect_stdout(io.StringIO()) as out: + code = main(["--root", root, "--today", "2026-10-16"]) + self.assertEqual(code, 0) + self.assertIn("1 quarantined test, every one live", out.getvalue()) + + def test_an_expired_quarantine_fails_and_names_where_it_is(self) -> None: + with repo({"web/x.spec.ts": "it('a', () => {})\n" + f"it.skip('b quarantine: {ISSUE} until 2026-10-01', () => {{}})\n"}) as root, redirect_stdout(io.StringIO()) as out: + code = main(["--root", root, "--today", "2026-10-16"]) + self.assertEqual(code, 1) + self.assertIn("::error file=web/x.spec.ts,line=2::quarantine expired on 2026-10-01", out.getvalue()) + + def test_a_marker_outside_test_files_is_not_checked(self) -> None: + with repo({"docs/standards.md": "quarantine: flaky until never\n"}) as root, redirect_stdout(io.StringIO()): + self.assertEqual(main(["--root", root, "--today", "2026-10-16"]), 0) + + def test_a_repository_without_quarantines_passes(self) -> None: + with repo({"a_test.go": "package a\n"}) as root, redirect_stdout(io.StringIO()) as out: + self.assertEqual(main(["--root", root, "--today", "2026-10-16"]), 0) + self.assertIn("No quarantined tests.", out.getvalue()) + + def test_today_defaults_to_the_current_date(self) -> None: + with repo({"a_test.go": "package a\n"}) as root, redirect_stdout(io.StringIO()): + self.assertEqual(main(["--root", root]), 0) + + def test_a_finding_reads_as_a_github_annotation(self) -> None: + self.assertEqual(Finding("a_test.go", 3, "names no issue").annotation(), "::error file=a_test.go,line=3::quarantine names no issue") + + +if __name__ == "__main__": + unittest.main()