[Findit] Bug fixing and implemented some feature requests.

tools/findit/findit_for_crash.py

-# Copyright 2014 The Chromium Authors. All rights reserved.
+# Copyright (c) 2014 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.
 
 import os
 from threading import Lock, Thread
 
 import blame
 from common import utils
 import component_dictionary
 import crash_utils
 import git_repository_parser
 import match_set
 import svn_repository_parser
+import svn_repository_parser

[stgao, 2014/08/22 06:50:54]

duplicate

[jeun, 2014/08/22 22:58:43]

Done.

 
 
 LINE_CHANGE_PRIORITY = 1
 FILE_CHANGE_PRIORITY = 2
 _THIS_DIR = os.path.abspath(os.path.dirname(__file__))
 CONFIG = crash_utils.ParseURLsFromConfig(os.path.join(_THIS_DIR,
                                                       'config.ini'))
 
 
 def GenerateMatchEntry(
@@ skipping 113 matching lines @@
   threads = []
 
   # Iterate through the crashed files in the stacktrace.
   for crashed_file_path in file_to_crash_info:
     # Ignore header file.
     if crashed_file_path.endswith('.h'):
       continue
 
     # If the file in the stacktrace is not changed in any commits, continue.
     for changed_file_path in file_to_revision_info:
-      changed_file_name = changed_file_path.split('/')[-1]
-      crashed_file_name = crashed_file_path.split('/')[-1]
-
+      changed_file_name = changed_file_path.split('/')[-1].lower()

[stgao, 2014/08/22 06:50:54]

use os.path.basename instead.

[jeun, 2014/08/22 22:58:43]

Changed to split('/') so that it would also work on windows machine.

+      crashed_file_name = crashed_file_path.split('/')[-1].lower()
       if changed_file_name != crashed_file_name:
         continue
 
       if not crash_utils.GuessIfSameSubPath(
-          changed_file_path, crashed_file_path):
+          changed_file_path.lower(), crashed_file_path.lower()):
         continue
 
       crashed_line_numbers = file_to_crash_info.GetCrashedLineNumbers(
           crashed_file_path)
       stack_frame_nums = file_to_crash_info.GetCrashStackFrameIndices(
           crashed_file_path)
       functions = file_to_crash_info.GetCrashFunctions(crashed_file_path)
 
       # Iterate through the CLs that this file path is changed.
       for (cl, file_change_type) in file_to_revision_info[changed_file_path]:
@@ skipping 183 matching lines @@
     match_with_stack_priority: A match object, with the CL it is from and what
                                callstack it is from.
 
   Returns:
     A sort key.
   """
   (stack_priority, _, match) = match_with_stack_priority
 
   return (min(match.priorities),
           stack_priority,
+          match.min_distance,

[stgao, 2014/08/22 06:50:53]

Why we do this change?

[jeun, 2014/08/22 22:58:43]

Abhishek suggested it.

           crash_utils.FindMinStackFrameNumber(match.stack_frame_indices,
-                                              match.priorities),
-          -len(match.changed_files), match.min_distance)
+                                              match.priorities))
 
 
 def SortAndFilterMatches(matches, num_important_frames=5):
   """Filters the list of potential culprit CLs to remove noise.
 
   Args:
     matches: A list containing match results.
     num_important_frames: A number of frames on the top of the frame to Check
                           for when filtering the results. A match with a file
                           that is in top num_important_frames of the stacktrace
                           is regarded more probable then others.
 
   Returns:
     Filtered match results.
   """
   new_matches = []
   line_changed = False
   is_important_frame = False
   highest_priority_stack = crash_utils.INFINITY
   matches.sort(key=SortMatchesFunction)
-
   # Iterate through the matches to find out what results are significant.
   for stack_priority, cl, match in matches:
     # Check if the current match changes crashed line.
     is_line_change = (min(match.priorities) == LINE_CHANGE_PRIORITY)
 
     # Check which stack this match is from, and finds the highest priority
     # callstack up to this point.
     current_stack = stack_priority
     if current_stack < highest_priority_stack:
       highest_priority_stack = current_stack
@@ skipping 33 matching lines @@
   Args:
     matches: A list of match objects.
   """
   # Iterate through the matches in the list.
   for i, _, match in matches:
     reason = []
 
     # Zip the files in the match by the reason they are suspected
     # (how the file is modified).
     match_by_priority = zip(
-        match.priorities, match.crashed_line_numbers, match.changed_files,
-        match.changed_file_urls)
+        match.priorities, match.crashed_line_numbers, match.changed_files)
 
     # Sort the zipped changed files in the match by their priority so that the
     # changed lines comes first in the reason.
     match_by_priority.sort(
-        key=lambda (priority, crashed_line_number, file_name, url): priority)
+        key=lambda (priority, crashed_line_numbers, file_name): priority)
 
     # Iterate through the sorted match.
     for i in range(len(match_by_priority)):
