Bug 132329 - [GTK] Enable skipped http/tests/media/hls/video-controls-live-stream.html
Summary: [GTK] Enable skipped http/tests/media/hls/video-controls-live-stream.html
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Xabier Rodríguez Calvar
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-29 03:01 PDT by Lorenzo Tilve
Modified: 2014-05-08 03:29 PDT (History)
15 users (show)

See Also:


Attachments
Patch (12.04 KB, patch)
2014-04-30 09:24 PDT, Xabier Rodríguez Calvar
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 (468.28 KB, application/zip)
2014-04-30 10:22 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion (496.49 KB, application/zip)
2014-04-30 10:51 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion (498.66 KB, application/zip)
2014-04-30 11:52 PDT, Build Bot
no flags Details
Patch (12.27 KB, patch)
2014-05-02 13:12 PDT, Xabier Rodríguez Calvar
no flags Details | Formatted Diff | Diff
Patch (12.28 KB, patch)
2014-05-07 03:59 PDT, Xabier Rodríguez Calvar
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Lorenzo Tilve 2014-04-29 03:01:18 PDT
As support for Live streams has been introduced at http://trac.webkit.org/changeset/167828 this test could be fixed and unskipped on GTK.

It's currently failing.

33EXPECTED (video.duration == 'Infinity') OK
44EXPECTED (mediaControlsElement(controls, "-webkit-media-controls-rewind-button") == 'null') OK
5 EXPECTED (mediaControlsElement(controls, "-webkit-media-controls-play-button") != 'null') OK
6 EXPECTED (mediaControlsElement(controls, "-webkit-media-controls-status-display") != 'null') OK
 5EXPECTED (mediaControlsElement(controls, "-webkit-media-controls-play-button") != 'null'), OBSERVED 'null' FAIL
 6EXPECTED (mediaControlsElement(controls, "-webkit-media-controls-status-display") != 'null'), OBSERVED 'null' FAIL
77EXPECTED (mediaControlsElement(controls, "-webkit-media-controls-timeline") == 'null') OK
88EXPECTED (mediaControlsElement(controls, "-webkit-media-controls-current-time-display") == 'null') OK
99EXPECTED (mediaControlsElement(controls, "-webkit-media-controls-remaining-time-display") == 'null') OK
Comment 1 Xabier Rodríguez Calvar 2014-04-30 09:24:33 PDT
Created attachment 230483 [details]
Patch
Comment 2 Martin Robinson 2014-04-30 10:03:00 PDT
Comment on attachment 230483 [details]
Patch

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

> LayoutTests/platform/mac/http/tests/media/hls/video-controls-live-stream-expected.txt:11
> +
> +EVENT(play)
> +EXPECTED (video.duration == 'Infinity') OK
> +EXPECTED (mediaControlsElement(controls, "-webkit-media-controls-rewind-button") == 'null') OK
> +EXPECTED (mediaControlsElement(controls, "-webkit-media-controls-play-button") != 'null') OK
> +EXPECTED (mediaControlsElement(controls, "-webkit-media-controls-status-display") != 'null') OK
> +EXPECTED (mediaControlsElement(controls, "-webkit-media-controls-timeline") == 'null') OK
> +EXPECTED (mediaControlsElement(controls, "-webkit-media-controls-current-time-display") == 'null') OK
> +EXPECTED (mediaControlsElement(controls, "-webkit-media-controls-remaining-time-display") == 'null') OK
> +END OF TEST
> +

You removed all the testExpected lines so aren't these results invalid now?
Comment 3 Build Bot 2014-04-30 10:22:52 PDT
Comment on attachment 230483 [details]
Patch

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

New failing tests:
http/tests/media/hls/video-controls-live-stream.html
Comment 4 Build Bot 2014-04-30 10:22:59 PDT
Created attachment 230489 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 5 Build Bot 2014-04-30 10:51:03 PDT
Comment on attachment 230483 [details]
Patch

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

