-
Notifications
You must be signed in to change notification settings - Fork 0
test(ci): smoke-run the review workflow in CI #27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
536421a
ccab514
8149a3b
d2e7aaf
6ba8072
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -25,6 +25,7 @@ cd "$(dirname "$0")/.." | |||||
|
|
||||||
| failures=0 | ||||||
| WORKFLOW_FILE=.github/workflows/claude-pr-review.yml | ||||||
| TESTS_FILE=.github/workflows/tests.yml | ||||||
|
|
||||||
| # The delimiter, assembled rather than written, so this file does not trip its own scan. | ||||||
| OPEN="\${$(printf '%s' '{')" | ||||||
|
|
@@ -97,6 +98,30 @@ for wf in "${WORKFLOWS[@]}"; do | |||||
| fi | ||||||
| done | ||||||
|
|
||||||
| # cancel-in-progress cancels whatever else is in the group, so the group needs a key that is | ||||||
| # never empty on any event the workflow accepts. It is keyed on the pull request number, and | ||||||
| # tests.yml calls this workflow on `push: branches: [main]` as well, where there is no pull | ||||||
| # request and that key expands to nothing -- collapsing every main-push run into one constant | ||||||
| # group. Two merges landing close together would then cancel each other, and because the | ||||||
| # cancellation lands on a *called* workflow it takes the caller's whole Tests run with it, so a | ||||||
| # commit on main silently loses its test signal. | ||||||
| # | ||||||
| # Checked as a property of the expression rather than by evaluating it: the PR number must be | ||||||
| # followed by a `||` fallback, so the group stays unique when there is no pull request. | ||||||
| group_line=$(grep -n '^ group:' "$WORKFLOW_FILE" | head -1) | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: under If The result is still a red CI, so this isn't a silent pass. But it exits after
Suggested change
|
||||||
| if [ -z "$group_line" ]; then | ||||||
| echo "FAIL $WORKFLOW_FILE has no workflow-level concurrency group; this check proves nothing" | ||||||
| failures=$((failures + 1)) | ||||||
| elif ! printf '%s\n' "$group_line" | grep -q 'pull_request\.number[[:space:]]*||'; then | ||||||
| echo "FAIL the concurrency group keys on the pull request number with no fallback, so on a" | ||||||
| echo " push to main it collapses to a constant and concurrent merges cancel each other's" | ||||||
| echo " Tests run:" | ||||||
| printf '%s\n' "$group_line" | sed 's/^/ /' | ||||||
| failures=$((failures + 1)) | ||||||
| else | ||||||
| echo "ok the concurrency group stays unique when there is no pull request" | ||||||
| fi | ||||||
|
|
||||||
| # Every read the context step makes needs a permission declared on the job, because the job | ||||||
| # declares `permissions:` explicitly and anything unlisted is `none`. That failure is silent | ||||||
| # by design -- each block degrades to its "could not read" sentence -- so a missing line here | ||||||
|
|
@@ -142,6 +167,36 @@ check_permission "statusCheckRollup" statuses "the StatusContext half of the CI | |||||
| check_permission "/compare/" contents "the since-last-review comparison" | ||||||
| check_permission "/pulls/" pull-requests "the PR reads" | ||||||
|
|
||||||
| # The table above forces a new API call in the context step to declare its permission on the | ||||||
| # review job. That does nothing for the smoke job in tests.yml, which calls the review workflow | ||||||
| # and has to grant the same set by hand: a caller cannot give a reusable workflow more than it | ||||||
| # holds, so a permission added on one side and not the other fails the smoke job with Actions' | ||||||
| # "is requesting 'x: read', but is only allowed 'x: none'" -- loud, but on a file that looks | ||||||
| # unrelated to the change that caused it. Asserting the two match keeps the claim in tests.yml's | ||||||
| # comment true by construction instead of by review. | ||||||
| # | ||||||
| # Name and value both, so pull-requests: write degrading to read is caught too. | ||||||
| perm_pairs() { | ||||||
| awk '/^ permissions:$/ { p = 1; next } | ||||||
| p && /^ [a-z-]+:[[:space:]]/ { print $1, $2 } | ||||||
| p && /^ [a-z]/ { exit }' "$1" | sort | ||||||
| } | ||||||
| review_perms=$(perm_pairs "$WORKFLOW_FILE") | ||||||
| smoke_perms=$(perm_pairs "$TESTS_FILE") | ||||||
| if [ -z "$review_perms" ] || [ -z "$smoke_perms" ]; then | ||||||
| echo "FAIL a permissions block came back empty (review: $(printf '%s' "$review_perms" | wc -l)," \ | ||||||
| "smoke: $(printf '%s' "$smoke_perms" | wc -l)); the parity check proves nothing" | ||||||
| failures=$((failures + 1)) | ||||||
| elif [ "$review_perms" != "$smoke_perms" ]; then | ||||||
| echo "FAIL the smoke job in $TESTS_FILE does not grant what the review job declares." | ||||||
| echo " A caller cannot grant a reusable workflow more than it holds, so the smoke job" | ||||||
| echo " fails until both sides agree. Difference (< review job, > smoke job):" | ||||||
| diff <(printf '%s\n' "$review_perms") <(printf '%s\n' "$smoke_perms") | sed 's/^/ /' | ||||||
| failures=$((failures + 1)) | ||||||
| else | ||||||
| echo "ok the smoke job grants exactly what the review job declares" | ||||||
| fi | ||||||
|
|
||||||
| # The scan above is a backstop for one class. actionlint checks the schema, the expression | ||||||
| # grammar, and the shell; run it when it is on PATH. shellcheck findings are excluded because | ||||||
| # the run blocks here intentionally use unquoted word splitting for job ids. | ||||||
|
|
||||||
Uh oh!
There was an error while loading. Please reload this page.