Chop off platform name from step name

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 126 matching lines @@
   if not failed_steps:
     return failed_steps_and_tests
 
   for step_name, step in failed_steps.iteritems():
     for test_name in step.get('tests', [None]):
       failed_steps_and_tests.append([step_name, test_name])
 
   return sorted(failed_steps_and_tests)
 
 
+def _ChopOffPlatformFromStepName(step_name):

[lijeffrey, 2016/08/10 18:48:08]

nit: rename this function '_RemovePlatformFromStepName'

[josiahk, 2016/08/10 21:22:34]

Done.

+  """Returns chopped off version of step name.
+
+  Args:
+    step_name: Raw step name. Example: "net_unittests on Windows-10"

[lijeffrey, 2016/08/10 18:48:08]

single quotes for strings

[josiahk, 2016/08/10 21:22:34]

Done.

+
+  Returns:
+    Step name without platform. Example: "net_unittests on "
+  """
+  if ' on ' in step_name:
+    return step_name[:step_name.find(' on ') + 4]

[lijeffrey, 2016/08/10 18:48:08]

4? what's this magic number and why is it being used here?

The code here is cryptic though all it's doing is blindly returning everything
up to and including ' on '. Better to store ' on ' as a variable and use +
len(variable)

[josiahk, 2016/08/10 21:22:35]

Fixed!

+  return step_name
+
+
 def GetSuspectedCLsWithFailures(heuristic_result):
   """Generates a list of suspected CLs with failures.
 
   Args:
     heuristic_result: the heuristic_result from which to generate the list of
     suspected CLs with failures.
 
   Returns:
     A list of suspected CLs with failures that each could look like:
 
         [step_name, revision, test_name]
 
     or could look like:
 
         [step_name, revision, None]
   """
   suspected_cls_with_failures = []
 
   if not heuristic_result:
     return suspected_cls_with_failures
 
   # Iterates through the failures, tests, and suspected_cls, appending suspected
   # CLs and failures to the list.
   for failure in heuristic_result['failures']:
     if failure.get('tests'):
       for test in failure['tests']:
         for suspected_cl in test.get('suspected_cls', []):
           suspected_cls_with_failures.append([
-              failure['step_name'],
+              _ChopOffPlatformFromStepName(failure['step_name']),
               suspected_cl['revision'],
               test['test_name']])
     else:
       for suspected_cl in failure['suspected_cls']:
         suspected_cls_with_failures.append([
-            failure['step_name'],
+            _ChopOffPlatformFromStepName(failure['step_name']),
             suspected_cl['revision'],
             None])
 
   return suspected_cls_with_failures
 
 
 def _LinkAnalysisToBuildFailureGroup(
     master_name, builder_name, build_number, failure_group_key):
   analysis = WfAnalysis.Get(master_name, builder_name, build_number)
   analysis.failure_group_key = failure_group_key
@@ skipping 119 matching lines @@
     need_new_try_job, last_pass = _CheckFailureForTryJobKey(
         master_name, builder_name, build_number,
         failure_result_map, TryJobType.COMPILE, failed_steps['compile'])
   else:
     try_job_type = TryJobType.TEST
     targeted_tests, need_new_try_job, last_pass = (
         _CheckIfNeedNewTryJobForTestFailure(
             'step', master_name, builder_name, build_number, failure_result_map,
             failed_steps))
 
-
   need_new_try_job = (
       need_new_try_job and ReviveOrCreateTryJobEntity(
           master_name, builder_name, build_number, force_try_job))
 
   # TODO(josiahk): Integrate _IsBuildFailureUniqueAcrossPlatforms() into
   # need_new_try_job boolean
   if need_new_try_job:
     _IsBuildFailureUniqueAcrossPlatforms(
         master_name, builder_name, build_number, build_failure_type,
         builds[str(build_number)]['blame_list'], failed_steps, signals,
@@ skipping 102 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