[Findit] Compare lists to lists instead of lists to tuples, and change tests

appengine/findit/waterfall/try_job_util.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.
 
 from datetime import datetime
 import logging
 
 from google.appengine.ext import ndb
 
 from common import appengine_util
@@ skipping 187 matching lines @@
     suspected_tuples, output_nodes=None, failed_steps_and_tests=None):
   new_group = WfFailureGroup.Create(master_name, builder_name, build_number)
   new_group.build_failure_type = build_failure_type
   new_group.blame_list = blame_list
   new_group.suspected_tuples = suspected_tuples
   new_group.output_nodes = output_nodes
   new_group.failed_steps_and_tests = failed_steps_and_tests
   new_group.put()
 
 
 def _GetMatchingGroup(wf_failure_groups, blame_list, suspected_tuples):

[lijeffrey, 2016/07/27 22:49:36]

a change like this is better tested with unit tests that call this function
directly, once with suspected_tuples as an actual list of tuples and once with
it list-ified by json

[josiahk, 2016/07/30 00:21:11]

Thanks! I changed the code to use lists instead of tuples.

+  # Tuples get turned into lists with ndb.JsonProperty. WfFailureGroup uses
+  # ndb.JsonProperty to store suspected_tuples.
+  suspected_tuples = [list(tup) for tup in suspected_tuples]

[chanli, 2016/07/28 17:40:25]

Instead of modifying it here, you can just change GenPotentialCulpritTupleList
to directly produce the expected format.

[josiahk, 2016/07/30 00:21:10]

Thank you. I changed GenPotentialCulpritTupleList (side note: renamed to
GenSuspectedCLsWithFailures) to return list of lists instead of list of tuples.

   for group in wf_failure_groups:
     if _BlameListsIntersection(group.blame_list, blame_list):
       if suspected_tuples == group.suspected_tuples:
         return group
 
   return None
 
+
 def _GetOutputNodes(signals):
   if not signals or 'compile' not in signals:
     return []
 
   # Compile failures with no output nodes will be considered unique.
   return signals['compile'].get('failed_output_nodes', [])
 
 
 def _GetMatchingCompileFailureGroups(output_nodes):
   # Output nodes should already be unique and sorted.
@@ skipping 33 matching lines @@
     return True
 
   suspected_tuples = sorted(GenPotentialCulpritTupleList(heuristic_result))
   existing_group = _GetMatchingGroup(groups, blame_list, suspected_tuples)
 
   # Create a new WfFailureGroup if we've encountered a unique build failure.
   if existing_group:
     logging.info('A group already exists, no need for a new try job.')
     _LinkAnalysisToBuildFailureGroup(
         master_name, builder_name, build_number, [existing_group.master_name,
-        existing_group.builder_name, existing_group.build_number])
+                                                  existing_group.builder_name,
+                                                  existing_group.build_number])

[chanli, 2016/07/28 17:40:26]

Move the list to a separated line.

[josiahk, 2016/07/30 00:21:10]

Thanks!

   else:
     logging.info('A new try job should be run for this unique build failure.')
     _CreateBuildFailureGroup(
         master_name, builder_name, build_number, build_failure_type, blame_list,
         suspected_tuples, output_nodes, failed_steps_and_tests)
     _LinkAnalysisToBuildFailureGroup(master_name, builder_name, build_number,
                                      [master_name, builder_name, build_number])
 
   return not existing_group
 
@@ skipping 150 matching lines @@
               pipeline.pipeline_status_path, try_job_type)
     else:  # pragma: no cover
       logging_str = (
           'Try job was scheduled for build %s, %s, %s: %s because of %s '
           'failure.') % (
               master_name, builder_name, build_number,
               pipeline.pipeline_status_path, try_job_type)
     logging.info(logging_str)
 
   return failure_result_map