78213 2012-02-09 01:14:38 -0800 run-webkit-tests should handle empty testlist 2012-06-19 14:42:41 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Unspecified Unspecified RESOLVED WONTFIX NRWT P2 Normal --- 78699 1 hnandor webkit-unassigned dpranke eric menard ojan ossy webkit.review.bot zoltan oldest_to_newest 552947 0 hnandor 2012-02-09 01:14:38 -0800 If we give an empty testlist to the run-webkit-tests it runs all the tests. 552954 1 126258 hnandor 2012-02-09 01:22:26 -0800 Created attachment 126258 Patch It checks also whether testlist is exists. 553090 2 126288 hnandor 2012-02-09 05:39:00 -0800 Created attachment 126288 Patch Fix indentation issues. 553135 3 ossy 2012-02-09 07:01:07 -0800 (In reply to comment #2) > Created an attachment (id=126288) [details] > Patch --test-list is an NRWT parameter, so NRWT should handle it, not the run-webkit-tests perl wrapper script. And the return value of the NRWT should be 0 if the file given in --test-list has empty content. 554025 4 126503 hnandor 2012-02-10 05:09:06 -0800 Created attachment 126503 Patch (In reply to comment #3) I have moved the modifications to the new-run-webkit-tests. 554171 5 126503 eric 2012-02-10 10:26:42 -0800 Comment on attachment 126503 Patch This needs unittests. Also free functions are generally discouraged (as they're difficult to mock). 554476 6 dpranke 2012-02-10 16:11:10 -0800 I could be convinced that passing an empty file to --test-list should not result in running all the tests, but why would we want it to do nothing and return success instead of returning an error? (This is an honest question; maybe you have some need for this behavior)? That said, this logic does not belong in the new-run-webkit-tests wrapper script; it needs to be next to the other logic for handling --test-file, and the code should be using the host/filesystem abstraction rather than calling os.path directly. Also, I agree w/ Eric's comments re: needing tests. 557197 7 ossy 2012-02-15 05:55:50 -0800 We need this change for selective testing. (see https://bugs.webkit.org/show_bug.cgi?id=78699 for details) If a change doesn't affect any tests, run-selective-webkit-tests would call run-webkit-tests with empty testlist. Now if you give an empty testlist to NRWT, it runs all tests. I think it shouldn't run any test and return with success (0), because a failing return code won't be good if we want to use it on a buildbot or in an EWS. 557801 8 dpranke 2012-02-15 18:29:23 -0800 (In reply to comment #7) > We need this change for selective testing. (see https://bugs.webkit.org/show_bug.cgi?id=78699 for details) If a change doesn't affect any tests, run-selective-webkit-tests would call run-webkit-tests with empty testlist. > Now if you give an empty testlist to NRWT, it runs all tests. I think it shouldn't run any test and return with success (0), because a failing return code won't be good if we want to use it on a buildbot or in an EWS. Okay, I now understand the context a bit better, but if you have no tests, why invoke run-webkit-tests at all? Can't you just short-circuit things in run-selective-webkit-tests? Given that it's not clear to me that this is generally the behavior one would want, it seems like getting custom behavior is better enforced in the caller, no? 557906 9 ossy 2012-02-15 22:34:09 -0800 (In reply to comment #8) > Okay, I now understand the context a bit better, but if you have no tests, why invoke run-webkit-tests at all? Can't you just short-circuit things in run-selective-webkit-tests? Given that it's not clear to me that this is generally the behavior one would want, it seems like getting custom behavior is better enforced in the caller, no? I thought over, you're absolutely right. Checking it in run-selective-webkit-tests is simpler and reasonable. We will do it. (In reply to comment #6) > I could be convinced that passing an empty file to --test-list should not result in running all the tests, but why would we want it to do nothing and return success instead of returning an error? (This is an honest question; maybe you have some need for this behavior)? After rethinking this one too, I support returning an error code if we still pass an empty testlist to NRWT. 652835 10 dpranke 2012-06-19 14:42:41 -0700 closing this as wontfix ... 126258 2012-02-09 01:22:26 -0800 2012-02-09 05:39:00 -0800 Patch runwktestlist.diff text/plain 1609 hnandor ZGlmZiAtLWdpdCBhL1Rvb2xzL0NoYW5nZUxvZyBiL1Rvb2xzL0NoYW5nZUxvZwppbmRleCBjZmQw YzY1Li43ZmJhYTI5IDEwMDY0NAotLS0gYS9Ub29scy9DaGFuZ2VMb2cKKysrIGIvVG9vbHMvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTItMDItMDkgIE7DoW5kb3IgSHVzemthICA8aHVz emthLm5hbmRvckBzdHVkLnUtc3plZ2VkLmh1PgorCisgICAgICAgIHJ1bi13ZWJraXQtdGVzdHMg c2hvdWxkIGhhbmRsZSBlbXB0eSB0ZXN0bGlzdC4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTc4MjEzCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZ IChPT1BTISkuCisKKyAgICAgICAgQ2hlY2sgdGhlIGdpdmVuIC0tdGVzdC1saXN0IHBhcmFtZXRl ciB3aGV0aGVyIGl0IGV4aXN0cyBhbmQgbm9uLWVtcHR5LgorCisgICAgICAgICogU2NyaXB0cy9y dW4td2Via2l0LXRlc3RzOgorICAgICAgICAocGFyYW1ldGVyVGVzdExpc3RJc0NvcnJlY3QpOgor CiAyMDEyLTAyLTA3ICBNT1JJVEEgSGFqaW1lICA8bW9ycml0YUBnb29nbGUuY29tPgogCiAgICAg ICAgIFJlcGxhY2VtZW50IHRleHQgc2hvdWxkIGJlIGF2YWlsYWJsZSBmcm9tIHRoZSBtYXJrZXIu CmRpZmYgLS1naXQgYS9Ub29scy9TY3JpcHRzL3J1bi13ZWJraXQtdGVzdHMgYi9Ub29scy9TY3Jp cHRzL3J1bi13ZWJraXQtdGVzdHMKaW5kZXggMjRmY2ZkMC4uOTI3NmRmNyAxMDA3NTUKLS0tIGEv VG9vbHMvU2NyaXB0cy9ydW4td2Via2l0LXRlc3RzCisrKyBiL1Rvb2xzL1NjcmlwdHMvcnVuLXdl YmtpdC10ZXN0cwpAQCAtNzksNiArNzksMjggQEAgc3ViIHBsYXRmb3JtSXNSZWFkeUZvclBhcmFs bGVsVGVzdGluZygpCiAgICAgcmV0dXJuICFpc1F0KCk7CiB9CiAKK3N1YiBwYXJhbWV0ZXJUZXN0 TGlzdElzQ29ycmVjdCgpCit7CisgICAgIyBUaGlzIGZ1bmN0aW9uIHRlc3RzIHdoZXRoZXIgdGhl IGdpdmVuIHRlc3RsaXN0IGlzIGV4aXN0cyBhbmQgbm90IGVtcHR5CisgICAgaWYgKGdyZXAoLy0t dGVzdC1saXN0LywgQEFSR1YpKSB7CisJbXkgQHRlc3RMaXN0QXJnID0gZ3JlcCgvLS10ZXN0LWxp c3QvLCBAQVJHVik7CisJbXkgJHRlc3RMaXN0T3B0aW9uID0gcG9wIEB0ZXN0TGlzdEFyZzsKKwlt eSAkY3V0dGluZ0luZGV4ID0gaW5kZXgoJHRlc3RMaXN0T3B0aW9uLCAnPScpKzE7CisJbXkgJHRl c3RMaXN0UGF0aCA9IHN1YnN0ciAkdGVzdExpc3RPcHRpb24sICRjdXR0aW5nSW5kZXg7CisKKwlp ZiAoIS1lICR0ZXN0TGlzdFBhdGgpIHsKKwkgIHByaW50ICJDYW5ub3QgZmluZCB0ZXN0bGlzdCAk dGVzdExpc3RQYXRoLlxuIjsKKwkgIHJldHVybiAxOworCX0gZWxzaWYgKCEtcyAkdGVzdExpc3RQ YXRoKSB7CisJICBwcmludCAiVGVzdGxpc3QgJHRlc3RMaXN0UGF0aCBjYW5ub3QgYmUgZW1wdHku XG4iOworCSAgcmV0dXJuIDE7CisJfQorICAgIH0KKyAgICByZXR1cm4gMDsKK30KKworcGFyYW1l dGVyVGVzdExpc3RJc0NvcnJlY3QoKSBhbmQgZGllICJUZXN0aW5nIGZhaWxlZCI7CisKIG15ICRo YXJuZXNzTmFtZSA9ICJvbGQtcnVuLXdlYmtpdC10ZXN0cyI7CiAKIGlmICh1c2VOZXdSdW5XZWJL aXRUZXN0cygpKSB7Cg== 126288 2012-02-09 05:39:00 -0800 2012-02-10 05:09:06 -0800 Patch runwktestlist.diff text/plain 1696 hnandor ZGlmZiAtLWdpdCBhL1Rvb2xzL0NoYW5nZUxvZyBiL1Rvb2xzL0NoYW5nZUxvZwppbmRleCBjZmQw YzY1Li43ZmJhYTI5IDEwMDY0NAotLS0gYS9Ub29scy9DaGFuZ2VMb2cKKysrIGIvVG9vbHMvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTItMDItMDkgIE7DoW5kb3IgSHVzemthICA8aHVz emthLm5hbmRvckBzdHVkLnUtc3plZ2VkLmh1PgorCisgICAgICAgIHJ1bi13ZWJraXQtdGVzdHMg c2hvdWxkIGhhbmRsZSBlbXB0eSB0ZXN0bGlzdC4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTc4MjEzCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZ IChPT1BTISkuCisKKyAgICAgICAgQ2hlY2sgdGhlIGdpdmVuIC0tdGVzdC1saXN0IHBhcmFtZXRl ciB3aGV0aGVyIGl0IGV4aXN0cyBhbmQgbm9uLWVtcHR5LgorCisgICAgICAgICogU2NyaXB0cy9y dW4td2Via2l0LXRlc3RzOgorICAgICAgICAocGFyYW1ldGVyVGVzdExpc3RJc0NvcnJlY3QpOgor CiAyMDEyLTAyLTA3ICBNT1JJVEEgSGFqaW1lICA8bW9ycml0YUBnb29nbGUuY29tPgogCiAgICAg ICAgIFJlcGxhY2VtZW50IHRleHQgc2hvdWxkIGJlIGF2YWlsYWJsZSBmcm9tIHRoZSBtYXJrZXIu CmRpZmYgLS1naXQgYS9Ub29scy9TY3JpcHRzL3J1bi13ZWJraXQtdGVzdHMgYi9Ub29scy9TY3Jp cHRzL3J1bi13ZWJraXQtdGVzdHMKaW5kZXggMjRmY2ZkMC4uMzE3NWMyYyAxMDA3NTUKLS0tIGEv VG9vbHMvU2NyaXB0cy9ydW4td2Via2l0LXRlc3RzCisrKyBiL1Rvb2xzL1NjcmlwdHMvcnVuLXdl YmtpdC10ZXN0cwpAQCAtNzksNiArNzksMjkgQEAgc3ViIHBsYXRmb3JtSXNSZWFkeUZvclBhcmFs bGVsVGVzdGluZygpCiAgICAgcmV0dXJuICFpc1F0KCk7CiB9CiAKK3N1YiBwYXJhbWV0ZXJUZXN0 TGlzdElzQ29ycmVjdCgpCit7CisgICAgIyBUaGlzIGZ1bmN0aW9uIHRlc3RzIHdoZXRoZXIgdGhl IGdpdmVuIHRlc3RsaXN0IGlzIGV4aXN0cyBhbmQgbm90IGVtcHR5CisgICAgaWYgKGdyZXAoLy0t dGVzdC1saXN0LywgQEFSR1YpKSB7CisgICAgICAgIG15IEB0ZXN0TGlzdEFyZyA9IGdyZXAoLy0t dGVzdC1saXN0LywgQEFSR1YpOworICAgICAgICBteSAkdGVzdExpc3RPcHRpb24gPSBwb3AgQHRl c3RMaXN0QXJnOworICAgICAgICBteSAkY3V0dGluZ0luZGV4ID0gaW5kZXgoJHRlc3RMaXN0T3B0 aW9uLCAnPScpKzE7CisgICAgICAgIG15ICR0ZXN0TGlzdFBhdGggPSBzdWJzdHIgJHRlc3RMaXN0 T3B0aW9uLCAkY3V0dGluZ0luZGV4OworCisgICAgICAgIGlmICghLWUgJHRlc3RMaXN0UGF0aCkg eworICAgICAgICAgICAgcHJpbnQgIkNhbm5vdCBmaW5kIHRlc3RsaXN0ICR0ZXN0TGlzdFBhdGgu XG4iOworICAgICAgICAgICAgcmV0dXJuIDE7CisgICAgICAgIH0gZWxzaWYgKCEtcyAkdGVzdExp c3RQYXRoKSB7CisgICAgICAgICAgICBwcmludCAiVGVzdGxpc3QgJHRlc3RMaXN0UGF0aCBjYW5u b3QgYmUgZW1wdHkuXG4iOworICAgICAgICAgICAgcmV0dXJuIDE7CisgICAgICAgIH0KKyAgICB9 CisKKyAgICByZXR1cm4gMDsKK30KKworcGFyYW1ldGVyVGVzdExpc3RJc0NvcnJlY3QoKSBhbmQg ZGllICJUZXN0aW5nIGZhaWxlZCI7CisKIG15ICRoYXJuZXNzTmFtZSA9ICJvbGQtcnVuLXdlYmtp dC10ZXN0cyI7CiAKIGlmICh1c2VOZXdSdW5XZWJLaXRUZXN0cygpKSB7Cg== 126503 2012-02-10 05:09:06 -0800 2012-02-10 10:26:41 -0800 Patch runwktestlist.diff text/plain 2010 hnandor ZGlmZiAtLWdpdCBhL1Rvb2xzL0NoYW5nZUxvZyBiL1Rvb2xzL0NoYW5nZUxvZwppbmRleCBjZmQw YzY1Li41NjM2OWQzIDEwMDY0NAotLS0gYS9Ub29scy9DaGFuZ2VMb2cKKysrIGIvVG9vbHMvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTItMDItMTAgIE7DoW5kb3IgSHVzemthICA8aHVz emthLm5hbmRvckBzdHVkLnUtc3plZ2VkLmh1PgorCisgICAgICAgIHJ1bi13ZWJraXQtdGVzdHMg c2hvdWxkIGhhbmRsZSBlbXB0eSB0ZXN0bGlzdC4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTc4MjEzCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZ IChPT1BTISkuCisKKyAgICAgICAgQ2hlY2sgdGhlIGdpdmVuIHRlc3RsaXN0IHBhcmFtZXRlciB3 aGV0aGVyIGl0IGV4aXN0cyBhbmQgbm9uLWVtcHR5LgorCisgICAgICAgICogU2NyaXB0cy9uZXct cnVuLXdlYmtpdC10ZXN0czoKKyAgICAgICAgKHBhcmFtZXRlcl90ZXN0X2xpc3RfaXNfaW5jb3Jy ZWN0KToKKwogMjAxMi0wMi0wNyAgTU9SSVRBIEhhamltZSAgPG1vcnJpdGFAZ29vZ2xlLmNvbT4K IAogICAgICAgICBSZXBsYWNlbWVudCB0ZXh0IHNob3VsZCBiZSBhdmFpbGFibGUgZnJvbSB0aGUg bWFya2VyLgpkaWZmIC0tZ2l0IGEvVG9vbHMvU2NyaXB0cy9uZXctcnVuLXdlYmtpdC10ZXN0cyBi L1Rvb2xzL1NjcmlwdHMvbmV3LXJ1bi13ZWJraXQtdGVzdHMKaW5kZXggOGY3Y2QwNC4uYWIxZGMx ZCAxMDA3NTUKLS0tIGEvVG9vbHMvU2NyaXB0cy9uZXctcnVuLXdlYmtpdC10ZXN0cworKysgYi9U b29scy9TY3JpcHRzL25ldy1ydW4td2Via2l0LXRlc3RzCkBAIC0zMiw5ICszMiwyMiBAQCBpbXBv cnQgb3MKIGltcG9ydCBzaWduYWwKIGltcG9ydCBzdWJwcm9jZXNzCiBpbXBvcnQgc3lzCitpbXBv cnQgc3RyaW5nCiAKIGltcG9ydCB3ZWJraXRweS5jb21tb24udmVyc2lvbl9jaGVjawogCitkZWYg cGFyYW1ldGVyX3Rlc3RfbGlzdF9pc19pbmNvcnJlY3QoKToKKyAgICBmb3IgYXJnIGluIHN5cy5h cmd2WzE6XToKKyAgICAgICAgaWYgKHN0cmluZy5maW5kKGFyZywgJy0tdGVzdC1saXN0PScpID09 IDApOgorICAgICAgICAgICAgdGVzdGxpc3RfcGF0aCA9IHN0cmluZy5sc3RyaXAoYXJnLCAnLS10 ZXN0LWxpc3Q9JykKKyAgICAgICAgICAgIGlmIChub3Qgb3MucGF0aC5leGlzdHModGVzdGxpc3Rf cGF0aCkpOgorICAgICAgICAgICAgICAgIHByaW50ICdUZXN0bGlzdCcsIHRlc3RsaXN0X3BhdGgs ICdjYW5ub3QgZm91bmQuXG4nCisgICAgICAgICAgICAgICAgcmV0dXJuIDEKKyAgICAgICAgICAg IGVsaWYgKG9zLnBhdGguZ2V0c2l6ZSh0ZXN0bGlzdF9wYXRoKSA9PSAwKToKKyAgICAgICAgICAg ICAgICBwcmludCAnVGVzdGxpc3QnLCB0ZXN0bGlzdF9wYXRoLCAnaXMgZW1wdHkuXG4nCisgICAg ICAgICAgICAgICAgcmV0dXJuIDEKKyAgICByZXR1cm4gMAorCiBpZiBfX25hbWVfXyA9PSAnX19t YWluX18nOgogICAgICMgSW4gb3JkZXIgZm9yIHRoZSBtdWx0aXByb2Nlc3NpbmcgbW9kdWxlIHRv IHNwYXduIGNoaWxkcmVuIGNvcnJlY3RseSBvbgogICAgICMgV2luZG93cywgd2UgbmVlZCB0byBi ZSBydW5uaW5nIGEgUHl0aG9uIG1vZHVsZSB0aGF0IGNhbiBiZSBpbXBvcnRlZApAQCAtNTEsNiAr NjQsOCBAQCBpZiBfX25hbWVfXyA9PSAnX19tYWluX18nOgogICAgIGVsc2U6CiAgICAgICAgIGVu dlsnUFlUSE9OUEFUSCddID0gc2NyaXB0X2RpcgogICAgIG1vZHVsZV9wYXRoID0gb3MucGF0aC5q b2luKHNjcmlwdF9kaXIsICd3ZWJraXRweScsICdsYXlvdXRfdGVzdHMnLCAncnVuX3dlYmtpdF90 ZXN0cy5weScpCisgICAgaWYgKHBhcmFtZXRlcl90ZXN0X2xpc3RfaXNfaW5jb3JyZWN0KCkpOgor ICAgICAgICBzeXMuZXhpdCgwKQogICAgIGNtZCA9IFtzeXMuZXhlY3V0YWJsZSwgbW9kdWxlX3Bh dGhdICsgc3lzLmFyZ3ZbMTpdCiAgICAgcHJvYyA9IHN1YnByb2Nlc3MuUG9wZW4oY21kLCBlbnY9 ZW52KQogICAgIHRyeToK