Convert video-source-[load|moved|removed].html tests to testharness.js

Convert video-source-[load|moved|removed].html tests to testharness.js

Cleaning up video-source-[load|moved|removed].html tests
in media/ 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/126675172c0f5c9cfb1410e9b86d1bdb03be53e2
Cr-Commit-Position: refs/heads/master@{#403793}

[Srirama, 2016-07-05 12:56:09]

Description was changed from

==========
fix

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

to

==========
Convert video-source-[load|moved|removed].html tests to testharness.js

Cleaning up video-source-[load|moved|removed].html tests
in media/ to use testharness.js instead of video-test.js.
This will enable to upstream these tests to web-platform-tests.

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

[Srirama, 2016-07-05 12:57:23]

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

[Srirama, 2016-07-05 12:57:23]

PTAL

[fs, 2016-07-05 13:33:04]

LGTM w/ nit and suggestions

https://codereview.chromium.org/2116293004/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/media/video-source-load.html (right):

https://codereview.chromium.org/2116293004/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/video-source-load.html:27: return;
Nit: indentation (or, remove since it's redundant)

https://codereview.chromium.org/2116293004/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/media/video-source-moved.html (right):

https://codereview.chromium.org/2116293004/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/video-source-moved.html:9: var testInfo = [
Optionally, this could be written using forEach:

var testInfo = [ ... ];
testInfo.forEach(function(info) { ... });

https://codereview.chromium.org/2116293004/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/video-source-moved.html:16: { func :
moveEarlier, errorCount : 0, moved : null, iteration : 4 }
I wonder if this test would be slighty easier to comprehend if 'iteration' was a
function (probably renamed), and then the 'case 2' would be something like:

test.moved = test.elementToMove(video, current);
...

(and the functions would just be declared inline in testInfo.)

https://codereview.chromium.org/2116293004/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/media/video-source-removed.html (right):

https://codereview.chromium.org/2116293004/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/video-source-removed.html:13:
setTimeout(t.step_func_done(), 100);
Not quite sure why we need 100ms here. I guess it's a "give it some time to
crash"-delay...

[Srirama, 2016-07-05 14:55:28]

https://codereview.chromium.org/2116293004/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/media/video-source-load.html (right):

https://codereview.chromium.org/2116293004/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/video-source-load.html:27: return;
On 2016/07/05 13:33:04, fs wrote:
> Nit: indentation (or, remove since it's redundant)

Done.

https://codereview.chromium.org/2116293004/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/media/video-source-moved.html (right):

https://codereview.chromium.org/2116293004/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/video-source-moved.html:9: var testInfo = [
On 2016/07/05 13:33:04, fs wrote:
> Optionally, this could be written using forEach:
> 
> var testInfo = [ ... ];
> testInfo.forEach(function(info) { ... });

You mean to avoid the for loop above? Problem is these tests are running in
parallel and that seems to be creating problem. If we move testInfo outside,
then we need to move "movetoEnd" and "moveEarlier" functions also outside and
that is creating problem. So i moved everything with a for loop outside so that
each test has its own copy of those two functions.

https://codereview.chromium.org/2116293004/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/video-source-moved.html:16: { func :
moveEarlier, errorCount : 0, moved : null, iteration : 4 }
On 2016/07/05 13:33:04, fs wrote:
> I wonder if this test would be slighty easier to comprehend if 'iteration' was
a
> function (probably renamed), and then the 'case 2' would be something like:
> 
> test.moved = test.elementToMove(video, current);
> ...
> 
> (and the functions would just be declared inline in testInfo.)

Done. Looks much better now, thanks.

https://codereview.chromium.org/2116293004/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/media/video-source-removed.html (right):

https://codereview.chromium.org/2116293004/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/video-source-removed.html:13:
setTimeout(t.step_func_done(), 100);
On 2016/07/05 13:33:04, fs wrote:
> Not quite sure why we need 100ms here. I guess it's a "give it some time to
> crash"-delay...

yes, for the crash reason.

[fs, 2016-07-05 15:03:35]

https://codereview.chromium.org/2116293004/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/media/video-source-moved.html (right):

https://codereview.chromium.org/2116293004/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/video-source-moved.html:9: var testInfo = [
On 2016/07/05 at 14:55:28, Srirama wrote:
> On 2016/07/05 13:33:04, fs wrote:
> > Optionally, this could be written using forEach:
> > 
> > var testInfo = [ ... ];
> > testInfo.forEach(function(info) { ... });
> 
> You mean to avoid the for loop above? Problem is these tests are running in
parallel and that seems to be creating problem. If we move testInfo outside,
then we need to move "movetoEnd" and "moveEarlier" functions also outside and
that is creating problem. So i moved everything with a for loop outside so that
each test has its own copy of those two functions.

It's possible that "this" could be used since we're calling via test.func, but
the current form looks ok too.

[Srirama, 2016-07-05 15:20:53]

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

[Srirama, 2016-07-05 15:20:54]

The patchset sent to the CQ was uploaded after l-g-t-m from fs@opera.com
Link to the patchset: https://codereview.chromium.org/2116293004/#ps20001
(title: "address comments")

[commit-bot: I haz the power, 2016-07-05 15:21:07]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-05 16:41:14]

Description was changed from

==========
Convert video-source-[load|moved|removed].html tests to testharness.js

Cleaning up video-source-[load|moved|removed].html tests
in media/ to use testharness.js instead of video-test.js.
This will enable to upstream these tests to web-platform-tests.

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

to

==========
Convert video-source-[load|moved|removed].html tests to testharness.js

Cleaning up video-source-[load|moved|removed].html tests
in media/ 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-07-05 16:41:15]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-05 16:44:00]

Description was changed from

==========
Convert video-source-[load|moved|removed].html tests to testharness.js

Cleaning up video-source-[load|moved|removed].html tests
in media/ to use testharness.js instead of video-test.js.
This will enable to upstream these tests to web-platform-tests.

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

to

==========
Convert video-source-[load|moved|removed].html tests to testharness.js

Cleaning up video-source-[load|moved|removed].html tests
in media/ 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/126675172c0f5c9cfb1410e9b86d1bdb03be53e2
Cr-Commit-Position: refs/heads/master@{#403793}
==========

[commit-bot: I haz the power, 2016-07-05 16:44:01]

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