Parse jsc_results.json for JSC tests. These results (from multiple jsc test runs) will help in determining whether the failures are introduced by the patch, or are pre-existing.
Created attachment 383323 [details] Patch
Sample runs: https://ews-build.webkit-uat.org/#/builders/21/builds/408 https://ews-build.webkit-uat.org/#/builders/21/builds/406
Comment on attachment 383323 [details] Patch Attachment 383323 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/13241808 New failing tests: imported/blink/fast/events/panScroll-crash.html
Created attachment 383339 [details] Archive of layout-test-results from ews213 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews213 Port: win-future Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Created attachment 383360 [details] Patch
- Added more unit-tests. - TestReRunJavaScriptCoreTests now runs all the unit-tests from TestRunJavaScriptCoreTests (by changing self.prefix accordingly). - Updated platform/fullPlatform for few JSC unit-tests, follow-up from https://trac.webkit.org/changeset/250102/webkit , also added new unit-test to cover jsc-only scenario. More sample runs: https://ews-build.webkit-uat.org/#/builders/22/builds/240 https://ews-build.webkit-uat.org/#/builders/21/builds/409 Note that build-step to Analyze all these failures would be added separately (in subsequent patch).
Comment on attachment 383360 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383360&action=review > Tools/BuildSlaveSupport/ews-build/steps.py:989 > + self.setProperty(self.prefix + 'binary_failures', self.binaryFailures) I'd like Zhifei to take a look at this too, the output format of run-javascriptcore-tests is something that confuses me... > Tools/BuildSlaveSupport/ews-build/steps_unittest.py:1033 > + command=['perl', 'Tools/Scripts/run-javascriptcore-tests', '--no-build', '--no-fail-fast', '--json-output={0}'.format(self.jsonFileName), '--release', '--remote-config-file=remote-machines.json', '--jsc-only'], Why the change here?
Comment on attachment 383360 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=383360&action=review >> Tools/BuildSlaveSupport/ews-build/steps_unittest.py:1033 >> + command=['perl', 'Tools/Scripts/run-javascriptcore-tests', '--no-build', '--no-fail-fast', '--json-output={0}'.format(self.jsonFileName), '--release', '--remote-config-file=remote-machines.json', '--jsc-only'], > > Why the change here? We didn't really had coverage for 'jsc-only' platform in unit-tests earlier. This test-case now covers 'jsc-only' platform. I set the 'platform' property to 'jsc-only', and the code (as expected) passes '--jsc-only' parameter to run-javascriptcore-tests.
Separated unit-tests changes related to https://trac.webkit.org/r250102 in Bug 204123.
Created attachment 383390 [details] Patch after rebasing on ToT
Comment on attachment 383390 [details] Patch after rebasing on ToT View in context: https://bugs.webkit.org/attachment.cgi?id=383390&action=review I think blocking this on sorting out the right thing to do with https://bugs.webkit.org/show_bug.cgi?id=204091 is a bad idea, especially since we already have infrastructure that relies on the json file you're using here. I do think that we'll be changing this in the near future, though. Maybe a FIXME in commandComplete with a bug about standardizing how we think about JSC test failures is a good idea. I don't think the standardization is in scope of this problem or https://bugs.webkit.org/show_bug.cgi?id=204091, but it does need to happen. > Tools/BuildSlaveSupport/ews-build/steps.py:983 > + self.binaryFailures.append('testapi') I don't feel strongly about this, but we could reduce some duplication here: for test in ['masm', 'air', 'b3', 'dog', 'api']: if jsc_results.get('all{}TestsPassed'.format(test.capitalize())) == False: self.binaryFailures.append('test{}'.format(test))
Comment on attachment 383390 [details] Patch after rebasing on ToT View in context: https://bugs.webkit.org/attachment.cgi?id=383390&action=review >> Tools/BuildSlaveSupport/ews-build/steps.py:983 >> + self.binaryFailures.append('testapi') > > I don't feel strongly about this, but we could reduce some duplication here: > > for test in ['masm', 'air', 'b3', 'dog', 'api']: > if jsc_results.get('all{}TestsPassed'.format(test.capitalize())) == False: > self.binaryFailures.append('test{}'.format(test)) unfortunately that doesn't work for 'DFG'. 'DFG' is all caps, while 'Api', 'Masm' and 'Air' have only first character capitalized. I will keep this part of the patch as is for now.
Comment on attachment 383390 [details] Patch after rebasing on ToT Clearing flags on attachment: 383390 Committed r252430: <https://trac.webkit.org/changeset/252430>
All reviewed patches have been landed. Closing bug.
<rdar://problem/57166574>