forked from heavy-duty/ceremony
test(issueflow): each of the three sites is caught by behaviour, not only by the pin (#210)
@codex-reviewer-andresmgsl's two scope items, applied before the first review round rather than after. 1. THE GUARD IS COMMENT-AWARE, WITH CONTROLS. It already stripped comments — it has to, because the #188 warning that explains why has("pull_request") is wrong contains the string. Without controls that was an untested property, and the pressure it creates is real: a raw grep would push a builder into deleting the very warning that prevents recurrence. Two fixtures now prove it: the explanatory comment is allowed, an executable jq filter is rejected. 2. ALL THREE SITES ARE DRIVEN BY BEHAVIOUR. The pin makes any revert red, but a pin proves a string is absent, not that each replacement means the intended thing: BOARD_RECORDS the forgejo-shaped board is not read as empty release_bodies an open `release` issue whose gate holds an open member makes a claimable NON-member draw a window flag — empty carriers, no flag, so the row discriminates the site instead of merely reaching it reconcile_issue_pass the scalar payload: key-present-null is an issue, object-valued is a PR, key-absent is still an issue The release_bodies row did NOT discriminate on its first write — it asserted an issue number that BOARD_RECORDS also produces, so reverting the site left it green. Caught by mutating each site separately rather than trusting the suite total. Mutation, per site: BOARD_RECORDS 3 red, release_bodies 2 red, reconcile_issue_pass 2 red. test/run.sh 28/28; issueflow 510/510; shellcheck 0.10.0 clean. Refs #210
This commit is contained in:
parent
877e09e015
commit
bada4ffff5
2 changed files with 64 additions and 1 deletions
|
|
@ -19,4 +19,9 @@
|
||||||
(#210).
|
(#210).
|
||||||
|
|
||||||
- A source pin forbids `has("pull_request")` on this surface, because the rule
|
- A source pin forbids `has("pull_request")` on this surface, because the rule
|
||||||
was stated in a comment and violated forty lines below it (#210).
|
was stated in a comment and violated forty lines below it. It strips comments,
|
||||||
|
so the #188 warning that explains the trap is allowed to stay (#210).
|
||||||
|
|
||||||
|
- All three sites are covered behaviourally, not only by the pin: the board
|
||||||
|
gather, the release-body gather through an observable window flag, and the
|
||||||
|
per-issue payload check (#210).
|
||||||
|
|
|
||||||
|
|
@ -1815,6 +1815,47 @@ check "...the sweep completes over it" 0 "" \
|
||||||
check "...and the PR row is still excluded from the issue set" 1 "" \
|
check "...and the PR row is still excluded from the issue set" 1 "" \
|
||||||
grep -qE '^issueflow: #61' <<<"$fjb_out"
|
grep -qE '^issueflow: #61' <<<"$fjb_out"
|
||||||
|
|
||||||
|
# release_bodies is the THIRD producer and has its own has() site. A `release`
|
||||||
|
# issue on a forgejo-shaped board must reach the window gather, or the #292
|
||||||
|
# flags are decided over an empty set (@codex-reviewer-andresmgsl, #210).
|
||||||
|
printf '%s\n' \
|
||||||
|
'[{"number":60,"pull_request":null,"labels":[{"name":"ready"}],"title":"an issue"},
|
||||||
|
{"number":62,"pull_request":null,"labels":[{"name":"release"}],"title":"Release 9.9.9","body":"Blocked by #60."},
|
||||||
|
{"number":63,"pull_request":null,"labels":[{"name":"ready"}],"title":"a claimable non-member"},
|
||||||
|
{"number":61,"pull_request":{"merged":false},"labels":[],"title":"a pull request"}]' \
|
||||||
|
>"$FORGEJO_BOARD/repos_owner_repo_issues_state_open.json"
|
||||||
|
jq -n --arg at "$(iso_at "$INOW")" \
|
||||||
|
'{number:63,user:{login:"triage-one"},created_at:$at,body:"",pull_request:null,
|
||||||
|
labels:[{name:"ready"}],assignees:[]}' \
|
||||||
|
>"$FORGEJO_BOARD/repos_owner_repo_issues_63.json"
|
||||||
|
printf '[]\n' >"$FORGEJO_BOARD/repos_owner_repo_issues_63_comments.json"
|
||||||
|
jq -n --arg at "$(iso_at "$INOW")" \
|
||||||
|
'{number:62,user:{login:"triage-one"},created_at:$at,body:"Blocked by #60.",pull_request:null,
|
||||||
|
labels:[{name:"release"}],assignees:[]}' \
|
||||||
|
>"$FORGEJO_BOARD/repos_owner_repo_issues_62.json"
|
||||||
|
printf '[]\n' >"$FORGEJO_BOARD/repos_owner_repo_issues_62_comments.json"
|
||||||
|
fjb2_out="$(forgejo_board_run)"
|
||||||
|
# The observable effect of release_bodies being NON-empty: an open `release`
|
||||||
|
# issue whose gate still holds an open member makes every claimable non-member
|
||||||
|
# draw a window flag. With that gather empty there are no carriers and no flag,
|
||||||
|
# so this row discriminates the site rather than merely reaching it.
|
||||||
|
check "a release issue on a forgejo-shaped board reaches the window gather" 0 "" \
|
||||||
|
grep -qE '#63: window flag' <<<"$fjb2_out"
|
||||||
|
check "...and the sweep still completes" 0 "" \
|
||||||
|
grep -qF 'issueflow: reconciled.' <<<"$fjb2_out"
|
||||||
|
|
||||||
|
# The THIRD site is reconcile_issue_pass's per-issue payload check. Same key,
|
||||||
|
# scalar rather than a list: null means issue, an object means PR.
|
||||||
|
pass_disc() { # $1 = the payload -> the exit status the guard would take
|
||||||
|
jq -e '.pull_request == null' <<<"$1" >/dev/null && echo issue || echo pr
|
||||||
|
}
|
||||||
|
check "a forgejo issue payload (key present, null) reads as an issue" 0 "issue" \
|
||||||
|
pass_disc '{"number":60,"pull_request":null}'
|
||||||
|
check "...and an object-valued one reads as a PR" 0 "pr" \
|
||||||
|
pass_disc '{"number":61,"pull_request":{"merged":false}}'
|
||||||
|
check "...and a github-shaped payload (key absent) still reads as an issue" 0 "issue" \
|
||||||
|
pass_disc '{"number":60}'
|
||||||
|
|
||||||
# -- the rule is pinned at the source, because a comment did not hold --------
|
# -- the rule is pinned at the source, because a comment did not hold --------
|
||||||
# `.pull_request == null` is stated in this file's own header AND at
|
# `.pull_request == null` is stated in this file's own header AND at
|
||||||
# issueflow-reconcile.sh:1113 — and the merge put `has("pull_request")` back 40
|
# issueflow-reconcile.sh:1113 — and the merge put `has("pull_request")` back 40
|
||||||
|
|
@ -1830,6 +1871,23 @@ no_has_pull_request() {
|
||||||
}
|
}
|
||||||
check "no executable has(\"pull_request\") survives on this surface" 0 "" \
|
check "no executable has(\"pull_request\") survives on this surface" 0 "" \
|
||||||
no_has_pull_request
|
no_has_pull_request
|
||||||
|
# The controls, because this guard MUST tolerate the #188 comment that explains
|
||||||
|
# why the form is wrong — a raw grep would either fail forever or pressure a
|
||||||
|
# builder into deleting the very warning that prevents recurrence
|
||||||
|
# (@codex-reviewer-andresmgsl, #210 review).
|
||||||
|
DTMP="$(mktemp -d)"; trap 'rm -rf "$DTMP"' EXIT
|
||||||
|
strip_and_find() { # $1 = file -> 0 when an EXECUTABLE use survives
|
||||||
|
sed 's/[[:space:]]#.*$//; s/^[[:space:]]*#.*$//' "$1" | grep -q 'has("pull_request")'
|
||||||
|
}
|
||||||
|
# shellcheck disable=SC2016 # fixture CONTENT: the literal text a scanned file would hold
|
||||||
|
printf '%s\n' '#!/usr/bin/env bash' \
|
||||||
|
'# `.pull_request == null`, NOT has("pull_request") | not (#188)' \
|
||||||
|
'jq -e ".pull_request == null" <<<"$J"' >"$DTMP/prose.sh"
|
||||||
|
check "the explanatory #188 comment is allowed" 1 "" strip_and_find "$DTMP/prose.sh"
|
||||||
|
# single-quoted so the fixture holds the LITERAL form the guard looks for
|
||||||
|
printf '%s\n' '#!/usr/bin/env bash' \
|
||||||
|
"jq -r '.[] | select(has(\"pull_request\") | not)' <<<\"\$J\"" >"$DTMP/exec.sh"
|
||||||
|
check "...while an executable jq filter is rejected" 0 "" strip_and_find "$DTMP/exec.sh"
|
||||||
|
|
||||||
# -- the OPEN-pull gather, at main() granularity ----------------------------
|
# -- the OPEN-pull gather, at main() granularity ----------------------------
|
||||||
# The closed/merged half above proves one REST path; this proves the other,
|
# The closed/merged half above proves one REST path; this proves the other,
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue