[TestDisabler] Move away from query test history

Bug: 1363154
Change-Id: I1dfcccf85d5dd2e3c0e41e8fafa3ffec197bb49a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3907081
Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
Commit-Queue: Tuan Nguyen <nqmtuan@google.com>
Cr-Commit-Position: refs/heads/main@{#1049469}

tools/disable_tests/disable.py

@@ -31,12 +31,17 @@ def main(argv: List[str]) -> int:
       description='Disables tests.',
       epilog=f"Valid conditions are:\n{valid_conds}")
 
-  parser.add_argument('test_id',
+  parser.add_argument(
+      'build',
+      type=str,
+      help='the Buildbucket build ID to search for tests to disable ')
+  parser.add_argument('test_regex',
                       type=str,
-                      help='the test to disable. For example: ' +
-                      'ninja://chrome/test:browser_tests/Suite.Name. You can ' +
-                      'also just pass Suite.Name, and the tool will search ' +
-                      'for a test with a matching ID')
+                      help='the regex for the test to disable. For example: ' +
+                      '".*CompressionUtilsTest.GzipCompression.*". Currently' +
+                      'we assume that there is at most one test matching' +
+                      'the regex. Disabling multiple tests at the same time' +
+                      'is not currently supported (crbug.com/1364416)')
   parser.add_argument('conditions',
                       type=str,
                       nargs='*',
@@ -81,7 +86,7 @@ def main(argv: List[str]) -> int:
       return 1
 
   try:
-    disable_test(args.test_id, args.conditions, message)
+    disable_test(args.build, args.test_regex, args.conditions, message)
     return 0
   except errors.UserError as e:
     print(e, file=sys.stderr)
@@ -153,16 +158,11 @@ def parse_bug(bug: str) -> Tuple[int, str]:
 #   * Printing out all valid configs.
 #   * Overwrite the existing state rather than adding to it. Probably leave this
 #     until it's requested.
-def disable_test(test_id: str, cond_strs: List[str], message: Optional[str]):
+def disable_test(build: str, test_regex: str, cond_strs: List[str],
+                 message: Optional[str]):
   conds = conditions.parse(cond_strs)
-
-  #  If the given ID starts with "ninja:", then it's a full test ID. If not,
-  #  assume it's a test name, and transform it into a query that will match the
-  #  full ID.
-  if not test_id.startswith('ninja:'):
-    test_id = f'ninja://.*/{extract_name_and_suite(test_id)}(/.*)?'
-
-  test_name, filename = resultdb.get_test_metadata(test_id)
+  invocation = "invocations/build-" + build
+  test_name, filename = resultdb.get_test_metadata(invocation, test_regex)
   test_name = extract_name_and_suite(test_name)
 
   # Paths returned from ResultDB look like //foo/bar, where // refers to the

tools/disable_tests/integration_test

@@ -0,0 +1,10 @@
+#!/bin/sh
+# Copyright 2022 The Chromium Authors
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+# disable -- a shell wrapper for disable.py, to avoid calling disable.py
+# directly. This will give us flexibility to restructure things later.
+
+base_dir=$(dirname "$0")
+PYTHONDONTWRITEBYTECODE=1 exec vpython3 "$base_dir/integration_test.py" "$@"

tools/disable_tests/resultdb.py

@@ -12,51 +12,27 @@
 import errors
 
 
-def get_test_metadata(test_id: str) -> Tuple[str, str]:
+def get_test_metadata(invocation, test_regex: str) -> Tuple[str, str]:
   """Fetch test metadata from ResultDB.
 
   Args:
-    test_id: The full ID of the test to fetch. For Chromium tests this will
-    begin with "ninja://"
+    invocation: the invocation to fetch the test
+    test_regex: The regex to match the test id
   Returns:
     A tuple of (test name, filename). The test name will for example have the
     form  SuitName.TestName for GTest tests. The filename is the location in the
     source tree where this test is defined.
   """
-
-  history = get_test_result_history(test_id, 1)
-
-  if 'entries' not in history:
+  test_results = query_test_result(invocation=invocation, test_regex=test_regex)
+  if 'testResults' not in test_results:
     raise errors.UserError(
-        f"ResultDB query couldn't find test with ID: {test_id}")
-
-  # TODO: Are there other statuses we need to exclude?
-  if history['entries'][0]['result'].get('status', '') == 'SKIP':
-    # Test metadata isn't populated for skipped tests. Ideally this wouldn't be
-    # the case, but for now we just work around it.
-    # On the off-chance this test is conditionally disabled, we request a bit
-    # more history in the hopes of finding a test run where it isn't skipped.
-    history = get_test_result_history(test_id, 10)
-
-    for entry in history['entries'][1:]:
-      if entry['result'].get('status', '') != 'SKIP':
-        break
-    else:
-      raise errors.UserError(
-          f"Unable to fetch metadata for test {test_id}, as the last 10 runs " +
-          "in ResultDB have status 'SKIP'. Is the test already disabled?")
-  else:
-    entry = history['entries'][0]
+        f"ResultDB couldn't query for invocation: {invocation}")
 
-  try:
-    inv_name = entry['result']['name']
-  except KeyError as e:
-    raise errors.InternalError(
-        f"Malformed GetTestResultHistory response: no key {e}") from e
-  # Ideally GetTestResultHistory would return metadata so we could avoid making
-  # this second RPC.
-  result = get_test_result(inv_name)
+  if len(test_results["testResults"]) == 0:
+    raise errors.UserError(
+        f"ResultDB couldn't find test result for test regex {test_regex}")
 
+  result = test_results["testResults"][0]
   try:
     name = result['testMetadata']['name']
     loc = result['testMetadata']['location']
@@ -115,6 +91,29 @@ def get_test_result(test_name: str) -> dict:
   })
 
 
+def query_test_result(invocation: str, test_regex: str):
+  """Make a QueryTestResults RPC call to ResultDB.
+
+  Args:
+    invocation_name: the name of the invocation to query for test results
+    test_regex: The test regex to filter
+
+  Returns:
+    The QueryTestResults response message, in dict form.
+  """
+  request = {
+      'invocations': [invocation],
+      'readMask': {
+          'paths': ['test_id', 'test_metadata'],
+      },
+      'pageSize': 1000,
+      'predicate': {
+          'testIdRegexp': test_regex,
+      },
+  }
+  return rdb_rpc('QueryTestResults', request)
+
+
 # Used for caching RPC responses, for development purposes.
 CANNED_RESPONSE_FILE: Optional[str] = None