Convert track-webvtt-tc[027-029] tests to testharness.js

Convert track-webvtt-tc[027-029] tests to testharness.js

Cleaning up track-webvtt-tc[027-029] tests in media/track
to use testharness.js instead of video-test.js.
This will enable to upstream these tests to web-platform-tests.

BUG=588956
Committed: https://crrev.com/9155d8db2891ec7f9eb7e6c66752774e57b19262
Cr-Commit-Position: refs/heads/master@{#397939}

[Srirama, 2016-06-03 14:04:17]

Description was changed from

==========
fix

BUG=
==========

to

==========
Convert track-webvtt-tc[027-029] tests to testharness.js

Cleaning up track-webvtt-tc[027-029] tests in media/track
to use testharness.js instead of video-test.js.
This will enable to upstream these tests to web-platform-tests.

BUG=588956
==========

[Srirama, 2016-06-03 14:09:28]

srirama.m@samsung.com changed reviewers:
+ foolip@chromium.org, fs@opera.com

[Srirama, 2016-06-03 14:09:29]

PTAL

https://codereview.chromium.org/2030383002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/media/track/track-webvtt-tc027-empty-cue-expected.txt
(right):

https://codereview.chromium.org/2030383002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/track/track-webvtt-tc027-empty-cue-expected.txt:2:
FAIL Check cues from captions-webvtt/tc027-empty-cue.vtt assert_equals: expected
0 but got 3
There are few more tests with similar reason for failure. Digged a bit into it
thinking that i will fix it, but ended up outside blink here
(https://cs.chromium.org/chromium/src/third_party/libwebm/source/webvtt/webvtt...).

https://codereview.chromium.org/2030383002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/media/track/track-webvtt-tc028-unsupported-markup.html
(right):

https://codereview.chromium.org/2030383002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/track/track-webvtt-tc028-unsupported-markup.html:26:
assert_equals(cues[i].getCueAsHTML().textContent, expected[i].innerHTML);
Can this test also be written in a better way like previous tests?

[fs, 2016-06-03 14:37:16]

lgtm

https://codereview.chromium.org/2030383002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/media/track/track-webvtt-tc027-empty-cue-expected.txt
(right):

https://codereview.chromium.org/2030383002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/track/track-webvtt-tc027-empty-cue-expected.txt:2:
FAIL Check cues from captions-webvtt/tc027-empty-cue.vtt assert_equals: expected
0 but got 3
On 2016/06/03 at 14:09:29, Srirama wrote:
> There are few more tests with similar reason for failure. Digged a bit into it
thinking that i will fix it, but ended up outside blink here
(https://cs.chromium.org/chromium/src/third_party/libwebm/source/webvtt/webvtt...).

The correct parser would be the one at core/html/track/vtt/VTTParser.cpp - this
test predates a change to the spec though, so you can just update the test to
expect 3 cues. (Looks like it was I that added the failing expectations...
164c59b7e86914c9a0eef9756ef6e8267bf13756)

https://codereview.chromium.org/2030383002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/media/track/track-webvtt-tc028-unsupported-markup.html
(right):

https://codereview.chromium.org/2030383002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/track/track-webvtt-tc028-unsupported-markup.html:26:
assert_equals(cues[i].getCueAsHTML().textContent, expected[i].innerHTML);
On 2016/06/03 at 14:09:29, Srirama wrote:
> Can this test also be written in a better way like previous tests?

I think that depends on how convoluted we want to make assert_cues_match =). I
guess allow specifying an object as the 'value' part of the list and then have:

if (expectedItem[property] instanceof Object)
    assert_equals(expectedItem[property].extractor(cue),
expectedItem[property].expected);
else
    assert_equals(...); // like before.

I.e have entries like:

{ extractor: cue => { return cue.getCueAsHTML().textContent; }, "..." }

This test would probably be better off in something like what
LayoutTests/media/track/opera/track/webvtt/parsing-cue-data/common.js does. The
way it is written now I'm not sure it even tests what it sets out to test...

Also, I'm fine with punting on any "fancy" stuff.

[Srirama, 2016-06-03 17:54:23]

https://codereview.chromium.org/2030383002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/media/track/track-webvtt-tc027-empty-cue-expected.txt
(right):

https://codereview.chromium.org/2030383002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/track/track-webvtt-tc027-empty-cue-expected.txt:2:
FAIL Check cues from captions-webvtt/tc027-empty-cue.vtt assert_equals: expected
0 but got 3
On 2016/06/03 14:37:16, fs wrote:
> On 2016/06/03 at 14:09:29, Srirama wrote:
> > There are few more tests with similar reason for failure. Digged a bit into
it
> thinking that i will fix it, but ended up outside blink here
>
(https://cs.chromium.org/chromium/src/third_party/libwebm/source/webvtt/webvtt...).
> 
> The correct parser would be the one at core/html/track/vtt/VTTParser.cpp -
this
> test predates a change to the spec though, so you can just update the test to
> expect 3 cues. (Looks like it was I that added the failing expectations...
> 164c59b7e86914c9a0eef9756ef6e8267bf13756)

Done.

https://codereview.chromium.org/2030383002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/media/track/track-webvtt-tc028-unsupported-markup.html
(right):

https://codereview.chromium.org/2030383002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/track/track-webvtt-tc028-unsupported-markup.html:26:
assert_equals(cues[i].getCueAsHTML().textContent, expected[i].innerHTML);
On 2016/06/03 14:37:16, fs wrote:
> On 2016/06/03 at 14:09:29, Srirama wrote:
> > Can this test also be written in a better way like previous tests?
> 
> I think that depends on how convoluted we want to make assert_cues_match =). I
> guess allow specifying an object as the 'value' part of the list and then
have:
> 
> if (expectedItem[property] instanceof Object)
>     assert_equals(expectedItem[property].extractor(cue),
> expectedItem[property].expected);
> else
>     assert_equals(...); // like before.
> 
> I.e have entries like:
> 
> { extractor: cue => { return cue.getCueAsHTML().textContent; }, "..." }
> 
> This test would probably be better off in something like what
> LayoutTests/media/track/opera/track/webvtt/parsing-cue-data/common.js does.
The
> way it is written now I'm not sure it even tests what it sets out to test...
> 
> Also, I'm fine with punting on any "fancy" stuff.

Thanks, looks like there is lot of stuff that i need to learn. I will revisit
this again. For now i am adding a TODO.

[fs, 2016-06-04 14:07:17]

On 2016/06/03 at 17:54:23, srirama.m wrote:
>
https://codereview.chromium.org/2030383002/diff/1/third_party/WebKit/LayoutTe...
> File
third_party/WebKit/LayoutTests/media/track/track-webvtt-tc027-empty-cue-expected.txt
(right):
> 
>
https://codereview.chromium.org/2030383002/diff/1/third_party/WebKit/LayoutTe...
>
third_party/WebKit/LayoutTests/media/track/track-webvtt-tc027-empty-cue-expected.txt:2:
FAIL Check cues from captions-webvtt/tc027-empty-cue.vtt assert_equals: expected
0 but got 3
> On 2016/06/03 14:37:16, fs wrote:
> > On 2016/06/03 at 14:09:29, Srirama wrote:
> > > There are few more tests with similar reason for failure. Digged a bit
into it
> > thinking that i will fix it, but ended up outside blink here
> >
(https://cs.chromium.org/chromium/src/third_party/libwebm/source/webvtt/webvtt...).
> > 
> > The correct parser would be the one at core/html/track/vtt/VTTParser.cpp -
this
> > test predates a change to the spec though, so you can just update the test
to
> > expect 3 cues. (Looks like it was I that added the failing expectations...
> > 164c59b7e86914c9a0eef9756ef6e8267bf13756)
> 
> Done.

Still lgtm.

[Srirama, 2016-06-04 15:30:32]

The CQ bit was checked by srirama.m@samsung.com

[commit-bot: I haz the power, 2016-06-04 15:30:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2030383002/20001

[commit-bot: I haz the power, 2016-06-04 19:32:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-04 19:32:24]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Srirama, 2016-06-05 02:47:18]

The CQ bit was checked by srirama.m@samsung.com

[commit-bot: I haz the power, 2016-06-05 02:47:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2030383002/20001

[commit-bot: I haz the power, 2016-06-05 20:03:12]

Description was changed from

==========
Convert track-webvtt-tc[027-029] tests to testharness.js

Cleaning up track-webvtt-tc[027-029] tests in media/track
to use testharness.js instead of video-test.js.
This will enable to upstream these tests to web-platform-tests.

BUG=588956
==========

to

==========
Convert track-webvtt-tc[027-029] tests to testharness.js

Cleaning up track-webvtt-tc[027-029] tests in media/track
to use testharness.js instead of video-test.js.
This will enable to upstream these tests to web-platform-tests.

BUG=588956
==========

[commit-bot: I haz the power, 2016-06-05 20:03:13]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-05 20:05:03]

Description was changed from

==========
Convert track-webvtt-tc[027-029] tests to testharness.js

Cleaning up track-webvtt-tc[027-029] tests in media/track
to use testharness.js instead of video-test.js.
This will enable to upstream these tests to web-platform-tests.

BUG=588956
==========

to

==========
Convert track-webvtt-tc[027-029] tests to testharness.js

Cleaning up track-webvtt-tc[027-029] tests in media/track
to use testharness.js instead of video-test.js.
This will enable to upstream these tests to web-platform-tests.

BUG=588956

Committed: https://crrev.com/9155d8db2891ec7f9eb7e6c66752774e57b19262
Cr-Commit-Position: refs/heads/master@{#397939}
==========

[commit-bot: I haz the power, 2016-06-05 20:05:04]

Patchset 2 (id:??) landed as
https://crrev.com/9155d8db2891ec7f9eb7e6c66752774e57b19262
Cr-Commit-Position: refs/heads/master@{#397939}