refactor(ci): extract alert formatting into composite action and isolate Python tests#315
Conversation
e50f50c to
70e00a4
Compare
rstata
left a comment
There was a problem hiding this comment.
Review
Good refactoring overall — the composite action is well-structured, output references are preserved correctly across all callers, and isolating the Python tests into their own job (15s standalone vs duplicated across 6 matrix cells) is a nice efficiency win.
One bug and one nit.
Bug: multi-line jobs_json will break $GITHUB_OUTPUT in ci-manual-trigger.yml
Lines 72/75/78 call jq without -c, so the output is pretty-printed (multi-line). Line 80 then writes it as:
echo "jobs_json=$jobs_json" >> "$GITHUB_OUTPUT"The echo "key=value" format only works for single-line values. When a CI job actually fails, the multi-line JSON will be truncated at the first newline, and the composite action will receive invalid JSON.
CI is green on this PR because the format-alerts job only runs on failure — this path was not exercised.
Fix: add -c (compact) to the jq calls:
jobs_json=$(echo "$jobs_json" | jq -c --arg name "Build and Test" --arg run_id "$BAT_RUN_ID" '. + [{"name": $name, "run_id": $run_id}]')(Same for the SDT and BCV lines.)
Nit: display name change
"Standalone Tests" was renamed to "Demo App Tests" on line 75. Intentional? Fine either way, just flagging since it'll change the alert text.
|
from claude:
If you confirm this is a bug is there a regression test you can add for it? |
Description
Three workflows duplicated the same inline shell pattern for formatting CI failure alerts (build JSON with jq, pipe to
format_alert.py, write to$GITHUB_OUTPUT).This PR extracts that pattern into a reusable composite action and moves Python script tests into their own lightweight job so they run in seconds without waiting for the Gradle build pipeline.
Key Changes
.github/actions/collect-failure-info/action.yml— new composite action that accepts ajobs_jsonarray and context inputs, constructs the alert payload, and callsformat_alert.py. Follows the same caller-must-checkout pattern assetup-build..github/workflows/ci-manual-trigger.yml— split the monolithicformat-alertsstep into a job-filtering step and a composite action call, removing ~20 lines of inline shell..github/workflows/periodic-green-check.yml— replaced inline staleness alert formatting with the composite action..github/workflows/post-alerts.yml— replaced inline test alert formatting with the composite action..github/workflows/build-and-test.yml— added a standalonepython-testsjob (no JVM/Gradle, runs in parallel) and removed Pythonteststeps from the Gradle test matrix job, eliminating 18 redundant step executions per CI run (3 steps x 6 matrix cells).How was it tested? What documentation was provided?
CI and gradle build
actionlintpasses on all modified and new workflow filespython3 -m unittest discover -s .github/scripts/tests