Skip to content
Open
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
82 changes: 82 additions & 0 deletions bert_e/tests/test_bert_e.py
Original file line number Diff line number Diff line change
Expand Up @@ -3768,6 +3768,88 @@ def test_integration_branch_and_source_branch_force_updated(self):
with self.assertRaises(exns.BranchHistoryMismatch):
self.handle(pr.id, options=self.bypass_all, backtrace=True)

Comment thread
delthas marked this conversation as resolved.
def test_history_mismatch_nondiverged_dev_branch(self):
"""A manually-resolved integration branch on a development branch that
has not diverged from its parent must not trigger a false
BranchHistoryMismatch.

development/4.4 is branched off development/4.3 with no commit of its
own, so both point at the exact same commit ``D``. A PR targeting
development/4.3 cascades into development/4.4. When the w/4.4
integration branch is manually resolved with a forced merge commit (as
an operator does when the higher development branch must ignore the
cascaded change, e.g. a `merge -s ours`), that merge commit's first
parent is ``D`` -- the commit shared with development/4.3. ``D`` is
part of the destination development branch and must be accepted.

"""
# development/4.4 == development/4.3 (no commit of its own)
self.gitrepo.cmd('git checkout development/4.3')
self.gitrepo.cmd('git checkout -b development/4.4')
self.gitrepo.cmd('git push -u origin development/4.4')
Comment on lines +3787 to +3789

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could pull that into a tiny helper (e.g. branch_nondiverged_dev('4.3', '4.4')) so the two tests only differ on the part that matters (manual merge vs late-PR auto-merge). Not blocking tho


pr = self.create_pr('bugfix/TEST-00001', 'development/4.3')
# First evaluation creates and pushes the integration branches.
with self.assertRaises(exns.BuildNotStarted):
self.handle(pr.id,
options=self.bypass_all_but(['bypass_build_status']),
backtrace=True)

# Manually resolve w/4.4 with a forced merge commit whose first parent
# is development/4.4's tip (== development/4.3's tip == D).
self.gitrepo.cmd('git fetch --all')
self.gitrepo.cmd('git checkout w/4.4/bugfix/TEST-00001')
self.gitrepo.cmd('git reset --hard development/4.4')
self.gitrepo.cmd('git merge --no-ff origin/bugfix/TEST-00001 '
'-m "manual resolution of w/4.4"')
self.gitrepo.cmd('git push -f origin w/4.4/bugfix/TEST-00001')

# Re-evaluation runs the history check on the manual merge commit. It
# must not raise a false BranchHistoryMismatch.
with self.assertRaises(exns.SuccessMessage):
self.handle(pr.id, options=self.bypass_all, backtrace=True)

def test_history_mismatch_nondiverged_dev_auto_integration(self):
"""When development/x.y+1 has not diverged from development/x.y (same
tip D) and the PR's feature branch was branched one commit before D,
bert-e auto-creates a w/x.y+1 integration branch via a real merge
commit (feature and D are siblings, not fast-forwardable). The merge
commit's first parent is D, which the git-log boundary excludes from
acceptable_parents. On re-evaluation the history check must accept D
as an ancestor of development/x.y+1 and not raise a false
BranchHistoryMismatch.

This exercises the same fix as
test_history_mismatch_nondiverged_dev_branch but via the auto-created
path rather than manual operator intervention.

"""
# development/4.4 == development/4.3 (no commit of its own)
self.gitrepo.cmd('git checkout development/4.3')
self.gitrepo.cmd('git checkout -b development/4.4')
self.gitrepo.cmd('git push -u origin development/4.4')

# Feature branch created one commit behind development/4.3's tip so
# that the merge for w/4.4 cannot fast-forward and produces a real
# merge commit whose first parent is development/4.4's tip (==
# development/4.3's tip).
parent_sha = self.gitrepo.cmd('git rev-parse development/4.3^').strip()
create_branch(self.gitrepo, 'bugfix/TEST-00001',
from_branch=parent_sha, file_=True)
pr = self.create_pr('bugfix/TEST-00001', 'development/4.3',
reuse_branch=True)

# First evaluation creates and pushes the integration branches.
with self.assertRaises(exns.BuildNotStarted):
self.handle(pr.id,
options=self.bypass_all_but(['bypass_build_status']),
backtrace=True)

# Re-evaluation runs the history check on the auto-created merge
# commit. It must not raise a false BranchHistoryMismatch.
with self.assertRaises(exns.SuccessMessage):
self.handle(pr.id, options=self.bypass_all, backtrace=True)

def test_success_message_content(self):
pr = self.create_pr('bugfix/TEST-00001', 'development/5.1')
try:
Expand Down
11 changes: 10 additions & 1 deletion bert_e/workflow/gitwaterflow/integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,18 @@ def update_integration_branches(job, wbranches):
# * the wbranch,
# * the previous integration branch,
# * the target development branch.
#
# A `<dev>..<branch>` diff excludes the boundary commit, so when the
# destination development branch has not diverged from its parent (it
# points at the same commit), the shared tip is in none of the sets
# above even though it legitimately belongs to the target development
# branch. Accept any parent that is an ancestor of the destination
# development branch to cover that case.
acceptable_parents = prev_set | dst_set | wbranch_set
for rev in wbranch_set:
if not all(p in acceptable_parents for p in rev.parents):
if not all(p in acceptable_parents or
wbranch.dst_branch.includes_commit(p)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why includes_commit rather than just adding the dev tip to dst_set (the boundary that .. drops)? I think includes_commit is the better choice , just confirming it's deliberate and that wbranch_set stays small enough that a per-parent merge-base is a non-issue.

for p in rev.parents):
raise exceptions.BranchHistoryMismatch(
integration_branch=wbranch, feature_branch=feature_branch,
development_branch=wbranch.dst_branch, commit=rev,
Expand Down
Loading