tools/findit/findit_for_crash.py - Issue 478763003: [Findit] Bug fixing and implemented some feature requests.

Unified Diff: tools/findit/findit_for_crash.py

Issue 478763003: [Findit] Bug fixing and implemented some feature requests. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: fixed a bug in intersection Created 6 years, 4 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

« tools/findit/findit_for_clusterfuzz.py ('K') | « tools/findit/findit_for_clusterfuzz.py ('k') | tools/findit/git_repository_parser.py » ('j') | tools/findit/git_repository_parser.py » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: tools/findit/findit_for_crash.py

diff --git a/tools/findit/findit_for_crash.py b/tools/findit/findit_for_crash.py

index 2fab473cb481838b774c381bd8593cffbfa62fa9..625a94dc21ffaf5d0930a19de7a0b745f23fb86d 100644

--- a/tools/findit/findit_for_crash.py

+++ b/tools/findit/findit_for_crash.py

@@ -1,4 +1,4 @@

# Use of this source code is governed by a BSD-style license that can be

# found in the LICENSE file.

@@ -12,6 +12,7 @@ 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

@@ -145,14 +146,13 @@ def FindMatch(revisions_info_map, file_to_revision_info, file_to_crash_info,

# 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 o

+ 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(

@@ -356,9 +356,9 @@ def SortMatchesFunction(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):

@@ -379,7 +379,6 @@ def SortAndFilterMatches(matches, num_important_frames=5):

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.

@@ -433,44 +432,44 @@ def GenerateReasonForMatches(matches):

# 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

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.

@@ -535,7 +534,6 @@ def ParseCrashComponents(main_stack):

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.

@@ -602,12 +600,16 @@ def FindItForCrash(stacktrace_list,

"""

# 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 ca

jeun 2014/08/22 22:58:43 Done.

return (return_message, result)

for stacktrace in stacktrace_list:

@@ -622,24 +624,29 @@ def FindItForCrash(stacktrace_list,

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)

« tools/findit/findit_for_clusterfuzz.py ('K') | « tools/findit/findit_for_clusterfuzz.py ('k') | tools/findit/git_repository_parser.py » ('j') | tools/findit/git_repository_parser.py » ('J')