[Findit] Modify tryjob pipelines to trigger try jobs for test failure.

appengine/findit/waterfall/identify_try_job_culprit_pipeline.py

Index: appengine/findit/waterfall/identify_try_job_culprit_pipeline.py
diff --git a/appengine/findit/waterfall/identify_try_job_culprit_pipeline.py b/appengine/findit/waterfall/identify_try_job_culprit_pipeline.py
index 873db7b33193b2f654898b808481ec07e649e6ce..5d25798340654158987f7348dcf13b126c898c45 100644
--- a/appengine/findit/waterfall/identify_try_job_culprit_pipeline.py
+++ b/appengine/findit/waterfall/identify_try_job_culprit_pipeline.py
@@ -9,45 +9,173 @@ from model.wf_try_job import WfTryJob
 from pipeline_wrapper import BasePipeline
 
 
+GIT_REPO = GitRepository(
+    'https://chromium.googlesource.com/chromium/src.git', HttpClient())
+
+
 class IdentifyTryJobCulpritPipeline(BasePipeline):
   """A pipeline to identify culprit CL info based on try job compile results."""
 
+  def _GetCulpritInfo(self, failed_revisions):
+    """Gets commit_positions and review_urls for revisions."""
+    culprits = {}
+    for failed_revision in failed_revisions:
+      change_log = GIT_REPO.GetChangeLog(failed_revision)
+      if change_log:
+        culprits[failed_revision] = {
+            'revision': failed_revision,
+            'commit_position': change_log.commit_position,
+            'review_url': change_log.code_review_url
+        }
+    return culprits
+
   # Arguments number differs from overridden method - pylint: disable=W0221
   def run(
-      self, master_name, builder_name, build_number, try_job_id,
-      compile_result):
-    culprit = None
-
-    if compile_result and len(compile_result.get('result', [])) > 0:
-      # For compile failures, the try job will stop if one revision fails, so
-      # the culprit will be the last revision in the result.
-      result_for_last_checked_revision = compile_result['result'][-1]
-      failed_revision = (
-          result_for_last_checked_revision[0] if
-          result_for_last_checked_revision[1].lower() == 'failed' else None)
-
-      if failed_revision:
-        git_repo = GitRepository(
-            'https://chromium.googlesource.com/chromium/src.git', HttpClient())
-        change_log = git_repo.GetChangeLog(failed_revision)
-        if change_log:
-          culprit = {
-              'revision': failed_revision,
-              'commit_position': change_log.commit_position,
-              'review_url': change_log.code_review_url
-          }
-          compile_result['culprit'] = culprit
+      self, master_name, builder_name, build_number, blame_list, try_job_type,

[qyearsley, 2016/01/17 03:43:12]

Besides "compile", what other valid values are there for try_job_type?

[chanli, 2016/01/20 18:07:09]

'test'.

