[Findit] Change test.py recipe to speed up test try jobs(Recipe side change).

scripts/slave/recipes/findit/chromium/test.py

 # Copyright 2015 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+from collections import defaultdict
 import json
 
 from recipe_engine.config import Dict
+from recipe_engine.config import List
 from recipe_engine.config import Single
 from recipe_engine.recipe_api import Property
 
 
 DEPS = [
     'adb',
     'depot_tools/bot_update',
     'chromium',
     'chromium_android',
     'chromium_tests',
@@ skipping 28 matching lines @@
     'tests': Property(
         kind=Dict(value_type=list),
         help='The failed tests, the test name should be full name, e.g.: {'
              '  "browser_tests": ['
              '    "suite.test1", "suite.test2"'
              '  ]'
              '}'),
     'use_analyze': Property(
         kind=Single(bool, empty_val=False, required=False), default=True,
         help='Use analyze to skip commits that do not affect tests.'),
+    'suspected_revisions': Property(
+        kind=List(basestring), default=[],
+        help='A list of suspected revisions from heuristic analysis.'),
 }
 
 
 class TestResult(object):
   SKIPPED = 'skipped'  # A commit doesn't impact the test.
   PASSED = 'passed'  # The compile or test passed.
   FAILED = 'failed'  # The compile or test failed.
 
 
 def _compile_and_test_at_revision(api, target_mastername, target_buildername,
@@ skipping 53 matching lines @@
           bot_config,
           bot_update_step,
           bot_db,
           actual_compile_targets,
           tests_including_triggered=actual_tests_to_run,
           mb_mastername=target_mastername,
           mb_buildername=target_buildername,
           override_bot_type='builder_tester')
 
     # Run the tests.
+    failed_tests_dict = defaultdict(list)
     with api.chromium_tests.wrap_chromium_tests(
         bot_config, actual_tests_to_run):
       failed_tests = api.test_utils.run_tests(
           api, actual_tests_to_run,
           suffix=revision, test_filters=requested_tests)
 
     # Process failed tests.
     for failed_test in failed_tests:
       valid = failed_test.has_valid_results(api, suffix=revision)
       results[failed_test.name] = {
           'status': TestResult.FAILED,
           'valid': valid,
       }
       if valid:
-        results[failed_test.name]['failures'] = list(
-            failed_test.failures(api, suffix=revision))
+        test_list = list(failed_test.failures(api, suffix=revision))
+        results[failed_test.name]['failures'] = test_list
+        failed_tests_dict[failed_test.name].extend(test_list)
 
     # Process passed tests.
     for test in actual_tests_to_run:
       if test not in failed_tests:
         results[test.name] = {
             'status': TestResult.PASSED,
             'valid': True,
         }
 
     # Process skipped tests in two scenarios:
     # 1. Skipped by "analyze": tests are not affected by the given revision.
     # 2. Skipped because the requested tests don't exist at the given revision.
     for test_name in requested_tests.keys():
       if test_name not in results:
         results[test_name] = {
             'status': TestResult.SKIPPED,
             'valid': True,
         }
 
-    return results
+    return results, failed_tests_dict
 
 
-def RunSteps(api, target_mastername, target_testername,
-             good_revision, bad_revision, tests, use_analyze):
+def _get_shrinked_test_dict(original_test_dict, failed_tests_dict):

[lijeffrey, 2016/05/03 23:25:44]

