[ews-build.webkit.org] Use twisted deferred to communicate with commi…

…ts.webkit.org

https://bugs.webkit.org/show_bug.cgi?id=249174
rdar://103269096

Reviewed by Dewei Zhu and Aakash Jain.

* Tools/CISupport/ews-build/steps.py:
(DetermineLandedIdentifier): Inherit from ShellCommandNewStyle.
(DetermineLandedIdentifier.__init__): Ditto.
(DetermineLandedIdentifier.run): Replace start and evaluateCommand.
(DetermineLandedIdentifier.identifier_for_hash): Use TwistedAdditions.request and return
a deferred object.
(DetermineLandedIdentifier.start): Deleted.
(DetermineLandedIdentifier.url_for_hash_details): Deleted.
(DetermineLandedIdentifier.evaluateCommand): Deleted.
* Tools/CISupport/ews-build/steps_unittest.py:

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

Tools/CISupport/ews-build/steps.py

@@ -28,11 +28,13 @@
 from buildbot.steps.worker import CompositeStepMixin
 from datetime import date
 from requests.auth import HTTPBasicAuth
-from twisted.internet import defer
+
+from twisted.internet import defer, reactor, task
 
 from layout_test_failures import LayoutTestFailures
 from send_email import send_email_to_patch_author, send_email_to_bot_watchers, send_email_to_github_admin, FROM_EMAIL
 from results_db import ResultsDatabase
+from twisted_additions import TwistedAdditions
 
 import json
 import mock
@@ -4885,7 +4887,7 @@ def hash_from_commit_text(self, commit_text):
         return None
 
 
-class DetermineLandedIdentifier(shell.ShellCommand):
+class DetermineLandedIdentifier(shell.ShellCommandNewStyle):
     name = 'determine-landed-identifier'
     descriptionDone = ['Determined landed identifier']
     command = ['git', 'log', '-1', '--no-decorate']
@@ -4896,20 +4898,19 @@ def __init__(self, **kwargs):
         self.identifier = None
         super(DetermineLandedIdentifier, self).__init__(logEnviron=False, timeout=300, **kwargs)
 
-    def start(self, BufferLogObserverClass=logobserver.BufferLogObserver):
-        self.log_observer = BufferLogObserverClass(wantStderr=True)
-        self.addLogObserver('stdio', self.log_observer)
-        return super(DetermineLandedIdentifier, self).start()
-
     def getResultSummary(self):
         if self.results == SUCCESS:
             return {'step': f'Identifier: {self.identifier}'}
         if self.results == FAILURE:
             return {'step': 'Failed to determine identifier'}
         return super(DetermineLandedIdentifier, self).getResultSummary()
 
-    def evaluateCommand(self, cmd):
-        rc = super(DetermineLandedIdentifier, self).evaluateCommand(cmd)
+    @defer.inlineCallbacks
+    def run(self, BufferLogObserverClass=logobserver.BufferLogObserver):
+        self.log_observer = BufferLogObserverClass(wantStderr=True)
+        self.addLogObserver('stdio', self.log_observer)
+
+        rc = yield super(DetermineLandedIdentifier, self).run()
 
         loglines = self.log_observer.getStdout().splitlines()
 
@@ -4921,9 +4922,9 @@ def evaluateCommand(self, cmd):
 
         landed_hash = self.getProperty('landed_hash')
         if not self.identifier:
-            time.sleep(60)  # It takes time for commits.webkit.org to digest commits
-            self.identifier = self.identifier_for_hash(landed_hash)
-            if '@' not in self.identifier:
+            yield task.deferLater(reactor, 60, lambda: None)  # It takes time for commits.webkit.org to digest commits
+            self.identifier = yield self.identifier_for_hash(landed_hash)
+            if not self.identifier or '@' not in self.identifier:
                 rc = FAILURE
 
         self.setProperty('comment_text', self.comment_text_for_bug(landed_hash, self.identifier))
@@ -4932,25 +4933,29 @@ def evaluateCommand(self, cmd):
         self.setProperty('build_summary', commit_summary)
         self.addURL(self.identifier, self.url_for_identifier(self.identifier))
 
-        return rc
-
-    def url_for_hash_details(self, hash):
-        return '{}{}/json'.format(COMMITS_INFO_URL, hash)
+        defer.returnValue(rc)
 
     def url_for_identifier(self, identifier):
         return '{}{}'.format(COMMITS_INFO_URL, identifier)
 
