190537 2018-10-12 14:27:39 -0700 [Tools][webkitpy] fix handling of JSCTESTS_OPTIONS 2018-10-12 16:28:26 -0700 1 1 1 Unclassified WebKit Tools / Tests WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 guijemont+jsc-armv7-ews webkit-unassigned ap commit-queue ews-watchlist glenn guijemont lforschler mcatanzaro webkit-bug-importer oldest_to_newest 1468576 0 guijemont+jsc-armv7-ews 2018-10-12 14:27:39 -0700 In DeprecatedPort.run_javascriptcore_tests_command() JSCTESTS_OPTIONS needs to be split before adding it to the command since it is treated as a list of args. 1468590 1 352204 guijemont 2018-10-12 14:31:52 -0700 Created attachment 352204 Patch The patch. 1468602 2 guijemont+jsc-armv7-ews 2018-10-12 14:40:27 -0700 (In reply to Guillaume Emont from comment #1) > Created attachment 352204 [details] > Patch > > The patch. I think (hope) the jsc-armv7 EWS should get green if this works. 1468617 3 guijemont+jsc-armv7-ews 2018-10-12 14:59:15 -0700 (In reply to Guillaume Emont (jsc-armv7-ews) from comment #2) > (In reply to Guillaume Emont from comment #1) > > Created attachment 352204 [details] > > Patch > > > > The patch. > > I think (hope) the jsc-armv7 EWS should get green if this works. ...Except webkitpy changes are skipped. 1468629 4 352204 ap 2018-10-12 15:09:20 -0700 Comment on attachment 352204 Patch The patch looks good. I’m not so sure about the idea of passing options via environment. The port object is supposed to implement the knowledge of how to run tools, and passing arguments verbatim violates incapsulation. Why was this approach chosen? 1468638 5 guijemont+jsc-armv7-ews 2018-10-12 15:26:59 -0700 (In reply to Alexey Proskuryakov from comment #4) > Comment on attachment 352204 [details] > Patch > > The patch looks good. > > I’m not so sure about the idea of passing options via environment. The port > object is supposed to implement the knowledge of how to run tools, and > passing arguments verbatim violates incapsulation. > > Why was this approach chosen? The goal is to be able to pass a --remote-config-file option so that run-javascriptcore-tests runs the test on the correct devices, which are dependent on the machine where it is run and not on the port. 1468639 6 guijemont+jsc-armv7-ews 2018-10-12 15:28:34 -0700 (In reply to Guillaume Emont (jsc-armv7-ews) from comment #5) > (In reply to Alexey Proskuryakov from comment #4) > > Comment on attachment 352204 [details] > > Patch > > > > The patch looks good. > > > > I’m not so sure about the idea of passing options via environment. The port > > object is supposed to implement the knowledge of how to run tools, and > > passing arguments verbatim violates incapsulation. > > > > Why was this approach chosen? > > The goal is to be able to pass a --remote-config-file option so that > run-javascriptcore-tests runs the test on the correct devices, which are > dependent on the machine where it is run and not on the port. Also, I'd love if that could be a parameter to webkit-patch <name of ews queue>, but I could not find a simple way to do that, and saw that there was a precedent with MAKEFLAGS. 1468697 7 352204 commit-queue 2018-10-12 16:27:55 -0700 Comment on attachment 352204 Patch Clearing flags on attachment: 352204 Committed r237089: <https://trac.webkit.org/changeset/237089> 1468698 8 commit-queue 2018-10-12 16:27:57 -0700 All reviewed patches have been landed. Closing bug. 1468699 9 webkit-bug-importer 2018-10-12 16:28:26 -0700 <rdar://problem/45241227> 352204 2018-10-12 14:31:52 -0700 2018-10-12 16:27:55 -0700 Patch bug-190537-20181012233151.patch text/plain 1640 guijemont U3VidmVyc2lvbiBSZXZpc2lvbjogMjM3MDc0CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggYzM5MzU0NmI4MDVmNWExZTJkMjNhOWRkZDE1MzIzNDEz ZjMzM2ZjOC4uN2NjNWU3NjUwZmFiZWMyZDc0NTMyNDI2OWE0ZTA0ZDA4MTIzZmE5MCAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3 IEBACisyMDE4LTEwLTEyICBHdWlsbGF1bWUgRW1vbnQgIDxndWlqZW1vbnRAaWdhbGlhLmNvbT4K KworICAgICAgICBbVG9vbHNdW3dlYmtpdHB5XSBmaXggaGFuZGxpbmcgb2YgSlNDVEVTVFNfT1BU SU9OUworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTkw NTM3CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgSW4g RGVwcmVjYXRlZFBvcnQucnVuX2phdmFzY3JpcHRjb3JlX3Rlc3RzX2NvbW1hbmQoKSBKU0NURVNU U19PUFRJT05TCisgICAgICAgIG5lZWRzIHRvIGJlIHNwbGl0IGJlZm9yZSBhZGRpbmcgaXQgdG8g dGhlIGNvbW1hbmQgc2luY2UgaXQgaXMgdHJlYXRlZAorICAgICAgICBhcyBhIGxpc3Qgb2YgYXJn cy4KKworICAgICAgICAqIFNjcmlwdHMvd2Via2l0cHkvY29tbW9uL2NvbmZpZy9wb3J0cy5weToK KyAgICAgICAgKERlcHJlY2F0ZWRQb3J0LnJ1bl9qYXZhc2NyaXB0Y29yZV90ZXN0c19jb21tYW5k KToKKwogMjAxOC0xMC0xMSAgWW91ZW5uIEZhYmxldCAgPHlvdWVubkBhcHBsZS5jb20+CiAKICAg ICAgICAgSU9TIDEyIC0gU2VydmljZSB3b3JrZXIgY2FjaGUgbm90IHNoYXJlZCB3aGVuIGFkZGVk IHRvIGhvbWVzY3JlZW4KZGlmZiAtLWdpdCBhL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvY29tbW9u L2NvbmZpZy9wb3J0cy5weSBiL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvY29tbW9uL2NvbmZpZy9w b3J0cy5weQppbmRleCAzYjNlYzIzYTQyYjM5MDI0YzcxYmVmODA2NTkxY2JlMjY1MTVkMTgwLi5k N2E3OGQ4YTUzMDRkOGMxYTdjOGE0Y2MzZjNkMjZmMjc1YzYxYjQ1IDEwMDY0NAotLS0gYS9Ub29s cy9TY3JpcHRzL3dlYmtpdHB5L2NvbW1vbi9jb25maWcvcG9ydHMucHkKKysrIGIvVG9vbHMvU2Ny aXB0cy93ZWJraXRweS9jb21tb24vY29uZmlnL3BvcnRzLnB5CkBAIC0xMTcsNyArMTE3LDcgQEAg Y2xhc3MgRGVwcmVjYXRlZFBvcnQob2JqZWN0KToKICAgICAgICAgY29tbWFuZCA9IHNlbGYuc2Ny aXB0X3NoZWxsX2NvbW1hbmQoInJ1bi1qYXZhc2NyaXB0Y29yZS10ZXN0cyIpCiAgICAgICAgIGNv bW1hbmQuYXBwZW5kKCItLW5vLWZhaWwtZmFzdCIpCiAgICAgICAgIGlmICdKU0NURVNUU19PUFRJ T05TJyBpbiBvcy5lbnZpcm9uOgotICAgICAgICAgICAgY29tbWFuZC5hcHBlbmQob3MuZW52aXJv blsnSlNDVEVTVFNfT1BUSU9OUyddKQorICAgICAgICAgICAgY29tbWFuZCArPSBvcy5lbnZpcm9u WydKU0NURVNUU19PUFRJT05TJ10uc3BsaXQoKQogICAgICAgICByZXR1cm4gc2VsZi5fYXBwZW5k X2J1aWxkX3N0eWxlX2ZsYWcoY29tbWFuZCwgYnVpbGRfc3R5bGUpCiAKICAgICBkZWYgcnVuX3dl YmtpdF90ZXN0c19jb21tYW5kKHNlbGYsIGJ1aWxkX3N0eWxlPU5vbmUpOgo=