[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_subtracted_test_dict(original_test_dict, failed_tests_dict):

[lijeffrey, 2016/05/04 19:15:19]

Naming this one's tricky, how about 'reduced' instead of 'subtracted'?

[chanli, 2016/05/04 22:52:43]

Done.

+  # Remove tests that are in both dicts from the original test dict.
+  if not failed_tests_dict:
+    return original_test_dict
+  subtracted_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]:
+          subtracted_dict[step].append(test)
+    else:
+      subtracted_dict[step].extend(tests)
+  return subtracted_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])

[stgao, 2016/05/04 21:35:28]

Can we handle this separately by operating just on ``suspected_revision_index``?
That will improve the readability.

[chanli, 2016/05/04 22:52:43]

I don't rally understand what you're suggesting here. I changed this logic a
little, is readability better?

+        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

[stgao, 2016/05/04 21:35:28]

naming nit: this name seems misleading -- these tests have no culprit, like
long-standing flaky tests.

[chanli, 2016/05/04 22:52:43]

Done.

+    # Iterates through sub_ranges and find culprits for each failed test.
+    for sub_range in sub_ranges:

[stgao, 2016/05/04 21:35:28]

Can we have a comment on why sub-ranges with newer revisions are tested first?

[chanli, 2016/05/04 22:52:43]

Done.

+      if not no_culprit_tests:  # All tests have found culprits.
+        break
+
+      # The revision right before the suspected revision provided by
+      # the heuristic result.
+      potential_green_rev = sub_range[0]
+      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_subtracted_test_dict(
+          no_culprit_tests, tests_failed_in_potential_green)
+
+      # Culprits for tests that failed in potential green should be earlier, so
+      # takes out passed tests and only runs failed ones in following revisions.

[lijeffrey, 2016/05/04 19:15:19]

nit: 'remove' instead of 'takes out'

[chanli, 2016/05/04 22:52:43]

Done.

+      if tests_passed_in_potential_green:
+        tests_to_run = tests_passed_in_potential_green
+        for revision in following_revisions:

[stgao, 2016/05/04 21:35:28]

Maybe add a comment on the general idea how this works?

[chanli, 2016/05/04 22:52:43]

Done.

+          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_subtracted_test_dict(
+              no_culprit_tests, tests_failed_in_revision)
+
+          # Only runs tests that have not found culprits in later revisions.
+          tests_to_run = _get_subtracted_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']))
+  )