Bug 188498 – [ews-build] Add build step to run WK1 layout-test

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

[Aakash Jain]

We should add support for running WK1 layout tests in OpenSource EWS Buildbot.

[Aakash Jain]

Created attachment 346995 [details]
Proposed patch

Sample run: http://ews-build.webkit-uat.org/#/builders/26/builds/13/steps/3/logs/stdio

[EWS Watchlist]

Attachment 346995 [details] did not pass style-queue:


ERROR: Tools/BuildSlaveSupport/ews-build/steps_unittest.py:777:  [TestRunWebKit1Tests.test_success] Passing unexpected keyword argument 'state_string' in function call  [pylint/E1123] [5]
ERROR: Tools/BuildSlaveSupport/ews-build/steps_unittest.py:777:  [TestRunWebKit1Tests.test_success] No value passed for parameter 'status_text' in function call  [pylint/E1120] [5]
ERROR: Tools/BuildSlaveSupport/ews-build/steps_unittest.py:791:  [TestRunWebKit1Tests.test_failure] Passing unexpected keyword argument 'state_string' in function call  [pylint/E1123] [5]
ERROR: Tools/BuildSlaveSupport/ews-build/steps_unittest.py:791:  [TestRunWebKit1Tests.test_failure] No value passed for parameter 'status_text' in function call  [pylint/E1120] [5]
Total errors found: 4 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Lucas Forschler]

Comment on attachment 346995 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=346995&action=review

I see we have style errors here... this seems to be common for buildbot code. I would rather us override the style check than continue to ignore them.

> Tools/BuildSlaveSupport/ews-build/steps_unittest.py:769
> +        self.setProperty('fullPlatform', 'ios-simulator')

Do we only run WK1 on simulator?

> Tools/BuildSlaveSupport/ews-build/steps_unittest.py:774
> +                        )

it looks like you add the "--dump-render-tree" option here, as well as in the start function of the RunWebKit1Tests class. is it needed both places?

> Tools/BuildSlaveSupport/ews-build/steps_unittest.py:787
> +                        )

ditto here.  I see this is for a test_success|failure function, but it wasn't clear to me if this was inherited.

[Aakash Jain]

Comment on attachment 346995 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=346995&action=review

>> Tools/BuildSlaveSupport/ews-build/steps_unittest.py:769
>> +        self.setProperty('fullPlatform', 'ios-simulator')
> 
> Do we only run WK1 on simulator?

Not really. I will change it.

>> Tools/BuildSlaveSupport/ews-build/steps_unittest.py:774
>> +                        )
> 
> it looks like you add the "--dump-render-tree" option here, as well as in the start function of the RunWebKit1Tests class. is it needed both places?

Yeah, one is in the code (steps.py), another one is in corresponding unit-test to match that.

[Aakash Jain]

(In reply to Lucas Forschler from comment #3)
> I see we have style errors here... this seems to be common for buildbot
> code. I would rather us override the style check than continue to ignore them.
Tracking the style checker issue in https://bugs.webkit.org/show_bug.cgi?id=188572

[Aakash Jain]

Created attachment 347103 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 347103 [details]
Patch for landing

Clearing flags on attachment: 347103

Committed r234861: <https://trac.webkit.org/changeset/234861>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/43300238>