Bug 202170 – [EWS] JSC queues should dynamically add required build steps for re-testing the patch

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Aakash Jain]

JSC queues should dynamically add required build steps for re-testing the patch, just like rest other queues (like API, webkitpy, layout tests) does.

[Aakash Jain]

Created attachment 379493 [details]
Patch

[Aakash Jain]

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 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 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 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 on attachment 379493 [details]
Patch

Clearing flags on attachment: 379493

Committed r250350: <https://trac.webkit.org/changeset/250350>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/55712225>