Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 43 additions & 2 deletions generator/tests/test_review_runs_corrections.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,17 @@
[ -n "${SEARCH_FAILS:-}" ] && { echo "API rate limit exceeded" >&2; exit 1; }
emit "$(cat "$CANDIDATES_JSON")" ;;
"pr list"*) emit "$(cat "$BOT_PRS_JSON")" ;;
*"/issues/comments"*) emit "$(cat "$ISSUE_COMMENTS_JSON")" ;;
*"/issues/comments"*)
[ -n "${ISSUE_COMMENTS_FAILS:-}" ] && { echo "502 Bad Gateway" >&2; exit 1; }
emit "$(cat "$ISSUE_COMMENTS_JSON")" ;;
*"/pulls/comments"*) emit "$(cat "$PR_COMMENTS_JSON")" ;;
*"/pulls/"*"/reviews"*) emit "$(cat "$REVIEWS_JSON")" ;;
# Unset, the placeholder matches no numeric PR path.
*"/pulls/"*"/reviews"*)
case "$*" in
*"/pulls/${REVIEWS_FAIL_FOR:-none}/reviews"*)
echo "502 Bad Gateway" >&2; exit 1 ;;
esac
emit "$(cat "$REVIEWS_JSON")" ;;
*) exit 1 ;;
esac
"""
Expand Down Expand Up @@ -157,6 +165,39 @@ def test_a_failed_candidate_search_aborts_rather_than_reporting_no_reviews(
assert result.stdout == ""


def test_a_failed_comment_endpoint_aborts_rather_than_reporting_the_other_half(
env: dict[str, str],
) -> None:
"""Both comment endpoints are read in one loop inside a command substitution.

Bash applies `-e` there only under `inherit_errexit`, and `pipefail` sees
just the loop's final iteration — so a dropped `issues` page would leave
every conversation comment out of a report that still exits 0.
"""
_write(env, "PR_COMMENTS_JSON", [_comment(IN_WINDOW)])
env["ISSUE_COMMENTS_FAILS"] = "1"

result = _run(env, SINCE)

assert result.returncode != 0
assert result.stdout == ""


def test_a_failed_review_fetch_aborts_rather_than_dropping_that_prs_reviews(
env: dict[str, str],
) -> None:
"""One `gh` call per candidate PR, in that same loop: a failure on any but
the last would drop that PR's reviews and report the rest as the window."""
_write(env, "CANDIDATES_JSON", [{"number": 1}, {"number": 2}])
_write(env, "REVIEWS_JSON", [_review(IN_WINDOW)])
env["REVIEWS_FAIL_FOR"] = "1"

result = _run(env, SINCE)

assert result.returncode != 0
assert result.stdout == ""


def test_a_quiet_window_reports_empty_lists(env: dict[str, str]) -> None:
out = _collect(env)

Expand Down
14 changes: 9 additions & 5 deletions plugins/tend-ci-runner/scripts/review-runs-corrections.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@
# env: GITHUB_REPOSITORY (optional; falls back to the checkout's remote)

set -euo pipefail
# Bash does not apply `-e` inside a command substitution unless this is on,
# and every collection below is one. Without it a `gh` call that fails in any
# but the last iteration of a loop is dropped from the JSON and the run
# reports the remainder as the whole window.
shopt -s inherit_errexit

SINCE="${1:-}"
if [ -z "$SINCE" ]; then
Expand Down Expand Up @@ -65,11 +70,10 @@ COMMENTS=$(
# wraps around a standalone inline reply, which would otherwise report a
# correction on every thread where anyone replied inline.
#
# The candidate list is its own assignment because `set -e` checks an
# assignment's command substitution but not a `for` word list's: inline, a
# failed `--search` — the search API's 30 req/min, not the 5000/h core budget —
# would yield an empty list, skip the loop, and report `reviews: []` as an
# all-clear.
# The candidate list is its own assignment because a `for` word list is the one
# place `inherit_errexit` does not reach: inline, a failed `--search` — the
# search API's 30 req/min, not the 5000/h core budget — would yield an empty
# list, skip the loop, and report `reviews: []` as an all-clear.
CANDIDATES=$(gh pr list --repo "$REPO" --state all --limit 200 \
--search "updated:>=$SINCE" --json number --jq '.[].number')
REVIEWS=$(
Expand Down
Loading