[Findit] Refactoring compile recipe return results into report dict instead of list

scripts/slave/recipes/findit/chromium/compile.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.
 
 import json
 
 from recipe_engine.config import List
 from recipe_engine.config import Single
 from recipe_engine.recipe_api import Property
 
 
 DEPS = [
-  'chromium',
-  'chromium_tests',
-  'findit',
-  'gclient',
-  'recipe_engine/json',
-  'recipe_engine/path',
-  'recipe_engine/properties',
-  'recipe_engine/python',
-  'recipe_engine/step',
+    'chromium',
+    'chromium_tests',
+    'findit',
+    'gclient',
+    'recipe_engine/json',
+    'recipe_engine/path',
+    'recipe_engine/properties',
+    'recipe_engine/python',
+    'recipe_engine/step',
 ]
 
 
 PROPERTIES = {
     'target_mastername': Property(
         kind=str, help='The target master to match compile config to.'),
     'target_buildername': Property(
         kind=str, help='The target builder to match compile config to.'),
     'good_revision': Property(
         kind=str, help='The last known good chromium revision.'),
@@ skipping 13 matching lines @@
   PASSED = 'passed'  # Compile passed.
   FAILED = 'failed'  # Compile failed.
 
 
 def _run_compile_at_revision(api, target_mastername, target_buildername,
                              revision, compile_targets, use_analyze):
   with api.step.nest('test %s' % str(revision)):
     # Checkout code at the given revision to recompile.
     bot_config = api.chromium_tests.create_bot_config_object(
         target_mastername, target_buildername)
-    bot_update_step, bot_db = \
-        api.chromium_tests.prepare_checkout(
-            bot_config,
-            root_solution_revision=revision)
+    bot_update_step, bot_db = api.chromium_tests.prepare_checkout(
+        bot_config, root_solution_revision=revision)
 
     # TODO(http://crbug.com/560991): if compile targets are provided, check
     # whether they exist and then use analyze to compile the impacted ones by
     # the given revision.
     compile_targets = sorted(set(compile_targets or []))
     if not compile_targets:
       _, tests_including_triggered = api.chromium_tests.get_tests(
           bot_config, bot_db)
       compile_targets = api.chromium_tests.get_compile_targets(
           bot_config, bot_db, tests_including_triggered)
@@ skipping 38 matching lines @@
       target_mastername, target_buildername)
   api.chromium_tests.configure_build(
       bot_config, override_bot_type='builder_tester')
 
   # 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)
 
-  results = []
+  compile_results = {}
+  try_job_metadata = {
+      'number_of_commits_in_range': len(revisions_to_check)

[stgao, 2016/01/29 02:15:52]

'regression_range_size' instead?

[lijeffrey, 2016/01/29 09:53:42]

Done.

+  }
+  report = {
+      'result': compile_results,
+      'metadata': try_job_metadata,
+  }
+
+  last_result = None
   try:
     for current_revision in revisions_to_check:
       compile_result = _run_compile_at_revision(
           api, target_mastername, target_buildername,
           current_revision, requested_compile_targets, use_analyze)
 
-      results.append([current_revision, compile_result])
+      compile_results[current_revision] = compile_result
+      last_revision = current_revision
+      last_result = compile_result

[stgao, 2016/01/29 02:15:52]

There seems a bug (not an obvious one): when testing at the current revision N,
there is an infra failure. When the finally block is run, the ``last_result`` is
actually what it means to be and it is for N-1 instead.

[lijeffrey, 2016/01/29 09:53:42]

Done.

       if compile_result == CompileResult.FAILED:
         # TODO(http://crbug.com/560991): if compile targets are specified,
         # compile may fail because those targets are added in a later revision.
-        break # Found the culprit, no need to check later revisions.
+        break  # Found the culprit, no need to check later revisions.
   finally:
     # Report the result.
     # TODO(http://crbug.com/563807): use api.python.succeeding_step instead.
     step_result = api.python.inline(
         'report', 'import sys; sys.exit(0)', add_python_log=False)
     if (not requested_compile_targets and
-        results and results[-1][1] == CompileResult.FAILED):
-      step_result.presentation.step_text = '<br/>Culprit: %s' % results[-1][0]
+        compile_results and last_result == CompileResult.FAILED):
+      step_result.presentation.step_text = (
+          '<br/>Culprit: %s' % compile_results[last_revision])

[stgao, 2016/01/29 02:15:52]

Shouldn't it be last_revision instead? And it could be undefined if line #132 is
not executed when testing the first revision results in an infra error.

[lijeffrey, 2016/01/29 09:53:42]

Done.

     step_result.presentation.logs.setdefault('result', []).append(
-        json.dumps(results, indent=2))
+        json.dumps(report, indent=2))
 
     # Set the result 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['result'] = results
+    step_result.presentation.properties['result'] = report
 
-  return results
+  return compile_results

[stgao, 2016/01/29 02:15:52]

Let's return the ``report`` instead. To be consistent with the build property
value and the log.

[lijeffrey, 2016/01/29 09:53:42]

Done.

 
 
 def GenTests(api):
   def props(compile_targets=None, use_analyze=False):
     properties = {
         'mastername': 'tryserver.chromium.linux',
         'buildername': 'linux_variable',
         'slavename': 'build1-a1',
         'buildnumber': '1',
         'target_mastername': 'chromium.linux',
         'target_buildername': 'Linux Builder',
         'good_revision': 'r0',
         'bad_revision': 'r1',
         'use_analyze': use_analyze,
     }
     if compile_targets:
       properties['compile_targets'] = compile_targets
     return api.properties(**properties)
 
   yield (
       api.test('compile_specified_targets') +
       props(compile_targets=['target_name'])
   )
 
   yield (
       api.test('compile_default_targets') +
       props() +
       api.override_step_data('test r1.read test spec',
                              api.json.output({
-                                'Linux Builder': {
-                                   'additional_compile_targets': [
-                                       'base_unittests',
-                                   ],
-                                }
+                                 'Linux Builder': {
+                                     'additional_compile_targets': [
+                                         'base_unittests',
+                                     ],
+                                 }
                              }))
   )
 
   yield (
       api.test('compile_succeeded') +
       props() +
       api.override_step_data('test r1.compile', retcode=1)
   )
 
   yield (
@@ skipping 30 matching lines @@
               'test_targets': [],
           })
       )
   )
 
   yield (
       api.test('compile_affected_targets_only') +
       props(use_analyze=True) +
       api.override_step_data('test r1.read test spec',
                              api.json.output({
-                                'Linux Builder': {
-                                   'additional_compile_targets': [
-                                       'a', 'a_run',
-                                       'b', 'b_run',
-                                   ],
-                                }
+                                 'Linux Builder': {
+                                     'additional_compile_targets': [
+                                         'a', 'a_run',
+                                         'b', 'b_run',
+                                     ],
+                                 }
                              })) +
       api.override_step_data(
           'test r1.analyze',
           api.json.output({
               'status': 'Found dependency',
               'compile_targets': ['a', 'a_run'],
               'test_targets': ['a', 'a_run'],
           })
       )
   )