Bug 188498

Summary: [ews-build] Add build step to run WK1 layout-test
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, ews-watchlist, lforschler, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=187674
Attachments:
Description Flags
Proposed patch
lforschler: review+
Patch for landing none

Description Aakash Jain 2018-08-12 22:50:44 PDT
We should add support for running WK1 layout tests in OpenSource EWS Buildbot.
Comment 1 Aakash Jain 2018-08-12 22:55:06 PDT
Created attachment 346995 [details]
Proposed patch

Sample run: http://ews-build.webkit-uat.org/#/builders/26/builds/13/steps/3/logs/stdio
Comment 2 EWS Watchlist 2018-08-12 22:57:46 PDT Comment hidden (obsolete)
Comment 3 Lucas Forschler 2018-08-14 09:10:43 PDT
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.
Comment 4 Aakash Jain 2018-08-14 09:45:21 PDT
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.
Comment 5 Aakash Jain 2018-08-14 12:16:08 PDT
(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
Comment 6 Aakash Jain 2018-08-14 12:34:34 PDT
Created attachment 347103 [details]
Patch for landing
Comment 7 WebKit Commit Bot 2018-08-14 13:13:46 PDT
Comment on attachment 347103 [details]
Patch for landing

Clearing flags on attachment: 347103

Committed r234861: <https://trac.webkit.org/changeset/234861>
Comment 8 WebKit Commit Bot 2018-08-14 13:13:48 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2018-08-14 13:14:22 PDT
<rdar://problem/43300238>