From bd3e68882f317e27431d0fbeac12c1641e54f1de Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Fri, 22 Apr 2022 16:05:15 -0700 Subject: [PATCH] Add base_sha support to gitlab driver This adds the base_sha field to the gitlab driver. This matches the behavior and tests for the github driver. Change-Id: I2abe7326b920c9844333972daa5356fc0fed69f7 --- tests/base.py | 27 ++++++++++++------- tests/fakegitlab.py | 4 +-- tests/fixtures/layouts/files-gitlab.yaml | 31 +++++++++++++++++++++ tests/unit/test_gitlab_driver.py | 34 ++++++++++++++++++++++++ zuul/driver/gitlab/gitlabconnection.py | 1 + 5 files changed, 86 insertions(+), 11 deletions(-) create mode 100644 tests/fixtures/layouts/files-gitlab.yaml diff --git a/tests/base.py b/tests/base.py index f680330aa7..c257147a1f 100644 --- a/tests/base.py +++ b/tests/base.py @@ -1978,11 +1978,12 @@ class FakeGitlabConnection(gitlabconnection.GitlabConnection): return super(FakeGitlabConnection, self).getGitUrl(project) def openFakeMergeRequest(self, project, - branch, title, description='', files=[]): + branch, title, description='', files=[], + base_sha=None): self.mr_number += 1 merge_request = FakeGitlabMergeRequest( self, self.mr_number, project, branch, title, self.upstream_root, - files=files, description=description) + files=files, description=description, base_sha=base_sha) self.merge_requests.setdefault( project, {})[str(self.mr_number)] = merge_request return merge_request @@ -2068,7 +2069,8 @@ class FakeGitlabMergeRequest(object): log = logging.getLogger("zuul.test.FakeGitlabMergeRequest") def __init__(self, gitlab, number, project, branch, - subject, upstream_root, files=[], description=''): + subject, upstream_root, files=[], description='', + base_sha=None): self.gitlab = gitlab self.source = gitlab self.number = number @@ -2089,18 +2091,20 @@ class FakeGitlabMergeRequest(object): self.notes = [] self.url = "https://%s/%s/merge_requests/%s" % ( self.gitlab.server, self.project, self.number) + self.base_sha = base_sha self.approved = False - self.mr_ref = self._createMRRef() + self.mr_ref = self._createMRRef(base_sha=base_sha) self._addCommitInMR(files=files) def _getRepo(self): repo_path = os.path.join(self.upstream_root, self.project) return git.Repo(repo_path) - def _createMRRef(self): + def _createMRRef(self, base_sha=None): + base_sha = base_sha or 'refs/tags/init' repo = self._getRepo() return GitlabChangeReference.create( - repo, self.getMRReference(), 'refs/tags/init') + repo, self.getMRReference(), base_sha) def getMRReference(self): return '%s/head' % self.number @@ -2113,8 +2117,8 @@ class FakeGitlabMergeRequest(object): } ) - def addCommit(self, files=[]): - self._addCommitInMR(files=files) + def addCommit(self, files=[], delete_files=None): + self._addCommitInMR(files=files, delete_files=delete_files) self._updateTimeStamp() def closeMergeRequest(self): @@ -2132,7 +2136,7 @@ class FakeGitlabMergeRequest(object): self._updateTimeStamp() self.merged_at = None - def _addCommitInMR(self, files=[], reset=False): + def _addCommitInMR(self, files=[], delete_files=None, reset=False): repo = self._getRepo() ref = repo.references[self.getMRReference()] if reset: @@ -2154,6 +2158,11 @@ class FakeGitlabMergeRequest(object): f.write(content) repo.index.add([fn]) + if delete_files: + for fn in delete_files: + fn = os.path.join(repo.working_dir, fn) + repo.index.remove([fn]) + self.sha = repo.index.commit(msg).hexsha repo.create_head(self.getMRReference(), self.sha, force=True) diff --git a/tests/fakegitlab.py b/tests/fakegitlab.py index 69b1faaa2a..1b166c855e 100644 --- a/tests/fakegitlab.py +++ b/tests/fakegitlab.py @@ -163,8 +163,8 @@ class GitlabWebServer(object): 'merged_at': mr.merged_at.strftime('%Y-%m-%dT%H:%M:%S.%fZ') if mr.merged_at else mr.merged_at, 'diff_refs': { - 'base_sha': 'c380d3acebd181f13629a25d2e2acca46ffe1e00', - 'head_sha': '2be7ddb704c7b6b83732fdd5b9f09d5a397b5f8f', + 'base_sha': mr.base_sha, + 'head_sha': mr.sha, 'start_sha': 'c380d3acebd181f13629a25d2e2acca46ffe1e00' }, 'merge_status': mr.merge_status, diff --git a/tests/fixtures/layouts/files-gitlab.yaml b/tests/fixtures/layouts/files-gitlab.yaml new file mode 100644 index 0000000000..a6b12e681b --- /dev/null +++ b/tests/fixtures/layouts/files-gitlab.yaml @@ -0,0 +1,31 @@ +- pipeline: + name: check + manager: independent + trigger: + gitlab: + - event: gl_merge_request + action: opened + +- job: + name: base + parent: null + run: playbooks/base.yaml + +- job: + name: project-test1 + files: + - .*-requires + run: playbooks/project-test1.yaml + +- job: + name: project-test2 + files: + - .*-removed + run: playbooks/project-test1.yaml + +- project: + name: org/project + check: + jobs: + - project-test1 + - project-test2 diff --git a/tests/unit/test_gitlab_driver.py b/tests/unit/test_gitlab_driver.py index 84c1744dfc..09184be13d 100644 --- a/tests/unit/test_gitlab_driver.py +++ b/tests/unit/test_gitlab_driver.py @@ -791,6 +791,40 @@ class TestGitlabDriver(ZuulTestCase): self.assertEqual("http://tokenname5:555@gitlabfivvve/org/project1.git", project_git_url) + @simple_layout('layouts/files-gitlab.yaml', driver='gitlab') + def test_changed_file_match_filter(self): + path = os.path.join(self.upstream_root, 'org/project') + base_sha = git.Repo(path).head.object.hexsha + + files = {'{:03d}.txt'.format(n): 'test' for n in range(300)} + files["foobar-requires"] = "test" + files["to-be-removed"] = "test" + A = self.fake_gitlab.openFakeMergeRequest( + 'org/project', 'master', 'A', files=files, base_sha=base_sha) + + self.fake_gitlab.emitEvent(A.getMergeRequestOpenedEvent()) + self.waitUntilSettled() + # project-test1 and project-test2 should be run + self.assertEqual(2, len(self.history)) + + @simple_layout('layouts/files-gitlab.yaml', driver='gitlab') + def test_changed_and_reverted_file_not_match_filter(self): + path = os.path.join(self.upstream_root, 'org/project') + base_sha = git.Repo(path).head.object.hexsha + + files = {'{:03d}.txt'.format(n): 'test' for n in range(300)} + files["foobar-requires"] = "test" + files["to-be-removed"] = "test" + A = self.fake_gitlab.openFakeMergeRequest( + 'org/project', 'master', 'A', files=files, base_sha=base_sha) + A.addCommit(delete_files=['to-be-removed']) + + self.fake_gitlab.emitEvent(A.getMergeRequestOpenedEvent()) + self.waitUntilSettled() + # Only project-test1 should be run, because the file to-be-removed + # is reverted and not in changed files to trigger project-test2 + self.assertEqual(1, len(self.history)) + class TestGitlabUnprotectedBranches(ZuulTestCase): config_file = 'zuul-gitlab-driver.conf' diff --git a/zuul/driver/gitlab/gitlabconnection.py b/zuul/driver/gitlab/gitlabconnection.py index 1175c136ae..54c3d66b81 100644 --- a/zuul/driver/gitlab/gitlabconnection.py +++ b/zuul/driver/gitlab/gitlabconnection.py @@ -660,6 +660,7 @@ class GitlabConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection): change.ref = "refs/merge-requests/%s/head" % change.number change.branch = change.mr['target_branch'] change.is_current_patchset = (change.mr['sha'] == change.patchset) + change.base_sha = change.mr['diff_refs'].get('base_sha') change.commit_id = change.mr['diff_refs'].get('head_sha') change.owner = change.mr['author'].get('username') # Files changes are not part of the Merge Request data