105061 2012-12-14 15:28:15 -0800 rpt --profile --chromium-android throws exception 2012-12-17 11:36:33 -0800 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 pdr eric abarth dbates dpranke eric webkit.review.bot oldest_to_newest 791909 0 pdr 2012-12-14 15:28:15 -0800 pdr@yourcomputer:/ssd/chromium$ ./src/third_party/WebKit/Tools/Scripts/run-perf-tests --release --profile --chromium-android src/third_party/WebKit/PerformanceTests/Bindings/first-child.html Throws the following: ANDROID_SYMFS not set, using /ssd/chromium/src/webkit/Release/layout-test-results/symfs Updating symfs libary (/ssd/chromium/src/webkit/Release/layout-test-results/symfs/data/app-lib/org.chromium.native_test-1/libDumpRenderTree.so) from built copy (/ssd/chromium/src/out/Release/lib/libDumpRenderTree.so) Running Bindings/first-child.html (1 of 1) Traceback (most recent call last): File "./src/third_party/WebKit/Tools/Scripts/run-perf-tests", line 40, in <module> sys.exit(PerfTestsRunner().run()) File "/ssd/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py", line 190, in run unexpected = self._run_tests_set(sorted(list(tests), key=lambda test: test.test_name()), self._port) File "/ssd/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py", line 330, in _run_tests_set if self._run_single_test(test, driver): File "/ssd/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py", line 344, in _run_single_test new_results = test.run(driver, self._options.time_out_ms) File "/ssd/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/performance_tests/perftest.py", line 71, in run output = self.run_single(driver, self.path_or_url(), time_out_ms) File "/ssd/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/performance_tests/perftest.py", line 78, in run_single return driver.run_test(DriverInput(path_or_url, time_out_ms, image_hash=None, should_run_pixel_test=should_run_pixel_test), stop_when_done=False) File "/ssd/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py", line 745, in run_test return super(ChromiumAndroidDriver, self).run_test(driver_input, stop_when_done) File "/ssd/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/driver.py", line 165, in run_test self.start(driver_input.should_run_pixel_test, driver_input.args) File "/ssd/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py", line 754, in start super(ChromiumAndroidDriver, self).start(pixel_tests, per_test_args) File "/ssd/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/driver.py", line 280, in start self._start(pixel_tests, per_test_args) File "/ssd/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py", line 757, in _start self._setup_test() File "/ssd/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py", line 553, in _setup_test self._setup_md5sum_and_push_data_if_needed() File "/ssd/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py", line 544, in _setup_md5sum_and_push_data_if_needed self._push_executable() File "/ssd/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py", line 597, in _push_executable self._push_file_if_needed(self._port.path_to_forwarder(), DEVICE_FORWARDER_PATH) File "/ssd/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py", line 591, in _push_file_if_needed stdout=subprocess.PIPE).stdout) File "/ssd/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 488, in popen string_args = self._stringify_args(*args) File "/ssd/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 476, in _stringify_args string_args = map(unicode, *args) TypeError: map() requires at least two args 792062 1 eric 2012-12-15 00:45:21 -0800 I had a subtle misunderstanding of *args. Fixing. 792063 2 179579 eric 2012-12-15 00:50:28 -0800 Created attachment 179579 Patch 792066 3 eric 2012-12-15 00:57:08 -0800 At some point along the line, popen(args, **kwargs) got written as popen(*args, **kwargs), which worked fine when we were just passing along *args to subprocess.popen. But popen actually only has one non-keyword argument, and that happens to be (confusingly) named "args". So at least one caller (in ChromiumAndroidDriver) was calling popen(args=[...]) explicitly naming the "args" argument. Thus when you write: def popen(*args, **kwargs) "args" is then a tuple of the non-keyword arguments, commonly containing as args[0] the list of arguments known as subprocess.Popens "args". We hit this bug because when you expand an empty tuple using the "*" operator, you get back nothing, no arguments, so: map(unicode, *args), turns into: map(unicode) when args is an empty tuple. The fix here is to not bother with the argument globbing (since we only have a single non-keyword argument anyway!) and instead write: def popen(args, **kwargs) The only reason this is at all confusing is because we have a naming collision between the standard "varargs" pattern for python and the "args" name for the first argument of subprocess.popen. 792068 4 eric 2012-12-15 00:59:56 -0800 In python all arguments are "named" and can be passed as keyword args (aka kwargs) to a function. When you want to glob the arguments (aka varargs in C) you use the standard pattern: def my_function(*args, **kwargs) Which gives you a tuple "args" containing all the arguments which were passed w/o keywords, and a dictionary kwargs with all the named args. Using * and ** respectively will expand a tuple or dictionary back into arguments when calling a function: some_other_function(*tuple, **dictionary) You don't have to use both, of course, but it's very common to see *args, **kwargs in code. 792155 5 179579 dbates 2012-12-15 11:48:19 -0800 Comment on attachment 179579 Patch r=me 792156 6 179579 dbates 2012-12-15 11:48:46 -0800 Comment on attachment 179579 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=179579&action=review > Tools/Scripts/webkitpy/common/system/executive.py:480 > + # The only required arugment to popen is named "args", the rest are optional keyword arguments. Nit: arugment => argument 792179 7 179579 dpranke 2012-12-15 13:19:07 -0800 Comment on attachment 179579 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=179579&action=review >> Tools/Scripts/webkitpy/common/system/executive.py:480 >> + # The only required arugment to popen is named "args", the rest are optional keyword arguments. > > Nit: arugment => argument I probably wouldn't bother with the comment. I also considered suggesting changing the name of the argument from "args" to "cmd" (or something) to avoid the potential confusion, but I think leaving the name the same as the underlying popen() is probably a better idea. 792889 8 179579 webkit.review.bot 2012-12-17 11:36:29 -0800 Comment on attachment 179579 Patch Clearing flags on attachment: 179579 Committed r137927: <http://trac.webkit.org/changeset/137927> 792890 9 webkit.review.bot 2012-12-17 11:36:33 -0800 All reviewed patches have been landed. Closing bug. 179579 2012-12-15 00:50:28 -0800 2012-12-17 11:36:28 -0800 Patch bug-105061-20121215004751.patch text/plain 3606 eric U3VidmVyc2lvbiBSZXZpc2lvbjogMTM3Nzk1CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggY2JlODdiMGQ0MWZkMjEzNjZmOTQ3NzlkYTNhYWZkMDQ0 OTdhYmM3OC4uOTEwMTIwNTc4YmY0MmE4OWJkOWMzNWY2YWI3ODZkOWNkNTIyNWExNyAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIx IEBACisyMDEyLTEyLTE1ICBFcmljIFNlaWRlbCAgPGVyaWNAd2Via2l0Lm9yZz4KKworICAgICAg ICBycHQgLS1wcm9maWxlIC0tY2hyb21pdW0tYW5kcm9pZCB0aHJvd3MgZXhjZXB0aW9uCisgICAg ICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMDUwNjEKKworICAg ICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBTb21laG93IHBvcGVu KGFyZ3MpIGdvdCB0dXJuZWQgaW50byBwb3BlbigqYXJncykgYXQgc29tZSBwb2ludCwKKyAgICAg ICAgd2hpY2ggY2F1c2VkIG15IG5ldyBzdHJpbmdpZnkgY29kZSB0byBmYWlsIHdoZW4gYXJncyB3 YXMgcGFzc2VkIGJ5IG5hbWUuCisgICAgICAgIEZpeGVkIGFuZCB0ZXN0ZWQuCisKKyAgICAgICAg KiBTY3JpcHRzL3dlYmtpdHB5L2NvbW1vbi9zeXN0ZW0vZXhlY3V0aXZlLnB5OgorICAgICAgICAo RXhlY3V0aXZlLl9zdHJpbmdpZnlfYXJncyk6CisgICAgICAgIChFeGVjdXRpdmUpOgorICAgICAg ICAoRXhlY3V0aXZlLnBvcGVuKToKKyAgICAgICAgKiBTY3JpcHRzL3dlYmtpdHB5L2NvbW1vbi9z eXN0ZW0vZXhlY3V0aXZlX3VuaXR0ZXN0LnB5OgorICAgICAgICAoRXhlY3V0aXZlVGVzdC50ZXN0 X3BvcGVuX2FyZ3MpOgorCiAyMDEyLTEyLTE0ICBEaXJrIFByYW5rZSAgPGRwcmFua2VAY2hyb21p dW0ub3JnPgogCiAgICAgICAgIHdlYmtpdHB5OiBtYWtlIHBvcnRfbmFtZSBhIHJlcXVpcmVkIHBh cmFtZXRlciB0byB0aGUgbm9uLXRlc3QgcG9ydHMKZGlmZiAtLWdpdCBhL1Rvb2xzL1NjcmlwdHMv d2Via2l0cHkvY29tbW9uL3N5c3RlbS9leGVjdXRpdmUucHkgYi9Ub29scy9TY3JpcHRzL3dlYmtp dHB5L2NvbW1vbi9zeXN0ZW0vZXhlY3V0aXZlLnB5CmluZGV4IGE5M2FiNzA2MzI4YWU5ZjM4OTY0 MjliYmUwMGIxY2UwYzZiZTM2NTUuLjI5N2NjNzNkMThhMzBhZWIzOGY5NWZmYWQ2Nzc0ZjA3OWQz ZjZhOGYgMTAwNjQ0Ci0tLSBhL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvY29tbW9uL3N5c3RlbS9l eGVjdXRpdmUucHkKKysrIGIvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9jb21tb24vc3lzdGVtL2V4 ZWN1dGl2ZS5weQpAQCAtNDcxLDEzICs0NzEsMTQgQEAgY2xhc3MgRXhlY3V0aXZlKG9iamVjdCk6 CiAgICAgICAgICAgICByZXR1cm4gYXJndW1lbnQKICAgICAgICAgcmV0dXJuIGFyZ3VtZW50LmVu Y29kZShzZWxmLl9jaGlsZF9wcm9jZXNzX2VuY29kaW5nKCkpCiAKLSAgICBkZWYgX3N0cmluZ2lm eV9hcmdzKHNlbGYsICphcmdzKToKKyAgICBkZWYgX3N0cmluZ2lmeV9hcmdzKHNlbGYsIGFyZ3Mp OgogICAgICAgICAjIFBvcGVuIHdpbGwgdGhyb3cgYW4gZXhjZXB0aW9uIGlmIGFyZ3MgYXJlIG5v bi1zdHJpbmdzIChsaWtlIGludCgpKQotICAgICAgICBzdHJpbmdfYXJncyA9IG1hcCh1bmljb2Rl LCAqYXJncykKKyAgICAgICAgc3RyaW5nX2FyZ3MgPSBtYXAodW5pY29kZSwgYXJncykKICAgICAg ICAgIyBUaGUgV2luZG93cyBpbXBsZW1lbnRhdGlvbiBvZiBQb3BlbiBjYW5ub3QgaGFuZGxlIHVu aWNvZGUgc3RyaW5ncy4gOigKICAgICAgICAgcmV0dXJuIG1hcChzZWxmLl9lbmNvZGVfYXJndW1l bnRfaWZfbmVlZGVkLCBzdHJpbmdfYXJncykKIAotICAgIGRlZiBwb3BlbihzZWxmLCAqYXJncywg Kiprd2FyZ3MpOgorICAgICMgVGhlIG9ubHkgcmVxdWlyZWQgYXJ1Z21lbnQgdG8gcG9wZW4gaXMg bmFtZWQgImFyZ3MiLCB0aGUgcmVzdCBhcmUgb3B0aW9uYWwga2V5d29yZCBhcmd1bWVudHMuCisg ICAgZGVmIHBvcGVuKHNlbGYsIGFyZ3MsICoqa3dhcmdzKToKICAgICAgICAgIyBGSVhNRTogV2Ug c2hvdWxkIGFsd2F5cyBiZSBzdHJpbmdpZnlpbmcgdGhlIGFyZ3MsIGJ1dCBjYWxsZXJzIHdobyBw YXNzIHNoZWxsPVRydWUKICAgICAgICAgIyBleHBlY3QgdGhhdCB0aGUgZXhhY3QgYnl0ZXMgcGFz c2VkIHdpbGwgZ2V0IHBhc3NlZCB0byB0aGUgc2hlbGwgKGV2ZW4gaWYgdGhleSdyZSB3cm9uZ2x5 IGVuY29kZWQpLgogICAgICAgICAjIHNoZWxsPVRydWUgaXMgd3JvbmcgZm9yIG1hbnkgb3RoZXIg cmVhc29ucywgYW5kIHdlIHNob3VsZCByZW1vdmUgdGhpcwpAQCAtNDg1LDcgKzQ4Niw3IEBAIGNs YXNzIEV4ZWN1dGl2ZShvYmplY3QpOgogICAgICAgICBpZiBrd2FyZ3MuZ2V0KCdzaGVsbCcpID09 IFRydWU6CiAgICAgICAgICAgICBzdHJpbmdfYXJncyA9IGFyZ3MKICAgICAgICAgZWxzZToKLSAg ICAgICAgICAgIHN0cmluZ19hcmdzID0gc2VsZi5fc3RyaW5naWZ5X2FyZ3MoKmFyZ3MpCisgICAg ICAgICAgICBzdHJpbmdfYXJncyA9IHNlbGYuX3N0cmluZ2lmeV9hcmdzKGFyZ3MpCiAgICAgICAg IHJldHVybiBzdWJwcm9jZXNzLlBvcGVuKHN0cmluZ19hcmdzLCAqKmt3YXJncykKIAogICAgIGRl ZiBydW5faW5fcGFyYWxsZWwoc2VsZiwgY29tbWFuZF9saW5lc19hbmRfY3dkcywgcHJvY2Vzc2Vz PU5vbmUpOgpkaWZmIC0tZ2l0IGEvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9jb21tb24vc3lzdGVt L2V4ZWN1dGl2ZV91bml0dGVzdC5weSBiL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvY29tbW9uL3N5 c3RlbS9leGVjdXRpdmVfdW5pdHRlc3QucHkKaW5kZXggZjQyNTE3ZGM0OWE5ZDcxNWZkZjNmM2U5 ZTg4MjZmODYyNDIxMzA5ZC4uMDQwNmY5ZGY2MTBjNWM3NTkwYjQ0ZjViOTNlZTk5MmIxZDk3ZWY4 YSAxMDA2NDQKLS0tIGEvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9jb21tb24vc3lzdGVtL2V4ZWN1 dGl2ZV91bml0dGVzdC5weQorKysgYi9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L2NvbW1vbi9zeXN0 ZW0vZXhlY3V0aXZlX3VuaXR0ZXN0LnB5CkBAIC0xMTgsNiArMTE4LDExIEBAIGNsYXNzIEV4ZWN1 dGl2ZVRlc3QodW5pdHRlc3QuVGVzdENhc2UpOgogICAgICAgICBleGVjdXRpdmUucnVuX2NvbW1h bmQoY29tbWFuZF9saW5lKCdlY2hvJywgMSkpCiAgICAgICAgIGV4ZWN1dGl2ZS5wb3Blbihjb21t YW5kX2xpbmUoJ2VjaG8nLCAxKSkud2FpdCgpCiAKKyAgICBkZWYgdGVzdF9wb3Blbl9hcmdzKHNl bGYpOgorICAgICAgICBleGVjdXRpdmUgPSBFeGVjdXRpdmUoKQorICAgICAgICAjIEV4cGxpY2l0 bHkgbmFtaW5nIHRoZSAnYXJncycgYXJndW1lbnQgc2hvdWxkIG5vdCB0aG93IGFuIGV4Y2VwdGlv bi4KKyAgICAgICAgZXhlY3V0aXZlLnBvcGVuKGFyZ3M9Y29tbWFuZF9saW5lKCdlY2hvJywgMSkp LndhaXQoKQorCiAgICAgZGVmIHRlc3RfcnVuX2NvbW1hbmRfd2l0aF91bmljb2RlKHNlbGYpOgog ICAgICAgICAiIiJWYWxpZGF0ZSB0aGF0IGl0IGlzIHNhZmUgdG8gcGFzcyB1bmljb2RlKCkgb2Jq ZWN0cwogICAgICAgICB0byBFeGVjdXRpdmUucnVuKiBtZXRob2RzLCBhbmQgdGhleSB3aWxsIHJl dHVybiB1bmljb2RlKCkK