Bug 119100 - [GTK] Add WestonDriver, use it when appropriate
Summary: [GTK] Add WestonDriver, use it when appropriate
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zan Dobersek
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-07-25 12:29 PDT by Zan Dobersek
Modified: 2013-07-29 13:08 PDT (History)
6 users (show)

See Also:


Attachments
Patch (10.76 KB, patch)
2013-07-25 12:57 PDT, Zan Dobersek
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 (577.16 KB, application/zip)
2013-07-26 16:57 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2013-07-25 12:29:36 PDT
[GTK] Add WestonDriver, use it when appropriate
Comment 1 Zan Dobersek 2013-07-25 12:57:27 PDT
Created attachment 207479 [details]
Patch
Comment 2 Zan Dobersek 2013-07-25 13:00:59 PDT
Comment on attachment 207479 [details]
Patch

Still needs a unit test. Reviews welcome, though.
Comment 3 Build Bot 2013-07-26 16:57:18 PDT
Comment on attachment 207479 [details]
Patch

Attachment 207479 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1241366

New failing tests:
http/tests/security/cross-origin-plugin-private-browsing-toggled.html
Comment 4 Build Bot 2013-07-26 16:57:20 PDT
Created attachment 207565 [details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-15  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.3
Comment 5 Martin Robinson 2013-07-29 09:32:47 PDT
Comment on attachment 207479 [details]
Patch

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

Looks good to me!

> Tools/Scripts/webkitpy/port/westondriver.py:64
> +        # Give Weston a bit of time to set itself up.
> +        time.sleep(self._startup_delay_secs)

I wonder if you could just listen to the socket and continue when it's alive?
Comment 6 Zan Dobersek 2013-07-29 10:21:14 PDT
Comment on attachment 207479 [details]
Patch

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

>> Tools/Scripts/webkitpy/port/westondriver.py:64
>> +        time.sleep(self._startup_delay_secs)
> 
> I wonder if you could just listen to the socket and continue when it's alive?

I understand how sleeping at the start is not really an ideal solution to the problem, but I think listening to sockets here would, on the other hand, be a complex and not-that-testable solution. I don't believe there's anything wrapping sockets (and making them testable) in webkitpy at the moment.

So at least for now I'd propose keeping the time.sleep call, possibly address it later while coming up with a unit test for the driver.
Comment 7 Martin Robinson 2013-07-29 12:26:33 PDT
(In reply to comment #6)
> (From update of attachment 207479 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=207479&action=review
> 
> >> Tools/Scripts/webkitpy/port/westondriver.py:64
> >> +        time.sleep(self._startup_delay_secs)
> > 
> > I wonder if you could just listen to the socket and continue when it's alive?
> 
> I understand how sleeping at the start is not really an ideal solution to the problem, but I think listening to sockets here would, on the other hand, be a complex and not-that-testable solution. I don't believe there's anything wrapping sockets (and making them testable) in webkitpy at the moment.
> 
> So at least for now I'd propose keeping the time.sleep call, possibly address it later while coming up with a unit test for the driver.

Sounds like a reasonable solution for now. If this is causing trouble, we can always re-approach it and use something more foolproof. I know that in the past we've had issues waiting for Xvfb to start.
Comment 8 Zan Dobersek 2013-07-29 13:08:08 PDT
Comment on attachment 207479 [details]
Patch

Clearing flags on attachment: 207479

Committed r153439: <http://trac.webkit.org/changeset/153439>
Comment 9 Zan Dobersek 2013-07-29 13:08:15 PDT
All reviewed patches have been landed.  Closing bug.