Checking for failure in requested builds.

scripts/slave/recipe_modules/auto_bisect/revision_state.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.
 
 """An interface for holding state and result of revisions in a bisect job.
 
 When implementing support for tests other than perf, one should extend this
 class so that the bisect module and recipe can use it.
 
 See perf_revision_state for an example.
@@ skipping 73 matching lines @@
       self.deps = dict(base_revision.deps)
       self.deps[self.depot_name] = self.commit_hash
     else:
       self.needs_patch = False
     self.build_url = self.bisector.get_platform_gs_prefix() + self._gs_suffix()
     self.values = []
     self.mean_value = None
     self.std_dev = None
     self.repeat_count = MINIMUM_SAMPLE_SIZE
     self._test_config = None
+    self.build_number = None
 
   @property
   def tested(self):
     return self.status in (RevisionState.TESTED,)
 
   @property
   def in_progress(self):
     return self.status in (RevisionState.BUILDING, RevisionState.TESTING,
                            RevisionState.NEED_MORE_DATA)
 
@@ skipping 108 matching lines @@
 
   def update_status(self):
     """Checks on the pending jobs and updates status accordingly.
 
     This method will check for the build to complete and then trigger the test,
     or will wait for the test as appropriate.
 
     To wait for the test we try to get the buildbot job url from GS, and if
     available, we query the status of such job.
     """
-    if self.status == RevisionState.BUILDING and self._is_build_archived():
-      self.start_job()
+    if self.status == RevisionState.BUILDING:
+      if self._is_build_archived():
+        self.start_job()
+      elif self._is_build_failed():
+        self.status = RevisionState.FAILED
     elif (self.status in (RevisionState.TESTING, RevisionState.NEED_MORE_DATA)
           and self._results_available()):
       # If we have already decided whether the revision is good or bad we
       # shouldn't check again
       check_revision_goodness = not(self.good or self.bad)
       self._read_test_results(
           check_revision_goodness=check_revision_goodness)
       # We assume _read_test_results may have changed the status to a broken
       # state such as FAILED or ABORTED.
       if self.status in (RevisionState.TESTING, RevisionState.NEED_MORE_DATA):
         self.status = RevisionState.TESTED
 
   def _is_build_archived(self):
     """Checks if the revision is already built and archived."""
     if not self.build_archived:
       api = self.bisector.api
       self.build_archived = api.gsutil_file_exists(self.build_url)
 
     if self.bisector.dummy_builds:
       self.build_archived = self.in_progress
 
     return self.build_archived
 
+  def _is_build_failed(self):
+    api = self.bisector.api
+    current_build = None
+    base_url = '%sjson/builders/%s'% (
+        os.environ.get('BUILDBOT_URL', 'http://localhost:8041/'),

[qyearsley, 2016/03/04 22:06:28]

Why http://localhost:8041/?

[RobertoCN, 2016/03/14 01:00:25]

The reasonable assumption if this env variable is not set, is that this is a
workstation running a local master. I added a comment to that effect.

+        self.bisector.get_builder_bot_for_this_platform())
+    if self.build_number is None:

[qyearsley, 2016/03/04 22:06:28]

Does this mean no build started yet, or we're not sure what the build number is
yet?

[RobertoCN, 2016/03/14 01:00:25]

It means we don't know the build number yet. The build number is not assigned
until the build is started however.

+      try:
+        builder_state_url = base_url + '?as_text=1'
+        fetch_step = api.m.url.fetch_to_file(
+            builder_state_url, None,
+            step_name='fetch builder state',
+            stdout=api.m.raw_io.output())
+        builder_state = fetch_step.stdout
+        builder_state = json.loads(builder_state or '{}')
+        for build_number in builder_state.get('cachedBuilds', []):
+          build_url = '%s/builds/%s?as_text=1' % (base_url, build_number)
+          fetch_result = api.m.url.fetch_to_file(
+              build_url, None,
+              step_name='fetch build details',
+              stdout=api.m.raw_io.output())
+          build = json.loads(fetch_result.stdout or '{}')
+          for p in build.get('properties', []):
+            if p[0] == 'build_archive_url' and p[1] == self.build_url:
+              current_build = build
+              self.build_number = build_number
+              break

[qyearsley, 2016/03/04 22:06:28]

Is build.get('properties') expected to be a list of 2-element lists? e.g.
[
 ['k', 'v'],
 ['build_archive_url', 'http://example.com/'],
]

If so, you can convert it to a dict, e.g.
{'k', 'v2'}

So, this could be changed to something like:

build_properties = dict(build.get('properties', []))
if build_properties.get('build_archive_url') == self.build_url:
  current_build = build
  self.build_number = build_number

[RobertoCN, 2016/03/14 01:00:25]

Thanks! I don't know how I didn't see it.
Done.

+          if self.build_number:
+            break
+      except api.m.step.StepFailure:  # pragma: no cover
+        # If we cannot get json from buildbot, we cannot determine if a build is
+        # failed, hence we consider it in progress until it times out.
+        return False
+    if self.build_number is None:
+      # The build hasn't started yet, therefore it's not failed.
+      return False
+    if not current_build:
+      build_url = '%s/builds/%s?as_text=1' % (base_url, self.build_number)
+      fetch_result = api.m.url.fetch_to_file(
+          build_url, None,
+          step_name='fetch build details',
+          stdout=api.m.raw_io.output())
+      current_build = json.loads(fetch_result.stdout or '{}')

[qyearsley, 2016/03/04 22:06:28]

This section of code (where the URL is put together, and a request for build
details JSON is made and then that JSON is parsed) is repeated -- it should be
extracted to a separate method (which takes a base URL and build number, and
returns the parsed data).

[RobertoCN, 2016/03/14 01:00:25]

Done.

+    return current_build.get('results') in [2, 3, 4]

[qyearsley, 2016/03/04 22:06:28]

These could be set as constants, ideally with some reference as to where they
come from. e.g.

# Buildbot job result codes.
# See http://docs.buildbot.net/current/developer/results.html
SUCCESS, WARNINGS, FAILURE, SKIPPED, EXCEPTION = range(5)

I'm not sure if that's correct though.

[RobertoCN, 2016/03/14 01:00:25]

Works for me.

+
   def _results_available(self):
     """Checks if the results for the test job have been uploaded."""
     api = self.bisector.api
     result = api.gsutil_file_exists(self.test_results_url)
     if self.bisector.dummy_builds:
       return self.in_progress
     return result  # pragma: no cover
 
   def _gs_suffix(self):
     """Provides the expected right half of the build filename.
