improvise-29w2: a CLOSED PR the loop waits on (MERGEABLE/BLOCKED) pins the state check at the loop top
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
@@ -286,8 +286,13 @@ The rule now:
|
||||
already merged, and can come mid-wait when someone merges it during the watch.
|
||||
A merged PR reports `UNKNOWN/UNKNOWN` permanently (#189, read live), so the
|
||||
UNKNOWN wait slept the whole timeout there and then blamed mergeability. A
|
||||
closed one need not: #74 reads `CONFLICTING/DIRTY`, which already ended the
|
||||
wait, but as a misleading exit 1. The script now fetches `state`. Exit 5 is
|
||||
closed PR's mergeability is whatever it was when it closed, so it can land on
|
||||
either side of the wait. #74 reads `CONFLICTING/DIRTY`, which ends the loop at
|
||||
once, but as a misleading exit 1. #46 has green checks and reads
|
||||
`MERGEABLE/BLOCKED`, which the loop waits on, so it slept the full timeout
|
||||
too (improvise-29w2). The suite carries all three shapes, and only the
|
||||
waited-on ones can tell whether the state is checked at the top of the loop.
|
||||
The script now fetches `state`. Exit 5 is
|
||||
its own code, not 1 or 0, because under merge-as-soon-as-green "someone else
|
||||
already merged it" is a normal answer, and the caller should hear it as that.
|
||||
- **The head checks come first, and the ended check after them**
|
||||
|
||||
@@ -718,6 +718,19 @@ Two rivals:
|
||||
- **Exit 1 instead of 5.** Both exit-code cases fail with exit 1. That keeps
|
||||
"already merged" apart from "not mergeable".
|
||||
|
||||
**29w2: the CLOSED cases could not discriminate.** Found reviewing #253 at
|
||||
`8cdea3a4`. The only CLOSED shape was #74's `CONFLICTING/DIRTY`, and that ends
|
||||
the loop on the first poll whether or not the state is checked there. So
|
||||
**rival R8** passed 52 of 52. R8 exits 5 for MERGED at the loop top, but for
|
||||
CLOSED only after the loop breaks. Run live against #46 with `--timeout 15`, R8
|
||||
exited 3 and `8cdea3a4` exited 5. A third shape is added, #46's: CLOSED, green
|
||||
checks, `MERGEABLE/BLOCKED`, which the loop waits on. #74's shape stays.
|
||||
|
||||
- **R8:** both new cases fail, the exit-code case with exit 3 and the read-once
|
||||
case after 65 polls. The other 52 pass.
|
||||
- **The state checked only at timeout:** the new read-once case fails (77 polls),
|
||||
alongside the MERGED read-once case and the `CONFLICTING/DIRTY` exit-code case.
|
||||
|
||||
### The pr-watch ended-PR ordering cases — `scripts/pr-watch-test.sh`
|
||||
|
||||
improvise-24lz, found reviewing #253 at `b13355c7`. That head checked `state`
|
||||
|
||||
@@ -160,22 +160,22 @@ gap_state "$work/blocked-red" last.json MERGEABLE BLOCKED \
|
||||
run_case "a failed required check under MERGEABLE/BLOCKED fails without waiting" 1 \
|
||||
"$work/blocked-red" --timeout 0
|
||||
|
||||
# improvise-1qaa. A merged PR reports UNKNOWN/UNKNOWN, which was waited on for
|
||||
# the full timeout. Each fixture is the shape GitHub returned live (#189, #74).
|
||||
# improvise-1qaa/29w2. Each ended fixture is a shape GitHub returned live (#189,
|
||||
# #74, #46); the last is the one a waiting loop would sit on.
|
||||
# $1 dir, $2 file, $3 head, $4 state, $5 mergeable, $6 mergeStateStatus
|
||||
ended() {
|
||||
mkdir -p "$1"
|
||||
printf '{"headRefOid":"%s","state":"%s","mergeable":"%s","mergeStateStatus":"%s","statusCheckRollup":%s}\n' \
|
||||
"$3" "$4" "$5" "$6" "$green_rollup" > "$1/$2"
|
||||
}
|
||||
for shape in "MERGED UNKNOWN UNKNOWN" "CLOSED CONFLICTING DIRTY"; do
|
||||
for shape in "MERGED UNKNOWN UNKNOWN" "CLOSED CONFLICTING DIRTY" "CLOSED MERGEABLE BLOCKED"; do
|
||||
set -- $shape
|
||||
d="$work/ended-$1"
|
||||
d="$work/ended-$1-$3"
|
||||
ended "$d" last.json aaaaaaaaaaaa "$1" "$2" "$3"
|
||||
run_case "a $1 PR ends the watch as no longer open" 5 "$d" --timeout 5
|
||||
run_case "a $1 PR at $2/$3 ends the watch as no longer open" 5 "$d" --timeout 5
|
||||
polls="$(cat "$d/.poll" 2>/dev/null || echo 0)"
|
||||
[ "$polls" = 1 ] && ok "a $1 PR is read once, not waited on" \
|
||||
|| bad "a $1 PR is read once, not waited on (polls)" "$polls" 1
|
||||
[ "$polls" = 1 ] && ok "a $1 PR at $2/$3 is read once, not waited on" \
|
||||
|| bad "a $1 PR at $2/$3 is read once, not waited on (polls)" "$polls" 1
|
||||
done
|
||||
|
||||
# improvise-24lz. A merge at a head the watcher never saw is the PR #67 hazard,
|
||||
|
||||
Reference in New Issue
Block a user