Revert "[resultdb] Add ResultDB indicator"

mi-ac · V8 LUCI CQ · commit f09dde9fbb54 · 2022-10-13T11:31:59.000Z
This reverts commit 237de89. Reason for revert: breaks flake bisect: https://ci.chromium.org/ui/p/v8/builders/try.triggered/v8_flako/b8800423657665797553/overview The change added the implicit requirement to run testing with vpython3, which is not given everywhere. Original change's description: > [resultdb] Add ResultDB indicator > > Adds a new indicator that will send every result to ResultDB (and ultimately in a bq table; to be configured later). > > If we are not running in a ResultDB context we introduce only a minimal overhead by exiting early from indicator. > > To test these changes in a luci context with ResultDB we activated resultdb feature flag via V8-Recipe-Flags. This feature got implemented in https://crrev.com/c/3925576 . > > > V8-Recipe-Flags: resultdb > Bug: v8:13316 > Change-Id: I5d98e8f27531b536686a8d63b993313b9d6f62c5 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3905385 > Commit-Queue: Liviu Rau <liviurau@google.com> > Reviewed-by: Alexander Schulze <alexschulze@chromium.org> > Cr-Commit-Position: refs/heads/main@{#83672} Bug: v8:13316 Change-Id: I7e55668e365475298ed46d2fc8ee0fe1282c3e8e No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3952131 Auto-Submit: Michael Achenbach <machenbach@chromium.org> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Cr-Commit-Position: refs/heads/main@{#83677}
diff --git a/.vpython3 b/.vpython3
@@ -74,8 +74,3 @@ wheel: <
   name: "infra/python/wheels/protobuf-py3"
   version: "version:3.19.3"
 >
-
-wheel: <
-  name: "infra/python/wheels/requests-py2_py3"
-  version: "version:2.13.0"
->
diff --git a/tools/testrunner/objects/testcase.py b/tools/testrunner/objects/testcase.py
@@ -447,12 +447,8 @@ def cmp(x, y):
         (other.suite.name, other.name, other.variant)
     )
 
-  @property
-  def full_name(self):
-    return self.suite.name + '/' + self.name
-
   def __str__(self):
-    return self.full_name
+    return self.suite.name + '/' + self.name
 
 
 class D8TestCase(TestCase):
diff --git a/tools/testrunner/testproc/indicators.py b/tools/testrunner/testproc/indicators.py
@@ -14,7 +14,7 @@
 
 
 def print_failure_header(test, is_flaky=False):
-  text = [test.full_name]
+  text = [str(test)]
   if test.output_proc.negative:
     text.append('[negative]')
   if is_flaky:
@@ -24,23 +24,6 @@ def print_failure_header(test, is_flaky=False):
   print(output.encode(encoding, errors='replace').decode(encoding))
 
 
-def formatted_result_output(result):
-  lines = []
-  if result.output.stderr:
-    lines.append("--- stderr ---")
-    lines.append(result.output.stderr.strip())
-  if result.output.stdout:
-    lines.append("--- stdout ---")
-    lines.append(result.output.stdout.strip())
-  lines.append("Command: %s" % result.cmd.to_string())
-  if result.output.HasCrashed():
-    lines.append("exit code: %s" % result.output.exit_code_string)
-    lines.append("--- CRASHED ---")
-  if result.output.HasTimedOut():
-    lines.append("--- TIMEOUT ---")
-  return '\n'.join(lines)
-
-
 class ProgressIndicator():
 
   def __init__(self, context, options, test_count):
@@ -85,7 +68,19 @@ def finished(self):
     for test, result, is_flaky in self._failed:
       flaky += int(is_flaky)
       print_failure_header(test, is_flaky=is_flaky)
-      print(formatted_result_output(result))
+      if result.output.stderr:
+        print("--- stderr ---")
+        print(result.output.stderr.strip())
+      if result.output.stdout:
+        print("--- stdout ---")
+        print(result.output.stdout.strip())
+      print("Command: %s" % result.cmd.to_string())
+      if result.output.HasCrashed():
+        print("exit code: %s" % result.output.exit_code_string)
+        print("--- CRASHED ---")
+        crashed += 1
+      if result.output.HasTimedOut():
+        print("--- TIMEOUT ---")
     if len(self._failed) == 0:
       print("===")
       print("=== All tests succeeded")
@@ -235,7 +230,7 @@ def on_test_result(self, test, result):
     else:
       self._passed += 1
 
-    self._print_progress(test.full_name)
+    self._print_progress(str(test))
     if result.has_unexpected_output:
       output = result.output
       stdout = output.stdout.strip()
@@ -363,7 +358,10 @@ def __init__(self, context, options, test_count, framework_name):
     self.test_count = 0
 
   def on_test_result(self, test, result):
-    self.process_results(test, result.as_list)
+    if result.is_rerun:
+      self.process_results(test, result.results)
+    else:
+      self.process_results(test, [result])
 
   def process_results(self, test, results):
     for run, result in enumerate(results):
@@ -378,7 +376,7 @@ def process_results(self, test, results):
       if not result.has_unexpected_output and run == 0:
         continue
 
