summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJames E. Blair <jim@acmegating.com>2022-07-02 17:15:36 -0700
committerJames E. Blair <jim@acmegating.com>2022-07-05 10:35:00 -0700
commit1d7a7df37391db7409c7211193bb40c2b9a1a50c (patch)
tree676b1212c5e70559e296dea8b4fa5629bb1871d0
parent87ea63eee39b061e6cb5e11697ac7c4df8247c6d (diff)
downloadzuul-1d7a7df37391db7409c7211193bb40c2b9a1a50c.tar.gz
Fix submitWholeTopic post-merge check
The fake Gerrit did not actually merge all of the changes that should be submitted together simultaneously. This meant our testing that the git repo branch pointer moved forward was not correct. This change improves the fake Gerrit so that all changes under submitWholeTopic are merged simultaneously. This then shows the error where after two changes are merged simultaneously, the git repo check fails. That is because our check is performed by simply ensuring that the branch pointer is at some sha other than the one it was at right before we started the merge operation. The sequence is this: 1) Start reporting change #1 2) Get branch sha and store on change #1: abc1 3) Submit change #1 (Gerrit also merges change #2) 4) Get branch sha: abc2 5) Verify abc1 != abc2 (success) 6) Start reporting change #2 7) Get branch sha and store on change #2: abc2 8) Submit change #2 (already merged) 9) Get branch sha: abc2 A) Verify abc2 != abc2 (failure) This is corrected by using only the first branch sha in a bundle. When we store the branch sha on the change, we check to see if any other changes in the bundle for the same project+branch already have a sha; if they do, we use that. Otherwise we know we are the first, so we fetch it. This changes the steps above to: 2) Get branch sha for all Gerrit changes in bundle 7) [not needed -- branch sha already stored in step 2] A) Verify abc1 != abc2 (success) Change-Id: Ia9ef411cbf24d1e4e31456ddc660e5b2a6eb5321
-rw-r--r--tests/base.py9
-rw-r--r--zuul/driver/gerrit/gerritreporter.py25
2 files changed, 29 insertions, 5 deletions
diff --git a/tests/base.py b/tests/base.py
index b72bf8380..5a85ea0d7 100644
--- a/tests/base.py
+++ b/tests/base.py
@@ -1079,6 +1079,7 @@ class GerritWebServer(object):
# One of the changes in this topic isn't
# ready to merge
return self._409()
+ changes_to_merge = set(change.data['number'])
if fake_gerrit._fake_submit_whole_topic:
results = fake_gerrit._test_get_submitted_together(change)
for record in results:
@@ -1088,11 +1089,13 @@ class GerritWebServer(object):
# One of the changes in this topic isn't
# ready to merge
return self._409()
+ changes_to_merge.add(candidate.data['number'])
message = None
labels = {}
- fake_gerrit._test_handle_review(
- int(change.data['number']), message, True, labels,
- False, True)
+ for change_number in changes_to_merge:
+ fake_gerrit._test_handle_review(
+ int(change_number), message, True, labels,
+ False, True)
self.send_response(200)
self.end_headers()
diff --git a/zuul/driver/gerrit/gerritreporter.py b/zuul/driver/gerrit/gerritreporter.py
index 984b8742a..b99133dce 100644
--- a/zuul/driver/gerrit/gerritreporter.py
+++ b/zuul/driver/gerrit/gerritreporter.py
@@ -16,6 +16,7 @@ import logging
import voluptuous as v
from zuul.driver.gerrit.gerritsource import GerritSource
+from zuul.driver.gerrit.gerritmodel import GerritChange
from zuul.lib.logutil import get_annotated_logger
from zuul.model import Change
from zuul.reporter import BaseReporter
@@ -65,8 +66,28 @@ class GerritReporter(BaseReporter):
log.debug("Report change %s, params %s, message: %s, comments: %s",
item.change, self.config, message, comments)
- item.change._ref_sha = item.change.project.source.getRefSha(
- item.change.project, 'refs/heads/' + item.change.branch)
+ if phase2 and self._submit and not hasattr(item.change, '_ref_sha'):
+ # If we're starting to submit a bundle, save the current
+ # ref sha for every item in the bundle.
+ changes = set([item.change])
+ if item.bundle:
+ for i in item.bundle.items:
+ changes.add(i.change)
+
+ # Store a dict of project,branch -> sha so that if we have
+ # duplicate project/branches, we only query once.
+ ref_shas = {}
+ for other_change in changes:
+ if not isinstance(other_change, GerritChange):
+ continue
+ key = (other_change.project, other_change.branch)
+ ref_sha = ref_shas.get(key)
+ if not ref_sha:
+ ref_sha = other_change.project.source.getRefSha(
+ other_change.project,
+ 'refs/heads/' + other_change.branch)
+ ref_shas[key] = ref_sha
+ other_change._ref_sha = ref_sha
return self.connection.review(item, message, self._submit,
self._labels, self._checks_api,