From a54b770d4edfd39eccd23fe2f9224e872ec94b89 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Tue, 6 Oct 2026 16:04:27 -0400 Subject: [PATCH 1/3] Deploy the documentation site from its own release job The Test job lost its OS matrix in #282 (2024-09-26) but kept the deploy step's `if: matrix.os == 'ubuntu-latest'` guard, so the step has been skipped on every release since and gh-pages last moved on 2024-09-26. Move the docs build and deploy into a Docs job that needs Test, has `contents: write`, and uses the v4 action's documented inputs. Keeping it out of Test means a failed deploy cannot hold back a PyPI release once Publish needs Test (#591). Add sphinx.ext.githubpages so the build writes the .nojekyll that a branch-sourced Pages site needs. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/push.yaml | 26 +++++- changelog.d/fix-docs-deploy.changed.md | 1 + docs/_config.yml | 1 + tests/core/test_docs_deployment.py | 123 +++++++++++++++++++++++++ 4 files changed, 147 insertions(+), 4 deletions(-) create mode 100644 changelog.d/fix-docs-deploy.changed.md create mode 100644 tests/core/test_docs_deployment.py diff --git a/.github/workflows/push.yaml b/.github/workflows/push.yaml index 8605abf3c..d0cbc2793 100644 --- a/.github/workflows/push.yaml +++ b/.github/workflows/push.yaml @@ -73,15 +73,33 @@ jobs: - name: Run tests run: make test - uses: codecov/codecov-action@v6 + Docs: + runs-on: ubuntu-latest + # A separate job, so a failed deploy cannot hold back a PyPI release. + needs: Test + if: | + (github.repository == 'PolicyEngine/policyengine-core') + && (github.event.head_commit.message == 'Update PolicyEngine Core') + permissions: + contents: write + steps: + - name: Checkout repo + uses: actions/checkout@v6 + - name: Set up Python + uses: actions/setup-python@v6 + with: + python-version: "3.14" + - name: Install uv + uses: astral-sh/setup-uv@v8.1.0 + - name: Install package + run: make install - name: Generate documentation run: make documentation - name: Deploy documentation - if: matrix.os == 'ubuntu-latest' uses: JamesIves/github-pages-deploy-action@v4 with: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - BRANCH: gh-pages # The branch the action should deploy to. - FOLDER: docs/_build/html # The folder the action should deploy. + branch: gh-pages + folder: docs/_build/html Publish: runs-on: ubuntu-latest if: | diff --git a/changelog.d/fix-docs-deploy.changed.md b/changelog.d/fix-docs-deploy.changed.md new file mode 100644 index 000000000..ba3798178 --- /dev/null +++ b/changelog.d/fix-docs-deploy.changed.md @@ -0,0 +1 @@ +The release workflow deploys the documentation site again, from a Docs job that runs after the tests pass; the deploy step had been skipped on every release since September 2024 because it checked a matrix the Test job no longer has. diff --git a/docs/_config.yml b/docs/_config.yml index 4bec1e9dd..e7f504b0e 100644 --- a/docs/_config.yml +++ b/docs/_config.yml @@ -20,6 +20,7 @@ sphinx: html_css_files: - style.css extra_extensions: + - "sphinx.ext.githubpages" - "sphinx.ext.autodoc" - "sphinxarg.ext" - "sphinx.ext.viewcode" diff --git a/tests/core/test_docs_deployment.py b/tests/core/test_docs_deployment.py new file mode 100644 index 000000000..f2fe34f10 --- /dev/null +++ b/tests/core/test_docs_deployment.py @@ -0,0 +1,123 @@ +import re +from pathlib import Path + +import pytest +import yaml + + +ROOT = Path(__file__).resolve().parents[2] +WORKFLOWS = ROOT / ".github" / "workflows" +PUSH_WORKFLOW = WORKFLOWS / "push.yaml" +DOCS_CONFIG = ROOT / "docs" / "_config.yml" +MAKEFILE = ROOT / "Makefile" +DEPLOY_ACTION = "JamesIves/github-pages-deploy-action@" +PYPI_ACTION = "pypa/gh-action-pypi-publish@" + + +def load_jobs(workflow: Path) -> dict: + return yaml.safe_load(workflow.read_text())["jobs"] + + +def as_list(value) -> list: + if value is None: + return [] + return [value] if isinstance(value, str) else list(value) + + +def strings_in(node): + if isinstance(node, str): + yield node + elif isinstance(node, dict): + for value in node.values(): + yield from strings_in(value) + elif isinstance(node, list): + for item in node: + yield from strings_in(item) + + +def matrix_keys(job: dict) -> set: + matrix = job.get("strategy", {}).get("matrix", {}) + keys = set(matrix) - {"include", "exclude"} + for entry in matrix.get("include", []): + keys |= set(entry) + return keys + + +def jobs_using(jobs: dict, action: str) -> list: + return [ + name + for name, job in jobs.items() + if any(step.get("uses", "").startswith(action) for step in job.get("steps", [])) + ] + + +def upstream_jobs(jobs: dict, name: str) -> set: + seen = set() + pending = as_list(jobs[name].get("needs")) + while pending: + needed = pending.pop() + if needed not in seen: + seen.add(needed) + pending.extend(as_list(jobs[needed].get("needs"))) + return seen + + +@pytest.mark.parametrize( + "workflow", sorted(WORKFLOWS.glob("*.y*ml")), ids=lambda path: path.name +) +def test_workflow_matrix_references_have_a_matrix(workflow: Path): + # The Test job lost its matrix in September 2024 but kept a step guarded + # by ``matrix.os == 'ubuntu-latest'``, so that condition never held and + # the documentation deploy was skipped on every release afterwards. + for name, job in load_jobs(workflow).items(): + referenced = { + key + for text in strings_in(job) + for key in re.findall(r"\bmatrix\.([A-Za-z_][\w-]*)", text) + } + missing = referenced - matrix_keys(job) + assert not missing, ( + f"{workflow.name} job {name} references matrix keys " + f"{sorted(missing)} that its strategy does not define" + ) + + +def test_release_deploys_documentation_after_tests_pass(): + jobs = load_jobs(PUSH_WORKFLOW) + assert jobs_using(jobs, DEPLOY_ACTION) == ["Docs"] + docs = jobs["Docs"] + steps = docs["steps"] + deploy = next( + step for step in steps if step.get("uses", "").startswith(DEPLOY_ACTION) + ) + build = next(step for step in steps if step.get("run") == "make documentation") + + assert docs["if"] == jobs["Test"]["if"] + assert "Test" in as_list(docs.get("needs")) + assert "strategy" not in docs + assert all("if" not in step for step in steps) + assert steps.index(build) < steps.index(deploy) + # GitHub Pages serves the gh-pages branch root. The action's ``token`` + # input defaults to the job token, which needs write access to push. + assert deploy["with"] == {"branch": "gh-pages", "folder": "docs/_build/html"} + assert docs["permissions"] == {"contents": "write"} + + +def test_documentation_build_matches_deploy_folder(): + recipe = MAKEFILE.read_text().split("documentation:\n", maxsplit=1)[1] + recipe = recipe.split("\n\n", maxsplit=1)[0] + extensions = yaml.safe_load(DOCS_CONFIG.read_text())["sphinx"]["extra_extensions"] + + # ``jb build docs`` writes HTML to docs/_build/html, the deployed folder. + assert "jb build docs" in recipe + # Pages builds a branch source with Jekyll unless its root has .nojekyll, + # which this extension writes into the HTML output. + assert "sphinx.ext.githubpages" in extensions + + +def test_documentation_deploy_cannot_block_pypi_publishing(): + jobs = load_jobs(PUSH_WORKFLOW) + + for name in jobs_using(jobs, PYPI_ACTION): + assert name != "Docs" + assert "Docs" not in upstream_jobs(jobs, name) From 0870347f040b2d190c3bcb8e29079104b3586be7 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Tue, 6 Oct 2026 16:31:43 -0400 Subject: [PATCH 2/3] Deploy docs only after the release reaches PyPI Review follow-ups on the Docs job: - Need Publish as well as Test, so the site never documents a version that failed to reach PyPI. Publish still does not depend on Docs. - Serialize deploys with a gh-pages-deploy concurrency group; a newer waiting deploy replaces an older one instead of racing it. - Loosen the structural tests to the properties that matter (branch, folder, contents: write, an unconditional deploy step), parse the Makefile recipe by its tab-indented lines, and skip jobs whose matrix is built by an expression. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/push.yaml | 9 ++++-- changelog.d/fix-docs-deploy.changed.md | 2 +- tests/core/test_docs_deployment.py | 39 ++++++++++++++++++-------- 3 files changed, 35 insertions(+), 15 deletions(-) diff --git a/.github/workflows/push.yaml b/.github/workflows/push.yaml index d0cbc2793..529b3c135 100644 --- a/.github/workflows/push.yaml +++ b/.github/workflows/push.yaml @@ -75,13 +75,18 @@ jobs: - uses: codecov/codecov-action@v6 Docs: runs-on: ubuntu-latest - # A separate job, so a failed deploy cannot hold back a PyPI release. - needs: Test + # Deploy only a release that reached PyPI. As its own job, a failed + # deploy cannot fail Test or Publish. + needs: [Test, Publish] if: | (github.repository == 'PolicyEngine/policyengine-core') && (github.event.head_commit.message == 'Update PolicyEngine Core') permissions: contents: write + # One deploy at a time; a newer release replaces an older waiting one. + concurrency: + group: gh-pages-deploy + cancel-in-progress: false steps: - name: Checkout repo uses: actions/checkout@v6 diff --git a/changelog.d/fix-docs-deploy.changed.md b/changelog.d/fix-docs-deploy.changed.md index ba3798178..25cff6928 100644 --- a/changelog.d/fix-docs-deploy.changed.md +++ b/changelog.d/fix-docs-deploy.changed.md @@ -1 +1 @@ -The release workflow deploys the documentation site again, from a Docs job that runs after the tests pass; the deploy step had been skipped on every release since September 2024 because it checked a matrix the Test job no longer has. +The release workflow deploys the documentation site again, from a Docs job that runs once the tests pass and the release reaches PyPI; the deploy step had been skipped on every release since September 2024 because it checked a matrix the Test job no longer has. diff --git a/tests/core/test_docs_deployment.py b/tests/core/test_docs_deployment.py index f2fe34f10..639246363 100644 --- a/tests/core/test_docs_deployment.py +++ b/tests/core/test_docs_deployment.py @@ -35,10 +35,18 @@ def strings_in(node): yield from strings_in(item) -def matrix_keys(job: dict) -> set: +def matrix_keys(job: dict) -> set | None: + """Keys a job's matrix defines, or None when an expression builds it.""" matrix = job.get("strategy", {}).get("matrix", {}) + if not isinstance(matrix, dict): + return None keys = set(matrix) - {"include", "exclude"} - for entry in matrix.get("include", []): + include = matrix.get("include", []) + if not isinstance(include, list): + return None + for entry in include: + if not isinstance(entry, dict): + return None keys |= set(entry) return keys @@ -70,19 +78,22 @@ def test_workflow_matrix_references_have_a_matrix(workflow: Path): # by ``matrix.os == 'ubuntu-latest'``, so that condition never held and # the documentation deploy was skipped on every release afterwards. for name, job in load_jobs(workflow).items(): + defined = matrix_keys(job) + if defined is None: + continue referenced = { key for text in strings_in(job) for key in re.findall(r"\bmatrix\.([A-Za-z_][\w-]*)", text) } - missing = referenced - matrix_keys(job) + missing = referenced - defined assert not missing, ( f"{workflow.name} job {name} references matrix keys " f"{sorted(missing)} that its strategy does not define" ) -def test_release_deploys_documentation_after_tests_pass(): +def test_release_deploys_documentation_after_publishing(): jobs = load_jobs(PUSH_WORKFLOW) assert jobs_using(jobs, DEPLOY_ACTION) == ["Docs"] docs = jobs["Docs"] @@ -93,23 +104,27 @@ def test_release_deploys_documentation_after_tests_pass(): build = next(step for step in steps if step.get("run") == "make documentation") assert docs["if"] == jobs["Test"]["if"] - assert "Test" in as_list(docs.get("needs")) - assert "strategy" not in docs - assert all("if" not in step for step in steps) + assert {"Test", "Publish"} <= set(as_list(docs.get("needs"))) + # A condition on the deploy step is how it went unnoticed for two years. + assert "if" not in deploy assert steps.index(build) < steps.index(deploy) # GitHub Pages serves the gh-pages branch root. The action's ``token`` # input defaults to the job token, which needs write access to push. - assert deploy["with"] == {"branch": "gh-pages", "folder": "docs/_build/html"} - assert docs["permissions"] == {"contents": "write"} + assert deploy["with"]["branch"] == "gh-pages" + assert deploy["with"]["folder"] == "docs/_build/html" + assert docs["permissions"].get("contents") == "write" + assert docs["concurrency"]["cancel-in-progress"] is False def test_documentation_build_matches_deploy_folder(): - recipe = MAKEFILE.read_text().split("documentation:\n", maxsplit=1)[1] - recipe = recipe.split("\n\n", maxsplit=1)[0] + recipe = re.search( + r"^documentation:.*\n((?:\t.*\n?)+)", MAKEFILE.read_text(), re.MULTILINE + ) extensions = yaml.safe_load(DOCS_CONFIG.read_text())["sphinx"]["extra_extensions"] # ``jb build docs`` writes HTML to docs/_build/html, the deployed folder. - assert "jb build docs" in recipe + assert recipe is not None + assert "jb build docs" in recipe.group(1) # Pages builds a branch source with Jekyll unless its root has .nojekyll, # which this extension writes into the HTML output. assert "sphinx.ext.githubpages" in extensions From d0b438765d812c89fb35d8d9012bcbc71db85a1e Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Tue, 6 Oct 2026 16:43:11 -0400 Subject: [PATCH 3/3] Tighten docs deploy checks from the delta review - Say what the concurrency group guarantees: one deploy at a time, a newly queued deploy replacing one still waiting. GitHub does not promise release order. - Require a constant concurrency group (a per-run group would not serialize anything) and accept the string shorthand. - Match `jb build docs` as a whole recipe line, so a changed output path or a commented-out line no longer passes. - Treat `strategy: null` like no strategy. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/push.yaml | 2 +- tests/core/test_docs_deployment.py | 11 ++++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/.github/workflows/push.yaml b/.github/workflows/push.yaml index 529b3c135..65549fc51 100644 --- a/.github/workflows/push.yaml +++ b/.github/workflows/push.yaml @@ -83,7 +83,7 @@ jobs: && (github.event.head_commit.message == 'Update PolicyEngine Core') permissions: contents: write - # One deploy at a time; a newer release replaces an older waiting one. + # One deploy at a time; a newly queued deploy replaces one still waiting. concurrency: group: gh-pages-deploy cancel-in-progress: false diff --git a/tests/core/test_docs_deployment.py b/tests/core/test_docs_deployment.py index 639246363..20f4a3706 100644 --- a/tests/core/test_docs_deployment.py +++ b/tests/core/test_docs_deployment.py @@ -37,7 +37,7 @@ def strings_in(node): def matrix_keys(job: dict) -> set | None: """Keys a job's matrix defines, or None when an expression builds it.""" - matrix = job.get("strategy", {}).get("matrix", {}) + matrix = (job.get("strategy") or {}).get("matrix") or {} if not isinstance(matrix, dict): return None keys = set(matrix) - {"include", "exclude"} @@ -113,7 +113,12 @@ def test_release_deploys_documentation_after_publishing(): assert deploy["with"]["branch"] == "gh-pages" assert deploy["with"]["folder"] == "docs/_build/html" assert docs["permissions"].get("contents") == "write" - assert docs["concurrency"]["cancel-in-progress"] is False + # A constant group serializes deploys across release runs. + concurrency = docs["concurrency"] + if isinstance(concurrency, str): + concurrency = {"group": concurrency} + assert "${{" not in concurrency["group"] + assert concurrency.get("cancel-in-progress", False) is False def test_documentation_build_matches_deploy_folder(): @@ -124,7 +129,7 @@ def test_documentation_build_matches_deploy_folder(): # ``jb build docs`` writes HTML to docs/_build/html, the deployed folder. assert recipe is not None - assert "jb build docs" in recipe.group(1) + assert "jb build docs" in [line.strip() for line in recipe.group(1).splitlines()] # Pages builds a branch source with Jekyll unless its root has .nojekyll, # which this extension writes into the HTML output. assert "sphinx.ext.githubpages" in extensions