187872 2018-07-20 15:00:33 -0700 test-webkitpy should take configuration command line options 2018-07-23 11:41:25 -0700 1 1 1 Unclassified WebKit Tools / Tests Safari Technology Preview Mac macOS 10.13 RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=187916 InRadar P2 Normal --- 187776 1 dbates dbates ap ddkilzer dino ews-watchlist glenn lforschler simon.fraser webkit-bug-importer oldest_to_newest 1443931 0 dbates 2018-07-20 15:00:33 -0700 Today I noticed that the Apple Sierra Debug WK2 Tests bot tries to build lldbWebKitTester for Release even though it is a debug tester and should have used the debug version of lldbWebKitTester that it downloaded: [[ Failed to find latest selenium, falling back to existing 3.9.0 version Building lldbWebKitTester ... Failed to run "['/Volumes/Data/slave/sierra-debug-tests-wk2/build/Tools/Scripts/build-lldbwebkittester', '--release']" exit_code: 65 ... ]] <https://build.webkit.org/builders/Apple%20Sierra%20Debug%20WK2%20%28Tests%29/builds/7313/steps/webkitpy-test/logs/stdio> 1443933 1 dbates 2018-07-20 15:09:59 -0700 Once we fix this bug then we can land the tests included in the patch for bug #187776. 1443934 2 dbates 2018-07-20 15:13:59 -0700 One way to fix this is to take a similar approach as our other tools and teach test-webkitpy to take an optional --release/--debug command line argument so that it knows whether it must build lldbWebKitTester for release or debug. 1444107 3 345532 dbates 2018-07-21 20:18:48 -0700 Created attachment 345532 Patch 1444110 4 dbates 2018-07-21 20:21:03 -0700 Once we fix this bug we should look to land the tests in attachment #345523 (bug #187884). 1444111 5 345532 dbates 2018-07-21 20:22:08 -0700 Comment on attachment 345532 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=345532&action=review > Tools/Scripts/webkitpy/test/main.py:143 > + configuration_group.add_option('--debug', action='store_const', const='Debug', dest="configuration", Will change " => ' before landing. > Tools/Scripts/webkitpy/test/main.py:145 > + configuration_group.add_option('--release', action='store_const', const='Release', dest="configuration", Ditto. 1444127 6 ap 2018-07-22 00:12:59 -0700 The growing complexity of this approach makes me wonder if test-webkitpy is even the right place to test the formatters. It's Python code for sure, but the tests are clearly not fitting here. 1444172 7 dbates 2018-07-22 15:19:34 -0700 (In reply to Alexey Proskuryakov from comment #6) > The growing complexity of this approach makes me wonder if test-webkitpy is > even the right place to test the formatters. It's Python code for sure, but > the tests are clearly not fitting here. I am open to suggestions on how to reduce the complexity. Would it help reduce complexity to separate out the running of the lldbwebkit tests into its own test runner, say test-lldbwebkit? 1444243 8 345532 ddkilzer 2018-07-23 05:41:31 -0700 Comment on attachment 345532 Patch r=me 1444291 9 345532 dbates 2018-07-23 10:02:41 -0700 Comment on attachment 345532 Patch Clearing flags on attachment: 345532 Committed r234099: <https://trac.webkit.org/changeset/234099> 1444292 10 dbates 2018-07-23 10:02:42 -0700 All reviewed patches have been landed. Closing bug. 1444293 11 webkit-bug-importer 2018-07-23 10:03:26 -0700 <rdar://problem/42503722> 1444299 12 ap 2018-07-23 10:16:27 -0700 > Would it help reduce complexity to separate out the running of the lldbwebkit tests into its own test runner, say test-lldbwebkit? That's a good idea. Since one needs to run these tests for any WebCore change and not just for webkitpy changes, it's not really a webkitpy test in the first place. 1444339 13 dbates 2018-07-23 11:40:59 -0700 (In reply to Alexey Proskuryakov from comment #12) > > Would it help reduce complexity to separate out the running of the lldbwebkit tests into its own test runner, say test-lldbwebkit? > > That's a good idea. Filed bug #187916 for this. > Since one needs to run these tests for any WebCore > change and not just for webkitpy changes, it's not really a webkitpy test in > the first place. For completeness, test-webkitpy is already a misnomer because it runs unit tests for the WebKit message generator code and QueueStatusServer unit tests (when the AppEngine SDK is installed) in addition to running tests for webkitpy. 345532 2018-07-21 20:18:48 -0700 2018-07-23 10:02:41 -0700 Patch bug-187872-20180721201847.patch text/plain 5103 dbates U3VidmVyc2lvbiBSZXZpc2lvbjogMjM0MDE5CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggZjEwY2JhMTZjYjA0M2NhMjFhMDgwZWRlNTk3YTg4YWI3 NmUxZDAzOS4uNjRlYTYwZTNiYzBkZTI2YWJmYjJlMjFmNzQ0Y2NiZDk0MzlkMDYwZSAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI2 IEBACisyMDE4LTA3LTIxICBEYW5pZWwgQmF0ZXMgIDxkYWJhdGVzQGFwcGxlLmNvbT4KKworICAg ICAgICB0ZXN0LXdlYmtpdHB5IHNob3VsZCB0YWtlIGNvbmZpZ3VyYXRpb24gY29tbWFuZCBsaW5l IG9wdGlvbnMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTE4Nzg3MgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IEFkZCAtLWRlYnVnIGFuZCAtLXJlbGVhc2Ugb3B0aW9uYWwgY29tbWFuZCBsaW5lIG9wdGlvbnMg YW5kIHRlYWNoIHRoZSB0ZXN0IGJvdHMgdG8gaW52b2tlIHRlc3Qtd2Via2l0cHkKKyAgICAgICAg d2l0aCB0aGUgYXBwcm9wcmlhdGUgY29uZmlndXJhdGlvbi4gQXQgdGhlIG1vbWVudCB0aGVzZSBv cHRpb25zIGFyZSBvbmx5IG1lYW5pbmdmdWwgb24gTWFjIHNpbmNlIHRoZXkKKyAgICAgICAgZWZm ZWN0IHRoZSBjb25maWd1cmF0aW9uIGxsZGJXZWJLaXRUZXN0ZXIgaXMgYnVpbHQgd2l0aCBhbmQg d2Ugb25seSBidWlsZCBsbGRiV2ViS2l0VGVzdGVyIG9uIE1hYy4KKworICAgICAgICAqIEJ1aWxk U2xhdmVTdXBwb3J0L2J1aWxkLndlYmtpdC5vcmctY29uZmlnL3N0ZXBzLnB5OgorICAgICAgICAo UnVuUHl0aG9uVGVzdHMpOiBQYXNzIHRoZSBjb25maWd1cmF0aW9uIHRvIHRlc3Qtd2Via2l0cHku IE9uIE1hYyB0aGlzIHdpbGwgZWZmZWN0CisgICAgICAgIGhvdyBsbGRiV2ViS2l0VGVzdGVyIGlz IGJ1aWx0LiBBbGwgb3RoZXIgcG9ydHMgZG8gbm90IG1ha2UgdXNlIG9mIHRoZSBzcGVjaWZpZWQg Y29uZmlndXJhdGlvbi4KKyAgICAgICAgKiBTY3JpcHRzL3dlYmtpdHB5L3Rlc3QvbWFpbi5weToK KyAgICAgICAgKF9idWlsZF9sbGRiX3dlYmtpdF90ZXN0ZXIpOiBNb2RpZmllZCB0byB0YWtlIHRo ZSBjb25maWd1cmF0aW9uIHRvIGJ1aWxkIGxsZGJXZWJLaXRUZXN0ZXIuIElmCisgICAgICAgIHRo ZSBzcGVjaWZpZWQgY29uZmlndXJhdGlvbiBpcyBOb25lIHRoZW4gd2UgZmFsbCBiYWNrIHRvIHVz aW5nIHRoZSBkZWZhdWx0IGNvbmZpZ3VyYXRpb24gKGFzIHNldAorICAgICAgICBieSBzY3JpcHQg c2V0LXdlYmtpdC1jb25maWd1cmF0aW9uKS4KKyAgICAgICAgKFRlc3Rlci5fcGFyc2VfYXJncyk6 IEFkZCBjb21tYW5kIGxpbmUgb3B0aW9ucyAtLWRlYnVnIGFuZCAtLXJlbGVhc2UgdG8gYnVpbGQg bGxkYldlYktpdFRlc3RlcgorICAgICAgICB3aXRoIGEgRGVidWcgY29uZmlndXJhdGlvbiBhbmQg YSBSZWxlYXNlIGNvbmZpZ3VyYXRpb24sIHJlc3BlY3RpdmVseQorICAgICAgICAoVGVzdGVyLl9y dW5fdGVzdHMpOiBQYXNzIHRoZSBjb25maWd1cmF0aW9uIHRvIGJ1aWxkIGxsZGJXZWJLaXRUZXN0 ZXIgdGhhdCB3ZSBwYXJzZWQgZnJvbQorICAgICAgICB0aGUgY29tbWFuZCBsaW5lIG9wdGlvbnMu IEl0IG1heSBiZSBOb25lIGlmIG5laXRoZXIgLS1kZWJ1ZyBub3IgLS1yZWxlYXNlIHdlcmUgcGFz c2VkLgorCiAyMDE4LTA3LTE5ICBMdWNhcyBGb3JzY2hsZXIgIDxsZm9yc2NobGVyQGFwcGxlLmNv bT4KIAogICAgICAgICBUZWFjaCB0aGUgQVdTIExhbWJkYSB0byB1c2UgdGhlIFttaW5pZmllZF0t cGxhdGZvcm1zIGRhdGFiYXNlCmRpZmYgLS1naXQgYS9Ub29scy9CdWlsZFNsYXZlU3VwcG9ydC9i dWlsZC53ZWJraXQub3JnLWNvbmZpZy9zdGVwcy5weSBiL1Rvb2xzL0J1aWxkU2xhdmVTdXBwb3J0 L2J1aWxkLndlYmtpdC5vcmctY29uZmlnL3N0ZXBzLnB5CmluZGV4IGNhZDliOWJiNTE5MWEyYTNm MmQ3M2IzOTAzN2ZiYjY2NmE4ODFjNzYuLjU0YmIxZjAyODA3NDhiMWI1NWNjZjU3NDc5ZmI0MWU1 ZWJmNzQ2YzIgMTAwNjQ0Ci0tLSBhL1Rvb2xzL0J1aWxkU2xhdmVTdXBwb3J0L2J1aWxkLndlYmtp dC5vcmctY29uZmlnL3N0ZXBzLnB5CisrKyBiL1Rvb2xzL0J1aWxkU2xhdmVTdXBwb3J0L2J1aWxk LndlYmtpdC5vcmctY29uZmlnL3N0ZXBzLnB5CkBAIC01MjYsNyArNTI2LDcgQEAgY2xhc3MgUnVu UHl0aG9uVGVzdHMoVGVzdFdpdGhGYWlsdXJlQ291bnQpOgogICAgIG5hbWUgPSAid2Via2l0cHkt dGVzdCIKICAgICBkZXNjcmlwdGlvbiA9IFsicHl0aG9uLXRlc3RzIHJ1bm5pbmciXQogICAgIGRl c2NyaXB0aW9uRG9uZSA9IFsicHl0aG9uLXRlc3RzIl0KLSAgICBjb21tYW5kID0gWyJweXRob24i LCAiLi9Ub29scy9TY3JpcHRzL3Rlc3Qtd2Via2l0cHkiLCAiLS12ZXJib3NlIl0KKyAgICBjb21t YW5kID0gWyJweXRob24iLCAiLi9Ub29scy9TY3JpcHRzL3Rlc3Qtd2Via2l0cHkiLCAiLS12ZXJi b3NlIiwgV2l0aFByb3BlcnRpZXMoIi0tJShjb25maWd1cmF0aW9uKXMiKV0KICAgICBmYWlsZWRU ZXN0c0Zvcm1hdFN0cmluZyA9ICIlZCBweXRob24gdGVzdCVzIGZhaWxlZCIKIAogICAgIGRlZiBz dGFydChzZWxmKToKZGlmZiAtLWdpdCBhL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvdGVzdC9tYWlu LnB5IGIvVG9vbHMvU2NyaXB0cy93ZWJraXRweS90ZXN0L21haW4ucHkKaW5kZXggZWRiOTA5YTA0 YjAwNzhlZWVkMmRiZjRlZjFlZDE4Y2NiZjk1Y2ZmZi4uNmE0YmI1ZTM0MmFiZjU5NTEzN2U4OTZh YjdjNDg2NWJhN2FmNzQ1YSAxMDA2NDQKLS0tIGEvVG9vbHMvU2NyaXB0cy93ZWJraXRweS90ZXN0 L21haW4ucHkKKysrIGIvVG9vbHMvU2NyaXB0cy93ZWJraXRweS90ZXN0L21haW4ucHkKQEAgLTUy LDE0ICs1MiwxNCBAQCBfaG9zdCA9IFN5c3RlbUhvc3QoKQogX3dlYmtpdF9yb290ID0gTm9uZQog CiAKLWRlZiBfYnVpbGRfbGxkYl93ZWJraXRfdGVzdGVyKCk6CitkZWYgX2J1aWxkX2xsZGJfd2Vi a2l0X3Rlc3Rlcihjb25maWd1cmF0aW9uKToKICAgICBpZiBub3QgX2hvc3QucGxhdGZvcm0uaXNf bWFjKCk6CiAgICAgICAgIF9sb2cuZXJyb3IoJ2xsZGJXZWJLaXRUZXN0ZXIgaXMgbm90IHN1cHBv cnRlZCBvbiB0aGlzIHBsYXRmb3JtLicpCiAgICAgICAgIHJldHVybiBGYWxzZQogICAgIGNvbmZp ZyA9IENvbmZpZyhfaG9zdC5leGVjdXRpdmUsIF9ob3N0LmZpbGVzeXN0ZW0pCiAgICAgYnVpbGRf bGxkYndlYmtpdHRlc3RlciA9IG9zLnBhdGguam9pbihfd2Via2l0X3Jvb3QsICdUb29scycsICdT Y3JpcHRzJywgJ2J1aWxkLWxsZGJ3ZWJraXR0ZXN0ZXInKQogICAgIHRyeToKLSAgICAgICAgX2hv c3QuZXhlY3V0aXZlLnJ1bl9hbmRfdGhyb3dfaWZfZmFpbChbYnVpbGRfbGxkYndlYmtpdHRlc3Rl ciwgY29uZmlnLmZsYWdfZm9yX2NvbmZpZ3VyYXRpb24oY29uZmlnLmRlZmF1bHRfY29uZmlndXJh dGlvbigpKV0sIHF1aWV0PVRydWUpCisgICAgICAgIF9ob3N0LmV4ZWN1dGl2ZS5ydW5fYW5kX3Ro cm93X2lmX2ZhaWwoW2J1aWxkX2xsZGJ3ZWJraXR0ZXN0ZXIsIGNvbmZpZy5mbGFnX2Zvcl9jb25m aWd1cmF0aW9uKGNvbmZpZ3VyYXRpb24gb3IgY29uZmlnLmRlZmF1bHRfY29uZmlndXJhdGlvbigp KV0sIHF1aWV0PVRydWUpCiAgICAgZXhjZXB0IFNjcmlwdEVycm9yIGFzIGU6CiAgICAgICAgIF9s b2cuZXJyb3IoZS5tZXNzYWdlX3dpdGhfb3V0cHV0KG91dHB1dF9saW1pdD1Ob25lKSkKICAgICAg ICAgcmV0dXJuIEZhbHNlCkBAIC0xMzcsNiArMTM3LDE1IEBAIGNsYXNzIFRlc3RlcihvYmplY3Qp OgogCiAgICAgZGVmIF9wYXJzZV9hcmdzKHNlbGYsIGFyZ3Y9Tm9uZSk6CiAgICAgICAgIHBhcnNl ciA9IG9wdHBhcnNlLk9wdGlvblBhcnNlcih1c2FnZT0ndXNhZ2U6ICVwcm9nIFtvcHRpb25zXSBb YXJncy4uLl0nKQorCisgICAgICAgICMgIENvbmZpZ3VyYXRpb24gb3B0aW9ucyBvbmx5IGVmZmVj dCB0aGUgYnVpbGRpbmcgb2YgbGxkYldlYktpdFRlc3Rlci4KKyAgICAgICAgY29uZmlndXJhdGlv bl9ncm91cCA9IG9wdHBhcnNlLk9wdGlvbkdyb3VwKHBhcnNlciwgJ0NvbmZpZ3VyYXRpb24gb3B0 aW9ucycpCisgICAgICAgIGNvbmZpZ3VyYXRpb25fZ3JvdXAuYWRkX29wdGlvbignLS1kZWJ1Zycs IGFjdGlvbj0nc3RvcmVfY29uc3QnLCBjb25zdD0nRGVidWcnLCBkZXN0PSJjb25maWd1cmF0aW9u IiwKKyAgICAgICAgICAgIGhlbHA9J1NldCB0aGUgY29uZmlndXJhdGlvbiB0byBEZWJ1ZycpCisg ICAgICAgIGNvbmZpZ3VyYXRpb25fZ3JvdXAuYWRkX29wdGlvbignLS1yZWxlYXNlJywgYWN0aW9u PSdzdG9yZV9jb25zdCcsIGNvbnN0PSdSZWxlYXNlJywgZGVzdD0iY29uZmlndXJhdGlvbiIsCisg ICAgICAgICAgICBoZWxwPSdTZXQgdGhlIGNvbmZpZ3VyYXRpb24gdG8gUmVsZWFzZScpCisgICAg ICAgIHBhcnNlci5hZGRfb3B0aW9uX2dyb3VwKGNvbmZpZ3VyYXRpb25fZ3JvdXApCisKICAgICAg ICAgcGFyc2VyLmFkZF9vcHRpb24oJy1hJywgJy0tYWxsJywgYWN0aW9uPSdzdG9yZV90cnVlJywg ZGVmYXVsdD1GYWxzZSwKICAgICAgICAgICAgICAgICAgICAgICAgICAgaGVscD0ncnVuIGFsbCB0 aGUgdGVzdHMnKQogICAgICAgICBwYXJzZXIuYWRkX29wdGlvbignLWMnLCAnLS1jb3ZlcmFnZScs IGFjdGlvbj0nc3RvcmVfdHJ1ZScsIGRlZmF1bHQ9RmFsc2UsCkBAIC0xODksNyArMTk4LDcgQEAg Y2xhc3MgVGVzdGVyKG9iamVjdCk6CiAKICAgICAgICAgaWYgd2lsbF9ydW5fbGxkYl93ZWJraXRf dGVzdHM6CiAgICAgICAgICAgICBzZWxmLnByaW50ZXIud3JpdGVfdXBkYXRlKCdCdWlsZGluZyBs bGRiV2ViS2l0VGVzdGVyIC4uLicpCi0gICAgICAgICAgICBpZiBub3QgX2J1aWxkX2xsZGJfd2Vi a2l0X3Rlc3RlcigpOgorICAgICAgICAgICAgaWYgbm90IF9idWlsZF9sbGRiX3dlYmtpdF90ZXN0 ZXIoY29uZmlndXJhdGlvbj1zZWxmLl9vcHRpb25zLmNvbmZpZ3VyYXRpb24pOgogICAgICAgICAg ICAgICAgIF9sb2cuZXJyb3IoJ0ZhaWxlZCB0byBidWlsZCBsbGRiV2ViS2l0VGVzdGVyLicpCiAg ICAgICAgICAgICAgICAgcmV0dXJuIEZhbHNlCiAK