Bug 188498 - [ews-build] Add build step to run WK1 layout-test
Summary: [ews-build] Add build step to run WK1 layout-test
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aakash Jain
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-08-12 22:50 PDT by Aakash Jain
Modified: 2018-08-14 13:14 PDT (History)
6 users (show)

See Also:


Attachments
Proposed patch (3.64 KB, patch)
2018-08-12 22:55 PDT, Aakash Jain
lforschler: review+
Details | Formatted Diff | Diff
Patch for landing (3.58 KB, patch)
2018-08-14 12:34 PDT, Aakash Jain
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>