amtronics/platform-actions

Watch

Fork 0

chore: Test the action checker against the shapes it has missed #31

New issue

Closed

opened 2026-09-04 04:03:14 +00:00 by ahmad · 1 comment

ahmad commented

2026-09-04 04:03:14 +00:00

Owner

Context

.forgejo/scripts/check-actions.py gates every pull request in this repository, and nothing tests it.

That was tolerable when it did one thing. It now has several branches that each exist because a specific shape slipped through: a runs: that is not a mapping, a steps: that is not a list, a step that is not a mapping, a missing steps: key, an empty steps: value, an empty list, and the bash -n pass over each run: body with ${{ }} substituted. Every one of those was added after something passed as ok (0 run step(s) checked).

The pattern is the problem. Each fix was verified by hand against fixtures at the time — including #28's, where I ran seven shapes against main and the branch and put the table in the pull request — and then the fixtures were thrown away. So the evidence is about the commit that made the change, not about the script as it stands. A later refactor can reintroduce any of them and CI will be green, because the only thing the pipeline runs the checker against is six actions that are all correct.

The script is also the one piece of this repository whose failure is silent by construction: a check that stops checking reports success.

Scope

In: a test for check-actions.py covering the shapes it is known to have missed, run by PR / validate.
In: fixtures that live somewhere ROOT.rglob("action.yml") cannot reach, or the test files fail the real run — this is a real trap, not a hypothetical.
Out: testing the actions themselves; changing what the checker validates.

Acceptance criteria

Given the checker is run against an action with each known-bad shape, when the test runs, then each is reported and the exit code is non-zero.
Given a valid composite action and a valid non-composite one, when the test runs, then neither is reported.
Given a fixture exists in the repository, when PR / validate runs the checker over the real actions, then the fixture is not picked up.
Given a branch of the checker is deleted, when the test runs, then it fails — verified by deleting one, not assumed.
Given the test runs in CI, when it fails, then its output identifies which shape regressed.

Notes

The last criterion in the list is the one that matters most. A test suite for a validator is itself a validator, and the same failure mode applies one level up: it can pass while checking nothing. Prove each case fails with its branch removed before trusting the suite.

### Context `.forgejo/scripts/check-actions.py` gates every pull request in this repository, and nothing tests it. That was tolerable when it did one thing. It now has several branches that each exist because a specific shape slipped through: a `runs:` that is not a mapping, a `steps:` that is not a list, a step that is not a mapping, a missing `steps:` key, an empty `steps:` value, an empty list, and the `bash -n` pass over each `run:` body with `${{ }}` substituted. Every one of those was added after something passed as `ok (0 run step(s) checked)`. The pattern is the problem. Each fix was verified by hand against fixtures at the time — including #28's, where I ran seven shapes against `main` and the branch and put the table in the pull request — and then the fixtures were thrown away. So the evidence is about the commit that made the change, not about the script as it stands. A later refactor can reintroduce any of them and CI will be green, because the only thing the pipeline runs the checker against is six actions that are all correct. The script is also the one piece of this repository whose failure is silent by construction: a check that stops checking reports success. ### Scope - In: a test for `check-actions.py` covering the shapes it is known to have missed, run by `PR / validate`. - In: fixtures that live somewhere `ROOT.rglob("action.yml")` cannot reach, or the test files fail the real run — this is a real trap, not a hypothetical. - Out: testing the actions themselves; changing what the checker validates. ### Acceptance criteria - [ ] Given the checker is run against an action with each known-bad shape, when the test runs, then each is reported and the exit code is non-zero. - [ ] Given a valid composite action and a valid non-composite one, when the test runs, then neither is reported. - [ ] Given a fixture exists in the repository, when `PR / validate` runs the checker over the real actions, then the fixture is not picked up. - [ ] Given a branch of the checker is deleted, when the test runs, then it fails — verified by deleting one, not assumed. - [ ] Given the test runs in CI, when it fails, then its output identifies which shape regressed. ### Notes The last criterion in the list is the one that matters most. A test suite for a validator is itself a validator, and the same failure mode applies one level up: it can pass while checking nothing. Prove each case fails with its branch removed before trusting the suite.

ahmad added the

labels

2026-09-04 04:03:14 +00:00

ahmad self-assigned this

2026-09-04 06:32:57 +00:00

ahmad added

state/in-progress

and removed

state/ready

labels

2026-09-04 06:32:58 +00:00

ahmad commented

2026-09-04 06:33:00 +00:00

Author

Owner

Picking this up. expiro has had no runner contact with Docker Hub since 02:06, so this repository remains where work can finish.

The shape. A test script beside the checker, run by PR / validate. Each case builds a throwaway tree containing a copy of check-actions.py and one fixture/action.yml, runs the checker there, and asserts both the exit code and a fragment of the message. I have now built this harness three times ad hoc tonight — for #28, for #29, and for verifying #30's fixtures — which is the argument for it existing as a file.

The trap the second criterion names is real and I hit it. ROOT.rglob("action.yml") finds everything beneath the repository root, so a deliberately-broken fixture committed anywhere under it fails the real run. Building each case in a temp directory outside the tree avoids it entirely, and is why I am not adding a fixtures/ folder.

The fourth criterion is the one I will spend the time on. Every case must be proved by deleting the branch of the checker it covers and watching it fail — not by watching it pass. A test suite for a validator can itself pass while checking nothing, which is the same failure the validator exists to prevent, one level up. I will run that deletion pass for each branch and put the table in the pull request.

Ambiguity I am settling: the issue says "run by PR / validate". I am adding it as its own step before the checker runs over the real actions, so a broken checker is reported as a failing test rather than as a confusing result from the real run.

Picking this up. `expiro` has had no runner contact with Docker Hub since 02:06, so this repository remains where work can finish. **The shape.** A test script beside the checker, run by `PR / validate`. Each case builds a throwaway tree containing a copy of `check-actions.py` and one `fixture/action.yml`, runs the checker there, and asserts both the exit code and a fragment of the message. I have now built this harness three times ad hoc tonight — for `#28`, for `#29`, and for verifying `#30`'s fixtures — which is the argument for it existing as a file. **The trap the second criterion names is real and I hit it.** `ROOT.rglob("action.yml")` finds everything beneath the repository root, so a deliberately-broken fixture committed anywhere under it fails the real run. Building each case in a temp directory outside the tree avoids it entirely, and is why I am not adding a `fixtures/` folder. **The fourth criterion is the one I will spend the time on.** Every case must be proved by deleting the branch of the checker it covers and watching it fail — not by watching it pass. A test suite for a validator can itself pass while checking nothing, which is the same failure the validator exists to prevent, one level up. I will run that deletion pass for each branch and put the table in the pull request. **Ambiguity I am settling:** the issue says "run by `PR / validate`". I am adding it as its own step before the checker runs over the real actions, so a broken checker is reported as a failing test rather than as a confusing result from the real run.

ahmad referenced this issue from a pull request that will close it,

2026-09-04 06:36:20 +00:00

test: Keep the fixtures that proved each checker fix #34

ahmad added

state/in-review

and removed

state/in-progress

labels

2026-09-04 06:36:50 +00:00

ahmad referenced this issue

2026-09-04 07:02:51 +00:00

test: Keep the fixtures that proved each checker fix #34