From df2e3af64244f47e4d8e66940b03bf07a7a9a0e1 Mon Sep 17 00:00:00 2001 From: Tobias Henkel Date: Tue, 25 Feb 2020 15:11:39 +0100 Subject: [PATCH] Refactor branch protection test infrastructure Branch protection rules are not just about required status so refactor it in the test framework so we can extend them in the future. Change-Id: I1e32ebb57b27bce898b074e419299a1803853608 --- tests/fakegithub.py | 45 +++++++++++++++++++++++--------- tests/unit/test_github_driver.py | 18 ++++++------- 2 files changed, 41 insertions(+), 22 deletions(-) diff --git a/tests/fakegithub.py b/tests/fakegithub.py index f3ab7e69bc..b16a8e410b 100644 --- a/tests/fakegithub.py +++ b/tests/fakegithub.py @@ -13,6 +13,7 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +from collections import defaultdict import github3.exceptions import re @@ -33,9 +34,13 @@ class FakeUser(object): class FakeBranch(object): - def __init__(self, branch='master', protected=False): + def __init__(self, fake_repo, branch='master', protected=False): self.name = branch - self.protected = protected + self._fake_repo = fake_repo + + @property + def protected(self): + return self.name in self._fake_repo._branch_protection_rules def as_dict(self): return { @@ -173,7 +178,7 @@ class FakeCommit(object): class FakeRepository(object): def __init__(self, name, data): self._api = FAKE_BASE_URL - self._branches = [FakeBranch()] + self._branches = [FakeBranch(self)] self._commits = {} self.data = data self.name = name @@ -187,6 +192,9 @@ class FakeRepository(object): # would do in case the permission is not sufficient or missing at all. self._permissions = {} + # List of branch protection rules + self._branch_protection_rules = defaultdict(FakeBranchProtectionRule) + # fail the next commit requests with 404 self.fail_not_found = 0 @@ -196,11 +204,15 @@ class FakeRepository(object): return [b for b in self._branches if b.protected] return self._branches - def _set_branch_protection(self, branch_name, protected): - for branch in self._branches: - if branch.name == branch_name: - branch.protected = protected - return + def _set_branch_protection(self, branch_name, protected=True, + contexts=None): + if not protected and branch_name in self._branch_protection_rules: + del self._branch_protection_rules[branch_name] + return + + rule = self._branch_protection_rules[branch_name] + rule.pattern = branch_name + rule.required_contexts = contexts or [] def _set_permission(self, key, value): # NOTE (felix): Currently, this is only used to mock a repo with @@ -220,7 +232,7 @@ class FakeRepository(object): return client.session.get(url, headers) def _create_branch(self, branch): - self._branches.append((FakeBranch(branch=branch))) + self._branches.append((FakeBranch(self, branch=branch))) def _delete_branch(self, branch_name): self._branches = [b for b in self._branches if b.name != branch_name] @@ -375,14 +387,15 @@ class FakeRepository(object): return None def get_url_protection(self, branch): - contexts = self.data.required_contexts.get((self.name, branch), []) - if not contexts: + rule = self._branch_protection_rules.get(branch) + + if not rule: # Note that GitHub returns 404 if branch protection is off so do # the same here as well return FakeResponse({}, 404) data = { 'required_status_checks': { - 'contexts': contexts + 'contexts': rule.required_contexts } } return FakeResponse(data) @@ -551,11 +564,17 @@ class FakeGithubSession(object): return repo.get_url(request, params=params) +class FakeBranchProtectionRule: + + def __init__(self): + self.pattern = None + self.required_contexts = [] + + class FakeGithubData(object): def __init__(self, pull_requests): self.pull_requests = pull_requests self.repos = {} - self.required_contexts = {} class FakeGithubClient(object): diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py index 3a9cbd0cac..6160fa947c 100644 --- a/tests/unit/test_github_driver.py +++ b/tests/unit/test_github_driver.py @@ -982,9 +982,9 @@ class TestGithubDriver(ZuulTestCase): @simple_layout('layouts/gate-github.yaml', driver='github') def test_status_checks(self): github = self.fake_github.getGithubClient() - github._data.required_contexts[('org/project', 'master')] = [ - 'tenant-one/check', - 'tenant-one/gate'] + repo = github.repo_from_project('org/project') + repo._set_branch_protection( + 'master', contexts=['tenant-one/check', 'tenant-one/gate']) A = self.fake_github.openFakePullRequest('org/project', 'master', 'A') self.fake_github.emitEvent(A.getPullRequestOpenedEvent()) @@ -1153,9 +1153,9 @@ class TestGithubDriver(ZuulTestCase): merge fails because cherry-pick is not supported by github. """ github = self.fake_github.getGithubClient() - github._data.required_contexts[('org/project', 'master')] = [ - 'tenant-one/check', - 'tenant-one/gate'] + repo = github.repo_from_project('org/project') + repo._set_branch_protection( + 'master', contexts=['tenant-one/check', 'tenant-one/gate']) A = self.fake_github.openFakePullRequest('org/project', 'master', 'A') self.fake_github.emitEvent(A.getPullRequestOpenedEvent()) @@ -1182,9 +1182,9 @@ class TestGithubDriver(ZuulTestCase): merge fails because cherry-pick is not supported by github. """ github = self.fake_github.getGithubClient() - github._data.required_contexts[('org/project', 'master')] = [ - 'tenant-one/check', - 'tenant-one/gate'] + repo = github.repo_from_project('org/project') + repo._set_branch_protection( + 'master', contexts=['tenant-one/check', 'tenant-one/gate']) A = self.fake_github.openFakePullRequest('org/project', 'master', 'A') self.fake_github.emitEvent(A.getPullRequestOpenedEvent())