diff --git a/tests/mock_gh.sh b/tests/mock_gh.sh index 3114109..e957532 100755 --- a/tests/mock_gh.sh +++ b/tests/mock_gh.sh @@ -4,6 +4,10 @@ # Only direct children are queried now (no recursive updates of indirect children). if [[ "$1" == "pr" && "$2" == "list" ]]; then + if [[ "${MOCK_PR_LIST_FAIL:-}" == 1 ]]; then + echo "mock gh: pr list API down" >&2 + exit 1 + fi # Parse the --base argument to determine which PRs to return base="" for ((i=1; i<=$#; i++)); do diff --git a/tests/test_conflict_resolution_resume.sh b/tests/test_conflict_resolution_resume.sh index c0e1cec..b82bacf 100644 --- a/tests/test_conflict_resolution_resume.sh +++ b/tests/test_conflict_resolution_resume.sh @@ -23,6 +23,7 @@ ok() { echo "✅ $1"; PASS=$((PASS+1)); } # MOCK_LABELS newline-separated labels returned by `pr view --json labels` # MOCK_COMMENTS_FILE file whose contents are returned by `pr view --json comments` # MOCK_LABELS_FAIL set to 1 to make `pr view --json labels` fail +# MOCK_PR_LIST_FAIL set to 1 to make `pr list` fail # The PR's base branch is not mocked: the script must take it from PR_BASE # (event payload), so a baseRefName query is an unhandled call and fails. make_mock_gh() { @@ -48,6 +49,7 @@ elif [[ "$1 $2" == "pr comment" ]]; then elif [[ "$1 $2" == "pr edit" ]]; then : elif [[ "$1 $2" == "pr list" ]]; then + [[ "${MOCK_PR_LIST_FAIL:-}" == 1 ]] && { echo "mock gh: pr list API down" >&2; exit 1; } : # no sibling conflicts elif [[ "$1 $2" == "label create" ]]; then : @@ -99,6 +101,7 @@ run_resume() { GH="$MOCK_DIR/mock_gh.sh" GIT="$MOCK_DIR/mock_git.sh" \ MOCK_LABELS="$MOCK_LABELS" MOCK_LABELS_FAIL="${MOCK_LABELS_FAIL:-}" \ MOCK_COMMENTS_FILE="$MOCK_COMMENTS_FILE" CALLS="$CALLS" \ + MOCK_PR_LIST_FAIL="${MOCK_PR_LIST_FAIL:-}" \ bash "$ROOT_DIR/update-pr-stack.sh" >"$WORK/out.log" 2>&1 || echo "EXIT=$?" >>"$WORK/out.log" } @@ -254,5 +257,26 @@ grep -q "remove-label" "$CALLS" && fail "H: label must NOT be touched on an API [[ "$(git -C "$ORIGIN" rev-parse child)" == "$CHILD_BEFORE" ]] || fail "H: child was pushed" ok "H: labels API failure fails the run instead of skipping the resume" +# --------------------------------------------------------------------------- +echo "### Scenario I: listing sibling conflicts fails -> keep the old base branch" +setup_repo +# Same successful-resume setup as scenario C, but the sibling listing that +# decides whether the old base branch can be deleted fails. Answering "no +# siblings" there would delete a branch another conflicted PR still needs. +git -C "$WORK" merge -q --no-edit main +git -C "$WORK" push -q origin child +MOCK_LABELS="autorestack-needs-conflict-resolution" +MOCK_LABELS_FAIL="" +PR_BASE="parent" +MOCK_PR_LIST_FAIL=1 +MOCK_COMMENTS_FILE="$WORK/comments.txt" +{ echo "### conflict"; echo; marker parent main "$SQUASH"; } > "$MOCK_COMMENTS_FILE" +run_resume + +grep -q "EXIT=" "$WORK/out.log" || fail "I: run should have failed" +git -C "$ORIGIN" rev-parse --verify -q parent >/dev/null || fail "I: old base branch was deleted on an API failure" +grep -q -- "push origin :parent" "$CALLS" && fail "I: deletion must not be attempted" +ok "I: sibling-listing API failure keeps the old base branch" + echo echo "All conflict-resume tests passed 🎉 ($PASS scenarios)" diff --git a/tests/test_update_pr_stack.sh b/tests/test_update_pr_stack.sh index 9127597..265b8d8 100755 --- a/tests/test_update_pr_stack.sh +++ b/tests/test_update_pr_stack.sh @@ -208,6 +208,31 @@ else exit 1 fi +# A failed children listing must fail the run before any mutation: silently +# treating it as "no children" would delete the merged branch under the +# children it never saw. +echo -e "\nRunning update script with a failing pr list..." +FAIL_LOG="$TEST_REPO/update_fail_run.log" +if log_cmd env \ + SQUASH_COMMIT=$SQUASH_COMMIT \ + MERGED_BRANCH=feature1 \ + PR_NUMBER=1 \ + TARGET_BRANCH=main \ + MOCK_PR_LIST_FAIL=1 \ + GH="$SCRIPT_DIR/mock_gh.sh" \ + GIT="$SCRIPT_DIR/mock_git.sh" \ + $SCRIPT_DIR/../update-pr-stack.sh > "$FAIL_LOG" 2>&1; then + echo "❌ run must fail when the children cannot be listed" + cat "$FAIL_LOG" + exit 1 +fi +if grep -q "git push origin :feature1" "$FAIL_LOG"; then + echo "❌ merged branch must not be deleted when the children cannot be listed" + cat "$FAIL_LOG" + exit 1 +fi +echo "✅ Failing pr list fails the run without deleting the merged branch" + echo -e "\nAll tests passed! 🎉" # Clean up diff --git a/update-pr-stack.sh b/update-pr-stack.sh index a53d939..68a0291 100755 --- a/update-pr-stack.sh +++ b/update-pr-stack.sh @@ -45,6 +45,9 @@ format_state_marker() { } # Echoes the most recent state-marker line found in our PR comments, or nothing. +# Dies when the comments cannot be read at all: an API failure must not pass +# for "no marker", which the caller treats as a reason to give up the resume +# and remove the conflict label for good. read_state_marker() { local PR_NUMBER="$1" local BODIES @@ -140,9 +143,10 @@ is_rebase_merge() { } # Echoes " " for each open PR based on the merged branch. -# try, not run: callers consume this through a process substitution, which -# swallows the exit status either way; a die in here would only leave that -# subshell. Making the callers notice the failure is a separate concern. +# try, not run: callers run this in a command substitution, where a die would +# only leave the subshell, so they capture the output and die themselves. An +# unhandled failure here must not pass for "no children": the caller would +# then delete the merged branch under the children it never saw. list_child_prs() { try gh pr list --base "$MERGED_BRANCH" --json number,headRefName --jq '.[] | "\(.number) \(.headRefName)"' } @@ -268,13 +272,16 @@ pr_has_conflict_label() { # Check if any other PRs with conflict label still depend on a given base branch # Returns 0 (true) if siblings exist, 1 (false) if no siblings +# Dies when the PRs cannot be listed: answering "no siblings" on an API failure +# makes the caller delete a branch a sibling may still need for its resolution. has_sibling_conflicts() { local BASE_BRANCH="$1" local EXCLUDE_BRANCH="$2" # Find all open PRs with the conflict label that are based on BASE_BRANCH local CONFLICTED_SIBLINGS - CONFLICTED_SIBLINGS=$(gh pr list --base "$BASE_BRANCH" --label "$CONFLICT_LABEL" --json headRefName --jq '.[].headRefName' 2>/dev/null || echo "") + CONFLICTED_SIBLINGS=$(gh pr list --base "$BASE_BRANCH" --label "$CONFLICT_LABEL" --json headRefName --jq '.[].headRefName') \ + || die "could not list conflicted PRs based on $BASE_BRANCH" for SIBLING in $CONFLICTED_SIBLINGS; do if [[ "$SIBLING" != "$EXCLUDE_BRANCH" ]]; then @@ -408,10 +415,11 @@ main() { # children and delete the merged branch. if git rev-parse --verify --quiet SQUASH_COMMIT^2 >/dev/null; then echo "✓ '$MERGED_BRANCH' was merged with a merge commit, not squashed; retargeting children without touching their heads" + CHILDREN=$(list_child_prs) || die "could not list the PRs based on $MERGED_BRANCH" while read -r NUMBER BRANCH; do [[ -n "$BRANCH" ]] || continue run gh pr edit "$NUMBER" --base "$TARGET_BRANCH" - done < <(list_child_prs) + done <<<"$CHILDREN" # Deleting a PR's base branch closes the PR, so the retargets come first. run git push origin ":$MERGED_BRANCH" return 0 @@ -423,21 +431,23 @@ main() { # the intermediate copies. Tell the children and leave everything alone. if is_rebase_merge "$PR_NUMBER"; then echo "âš ī¸ '$MERGED_BRANCH' looks rebase-merged; rebase merges are not supported, leaving the stack alone" + CHILDREN=$(list_child_prs) || die "could not list the PRs based on $MERGED_BRANCH" while read -r NUMBER BRANCH; do [[ -n "$BRANCH" ]] || continue run gh pr comment "$NUMBER" --body "â„šī¸ The base branch \`$MERGED_BRANCH\` of this PR was merged with \"Rebase and merge\", which autorestack does not support. Update this PR manually. \`$MERGED_BRANCH\` was kept so this PR stays open." - done < <(list_child_prs) + done <<<"$CHILDREN" return 0 fi # Find all PRs directly targeting the merged PR's head + CHILDREN=$(list_child_prs) || die "could not list the PRs based on $MERGED_BRANCH" INITIAL_NUMBERS=() INITIAL_TARGETS=() while read -r NUMBER BRANCH; do [[ -n "$BRANCH" ]] || continue INITIAL_NUMBERS+=("$NUMBER") INITIAL_TARGETS+=("$BRANCH") - done < <(list_child_prs) + done <<<"$CHILDREN" # Track successfully updated vs conflicted branches separately UPDATED_TARGETS=()