[webkit-reviews] review denied: [Bug 39257] Chromium: new-run-webkit-httpd fails to setup_mount : [Attachment 56314] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon May 17 22:13:22 PDT 2010
Eric Seidel <eric at webkit.org> has denied Fumitoshi Ukai <ukai at chromium.org>'s
request for review:
Bug 39257: Chromium: new-run-webkit-httpd fails to setup_mount
https://bugs.webkit.org/show_bug.cgi?id=39257
Attachment 56314: Patch
https://bugs.webkit.org/attachment.cgi?id=56314&action=review
------- Additional Comments from Eric Seidel <eric at webkit.org>
WebKitTools/Scripts/webkitpy/common/system/executive.py:268
+ if not isinstance(args, list) and not isinstance(args, tuple):
I think assert(isinstance(args, list) or isinstance(args, tuple)) would be
clearer. ScriptError is generally used for when the script being run has a
error.
WebKitTools/Scripts/webkitpy/common/system/executive_unittest.py:47
+ self.assertRaises(ScriptError, executive.run_command, "echo")
This would be AssertionError then.
WebKitTools/Scripts/webkitpy/common/system/executive_unittest.py:49
+ executive.run_command(["echo", "foo"])
Is this going to end up outputing "foo" on the console when test-webkitpy is
run?
WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_win_unittest.py:59
+ setup_mount = port.path_from_chromium_base("third_party",
This will fail to run in a non-chromium environment. Instead you should mock
out path_from_chromium_base for the test, no?
r- because the test would fail from a normal webkit checkout.
More information about the webkit-reviews
mailing list