Title: [291060] trunk/Tools
Revision
291060
Author
jbed...@apple.com
Date
2022-03-09 12:29:54 -0800 (Wed, 09 Mar 2022)

Log Message

[EWS] Support concept of 'blocked' pull requests
https://bugs.webkit.org/show_bug.cgi?id=237370
<rdar://problem/89689094>

Reviewed by Ryan Haddad.

cq- communicates that a change is blocked, either by an engineer or by a failed EWS run.
"changes requested" is not as dramatic as "cq-", so we are adding the concept of a "blocked"
pull request, which has simlar effects to "cq-".

* Tools/CISupport/ews-build/steps.py:
(GitHubMixin._is_pr_blocked): Check if a pull request currently has the "blocked" tag.
(GitHubMixin.modify_label): Add or remove a label from a pull request.
(SetCommitQueueMinusFlagOnPatch.hideStepIf): Added.
(BlockPullRequest): Added.
(AnalyzeCompileWebKitResults.analyzeResults): Block PR if failed.
(AnalyzeLayoutTestsResults.report_failure): Ditto.

Canonical link: https://commits.webkit.org/248233@main

Modified Paths

Diff

Modified: trunk/Tools/CISupport/ews-build/steps.py (291059 => 291060)


--- trunk/Tools/CISupport/ews-build/steps.py	2022-03-09 19:59:42 UTC (rev 291059)
+++ trunk/Tools/CISupport/ews-build/steps.py	2022-03-09 20:29:54 UTC (rev 291060)
@@ -191,6 +191,12 @@
             return -1
         return 0 if pr_sha == self.getProperty('github.head.sha', '?') else 1
 
+    def _is_pr_blocked(self, pr_json):
+        for label in (pr_json or {}).get('labels', {}):
+            if label.get('name', '') == 'blocked':
+                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:
@@ -204,7 +210,33 @@
             return False
         return True
 
+    def modify_label(self, pr_number, label, repository_url=None, action=''):
+        api_url = GitHub.api_url(repository_url)
+        if not api_url:
+            return False
+        if action not in ('add', 'delete'):
+            self._addToLog('stdio', "'{}' is not a valid label modifcation action".format(action))
+            return False
 
+        pr_label_url = '{}/issues/{}/labels'.format(api_url, pr_number)
+        try:
+            username, access_token = GitHub.credentials()
+            auth = HTTPBasicAuth(username, access_token) if username and access_token else None
+            response = requests.request(
+                'POST' if action == 'add' else 'DELETE',
+                pr_label_url, timeout=60, auth=auth,
+                headers=dict(Accept='application/vnd.github.v3+json'),
+                json=dict(labels=[label]),
+            )
+            if response.status_code // 100 != 2:
+                self._addToLog('stdio', "Unable to {} '{}' label on PR {}. Unexpected response code from GitHub: {}".format(action, label, pr_number, response.status_code))
+                return False
+        except Exception as e:
+            self._addToLog('stdio', "Error in {}ing '{}' label on PR {}".format(action, label, pr_number))
+            return False
+        return True
+
+
 class ShellMixin(object):
     WINDOWS_SHELL_PLATFORMS = ['wincairo']
 
@@ -1293,7 +1325,12 @@
             self.skip_build('Hash {} on PR {} is outdated'.format(self.getProperty('github.head.sha', '?')[:HASH_LENGTH_TO_DISPLAY], pr_number))
             return False
 
-        if obsolete == -1 or pr_closed == -1:
+        blocked = self._is_pr_blocked(pr_json) if self.verifyReviewDenied else 0
+        if blocked == 1:
+            self.skip_build("PR {} has been marked as 'blocked'".format(pr_number))
+            return False
+
+        if -1 in (obsolete, pr_closed, blocked):
             self.finished(WARNINGS)
             return False
 
@@ -1435,7 +1472,48 @@
     def doStepIf(self, step):
         return self.getProperty('patch_id', False)
 
+    def hideStepIf(self, results, step):
+        return not self.doStepIf(step)
 
