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

appengine/findit/waterfall/monitor_try_job_pipeline.py

Index: appengine/findit/waterfall/monitor_try_job_pipeline.py
diff --git a/appengine/findit/waterfall/monitor_try_job_pipeline.py b/appengine/findit/waterfall/monitor_try_job_pipeline.py
index af4e4558df1f8bbe20565cda6f938a62889e5e91..4b6a6977f47006652a6d4defd4b6d03de609d2bd 100644
--- a/appengine/findit/waterfall/monitor_try_job_pipeline.py
+++ b/appengine/findit/waterfall/monitor_try_job_pipeline.py
@@ -12,7 +12,7 @@ from pipeline_wrapper import pipeline
 
 
 class MonitorTryJobPipeline(BasePipeline):
-  """A pipeline for monitoring a tryjob and recording results when it's done.
+  """A pipeline for monitoring a try job and recording results when it's done.
 
   The result will be stored to compile_results or test_results according to
   which type of build failure we are running try job for.
@@ -20,7 +20,8 @@ class MonitorTryJobPipeline(BasePipeline):
 
   # Arguments number differs from overridden method - pylint: disable=W0221
   # TODO(chanli): Handle try job for test failures later.
-  def run(self, master_name, builder_name, build_number, try_job_id):
+  def run(
+      self, master_name, builder_name, build_number, try_job_type, try_job_id):
     assert try_job_id
 
     timeout_hours = 5  # Timeout after 5 hours.
@@ -40,14 +41,18 @@ class MonitorTryJobPipeline(BasePipeline):
         }
 
         try_job_result = WfTryJob.Get(master_name, builder_name, build_number)
-        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(result)
+        if try_job_type == 'compile':

[stgao, 2016/01/26 00:51:40]

use a CONSTANT instead? An enum might be better too.

[chanli, 2016/01/27 18:49:55]

Done.

+          result_needs_update = try_job_result.compile_results

[stgao, 2016/01/26 00:51:40]

the var name seems like a True/False flag, but it's actually not.
How about naming it "result_to_update"?

[chanli, 2016/01/27 18:49:55]

Done.

+        else:
+          result_needs_update = 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(result)
-
+          result_needs_update.append(result)
         try_job_result.put()
-        return try_job_result.compile_results[-1]
+        return result_needs_update[-1]
+
       else:  # pragma: no cover
         if build.status == 'STARTED' and not already_set_started:

[stgao, 2016/01/26 00:51:40]

unrelated to this CL: use enum or constant instead of hardcoded string.

[chanli, 2016/01/27 18:49:55]

Done.

           result = {
@@ -57,13 +62,17 @@ class MonitorTryJobPipeline(BasePipeline):
           }
 
           try_job_result = WfTryJob.Get(master_name, builder_name, build_number)
-          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(result)
+          if try_job_type == 'compile':
+            result_needs_update = try_job_result.compile_results
+          else:
+            result_needs_update = 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
             # Normally result for current try job should've been saved in
             # schedule_try_job_pipeline, so this branch shouldn't be reached.
-            try_job_result.compile_results.append(result)
+            result_needs_update.append(result)

[stgao, 2016/01/26 00:51:40]

Line #58-75 and #37-53 are very similar. How about extracting them into a
separate function ``UpdateTryJobResult`` or the like?

This could simplify unittest too, as we test the new function directly.
To simplify unittest (especially setting up dummy data), we'd better go in this
way later.

[chanli, 2016/01/27 18:49:55]

Done.

 
           try_job_result.status = wf_analysis_status.ANALYZING
           try_job_result.put()