[Webkit-unassigned] [Bug 150056] run-webkit-tests does not copy all crash logs for layout test failures on iOS

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Oct 16 12:59:46 PDT 2015


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

David Kilzer (:ddkilzer) <ddkilzer at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #262955|review?, commit-queue?      |review+, commit-queue-
              Flags|                            |

--- Comment #5 from David Kilzer (:ddkilzer) <ddkilzer at webkit.org> ---
Comment on attachment 262955
  --> https://bugs.webkit.org/attachment.cgi?id=262955
Proposed patch

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

r=me, but please review comments.

>> Tools/Scripts/webkitpy/common/system/crashlogs.py:122
>> +            return  basename.endswith(".crash")
> 
> Why are there two spaces here?

Please remove the double space before landing.

>> Tools/Scripts/webkitpy/common/system/crashlogs.py:146
>> +                        test_name = process_name + "-" + str(pid)
> 
> This is not great naming - it's not "test name" in this case.

test_or_process_name might be a better variable name.

This change is not required to land the patch, but please consider changing it.

>> Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:310
>> +                if not (any(process[0] == test for process in crashed_processes)):
> 
> Are the outer parentheses necessary?

Please remove outer parenthesis if not necessary.

>> Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py:45
>> +(PASS, FAIL, TEXT, IMAGE, IMAGE_PLUS_TEXT, AUDIO, TIMEOUT, CRASH, CRASH_OTHER, SKIP, WONTFIX,
> 
> Don't we need to add support for this new result everywhere? Bot watcher's dashboard and flakiness dashboard come to mind first. Maybe EWS too.
> 
> But since these crashes are already in a separate section of the JSON, I don't see why we need a new result.

I'm not sure what Alexey's concern is here.  As long as this generates new results.html files without causing the Dashboards to fail in some way, the patch seems fine.

Please follow-up with Alexey before landing this.

-- 
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/20151016/4ceaf303/attachment.html>


More information about the webkit-unassigned mailing list