From 9948a8dd70dd3bc423559ae78a798b7c791aff2f Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Tue, 6 Oct 2026 13:03:01 -0400 Subject: [PATCH 1/3] Publish a release only after its tests pass push.yaml ran Test and Publish side by side on the release commit, so a release could reach PyPI while its tests were running or after they failed. In the 2026-10-06 13:11Z release run, Publish finished at 13:40:07 and Test at 13:44:04. Publish now has needs: Test. Both jobs share the same if condition, so Publish still runs on every release commit and waits for Test first. Adds a check in test_release_tagging.py that every PyPI-publishing job needs Test and runs under Test's condition. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/push.yaml | 1 + changelog.d/fix-publish-needs-test.fixed.md | 1 + tests/core/test_release_tagging.py | 22 +++++++++++++++++++++ 3 files changed, 24 insertions(+) create mode 100644 changelog.d/fix-publish-needs-test.fixed.md diff --git a/.github/workflows/push.yaml b/.github/workflows/push.yaml index 8605abf3c..3ae5c698b 100644 --- a/.github/workflows/push.yaml +++ b/.github/workflows/push.yaml @@ -84,6 +84,7 @@ jobs: FOLDER: docs/_build/html # The folder the action should deploy. Publish: runs-on: ubuntu-latest + needs: Test if: | (github.repository == 'PolicyEngine/policyengine-core') && (github.event.head_commit.message == 'Update PolicyEngine Core') diff --git a/changelog.d/fix-publish-needs-test.fixed.md b/changelog.d/fix-publish-needs-test.fixed.md new file mode 100644 index 000000000..e37fd7989 --- /dev/null +++ b/changelog.d/fix-publish-needs-test.fixed.md @@ -0,0 +1 @@ +The release workflow now publishes to PyPI only after the Test job passes, instead of running Publish beside Test. diff --git a/tests/core/test_release_tagging.py b/tests/core/test_release_tagging.py index 64b88f3f3..fc7033f53 100644 --- a/tests/core/test_release_tagging.py +++ b/tests/core/test_release_tagging.py @@ -4,6 +4,7 @@ from pathlib import Path import pytest +import yaml ROOT = Path(__file__).resolve().parents[2] @@ -154,3 +155,24 @@ def test_publish_workflow_tags_only_after_pypi_succeeds(): assert "fetch-depth: 0" in publish_job assert publish_job.index(tag_step) > publish_job.index(publish_step) assert "publish-git-tag.sh || true" not in publish_job + + +def test_publish_workflow_publishes_only_after_tests_pass(): + # Without ``needs: Test``, Publish ran beside Test, so a release could + # reach PyPI while its tests were still running or after they failed. + jobs = yaml.safe_load(PUSH_WORKFLOW.read_text())["jobs"] + publishing = [ + name + for name, job in jobs.items() + if any( + "gh-action-pypi-publish" in step.get("uses", "") + for step in job.get("steps", []) + ) + ] + + assert publishing == ["Publish"] + needs = jobs["Publish"].get("needs", []) + assert "Test" in ([needs] if isinstance(needs, str) else needs) + # A job that needs Test is skipped whenever Test is, so both must run on + # the same commits for releases to happen at all. + assert jobs["Publish"]["if"] == jobs["Test"]["if"] From a093ae0bfcd6bab9442dbe3f1fbd805570b61b64 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Tue, 6 Oct 2026 16:59:54 -0400 Subject: [PATCH 2/3] Reject status-function overrides in Publish's condition From the independent review of 9948a8dd (approved, nit): adding always() to both jobs' shared condition kept them equal and passed the check, although it would let Publish run after a failed Test. The check now also requires Publish's condition to contain no status function. Co-Authored-By: Claude Opus 5.5 --- tests/core/test_release_tagging.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/core/test_release_tagging.py b/tests/core/test_release_tagging.py index fc7033f53..c7a67bac0 100644 --- a/tests/core/test_release_tagging.py +++ b/tests/core/test_release_tagging.py @@ -176,3 +176,7 @@ def test_publish_workflow_publishes_only_after_tests_pass(): # A job that needs Test is skipped whenever Test is, so both must run on # the same commits for releases to happen at all. assert jobs["Publish"]["if"] == jobs["Test"]["if"] + # A status function such as always() would let Publish run after a + # failed Test; without one, Publish runs only when Test succeeds. + for function in ("always()", "failure()", "cancelled()", "success()"): + assert function not in jobs["Publish"]["if"] From fef0d21f739dcfeb73a85be6cb05d1f9d72d2c84 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Tue, 6 Oct 2026 18:53:47 -0400 Subject: [PATCH 3/3] Match status functions in Publish's condition case-insensitively From the second independent review (approved, nit): Always() and ALWAYS() passed the check. It now uses a case-insensitive regex; all five override variants are rejected and the current workflow passes. Co-Authored-By: Claude Opus 5.5 --- tests/core/test_release_tagging.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/core/test_release_tagging.py b/tests/core/test_release_tagging.py index c7a67bac0..8ebc66c59 100644 --- a/tests/core/test_release_tagging.py +++ b/tests/core/test_release_tagging.py @@ -1,3 +1,4 @@ +import re import shutil import subprocess import sys @@ -178,5 +179,9 @@ def test_publish_workflow_publishes_only_after_tests_pass(): assert jobs["Publish"]["if"] == jobs["Test"]["if"] # A status function such as always() would let Publish run after a # failed Test; without one, Publish runs only when Test succeeds. - for function in ("always()", "failure()", "cancelled()", "success()"): - assert function not in jobs["Publish"]["if"] + # (Expression function names are matched case-insensitively.) + assert not re.search( + r"\b(always|failure|cancelled|success)\s*\(", + jobs["Publish"]["if"], + re.IGNORECASE, + )