Bug 191974

Summary: webkitpy: Add watchOS ports
Product: WebKit Reporter: Jonathan Bedard <jbedard>
Component: Tools / TestsAssignee: Jonathan Bedard <jbedard>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, commit-queue, ddkilzer, ews-watchlist, glenn, lforschler, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Description Jonathan Bedard 2018-11-26 11:33:57 PST
Add ports for running layout tests on watchOS.
Comment 1 Radar WebKit Bug Importer 2018-11-26 11:34:41 PST
<rdar://problem/46251051>
Comment 2 Jonathan Bedard 2018-11-26 11:36:44 PST
Created attachment 355660 [details]
Patch
Comment 3 Jonathan Bedard 2018-11-26 13:09:31 PST
Created attachment 355668 [details]
Patch
Comment 4 Lucas Forschler 2018-11-27 13:48:30 PST
Comment on attachment 355668 [details]
Patch

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

> Tools/Scripts/webkitpy/port/watch_device.py:42
> +    NO_ON_DEVICE_TESTING = 'On-device testing is not supported on this machine'

is 'machine' the right noun here?  How about: configuration | device | platform | scenario ?

> Tools/Scripts/webkitpy/port/watch_simulator.py:39
> +    DEFAULT_ARCHITECTURE = 'x86'

I am not certain of watchOS conventions, but other places we use i386, or x86_64.

> Tools/Scripts/webkitpy/port/watch_simulator.py:116
> +        _log.error('No Simulated device matching "{}" found in watchOS SDK'.format(str(target_device_type)))

nit: lower case simulated.

> Tools/Scripts/webkitpy/port/watch_simulator.py:119
> +    def setup_environ_for_server(self, server_name=None):

I think this should be setup_env_for_server, or fully spelled out as setup_environment_for_server
Comment 5 Aakash Jain 2018-11-27 14:06:17 PST
Comment on attachment 355668 [details]
Patch

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

> Tools/Scripts/webkitpy/port/watch.py:23
> +

Nit: Do we need 2 or 1 empty lines here?

> Tools/Scripts/webkitpy/port/watch.py:103
> +        # The implementation of this funciton differs between on-device and simulator testing.

typo: funciton => function
Comment 6 Jonathan Bedard 2018-11-27 14:10:28 PST
Comment on attachment 355668 [details]
Patch

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

>> Tools/Scripts/webkitpy/port/watch_simulator.py:39
>> +    DEFAULT_ARCHITECTURE = 'x86'
> 
> I am not certain of watchOS conventions, but other places we use i386, or x86_64.

Testing this now, pretty sure you're right and i386 is required.

>> Tools/Scripts/webkitpy/port/watch_simulator.py:119
>> +    def setup_environ_for_server(self, server_name=None):
> 
> I think this should be setup_env_for_server, or fully spelled out as setup_environment_for_server

This function is coming from a base class, so we shouldn't change it's name here.
Comment 7 Jonathan Bedard 2018-11-27 15:52:00 PST
Created attachment 355800 [details]
Patch for landing
Comment 8 WebKit Commit Bot 2018-11-27 16:29:49 PST
Comment on attachment 355800 [details]
Patch for landing

Clearing flags on attachment: 355800

Committed r238590: <https://trac.webkit.org/changeset/238590>
Comment 9 WebKit Commit Bot 2018-11-27 16:29:51 PST
All reviewed patches have been landed.  Closing bug.