Guard the release tests against masked test failures and other PyPI uploads

.github/actionlint.yaml

@@ -1,11 +1,2 @@
 # actionlint reads this file from .github/ on every run, in CI and locally.
 # Format: https://github.com/rhysd/actionlint/blob/v1.7.12/docs/config.md
-paths:
-  .github/workflows/push.yaml:
-    ignore:
-      # Both come from the Test job's "Deploy documentation" step, which
-      # PolicyEngine/policyengine-core#594 moves into its own Docs job and
-      # fixes. Delete these two entries in whichever PR fixes or removes that
-      # step.
-      - 'property "os" is not defined in object type \{\}'
-      - 'input "GITHUB_TOKEN" is not defined in action "JamesIves/github-pages-deploy-action@v4"'

changelog.d/core591-release-guards.changed.md

@@ -0,0 +1 @@
+The release tests now fail if `push.yaml`'s Test job or one of its steps sets `continue-on-error`, or if its `make test` step has an `if:` condition or runs more than that command. They also fail if any job other than `push.yaml`'s Publish uploads to PyPI with `pypa/gh-action-pypi-publish`, `twine upload` or `uv publish`. The actionlint config drops two ignores for a step that no longer exists.

tests/core/test_release_tagging.py

@@ -339,3 +339,114 @@ def test_publish_workflow_publishes_only_after_tests_pass():
         jobs["Publish"]["if"],
         re.IGNORECASE,
     )
+
+
+WORKFLOWS = ROOT / ".github" / "workflows"
+PYPI_UPLOAD_COMMAND = re.compile(r"\b(twine\s+upload|uv\s+publish)\b")
+
+
+def uploads_to_pypi(step: dict) -> bool:
+    return step.get("uses", "").startswith("pypa/gh-action-pypi-publish@") or bool(
+        PYPI_UPLOAD_COMMAND.search(step.get("run", ""))
+    )
+
+
+def pypi_publishing_jobs(workflows: Path) -> list[tuple[str, str]]:
+    return [
+        (path.name, name)
+        for path in sorted([*workflows.glob("*.yaml"), *workflows.glob("*.yml")])
+        for name, job in (yaml.safe_load(path.read_text()).get("jobs") or {}).items()
+        if any(uploads_to_pypi(step) for step in job.get("steps", []))
+    ]
+
+
+def test_only_push_publish_job_uploads_to_pypi():
+    # The checks above cover push.yaml's Publish job alone, so an upload in
+    # any other job or workflow file would reach PyPI without them.
+    assert pypi_publishing_jobs(WORKFLOWS) == [("push.yaml", "Publish")]
+
+
+@pytest.mark.parametrize(
+    "upload",
+    [
+        {"uses": "pypa/gh-action-pypi-publish@release/v1"},
+        {"run": "python -m twine upload dist/*"},
+        {"run": "make build\nuv publish"},
+    ],
+    ids=["pypi-publish-action", "twine-upload", "uv-publish"],
+)
+def test_pypi_upload_check_finds_upload_in_another_workflow(tmp_path, upload):
+    for path in WORKFLOWS.iterdir():
+        shutil.copy(path, tmp_path / path.name)
+    pr_workflow = tmp_path / "pr.yaml"
+    workflow = yaml.safe_load(pr_workflow.read_text())
+    workflow["jobs"]["Test"]["steps"].append(upload)
+    pr_workflow.write_text(yaml.safe_dump(workflow))
+
+    assert pypi_publishing_jobs(tmp_path) == [
+        ("pr.yaml", "Test"),
+        ("push.yaml", "Publish"),
+    ]
+
+
+def assert_test_job_fails_when_tests_fail(test_job: dict) -> None:
+    steps = test_job["steps"]
+    # continue-on-error on Test or one of its steps lets the job succeed
+    # after a step fails, and Publish would then run.
+    assert not any("continue-on-error" in item for item in [test_job, *steps]), (
+        "Test continues on error"
+    )
+    test_step = steps[
+        only_step(steps, lambda step: "make test" in step.get("run", ""), "test")
+    ]
+    # A skipped step does not fail the job, so a condition on the step that
+    # runs the tests could let Test succeed without running them.
+    assert "if" not in test_step, "test step has a condition"
+    # Anything after the command, such as `|| true`, could mask its failure.
+    assert test_step["run"].strip() == "make test", "test step runs more than make test"
+
+
+def test_test_job_cannot_pass_with_failing_tests():
+    test_job = yaml.safe_load(PUSH_WORKFLOW.read_text())["jobs"]["Test"]
+
+    assert_test_job_fails_when_tests_fail(test_job)
+
+
+def find_test_step(test_job: dict) -> dict:
+    return next(step for step in test_job["steps"] if step.get("run") == "make test")
+
+
+def continue_test_job_on_error(test_job: dict) -> None:
+    test_job["continue-on-error"] = True
+
+
+def continue_after_test_failure(test_job: dict) -> None:
+    find_test_step(test_job)["continue-on-error"] = True
+
+
+def ignore_test_failure(test_job: dict) -> None:
+    find_test_step(test_job)["run"] += " || true"
+
+
+def skip_tests(test_job: dict) -> None:
+    find_test_step(test_job)["if"] = False
+
+
+@pytest.mark.parametrize(
+    ("mutate", "violation"),
+    [
+        pytest.param(mutate, violation, id=mutate.__name__)
+        for mutate, violation in [
+            (continue_test_job_on_error, "Test continues on error"),
+            (continue_after_test_failure, "Test continues on error"),
+            (ignore_test_failure, "test step runs more than make test"),
+            (skip_tests, "test step has a condition"),
+        ]
+    ],
+)
+def test_test_job_check_rejects_masked_test_failure(mutate, violation):
+    test_job = yaml.safe_load(PUSH_WORKFLOW.read_text())["jobs"]["Test"]
+    mutate(test_job)
+
+    with pytest.raises(AssertionError, match=violation):
+        assert_test_job_fails_when_tests_fail(test_job)