+
+class BlockPullRequest(buildstep.BuildStep, GitHubMixin):
+    name = 'block-pull-request'
+
+    @defer.inlineCallbacks
+    def _addToLog(self, logName, message):
+        try:
+            log = self.getLog(logName)
+        except KeyError:
+            log = yield self.addLog(logName)
+        log.addStdout(message)
+
+    def start(self):
+        pr_number = self.getProperty('github.number', '')
+        build_finish_summary = self.getProperty('build_finish_summary', None)
+
+        rc = SKIPPED
+        if CURRENT_HOSTNAME == EWS_BUILD_HOSTNAME:
+            rc = SUCCESS if self.modify_label(pr_number, 'blocked', repository_url=self.getProperty('repository', '')) else FAILURE
+        self.finished(rc)
+        if build_finish_summary:
+            self.build.buildFinished([build_finish_summary], FAILURE)
+        return None
+
+    def getResultSummary(self):
+        if self.results == SUCCESS:
+            return {'step': 'Added blocked label pull request'}
+        elif self.results == SKIPPED:
+            return buildstep.BuildStep.getResultSummary(self)
+        return {'step': 'Failed to add blocked label to pull request'}
+
+    def doStepIf(self, step):
+        return self.getProperty('github.number')
+
+    def hideStepIf(self, results, step):
+        return not self.doStepIf(step)
+
+
+
 class RemoveFlagsOnPatch(buildstep.BuildStep, BugzillaMixin):
     name = 'remove-flags-from-patch'
     flunkOnFailure = False
@@ -2117,7 +2195,6 @@
         self.finished(FAILURE)
         self.setProperty('build_finish_summary', message)
 
-        # FIXME: Need a cq- equivalent for GitHub
         if patch_id:
             if self.getProperty('buildername', '').lower() == 'commit-queue':
                 self.setProperty('bugzilla_comment_text', message)
@@ -2124,6 +2201,8 @@
                 self.build.addStepsAfterCurrentStep([CommentOnBug(), SetCommitQueueMinusFlagOnPatch()])
             else:
                 self.build.addStepsAfterCurrentStep([SetCommitQueueMinusFlagOnPatch()])
+        else:
+            self.build.addStepsAfterCurrentStep([BlockPullRequest()])
 
     @defer.inlineCallbacks
     def getResults(self, name):
@@ -2947,7 +3026,7 @@
             self.setProperty('bugzilla_comment_text', message)
             self.build.addStepsAfterCurrentStep([CommentOnBug(), SetCommitQueueMinusFlagOnPatch()])
         else:
-            self.build.addStepsAfterCurrentStep([SetCommitQueueMinusFlagOnPatch()])
+            self.build.addStepsAfterCurrentStep([SetCommitQueueMinusFlagOnPatch(), BlockPullRequest()])
         return defer.succeed(None)
 
     def report_pre_existing_failures(self, clean_tree_failures, flaky_failures):

Modified: trunk/Tools/ChangeLog (291059 => 291060)


--- trunk/Tools/ChangeLog	2022-03-09 19:59:42 UTC (rev 291059)
+++ trunk/Tools/ChangeLog	2022-03-09 20:29:54 UTC (rev 291060)
@@ -1,3 +1,23 @@
+2022-03-01  Jonathan Bedard  <jbed...@apple.com>
+
+        [EWS] Support concept of 'blocked' pull requests
+        https://bugs.webkit.org/show_bug.cgi?id=237370
+        <rdar://problem/89689094>
+
+        Reviewed by Ryan Haddad.
+
+        cq- communicates that a change is blocked, either by an engineer or by a failed EWS run.
+        "changes requested" is not as dramatic as "cq-", so we are adding the concept of a "blocked"
+        pull request, which has simlar effects to "cq-".
+
+        * CISupport/ews-build/steps.py:
+        (GitHubMixin._is_pr_blocked): Check if a pull request currently has the "blocked" tag.
+        (GitHubMixin.modify_label): Add or remove a label from a pull request.
+        (SetCommitQueueMinusFlagOnPatch.hideStepIf): Added.
+        (BlockPullRequest): Added.
+        (AnalyzeCompileWebKitResults.analyzeResults): Block PR if failed.
+        (AnalyzeLayoutTestsResults.report_failure): Ditto.
+
 2022-03-09  Jonathan Bedard  <jbed...@apple.com>
 
         [git-webkit] Extract revision from `git svn dcommit` (Follow-up fix)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to