From 7aba198bedcf2080f26e2498f834002971175c33 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Mon, 10 Oct 2022 10:53:14 -0700 Subject: Add "draft" github pipeline requirement This adds the "draft" PR status as a pipeline requirement to the GitHub driver. It is already used implicitly in dependent pipelines, but this will allow it to be added explicitly to other pipelines (for example, check). This also fixes some minor copy/pasta errors in debug messages related to github pipeline requirements. Change-Id: I05f8f61aee251af24c1479274904b429baedb29d --- doc/source/drivers/github.rst | 6 +++ .../github-draft-requirement-29b4f44229bb1af1.yaml | 6 +++ tests/fixtures/layouts/requirements-github.yaml | 48 ++++++++++++++++++++++ tests/unit/test_github_requirements.py | 40 ++++++++++++++++++ zuul/driver/github/githubmodel.py | 42 ++++++++++++++----- zuul/driver/github/githubsource.py | 4 ++ 6 files changed, 135 insertions(+), 11 deletions(-) create mode 100644 releasenotes/notes/github-draft-requirement-29b4f44229bb1af1.yaml diff --git a/doc/source/drivers/github.rst b/doc/source/drivers/github.rst index 9ca7a1f38..9e2763d49 100644 --- a/doc/source/drivers/github.rst +++ b/doc/source/drivers/github.rst @@ -551,6 +551,12 @@ enqueued into the pipeline. .. TODO: this could probably be expanded upon -- under what circumstances might this happen with github + .. attr:: draft + + A boolean value (``true`` or ``false``) that indicates whether + or not the change must be marked as a draft in GitHub in order + to be enqueued. + .. attr:: status A string value that corresponds with the status of the pull diff --git a/releasenotes/notes/github-draft-requirement-29b4f44229bb1af1.yaml b/releasenotes/notes/github-draft-requirement-29b4f44229bb1af1.yaml new file mode 100644 index 000000000..1e3493080 --- /dev/null +++ b/releasenotes/notes/github-draft-requirement-29b4f44229bb1af1.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + The GitHub driver now supports specifying the `draft` status of a + PR as a pipeline requirement. + See :attr:`pipeline.require..draft`. diff --git a/tests/fixtures/layouts/requirements-github.yaml b/tests/fixtures/layouts/requirements-github.yaml index 9e376b8b6..51870e5db 100644 --- a/tests/fixtures/layouts/requirements-github.yaml +++ b/tests/fixtures/layouts/requirements-github.yaml @@ -285,6 +285,34 @@ github: comment: true +- pipeline: + name: require_draft + manager: independent + require: + github: + draft: true + trigger: + github: + - event: pull_request + action: changed + success: + github: + comment: true + +- pipeline: + name: reject_draft + manager: independent + reject: + github: + draft: true + trigger: + github: + - event: pull_request + action: changed + success: + github: + comment: true + - pipeline: name: require_label manager: independent @@ -390,6 +418,14 @@ name: project16-require-check-run run: playbooks/project16-require-check-run.yaml +- job: + name: project17-require-draft + run: playbooks/project17-require-draft.yaml + +- job: + name: project18-reject-draft + run: playbooks/project18-reject-draft.yaml + - project: name: org/project1 pipeline: @@ -494,3 +530,15 @@ require_check_run: jobs: - project16-require-check-run + +- project: + name: org/project17 + require_draft: + jobs: + - project17-require-draft + +- project: + name: org/project18 + reject_draft: + jobs: + - project18-reject-draft diff --git a/tests/unit/test_github_requirements.py b/tests/unit/test_github_requirements.py index 0e2d2b7d5..ef1f75944 100644 --- a/tests/unit/test_github_requirements.py +++ b/tests/unit/test_github_requirements.py @@ -502,6 +502,46 @@ class TestGithubRequirements(ZuulTestCase): # Event hash is not current, should trigger self.assertEqual(len(self.history), 1) + @simple_layout('layouts/requirements-github.yaml', driver='github') + def test_require_draft(self): + + A = self.fake_github.openFakePullRequest('org/project17', 'master', + 'A', draft=True) + # A sync event that we will keep submitting to trigger + sync = A.getPullRequestSynchronizeEvent() + self.fake_github.emitEvent(sync) + self.waitUntilSettled() + + # PR is a draft, should enqueue + self.assertEqual(len(self.history), 1) + + # Make the PR not a draft + A.draft = False + self.fake_github.emitEvent(sync) + self.waitUntilSettled() + # PR is not a draft, should not enqueue + self.assertEqual(len(self.history), 1) + + @simple_layout('layouts/requirements-github.yaml', driver='github') + def test_reject_draft(self): + + A = self.fake_github.openFakePullRequest('org/project18', 'master', + 'A', draft=True) + # A sync event that we will keep submitting to trigger + sync = A.getPullRequestSynchronizeEvent() + self.fake_github.emitEvent(sync) + self.waitUntilSettled() + + # PR is a draft, should not enqueue + self.assertEqual(len(self.history), 0) + + # Make the PR not a draft + A.draft = False + self.fake_github.emitEvent(sync) + self.waitUntilSettled() + # PR is not a draft, should enqueue + self.assertEqual(len(self.history), 1) + @simple_layout('layouts/requirements-github.yaml', driver='github') def test_pipeline_require_label(self): "Test pipeline requirement: label" diff --git a/zuul/driver/github/githubmodel.py b/zuul/driver/github/githubmodel.py index 5fa076316..256333234 100644 --- a/zuul/driver/github/githubmodel.py +++ b/zuul/driver/github/githubmodel.py @@ -455,11 +455,12 @@ class GithubEventFilter(EventFilter, GithubCommonFilter): class GithubRefFilter(RefFilter, GithubCommonFilter): - def __init__(self, connection_name, statuses=[], required_reviews=[], - reject_reviews=[], open=None, merged=None, - current_patchset=None, reject_open=None, reject_merged=None, - reject_current_patchset=None, labels=[], reject_labels=[], - reject_statuses=[]): + def __init__(self, connection_name, statuses=[], + required_reviews=[], reject_reviews=[], open=None, + merged=None, current_patchset=None, draft=None, + reject_open=None, reject_merged=None, + reject_current_patchset=None, reject_draft=None, + labels=[], reject_labels=[], reject_statuses=[]): RefFilter.__init__(self, connection_name) GithubCommonFilter.__init__(self, required_reviews=required_reviews, @@ -479,6 +480,10 @@ class GithubRefFilter(RefFilter, GithubCommonFilter): self.current_patchset = not reject_current_patchset else: self.current_patchset = current_patchset + if reject_draft is not None: + self.draft = not reject_draft + else: + self.draft = draft self.labels = labels self.reject_labels = reject_labels @@ -496,12 +501,14 @@ class GithubRefFilter(RefFilter, GithubCommonFilter): if self.reject_reviews: ret += (' reject-reviews: %s' % str(self.reject_reviews)) - if self.open: + if self.open is not None: ret += ' open: %s' % self.open - if self.merged: + if self.merged is not None: ret += ' merged: %s' % self.merged - if self.current_patchset: + if self.current_patchset is not None: ret += ' current-patchset: %s' % self.current_patchset + if self.draft is not None: + ret += ' draft: %s' % self.draft if self.labels: ret += ' labels: %s' % self.labels if self.reject_labels: @@ -521,7 +528,8 @@ class GithubRefFilter(RefFilter, GithubCommonFilter): # and cannot possibly pass this test. if hasattr(change, 'number'): if self.open != change.open: - return FalseWithReason("Change is not a PR") + return FalseWithReason( + "Change does not match open requirement") else: return FalseWithReason("Change is not a PR") @@ -530,7 +538,8 @@ class GithubRefFilter(RefFilter, GithubCommonFilter): # and cannot possibly pass this test. if hasattr(change, 'number'): if self.merged != change.is_merged: - return FalseWithReason("Change is not a PR") + return FalseWithReason( + "Change does not match merged requirement") else: return FalseWithReason("Change is not a PR") @@ -539,7 +548,18 @@ class GithubRefFilter(RefFilter, GithubCommonFilter): # and cannot possibly pass this test. if hasattr(change, 'number'): if self.current_patchset != change.is_current_patchset: - return FalseWithReason("Change is not current") + return FalseWithReason( + "Change does not match current-patchset requirement") + else: + return FalseWithReason("Change is not a PR") + + if self.draft is not None: + # if a "change" has no number, it's not a change, but a push + # and cannot possibly pass this test. + if hasattr(change, 'number'): + if self.draft != change.draft: + return FalseWithReason( + "Change does not match draft requirement") else: return FalseWithReason("Change is not a PR") diff --git a/zuul/driver/github/githubsource.py b/zuul/driver/github/githubsource.py index 7d4815237..bdc373f79 100644 --- a/zuul/driver/github/githubsource.py +++ b/zuul/driver/github/githubsource.py @@ -169,6 +169,7 @@ class GithubSource(BaseSource): open=config.get('open'), merged=config.get('merged'), current_patchset=config.get('current-patchset'), + draft=config.get('draft'), labels=to_list(config.get('label')), ) return [f] @@ -182,6 +183,7 @@ class GithubSource(BaseSource): reject_open=config.get('open'), reject_merged=config.get('merged'), reject_current_patchset=config.get('current-patchset'), + reject_draft=config.get('draft'), ) return [f] @@ -207,6 +209,7 @@ def getRequireSchema(): 'open': bool, 'merged': bool, 'current-patchset': bool, + 'draft': bool, 'label': scalar_or_list(str)} return require @@ -217,5 +220,6 @@ def getRejectSchema(): 'open': bool, 'merged': bool, 'current-patchset': bool, + 'draft': bool, 'label': scalar_or_list(str)} return reject -- cgit v1.2.1