Bug 191974 - webkitpy: Add watchOS ports
Summary: webkitpy: Add watchOS ports
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jonathan Bedard
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-11-26 11:33 PST by Jonathan Bedard
Modified: 2018-11-27 16:29 PST (History)
7 users (show)

See Also:


Attachments
Patch (24.78 KB, patch)
2018-11-26 11:36 PST, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (24.91 KB, patch)
2018-11-26 13:09 PST, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch for landing (25.01 KB, patch)
2018-11-27 15:52 PST, Jonathan Bedard
no flags Details | Formatted Diff | Diff

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