@@ skipping 53 matching lines @@
     file_name = str(api.m.path['tmp_base'].join(build_name + '.diff'))
     api.m.file.write('Saving diff patch for ' + self.commit_hash,
                      file_name, self.deps_patch + self.deps_sha_patch)
     return file_name
 
   def _request_build(self):
     """Posts a request to buildbot to build this revision and archive it."""
     # TODO: Rewrite using the trigger module.
     api = self.bisector.api
     bot_name = self.bisector.get_builder_bot_for_this_platform()
-    if self.bisector.dummy_builds:
+    if self.bisector.bisect_config.get('dummy_job_names'):
       self.job_name = self.commit_hash + '-build'
     else:  # pragma: no cover
       self.job_name = uuid.uuid4().hex
     if self.needs_patch:
       self.patch_file = self._write_deps_patch_file(
           self.job_name)
     else:
       self.patch_file = '/dev/null'
 
     # To allow multiple nested levels, we go to the topmost revision.
@@ skipping 45 matching lines @@
     self._test_config = result
     return result
 
   def _do_test(self):
     """Triggers tests for a revision, either locally or via try job.
 
     If local testing is enabled (i.e. director/tester merged) then
     the test will be run on the same machine. Otherwise, this posts
     a request to buildbot to download and perf-test this build.
     """
-    if self.bisector.dummy_builds:
+    if self.bisector.bisect_config.get('dummy_job_names'):
       self.job_name = self.commit_hash + '-test'
     else:  # pragma: no cover
       self.job_name = uuid.uuid4().hex
     api = self.bisector.api
     top_revision = self
     while top_revision.base_revision:  # pragma: no cover
       top_revision = top_revision.base_revision
     perf_test_properties = {
         'builder_name': self.bisector.get_perf_tester_name(),
         'properties': {
@@ skipping 116 matching lines @@
         key=lambda x: len(x.values))
     if (len(self.bisector.last_tested_revision.values) ==
         next_revision_to_test.values):
       self.bisector.last_tested_revision.retest()
     else:
       next_revision_to_test.retest()
 
   def __repr__(self):
     return ('RevisionState(rev=%s, values=%r, mean_value=%r, std_dev=%r)' % (
         self.revision_string(), self.values, self.mean_value, self.std_dev))