[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 49 matching lines @@
   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])
       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]
     step_result.presentation.logs.setdefault('result', []).append(
         json.dumps(results, 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['number_of_commits_in_range'] = len(

[stgao, 2016/01/25 22:21:35]

As discussed offline, we'd better not add a new build properties.
And instead, we could change the ``results`` into a dict and add more
information into it.

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

Done.

+        revisions_to_check)

[chanli, 2016/01/22 18:05:27]

I think this number is actually
len(failure_info['builds'][current_failure]['blame_list']), right?

[lijeffrey, 2016/01/22 22:26:43]

I think you're right, it should be the same. In the case we use failure_info,
then this CL is not needed. If we use that approach then we can re factor
ScheduleTryJobPipeline and MonitorTryJobPipeline to consume failure_info instead
of master_name, builder_name, etc which it can get from failure_info (I plan to
use MonitorTryJobPipeline to handle updating WfTryJobData). The downside is if
for whatever reason if there's a discrepancy between what the try job derived as
the list of revisions to check and what's in failure_info's blame list then the
data stored may not be representative of what the try job would have tried in a
worst-case scenario. What do you guys think?

 
   return results
 
 
 def GenTests(api):
   def props(compile_targets=None, use_analyze=False):
     properties = {
         'mastername': 'tryserver.chromium.linux',
         'buildername': 'linux_variable',
         'slavename': 'build1-a1',
@@ skipping 11 matching lines @@
   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'],
           })
       )
   )