Repository navigation
Guard the release tests against masked test failures and other PyPI uploads - #648
Conversation
…job uploads to PyPI Publish waits on Test, so continue-on-error on Test or its steps, a condition on the test step, or anything such as || true after make test would let a release whose tests failed reach PyPI. The publishing check also read push.yaml alone, so an upload in any other workflow file would have bypassed it. Follow-ups to the review of #591 (P3 1 and 6). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwQq6Jzt9cKvgaMoYvZDfG
#594 moved that step out of Test into the Docs job, so both ignored messages no longer occur; keeping them would hide them if they came back. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LwQq6Jzt9cKvgaMoYvZDfG
|
Ready for hub review Before pushing, I checked that neither Items1. Test can't pass after its tests fail (review P3 1): FIXED
2. Every workflow file is checked for PyPI uploads (review P3 6): FIXED
3. Dead actionlint ignores (review section 8): FIXED
Changelog: Tests (each file in its own process)
MutantsI made each change on the working copy of the workflow, ran both test files in separate processes, and restored the file from a saved copy afterwards.
The extra failures in M6–M9 come from the in-suite mutant tests. They load the already-mutated file, so either the check fires before the expected message or CIThis is a draft, so CI hasn't run. No runs exist at the head. Judgment calls
Proposed follow-up
Generated by Claude Code |
|
Hub review in progress Head
Generated by Claude Code |
|
Ready for hub review I reviewed head The PR does all three items in the build brief. The new Test-job guard kills every masking mutant the brief named (M6–M9, M11–M13). The new PyPI scan kills M10 and M14. Deleting any one of the helper's three checks fails the in-suite mutant test aimed at it. actionlint 1.7.12 reports 0 errors with the comment-only config, and CI is green (see CI). I found nothing at P1 or P2. All five findings are P3, and none blocks the merge. On the assignment: the PR leaves out nothing the build brief asked for. It differs from the brief in two places, both disclosed in the hand-off and neither of any consequence:
P31. The PyPI scan, its test name, the PR body and the changelog claim more than the code checks (
2. The hand-off says the scan misses reusable workflows. That holds only for remote ones (hand-off "Judgment calls" and "Proposed follow-up").
3.
4. The exact-match check is the right strictness, but its message doesn't say what to do (
5. Masks the guard still misses (none exists today, and no change is needed in this PR).
The rule (section 1)The proxy blocks docs.github.com from this session, both through curl and the fetch tool. I read the same pages' source from the
What follows for
None of the misses exists in this repository today. Checked and sound
What I couldn't check
Generated by Claude Code |
The fragment said the release tests fail if any workflow other than Publish uploads to PyPI, but the check recognizes only pypa/gh-action-pypi-publish, twine upload and uv publish, so a step that runs poetry publish passes it (review P3 1). The fragment now names the three upload forms and the three Test-job checks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
US + core hub merge audit, core#648 at
🤖 Generated with Claude Code |
* Extend the release tests' PyPI scan and Test-job guard (core#648 review) The independent review of #648 found uploads the scan missed (other publishers, Makefile recipes, remote reusable workflows, composite actions), a crash on a subdirectory under .github/workflows, a test-step locator that also matched make test-country-template, and masks the Test-job guard missed (a custom shell, PYTEST_ADDOPTS, a - prefix on a make test recipe line). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Restore uv.lock, which a local uv sync changed This PR changes only the release tests and its changelog fragment. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Follow-ups from the independent review of #591 (#591 (comment)): P3 1, P3 6 and the actionlint note in section 8. No file under
policyengine_core/or.github/workflows/changes.Changes
test_test_job_cannot_pass_with_failing_testsfails ifpush.yaml's Test job or any of its steps hascontinue-on-error, if the step that runsmake testhas anif:, or if that step runs anything besidesmake test(for example|| true). Publishneeds: Test, so any of these would let a release whose tests failed reach PyPI. A parametrized test checks that the guard rejects each of the four mutations. The guard misses ashell:on the test step that ignores its script, a-prefix on the Makefile's test recipe andPYTEST_ADDOPTS: --collect-onlyon the job (review P3 5).test_only_push_publish_job_uploads_to_pypireads every*.yaml/*.ymlin.github/workflows/and collects each job with a step that usespypa/gh-action-pypi-publish@, or runstwine uploadoruv publish. It asserts that no job butpush.yaml'sPublishuploads. A parametrized test adds each upload form to a copy ofpr.yamland checks that the scan finds it. Other publishers (poetry,flit,hatch,pdmandmaturin publish), an upload inside amaketarget and a reusable workflow called from another repository pass the scan (review P3 1 and 2)..github/actionlint.yamldrops both ignores for the old "Deploy documentation" step, which Deploy the documentation site from its own release job #594 moved into the Docs job, and drops the now-emptypaths:key. The header comment stays.Tests
uv run pytest -q -p no:cacheprovider tests/core/test_release_tagging.py: 16 passed before, 25 passed after.tests/core/test_docs_deployment.py: 5 passed before and after.push.yaml/pr.yaml, restored after each one: M6 (jobcontinue-on-error), M7 (stepcontinue-on-error), M8 (make test || true), M9 (if: falseon the test step) and M10 (a PyPI upload step inpr.yaml) each fail the new guard.Independent review: #648 (comment)
🤖 Generated with Claude Code