Bug 161218 – The same crash shows up in both "Tests that crashed" and "Other Crashes"

• Component: Tools / Tests
• Status: RESOLVED LATER

[Jonathan Bedard]

From rdar://problem/28012721

Saw exactly the same crash getting copied twice.

https://build-safari.apple.com/results/Trunk%20Monarch%20iOS%20Simulator%20Debug%20WK2%20Tests/r204958_86618_dc022086176755f24da70a0590374f77500cfa87%20(4867)/results.html

[Jonathan Bedard]

Created attachment 287038 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 287038 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=287038&action=review

> Tools/ChangeLog:12
> +        (Manager._look_for_new_crash_logs): Tied search to PID and test name, instead of just test name.

Will there be confusion with regards to UI process vs. web process PID?

My suspicion was that this was a logic error related to tests that are expected crashes per TestExpectations.

[Jonathan Bedard]

Comment on attachment 287038 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=287038&action=review

>> Tools/ChangeLog:12
>> +        (Manager._look_for_new_crash_logs): Tied search to PID and test name, instead of just test name.
> 
> Will there be confusion with regards to UI process vs. web process PID?
> 
> My suspicion was that this was a logic error related to tests that are expected crashes per TestExpectations.

Well, in the specific crash that prompted this change, the crash-log was explicitly added twice (i.e., two "Adding results for other crash: ..." messages were in standard out). Previously, we were comparing the entire process name to all previously recognized crashed processes, assuming that our list of crashlogs had no doubles in it.  First, this fix eliminates a problem when a crashlog is somehow included twice in the list of crash logs, since that's the only way to get two "Adding results for other crash: ..." Messages.

Second, it bases comparison off of process names and ID instead of assuming that the test name will be the process name.  I don't think that there should be any issues confusing the UI process and web process, since we are still tracking the process name (that's process[1]) .

[Jonathan Bedard]

This is a symptom of a larger issue with Webkitpy confusing process names and test names.  Marking resolved, will revisit in a later over-haul of crash handling in Webkitpy.