-
Notifications
You must be signed in to change notification settings - Fork 0
Reference setup-rust remotely so its post hook survives relocation (#365) #375
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |
|
|
||
| from __future__ import annotations | ||
|
|
||
| import re | ||
| import typing as typ | ||
| from pathlib import Path | ||
|
|
||
|
|
@@ -25,6 +26,8 @@ | |
| CHECKOUT_STEP = "Checkout workflow repository" | ||
| RELOCATE_STEP = "Relocate workflow source" | ||
| RELOCATED_DIR_EXPR = "${{ steps.relocate-workflow-source.outputs.workflow_dir }}" | ||
| LOCAL_WORKFLOW_SRC_PREFIX = "./workflow-src/" | ||
| REMOTE_SETUP_RUST_PREFIX = "leynos/shared-actions/.github/actions/setup-rust@" | ||
|
|
||
| pytestmark = pytest.mark.skipif( | ||
| not all((WORKFLOWS_DIR / name).exists() for name in WORKFLOW_NAMES), | ||
|
|
@@ -55,6 +58,59 @@ def _step_names(steps: list[dict[str, object]]) -> list[object]: | |
| return [step.get("name") for step in steps] | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("workflow_name", WORKFLOW_NAMES) | ||
| def test_no_step_references_a_relocated_workflow_src_action( | ||
| workflow_name: str, | ||
| ) -> None: | ||
| """No step invokes a composite action via the ``./workflow-src/`` path. | ||
|
|
||
| A workspace-relative ``uses: ./workflow-src/...`` reference breaks at | ||
| job teardown: the Actions runtime re-resolves the local action's | ||
| ``action.yml`` from that path when running its post hook, but the | ||
| "Relocate workflow source" step has by then moved ``workflow-src`` to | ||
| ``$RUNNER_TEMP``. The post step fails to find the definition and reddens | ||
| an otherwise green run (issue #365). Composite actions with post hooks | ||
| must instead be referenced remotely (pinned by SHA); the runner | ||
| materialises those under its managed ``_actions`` directory, outside the | ||
| caller's workspace and surviving teardown. | ||
| """ | ||
| for job_name, job in _jobs(workflow_name).items(): | ||
| for step in _steps(job): | ||
| uses = step.get("uses") | ||
| if not isinstance(uses, str): | ||
| continue | ||
| assert not uses.startswith(LOCAL_WORKFLOW_SRC_PREFIX), ( | ||
| f"{workflow_name}:{job_name} step {step.get('name')!r} " | ||
| f"references {uses!r}; a workspace-relative workflow-src " | ||
| f"action path breaks its post hook once workflow-src is " | ||
| f"relocated to $RUNNER_TEMP (issue #365). Reference the " | ||
| f"action remotely, pinned by SHA, instead." | ||
| ) | ||
| return | ||
|
|
||
|
|
||
| def test_mutants_setup_rust_survives_workflow_source_relocation() -> None: | ||
| """Setup Rust is remote, SHA-pinned, guarded, and precedes relocation.""" | ||
| steps = _steps(_jobs("mutation-cargo.yml")["mutants"]) | ||
| names = _step_names(steps) | ||
| assert "Setup Rust" in names, "mutants job must contain a Setup Rust step" | ||
| assert RELOCATE_STEP in names, "mutants job must relocate workflow-src" | ||
| setup_rust = steps[names.index("Setup Rust")] | ||
| uses = setup_rust.get("uses") | ||
|
|
||
| assert isinstance(uses, str) | ||
| assert re.fullmatch( | ||
| rf"{re.escape(REMOTE_SETUP_RUST_PREFIX)}[0-9a-f]{{40}}", uses | ||
| ), "mutants:Setup Rust must use the remote action pinned by a full SHA" | ||
| assert setup_rust.get("if") == "${{ env.ACT != 'true' }}" | ||
| assert names.index("Setup Rust") < names.index(RELOCATE_STEP) | ||
|
|
||
| relocate = steps[names.index(RELOCATE_STEP)] | ||
| run = relocate.get("run") | ||
| assert isinstance(run, str) | ||
| assert '"${RUNNER_TEMP}/workflow-src"' in run | ||
|
|
||
|
|
||
|
Comment on lines
+61
to
+113
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Add the required terminal return. Add As per coding guidelines, add an explicit return at the end of every function and avoid relying on implicit Apply the explicit terminal return@@
)
+ return🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| @pytest.mark.parametrize("workflow_name", WORKFLOW_NAMES) | ||
| def test_every_workflow_checkout_is_followed_by_relocation( | ||
| workflow_name: str, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the stale relocation comment.
Update the later comment at Line 263. The
setup-ruststep now uses the remote action from the runner-managed_actionsdirectory, so the statement thatSetup Rustneeds the workspace-local action path is false. Remove that rationale or replace it with the actual ordering requirement.🤖 Prompt for AI Agents