Add bisect config validation.

scripts/slave/recipe_modules/auto_bisect/bisector.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
 import re
 import time
 import urllib
 
+from . import config_validation
 from . import depot_config
 from . import revision_state
 
 _DEPS_SHA_PATCH = """
 diff --git DEPS.sha DEPS.sha
 new file mode 100644
 --- /dev/null
 +++ DEPS.sha
 @@ -0,0 +1 @@
 +%(deps_sha)s
@@ skipping 48 matching lines @@
   """This class abstracts an ongoing bisect (or n-sect) job."""
 
   def __init__(self, api, bisect_config, revision_class, init_revisions=True):
     """Initializes the state of a new bisect job from a dictionary.
 
     Note that the initial good_rev and bad_rev MUST resolve to a commit position
     in the chromium repo.
     """
     super(Bisector, self).__init__()
     self._api = api
+    self.result_codes = set()
     self.ensure_sync_master_branch()
     self.bisect_config = bisect_config
     self.config_step()
     self.revision_class = revision_class
-    self.result_codes = set()
     self.last_tested_revision = None
 
     # Test-only properties.
     # TODO: Replace these with proper mod_test_data.
     self.dummy_builds = bisect_config.get('dummy_builds', False)
     self.bypass_stats_check = bool(bisect_config.get('bypass_stats_check'))
 
     # Load configuration items.
     self.test_type = bisect_config.get('test_type', 'perf')
     self.improvement_direction = int(bisect_config.get(
@@ skipping 89 matching lines @@
     results = step_result.stdout
     if results is None:
       assert self.dummy_builds
       return True
     significantly_different = results['significantly_different']
     step_result.presentation.logs[str(significantly_different)] = [
         'See json.output for details']
     return significantly_different
 
   def config_step(self):
-    """Yields a simple echo step that outputs the bisect config."""
+    """Validates the config and yields a step that prints the bisect config."""
     api = self.api
+
     # bisect_config may come as a FrozenDict (which is not serializable).
     bisect_config = dict(self.bisect_config)
 
     def fix_windows_backslashes(s):
       backslash_regex = re.compile(r'(?<!\\)\\(?!\\)')
       return backslash_regex.sub(r'\\', s)
 
     for k, v in bisect_config.iteritems():
       if isinstance(v, basestring):
         bisect_config[k] = fix_windows_backslashes(v)
+
+    step = api.m.step('config', [])
+    try:
+      config_validation.validate_config(bisect_config)
+    except config_validation.ValidationFail as error:
+      self.surface_result('BAD_CONFIG')
+      raise self.api.m.step.StepFailure(error.message)

[qyearsley, 2016/03/11 01:41:34]

Would this provide everything in the buildbot job UI that we want? Or should I
print the error message in some other way?

[RobertoCN, 2016/03/14 01:26:23]

Just set the step to failed as well before raising the StepFailure:
self.api.m.step.active_result.presentation.status = self.api.m.step.FAILURE

If you wanted the failure to be its own step you could use the halt recipe
module.

[qyearsley, 2016/03/18 22:50:19]

Done; making it it's own step may make it be clearer when it fails.

+
     # We sort the keys to prevent problems with orders changing when
     # recipe_simulation_test compares against expectation files.
     config_string = json.dumps(bisect_config, indent=2, sort_keys=True)
-    result = api.m.step('config', [])
     config_lines = config_string.splitlines()
-    result.presentation.logs['Bisect job configuration'] = config_lines
+    step.presentation.logs['Bisect job configuration'] = config_lines
 
   @property
   def api(self):
     return self._api
 
   def compute_relative_change(self):
     old_value = float(self.good_rev.mean_value)
     new_value = float(self.bad_rev.mean_value)
 
     if new_value and not old_value:  # pragma: no cover
@@ skipping 623 matching lines @@
         })
     return revision_rows
 
   def _get_build_url(self):
     properties = self.api.m.properties
     bot_url = properties.get('buildbotURL',
                              'http://build.chromium.org/p/chromium/')
     builder_name = urllib.quote(properties.get('buildername', ''))
     builder_number = str(properties.get('buildnumber', ''))
     return '%sbuilders/%s/builds/%s' % (bot_url, builder_name, builder_number)