[Webkit-unassigned] [Bug 99588] Use forwarder2 in Chrome for Android layout tests.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Dec 5 13:27:36 PST 2012
https://bugs.webkit.org/show_bug.cgi?id=99588
Tony Chang <tony at chromium.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #177733|review? |review-
Flag| |
--- Comment #35 from Tony Chang <tony at chromium.org> 2012-12-05 13:30:02 PST ---
(From update of attachment 177733)
View in context: https://bugs.webkit.org/attachment.cgi?id=177733&action=review
> Tools/Scripts/webkitpy/common/system/executive.py:388
> + return_exit_code_and_output=False,
Is this necessary? Would it be sufficient to include an error_handler? The error_handler gets both the exit_code and the output and will always be called before |run_command| returns the output.
> Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:33
> +import sets
Nit: This appears to be unused (and should be unnecessary because of the built-in "set" type).
> Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:450
> + (exit_code, output) = Forwarder._run_shell_command_and_get_exit_code(
Nit: No () on the left side, although if you use an error handler, I don't think you need to check the return value at all. Same with the other calls to _run_adb_shell_command_and_get_exit_code below.
> Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:521
> + @staticmethod
> + def _run_adb_shell_command_and_get_exit_code(command_executor, android_cmd, cmd_array):
> + # Note that the extra '*' character below is used to handle the case where the process' output is not newline terminated.
Can we add some unit tests for this function? It looks pretty fragile.
> Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:528
> + if status_separator_pos == 0:
Nit: if not status_separator_pos:
> Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:563
> + self._android_cmd = AndroidCommands(
Nit: android_cmd -> android_commands. We don't normally use abbreviations for variable names in WebKit.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list