+      try_job_id, result):
+    """Identify the information for failed revisions.

[qyearsley, 2016/01/17 03:43:12]

Identify -> Identifies
This docstring means that this pipeline is used to fetch/collection information
about failed revisions, right?

From below, it looks like the main purpose is to identify culprits of failures
based on try job results and provide information about those culprits right?

[chanli, 2016/01/20 18:07:08]

Yes. From try job result we can get the failed revision. And in this pipeline we
will get commit position and review url based on the revision.

+
+    The format for final try-job result for compile failures is:
+    {
+        'result': [
+            ['rev1', 'passed'],
+            ['rev2', 'failed']
+        ],
+        'url': 'url',
+        'try_job_id': '1',
+        'culprit': {
+            'revision': 'rev2',
+            'commit_position': '2',
+            'review_url': 'url_2'
+        }
+    }
+
+    The format for final try-job result for test failures is:
+    {
+        'result': {
+            'rev1': {
+                'a_test': {
+                    'status': 'failed',
+                    'valid': True,
+                    'failures': ['a_test1']
+                },
+                'b_test': {
+                    'status': 'failed',
+                    'valid': True,
+                    'failures': ['b_test1']
+                }
+            },
+            'rev2': {
+                'a_test': {
+                    'status': 'failed',
+                    'valid': True,
+                    'failures': ['a_test2']
+                },
+                'b_test': {
+                    'status': 'passed',
+                    'valid': True
+                }
+            }
+        },
+        'url': 'url',
+        'try_job_id': '1',
+        'culprit': {
+            'a_test': {
+                'revision': 'rev1',
+                'commit_position': '1',
+                'review_url': 'url_1',
+                'tests': {
+                    'a_test1': {
+                      'revision': 'rev1',
+                      'commit_position': '1',
+                      'review_url': 'url_1'
+                    },
+                    'a_test2': {
+                      'revision': 'rev2',
+                      'commit_position': '2',
+                      'review_url': 'url_2'
+                    }
+                }
+            },
+            'b_test': {
+                'revision': 'rev1',
+                'commit_position': '1',
+                'review_url': 'url_1',
+                'tests': {
+                    'b_test1': {
+                      'revision': 'rev1',
+                      'commit_position': '1',
+                      'review_url': 'url_1'
+                    }
+                }
+            }
+        }
+    }
+    """
+    culprits = None
+
+    if try_job_type == 'compile':
+      if result and len(result.get('result', [])) > 0:
+        # For compile failures, the try job will stop if one revision fails, so
+        # the culprit will be the last revision in the result.
+        result_for_last_checked_revision = result['result'][-1]
+        failed_revision = (
+            result_for_last_checked_revision[0] if
+            result_for_last_checked_revision[1].lower() == 'failed' else None)
+
+        if failed_revision:
+          culprits = self._GetCulpritInfo([failed_revision])
+          if culprits:
+            result['culprit'] = culprits[failed_revision]
+    else:
+      if result and result.get('result'):
+        # For test failures, the try job will run against every revision,
+        # so we need to traverse the result dict in order to identify the
+        # culprits to each failed step or test.
+        culprit_map = {}
+        failed_revisions = []
+        for revision in blame_list:
+          for step, step_result in result['result'][revision].iteritems():
+            if step_result['valid'] and step_result['status'] == 'failed':
+              if revision not in failed_revisions:
+                failed_revisions.append(revision)
+
+              if step not in culprit_map:
+                culprit_map[step] = {}
+                culprit_map[step]['revision'] = revision
+                culprit_map[step]['tests'] = {}
+              for failed_test in step_result['failures']:
+                if failed_test not in culprit_map[step]['tests']:
+                  culprit_map[step]['tests'][failed_test] = {}
+                  culprit_map[step]['tests'][failed_test]['revision'] = (
+                      revision)
+
+        if failed_revisions:
+          culprits = self._GetCulpritInfo(failed_revisions)
+          if culprits:
+            for step_culprit in culprit_map.values():
+              step_revision = step_culprit['revision']
+              step_culprit['commit_position'] = (
+                  culprits[step_revision]['commit_position'])
+              step_culprit['review_url'] = culprits[step_revision]['review_url']
+              for test_culprit in step_culprit.get('tests', {}).values():
+                test_revision = test_culprit['revision']
+                test_culprit.update(culprits[test_revision])
+            result['culprit'] = culprit_map

[qyearsley, 2016/01/17 03:43:12]

This function is relatively long and nested, so it may be hard to understand
later; might it be a good idea to extract some helper functions (e.g. one for
the compile case and one for the non-compile case?)

[chanli, 2016/01/20 18:07:08]

Done.

 
     # Store try job results.
     try_job_result = WfTryJob.Get(master_name, builder_name, build_number)
-    if culprit:
-      if (try_job_result.compile_results and
-          try_job_result.compile_results[-1]['try_job_id'] == try_job_id):
-        try_job_result.compile_results[-1].update(compile_result)
+    if culprits:
+      result_needs_update = (
+          try_job_result.compile_results if
+          try_job_type == 'compile' else try_job_result.test_results)
+      if (result_needs_update and
+          result_needs_update[-1]['try_job_id'] == try_job_id):
+        result_needs_update[-1].update(result)
       else:  # pragma: no cover
-        try_job_result.compile_results.append(compile_result)
+        result_needs_update.append(result)
 
     try_job_result.status = wf_analysis_status.ANALYZED
     try_job_result.put()
 
-    return culprit
+    return result.get('culprit', None) if result else None