From d1d51d2308790129203d403305f4f190fd7a63e2 Mon Sep 17 00:00:00 2001 From: K Jonathan Harker Date: Wed, 23 Aug 2017 11:57:40 -0400 Subject: [PATCH] Only depend-on open changes Instead of checking if changes found from a Depends-On line are merged, check if they are open. This ignores abandoned changes when building the list of dependent changes. In the event that change I0000 depends-on change-id I1234 and change-id I1234 matches multiple changes, one of which is abandoned, then change I0000 will never enter the gate as it waits indefinitely for the abandoned I1234 to become eligible for the gate. Change-Id: I06427df8631982f02796dd1e539c79ed088766cd --- tests/unit/test_scheduler.py | 18 +++++++++++------- zuul/driver/gerrit/gerritconnection.py | 9 ++++++--- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py index 97d53e0b70..0980d1f83f 100755 --- a/tests/unit/test_scheduler.py +++ b/tests/unit/test_scheduler.py @@ -3836,19 +3836,23 @@ For CI problems and help debugging, contact ci@example.org""" self.create_branch('org/project2', 'mp') A = self.fake_gerrit.addFakeChange('org/project1', 'master', 'A') B = self.fake_gerrit.addFakeChange('org/project2', 'master', 'B') - C = self.fake_gerrit.addFakeChange('org/project2', 'mp', 'C') - C.data['id'] = B.data['id'] + C1 = self.fake_gerrit.addFakeChange('org/project2', 'mp', 'C1') + C2 = self.fake_gerrit.addFakeChange('org/project2', 'mp', 'C2', + status='ABANDONED') + C1.data['id'] = B.data['id'] + C2.data['id'] = B.data['id'] + A.addApproval('Code-Review', 2) B.addApproval('Code-Review', 2) - C.addApproval('Code-Review', 2) + C1.addApproval('Code-Review', 2) - # A Depends-On: B+C + # A Depends-On: B+C1 A.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % ( A.subject, B.data['id']) self.executor_server.hold_jobs_in_build = True B.addApproval('Approved', 1) - C.addApproval('Approved', 1) + C1.addApproval('Approved', 1) self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) self.waitUntilSettled() @@ -3864,10 +3868,10 @@ For CI problems and help debugging, contact ci@example.org""" self.assertEqual(A.data['status'], 'MERGED') self.assertEqual(B.data['status'], 'MERGED') - self.assertEqual(C.data['status'], 'MERGED') + self.assertEqual(C1.data['status'], 'MERGED') self.assertEqual(A.reported, 2) self.assertEqual(B.reported, 2) - self.assertEqual(C.reported, 2) + self.assertEqual(C1.reported, 2) changes = self.getJobFromHistory( 'project-merge', 'org/project1').changes diff --git a/zuul/driver/gerrit/gerritconnection.py b/zuul/driver/gerrit/gerritconnection.py index 460eabb94d..35137c7b23 100644 --- a/zuul/driver/gerrit/gerritconnection.py +++ b/zuul/driver/gerrit/gerritconnection.py @@ -470,6 +470,9 @@ class GerritConnection(BaseConnection): self.log.debug("Updating %s: Getting git-dependent change %s,%s" % (change, dep_num, dep_ps)) dep = self._getChange(dep_num, dep_ps, history=history) + # This is a git commit dependency. So we only ignore it if it is + # already merged. So even if it is "ABANDONED", we should not + # ignore it. if (not dep.is_merged) and dep not in needs_changes: needs_changes.append(dep) @@ -481,7 +484,7 @@ class GerritConnection(BaseConnection): "change %s,%s" % (change, dep_num, dep_ps)) dep = self._getChange(dep_num, dep_ps, history=history) - if (not dep.is_merged) and dep not in needs_changes: + if dep.open and dep not in needs_changes: needs_changes.append(dep) change.needs_changes = needs_changes @@ -493,7 +496,7 @@ class GerritConnection(BaseConnection): self.log.debug("Updating %s: Getting git-needed change %s,%s" % (change, dep_num, dep_ps)) dep = self._getChange(dep_num, dep_ps, history=history) - if (not dep.is_merged) and dep.is_current_patchset: + if dep.open and dep.is_current_patchset: needed_by_changes.append(dep) for record in self._getNeededByFromCommit(data['id'], change): @@ -509,7 +512,7 @@ class GerritConnection(BaseConnection): refresh = (dep_num, dep_ps) not in history dep = self._getChange( dep_num, dep_ps, refresh=refresh, history=history) - if (not dep.is_merged) and dep.is_current_patchset: + if dep.open and dep.is_current_patchset: needed_by_changes.append(dep) change.needed_by_changes = needed_by_changes