[blinkpy] 3/N: Extract build selection logic from rebaseline-cl

This change refactors `rebaseline-cl` so that the try builder/build
selection logic can be shared with `update-metadata`. Some improvements:
* The build resolver now supports builds with explicit numbers as well
  CI builders without a number (fetches the latest failing build). This
  will allow engineers to update WPT metadata to suppress waterfall
  failures, even without a current CL. For now, this support is not
  exposed in `rebaseline-cl`.
* Logs of builder statuses are consolidated into a visually aligned
  table with more information (build numbers, status, ci/try).

Bug: 1299650
Change-Id: I495dd00d3be306b69bab5bce4efff74cd33630d7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3820113
Reviewed-by: Weizhong Xia <weizhong@google.com>
Commit-Queue: Jonathan Lee <jonathanjlee@google.com>
Cr-Commit-Position: refs/heads/main@{#1035105}

third_party/blink/tools/blinkpy/common/net/git_cl.py

@@ -12,11 +12,11 @@
 import logging
 import re
 import six
+from typing import Literal, Mapping, NamedTuple
 
 from blinkpy.common.checkout.git import Git
-from blinkpy.common.net.luci_auth import LuciAuth
-from blinkpy.common.net.results_fetcher import Build, filter_latest_builds
-from blinkpy.common.net.rpc import BuildbucketClient
+from blinkpy.common.net.results_fetcher import filter_latest_builds
+from blinkpy.common.net.rpc import Build, BuildbucketClient
 
 _log = logging.getLogger(__name__)
 
@@ -25,31 +25,19 @@
 _COMMANDS_THAT_TAKE_REFRESH_TOKEN = ('try', )
 
 
-class CLStatus(
-        collections.namedtuple('CLStatus', ('status', 'try_job_results'))):
-    """Represents the current status of a particular CL.
-
-    It contains both the CL's status as reported by `git-cl status' as well as
-    a mapping of Build objects to TryJobStatus objects.
-    """
-    pass
-
-
-class TryJobStatus(
-        collections.namedtuple('TryJobStatus', ('status', 'result'))):
-    """Represents a current status of a particular job.
+# TODO(crbug.com/1299650): Rename to `BuildStatus` to include CI builds.
+class TryJobStatus(NamedTuple):
+    """The current status of a particular build.
 
     Specifically, whether it is scheduled or started or finished, and if
-    it is finished, whether it failed or succeeded. If it failed,
+    it is finished, whether it failed or succeeded.
     """
-
-    def __new__(cls, status, result=None):
-        assert status in ('SCHEDULED', 'STARTED', 'COMPLETED')
-        assert result in (None, 'FAILURE', 'SUCCESS', 'CANCELED')
-        return super(TryJobStatus, cls).__new__(cls, status, result)
+    status: Literal['MISSING', 'TRIGGERED', 'SCHEDULED', 'STARTED',
+                    'COMPLETED']
+    result: Literal[None, 'FAILURE', 'SUCCESS', 'CANCELED'] = None
 
     @staticmethod
-    def from_bb_status(bb_status):
+    def from_bb_status(bb_status: str) -> 'TryJobStatus':
         """Converts a buildbucket status into a TryJobStatus object."""
         assert bb_status in ('SCHEDULED', 'STARTED', 'SUCCESS', 'FAILURE',
                              'INFRA_FAILURE', 'CANCELLED')
@@ -63,15 +51,27 @@ def from_bb_status(bb_status):
                 'FAILURE' if bb_status == 'INFRA_FAILURE' else bb_status)
 
 
+BuildStatuses = Mapping[Build, TryJobStatus]
+
+
+class CLStatus(NamedTuple):
+    """The current status of a particular CL.
+
+    It contains both the CL's status as reported by `git-cl status' as well as
+    a mapping of Build objects to TryJobStatus objects.
+    """
+    status: str
+    try_job_results: BuildStatuses
+
+
 class GitCL(object):
     def __init__(self,
                  host,
                  auth_refresh_token_json=None,
                  cwd=None,
                  bb_client=None):
         self._host = host
-        self.bb_client = bb_client or BuildbucketClient(
-            host.web, LuciAuth(host))
+        self.bb_client = bb_client or BuildbucketClient.from_host(host)
         self._auth_refresh_token_json = auth_refresh_token_json
         self._cwd = cwd
         self._git_executable_name = Git.find_executable_name(

third_party/blink/tools/blinkpy/common/net/git_cl_mock.py

@@ -3,6 +3,7 @@
 # found in the LICENSE file.
 
 from blinkpy.common.net.git_cl import CLStatus, GitCL
+from blinkpy.common.net.rpc import BuildbucketClient
 from blinkpy.common.system.executive import ScriptError
 
 # pylint: disable=unused-argument
@@ -26,6 +27,7 @@ def __init__(self,
             time_out: Whether to simulate timing out while waiting.
             git_error_output: A dict of git-cl args to exception output.
         """
+        self.bb_client = BuildbucketClient.from_host(host)
         self._builders = host.builders.all_try_builder_names()
         self._status = status
         self._try_job_results = try_job_results

third_party/blink/tools/blinkpy/common/net/rpc.py

@@ -26,25 +26,31 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-import collections
 import logging
 import json
+from typing import NamedTuple, Optional
 from urllib.parse import urlunsplit
 
 import six
 
 from blinkpy.common.memoized import memoized
+from blinkpy.common.net.luci_auth import LuciAuth
 
 _log = logging.getLogger(__name__)
 
 # These characters always appear at the beginning of the RPC response.
 RESPONSE_PREFIX = b")]}'"
 
-# Represents a combination of builder and build number.
-# If build number is None, this represents the latest build for a given builder.
-Build = collections.namedtuple('Build',
-                               ['builder_name', 'build_number', 'build_id'],
-                               defaults=[None, None])
+
+class Build(NamedTuple):
+    """A combination of builder and build number.
+
+    If the build number is absent, this represents the latest build for a given
+    builder.
+    """
+    builder_name: str
+    build_number: Optional[int] = None
+    build_id: Optional[str] = None
 
 
 class RPCError(Exception):
@@ -76,6 +82,10 @@ def __init__(self, web, luci_auth, hostname, service):
         self._hostname = hostname
         self._service = service
 
+    @classmethod
+    def from_host(cls, host, *args, **kwargs):
+        return cls(host.web, LuciAuth(host), *args, **kwargs)
+
     @memoized
     def _make_url(self, method):
         return urlunsplit((

third_party/blink/tools/blinkpy/common/net/rpc_unittest.py

@@ -6,7 +6,6 @@
 import unittest
 
 from blinkpy.common.host_mock import MockHost
-from blinkpy.common.net.luci_auth import LuciAuth
 from blinkpy.common.net.rpc import Build, BuildbucketClient, RPCError
 
 
@@ -18,20 +17,10 @@ class BuildbucketTest(unittest.TestCase):
 
     def setUp(self):
         self.host = MockHost()
-        self.client = BuildbucketClient(
-            self.host.web,
-            LuciAuth(self.host),
-            'cr-buildbucket.appspot.com',
-            'buildbucket.v2.Builds',
-        )
-
-    def add_response(self, response, status=200):
-        self.host.web.responses.append({
-            'status_code':
-            status,
-            'body':
-            b")]}'" + json.dumps(response).encode(),
-        })
+        self.client = BuildbucketClient.from_host(
+            self.host,
+            hostname='cr-buildbucket.appspot.com',
+            service='buildbucket.v2.Builds')
 
     def test_search_builds_one_page(self):
         predicate = {
@@ -41,7 +30,7 @@ def test_search_builds_one_page(self):
                 'builder': 'linux-rel',
             },
         }
-        self.add_response({
+        self.host.web.append_prpc_response({
             'builds': [{
                 'id': '123',
                 'number': 123
@@ -74,7 +63,7 @@ def test_search_builds_follow_pages(self):
                 'builder': 'linux-rel',
             },
         }
-        self.add_response({
+        self.host.web.append_prpc_response({
             'builds': [{
                 'id': '123',
                 'number': 123
@@ -85,7 +74,7 @@ def test_search_builds_follow_pages(self):
             'nextPageToken':
             'id>789',
         })
-        self.add_response({
+        self.host.web.append_prpc_response({
             'builds': [{
                 'id': '789',
                 'number': 789
@@ -122,13 +111,17 @@ def test_search_builds_follow_pages(self):
         self.assertEqual(build3, {'id': '789', 'number': 789})
 
     def test_search_builds_no_next_token(self):
-        self.add_response({'builds': [{'id': '123', 'number': 123}]})
+        self.host.web.append_prpc_response(
+            {'builds': [{
+                'id': '123',
+                'number': 123
+            }]})
         builds = self.client.search_builds({}, ['id', 'number'], count=3)
         self.assertEqual(len(self.host.web.requests), 1)
         self.assertEqual(builds, [{'id': '123', 'number': 123}])
 
     def test_execute_batch(self):
-        self.add_response({
+        self.host.web.append_prpc_response({
             'responses': [{
                 'getBuild': {
                     'id': '123'
@@ -176,7 +169,7 @@ def test_execute_batch(self):
         self.assertEqual(build3, {'id': '789'})
 
     def test_execute_batch_with_error(self):
-        self.add_response({
+        self.host.web.append_prpc_response({
             'responses': [{
                 'error': {
                     'code': 5,

third_party/blink/tools/blinkpy/common/net/web_mock.py

@@ -26,8 +26,11 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+import json
 from six.moves.urllib.error import HTTPError
 
+from blinkpy.common.net.rpc import RESPONSE_PREFIX
+
 
 class MockWeb(object):
     def __init__(self, urls=None, responses=None):
@@ -48,6 +51,18 @@ def request(self, method, url, data=None, headers=None):  # pylint: disable=unus
         self.requests.append((url, data))
         return MockResponse(self.responses.pop(0))
 
+    def append_prpc_response(self, payload, status_code=200, headers=None):
+        headers = headers or {}
+        headers.setdefault('Content-Type', 'application/json')
+        self.responses.append({
+            'status_code':
+            200,
+            'body':
+            RESPONSE_PREFIX + json.dumps(payload).encode(),
+            'headers':
+            headers,
+        })
+
 
 class MockResponse(object):
     def __init__(self, values):

third_party/blink/tools/blinkpy/common/system/log_testing.py

@@ -67,7 +67,7 @@ def flush(self):
 
     def assertMessages(self, messages):
         """Asserts that the given messages match the logged messages."""
-        self._test_case.assertEqual(sorted(messages), sorted(self.messages))
+        self._test_case.assertEqual(messages, self.messages)
 
 
 class LogTesting(object):