Bug 202170

Summary: [EWS] JSC queues should dynamically add required build steps for re-testing the patch
Product: WebKit Reporter: Aakash Jain <aakash_jain>
Component: Tools / TestsAssignee: Aakash Jain <aakash_jain>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, ap, commit-queue, jbedard, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=201999
Attachments:
Description Flags
Patch none

Aakash Jain
Reported 2019-09-24 14:02:21 PDT
JSC queues should dynamically add required build steps for re-testing the patch, just like rest other queues (like API, webkitpy, layout tests) does.
Attachments
Patch (11.19 KB, patch)
2019-09-24 14:32 PDT, Aakash Jain 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Aakash Jain
Comment 1 2019-09-24 14:32:59 PDT
Created attachment 379493 [details] Patch
Aakash Jain
Comment 2 2019-09-24 14:42:53 PDT
Sample runs: Success: https://ews-build.webkit-uat.org/#/builders/17/builds/4056 Failures in JSC-tests: https://ews-build.webkit-uat.org/#/builders/17/builds/4105 Failures in both JSC-tests and Rerun: https://ews-build.webkit-uat.org/#/builders/17/builds/4099
Jonathan Bedard
Comment 3 2019-09-25 09:55:01 PDT
Comment on attachment 379493 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=379493&action=review > Tools/BuildSlaveSupport/ews-build/steps.py:839 > + rc = shell.Test.evaluateCommand(self, cmd) Any reason we aren't using super(...) here? > Tools/BuildSlaveSupport/ews-build/steps.py:854 > + rc = shell.Test.evaluateCommand(self, cmd) Again, why move away from super(...)?
Aakash Jain
Comment 4 2019-09-25 10:38:39 PDT
Comment on attachment 379493 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=379493&action=review >> Tools/BuildSlaveSupport/ews-build/steps.py:839 >> + rc = shell.Test.evaluateCommand(self, cmd) > > Any reason we aren't using super(...) here? To avoid the style warning: "ERROR: Tools/BuildSlaveSupport/ews-build/steps.py:838: [RunJavaScriptCoreTests.evaluateCommand] Use of super on an old style class [pylint/E1002] [5]" >> Tools/BuildSlaveSupport/ews-build/steps.py:854 >> + rc = shell.Test.evaluateCommand(self, cmd) > > Again, why move away from super(...)? In this case we can't use super(ReRunJavaScriptCoreTests) since this method in super-class (which is RunJavaScriptCoreTests) dynamically adds different build step. We explicitly want to use evaluateCommand() from shell.Test. This is similar to what we do in other similar step like: ReRunAPITests(). We can explicitly use super(RunJavaScriptCoreTests), but that would be confusing.
Jonathan Bedard
Comment 5 2019-09-25 10:42:34 PDT
Comment on attachment 379493 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=379493&action=review >>> Tools/BuildSlaveSupport/ews-build/steps.py:839 >>> + rc = shell.Test.evaluateCommand(self, cmd) >> >> Any reason we aren't using super(...) here? > > To avoid the style warning: "ERROR: Tools/BuildSlaveSupport/ews-build/steps.py:838: [RunJavaScriptCoreTests.evaluateCommand] Use of super on an old style class [pylint/E1002] [5]" Ah, shell.Test is an old style class.
WebKit Commit Bot
Comment 6 2019-09-25 11:35:19 PDT
Comment on attachment 379493 [details] Patch Clearing flags on attachment: 379493 Committed r250350: <https://trac.webkit.org/changeset/250350>
WebKit Commit Bot
Comment 7 2019-09-25 11:35:21 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 8 2019-09-25 11:36:19 PDT
<rdar://problem/55712225>
Note You need to log in before you can comment on or make changes to this bug.