diff options
author | James E. Blair <jim@acmegating.com> | 2022-07-02 17:15:36 -0700 |
---|---|---|
committer | James E. Blair <jim@acmegating.com> | 2022-07-05 10:35:00 -0700 |
commit | 1d7a7df37391db7409c7211193bb40c2b9a1a50c (patch) | |
tree | 676b1212c5e70559e296dea8b4fa5629bb1871d0 | |
parent | 87ea63eee39b061e6cb5e11697ac7c4df8247c6d (diff) | |
download | zuul-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.py | 9 | ||||
-rw-r--r-- | zuul/driver/gerrit/gerritreporter.py | 25 |
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, |