Bug 64921 – Layout tests for validating the new WebVTT Parser

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Anna Cavender]

These tests will validate a new WebVTT parser.

Please note, the WebVTT parser is currently under review (https://bugs.webkit.org/show_bug.cgi?id=62882) and the <track> feature is not fully implemented (more patches coming soon): https://bugs.webkit.org/show_bug.cgi?id=43668

[Anna Cavender]

Created attachment 101554 [details]
new layout tests for webvtt parser

This is not a complete list and a few more tests are coming soon: e.g. validating cue content text.  I am unsure of all of the test_expectations.txt files that need to have SKIPs, so please advise.  Thanks Silvia Pfeiffer for creating the test case .vtt files.

[Eric Carlson]

Comment on attachment 101554 [details]
new layout tests for webvtt parser

These look great, thanks! I wonder if it might make sense to put all of these in a "tracks" directory inside of LayoutTests/media. Besides making it easier to find all track specific tests, it would allow you to skip all track tests with a single entry in the skipped list.

[Anna Cavender]

Created attachment 101610 [details]
moving tests to media/track

Good idea, Eric.  Tests are moved.

[Eric Carlson]

Comment on attachment 101610 [details]
moving tests to media/track

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

> LayoutTests/media/track/track-webvtt-tc003-newlines.html:19
> +                testExpected("video.textTracks["+i+"].cues.length", "1");
> +                
> +                testExpected("video.textTracks["+i+"].cues[0].id","1");
> +                testExpected("video.textTracks["+i+"].cues[0].startTime","0.0");
> +                testExpected("video.textTracks["+i+"].cues[0].endTime","30.5");
> +                testExpected("video.textTracks["+i+"].cues[0].getCueAsSource()", "Bear is Coming!!!!!");

Nit: I think the WebKit style is to have a space on both sides of the '+':  video.textTracks[" + i + "] (here and elsewhere).

> LayoutTests/media/track/track-webvtt-tc003-newlines.html:29
> +                numberTests++;
> +                allTestsEnded();
> +            }
> +            
> +            function allTestsEnded()
> +            {
> +                if (numberTests >= 2)
> +                    endTest();
> +            }

Nit: I think it would be slightly clearer to increment the global in allTestsEnded (here and elsewhere).

[Anna Cavender]

Created attachment 101924 [details]
addressing eric's comments

[Anna Cavender]

Created attachment 102318 [details]
updating to ToT

Let's get these in even though there might be more tests coming soon.  Also, looks like it may have to be committed manually: I think some funky characters in the test files are preventing the patch from applying.

[WebKit Review Bot]

Comment on attachment 102318 [details]
updating to ToT

Rejecting attachment 102318 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-01', '--port..." exit_code: 2

Last 500 characters of output:
m patch file(s).
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patch: **** Only garbage was found in the patch input.
fatal: pathspec 'LayoutTests/media/track/captions/tc000_empty.vtt' did not match any files
Failed to git add LayoutTests/media/track/captions/tc000_empty.vtt. at /mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply line 439.

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Eric Carlson', u'--for..." exit_code: 2

Full output: http://queues.webkit.org/results/9265514

[David Levin]

Committed as http://trac.webkit.org/changeset/92003

[Ryosuke Niwa]

The following tests started failing on SnowLeopard Intel Release (Tests) in r92003 <http://trac.webkit.org/changeset/92003>:

    media/track/track-webvtt-tc006-cueidentifiers.html
    media/track/track-webvtt-tc001-utf8.html
    media/track/track-webvtt-tc011-blanklines.html
    media/track/track-webvtt-tc005-headercomment.html
    media/track/track-webvtt-tc004-magicheader.html
    media/track/track-webvtt-tc002-bom.html
    media/track/track-webvtt-tc013-settings.html
    media/track/track-webvtt-tc003-newlines.html
    media/track/track-webvtt-tc009-timingshour.html
    media/track/track-webvtt-tc012-outoforder.html
    media/track/track-webvtt-tc007-cuenoid.html
    media/track/track-webvtt-tc000-empty.html
    media/track/track-webvtt-tc010-notimings.html
    media/track/track-webvtt-tc008-timingsnohours.html

[Eric Carlson]

(In reply to comment #9)
> The following tests started failing on SnowLeopard Intel Release (Tests) in r92003 <http://trac.webkit.org/changeset/92003>:
> 
>     media/track/track-webvtt-tc006-cueidentifiers.html
>     media/track/track-webvtt-tc001-utf8.html
>     media/track/track-webvtt-tc011-blanklines.html
>     media/track/track-webvtt-tc005-headercomment.html
>     media/track/track-webvtt-tc004-magicheader.html
>     media/track/track-webvtt-tc002-bom.html
>     media/track/track-webvtt-tc013-settings.html
>     media/track/track-webvtt-tc003-newlines.html
>     media/track/track-webvtt-tc009-timingshour.html
>     media/track/track-webvtt-tc012-outoforder.html
>     media/track/track-webvtt-tc007-cuenoid.html
>     media/track/track-webvtt-tc000-empty.html
>     media/track/track-webvtt-tc010-notimings.html
>     media/track/track-webvtt-tc008-timingsnohours.html

None of these tests should be run on *any* platform at the moment.

[Ryosuke Niwa]

(In reply to comment #10)
> None of these tests should be run on *any* platform at the moment.

Please add them to the skipped list then.

[Ryosuke Niwa]

They are also failing on Win7 (Tests):
http://build.webkit.org/results/Windows%207%20Release%20(Tests)/r92084%20(15067)/results.html