'shrinked' sounds a bit strange, how about rename this (and the corresponding
variables' to 'unique' intsead?

[chanli, 2016/05/04 17:14:40]

It's actually not 'unique'... What I do here is just to take out tests that are
in both dicts from original_test_dict. But I agree I need to think of a better
name.

+  # Remove tests that are in both dicts from the original test dict.
+  if not failed_tests_dict:
+    return original_test_dict
+  shrinked_dict = defaultdict(list)
+  for step, tests in original_test_dict.iteritems():
+    if step in failed_tests_dict:
+      for test in tests:
+        if test not in failed_tests_dict[step]:
+          shrinked_dict[step].append(test)
+    else:
+      shrinked_dict[step].extend(tests)
+  return shrinked_dict
+
+
+def RunSteps(api, target_mastername, target_testername, good_revision,
+             bad_revision, tests, use_analyze, suspected_revisions):
   assert tests, 'No failed tests were specified.'
 
   # Figure out which builder configuration we should match for compile config.
   # Sometimes, the builder itself runs the tests and there is no tester. In
   # such cases, just treat the builder as a "tester". Thus, we default to
   # the target tester.
   tester_config = api.chromium_tests.builders.get(
       target_mastername).get('builders', {}).get(target_testername)
   target_buildername = (tester_config.get('parent_buildername') or
                         target_testername)
@@ skipping 11 matching lines @@
       api.swarming.set_default_dimension(key, value)
   # TODO(stgao): Fix the issue that precommit=False adds the tag 'purpose:CI'.
   api.chromium_tests.configure_swarming('chromium', precommit=False)
 
   # Sync to bad revision, and retrieve revisions in the regression range.
   api.chromium_tests.prepare_checkout(
       bot_config,
       root_solution_revision=bad_revision)
   revisions_to_check = api.findit.revisions_between(good_revision, bad_revision)
 
+  suspected_revision_index = [
+      revisions_to_check.index(r)
+          for r in set(suspected_revisions) if r in revisions_to_check]
+
+  # Segments revisions_to_check by suspected_revisions.
+  # Each sub_range will contain following elements:
+  # 1. Revision before a suspected_revision or None as a placeholder 
+  #    when no such revision
+  # 2. Suspected_revision
+  # 3. Revisions between a suspected_revision and the revision before next
+  #    suspected_revision, or remaining revisions before all suspect_revisions.
+  # For example, if revisions_to_check are [r0, r1, ..., r6] and
+  # suspected_revisions are [r2, r5], sub_ranges will be:
+  # [[None, r0], [r1, r2, r3], [r4, r5, r6]]
+  if suspected_revision_index:
+    sub_ranges = []
+    remaining_revisions = revisions_to_check[:]
+    for index in sorted(suspected_revision_index, reverse=True):
+      if index > 0:
+        if index < len(remaining_revisions):
+          sub_ranges.append(remaining_revisions[index - 1:])
+        else:  # Consecutive revisions are suspected, merges them in one range.
+          sub_ranges[-1].insert(0, remaining_revisions[index - 1])
+        remaining_revisions = remaining_revisions[:index - 1]
+    # None is a placeholder for the last known good revision.
+    sub_ranges.append([None] + remaining_revisions)
+  else:
+    # Treats the entire regression range as a single sub-range.
+    sub_ranges = [[None] + revisions_to_check]
+
   test_results = {}
   try_job_metadata = {
       'regression_range_size': len(revisions_to_check)
   }
   report = {
       'result': test_results,
       'metadata': try_job_metadata
   }
 
   try:
-    # We compile & run tests from the first revision to the last revision in the
-    # regression range serially instead of a typical bisecting, because jumping
-    # between new and old revisions might affect Goma capacity and build cycle
-    # times. Thus we plan to go with this simple serial approach first so that
-    # compile would be fast due to incremental compile.
-    # If this won't work out, we will figure out a better solution for speed of
-    # both compile and test.
-    for current_revision in revisions_to_check:
-      test_results[current_revision] = _compile_and_test_at_revision(
-          api, target_mastername, target_buildername, target_testername,
-          current_revision, tests, use_analyze)
-      # TODO(http://crbug.com/566975): check whether culprits for all failed
-      # tests are found and stop running tests at later revisions if so.
+    # Tests that haven't found culprits in tested revision(s).
+    no_culprit_tests = tests
+    # Iterates through sub_ranges and find culprits for each failed test.
+    for sub_range in sub_ranges:
+      if not no_culprit_tests:  # All tests have found culprits

[lijeffrey, 2016/05/03 23:25:44]

nit: comment should end with a .

[chanli, 2016/05/04 17:14:40]

Done.

+        break
+
+      potential_green_rev = sub_range[0]

[lijeffrey, 2016/05/03 23:25:44]

nit: Add a comment that potential_green_rev is the revision right before the
suspected revision provided by the heuristic result.

[chanli, 2016/05/04 17:14:40]

Done.

+      following_revisions = sub_range[1:]
+      if potential_green_rev:
+        test_results[potential_green_rev], tests_failed_in_potential_green = (
+            _compile_and_test_at_revision(
+                api, target_mastername, target_buildername, target_testername,
+                potential_green_rev, no_culprit_tests, use_analyze))
+      else:
+        tests_failed_in_potential_green = {}
+
+      tests_passed_in_potential_green = _get_shrinked_test_dict(
+          no_culprit_tests, tests_failed_in_potential_green)
+
+      # Culprits for tests that failed in potential green should be earlier.

[lijeffrey, 2016/05/03 23:25:44]

nit: failed in the revision before suspected revisions should be earlier. etc...

[chanli, 2016/05/04 17:14:40]

Modified the whole sentence.

+      # Thus no need to run passed tests in following revisions,
+      # only runs the tests that failed in following revisions.

[lijeffrey, 2016/05/03 23:25:44]

nit: Culprits for tests that failed in potential green should be earlier, thus
no need to run passed tests in the following revisions. Instead, only run the
tests that failed in the following revisions.

[chanli, 2016/05/04 17:14:40]

Done.

+      if tests_passed_in_potential_green:
+        tests_to_run = tests_passed_in_potential_green
+        for revision in following_revisions:
+          test_results[revision], tests_failed_in_revision = (
+              _compile_and_test_at_revision(
+                  api, target_mastername, target_buildername, target_testername,
+                  revision, tests_to_run, use_analyze))
+
+          # Takes out tests that passed in potential green and failed in
+          # following revisions: culprits have been found for them.
+          no_culprit_tests = _get_shrinked_test_dict(
+              no_culprit_tests, tests_failed_in_revision)
+
+          # Only runs tests that have not found culprits in later revisions.
+          tests_to_run = _get_shrinked_test_dict(
+              tests_to_run, tests_failed_in_revision)
+
+          if not tests_to_run:
+            break
+
   finally:
     # Give the full report including test results and metadata.
     step_result = api.python.succeeding_step(
         'report', [json.dumps(report, indent=2)], as_log='report')
 
     # Set the report as a build property too, so that it will be reported back
     # to Buildbucket and Findit will pull from there instead of buildbot master.
     step_result.presentation.properties['report'] = report
 
   return report
 
 
 def GenTests(api):
-  def props(tests, platform_name, tester_name, use_analyze=False):
+  def props(
+      tests, platform_name, tester_name, use_analyze=False, good_revision=None,
+      bad_revision=None, suspected_revisions=None):
     properties = {
         'mastername': 'tryserver.chromium.%s' % platform_name,
         'buildername': '%s_chromium_variable' % platform_name,
         'slavename': 'build1-a1',
         'buildnumber': 1,
         'target_mastername': 'chromium.%s' % platform_name,
         'target_testername': tester_name,
-        'good_revision': 'r0',
-        'bad_revision': 'r1',
+        'good_revision': good_revision or 'r0',
+        'bad_revision': bad_revision or 'r1',
         'tests': tests,
         'use_analyze': use_analyze,
     }
+    if suspected_revisions:
+      properties['suspected_revisions'] = suspected_revisions
     return api.properties(**properties) + api.platform.name(platform_name)
 
   def simulated_gtest_output(failed_test_names=(), passed_test_names=()):
     cur_iteration_data = {}
     for test_name in failed_test_names:
       cur_iteration_data[test_name] = [{
           'elapsed_time_ms': 0,
           'output_snippet': '',
           'status': 'FAILURE',
       }]
@@ skipping 201 matching lines @@
                       'swarming': {'can_use_on_swarming_builders': True},
                   },
               ],
           },
       })) +
       api.override_step_data(
           'test r1.gl_tests (r1) on Mac-10.9',
           simulated_gtest_output(passed_test_names=['Test.One'])
       )
   )
