Edward Langley
051f54111b
Both found by ChatGPT Codex on PR #133, both invisible on macOS, both arriving as exit 2 with no output -- jq's answer to an invalid --argjson, so a shell fault at the top of the script wears jq's exit code. `local n="$1" name="$2" f="$DIR/$n.$name.json"` creates every name before running the assignments on bash 5, so $n is unbound in the third clause and set -u kills the shell. The dead command substitution then hands --argjson an empty string. Works on macOS bash 3.2: the suite passed 19/19 locally and 2/19 in CI on the same commit. Each local now gets its own statement. `gh api --paginate` emits one JSON array PER PAGE, so a comment thread past the page size is several concatenated documents that --argjson rejects the same way. Both fetch paths now flatten through one pages_to_array (`jq -s 'add // []'`), which is a no-op on a single page. The fixture path shares that helper, which is the only reason a fixture can test it: a fixture holding a single array would exercise a shape the live fetch never produces. Three cases added, and removing the helper from the fixture path fails exactly those three, the wait case with the live exit-2 signature. Suite is 22 cases. No PR in this repo has yet had enough inline comments to reach a second page, so nothing but the test will see that one before a user does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
240 lines
9.6 KiB
Bash
Executable File
240 lines
9.6 KiB
Bash
Executable File
#!/usr/bin/env bash
|
|
# check_the_checker for scripts/pr-reviews. Fixture-driven, no network.
|
|
# The case that matters is improvise-xgra: a reviewer whose substance is
|
|
# entirely inline. See context/test-evidence/tooling.md.
|
|
set -uo pipefail
|
|
|
|
here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
|
REVIEWS="$here/pr-reviews"
|
|
work="$(mktemp -d)"
|
|
trap 'rm -rf "$work"' EXIT
|
|
|
|
pass=0; fail=0
|
|
ok() { printf ' ok %s\n' "$1"; pass=$((pass+1)); }
|
|
bad() { printf ' FAIL %s (%s)\n' "$1" "$2"; fail=$((fail+1)); }
|
|
|
|
# $1 name, $2 expected exit, $3 fixture dir, rest: extra args. Output goes to
|
|
# $work/out so a case may assert on what was PRINTED as well as on $?; nothing
|
|
# follows the script in its own pipeline, which is the rule its header states.
|
|
run_case() {
|
|
local name="$1" want="$2" dir="$3"; shift 3
|
|
rm -f "$dir/.poll"
|
|
PR_REVIEWS_FIXTURE_DIR="$dir" "$REVIEWS" 1 "$@" > "$work/out" 2>&1
|
|
local got=$?
|
|
[ "$got" = "$want" ] && ok "$name" || bad "$name" "exit $got, wanted $want"
|
|
}
|
|
|
|
# $1 name, $2 fixture dir, $3 grep -E pattern the output must contain
|
|
prints() {
|
|
local name="$1" dir="$2" pat="$3"
|
|
rm -f "$dir/.poll"
|
|
PR_REVIEWS_FIXTURE_DIR="$dir" "$REVIEWS" 1 > "$work/out" 2>&1
|
|
grep -Eq "$pat" "$work/out" && ok "$name" || bad "$name" "no /$pat/ in output"
|
|
}
|
|
|
|
mk() { mkdir -p "$1"; cat > "$1/$2"; }
|
|
|
|
HEAD=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
|
|
OLD=bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# The bug this script exists for. The review BODY is a one-liner at an older
|
|
# head; every ruling is an inline comment at the current head. A pr-reviews
|
|
# that reads only .reviews prints the one-liner, reports exit 1, and the
|
|
# rulings are invisible -- which is what happened on PR #122.
|
|
d="$work/inline-at-head"
|
|
mk "$d" last.pr.json <<EOF
|
|
{"headRefOid":"$HEAD","reviews":[
|
|
{"state":"COMMENTED","submittedAt":"2026-09-22T10:00:00Z","author":{"login":"owner"},
|
|
"commit":{"oid":"$OLD"},"body":"Generally good, added comments inline."}]}
|
|
EOF
|
|
mk "$d" last.inline.json <<EOF
|
|
[{"created_at":"2026-09-22T10:00:05Z","user":{"login":"owner"},"commit_id":"$HEAD",
|
|
"path":"crates/improvise-core/src/model/types.rs","line":412,
|
|
"body":"Use Aggregation::Exact here, not the none_cats list."}]
|
|
EOF
|
|
mk "$d" last.issue.json <<'EOF'
|
|
[]
|
|
EOF
|
|
run_case "an inline comment at the current head satisfies --wait" 0 "$d" --wait 1 --interval 1
|
|
prints "an inline comment's body is printed" "$d" 'Aggregation::Exact'
|
|
prints "an inline comment names its file and line" "$d" 'types\.rs:412'
|
|
prints "an inline comment at the head is marked CURRENT HEAD" "$d" 'types\.rs:412 commit=aaaaaaaa <- CURRENT HEAD'
|
|
prints "a review body at an earlier head still says so" "$d" 'commit=bbbbbbbb <- an EARLIER head'
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# The mirror-image fault: an inline comment about a tree that no longer
|
|
# exists must NOT satisfy the wait. A version that counts inline comments
|
|
# rather than comparing their commit passes the case above and this one
|
|
# wrongly, so the pair is what discriminates.
|
|
d="$work/inline-at-old"
|
|
mk "$d" last.pr.json <<EOF
|
|
{"headRefOid":"$HEAD","reviews":[]}
|
|
EOF
|
|
mk "$d" last.inline.json <<EOF
|
|
[{"created_at":"2026-09-22T09:00:00Z","user":{"login":"owner"},"commit_id":"$OLD",
|
|
"path":"src/draw.rs","line":10,"body":"stale finding"}]
|
|
EOF
|
|
mk "$d" last.issue.json <<'EOF'
|
|
[]
|
|
EOF
|
|
run_case "an inline comment at an EARLIER head does not satisfy --wait" 1 "$d" --wait 1 --interval 1
|
|
prints "an earlier-head inline comment is still printed, flagged" "$d" \
|
|
'draw\.rs:10 commit=bbbbbbbb <- an EARLIER head'
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# The contract that must not change: a review body at the head still counts.
|
|
d="$work/review-at-head"
|
|
mk "$d" last.pr.json <<EOF
|
|
{"headRefOid":"$HEAD","reviews":[
|
|
{"state":"APPROVED","submittedAt":"2026-09-22T11:00:00Z","author":{"login":"owner"},
|
|
"commit":{"oid":"$HEAD"},"body":"LGTM"}]}
|
|
EOF
|
|
mk "$d" last.inline.json <<'EOF'
|
|
[]
|
|
EOF
|
|
mk "$d" last.issue.json <<'EOF'
|
|
[]
|
|
EOF
|
|
run_case "a review body at the current head still satisfies --wait" 0 "$d" --wait 1 --interval 1
|
|
prints "a review prints its state" "$d" '\[REVIEW APPROVED\]'
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# An issue-level comment is attached to no commit, so it makes no claim about
|
|
# which tree was read and cannot satisfy the wait. Widening --wait to "any
|
|
# text at all" would pass the first case and fail this one.
|
|
d="$work/issue-only"
|
|
mk "$d" last.pr.json <<EOF
|
|
{"headRefOid":"$HEAD","reviews":[]}
|
|
EOF
|
|
mk "$d" last.inline.json <<'EOF'
|
|
[]
|
|
EOF
|
|
mk "$d" last.issue.json <<'EOF'
|
|
[{"created_at":"2026-09-22T12:00:00Z","user":{"login":"owner"},
|
|
"body":"rebasing this shortly"}]
|
|
EOF
|
|
run_case "an issue-level comment does not satisfy --wait" 1 "$d" --wait 1 --interval 1
|
|
prints "an issue-level comment is still printed" "$d" '\[COMMENT\].*<- not attached to a commit'
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Time order across the three surfaces. A concatenation that prints each
|
|
# surface in its own block passes every case above and fails this one.
|
|
d="$work/interleaved"
|
|
mk "$d" last.pr.json <<EOF
|
|
{"headRefOid":"$HEAD","reviews":[
|
|
{"state":"COMMENTED","submittedAt":"2026-09-22T10:00:00Z","author":{"login":"owner"},
|
|
"commit":{"oid":"$HEAD"},"body":"SECOND"}]}
|
|
EOF
|
|
mk "$d" last.inline.json <<EOF
|
|
[{"created_at":"2026-09-22T11:00:00Z","user":{"login":"owner"},"commit_id":"$HEAD",
|
|
"path":"a.rs","line":1,"body":"THIRD"}]
|
|
EOF
|
|
mk "$d" last.issue.json <<'EOF'
|
|
[{"created_at":"2026-09-22T09:00:00Z","user":{"login":"owner"},"body":"FIRST"}]
|
|
EOF
|
|
rm -f "$d/.poll"
|
|
PR_REVIEWS_FIXTURE_DIR="$d" "$REVIEWS" 1 > "$work/out" 2>&1
|
|
order=$(grep -oE 'FIRST|SECOND|THIRD' "$work/out" | tr '\n' ' ')
|
|
[ "$order" = "FIRST SECOND THIRD " ] \
|
|
&& ok "the three surfaces interleave in time order" \
|
|
|| bad "the three surfaces interleave in time order" "got: $order"
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# An outdated comment has `line: null` and only `original_line` left. A
|
|
# renderer that reads `.line` alone prints "a.rs:null" and loses the location.
|
|
d="$work/outdated-line"
|
|
mk "$d" last.pr.json <<EOF
|
|
{"headRefOid":"$HEAD","reviews":[]}
|
|
EOF
|
|
mk "$d" last.inline.json <<EOF
|
|
[{"created_at":"2026-09-22T10:00:00Z","user":{"login":"owner"},"commit_id":"$HEAD",
|
|
"path":"src/draw.rs","line":null,"original_line":77,"body":"moved since"}]
|
|
EOF
|
|
mk "$d" last.issue.json <<'EOF'
|
|
[]
|
|
EOF
|
|
prints "an outdated inline comment falls back to original_line" "$d" 'draw\.rs:77'
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Nothing on any surface, and the two shapes of that.
|
|
d="$work/empty"
|
|
mk "$d" last.pr.json <<EOF
|
|
{"headRefOid":"$HEAD","reviews":[]}
|
|
EOF
|
|
mk "$d" last.inline.json <<'EOF'
|
|
[]
|
|
EOF
|
|
mk "$d" last.issue.json <<'EOF'
|
|
[]
|
|
EOF
|
|
run_case "--wait 0 over an empty PR is success" 0 "$d"
|
|
run_case "--wait over an empty PR times out" 1 "$d" --wait 1 --interval 1
|
|
prints "an empty PR says so" "$d" 'nothing written on any of the three surfaces'
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# An inline comment that arrives on a later poll. A script that fetched once
|
|
# and then only re-read the first response would sit here until the deadline.
|
|
d="$work/arrives-later"
|
|
mk "$d" 1.pr.json <<EOF
|
|
{"headRefOid":"$HEAD","reviews":[]}
|
|
EOF
|
|
mk "$d" 1.inline.json <<'EOF'
|
|
[]
|
|
EOF
|
|
mk "$d" 1.issue.json <<'EOF'
|
|
[]
|
|
EOF
|
|
mk "$d" last.pr.json <<EOF
|
|
{"headRefOid":"$HEAD","reviews":[]}
|
|
EOF
|
|
mk "$d" last.inline.json <<EOF
|
|
[{"created_at":"2026-09-22T10:00:00Z","user":{"login":"owner"},"commit_id":"$HEAD",
|
|
"path":"a.rs","line":3,"body":"arrived on the second poll"}]
|
|
EOF
|
|
mk "$d" last.issue.json <<'EOF'
|
|
[]
|
|
EOF
|
|
run_case "an inline comment arriving mid-wait is seen" 0 "$d" --wait 1 --interval 1
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# `gh api --paginate` emits one array PER PAGE, so a thread past the page size
|
|
# arrives as several concatenated documents. Passing that straight to
|
|
# --argjson is a jq usage error -- exit 2, no output, every case dead. The
|
|
# fixture carries two concatenated arrays because both paths flatten through
|
|
# the same pages_to_array; a fixture that could only hold one would be testing
|
|
# a code path the live fetch does not take.
|
|
d="$work/paginated"
|
|
mk "$d" last.pr.json <<EOF
|
|
{"headRefOid":"$HEAD","reviews":[]}
|
|
EOF
|
|
mk "$d" last.inline.json <<EOF
|
|
[{"created_at":"2026-09-22T10:00:00Z","user":{"login":"owner"},"commit_id":"$HEAD",
|
|
"path":"page1.rs","line":1,"body":"FIRSTPAGE"}]
|
|
[{"created_at":"2026-09-22T10:00:01Z","user":{"login":"owner"},"commit_id":"$HEAD",
|
|
"path":"page2.rs","line":2,"body":"SECONDPAGE"}]
|
|
EOF
|
|
mk "$d" last.issue.json <<'EOF'
|
|
[]
|
|
EOF
|
|
prints "a comment on the first page survives pagination" "$d" 'page1\.rs:1'
|
|
prints "a comment on the second page survives pagination" "$d" 'page2\.rs:2'
|
|
run_case "a paginated thread still satisfies --wait" 0 "$d" --wait 1 --interval 1
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Usage is exit 4 and must stay there: briefs distinguish it from 1.
|
|
rm -f "$work/empty/.poll"
|
|
PR_REVIEWS_FIXTURE_DIR="$work/empty" "$REVIEWS" > "$work/out" 2>&1
|
|
got=$?
|
|
[ "$got" = 4 ] && ok "no PR number is exit 4" || bad "no PR number is exit 4" "exit $got"
|
|
|
|
# The header must keep saying that nothing may follow this script, because the
|
|
# `; echo` half of that trap is the one an agent hit this week.
|
|
"$REVIEWS" --help > "$work/out" 2>&1
|
|
grep -q 'echo "exit=\$?"' "$work/out" \
|
|
&& ok "the usage warns about a trailing \$? reader" \
|
|
|| bad "the usage warns about a trailing \$? reader" "not in --help"
|
|
|
|
printf '\npr-reviews-test: %d passed, %d failed\n' "$pass" "$fail"
|
|
[ "$fail" -eq 0 ]
|