Revert 178345

Causing sql_unittest to report failure on "Android Tests (dbg)"
http://build.chromium.org/p/chromium.linux/builders/Android%20Tests%20(dbg)

> [Android] Keep track of unknown test results at the TestRunner layer.
> 
> If the entire gtest run crashes/failes/times out, the tests that
> were not run are marked as 'unknown'. These tests are reported
> by GetAllBroken() and are retried by BaseTestSharder.
> 
> Get rid of overall_* flags which are now redundant.
> 
> BUG=171466
> 
> Review URL: https://codereview.chromium.org/11896050

TBR=frankf@chromium.org
Review URL: https://codereview.chromium.org/12049046

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@178363 0039d316-1c4b-4281-b951-d872f2087c98

build/android/pylib/base/base_test_sharder.py

@@ -47,13 +47,6 @@ def __init__(self, attached_devices, build_type='Debug'):
     self.attached_devices = attached_devices
     # Worst case scenario: a device will drop offline per run, so we need
     # to retry until we're out of devices.
-
-    # TODO(frankf): There are two sources of flakiness:
-    # 1. Device flakiness
-    # 2. Test/product flakiness
-    # We should differentiate between these. Otherwise, blindly retrying tests
-    # might mask test/product flakiness. For type 2, we should follow the
-    # general chrome best practices.
     self.retries = len(self.attached_devices)
     self.tests = []
     self.build_type = build_type
@@ -97,8 +90,6 @@ def RunShardedTests(self):
     self._KillHostForwarder()
     for retry in xrange(self.retries):
       logging.warning('Try %d of %d', retry + 1, self.retries)
-      logging.warning('Attempting to run %d tests: %s'
-                      % (len(self.tests), self.tests))
       self.SetupSharding(self.tests)
       test_runners = []
 
@@ -137,7 +128,9 @@ def RunShardedTests(self):
       # Retry on devices that didn't have any exception.
       self.attached_devices = list(retry_devices)
 
-      # TODO(frankf): Do not break TestResults encapsulation.
+      # TODO(frankf): Fix the retry logic:
+      # - GetAllBroken() should report all tests not ran.
+      # - Do not break TestResults encapsulation.
       if (retry == self.retries - 1 or
           len(self.attached_devices) == 0):
         all_passed = final_results.ok + test_results.ok
@@ -146,6 +139,10 @@ def RunShardedTests(self):
         break
       else:
         final_results.ok += test_results.ok
+        final_results.overall_timed_out = (final_results.overall_timed_out or
+                                           test_results.overall_timed_out)
+        final_results.overall_fail = (final_results.overall_fail or
+                                      test_results.overall_fail)
 
         self.tests = []
         for t in test_results.GetAllBroken():

build/android/pylib/base/test_result.py

@@ -55,15 +55,22 @@ def __init__(self):
     self.crashed = []
     self.unknown = []
     self.timed_out = []
+    self.overall_timed_out = False
+    self.overall_fail = False
     self.device_exception = None
 
   @staticmethod
-  def FromRun(ok=None, failed=None, crashed=None, timed_out=None):
+  def FromRun(ok=None, failed=None, crashed=None, timed_out=None,
+              overall_timed_out=False, overall_fail=False,
+              device_exception=None):
     ret = TestResults()
     ret.ok = ok or []
     ret.failed = failed or []
     ret.crashed = crashed or []
     ret.timed_out = timed_out or []
+    ret.overall_timed_out = overall_timed_out
+    ret.overall_fail = overall_fail
+    ret.device_exception = device_exception
     return ret
 
   @staticmethod
@@ -76,6 +83,10 @@ def FromTestResults(results):
       ret.crashed += t.crashed
       ret.unknown += t.unknown
       ret.timed_out += t.timed_out
+      if t.overall_timed_out:
+        ret.overall_timed_out = True
+      if t.overall_fail:
+        ret.overall_fail = True
     return ret
 
   @staticmethod
@@ -117,13 +128,9 @@ def _Log(self, sorted_list):
         logging.critical(t.log)
 
   def GetAllBroken(self):
-    """Returns all the broken tests."""
+    """Returns the all broken tests."""
     return self.failed + self.crashed + self.unknown + self.timed_out
 
