Diff
Modified: trunk/Tools/CISupport/ews-build/factories.py (291291 => 291292)
--- trunk/Tools/CISupport/ews-build/factories.py 2022-03-15 15:26:35 UTC (rev 291291)
+++ trunk/Tools/CISupport/ews-build/factories.py 2022-03-15 15:38:21 UTC (rev 291292)
@@ -320,4 +320,4 @@
def __init__(self, platform, configuration=None, architectures=None, additionalArguments=None, **kwargs):
factory.BuildFactory.__init__(self)
self.addStep(ConfigureBuild(platform=platform, configuration=configuration, architectures=architectures, buildOnly=False, triggers=None, remotes=None, additionalArguments=additionalArguments))
- self.addStep(ValidateChange(verifyMergeQueue=True))
+ self.addStep(ValidateChange(verifyMergeQueue=True, verifyNoDraftForMergeQueue=True))
Modified: trunk/Tools/CISupport/ews-build/steps.py (291291 => 291292)
--- trunk/Tools/CISupport/ews-build/steps.py 2022-03-15 15:26:35 UTC (rev 291291)
+++ trunk/Tools/CISupport/ews-build/steps.py 2022-03-15 15:38:21 UTC (rev 291292)
@@ -216,6 +216,11 @@
return 1
return 0
+ def _is_pr_draft(self, pr_json):
+ if pr_json.get('draft', False):
+ return 1
+ return 0
+
def should_send_email_for_pr(self, pr_number):
pr_json = self.get_pr_json(pr_number)
if not pr_json:
@@ -1242,6 +1247,7 @@
addURLs=True,
verifycqplus=False,
verifyMergeQueue=False,
+ verifyNoDraftForMergeQueue=False,
):
self.verifyObsolete = verifyObsolete
self.verifyBugClosed = verifyBugClosed
@@ -1248,6 +1254,7 @@
self.verifyReviewDenied = verifyReviewDenied
self.verifycqplus = verifycqplus
self.verifyMergeQueue = verifyMergeQueue
+ self.verifyNoDraftForMergeQueue = verifyNoDraftForMergeQueue
self.addURLs = addURLs
buildstep.BuildStep.__init__(self)
@@ -1266,6 +1273,13 @@
self.descriptionDone = reason
self.build.buildFinished([reason], SKIPPED)
+ def fail_build(self, reason):
+ self._addToLog('stdio', reason)
+ self.finished(FAILURE)
+ self.build.results = FAILURE
+ self.descriptionDone = reason
+ self.build.buildFinished([reason], FAILURE)
+
def start(self):
patch_id = self.getProperty('patch_id', '')
pr_number = self.getProperty('github.number', '')
@@ -1296,6 +1310,8 @@
if self.verifycqplus and patch_id:
self._addToLog('stdio', 'Change is in commit queue.\n')
self._addToLog('stdio', 'Change has been reviewed.\n')
+ if self.verifyNoDraftForMergeQueue and pr_number:
+ self._addToLog('stdio', 'Change is not a draft.\n')
if self.verifyMergeQueue and pr_number:
self._addToLog('stdio', 'Change is in merge queue.\n')
self.finished(SUCCESS)
@@ -1361,7 +1377,12 @@
self.skip_build("PR {} does not have a merge queue label".format(pr_number))
return False
- if -1 in (obsolete, pr_closed, blocked, merge_queue):
+ draft = self._is_pr_draft(pr_json) if self.verifyNoDraftForMergeQueue else 0
+ if draft == 1:
+ self.fail_build("PR {} is a draft pull request".format(pr_number))
+ return False
+
+ if -1 in (obsolete, pr_closed, blocked, merge_queue, draft):
self.finished(WARNINGS)
return False
Modified: trunk/Tools/CISupport/ews-build/steps_unittest.py (291291 => 291292)
--- trunk/Tools/CISupport/ews-build/steps_unittest.py 2022-03-15 15:26:35 UTC (rev 291291)
+++ trunk/Tools/CISupport/ews-build/steps_unittest.py 2022-03-15 15:38:21 UTC (rev 291292)
@@ -5021,12 +5021,13 @@
"is_patch": 1,
"summary": "{}"}}'''.format(obsolete, title))
- def get_pr(self, pr_number, title='Sample pull request', closed=False, labels=None):
+ def get_pr(self, pr_number, title='Sample pull request', closed=False, labels=None, draft=False):
return dict(
number=pr_number,
state='closed' if closed else 'open',
title=title,
user=dict(login='JonWBedard'),
+ draft=draft,
head=dict(
sha='7496f8ecc4cc8011f19c8cc1bc7b18fe4a88ad5c',
ref='eng/pull-request',
@@ -5133,7 +5134,29 @@
self.assertEqual(self.getProperty('fast_commit_queue'), None, 'fast_commit_queue is unexpectedly set')
return rc
+ def test_draft(self):
+ self.setupStep(ValidateChange(verifyNoDraftForMergeQueue=True))
+ ValidateChange.get_pr_json = lambda x, pull_request, repository_url=None, retry=None: self.get_pr(pr_number=pull_request, draft=True)
+ self.setProperty('github.number', '1234')
+ self.setProperty('repository', 'https://github.com/WebKit/WebKit')
+ self.setProperty('github.head.sha', '7496f8ecc4cc8011f19c8cc1bc7b18fe4a88ad5c')
+ self.expectOutcome(result=FAILURE, state_string='PR 1234 is a draft pull request')
+ rc = self.runStep()
+ self.assertEqual(self.getProperty('fast_commit_queue'), None, 'fast_commit_queue is unexpectedly set')
+ return rc
+ def test_no_draft(self):
+ self.setupStep(ValidateChange(verifyNoDraftForMergeQueue=True))
+ ValidateChange.get_pr_json = lambda x, pull_request, repository_url=None, retry=None: self.get_pr(pr_number=pull_request, draft=False)
+ self.setProperty('github.number', '1234')
+ self.setProperty('repository', 'https://github.com/WebKit/WebKit')
+ self.setProperty('github.head.sha', '7496f8ecc4cc8011f19c8cc1bc7b18fe4a88ad5c')
+ self.expectOutcome(result=SUCCESS, state_string='Validated change')
+ rc = self.runStep()
+ self.assertEqual(self.getProperty('fast_commit_queue'), None, 'fast_commit_queue is unexpectedly set')
+ return rc
+
+
class TestValidateCommiterAndReviewer(BuildStepMixinAdditions, unittest.TestCase):
def setUp(self):
self.longMessage = True
Modified: trunk/Tools/ChangeLog (291291 => 291292)
--- trunk/Tools/ChangeLog 2022-03-15 15:26:35 UTC (rev 291291)
+++ trunk/Tools/ChangeLog 2022-03-15 15:38:21 UTC (rev 291292)
@@ -1,3 +1,23 @@
+2022-03-14 Jonathan Bedard <[email protected]>
+
+ [Merge-Queue] Fail draft PRs
+ https://bugs.webkit.org/show_bug.cgi?id=237859
+ <rdar://problem/90277316>
+
+ Reviewed by Aakash Jain.
+
+ We should never merge a draft pull request.
+
+ * CISupport/ews-build/factories.py:
+ (MergeQueueFactory.__init__): Verify that provided change is not a draft.
+ * CISupport/ews-build/steps.py:
+ (GitHubMixin._is_pr_draft): Check if pr_json indicates a draft PR.
+ (ValidateChange.__init__): Accept verifyNoDraft flag.
+ (ValidateChange.start):
+ (ValidateChange.fail_build):
+ (ValidateChange.validate_github): Fail the build if the provided PR is a draft.
+ * CISupport/ews-build/steps_unittest.py:
+
2022-03-15 Youenn Fablet <[email protected]>
Mark permission as denied if system forbids access to camera and/or microphone
