Skip to content

Commit 3d42e54

Browse files
claudeclaude
committed
Refactor ancestry checks for cleaner abstraction
Extract is_base_ancestor() for simple ancestry checks that work in all modes. Rename skip_if_clean to has_squash_commit for clarity - it returns a boolean, not performs an action. update_indirect_target now uses is_base_ancestor() since indirect targets only need to verify their parent is already an ancestor. has_squash_commit remains for direct targets in squash-merge mode, where checking both base ancestry AND squash commit presence is needed. Also add feature4 branch to e2e tests to verify grandchildren are properly updated during conflict resolution continuation workflow.
1 parent 039371d commit 3d42e54

2 files changed

Lines changed: 73 additions & 11 deletions

File tree

‎tests/test_e2e.sh‎

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
# Tests the happy path where PRs are merged without conflicts.
2121
#
2222
# Setup:
23-
# - Create a stack of 3 PRs: main <- feature1 <- feature2 <- feature3
23+
# - Create a stack of 4 PRs: main <- feature1 <- feature2 <- feature3 <- feature4
2424
# - Each PR modifies line 2 of file.txt (same line, different content)
2525
#
2626
# Action Trigger:
@@ -30,14 +30,14 @@
3030
# - The action should detect that PR2 (feature2) was based on feature1
3131
# - Update PR2's base branch from feature1 to main
3232
# - Merge main into feature2 to incorporate the squash commit
33-
# - Propagate the merge to feature3 as well
33+
# - Propagate the merge to feature3 and feature4 as well
3434
# - Delete the merged branch (feature1)
3535
#
3636
# Verifications:
3737
# - feature1 branch is deleted from remote
3838
# - PR2 base branch is updated from feature1 to main
3939
# - PR3 base branch remains feature2 (only direct children are updated)
40-
# - feature2 and feature3 branches contain the squash merge commit
40+
# - feature2, feature3, and feature4 branches contain the squash merge commit
4141
# - PR diffs show the correct changes relative to their new bases
4242
#
4343
# SCENARIO 2: Conflict Handling (Steps 8-13)
@@ -47,6 +47,7 @@
4747
# Setup:
4848
# - After Scenario 1, modify line 7 on feature3 and push
4949
# - Also modify line 7 on main with different content (creating a conflict)
50+
# - feature4 (grandchild) exists based on feature3
5051
#
5152
# Action Trigger:
5253
# - Squash merge PR2 (feature2) into main
@@ -67,13 +68,19 @@
6768
# - Conflict label "autorestack-needs-conflict-resolution" exists on PR3
6869
# - feature3 branch was NOT updated (still at pre-conflict SHA)
6970
#
70-
# Manual Conflict Resolution (Steps 12-14):
71+
# Manual Conflict Resolution (Steps 12-15):
7172
# - Test simulates user resolving the conflict manually
7273
# - Merge main into feature3, resolve conflict (keep feature3's changes)
7374
# - Push the resolved branch
7475
# - The push triggers the 'synchronize' event on PR3
7576
# - The action detects the conflict label and removes it
76-
# - Verify the label is removed and resolution comment is posted
77+
# - The continuation workflow updates feature4 (grandchild) recursively
78+
# - Verify the label is removed, resolution comment is posted, and feature4 is updated
79+
#
80+
# Grandchild Update (feature4):
81+
# - Tests that update_branch_recursive properly handles grandchildren
82+
# - Even when SQUASH_COMMIT is undefined (in conflict-resolved mode)
83+
# - The skip_if_clean guard must handle the missing SQUASH_COMMIT ref
7784
#
7885
# =============================================================================
7986
set -e # Exit immediately if a command exits with a non-zero status.
@@ -487,6 +494,16 @@ PR3_URL=$(log_cmd gh pr create --repo "$REPO_FULL_NAME" --base feature2 --head f
487494
PR3_NUM=$(echo "$PR3_URL" | awk -F'/' '{print $NF}')
488495
echo >&2 "Created PR #$PR3_NUM: $PR3_URL"
489496

497+
# Branch feature4 (base: feature3) - tests grandchildren in conflict resolution
498+
log_cmd git checkout -b feature4 feature3
499+
sed -i '2s/.*/Feature 4 content line 2/' file.txt # Edit line 2 again
500+
log_cmd git add file.txt
501+
log_cmd git commit -m "Add feature 4"
502+
log_cmd git push origin feature4
503+
PR4_URL=$(log_cmd gh pr create --repo "$REPO_FULL_NAME" --base feature3 --head feature4 --title "Feature 4" --body "This is PR 4, based on PR 3 (grandchild for conflict resolution test)")
504+
PR4_NUM=$(echo "$PR4_URL" | awk -F'/' '{print $NF}')
505+
echo >&2 "Created PR #$PR4_NUM: $PR4_URL"
506+
490507
# --- Initial Merge Scenario ---
491508
echo >&2 "--- Testing Initial Merge (PR1) ---"
492509

@@ -540,6 +557,8 @@ log_cmd git checkout feature2 # Checkout local branch first
540557
log_cmd git pull origin feature2 # Pull updates pushed by the action
541558
log_cmd git checkout feature3
542559
log_cmd git pull origin feature3
560+
log_cmd git checkout feature4
561+
log_cmd git pull origin feature4
543562

544563
# Check ancestry
545564
if log_cmd git merge-base --is-ancestor "$MERGE_COMMIT_SHA1" feature2; then
@@ -556,6 +575,13 @@ else
556575
log_cmd git log --graph --oneline feature3 main
557576
exit 1
558577
fi
578+
if log_cmd git merge-base --is-ancestor "$MERGE_COMMIT_SHA1" feature4; then
579+
echo >&2 "✅ Verification Passed: feature4 correctly incorporates the squash commit $MERGE_COMMIT_SHA1."
580+
else
581+
echo >&2 "❌ Verification Failed: feature4 does not include the squash commit $MERGE_COMMIT_SHA1."
582+
log_cmd git log --graph --oneline feature4 main
583+
exit 1
584+
fi
559585
# Verify diffs (using triple-dot diff against the *new* base: main)
560586
echo >&2 "Verifying diff content for updated PRs..."
561587
# Expected diff for feature2 vs main (should only contain feature2 changes relative to feature1)
@@ -587,6 +613,20 @@ else
587613
exit 1
588614
fi
589615

616+
# Expected diff for feature4 vs feature3 (should only contain feature4 changes relative to feature3)
617+
EXPECTED_DIFF4_CONTENT="Feature 4 content line 2"
618+
ACTUAL_DIFF4_CONTENT=$(log_cmd gh pr diff "$PR4_URL" --repo "$REPO_FULL_NAME" | grep '^+Feature 4' | sed 's/^+//')
619+
620+
if [[ "$ACTUAL_DIFF4_CONTENT" == "$EXPECTED_DIFF4_CONTENT" ]]; then
621+
echo >&2 "✅ Verification Passed: Diff content for PR #$PR4_NUM seems correct."
622+
else
623+
echo >&2 "❌ Verification Failed: Diff content for PR #$PR4_NUM is incorrect."
624+
echo "Expected Added Line Content: $EXPECTED_DIFF4_CONTENT"
625+
echo "Actual Added Line Content: $ACTUAL_DIFF4_CONTENT"
626+
gh pr diff "$PR4_URL" --repo "$REPO_FULL_NAME"
627+
exit 1
628+
fi
629+
590630
echo >&2 "--- Initial Merge Test Completed Successfully ---"
591631

592632

@@ -765,6 +805,8 @@ echo >&2 "15. Verifying conflict resolution content..."
765805
log_cmd git fetch origin
766806
log_cmd git checkout feature3
767807
log_cmd git pull origin feature3
808+
log_cmd git checkout feature4
809+
log_cmd git pull origin feature4
768810

769811
# Verify feature3 now incorporates main (including PR2 merge commit and main's conflict commit)
770812
if log_cmd git merge-base --is-ancestor origin/main feature3; then
@@ -775,6 +817,16 @@ else
775817
exit 1
776818
fi
777819

820+
# Verify feature4 (grandchild) was updated by continuation workflow
821+
# This tests that update_branch_recursive properly handles grandchildren even when SQUASH_COMMIT is undefined
822+
if log_cmd git merge-base --is-ancestor origin/feature3 feature4; then
823+
echo >&2 "✅ Verification Passed: feature4 (grandchild) correctly incorporates resolved feature3."
824+
else
825+
echo >&2 "❌ Verification Failed: feature4 does not include the resolved feature3."
826+
log_cmd git log --graph --oneline feature4 feature3
827+
exit 1
828+
fi
829+
778830
# Verify the final content of file.txt on feature3
779831
# Line 1: Original base
780832
# Line 2: From feature 3 commit ("Feature 3 content line 2")

‎update-pr-stack.sh‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,19 @@ check_env_var() {
2626
fi
2727
}
2828

29-
skip_if_clean() {
29+
# Check if BASE is already an ancestor of BRANCH (simple merge check)
30+
is_base_ancestor() {
3031
local BRANCH="$1"
3132
local BASE="$2"
32-
# If BASE is already an ancestor of BRANCH *and*
33-
# the squash commit is already in history, we're done.
34-
git merge-base --is-ancestor "origin/$BASE" "origin/$BRANCH" \
33+
git merge-base --is-ancestor "origin/$BASE" "origin/$BRANCH"
34+
}
35+
36+
# Check if a branch already has the squash commit merged (squash-merge mode only)
37+
# Requires SQUASH_COMMIT ref to be set via git update-ref
38+
has_squash_commit() {
39+
local BRANCH="$1"
40+
local BASE="$2"
41+
is_base_ancestor "$BRANCH" "$BASE" \
3542
&& git merge-base --is-ancestor SQUASH_COMMIT "origin/$BRANCH"
3643
}
3744

@@ -50,7 +57,7 @@ update_direct_target() {
5057
local BRANCH="$1"
5158
local BASE_BRANCH="$2"
5259

53-
if skip_if_clean "$BRANCH" "$TARGET_BRANCH"; then
60+
if has_squash_commit "$BRANCH" "$TARGET_BRANCH"; then
5461
echo "✓ $BRANCH already up-to-date; skipping"
5562
return
5663
fi
@@ -108,7 +115,10 @@ update_indirect_target() {
108115
local BRANCH="$1"
109116
local BASE_BRANCH="$2"
110117

111-
if skip_if_clean "$BRANCH" "$BASE_BRANCH"; then
118+
# For indirect targets, we only need to check if the parent is already
119+
# an ancestor. If so, the branch already has all updates from the parent.
120+
# (No SQUASH_COMMIT check needed - that's only for direct targets)
121+
if is_base_ancestor "$BRANCH" "$BASE_BRANCH"; then
112122
echo "✓ $BRANCH already up-to-date with $BASE_BRANCH; skipping"
113123
return
114124
fi

0 commit comments

Comments
 (0)