fix: the action manifest could not load, and CI could not tell - #27
Merged
Conversation
An input description contained `${{ toJSON(needs) }}` as an example of what
to pass. Actions evaluates expressions inside input descriptions when it
loads the manifest, and `needs` is not a named value there, so the action
failed to load — for every consumer, with an error pointing at this file
rather than at their workflow.
CI was green through all of it. The workflow ran test_gate.py and never
invoked the action, so it proved gate.py correct and said nothing about
action.yml. The defect surfaced in the first consumer instead.
A test that exercises the script but not the artifact consumers load is not
testing the thing that ships. The workflow's own gate job now uses the
composite action for real, so a manifest that cannot load fails here rather
than downstream. That job is also this repository's required check, so the
invocation is not a fixture that could be dropped as redundant.
The exhaustive failure-mode coverage stays in test_gate.py against the same
gate.py the action runs; what only a live invocation can prove is that the
manifest loads and the inputs are wired, which is precisely what broke.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The action shipped in #26 cannot load. First consumer to try it:
An input description contained
${{ toJSON(needs) }}as an example of what to pass. Actions evaluates expressions inside input descriptions at manifest load, andneedsis not a named value in that context — so the whole action fails to load, for every consumer, with an error pointing at this file rather than at their workflow.The part worth fixing properly
CI was green through all of it. The workflow ran
test_gate.pyand never invoked the action — it provedgate.pycorrect and said nothing aboutaction.yml. The defect surfaced in incident-response#76 instead.A test that exercises the script but not the artifact consumers load is not testing the thing that ships.
So the workflow's own
merge gatejob now uses the composite action for real. A manifest that cannot load fails here rather than downstream. That job is also this repository's required check, which means the invocation can't later be dropped as a redundant fixture.What is tested where
test_gate.py— all ten failure modes against the samegate.pythe action runs. Unchanged.manifest— the YAML parses.merge gate— a live invocation. The only thing that proves the manifest loads and the inputs are wired, which is exactly what broke.I did try a richer selftest first (a deliberately-red fixture job asserting the gate goes red through the real runtime) and dropped it: the gate's own completeness check correctly refused, because those fixture jobs sat in the same workflow without being in the gate's
needs. That was the check working as designed against a bad test, not a reason to weaken it.Verified: gate watches 2 of 2 jobs, nothing unwatched,
if: always().Consumers repin after this merges.