[webkit-reviews] review granted: [Bug 188498] [ews-build] Add build step to run WK1 layout-test : [Attachment 346995] Proposed patch

[bugzilla-daemon at webkit.org]

Lucas Forschler <lforschler at apple.com> has granted Aakash Jain
<aakash_jain at apple.com>'s request for review:
Bug 188498: [ews-build] Add build step to run WK1 layout-test
https://bugs.webkit.org/show_bug.cgi?id=188498

Attachment 346995: Proposed patch

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




--- Comment #3 from Lucas Forschler <lforschler at apple.com> ---
Comment on attachment 346995
  --> https://bugs.webkit.org/attachment.cgi?id=346995
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.