zuul
/
zuul
Code Issues Proposed changes

De-duplicate change dependencies 

Gerrit will return git parents in a submitted together response,
regardless of whether submitWholeTopic is set.  Make sure we emulate
that in our tests and validate the outcome.

Adding these tests has shown that Zuul is unable to enqueue dependency
cycles when dependencies are duplicated, so deduplicate them in the
model class.  This is done as late as possible so that dependencies
from as many sources as are available are de-duplicated.

Change-Id: Iaf94fc8947975ecbb615dbbc1c20a885835f69cd
This commit is contained in:
James E. Blair 2022-03-28 14:52:27 -07:00
parent 38770a7867
commit 3ccbc89b26
4 changed files with 102 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
tests/base.py
View File

@ -815,8 +815,9 @@ class FakeGerritChange(object):
d['isCurrentPatchSet'] = False
return json.loads(json.dumps(self.data))
def queryHTTP(self):
self.queried += 1
def queryHTTP(self, internal=False):
if not internal:
self.queried += 1
labels = {}
for cat in self.categories:
labels[cat] = {}
@ -1155,6 +1156,11 @@ class GerritWebServer(object):
f'topic:{topic}', http=True)
else:
results = []
for dep in change.data.get('dependsOn', []):
dep_change = fake_gerrit.changes.get(int(dep['number']))
r = dep_change.queryHTTP(internal=True)
if r not in results:
results.append(r)
self.send_data(results)
self.end_headers()

43
tests/unit/test_circular_dependencies.py
View File

@ -1539,6 +1539,43 @@ class TestGerritCircularDependencies(ZuulTestCase):
def test_submitted_together_git(self):
self.fake_gerrit._fake_submit_whole_topic = True
A = self.fake_gerrit.addFakeChange('org/project1', "master", "A")
B = self.fake_gerrit.addFakeChange('org/project1', "master", "B")
C = self.fake_gerrit.addFakeChange('org/project1', "master", "C")
D = self.fake_gerrit.addFakeChange('org/project1', "master", "D")
E = self.fake_gerrit.addFakeChange('org/project1', "master", "E")
F = self.fake_gerrit.addFakeChange('org/project1', "master", "F")
G = self.fake_gerrit.addFakeChange('org/project1', "master", "G")
G.setDependsOn(F, 1)
F.setDependsOn(E, 1)
E.setDependsOn(D, 1)
D.setDependsOn(C, 1)
C.setDependsOn(B, 1)
B.setDependsOn(A, 1)
self.fake_gerrit.addEvent(C.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
self.assertEqual(len(C.patchsets[-1]["approvals"]), 1)
self.assertEqual(C.patchsets[-1]["approvals"][0]["type"], "Verified")
self.assertEqual(C.patchsets[-1]["approvals"][0]["value"], "1")
self.assertEqual(A.queried, 1)
self.assertEqual(B.queried, 1)
self.assertEqual(C.queried, 1)
self.assertEqual(D.queried, 1)
self.assertEqual(E.queried, 1)
self.assertEqual(F.queried, 1)
self.assertEqual(G.queried, 1)
self.assertHistory([
dict(name="project1-job", result="SUCCESS",
changes="1,1 2,1 3,1"),
dict(name="project-vars-job", result="SUCCESS",
changes="1,1 2,1 3,1"),
], ordered=False)
def test_submitted_together_git_topic(self):
self.fake_gerrit._fake_submit_whole_topic = True
A = self.fake_gerrit.addFakeChange('org/project1', "master", "A",
topic='test-topic')
B = self.fake_gerrit.addFakeChange('org/project1', "master", "B",
@ -1573,6 +1610,12 @@ class TestGerritCircularDependencies(ZuulTestCase):
self.assertEqual(E.queried, 8)
self.assertEqual(F.queried, 8)
self.assertEqual(G.queried, 8)
self.assertHistory([
dict(name="project1-job", result="SUCCESS",
changes="7,1 6,1 5,1 4,1 1,1 2,1 3,1"),
dict(name="project-vars-job", result="SUCCESS",
changes="7,1 6,1 5,1 4,1 1,1 2,1 3,1"),
], ordered=False)
@simple_layout('layouts/submitted-together-per-branch.yaml')
def test_submitted_together_per_branch(self):

42
tests/unit/test_gerrit.py
View File

@ -297,6 +297,48 @@ class TestGerritWeb(ZuulTestCase):
self.waitUntilSettled()
self.assertEqual(9, len(self.history))
def test_submitted_together_git(self):
# This tests that the circular dependency handling for submit
# whole topic doesn't activate for changes which are only in a
# git dependency.
A = self.fake_gerrit.addFakeChange('org/project1', "master", "A")
B = self.fake_gerrit.addFakeChange('org/project1', "master", "B")
C = self.fake_gerrit.addFakeChange('org/project1', "master", "C")
D = self.fake_gerrit.addFakeChange('org/project1', "master", "D")
E = self.fake_gerrit.addFakeChange('org/project1', "master", "E")
F = self.fake_gerrit.addFakeChange('org/project1', "master", "F")
G = self.fake_gerrit.addFakeChange('org/project1', "master", "G")
G.setDependsOn(F, 1)
F.setDependsOn(E, 1)
E.setDependsOn(D, 1)
D.setDependsOn(C, 1)
C.setDependsOn(B, 1)
B.setDependsOn(A, 1)
self.fake_gerrit.addEvent(C.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
self.assertEqual(len(C.patchsets[-1]["approvals"]), 1)
self.assertEqual(C.patchsets[-1]["approvals"][0]["type"], "Verified")
self.assertEqual(C.patchsets[-1]["approvals"][0]["value"], "1")
self.assertEqual(A.queried, 1)
self.assertEqual(B.queried, 1)
self.assertEqual(C.queried, 1)
self.assertEqual(D.queried, 1)
self.assertEqual(E.queried, 1)
self.assertEqual(F.queried, 1)
self.assertEqual(G.queried, 1)
self.assertHistory([
dict(name="project-merge", result="SUCCESS",
changes="1,1 2,1 3,1"),
dict(name="project-test1", result="SUCCESS",
changes="1,1 2,1 3,1"),
dict(name="project-test2", result="SUCCESS",
changes="1,1 2,1 3,1"),
dict(name="project1-project2-integration", result="SUCCESS",
changes="1,1 2,1 3,1"),
], ordered=False)
class TestFileComments(AnsibleZuulTestCase):
config_file = 'zuul-gerrit-web.conf'

12
zuul/model.py
View File

@ -5523,12 +5523,18 @@ class Change(Branch):
def needs_changes(self):
commit_needs_changes = self.commit_needs_changes or []
topic_needs_changes = self.topic_needs_changes or []
return (self.git_needs_changes + self.compat_needs_changes +
commit_needs_changes + topic_needs_changes)
r = OrderedDict()
for x in (self.git_needs_changes + self.compat_needs_changes +
commit_needs_changes + topic_needs_changes):
r[x] = None
return tuple(r.keys())
@property
def needed_by_changes(self):
return (self.git_needed_by_changes + self.compat_needed_by_changes)
r = OrderedDict()
for x in (self.git_needed_by_changes + self.compat_needed_by_changes):
r[x] = None
return tuple(r.keys())
def isUpdateOf(self, other):
if (self.project == other.project and