test: Cover the release retry loop and the failure reporter #63

Merged

ahmad merged 3 commits from feature/48-behaviour-suites-2 into main 2026-09-08 20:39:12 +00:00

Conversation 6 Commits 3 Files changed 3 +443 -16

ahmad commented 2026-09-08 19:23:04 +00:00

Owner

Copy link

Issue

Closes #48

Problem

PR 1 (#62) built the harness and covered deploy-vps's profile guard. The two verifications the issue actually opens with — the release retry loop and report-job-failure's fallbacks — were still hand work discarded with the pull requests that produced them, in the repository whose v3 tag every other repository consumes.

Solution

Two suites, twelve cases. The release loop over a stub curl that answers a scripted sequence of statuses; the reporter over a stub curl and a real jq or node, with the tools it does not have genuinely absent.

Three additions to the harness, each because a case needed it:

• extract_from takes a body from a marker line to its end, so the retry loop arrives with the attempt/max_attempts/delay lines above it. Supplying those from the suite would have meant asserting against my own numbers — changing max_attempts in the action would have left the table green.
• extract_block generalises the if/fi tracker to while/until/for/done; extract_if_block stays as its name.
• tools runs a case with only the named real commands on PATH, which is the only way to cover "curl is not installed". A stub cannot express absence — a stub file is the tool being present.

Review notes

Every kill label was earned by watching the case fail against a mutation of the real action, not a fixture. Ten mutants, nine dead:

release: drop 000 from the retryable arm          RED
release: the historical `|| echo 000`             RED
release: 409 no longer success                    RED
release: max_attempts 5 -> 3                      RED
release: break on 2xx removed                     RED
reporter: drop the `command -v curl` guard        RED
reporter: drop the empty-tail fallback            RED
reporter: node branch removed                     RED
reporter: refused post reported as success        RED
reporter: `-s` log test -> `-f`                   GREEN

The survivor is equivalent, and worth stating rather than papering over. With -f, an existing-but-empty log lets tail produce nothing and the emptiness check below catches it anyway, so no case can separate the two. There is a guard case pinning that state — an empty log reaches the same explanation as an absent one — and its comment says why it is not a kill.

And one of these mutants exposed a defect in my own stub before it exposed anything about the action. The historical || echo 000 survived the first campaign, because my stub curl always exited 0 — real curl writes 000 through -w and exits non-zero, and the concatenation only happens on that non-zero exit. The stub now fails the way curl does, and the mutant dies. A stub that is wrong in the direction of success is exactly how a suite records passing tables and catches nothing.

Risks and trade-offs

tools symlinks real commands from this machine, so a case naming one the runner lacks fails loudly rather than skipping. That is the intent — a silently skipped case is what this issue exists to end — but it does mean the suites assume jq and node on the runner, which the pipeline image has.

### Issue Closes #48 ### Problem PR 1 (#62) built the harness and covered `deploy-vps`'s profile guard. The two verifications the issue actually opens with — the release retry loop and `report-job-failure`'s fallbacks — were still hand work discarded with the pull requests that produced them, in the repository whose `v3` tag every other repository consumes. ### Solution Two suites, twelve cases. The release loop over a stub `curl` that answers a scripted sequence of statuses; the reporter over a stub `curl` and a real `jq` or `node`, with the tools it does *not* have genuinely absent. Three additions to the harness, each because a case needed it: - `extract_from` takes a body from a marker line to its end, so the retry loop arrives **with** the `attempt`/`max_attempts`/`delay` lines above it. Supplying those from the suite would have meant asserting against my own numbers — changing `max_attempts` in the action would have left the table green. - `extract_block` generalises the `if`/`fi` tracker to `while`/`until`/`for`/`done`; `extract_if_block` stays as its name. - `tools` runs a case with only the named real commands on PATH, which is the only way to cover "curl is not installed". A stub cannot express absence — a stub file *is* the tool being present. ### Review notes Every `kill` label was earned by watching the case fail against a mutation of the real action, not a fixture. Ten mutants, nine dead: release: drop 000 from the retryable arm RED release: the historical `|| echo 000` RED release: 409 no longer success RED release: max_attempts 5 -> 3 RED release: break on 2xx removed RED reporter: drop the `command -v curl` guard RED reporter: drop the empty-tail fallback RED reporter: node branch removed RED reporter: refused post reported as success RED reporter: `-s` log test -> `-f` GREEN **The survivor is equivalent, and worth stating rather than papering over.** With `-f`, an existing-but-empty log lets `tail` produce nothing and the emptiness check below catches it anyway, so no case can separate the two. There is a `guard` case pinning that state — an empty log reaches the same explanation as an absent one — and its comment says why it is not a `kill`. **And one of these mutants exposed a defect in my own stub before it exposed anything about the action.** The historical `|| echo 000` survived the first campaign, because my stub curl always exited 0 — real curl writes `000` through `-w` *and* exits non-zero, and the concatenation only happens on that non-zero exit. The stub now fails the way curl does, and the mutant dies. A stub that is wrong in the direction of success is exactly how a suite records passing tables and catches nothing. ### Risks and trade-offs `tools` symlinks real commands from this machine, so a case naming one the runner lacks fails loudly rather than skipping. That is the intent — a silently skipped case is what this issue exists to end — but it does mean the suites assume `jq` and `node` on the runner, which the pipeline image has.

ahmad added 1 commit 2026-09-08 19:23:04 +00:00

test: Cover the release retry loop and the failure reporter ... d6869d0e58

The release action's retry loop and report-job-failure's fallbacks
were verified by hand and the verification discarded with the pull
request that produced it. Both are now suites the pipeline reruns:
twelve cases across the two, each watched failing against a mutation
of the real action before its kill label was kept.

forgejo-actions commented 2026-09-08 19:23:16 +00:00

Copy link

The validate job failed. The tail of its output, so it can be read here rather than by fetching the run log:

ok  substitution / plain
ok  substitution / an expression containing braces
ok  substitution / two expressions keep what is between them
ok  substitution / a multi-line expression
ok  substitution / an unclosed expression
ok  valid composite action
ok  composite action whose steps are all uses
ok  runs.steps missing
ok  runs.steps written with no value
ok  runs.steps is an empty list
ok  runs.steps is not a list
ok  a step that is not a mapping
ok  runs is not a mapping
ok  runs.using is not composite
ok  a missing required field
ok  a shell syntax error in a run body
ok  an expression does not produce a false positive
ok  an expression containing braces is substituted
ok  two expressions on one line stay separate
ok  a multi-line expression is substituted
ok  an unclosed expression is reported, not swallowed
ok  an action file that is not YAML
ok  an action file that is not a mapping
ok  a valid template passes and its run steps are counted
ok  a shell syntax error in a template is reported with its job
ok  a template that is not YAML is reported, not skipped
ok  a template whose jobs is not a mapping is reported
ok  a job whose steps is malformed is reported, not silently skipped
ok  a job with no steps at all is legitimate and does not fail
ok  a .yaml template is checked too, not silently skipped
ok  no action.yml anywhere is reported, not passed

31/31 cases passed
deploy-vps/action.yml: ok (5 run step(s) checked)
grafana-dashboard/action.yml: ok (1 run step(s) checked)
notify-telegram/action.yml: ok (1 run step(s) checked)
openbao-fetch/action.yml: ok (1 run step(s) checked)
release/action.yml: ok (3 run step(s) checked)
report-job-failure/action.yml: ok (1 run step(s) checked)
security-scan/action.yml: ok (4 run step(s) checked)
templates/release.yml: ok (7 run step(s) checked)
pass  a wrong exit expectation fails the case
pass  a missing output fragment fails the case
pass  an accurate case passes
pass  the undeclared-profile case kills the neutered guard
pass  tools hides a command the machine has
pass  tools keeps the commands it names
pass  a missing step is a loud failure
pass  a missing marker is a loud failure
test-behaviour-harness: all pass
pass  [guard]  no profiles armed: the guard is skipped and nothing runs docker
pass  [kill]  armed and declared: the deploy proceeds
pass  [kill]  armed but undeclared: refused, naming the profile
pass  [kill]  a declared profile is matched whole, not as a substring
pass  [kill]  a profile name is a literal, not a regular expression
pass  [kill]  docker cannot answer: failed with docker's words, not 'declares nothing'
pass  [guard]  created first time: published, and nothing is retried
pass  [kill]  a 500 is retried and the next attempt succeeds
pass  [kill]  no response at all is retryable, and 000 is matched as itself
pass  [kill]  409 is success: the release exists from an attempt whose answer was lost
pass  [kill]  422 fails at once, with the body, and is never retried
pass  [kill]  five failures give up, naming the stranded tag and the way back
case asked for a tool this machine lacks: jq

The `validate` job failed. The tail of its output, so it can be read here rather than by fetching the run log: ``` ok substitution / plain ok substitution / an expression containing braces ok substitution / two expressions keep what is between them ok substitution / a multi-line expression ok substitution / an unclosed expression ok valid composite action ok composite action whose steps are all uses ok runs.steps missing ok runs.steps written with no value ok runs.steps is an empty list ok runs.steps is not a list ok a step that is not a mapping ok runs is not a mapping ok runs.using is not composite ok a missing required field ok a shell syntax error in a run body ok an expression does not produce a false positive ok an expression containing braces is substituted ok two expressions on one line stay separate ok a multi-line expression is substituted ok an unclosed expression is reported, not swallowed ok an action file that is not YAML ok an action file that is not a mapping ok a valid template passes and its run steps are counted ok a shell syntax error in a template is reported with its job ok a template that is not YAML is reported, not skipped ok a template whose jobs is not a mapping is reported ok a job whose steps is malformed is reported, not silently skipped ok a job with no steps at all is legitimate and does not fail ok a .yaml template is checked too, not silently skipped ok no action.yml anywhere is reported, not passed 31/31 cases passed deploy-vps/action.yml: ok (5 run step(s) checked) grafana-dashboard/action.yml: ok (1 run step(s) checked) notify-telegram/action.yml: ok (1 run step(s) checked) openbao-fetch/action.yml: ok (1 run step(s) checked) release/action.yml: ok (3 run step(s) checked) report-job-failure/action.yml: ok (1 run step(s) checked) security-scan/action.yml: ok (4 run step(s) checked) templates/release.yml: ok (7 run step(s) checked) pass a wrong exit expectation fails the case pass a missing output fragment fails the case pass an accurate case passes pass the undeclared-profile case kills the neutered guard pass tools hides a command the machine has pass tools keeps the commands it names pass a missing step is a loud failure pass a missing marker is a loud failure test-behaviour-harness: all pass pass [guard] no profiles armed: the guard is skipped and nothing runs docker pass [kill] armed and declared: the deploy proceeds pass [kill] armed but undeclared: refused, naming the profile pass [kill] a declared profile is matched whole, not as a substring pass [kill] a profile name is a literal, not a regular expression pass [kill] docker cannot answer: failed with docker's words, not 'declares nothing' pass [guard] created first time: published, and nothing is retried pass [kill] a 500 is retried and the next attempt succeeds pass [kill] no response at all is retryable, and 000 is matched as itself pass [kill] 409 is success: the release exists from an attempt whose answer was lost pass [kill] 422 fails at once, with the body, and is never retried pass [kill] five failures give up, naming the stranded tag and the way back case asked for a tool this machine lacks: jq ```

ahmad added 1 commit 2026-09-08 19:26:32 +00:00

test: Do not require a jq the CI image does not have ... fc2e0b4874

The suite asked for a real jq, which node:22-bookworm — the image
every consumer of this action runs in, and the one the pipeline uses
— does not carry. That is the very reason the action has a node
branch. The node path is now what the body-building cases exercise,
and a stub proves only the choice between the two, which is the part
that belongs to this action rather than to jq.

ahmad commented 2026-09-08 19:27:34 +00:00

Author

Owner

Copy link

Self-verification against #48's acceptance criteria, on fc2e0b4. The first three were met by PR 1 and are re-checked here because this PR extends the harness they rest on.

1. A run: body executes with named commands stubbed and its output and status asserted — twelve new cases do exactly that across two actions.
2. All four states of the deploy-vps profile guard are covered — unchanged from PR 1, still passing, plus the two matching-rule cases its review added.
3. A case written for behaviour an action does not have fails — the meta-test's three proofs still pass, and it gained two: that tools really removes a command the machine has, and that a missing extract_from marker fails loudly rather than testing nothing.
4. The harness runs in the PR pipeline beside check-actions.py — wired in PR 1; this run is the evidence, since the failure below came from the pipeline rather than from a laptop.

Every kill was earned against a mutation of the real action. Ten mutants, nine dead; the survivor (-s → -f on the log test) is genuinely equivalent, because an existing-but-empty log lets tail produce nothing and the emptiness check catches it either way. It has a guard case pinning the state and a comment saying why it is not a kill.

Two things this PR got wrong first, both worth recording.

The historical || echo 000 defect survived the first campaign — my stub curl always exited 0, while real curl writes 000 through -w and exits non-zero, and only that non-zero exit triggers the concatenation. The suite was fine; the stub was wrong in the direction of success, which is how a table of passes covers nothing.

And the first CI run went red: case asked for a tool this machine lacks: jq. The knowledge file already said so — jq is absent from node:22-bookworm, which is the image every CI job here runs in — and this action has a node branch for precisely that reason, a fact I quoted into a test comment while writing cases that contradicted it. The body-building cases now run the node branch, and a stub proves only the choice between the two, which is this action's part rather than jq's.

check-actions.py (31 cases), the meta-test (8) and the suites (18) all pass; CI is green on this head.

Self-verification against #48's acceptance criteria, on fc2e0b4. The first three were met by PR 1 and are re-checked here because this PR extends the harness they rest on. 1. **A `run:` body executes with named commands stubbed and its output and status asserted** — twelve new cases do exactly that across two actions. 2. **All four states of the deploy-vps profile guard are covered** — unchanged from PR 1, still passing, plus the two matching-rule cases its review added. 3. **A case written for behaviour an action does not have fails** — the meta-test's three proofs still pass, and it gained two: that `tools` really removes a command the machine has, and that a missing `extract_from` marker fails loudly rather than testing nothing. 4. **The harness runs in the PR pipeline beside `check-actions.py`** — wired in PR 1; this run is the evidence, since the failure below came from the pipeline rather than from a laptop. Every `kill` was earned against a mutation of the real action. Ten mutants, nine dead; the survivor (`-s` → `-f` on the log test) is genuinely equivalent, because an existing-but-empty log lets `tail` produce nothing and the emptiness check catches it either way. It has a `guard` case pinning the state and a comment saying why it is not a `kill`. **Two things this PR got wrong first, both worth recording.** The historical `|| echo 000` defect **survived** the first campaign — my stub curl always exited 0, while real curl writes `000` through `-w` *and* exits non-zero, and only that non-zero exit triggers the concatenation. The suite was fine; the stub was wrong in the direction of success, which is how a table of passes covers nothing. And the first CI run went red: `case asked for a tool this machine lacks: jq`. The knowledge file already said so — `jq` is absent from `node:22-bookworm`, which is the image every CI job here runs in — and this action has a node branch for precisely that reason, a fact I quoted into a test comment while writing cases that contradicted it. The body-building cases now run the node branch, and a stub proves only the choice between the two, which is this action's part rather than jq's. `check-actions.py` (31 cases), the meta-test (8) and the suites (18) all pass; CI is green on this head.

ahmad changed title from WIP: test: Cover the release retry loop and the failure reporter to test: Cover the release retry loop and the failure reporter 2026-09-08 19:27:35 +00:00

ahmad_bot approved these changes 2026-09-08 19:48:11 +00:00

Dismissed

ahmad_bot left a comment

Copy link

Approved. Twelve cases over the two verifications the issue actually opens with, and a description that reports its own surviving mutant instead of quietly dropping it — that honesty is worth as much as the coverage.

Both suites run green in a clean copy (21 cases, plus 8 in the harness self-test), and I re-ran the campaign myself rather than reading the table.

Your survivor is genuinely equivalent, and for a better reason than the one given. The claim is that -s → -f cannot be separated because an empty log makes tail produce nothing and the check below catches it. It is stronger than that: the check is on content, not on the file —

TAIL="$(cat "${TAIL_PATH}" 2>/dev/null || true)"
if [ -z "$(printf '%s' "${TAIL}" | tr -d '[:space:]')" ]; then

so absent, existing-but-empty and whitespace-only all reach the same branch under either flag. That makes it equivalent for every input, not just the empty one — leaving it alive and labelled guard is the right call, and the comment explaining why is the right artefact.

Two mutants you did not list, both dead: max_attempts=5 → 6 (the give-up case), and tail -c → head -c (the node-body case). So the loop's bound is pinned from both directions and the tail really is the tail.

The stub-defect story is the most valuable paragraph in the description. A stub that exits 0 where real curl exits non-zero is a stub wrong in the direction of success, and it made a real mutant look dead. That is the same failure as a double kinder than its service, one layer down in the test tooling itself.

One finding inline — a survivor on the predicate rather than the control flow, which is where they hide.

Approved. Twelve cases over the two verifications the issue actually opens with, and a description that reports its own surviving mutant instead of quietly dropping it — that honesty is worth as much as the coverage. **Both suites run green in a clean copy** (21 cases, plus 8 in the harness self-test), and I re-ran the campaign myself rather than reading the table. **Your survivor is genuinely equivalent, and for a better reason than the one given.** The claim is that `-s` → `-f` cannot be separated because an empty log makes `tail` produce nothing and the check below catches it. It is stronger than that: the check is on *content*, not on the file — TAIL="$(cat "${TAIL_PATH}" 2>/dev/null || true)" if [ -z "$(printf '%s' "${TAIL}" | tr -d '[:space:]')" ]; then so absent, existing-but-empty and whitespace-only all reach the same branch under either flag. That makes it equivalent for every input, not just the empty one — leaving it alive and labelled `guard` is the right call, and the comment explaining why is the right artefact. **Two mutants you did not list, both dead:** `max_attempts=5` → `6` (the give-up case), and `tail -c` → `head -c` (the node-body case). So the loop's bound is pinned from both directions and the tail really is the tail. **The stub-defect story is the most valuable paragraph in the description.** A stub that exits 0 where real `curl` exits non-zero is a stub wrong in the direction of success, and it made a real mutant look dead. That is the same failure as a double kinder than its service, one layer down in the test tooling itself. One finding inline — a survivor on the predicate rather than the control flow, which is where they hide.

.forgejo/scripts/behaviour-tests.py

ahmad_bot commented 2026-09-08 19:48:11 +00:00

Member

Copy link

tr -d '[:space:]' is unpinned: removing it leaves the whole suite green.

if [ -z "${TAIL}" ]; then        # the mutant
behaviour-tests: all cases pass  <- survives

The inverted control (-z → -n) kills three cases, so the suite is not blind to this line in general — it is specifically the whitespace class that no case reaches. : > job.log gives an empty log, and cat of an empty file is empty either way; nothing exercises a log that has content which is all whitespace.

That matters more here than a usual gap, because the stripping is what stops the exact bug this action exists to prevent. A job that fails having teed only blank lines — a step that outputs nothing but a trailing newline is enough — would, without the tr, post a comment containing an empty code fence. That is ahmad/expiro#27 verbatim, and the comment eight lines above says as much: "the bug this action exists to stop being copied".

It is also what makes your -s/-f equivalence argument true. So the survivor you documented and the one below rest on the same line, and only one of them is genuinely inseparable.

I wrote the case and ran it both ways — it passes against the action and fails against the mutant:

Case(
    name="a whitespace-only log reaches the explanation, not a blank fence",
    kind="kill",
    script='printf \'\\n\\n   \\n\' > job.log\n' + body,
    stubs={"curl": STUB_POST},
    env=reporter_env(),
    tools=WITH_NODE,
    exit=0,
    stdout_contains=["Posted the failure tail"],
    stderr_contains=["No output was captured"],
),

against the real action   pass  [kill]  a whitespace-only log reaches the explanation…
with tr -d removed        FAIL  [kill]  …

Thirteenth case, sits next to the empty-log guard it completes, and it turns that guard's neighbour from "we believe the stripping matters" into something that says so.

`tr -d '[:space:]'` is unpinned: removing it leaves the whole suite green. if [ -z "${TAIL}" ]; then # the mutant behaviour-tests: all cases pass <- survives The inverted control (`-z` → `-n`) kills three cases, so the suite is not blind to this line in general — it is specifically the whitespace class that no case reaches. `: > job.log` gives an *empty* log, and `cat` of an empty file is empty either way; nothing exercises a log that has content which is all whitespace. That matters more here than a usual gap, because the stripping is what stops the exact bug this action exists to prevent. A job that fails having teed only blank lines — a step that outputs nothing but a trailing newline is enough — would, without the `tr`, post a comment containing an empty code fence. That is `ahmad/expiro#27` verbatim, and the comment eight lines above says as much: *"the bug this action exists to stop being copied"*. It is also what makes your `-s`/`-f` equivalence argument true. So the survivor you documented and the one below rest on the same line, and only one of them is genuinely inseparable. I wrote the case and ran it both ways — it passes against the action and fails against the mutant: ```python Case( name="a whitespace-only log reaches the explanation, not a blank fence", kind="kill", script='printf \'\\n\\n \\n\' > job.log\n' + body, stubs={"curl": STUB_POST}, env=reporter_env(), tools=WITH_NODE, exit=0, stdout_contains=["Posted the failure tail"], stderr_contains=["No output was captured"], ), ``` against the real action pass [kill] a whitespace-only log reaches the explanation… with tr -d removed FAIL [kill] … Thirteenth case, sits next to the empty-log guard it completes, and it turns that guard's neighbour from "we believe the stripping matters" into something that says so.

ahmad changed title from test: Cover the release retry loop and the failure reporter to WIP: test: Cover the release retry loop and the failure reporter 2026-09-08 20:13:34 +00:00

ahmad added 1 commit 2026-09-08 20:14:27 +00:00

test: Cover the whitespace strip the fence bug rests on bca254a5db

ahmad dismissed ahmad_bot's review 2026-09-08 20:14:27 +00:00

Reason:

New commits pushed, approval review dismissed automatically according to repository settings

ahmad commented 2026-09-08 20:14:46 +00:00

Author

Owner

Copy link

Fixed in bca254a — added as written, and the point about why it matters more than a usual gap is the part I had wrong.

My "empty log" case writes : > job.log, which is a zero-byte file, so cat returns empty whether or not anything strips whitespace. Nothing in the suite ever produced a log with content that is all whitespace, which is the only input the tr -d '[:space:]' exists for. So the line most directly responsible for preventing ahmad/expiro#27 — a comment containing an empty code fence — was the one line nothing tested. The inverted control passing is exactly what made that invisible: the suite is not blind to the condition, only to that class of input.

Verified the same way as the rest before keeping the label: against the real action it passes; with the tr removed it fails on the fallback text. Ten mutants now, nine dead.

And you are right that this narrows my equivalence claim rather than removing it. I re-ran -s → -f with the new case in place and it still survives: a whitespace-only log has size greater than zero, so both tests take the same branch, and tail produces the same bytes either way. The two really are inseparable on behaviour — but the reason is now stated on the right line, and the guard case sits beside a kill that pins the stripping rather than standing in for it.

Fixed in bca254a — added as written, and the point about *why* it matters more than a usual gap is the part I had wrong. My "empty log" case writes `: > job.log`, which is a zero-byte file, so `cat` returns empty whether or not anything strips whitespace. Nothing in the suite ever produced a log with content that is *all* whitespace, which is the only input the `tr -d '[:space:]'` exists for. So the line most directly responsible for preventing `ahmad/expiro#27` — a comment containing an empty code fence — was the one line nothing tested. The inverted control passing is exactly what made that invisible: the suite is not blind to the condition, only to that class of input. Verified the same way as the rest before keeping the label: against the real action it passes; with the `tr` removed it fails on the fallback text. Ten mutants now, nine dead. **And you are right that this narrows my equivalence claim rather than removing it.** I re-ran `-s` → `-f` with the new case in place and it still survives: a whitespace-only log has size greater than zero, so both tests take the same branch, and `tail` produces the same bytes either way. The two really are inseparable on behaviour — but the reason is now stated on the right line, and the `guard` case sits beside a `kill` that pins the stripping rather than standing in for it.

ahmad commented 2026-09-08 20:27:17 +00:00

Author

Owner

Copy link

Re-verified on bca254a after adding the whitespace case.

The suite is 19 cases across three actions. Ten mutations of the real actions, nine dead; the survivor (-s → -f on the log test) is genuinely equivalent, and now demonstrably so rather than by assertion — with the whitespace case in place I re-ran it and it still survives, because a whitespace-only log has non-zero size and both tests take the same branch.

check-actions.py (31 cases), the meta-test (8, including the two for tools and extract_from) and the suites (19) all pass. CI is green on this head.

Re-verified on bca254a after adding the whitespace case. The suite is 19 cases across three actions. Ten mutations of the real actions, **nine dead**; the survivor (`-s` → `-f` on the log test) is genuinely equivalent, and now demonstrably so rather than by assertion — with the whitespace case in place I re-ran it and it still survives, because a whitespace-only log has non-zero size and both tests take the same branch. `check-actions.py` (31 cases), the meta-test (8, including the two for `tools` and `extract_from`) and the suites (19) all pass. CI is green on this head.

ahmad changed title from WIP: test: Cover the release retry loop and the failure reporter to test: Cover the release retry loop and the failure reporter 2026-09-08 20:27:17 +00:00

ahmad_bot approved these changes 2026-09-08 20:35:04 +00:00

ahmad_bot left a comment

Copy link

Re-reviewed the delta only — one commit, bca254a, adding the whitespace case from the last review. Approved.

I ran it rather than read it, in a copy of this head:

• Unmutated: 22 cases pass (18 kill, 4 guard), and the harness self-test passes.

• With the strip removed (if [ -z "${TAIL}" ]), exactly one case fails — a whitespace-only log reaches the explanation, not a blank fence — and the captured body is the failure this action exists to prevent, verbatim:

  {"body":"The `validate` job failed. … \n\n```\n\n\n   \n```"}
  

  An empty fence, posted on a pull request. Nothing else in the suite moves, which is what makes it a killer for that line specifically rather than a case that happens to be near it.

• Your narrowed survivor claim holds. With the whitespace case in place I mutated -s → -f again: still all green. A whitespace-only log has non-zero size, so both tests take the same branch for it too — the equivalence is now demonstrated over the input class that previously made it unverifiable, not asserted.

So the one gap I raised is closed, and closing it also settled the open question about the survivor beside it. Nothing further from me.

Re-reviewed the delta only — one commit, `bca254a`, adding the whitespace case from the last review. Approved. I ran it rather than read it, in a copy of this head: - **Unmutated:** 22 cases pass (18 `kill`, 4 `guard`), and the harness self-test passes. - **With the strip removed** (`if [ -z "${TAIL}" ]`), exactly one case fails — `a whitespace-only log reaches the explanation, not a blank fence` — and the captured body is the failure this action exists to prevent, verbatim: {"body":"The `validate` job failed. … \n\n```\n\n\n \n```"} An empty fence, posted on a pull request. Nothing else in the suite moves, which is what makes it a killer for that line specifically rather than a case that happens to be near it. - **Your narrowed survivor claim holds.** With the whitespace case in place I mutated `-s` → `-f` again: still all green. A whitespace-only log has non-zero size, so both tests take the same branch for it too — the equivalence is now demonstrated over the input class that previously made it unverifiable, not asserted. So the one gap I raised is closed, and closing it also settled the open question about the survivor beside it. Nothing further from me.

ahmad merged commit 8471f0d6fa into main 2026-09-08 20:39:12 +00:00

ahmad deleted branch feature/48-behaviour-suites-2 2026-09-08 20:39:12 +00:00

ahmad referenced this pull request from a commit 2026-09-08 20:39:13 +00:00

test: Cover the release retry loop and the failure reporter (#63)

Sign in to join this conversation.

Reviewers

No reviewers

ahmad_bot

Labels

Clear labels   

priority/P0

Drop everything

  

priority/P1

High

  

priority/P2

Normal

  

priority/P3

Low

  

size/L

Several days

  

size/M

A day or two

  

size/S

A few hours

  

size/XL

  

size/XS

Under an hour

  

state/blocked

  

state/done

Merged and verified

  

state/in-progress

Being worked on

  

state/in-review

PR open

  

state/needs-refinement

Not ready to pick up

  

state/ready

Refined and ready to start

  

type/bug

Something is broken

  

type/chore

Maintenance, tooling, setup

  

type/feature

New functionality

  

type/refactor

Restructuring without behaviour change

No labels

priority/P0

priority/P1

priority/P2

priority/P3

size/L

size/M

size/S

size/XL

size/XS

state/blocked

state/done

state/in-progress

state/in-review

state/needs-refinement

state/ready

type/bug

type/chore

type/feature

type/refactor

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

3 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

amtronics/platform-actions!63

No description provided.