Bug 64921 - Layout tests for validating the new WebVTT Parser
Summary: Layout tests for validating the new WebVTT Parser
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Anna Cavender
URL:
Keywords:
Depends on: 65482
Blocks: 62882
  Show dependency treegraph
 
Reported: 2011-07-20 20:28 PDT by Anna Cavender
Modified: 2011-08-01 13:15 PDT (History)
8 users (show)

See Also:


Attachments
new layout tests for webvtt parser (68.98 KB, patch)
2011-07-20 21:46 PDT, Anna Cavender
no flags Details | Formatted Diff | Diff
moving tests to media/track (66.02 KB, patch)
2011-07-21 11:23 PDT, Anna Cavender
no flags Details | Formatted Diff | Diff
addressing eric's comments (65.87 KB, patch)
2011-07-25 15:24 PDT, Anna Cavender
no flags Details | Formatted Diff | Diff
updating to ToT (65.95 KB, patch)
2011-07-28 17:54 PDT, Anna Cavender
eric.carlson: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anna Cavender 2011-07-20 20:28:56 PDT
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
Comment 1 Anna Cavender 2011-07-20 21:46:49 PDT
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.
Comment 2 Eric Carlson 2011-07-21 08:48:58 PDT
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.
Comment 3 Anna Cavender 2011-07-21 11:23:37 PDT
Created attachment 101610 [details]
moving tests to media/track

Good idea, Eric.  Tests are moved.
Comment 4 Eric Carlson 2011-07-22 05:50:40 PDT
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).
Comment 5 Anna Cavender 2011-07-25 15:24:38 PDT
Created attachment 101924 [details]
addressing eric's comments
Comment 6 Anna Cavender 2011-07-28 17:54:33 PDT
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.
Comment 7 WebKit Review Bot 2011-07-29 10:06:03 PDT
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
Comment 8 David Levin 2011-07-29 12:57:35 PDT
Committed as http://trac.webkit.org/changeset/92003
Comment 9 Ryosuke Niwa 2011-07-31 12:50:42 PDT
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
Comment 10 Eric Carlson 2011-07-31 12:55:10 PDT
(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.
Comment 11 Ryosuke Niwa 2011-07-31 13:08:38 PDT
(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.
Comment 12 Ryosuke Niwa 2011-07-31 13:15:59 PDT
They are also failing on Win7 (Tests):
http://build.webkit.org/results/Windows%207%20Release%20(Tests)/r92084%20(15067)/results.html