-      (priority, crashed_line_number, file_name, file_url) = \
-          match_by_priority[i]
+      (priority, crashed_line_numbers, file_name) = match_by_priority[i]
 
       # If the file in the match is a line change, append a explanation.
       if priority == LINE_CHANGE_PRIORITY:
+        crashed_line_numbers = [crashed_line_number
+                                for lines in crashed_line_numbers
+                                for crashed_line_number in lines]
         reason.append(
-            'Line %s of file %s      which potentially caused the crash '
+            'Line %s of file %s which potentially caused the crash '
             'according to the stacktrace, is changed in this cl.\n' %
-            (crash_utils.PrettifyList(crashed_line_number),
-             crash_utils.PrettifyFiles([(file_name, file_url)])))
+            (crash_utils.PrettifyList(crashed_line_numbers),
+             file_name))
 
       else:
         # Get all the files that are not line change.
         rest_of_the_files = match_by_priority[i:]
 
         if len(rest_of_the_files) == 1:
-          file_string = 'File %s      is changed in this cl.\n'
+          file_string = 'File %s is changed in this cl.\n'
         else:
-          file_string = 'Files %s      are changed in this cl.\n'
+          file_string = 'Files %s are changed in this cl.\n'
 
-        # Create a list of file name and its url, and prettify the list.
-        file_name_with_url = [(file_name, file_url)
-                              for (_, _, file_name, file_url)
-                              in rest_of_the_files]
-        pretty_file_name_url = crash_utils.PrettifyFiles(file_name_with_url)
+        # Create a list of file names, and prettify the list.
+        file_names = [file_name

[Martin Barbella, 2014/08/22 02:24:14]

Nit: just include all of this as a continuation on the next line (move file_name
down with the rest)

[jeun, 2014/08/22 22:58:43]

Done.

+                      for (_, _, file_name) in rest_of_the_files]
+        pretty_file_names = crash_utils.PrettifyList(file_names)
 
         # Add the reason, break because we took care of the rest of the files.
-        reason.append(file_string % pretty_file_name_url)
+        reason.append(file_string % pretty_file_names)
         break
 
     # Set the reason as string.
     match.reason = ''.join(reason)
 
 
 def CombineMatches(matches):
   """Combine possible duplicates in matches.
 
   Args:
@@ skipping 44 matching lines @@
 
 
 def ParseCrashComponents(main_stack):
   """Parses the crashing component.
 
   Crashing components is a component that top_n_frames of the stacktrace is
   from.
 
   Args:
     main_stack: Main stack from the stacktrace.
-    top_n_frames: A number of frames to regard as crashing frame.
 
   Returns:
     A set of components.
   """
   components = set()
 
   for frame in main_stack.frame_list:
     components.add(frame.component_path)
 
   return components
@@ skipping 46 matching lines @@
                no results for all stacktraces in the stacktrace_list.
     component_to_regression_dict: A parsed regression information as a
                                   result of parsing DEPS file.
     component_to_crash_revision_dict: A parsed crash revision information.
 
   Returns:
     A list of result objects, with the message how the result is created.
   """
   # If regression information is not available, return blame information.
   if not component_to_regression_dict:
-    return_message = (
-        'Regression information is not available. The result is '
-        'the blame information.')
     result = GenerateAndFilterBlameList(callstack,
                                         component_to_crash_revision_dict,
                                         component_to_regression_dict)
+    if result:
+      return_message = (
+          'Regression information is not available. The result is '
+          'the blame information.')
+    else:
+      return_message = ('Findit could not find any suspected CLs.')

[stgao, 2014/08/22 06:50:54]

Please add a comment on why we would reach this case.

[jeun, 2014/08/22 22:58:43]

Done.

+
     return (return_message, result)
 
   for stacktrace in stacktrace_list:
     # Check the next stacktrace if current one is empty.
     if not stacktrace.stack_list:
       continue
 
     # Get the crash stack for this stacktrace, and extract crashing components
     # from it.
     main_stack = stacktrace.GetCrashStack()
     components = ParseCrashComponents(main_stack)
 
     result_for_stacktrace = FindMatchForStacktrace(
         stacktrace, components, component_to_regression_dict)
+    filtered_result = FilterAndGenerateReasonForMatches(result_for_stacktrace)
 
     # If the result is empty, check the next stacktrace. Else, return the
     # filtered result.
-    if not result_for_stacktrace:
+    if not filtered_result:
       continue
 
     return_message = (
         'The result is a list of CLs that change the crashed files.')
-    result = FilterAndGenerateReasonForMatches(result_for_stacktrace)
-    return (return_message, result)
+    return (return_message, filtered_result)
 
   # If no match is found, return the blame information for the input
   # callstack.
-  return_message = (
-      'There are no CLs that change the crashed files. The result is the '
-      'blame information.')
   result = GenerateAndFilterBlameList(
       callstack, component_to_crash_revision_dict,
       component_to_regression_dict)
+
+  if result:
+    return_message = (
+        'No CL in the regression changes the crashed files. The result is '
+        'the blame information.')
+  else:
+    return_message = ('Findit could not find any suspected CLs.')
+
   return (return_message, result)