[Webkit-unassigned] [Bug 159827] run-webkit-tests should trigger a spindump when WebContent process is unresponsive

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=159827

--- Comment #50 from Jonathan Bedard <jbedard at apple.com> ---
Comment on attachment 285120
  --> https://bugs.webkit.org/attachment.cgi?id=285120
Patch

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

>> Tools/Scripts/webkitpy/port/ios.py:423
>> +            ], return_exit_code=True)
> 
> Notice that Executive.run_command() only throws a ScriptError exception if return_exit_code := False. It is good programming practice to limit the scope of a try-except to the minimum code that can raise an exception to help make it straightforward to debug the cause of an exception. In comment #25 I gave an example of one way we can write this code such that we limit the scope of the try-except block.

This perhaps brings up another question, then, about sample_file_path(...), which calls filesystem.join(...).  In the original sample_process code, finding the report was in the try catch block, which is why I kept it there after adding spindump.  If filesystem.join(...) throws an exception (or results_directory(), actually), what is our desired functionality?  I think for the time being, keeping the generation of the hang_report file path inside the try-except block is prudent, as the tests can continue if an error happens while attempting to generate either version of the crash report, and it would seem unhelpful to be to have two try-except blocks in this function. 

I will look into this while refactoring the Mac and IOS ports, which will address some of the code duplication in this patch, if you think this is worth changing.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160802/a7ca32e2/attachment.html>