diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 031f12a..a60a64a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -23,8 +23,20 @@ jobs: with: python-version: "3.12" - - name: Install patchrail and pytest - run: python -m pip install --upgrade patchrail pytest pyyaml + # Test the range the action actually installs, read straight out of + # action.yml so the two cannot drift. Installing the newest patchrail here + # instead would tie this repo's release train to upstream: a breaking + # release would turn this job red and, through `sync-v1`, stop merged fixes + # from ever reaching `@v1` users. `latest-patchrail` is where the newest + # release gets tested, and it is deliberately not a gate on the tag. + - name: Install the patchrail range the action ships, plus pytest + shell: bash + run: | + set -euo pipefail + spec="$(grep -o 'patchrail>=[^"]*' action.yml)" + test -n "${spec}" + echo "action.yml installs: ${spec}" + python -m pip install --upgrade "${spec}" pytest pyyaml - name: Check the guide slugs, the README snippets and the no-log path run: python -m pytest -q @@ -46,11 +58,13 @@ jobs: set -euo pipefail echo "failure-class=${{ steps.triage.outputs.failure-class }}" echo "guide-url=${{ steps.triage.outputs.guide-url }}" - test -n "${{ steps.triage.outputs.failure-class }}" - case "${{ steps.triage.outputs.guide-url }}" in - https://getpatchrail.com/fix*) echo "OK" ;; - *) echo "unexpected guide-url"; exit 1 ;; - esac + # Assert the class the sample log actually is, and the guide for it. + # A `test -n` on the class and a `fix*` glob on the URL both pass on a + # degraded `unknown` + bare-index result, which is exactly the output a + # contract drift produces: the assertion has to be able to see it. + test "${{ steps.triage.outputs.failure-class }}" = "python_test_failure" + test "${{ steps.triage.outputs.guide-url }}" = "https://getpatchrail.com/fix/python-test-failure" + echo "OK" # A capture that drops stderr leaves an empty log, and a step that dies early # leaves no log at all. Both land on an already-red run, so the action must @@ -88,6 +102,41 @@ jobs: test "${{ steps.missing.outputs.guide-url }}" = "https://getpatchrail.com/fix" echo "OK: no classification, no extra failure" + # The action installs a bounded patchrail range, so a breaking release cannot + # reach consumers on its own. It must not reach them by surprise either: this + # runs the real explain -> annotate path against the NEWEST patchrail on PyPI, + # on every push and weekly. When it goes red, a new release changed the + # contract, and the range in action.yml gets widened deliberately (or the + # reader gets fixed) instead of users discovering it in their own red builds. + latest-patchrail: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + + - name: Install the newest patchrail from PyPI + run: python -m pip install --upgrade patchrail + + - name: Explain the sample failure and annotate, as the action does + shell: bash + run: | + set -euo pipefail + patchrail --version + patchrail ci explain --log examples/sample-failure.log \ + --format json --out result.json + python scripts/annotate.py result.json | tee annotation.txt + + if grep -q "No classification" annotation.txt; then + echo "::error::the newest patchrail no longer emits the ci-result schema this action reads; update scripts/annotate.py and the range in action.yml" + exit 1 + fi + grep -q "python_test_failure" annotation.txt + grep -q "fix/python-test-failure" annotation.txt + echo "OK: the newest patchrail still matches the contract" + # Everyone uses `patchrail/ci-triage-action@v1`, but the jobs above test the # commit, not the tag. Without this the two drift apart in silence: a merged # fix stays invisible to every user while CI keeps reporting green. Move the diff --git a/action.yml b/action.yml index 8f0400b..fd37fc7 100644 --- a/action.yml +++ b/action.yml @@ -19,7 +19,7 @@ inputs: required: false default: 'true' patchrail-version: - description: 'Pin a specific patchrail version from PyPI (defaults to latest).' + description: 'Pin an exact patchrail version from PyPI. Defaults to the range this action is tested against.' required: false default: '' python-version: @@ -52,7 +52,12 @@ runs: if [ -n "${{ inputs.patchrail-version }}" ]; then python -m pip install --quiet "patchrail==${{ inputs.patchrail-version }}" else - python -m pip install --quiet patchrail + # Bounded on purpose. patchrail ships breaking JSON contract changes in + # minor bumps, so an unpinned install would put the next one straight + # into every consumer's CI with no commit here to review it. The + # `latest-patchrail` job in .github/workflows/test.yml runs this same + # path against the newest release, so the range moves deliberately. + python -m pip install --quiet "patchrail>=0.3.1,<0.5.0" fi - name: Explain failure and annotate diff --git a/scripts/annotate.py b/scripts/annotate.py index 9a1a1b4..dba82dd 100644 --- a/scripts/annotate.py +++ b/scripts/annotate.py @@ -13,6 +13,13 @@ FIX_GUIDE_BASE = "https://getpatchrail.com/fix" +# The ci-result contract this action knows how to read. patchrail ships breaking +# JSON contract changes in minor bumps (0.4.0 moved `ci classes` to schema v2), +# and every field below is read with `.get()`, so a renamed key would not raise: +# it would quietly annotate `unknown (confidence None)` on a run that is already +# red. Refuse a schema we do not know instead of inventing a classification. +RESULT_SCHEMA = "patchrail.ci_result.v1" + # Shown whenever there is no log to classify. Both halves matter: without # `2>&1` a tool that reports only on stderr leaves an empty log, and without # pipefail (`shell: bash`) a failing command piped into `tee` exits 0, so the @@ -106,6 +113,38 @@ def unclassified(reason: str) -> int: return 0 +def incompatible_schema(found: str) -> int: + """Report that the installed patchrail speaks a contract this action cannot read. + + Same contract as `unclassified`: one annotation line, empty outputs, the guide + index, exit 0. Naming both versions is the point — the alternative is an + `unknown (confidence None)` annotation that looks like a real classification + and tells the user nothing about why their pinned action stopped working. + """ + found = " ".join(str(found or "").split()) or "none" + reason = ( + f"the installed patchrail emits ci-result schema '{found}', " + f"and this action reads '{RESULT_SCHEMA}'. Pin a compatible release with " + f"the `patchrail-version` input, or upgrade patchrail-ci-triage." + ) + print(f"::warning title=PatchRail CI Triage::No classification: {reason}") + write_kv( + "GITHUB_STEP_SUMMARY", + [ + "## PatchRail CI Triage", + "", + f"- **No classification:** {reason}", + "", + "_Classified locally. No pull request, comment or external call was made._", + ], + ) + write_kv( + "GITHUB_OUTPUT", + ["failure-class=", "confidence=", f"guide-url={FIX_GUIDE_BASE}"], + ) + return 0 + + def main() -> int: argv = sys.argv[1:] if argv and argv[0] == "--unclassified": @@ -115,6 +154,9 @@ def main() -> int: with open(result_path, encoding="utf-8") as handle: result = json.load(handle) + if result.get("schema_version") != RESULT_SCHEMA: + return incompatible_schema(result.get("schema_version")) + failure_class = str(result.get("failure_class") or "unknown") confidence = result.get("confidence") subsystem = result.get("likely_subsystem") or "unknown" diff --git a/tests/test_schema_guard.py b/tests/test_schema_guard.py new file mode 100644 index 0000000..2dfe134 --- /dev/null +++ b/tests/test_schema_guard.py @@ -0,0 +1,124 @@ +"""The action must not invent a classification when patchrail changes its contract. + +`annotate.py` reads every field of the ci-result with `.get()`, so a renamed key +never raises. Before the guard, a patchrail release that bumped the ci-result +schema turned every consumer's annotation into `unknown (confidence None)` linking +to the bare guide index — on a run that is already red, under a `@v1` tag the user +pinned precisely so nothing would move under them. Worse, it was silent: the smoke +assertions (`test -n` on the class, a `fix*` glob on the URL) passed on that output. + +patchrail ships breaking JSON contract changes in minor bumps (0.4.0 moved +`ci classes` to schema v2), so this is a question of when, not if. +""" +from __future__ import annotations + +import json +import subprocess +import sys +from pathlib import Path + +import pytest + +ANNOTATE = Path(__file__).resolve().parent.parent / "scripts" / "annotate.py" +GUIDE_INDEX = "https://getpatchrail.com/fix" + +# Kept literal on purpose: importing the constant from annotate.py would make this +# test agree with any future edit to it, which is the drift it exists to catch. +SUPPORTED_SCHEMA = "patchrail.ci_result.v1" + +# What patchrail emits today (verified against 0.3.1 and 0.4.0). +V1_RESULT = { + "schema_version": SUPPORTED_SCHEMA, + "failure_class": "python_test_failure", + "confidence": 0.89, + "likely_subsystem": "Python tests", + "reproduction_command": "python -m pytest -q", + "minimal_repair_strategy": "Reproduce the failing test and patch the drift.", +} + +# A future patchrail that regroups the classification fields, the way 0.4.0 did to +# `ci classes`. Every key annotate.py reads is gone, and none of them raise. +V2_RESULT = { + "schema_version": "patchrail.ci_result.v2", + "classification": {"class": "python_test_failure", "confidence": 0.89}, + "likely_subsystem": "Python tests", + "reproduction_command": "python -m pytest -q", +} + + +@pytest.fixture() +def annotate(tmp_path): + def run(result: dict) -> tuple[subprocess.CompletedProcess, str, str]: + result_path = tmp_path / "patchrail-ci-result.json" + result_path.write_text(json.dumps(result), encoding="utf-8") + output = tmp_path / "output" + summary = tmp_path / "summary" + output.touch() + summary.touch() + proc = subprocess.run( + [sys.executable, str(ANNOTATE), str(result_path)], + capture_output=True, + text=True, + env={ + "PATH": "/usr/bin:/bin", + "GITHUB_OUTPUT": str(output), + "GITHUB_STEP_SUMMARY": str(summary), + }, + ) + return proc, output.read_text(), summary.read_text() + + return run + + +def test_the_current_schema_is_annotated_normally(annotate) -> None: + """The guard must not cost anything on the contract patchrail ships today.""" + proc, output, summary = annotate(V1_RESULT) + assert proc.returncode == 0, proc.stderr + assert "python_test_failure (confidence 0.89)" in proc.stdout + assert f"guide-url={GUIDE_INDEX}/python-test-failure" in output + assert "**Root cause:** `python_test_failure`" in summary + + +def test_an_unreadable_schema_is_never_dressed_up_as_a_classification(annotate) -> None: + """The bug: `unknown (confidence None)` looks like triage and says nothing.""" + proc, output, _ = annotate(V2_RESULT) + assert "unknown (confidence None)" not in proc.stdout + assert "failure-class=unknown" not in output + assert "confidence=None" not in output + + +def test_an_unreadable_schema_names_both_versions_and_the_way_out(annotate) -> None: + proc, _, summary = annotate(V2_RESULT) + assert "patchrail.ci_result.v2" in proc.stdout # what the runner installed + assert SUPPORTED_SCHEMA in proc.stdout # what this action reads + assert "patchrail-version" in proc.stdout # how to get unstuck today + assert "patchrail.ci_result.v2" in summary + + +def test_an_unreadable_schema_leaves_the_outputs_empty(annotate) -> None: + """Downstream steps must be able to tell "no classification" from a real one.""" + _, output, _ = annotate(V2_RESULT) + lines = output.strip().splitlines() + assert "failure-class=" in lines + assert "confidence=" in lines + assert f"guide-url={GUIDE_INDEX}" in lines + + +def test_a_result_without_a_schema_version_is_not_trusted(annotate) -> None: + """Covers a pre-schema patchrail, and any JSON that is not a ci-result at all.""" + proc, output, _ = annotate({"failure_class": "python_test_failure", "confidence": 0.89}) + assert "No classification" in proc.stdout + assert "failure-class=python_test_failure" not in output + + +def test_an_unreadable_schema_still_does_not_fail_the_job(annotate) -> None: + """It runs under `if: failure()`: a second red step buries the real failure.""" + proc, _, _ = annotate(V2_RESULT) + assert proc.returncode == 0, proc.stderr + + +def test_the_annotation_stays_on_one_line(annotate) -> None: + """GitHub reads one annotation per line; a wrapped message would be truncated.""" + proc, _, _ = annotate(V2_RESULT) + warnings = [ln for ln in proc.stdout.splitlines() if ln.startswith("::warning")] + assert len(warnings) == 1