+
+  yield (
+      api.test('findit_culprit_in_last_sub_range') +
+      props(
+          {'gl_tests': ['Test.One']}, 'mac', 'Mac10.9 Tests', use_analyze=False,
+           good_revision='r0', bad_revision='r6', suspected_revisions=['r3']) +
+      api.override_step_data('test r2.read test spec', api.json.output({
+          'Mac10.9 Tests': {
+              'gtest_tests': [
+                  {
+                      'test': 'gl_tests',
+                      'swarming': {'can_use_on_swarming_builders': True},
+                  },
+              ],
+          },
+      })) +
+      api.override_step_data('test r3.read test spec', api.json.output({
+          'Mac10.9 Tests': {
+              'gtest_tests': [
+                  {
+                      'test': 'gl_tests',
+                      'swarming': {'can_use_on_swarming_builders': True},
+                  },
+              ],
+          },
+      })) +
+      api.override_step_data(
+          'git commits in range',
+          api.raw_io.stream_output(
+              '\n'.join('r%d' % i for i in reversed(range(1, 7))))) +
+      api.override_step_data(
+          'test r2.gl_tests (r2) on Mac-10.9',
+          simulated_gtest_output(passed_test_names=['Test.One'])) +
+      api.override_step_data(
+          'test r3.gl_tests (r3) on Mac-10.9',
+          simulated_gtest_output(failed_test_names=['Test.One']))
+  )
+
+  yield (
+      api.test('findit_culprit_in_middle_sub_range') +
+      props(
+          {'gl_tests': ['Test.One']}, 'mac', 'Mac10.9 Tests', use_analyze=False,
+           good_revision='r0', bad_revision='r6',
+           suspected_revisions=['r3', 'r6']) +
+      api.override_step_data('test r2.read test spec', api.json.output({
+          'Mac10.9 Tests': {
+              'gtest_tests': [
+                  {
+                      'test': 'gl_tests',
+                      'swarming': {'can_use_on_swarming_builders': True},
+                  },
+              ],
+          },
+      })) +
+      api.override_step_data('test r3.read test spec', api.json.output({
+          'Mac10.9 Tests': {
+              'gtest_tests': [
+                  {
+                      'test': 'gl_tests',
+                      'swarming': {'can_use_on_swarming_builders': True},
+                  },
+              ],
+          },
+      })) +
+      api.override_step_data('test r5.read test spec', api.json.output({
+          'Mac10.9 Tests': {
+              'gtest_tests': [
+                  {
+                      'test': 'gl_tests',
+                      'swarming': {'can_use_on_swarming_builders': True},
+                  },
+              ],
+          },
+      })) +
+      api.override_step_data('test r6.read test spec', api.json.output({
+          'Mac10.9 Tests': {
+              'gtest_tests': [
+                  {
+                      'test': 'gl_tests',
+                      'swarming': {'can_use_on_swarming_builders': True},
+                  },
+              ],
+          },
+      })) +
+      api.override_step_data(
+          'git commits in range',
+          api.raw_io.stream_output(
+              '\n'.join('r%d' % i for i in reversed(range(1, 7))))) +
+      api.override_step_data(
+          'test r2.gl_tests (r2) on Mac-10.9',
+          simulated_gtest_output(passed_test_names=['Test.One'])) +
+      api.override_step_data(
+          'test r3.gl_tests (r3) on Mac-10.9',
+          simulated_gtest_output(failed_test_names=['Test.One'])) +
+      api.override_step_data(
+          'test r5.gl_tests (r5) on Mac-10.9',
+          simulated_gtest_output(passed_test_names=['Test.One'])) +
+      api.override_step_data(
+          'test r6.gl_tests (r6) on Mac-10.9',
+          simulated_gtest_output(passed_test_names=['Test.One']))
+  )
+
+  yield (
+      api.test('findit_culprit_in_first_sub_range') +
+      props(
+          {'gl_tests': ['Test.One']}, 'mac', 'Mac10.9 Tests', use_analyze=False,
+           good_revision='r0', bad_revision='r6',
+           suspected_revisions=['r6']) +
+      api.override_step_data('test r1.read test spec', api.json.output({
+          'Mac10.9 Tests': {
+              'gtest_tests': [
+                  {
+                      'test': 'gl_tests',
+                      'swarming': {'can_use_on_swarming_builders': True},
+                  },
+              ],
+          },
+      })) +
+      api.override_step_data('test r5.read test spec', api.json.output({
+          'Mac10.9 Tests': {
+              'gtest_tests': [
+                  {
+                      'test': 'gl_tests',
+                      'swarming': {'can_use_on_swarming_builders': True},
+                  },
+              ],
+          },
+      })) +
+      api.override_step_data('test r6.read test spec', api.json.output({
+          'Mac10.9 Tests': {
+              'gtest_tests': [
+                  {
+                      'test': 'gl_tests',
+                      'swarming': {'can_use_on_swarming_builders': True},
+                  },
+              ],
+          },
+      })) +
+      api.override_step_data(
+          'git commits in range',
+          api.raw_io.stream_output(
+              '\n'.join('r%d' % i for i in reversed(range(1, 7))))) +
+      api.override_step_data(
+          'test r1.gl_tests (r1) on Mac-10.9',
+          simulated_gtest_output(failed_test_names=['Test.One'])) +
+      api.override_step_data(
+          'test r5.gl_tests (r5) on Mac-10.9',
+          simulated_gtest_output(passed_test_names=['Test.One'])) +
+      api.override_step_data(
+          'test r6.gl_tests (r6) on Mac-10.9',
+          simulated_gtest_output(passed_test_names=['Test.One']))
+  )
+
+  yield (
+      api.test('findit_steps_multiple_culprits') +
+      props(
+          {'gl_tests': ['Test.gl_One'], 'browser_tests': ['Test.browser_One']},
+          'mac', 'Mac10.9 Tests', use_analyze=False,
+           good_revision='r0', bad_revision='r6',
+           suspected_revisions=['r3','r6']) +
+      api.override_step_data('test r2.read test spec', api.json.output({
+          'Mac10.9 Tests': {
+              'gtest_tests': [
+                  {
+                      'test': 'gl_tests',
+                      'swarming': {'can_use_on_swarming_builders': True},
+                  },
+                  {
+                      'test': 'browser_tests',
+                      'swarming': {'can_use_on_swarming_builders': True},
+                  },
+              ],
+          },
+      })) +
+      api.override_step_data('test r3.read test spec', api.json.output({
+          'Mac10.9 Tests': {
+              'gtest_tests': [
+                  {
+                      'test': 'gl_tests',
+                      'swarming': {'can_use_on_swarming_builders': True},
+                  },
+                  {
+                      'test': 'browser_tests',
+                      'swarming': {'can_use_on_swarming_builders': True},
+                  },
+              ],
+          },
+      })) +
+      api.override_step_data('test r5.read test spec', api.json.output({
+          'Mac10.9 Tests': {
+              'gtest_tests': [
+                  {
+                      'test': 'gl_tests',
+                      'swarming': {'can_use_on_swarming_builders': True},
+                  },
+                  {
+                      'test': 'browser_tests',
+                      'swarming': {'can_use_on_swarming_builders': True},
+                  },
+              ],
+          },
+      })) +
+      api.override_step_data('test r6.read test spec', api.json.output({
+          'Mac10.9 Tests': {
+              'gtest_tests': [
+                  {
+                      'test': 'gl_tests',
+                      'swarming': {'can_use_on_swarming_builders': True},
+                  },
+                  {
+                      'test': 'browser_tests',
+                      'swarming': {'can_use_on_swarming_builders': True},
+                  },
+              ],
+          },
+      })) +
+      api.override_step_data(
+          'git commits in range',
+          api.raw_io.stream_output(
+              '\n'.join('r%d' % i for i in reversed(range(1, 7))))) +
+      api.override_step_data(
+          'test r5.gl_tests (r5) on Mac-10.9',
+          simulated_gtest_output(failed_test_names=['Test.gl_One'])) +
+      api.override_step_data(
+          'test r5.browser_tests (r5) on Mac-10.9',
+          simulated_gtest_output(passed_test_names=['Test.browser_One'])) +
+      api.override_step_data(
+          'test r6.browser_tests (r6) on Mac-10.9',
+          simulated_gtest_output(failed_test_names=['Test.browser_One']))+
+      api.override_step_data(
+          'test r2.gl_tests (r2) on Mac-10.9',
+          simulated_gtest_output(passed_test_names=['Test.gl_One'])) +
+      api.override_step_data(
+          'test r3.gl_tests (r3) on Mac-10.9',
+          simulated_gtest_output(failed_test_names=['Test.gl_One']))
+  )
+
+  yield (
+      api.test('findit_tests_multiple_culprits') +
+      props(
+          {'gl_tests': ['Test.One', 'Test.Two', 'Test.Three']},
+          'mac', 'Mac10.9 Tests', use_analyze=False,
+           good_revision='r0', bad_revision='r6',
+           suspected_revisions=['r3', 'r5']) +
+      api.override_step_data('test r2.read test spec', api.json.output({
+          'Mac10.9 Tests': {
+              'gtest_tests': [
+                  {
+                      'test': 'gl_tests',
+                      'swarming': {'can_use_on_swarming_builders': True},
+                  },
+              ],
+          },
+      })) +
+      api.override_step_data('test r3.read test spec', api.json.output({
+          'Mac10.9 Tests': {
+              'gtest_tests': [
+                  {
+                      'test': 'gl_tests',
+                      'swarming': {'can_use_on_swarming_builders': True},
+                  },
+              ],
+          },
+      })) +
+      api.override_step_data('test r4.read test spec', api.json.output({
+          'Mac10.9 Tests': {
+              'gtest_tests': [
+                  {
+                      'test': 'gl_tests',
+                      'swarming': {'can_use_on_swarming_builders': True},
+                  },
+              ],
+          },
+      })) +
+      api.override_step_data('test r5.read test spec', api.json.output({
+          'Mac10.9 Tests': {
+              'gtest_tests': [
+                  {
+                      'test': 'gl_tests',
+                      'swarming': {'can_use_on_swarming_builders': True},
+                  },
+              ],
+          },
+      })) +
+      api.override_step_data('test r6.read test spec', api.json.output({
+          'Mac10.9 Tests': {
+              'gtest_tests': [
+                  {
+                      'test': 'gl_tests',
+                      'swarming': {'can_use_on_swarming_builders': True},
+                  },
+              ],
+          },
+      })) +
+      api.override_step_data(
+          'git commits in range',
+          api.raw_io.stream_output(
+              '\n'.join('r%d' % i for i in reversed(range(1, 7))))) +
+      api.override_step_data(
+          'test r4.gl_tests (r4) on Mac-10.9',
+          simulated_gtest_output(passed_test_names=['Test.One', 'Test.Three'],
+                                 failed_test_names=['Test.Two'])) +
+      api.override_step_data(
+          'test r5.gl_tests (r5) on Mac-10.9',
+          simulated_gtest_output(passed_test_names=['Test.One'],
+                                 failed_test_names=['Test.Three'])) +
+      api.override_step_data(
+          'test r6.gl_tests (r6) on Mac-10.9',
+          simulated_gtest_output(failed_test_names=['Test.One']))+
+      api.override_step_data(
+          'test r2.gl_tests (r2) on Mac-10.9',
+          simulated_gtest_output(passed_test_names=['Test.Two'])) +
+      api.override_step_data(
+          'test r3.gl_tests (r3) on Mac-10.9',
+          simulated_gtest_output(failed_test_names=['Test.Two']))
+  )