ci: let the summarisers summarise and the gate do the failing (#142) #155
No Assignees
bermudalamb
bermudalamb
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: bermudalamb/redefined-designs#155
Reference in New Issue
Block a user
A failing end-to-end run reported itself like this:
which reads as a broken summary script. It was not — it was the summariser correctly reporting that tests had failed, with nothing said about it.
Two things combined to produce that.
The summariser doubled as the gate. It ended with
process.exit(stats.unexpected > 0 ? 1 : 0), so it failed the job itself. The workflow already has a step written for exactly that —Fail if either suite failed, whose comment explains it exists socontinue-on-erroron the suites cannot turn a failing suite into a passing job. That step was being skipped while its condition was true, because a step whoseif:omitsalways()still implicitly requires its predecessors to have succeeded, and the summariser had already failed the job one step earlier. The gate written to be the place the job fails was dead code.And the explanation went where the log is not.
report()writes to GITEA_STEP_SUMMARY when it is set, which under Actions is always, so the counts and the list of failing tests landed in the Summary tab while the log showed a bare exit with no output at all.So both scripts now exit 0 whatever they find, both print one line of counts to stdout as well as the markdown to the summary, and the gate carries
always() &&so it actually runs. A failing suite now fails at a step named for what failed, and the log says how many.Verified by executing both scripts against crafted results rather than by reading them: failing counts, passing counts, and a missing results file, each with and without GITEA_STEP_SUMMARY set. All four exit 0, the headline appears on stdout in every case, and the step summary still receives the full table and the failure list.
Not covered: nothing in this repository runs the scripts under
scripts/. Jest's testMatch is scoped to backend/tests/unit, so a permanent regression test would need a runner these files do not have. Worth its own issue rather than widening the backend suite's roots to reach the repository root.Closes #142
A failing end-to-end run reported itself like this: Run node scripts/summarize-playwright.js frontend/playwright-results.json ❌ Failure - Main Summarize end-to-end tests exitcode '1': failure which reads as a broken summary script. It was not — it was the summariser correctly reporting that tests had failed, with nothing said about it. Two things combined to produce that. The summariser doubled as the gate. It ended with `process.exit(stats.unexpected > 0 ? 1 : 0)`, so it failed the job itself. The workflow already has a step written for exactly that — `Fail if either suite failed`, whose comment explains it exists so `continue-on-error` on the suites cannot turn a failing suite into a passing job. That step was being skipped while its condition was true, because a step whose `if:` omits `always()` still implicitly requires its predecessors to have succeeded, and the summariser had already failed the job one step earlier. The gate written to be the place the job fails was dead code. And the explanation went where the log is not. `report()` writes to GITEA_STEP_SUMMARY when it is set, which under Actions is always, so the counts and the list of failing tests landed in the Summary tab while the log showed a bare exit with no output at all. So both scripts now exit 0 whatever they find, both print one line of counts to stdout as well as the markdown to the summary, and the gate carries `always() &&` so it actually runs. A failing suite now fails at a step named for what failed, and the log says how many. Verified by executing both scripts against crafted results rather than by reading them: failing counts, passing counts, and a missing results file, each with and without GITEA_STEP_SUMMARY set. All four exit 0, the headline appears on stdout in every case, and the step summary still receives the full table and the failure list. Not covered: nothing in this repository runs the scripts under `scripts/`. Jest's testMatch is scoped to backend/tests/unit, so a permanent regression test would need a runner these files do not have. Worth its own issue rather than widening the backend suite's roots to reach the repository root. Closes #142