+    @defer.inlineCallbacks
     def identifier_for_hash(self, hash):
         try:
-            response = requests.get(self.url_for_hash_details(hash), timeout=60)
+            response = yield TwistedAdditions.request(
+                url=f'{COMMITS_INFO_URL}{hash}/json', logger=print,
+            )
             if response and response.status_code == 200:
-                return response.json().get('identifier', '{}'.format(hash)).replace('@trunk', '@main')
-            else:
-                print('Non-200 status code received from {}: {}'.format(COMMITS_INFO_URL, response.status_code))
+                defer.returnValue(response.json().get('identifier', hash).replace('@trunk', '@main'))
+                return
+            elif response:
+                print(f'Non-200 status code received from {COMMITS_INFO_URL}: {response.status_code}')
                 print(response.text)
-        except Exception as e:
-            print(e)
-        return hash
+                defer.returnValue(hash)
+                return
+        except json.decoder.JSONDecodeError:
+            print(f'Response from {COMMITS_INFO_URL} was not JSON')
+            print(response.text)
+        defer.returnValue(hash)
 
     def comment_text_for_bug(self, hash=None, identifier=None):
         identifier_str = identifier if identifier and '@' in identifier else '?'

Tools/CISupport/ews-build/steps_unittest.py

@@ -58,7 +58,7 @@
                    RunWebKitPyPython3Tests, RunWebKitTests, RunWebKitTestsInStressMode, RunWebKitTestsInStressGuardmallocMode,
                    RunWebKitTestsWithoutChange, RunWebKitTestsRedTree, RunWebKitTestsRepeatFailuresRedTree, RunWebKitTestsRepeatFailuresWithoutChangeRedTree,
                    RunWebKitTestsWithoutChangeRedTree, AnalyzeLayoutTestsResultsRedTree, TestWithFailureCount, ShowIdentifier,
-                   Trigger, TransferToS3, UnApplyPatch, UpdatePullRequest, UpdateWorkingDirectory, UploadBuiltProduct,
+                   Trigger, TransferToS3, TwistedAdditions, UnApplyPatch, UpdatePullRequest, UpdateWorkingDirectory, UploadBuiltProduct,
                    UploadTestResults, ValidateCommitMessage, ValidateCommitterAndReviewer, ValidateChange, ValidateRemote, ValidateSquashed)
 
 # Workaround for https://github.com/buildbot/buildbot/issues/4669
@@ -5708,27 +5708,16 @@ def tearDown(self):
         return self.tearDownBuildStep()
 
     def mock_commits_webkit_org(self, identifier=None):
-        class Response(object):
-            def __init__(self, data=None, status_code=200):
-                self.status_code = status_code
-                self.headers = {'Content-Type': 'text/json'}
-                self.text = json.dumps(data or {})
-
-            def json(self):
-                return json.loads(self.text)
-
-        return patch(
-            'requests.get',
-            lambda *args, **kwargs: Response(
-                data=dict(identifier=identifier) if identifier else dict(status='Not Found'),
-                status_code=200 if identifier else 404,
-            )
-        )
+        return patch('steps.TwistedAdditions.request', lambda *args, **kwargs: TwistedAdditions.Response(
+            status_code=200,
+            content=json.dumps(dict(identifier=identifier) if identifier else dict(status='Not Found')).encode('utf-8'),
+        ))
 
     @classmethod
     def mock_sleep(cls):
-        return patch('time.sleep', lambda _: None)
+        return patch('twisted.internet.task.deferLater', lambda *_, **__: None)
 
+    @defer.inlineCallbacks
     def test_success_pr(self):
         with self.mock_commits_webkit_org(), self.mock_sleep():
             self.setupStep(DetermineLandedIdentifier())
@@ -5761,12 +5750,12 @@ def test_success_pr(self):
             )
             self.expectOutcome(result=SUCCESS, state_string='Identifier: 220797@main')
             with current_hostname(EWS_BUILD_HOSTNAME):
-                rc = self.runStep()
+                yield self.runStep()
 
         self.assertEqual(self.getProperty('comment_text'), 'Committed 220797@main (14dbf1155cf5): <https://commits.webkit.org/220797@main>\n\nReviewed commits have been landed. Closing PR #1234 and removing active labels.')
         self.assertEqual(self.getProperty('build_summary'), 'Committed 220797@main')
-        return rc
 
