[go: nahoru, domu]

Reland "[exception-db] Record exceptions during test run on Android"

This reverts commit c34d363de38daf73db4a58b5b52252b67ef51946.

Reason for revert: Fixed the dependency issue in telemetry test suites.
e.g. https://ci.chromium.org/ui/p/chromium/builders/try/android-x86-rel/299411/infra

Original change's description:
> Revert "[exception-db] Record exceptions during test run on Android"
>
> This reverts commit 5fdb778eb3bab157cd16bf101acefa4b61761300.
>
> Reason for revert: Suspected of telemetry test failures on
> chromium/ci/android-oreo-x86-rel
>
> Original change's description:
> > [exception-db] Record exceptions during test run on Android
> >
> > Add initial tracking for device errors during test run and AVD startup.
> >
> > Bug: 341362003
> > Change-Id: I4ce392c5d9378ccca81535040af27218e361fbeb
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5637217
> > Commit-Queue: Haiyang Pan <hypan@google.com>
> > Reviewed-by: Andrew Grieve <agrieve@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#1317655}
>
> Bug: 341362003, 348572844
> Change-Id: I94a1b525d39810a0e3198b5c45b3af1ebbe63e07
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5647669
> Owners-Override: Salvador Guerrero Ramos <salg@google.com>
> Reviewed-by: Haiyang Pan <hypan@google.com>
> Commit-Queue: Salvador Guerrero Ramos <salg@google.com>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Cr-Commit-Position: refs/heads/main@{#1318009}

Bug: 341362003, 348572844
Change-Id: Idd94b84ba9c10057741c152c441574aa4bf11454
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5648234
Commit-Queue: Haiyang Pan <hypan@google.com>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1318714}
diff --git a/build/android/pylib/local/device/local_device_test_run.py b/build/android/pylib/local/device/local_device_test_run.py
index 099c88a..3a3392a 100644
--- a/build/android/pylib/local/device/local_device_test_run.py
+++ b/build/android/pylib/local/device/local_device_test_run.py
@@ -25,6 +25,9 @@
 from pylib.base import test_run
 from pylib.local.device import local_device_environment
 
+# TODO(crbug.com/348470234): Fix the pylint import-error
+from lib.proto import exception_recorder  # pylint: disable=import-error
+
 
 _SIGTERM_TEST_LOG = (
   '  Suite execution terminated, probably due to swarming timeout.\n'
@@ -86,7 +89,8 @@
           else:
             raise Exception(
                 'Unexpected result type: %s' % type(result).__name__)
-        except device_errors.CommandTimeoutError:
+        except device_errors.CommandTimeoutError as e:
+          exception_recorder.register(e)
           # Test timeouts don't count as device errors for the purpose
           # of bad device detection.
           consecutive_device_errors = 0
@@ -113,11 +117,13 @@
                 base_test_result.BaseTestResult(
                     self._GetUniqueTestName(test),
                     base_test_result.ResultType.TIMEOUT))
-        except device_errors.DeviceUnreachableError:
+        except device_errors.DeviceUnreachableError as e:
+          exception_recorder.register(e)
           # If the device is no longer reachable then terminate this
           # run_tests_on_device call.
           raise
-        except base_error.BaseError:
+        except base_error.BaseError as e:
+          exception_recorder.register(e)
           # If we get a device error but believe the device is still
           # reachable, attempt to continue using it.
           if isinstance(tests, test_collection.TestCollection):
diff --git a/build/android/pylib/local/emulator/local_emulator_environment.py b/build/android/pylib/local/emulator/local_emulator_environment.py
index 322cb01..56aae52 100644
--- a/build/android/pylib/local/emulator/local_emulator_environment.py
+++ b/build/android/pylib/local/emulator/local_emulator_environment.py
@@ -13,6 +13,9 @@
 from pylib.local.device import local_device_environment
 from pylib.local.emulator import avd
 
+# TODO(crbug.com/348470234): Fix the pylint import-error
+from lib.proto import exception_recorder  # pylint: disable=import-error
+
 # Mirroring https://bit.ly/2OjuxcS#23
 _MAX_ANDROID_EMULATORS = 16
 
@@ -71,10 +74,12 @@
                      debug_tags=self._emulator_debug_tags,
                      enable_network=self._emulator_enable_network,
                      require_fast_start=True)
-        except avd.AvdException:
+        except avd.AvdException as e:
+          exception_recorder.register(e)
           logging.exception('Failed to start emulator instance.')
           return None
         except base_error.BaseError as e:
+          exception_recorder.register(e)
           # Timeout error usually indicates the emulator is not responding.
           # In this case, we should stop it forcely.
           logging.info("Force stop the emulator")
diff --git a/build/android/test_runner.py b/build/android/test_runner.py
index a6feb17..51f464c7 100755
--- a/build/android/test_runner.py
+++ b/build/android/test_runner.py
@@ -56,6 +56,8 @@
 
 from py_utils import contextlib_ext
 
