204386 2019-11-19 16:42:47 -0800 run-javascriptcore-tests: Hide the output of binaries behind --verbose 2019-11-22 10:27:19 -0800 1 1 1 Unclassified WebKit Tools / Tests WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=204091 InRadar P2 Normal --- 1 jbedard jbedard aakash_jain commit-queue jbedard webkit-bug-importer oldest_to_newest 1591763 0 jbedard 2019-11-19 16:42:47 -0800 Aakash pointed out that we might have way too many logs coming out of run-javascriptcore-tests by default. We should gate some of these logs on the --verbose flag. 1591766 1 383921 jbedard 2019-11-19 16:47:26 -0800 Created attachment 383921 Patch 1592396 2 383921 aakash_jain 2019-11-21 09:22:36 -0800 Comment on attachment 383921 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383921&action=review > Tools/Scripts/run-javascriptcore-tests:585 > + print "$subTestName failed\n" if ($failureFlag && !$verbose); Shouldn't we print the failures irrespective of verbose flag. Hiding failure might make it difficult to debug those failures. Especially important for EWS, where engineers can't change the flag. 1592400 3 jbedard 2019-11-21 09:35:04 -0800 (In reply to Aakash Jain from comment #2) > Comment on attachment 383921 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=383921&action=review > > > Tools/Scripts/run-javascriptcore-tests:585 > > + print "$subTestName failed\n" if ($failureFlag && !$verbose); > > Shouldn't we print the failures irrespective of verbose flag. Hiding failure > might make it difficult to debug those failures. Especially important for > EWS, where engineers can't change the flag. If we're verbose, we would have printed the error line already on line 522, I was thinking that we wouldn't want to duplicate the printing. I don't feel super strongly about it either way, though. 1592407 4 383921 aakash_jain 2019-11-21 09:40:58 -0800 Comment on attachment 383921 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383921&action=review >>> Tools/Scripts/run-javascriptcore-tests:585 >>> + print "$subTestName failed\n" if ($failureFlag && !$verbose); >> >> Shouldn't we print the failures irrespective of verbose flag. Hiding failure might make it difficult to debug those failures. Especially important for EWS, where engineers can't change the flag. > > If we're verbose, we would have printed the error line already on line 522, I was thinking that we wouldn't want to duplicate the printing. > > I don't feel super strongly about it either way, though. That's ok either way. But why is this print inside if reportData? 1592414 5 383921 jbedard 2019-11-21 09:49:58 -0800 Comment on attachment 383921 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383921&action=review >>>> Tools/Scripts/run-javascriptcore-tests:585 >>>> + print "$subTestName failed\n" if ($failureFlag && !$verbose); >>> >>> Shouldn't we print the failures irrespective of verbose flag. Hiding failure might make it difficult to debug those failures. Especially important for EWS, where engineers can't change the flag. >> >> If we're verbose, we would have printed the error line already on line 522, I was thinking that we wouldn't want to duplicate the printing. >> >> I don't feel super strongly about it either way, though. > > That's ok either way. > > But why is this print inside if reportData? This is handling the case where we somehow duplicate a test name when parsing binary output. This is possible because we don't have great standards for how JSC binary tests are organized. That line of code is basically saying 'If we don't already have a result for this test, then' 1592871 6 383921 commit-queue 2019-11-22 10:26:02 -0800 Comment on attachment 383921 Patch Clearing flags on attachment: 383921 Committed r252781: <https://trac.webkit.org/changeset/252781> 1592872 7 commit-queue 2019-11-22 10:26:03 -0800 All reviewed patches have been landed. Closing bug. 1592874 8 webkit-bug-importer 2019-11-22 10:27:19 -0800 <rdar://problem/57432918> 383921 2019-11-19 16:47:26 -0800 2019-11-22 10:26:02 -0800 Patch bug-204386-20191119164726.patch text/plain 1633 jbedard SW5kZXg6IFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBUb29scy9DaGFuZ2VMb2cJKHJl dmlzaW9uIDI1MjY1NykKKysrIFRvb2xzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwz ICsxLDEzIEBACisyMDE5LTExLTE5ICBKb25hdGhhbiBCZWRhcmQgIDxqYmVkYXJkQGFwcGxlLmNv bT4KKworICAgICAgICBydW4tamF2YXNjcmlwdGNvcmUtdGVzdHM6IEhpZGUgdGhlIG91dHB1dCBv ZiBiaW5hcmllcyBiZWhpbmQgLS12ZXJib3NlCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQu b3JnL3Nob3dfYnVnLmNnaT9pZD0yMDQzODYKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkg KE9PUFMhKS4KKworICAgICAgICAqIFNjcmlwdHMvcnVuLWphdmFzY3JpcHRjb3JlLXRlc3RzOgor ICAgICAgICAocnVuVGVzdCk6IERvIG5vdCBsb2cgYmluYXJ5IG91dHB1dCBieSBkZWZhdWx0LCBh bHdheXMgbG9nIHRlc3QgZmFpbHVyZXMuCisKIDIwMTktMTEtMTkgIEpvbmF0aGFuIEJlZGFyZCAg PGpiZWRhcmRAYXBwbGUuY29tPgogCiAgICAgICAgIHJlc3VsdHMud2Via2l0Lm9yZzogSGF2ZSBi dWlsZC53ZWJraXQub3JnIHJlcG9ydCBKU0MgdGVzdHMKSW5kZXg6IFRvb2xzL1NjcmlwdHMvcnVu LWphdmFzY3JpcHRjb3JlLXRlc3RzCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFRvb2xzL1NjcmlwdHMvcnVuLWph dmFzY3JpcHRjb3JlLXRlc3RzCShyZXZpc2lvbiAyNTI2NTIpCisrKyBUb29scy9TY3JpcHRzL3J1 bi1qYXZhc2NyaXB0Y29yZS10ZXN0cwkod29ya2luZyBjb3B5KQpAQCAtNTE5LDcgKzUxOSw3IEBA IHN1YiBydW5UZXN0IHsKICAgICBvcGVuKFRFU1QsICItfCIsICJAY29tbWFuZCAyPiYxIikgb3Ig ZGllICJGYWlsZWQgdG8gcnVuIEBjb21tYW5kIjsKICAgICB3aGlsZSAoIG15ICRsaW5lID0gPFRF U1Q+ICkgewogICAgICAgICBteSAkc3ViVGVzdE5hbWU7Ci0gICAgICAgIHByaW50ICRsaW5lOwor ICAgICAgICBwcmludCAkbGluZSBpZiAoJHZlcmJvc2UpOwogCiAgICAgICAgICMgZS5nLjogJ08y OiB0ZXN0Um90UldpdGhJbW1TaGlmdCh1aW50NjQtbWF4LCB1aW50MzItbWluKTogT0shJycKICAg ICAgICAgaWYgKCRsaW5lID1+IC8oT1xkKTogKFxTKykoXCguKlwpKTogKFtBLVphLXpdKykhXG4v KSB7CkBAIC01ODIsNyArNTgyLDcgQEAgc3ViIHJ1blRlc3QgewogICAgICAgICB9CiAKICAgICAg ICAgaWYgKCFkZWZpbmVkICRyZXBvcnREYXRheyRzdWJUZXN0TmFtZX0pIHsKLSAgICAgICAgICAg IHByaW50KCIkc3ViVGVzdE5hbWUgJGZhaWx1cmVGbGFnXG4iKSBpZiAkZmFpbHVyZUZsYWc7Cisg ICAgICAgICAgICBwcmludCAiJHN1YlRlc3ROYW1lIGZhaWxlZFxuIiBpZiAoJGZhaWx1cmVGbGFn ICYmICEkdmVyYm9zZSk7CiAgICAgICAgICAgICAkcmVwb3J0RGF0YXskc3ViVGVzdE5hbWV9ID0g JGZhaWx1cmVGbGFnID8ge2FjdHVhbCA9PiAiRkFJTCJ9IDoge2FjdHVhbCA9PiAiUEFTUyJ9Owog ICAgICAgICB9CiAgICAgICAgICRmYWlsdXJlRmxhZyA9IDA7Cg==