-      record = self._test_record(test, result, run)
+      record = self._test_record(test, result, output, run)
       record.update({
           "result": test.output_proc.get_outcome(output),
           "stdout": output.stdout,
@@ -394,33 +392,41 @@ def result_value(test, result, output):
         return ""
       return test.output_proc.get_outcome(output)
 
-    record = self._test_record(test, result, run)
-    record.update(
-        result=result_value(test, result, output),
-        marked_slow=test.is_slow,
-    )
+    record = self._test_record(test, result, output, run)
+    record.update({
+        "result": result_value(test, result, output),
+        "marked_slow": test.is_slow,
+    })
     self.tests.add(record)
     self.duration_sum += record['duration']
     self.test_count += 1
 
-  def _test_record(self, test, result, run):
-    record = util.base_test_record(test, result, run)
-    record.update(
-        framework_name=self.framework_name,
-        command=result.cmd.to_string(relative=True),
-    )
-    return record
+  def _test_record(self, test, result, output, run):
+    return {
+        "name": str(test),
+        "flags": result.cmd.args,
+        "command": result.cmd.to_string(relative=True),
+        "run": run + 1,
+        "exit_code": output.exit_code,
+        "expected": test.expected_outcomes,
+        "duration": output.duration,
+        "random_seed": test.random_seed,
+        "target_name": test.get_shell(),
+        "variant": test.variant,
+        "variant_flags": test.variant_flags,
+        "framework_name": self.framework_name,
+    }
 
   def finished(self):
     duration_mean = None
     if self.test_count:
       duration_mean = self.duration_sum / self.test_count
 
     result = {
-        'results': self.results,
-        'slowest_tests': self.tests.as_list(),
-        'duration_mean': duration_mean,
-        'test_total': self.test_count,
+        "results": self.results,
+        "slowest_tests": self.tests.as_list(),
+        "duration_mean": duration_mean,
+        "test_total": self.test_count,
     }
 
     with open(self.options.json_test_results, "w") as f:
diff --git a/tools/testrunner/testproc/progress.py b/tools/testrunner/testproc/progress.py
@@ -6,7 +6,6 @@
 from . import base
 from testrunner.local import utils
 from testrunner.testproc.indicators import JsonTestProgressIndicator, PROGRESS_INDICATORS
-from testrunner.testproc.resultdb import ResultDBIndicator
 
 
 class ResultsTracker(base.TestProcObserver):
@@ -67,7 +66,7 @@ def __init__(self, context, options, framework_name, test_count):
           0,
           JsonTestProgressIndicator(context, options, test_count,
                                     framework_name))
-    self.procs.append(ResultDBIndicator(context, options, test_count))
+
     self._requirement = max(proc._requirement for proc in self.procs)
 
   def _on_result_for(self, test, result):
diff --git a/tools/testrunner/testproc/result.py b/tools/testrunner/testproc/result.py
@@ -16,10 +16,6 @@ def is_grouped(self):
   def is_rerun(self):
     return False
 
-  @property
-  def as_list(self):
-    return [self]
-
 
 class Result(ResultBase):
   """Result created by the output processor."""
@@ -116,9 +112,5 @@ def __init__(self, results):
   def is_rerun(self):
     return True
 
-  @property
-  def as_list(self):
-    return self.results
-
   def status(self):
     return ' '.join(r.status() for r in self.results)
diff --git a/tools/testrunner/testproc/resultdb.py b/tools/testrunner/testproc/resultdb.py
diff --git a/tools/testrunner/testproc/util.py b/tools/testrunner/testproc/util.py
@@ -7,7 +7,6 @@
 import logging
 import os
 import platform
-import re
 import signal
 import subprocess
 
@@ -54,43 +53,6 @@ def kill_processes_linux():
       logging.exception('Failed to kill process')
 
 
-def strip_ascii_control_characters(unicode_string):
-  return re.sub(r'[^\x20-\x7E]', '?', str(unicode_string))
-
-
-def base_test_record(test, result, run):
-  record = {
-      'name': test.full_name,
-      'flags': result.cmd.args,
-      'run': run + 1,
-      'expected': test.expected_outcomes,
-      'random_seed': test.random_seed,
-      'target_name': test.get_shell(),
-      'variant': test.variant,
-      'variant_flags': test.variant_flags,
-  }
-  if result.output:
-    record.update(
-        exit_code=result.output.exit_code,
-        duration=result.output.duration,
-    )
-  return record
-
-
-def extract_tags(record):
-  tags = []
-  for k, v in record.items():
-    if type(v) == list:
-      tags += [sanitized_kv_dict(k, e) for e in v]
-    else:
-      tags.append(sanitized_kv_dict(k, v))
-  return tags
-
-
-def sanitized_kv_dict(k, v):
-  return dict(key=k, value=strip_ascii_control_characters(v))
-
-
 class FixedSizeTopList():
   """Utility collection for gathering a fixed number of elements with the
   biggest value for the given key. It employs a heap from which we pop the

Original file line number	Diff line number	Diff line change
`@@ -74,8 +74,3 @@ wheel: <`
`74`	`74`	`name: "infra/python/wheels/protobuf-py3"`
`75`	`75`	`version: "version:3.19.3"`
`76`	`76`	`>`
`77`		`-`
`78`		`-wheel: <`
`79`		`- name: "infra/python/wheels/requests-py2_py3"`
`80`		`- version: "version:2.13.0"`
`81`		`->`