+# TODO(crbug.com/348470234): Fix the pylint import-error
+from lib.proto import exception_recorder  # pylint: disable=import-error
 from lib.results import result_sink  # pylint: disable=import-error
 
 _DEVIL_STATIC_CONFIG_FILE = os.path.abspath(os.path.join(
@@ -1158,6 +1160,20 @@
   save_detailed_results = (args.local_output or not local_utils.IsOnSwarming()
                            ) and not args.isolated_script_test_output
 
+  @contextlib.contextmanager
+  def exceptions_uploader():
+    try:
+      yield
+    finally:
+      if result_sink_client and exception_recorder.size():
+        logging.info('Uploading exception records to RDB.')
+        prop = {
+            exception_recorder.EXCEPTION_OCCURRENCES_KEY:
+            exception_recorder.to_dict(),
+        }
+        result_sink_client.UpdateInvocationExtendedProperties(prop)
+        exception_recorder.clear()
+
   ### Set up test objects.
 
   out_manager = output_manager_factory.CreateOutputManager(args)
@@ -1187,7 +1203,8 @@
     # |raw_logs_fh| is only used by Robolectric tests.
     raw_logs_fh = io.StringIO() if save_detailed_results else None
 
-    with json_writer(), logcats_uploader, env, test_instance, test_run:
+    with json_writer(), exceptions_uploader(), logcats_uploader, \
+         env, test_instance, test_run:
 
       repetitions = (range(args.repeat +
                            1) if args.repeat >= 0 else itertools.count())
diff --git a/build/android/test_runner.pydeps b/build/android/test_runner.pydeps
index b58e857..7ae9226 100644
--- a/build/android/test_runner.pydeps
+++ b/build/android/test_runner.pydeps
@@ -139,6 +139,9 @@
 ../util/lib/__init__.py
 ../util/lib/common/chrome_test_server_spawner.py
 ../util/lib/common/unittest_util.py
+../util/lib/proto/__init__.py
+../util/lib/proto/exception_occurrences_pb2.py
+../util/lib/proto/exception_recorder.py
 ../util/lib/results/__init__.py
 ../util/lib/results/result_sink.py
 ../util/lib/results/result_types.py
diff --git a/build/util/BUILD.gn b/build/util/BUILD.gn
index a96d326..ea9abdc3 100644
--- a/build/util/BUILD.gn
+++ b/build/util/BUILD.gn
@@ -35,3 +35,11 @@
     "//build/util/lib/results/",
   ]
 }
+
+group("proto") {
+  data = [
+    "//.vpython3",
+    "//build/util/lib/__init__.py",
+    "//build/util/lib/proto/",
+  ]
+}
diff --git a/build/util/lib/proto/exception_recorder.py b/build/util/lib/proto/exception_recorder.py
index 671337ea..e86fff5 100644
--- a/build/util/lib/proto/exception_recorder.py
+++ b/build/util/lib/proto/exception_recorder.py
@@ -13,8 +13,8 @@
 from google.protobuf import any_pb2
 from google.protobuf.json_format import MessageToDict
 
-from exception_occurrences_pb2 import ExceptionOccurrence
-from exception_occurrences_pb2 import ExceptionOccurrences
+from lib.proto.exception_occurrences_pb2 import ExceptionOccurrence
+from lib.proto.exception_occurrences_pb2 import ExceptionOccurrences
 
 # This is used as the key when being uploaded to ResultDB via result_sink
 # and shouldn't be changed
@@ -65,6 +65,11 @@
   return ret
 
 
+def size() -> int:
+  """Get the current size of registered ExceptionOccurrence records."""
+  return len(_records)
+
+
 def clear() -> None:
   """Clear all the registered ExceptionOccurrence records."""
   _records.clear()
diff --git a/build/util/lib/proto/exception_recorder_unittests.py b/build/util/lib/proto/exception_recorder_unittests.py
index 645c1fb..414d0d6 100755
--- a/build/util/lib/proto/exception_recorder_unittests.py
+++ b/build/util/lib/proto/exception_recorder_unittests.py
@@ -6,12 +6,18 @@
 """File for testing exception_recorder.py."""
 
 import os
+import sys
 import tempfile
 import unittest
 import unittest.mock
 
-import exception_recorder
-from exception_occurrences_pb2 import ExceptionOccurrences
+_BUILD_UTIL_PATH = os.path.abspath(
+    os.path.join(os.path.dirname(__file__), '..', '..'))
+if _BUILD_UTIL_PATH not in sys.path:
+  sys.path.insert(0, _BUILD_UTIL_PATH)
+
+from lib.proto import exception_recorder
+from lib.proto.exception_occurrences_pb2 import ExceptionOccurrences
 
 
 class MyClass:
diff --git a/testing/BUILD.gn b/testing/BUILD.gn
index cf0d762..83538d4 100644
--- a/testing/BUILD.gn
+++ b/testing/BUILD.gn
@@ -23,6 +23,9 @@
 # Targets needed for isolate script to execute.
 group("test_scripts_shared") {
   data_deps = [
+    # Used for reporting exceptions and test script metrics to RDB.
+    "//build/util:proto",
+
     # Used for reporting test results to RDB.
     "//build/util:test_results",
   ]