+    @defer.inlineCallbacks
     def test_success_gardening_pr(self):
         with self.mock_commits_webkit_org(), self.mock_sleep():
             self.setupStep(DetermineLandedIdentifier())
@@ -5797,12 +5786,12 @@ def test_success_gardening_pr(self):
             )
             self.expectOutcome(result=SUCCESS, state_string='Identifier: 249903@main')
             with current_hostname(EWS_BUILD_HOSTNAME):
-                rc = self.runStep()
+                yield self.runStep()
 
         self.assertEqual(self.getProperty('comment_text'), 'Test gardening commit 249903@main (5dc27962b4c5): <https://commits.webkit.org/249903@main>\n\nReviewed commits have been landed. Closing PR #1234 and removing active labels.')
         self.assertEqual(self.getProperty('build_summary'), 'Committed 249903@main')
-        return rc
 
+    @defer.inlineCallbacks
     def test_success_pr_fallback(self):
         with self.mock_commits_webkit_org(identifier='220797@main'), self.mock_sleep():
             self.setupStep(DetermineLandedIdentifier())
@@ -5818,12 +5807,12 @@ def test_success_pr_fallback(self):
             )
             self.expectOutcome(result=SUCCESS, state_string='Identifier: 220797@main')
             with current_hostname(EWS_BUILD_HOSTNAME):
-                rc = self.runStep()
+                yield self.runStep()
 
         self.assertEqual(self.getProperty('comment_text'), 'Committed 220797@main (5dc27962b4c5): <https://commits.webkit.org/220797@main>\n\nReviewed commits have been landed. Closing PR #1234 and removing active labels.')
         self.assertEqual(self.getProperty('build_summary'), 'Committed 220797@main')
-        return rc
 
+    @defer.inlineCallbacks
     def test_pr_no_identifier(self):
         with self.mock_commits_webkit_org(), self.mock_sleep():
             self.setupStep(DetermineLandedIdentifier())
@@ -5839,12 +5828,12 @@ def test_pr_no_identifier(self):
             )
             self.expectOutcome(result=FAILURE, state_string='Failed to determine identifier')
             with current_hostname(EWS_BUILD_HOSTNAME):
-                rc = self.runStep()
+                yield self.runStep()
 
         self.assertEqual(self.getProperty('comment_text'), 'Committed ? (5dc27962b4c5): <https://commits.webkit.org/5dc27962b4c5>\n\nReviewed commits have been landed. Closing PR #1234 and removing active labels.')
         self.assertEqual(self.getProperty('build_summary'), 'Committed 5dc27962b4c5')
-        return rc
 
+    @defer.inlineCallbacks
     def test_success_patch(self):
         with self.mock_commits_webkit_org(identifier='220797@main'), self.mock_sleep():
             self.setupStep(DetermineLandedIdentifier())
@@ -5860,12 +5849,12 @@ def test_success_patch(self):
             )
             self.expectOutcome(result=SUCCESS, state_string='Identifier: 220797@main')
             with current_hostname(EWS_BUILD_HOSTNAME):
-                rc = self.runStep()
+                yield self.runStep()
 
         self.assertEqual(self.getProperty('comment_text'), 'Committed 220797@main (5dc27962b4c5): <https://commits.webkit.org/220797@main>\n\nAll reviewed patches have been landed. Closing bug and clearing flags on attachment 1234.')
         self.assertEqual(self.getProperty('build_summary'), 'Committed 220797@main')
-        return rc
 
+    @defer.inlineCallbacks
     def test_patch_no_identifier(self):
         with self.mock_commits_webkit_org(), self.mock_sleep():
             self.setupStep(DetermineLandedIdentifier())
@@ -5881,11 +5870,11 @@ def test_patch_no_identifier(self):
             )
             self.expectOutcome(result=FAILURE, state_string='Failed to determine identifier')
             with current_hostname(EWS_BUILD_HOSTNAME):
-                rc = self.runStep()
+                yield self.runStep()
 
         self.assertEqual(self.getProperty('comment_text'), 'Committed ? (5dc27962b4c5): <https://commits.webkit.org/5dc27962b4c5>\n\nAll reviewed patches have been landed. Closing bug and clearing flags on attachment 1234.')
         self.assertEqual(self.getProperty('build_summary'), 'Committed 5dc27962b4c5')
-        return rc
+
 
 class TestShowIdentifier(BuildStepMixinAdditions, unittest.TestCase):
     class MockPreviousStep(object):