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.
Created attachment 383921 [details] Patch
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.
(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.
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. That's ok either way. But why is this print inside if reportData?
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. > > 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'
Comment on attachment 383921 [details] Patch Clearing flags on attachment: 383921 Committed r252781: <https://trac.webkit.org/changeset/252781>
All reviewed patches have been landed. Closing bug.
<rdar://problem/57432918>