New failing tests:
http/tests/media/hls/video-controls-live-stream.html
Comment 6 Build Bot 2014-04-30 10:51:10 PDT
Created attachment 230494 [details]
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-03  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 7 Philippe Normand 2014-04-30 11:40:11 PDT
Yep, you can manually rebase the results from the mac EWS. That'd be great to do before landing the patch.
Comment 8 Build Bot 2014-04-30 11:52:47 PDT
Comment on attachment 230483 [details]
Patch

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

New failing tests:
http/tests/media/hls/video-controls-live-stream.html
Comment 9 Build Bot 2014-04-30 11:52:54 PDT
Created attachment 230504 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 10 Xabier Rodríguez Calvar 2014-04-30 15:55:15 PDT
(In reply to comment #2)
> (From update of attachment 230483 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=230483&action=review
> 
> > LayoutTests/platform/mac/http/tests/media/hls/video-controls-live-stream-expected.txt:11
> > +
> > +EVENT(play)
> > +EXPECTED (video.duration == 'Infinity') OK
> > +EXPECTED (mediaControlsElement(controls, "-webkit-media-controls-rewind-button") == 'null') OK
> > +EXPECTED (mediaControlsElement(controls, "-webkit-media-controls-play-button") != 'null') OK
> > +EXPECTED (mediaControlsElement(controls, "-webkit-media-controls-status-display") != 'null') OK
> > +EXPECTED (mediaControlsElement(controls, "-webkit-media-controls-timeline") == 'null') OK
> > +EXPECTED (mediaControlsElement(controls, "-webkit-media-controls-current-time-display") == 'null') OK
> > +EXPECTED (mediaControlsElement(controls, "-webkit-media-controls-remaining-time-display") == 'null') OK
> > +END OF TEST
> > +
> 
> You removed all the testExpected lines so aren't these results invalid now?

Regarding this there are two things: the first is that I moved this file and that's why all these lines are going away in one side and coming in the other; you can see them in the platform/mac directory. The second is that I do need to get the new baseline for the test as Phil suggests because I was waiting for the failures to update it.
Comment 11 Xabier Rodríguez Calvar 2014-05-02 13:12:13 PDT
Created attachment 230689 [details]
Patch
Comment 12 Martin Robinson 2014-05-05 12:50:21 PDT
Comment on attachment 230689 [details]
Patch

The GTK+ bits look fine to me, but I think someone else should review the changes to the tests.
Comment 13 Xabier Rodríguez Calvar 2014-05-05 15:03:26 PDT
(In reply to comment #12)
> (From update of attachment 230689 [details])
> The GTK+ bits look fine to me, but I think someone else should review the changes to the tests.

Eric, Jer, would you have a moment for a review?
Comment 14 Xabier Rodríguez Calvar 2014-05-06 09:23:31 PDT
Brent, you reviewed fix for bug 131390, would you have a minute for this?
Comment 15 Eric Carlson 2014-05-06 10:21:20 PDT
Comment on attachment 230689 [details]
Patch

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

> Source/WebCore/Modules/mediacontrols/mediaControlsGtk.js:63
> +    configureControls: function() {
> +        if (!this.controls.configured) {
> +            this.configureInlineControls();
> +            this.controls.configured = true;
> +            this.addControls();
> +        }
> +    },

Nit: you should use an early return here:

    if (this.controls.configured) 
        return;
Comment 16 Xabier Rodríguez Calvar 2014-05-07 03:59:41 PDT
Created attachment 230985 [details]
Patch
Comment 17 Xabier Rodríguez Calvar 2014-05-07 04:01:26 PDT
(In reply to comment #15)
> Nit: you should use an early return here:
> 
>     if (this.controls.configured) 
>         return;

Done this.

I also rebased the test because of the long time labels. I think that requires a new review.
Comment 18 Eric Carlson 2014-05-07 11:12:19 PDT
Comment on attachment 230985 [details]
Patch

Thank you!
Comment 19 WebKit Commit Bot 2014-05-08 03:29:06 PDT
Comment on attachment 230985 [details]
Patch

Clearing flags on attachment: 230985

Committed r168467: <http://trac.webkit.org/changeset/168467>
Comment 20 WebKit Commit Bot 2014-05-08 03:29:13 PDT
All reviewed patches have been landed.  Closing bug.