-  def GetAll(self):
-    """Returns all the tests."""
-    return self.ok + self.GetAllBroken()
-
   def _LogToFile(self, test_type, test_suite, build_type):
     """Log results to local files which can be used for aggregation later."""
     # TODO(frankf): Report tests that failed to run here too.
@@ -194,7 +201,7 @@ def _LogToFlakinessDashboard(self, test_type, test_package, flakiness_server):
       logging.error(e)
 
   def LogFull(self, test_type, test_package, annotation=None,
-              build_type='Debug', flakiness_server=None):
+              build_type='Debug', all_tests=None, flakiness_server=None):
     """Log the tests results for the test suite.
 
     The results will be logged three different ways:
@@ -210,6 +217,9 @@ def LogFull(self, test_type, test_package, annotation=None,
       annotation: If instrumenation test type, this is a list of annotations
                   (e.g. ['Smoke', 'SmallTest']).
       build_type: Release/Debug
+      all_tests: A list of all tests that were supposed to run.
+                 This is used to determine which tests have failed to run.
+                 If None, we assume all tests ran.
       flakiness_server: If provider, upload the results to flakiness dashboard
                         with this URL.
       """
@@ -232,20 +242,31 @@ def LogFull(self, test_type, test_package, annotation=None,
       logging.critical('Passed')
 
     # Summarize in the test output.
-    num_tests_ran = len(self.GetAll())
+    logging.critical('*' * 80)
+    summary = ['Summary:\n']
+    if all_tests:
+      summary += ['TESTS_TO_RUN=%d\n' % len(all_tests)]
+    num_tests_ran = (len(self.ok) + len(self.failed) +
+                     len(self.crashed) + len(self.unknown) +
+                     len(self.timed_out))
     tests_passed = [t.name for t in self.ok]
     tests_failed = [t.name for t in self.failed]
     tests_crashed = [t.name for t in self.crashed]
-    tests_timed_out = [t.name for t in self.timed_out]
     tests_unknown = [t.name for t in self.unknown]
-    logging.critical('*' * 80)
-    summary = ['Summary:\n']
+    tests_timed_out = [t.name for t in self.timed_out]
     summary += ['RAN=%d\n' % (num_tests_ran),
                 'PASSED=%d\n' % len(tests_passed),
                 'FAILED=%d %s\n' % (len(tests_failed), tests_failed),
                 'CRASHED=%d %s\n' % (len(tests_crashed), tests_crashed),
                 'TIMEDOUT=%d %s\n' % (len(tests_timed_out), tests_timed_out),
                 'UNKNOWN=%d %s\n' % (len(tests_unknown), tests_unknown)]
+    if all_tests and num_tests_ran != len(all_tests):
+      # Add the list of tests we failed to run.
+      tests_failed_to_run = list(set(all_tests) - set(tests_passed) -
+                            set(tests_failed) - set(tests_crashed) -
+                            set(tests_unknown) - set(tests_timed_out))
+      summary += ['FAILED_TO_RUN=%d %s\n' % (len(tests_failed_to_run),
+                                             tests_failed_to_run)]
     summary_string = ''.join(summary)
     logging.critical(summary_string)
     logging.critical('*' * 80)
@@ -264,7 +285,8 @@ def LogFull(self, test_type, test_package, annotation=None,
 
   def PrintAnnotation(self):
     """Print buildbot annotations for test results."""
-    if self.GetAllBroken():
+    if (self.failed or self.crashed or self.overall_fail or
+        self.overall_timed_out):
       buildbot_report.PrintError()
     else:
       print 'Step success!'  # No annotation needed

build/android/pylib/gtest/single_test_runner.py

@@ -253,17 +253,17 @@ def RunTests(self):
       self.test_results = self.test_package.RunTestsAndListResults()
     except errors.DeviceUnresponsiveError as e:
       # Make sure this device is not attached
-      logging.warning(e)
       if android_commands.IsDeviceAttached(self.device):
         raise e
-      self.test_results.device_exception = device_exception
-    # Calculate unknown test results.
-    finally:
-      # TODO(frankf): Do not break TestResults encapsulation.
-      all_tests = set(self._gtest_filter.split(':'))
-      all_tests_ran = set([t.name for t in self.test_results.GetAll()])
-      unknown_tests = all_tests - all_tests_ran
-      self.test_results.unknown = [BaseTestResult(t, '') for t in unknown_tests]
+
+      # TODO(frankf): We should report these as "skipped" not "failures".
+      # Wrap the results
+      logging.warning(e)
+      failed_tests = []
+      for t in self._gtest_filter.split(':'):
+        failed_tests += [BaseTestResult(t, '')]
+      self.test_results = TestResults.FromRun(
+          failed=failed_tests, device_exception=self.device)
 
     return self.test_results
 

build/android/pylib/gtest/test_package.py

@@ -122,6 +122,8 @@ def _WatchTestOutput(self, p):
     failed_tests = []
     crashed_tests = []
     timed_out_tests = []
+    overall_fail = False
+    overall_timed_out = False
 
     # Test case statuses.
     re_run = re.compile('\[ RUN      \] ?(.*)\r\n')
@@ -144,6 +146,7 @@ def _WatchTestOutput(self, p):
         if found == 1:  # re_passed
           break
         elif found == 2:  # re_runner_fail
+          overall_fail = True
           break
         else:  # re_run
           if self.dump_debug_info:
@@ -156,6 +159,7 @@ def _WatchTestOutput(self, p):
               ok_tests += [BaseTestResult(full_test_name, p.before)]
           elif found == 2:  # re_crash
             crashed_tests += [BaseTestResult(full_test_name, p.before)]
+            overall_fail = True
             break
           else:  # re_fail
             failed_tests += [BaseTestResult(full_test_name, p.before)]
@@ -165,6 +169,7 @@ def _WatchTestOutput(self, p):
     except pexpect.TIMEOUT:
       logging.error('Test terminated after %d second timeout.',
                     self.timeout)
+      overall_timed_out = True
       if full_test_name:
         timed_out_tests += [BaseTestResult(full_test_name, p.before)]
     finally:
@@ -175,7 +180,10 @@ def _WatchTestOutput(self, p):
       logging.critical(
           'gtest exit code: %d\npexpect.before: %s\npexpect.after: %s',
           ret_code, p.before, p.after)
+      overall_fail = True
 
     # Create TestResults and return
     return TestResults.FromRun(ok=ok_tests, failed=failed_tests,
-                               crashed=crashed_tests, timed_out=timed_out_tests)
+                               crashed=crashed_tests, timed_out=timed_out_tests,
+                               overall_fail=overall_fail,
+                               overall_timed_out=overall_timed_out)

build/android/pylib/utils/flakiness_dashboard_results_uploader.py

@@ -151,16 +151,13 @@ def __init__(self, tests_type):
     self._test_results_map = {}
 
   def AddResults(self, test_results):
-    # TODO(frankf): Differentiate between fail/crash/timeouts.
     conversion_map = [
         (test_results.ok, False,
             json_results_generator.JSONResultsGeneratorBase.PASS_RESULT),
         (test_results.failed, True,
             json_results_generator.JSONResultsGeneratorBase.FAIL_RESULT),
         (test_results.crashed, True,
-            json_results_generator.JSONResultsGeneratorBase.FAIL_RESULT),
-        (test_results.timed_out, True,
-            json_results_generator.JSONResultsGeneratorBase.FAIL_RESULT),
+            "C"),
         (test_results.unknown, True,
             json_results_generator.JSONResultsGeneratorBase.NO_DATA_RESULT),
         ]

build/android/run_tests.py

@@ -196,6 +196,7 @@ def OnTestsCompleted(self, test_runners, test_results):
         test_type='Unit test',
         test_package=test_runners[0].test_package.test_suite_basename,
         build_type=self.build_type,
+        all_tests=self.all_tests,
         flakiness_server=self.flakiness_server)
     test_results.PrintAnnotation()
 
@@ -264,7 +265,7 @@ def _RunATestSuite(options):
   for buildbot_emulator in buildbot_emulators:
     buildbot_emulator.Shutdown()
 
-  return len(test_results.GetAllBroken())
+  return len(test_results.failed)
 
 
 def Dispatch(options):