Remove video-test.js dependency from [audio, video]-autoplay-* tests

Remove video-test.js dependency from [audio, video]-autoplay-* tests

BUG=588956
Committed: https://crrev.com/2fce54e4d537bc7a36acc0bd7e514aa5a2818564
Cr-Commit-Position: refs/heads/master@{#405166}

[Srirama, 2016-07-09 12:31:20]

Description was changed from

==========
fix

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

to

==========
Remove video-test.js dependency from [audio, video]-autoplay-* tests

Remove video-test.js dependency from [audio, video]-autoplay-* tests.
This will enable to upstream these tests to web-platform-tests.

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

[Srirama, 2016-07-11 12:44:43]

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

[Srirama, 2016-07-11 12:44:44]

PTAL

[fs, 2016-07-12 17:42:34]

lgtm

This CL doesn't appear to improve/aid "upstreamability", so the description
should probably be updated accordingly. (Removing the TODO seems ok to me though
because of what aspects of the implementation these tests test.)

[Srirama, 2016-07-13 07:02:28]

On 2016/07/12 17:42:34, fs (ooo to July 18) wrote:
> lgtm
> 
> This CL doesn't appear to improve/aid "upstreamability", so the description
> should probably be updated accordingly. (Removing the TODO seems ok to me
though
> because of what aspects of the implementation these tests test.)

doesn't getting rid of video-test.js qualify for upstreaming? Only tests using
testharness.js are qualified for upstreaming?

[fs, 2016-07-13 14:27:31]

On 2016/07/13 at 07:02:28, srirama.m wrote:
> On 2016/07/12 17:42:34, fs (ooo to July 18) wrote:
> > lgtm
> > 
> > This CL doesn't appear to improve/aid "upstreamability", so the description
> > should probably be updated accordingly. (Removing the TODO seems ok to me
though
> > because of what aspects of the implementation these tests test.)
> 
> doesn't getting rid of video-test.js qualify for upstreaming? Only tests using
testharness.js are qualified for upstreaming?

It may not be a strict requirement, but I'd go as far as saying it is preferable
(especially for "can write as JS-only"-tests.) That being said though, these
tests are for some non-uservisible "feature" ("autoplay experiment"), so can't
make little sense to upstream regardless.

[Srirama, 2016-07-13 14:33:38]

Description was changed from

==========
Remove video-test.js dependency from [audio, video]-autoplay-* tests

Remove video-test.js dependency from [audio, video]-autoplay-* tests.
This will enable to upstream these tests to web-platform-tests.

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

to

==========
Remove video-test.js dependency from [audio, video]-autoplay-* tests

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

[Srirama, 2016-07-13 14:34:21]

On 2016/07/13 14:27:31, fs (ooo to July 18) wrote:
> On 2016/07/13 at 07:02:28, srirama.m wrote:
> > On 2016/07/12 17:42:34, fs (ooo to July 18) wrote:
> > > lgtm
> > > 
> > > This CL doesn't appear to improve/aid "upstreamability", so the
description
> > > should probably be updated accordingly. (Removing the TODO seems ok to me
> though
> > > because of what aspects of the implementation these tests test.)
> > 
> > doesn't getting rid of video-test.js qualify for upstreaming? Only tests
using
> testharness.js are qualified for upstreaming?
> 
> It may not be a strict requirement, but I'd go as far as saying it is
preferable
> (especially for "can write as JS-only"-tests.) That being said though, these
> tests are for some non-uservisible "feature" ("autoplay experiment"), so can't
> make little sense to upstream regardless.

Ok, understood, thanks.

[Srirama, 2016-07-13 14:34:47]

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

[commit-bot: I haz the power, 2016-07-13 14:35:07]

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

[foolip, 2016-07-13 15:09:53]

The CQ bit was unchecked by foolip@chromium.org

[foolip, 2016-07-13 15:11:00]

I've unchecked CQ. These are tests that cannot be upstreamed and will be removed
as soon as the experiment is over, so I'd suggest instead just removing the
TODOs.

[foolip, 2016-07-13 15:11:22]

On 2016/07/13 15:11:00, foolip wrote:
> I've unchecked CQ. These are tests that cannot be upstreamed and will be
removed
> as soon as the experiment is over, so I'd suggest instead just removing the
> TODOs.

Sorry that I didn't check all the tests when adding the TODOs, there were so
many :/

[fs, 2016-07-13 15:14:46]

On 2016/07/13 at 15:11:22, foolip wrote:
> On 2016/07/13 15:11:00, foolip wrote:
> > I've unchecked CQ. These are tests that cannot be upstreamed and will be
removed
> > as soon as the experiment is over, so I'd suggest instead just removing the
> > TODOs.
> 
> Sorry that I didn't check all the tests when adding the TODOs, there were so
many :/

I think removing all traces of video-test.js is worthwhile - leave a tool in the
toolbox and someone will undoubtedly use it...

[foolip, 2016-07-13 15:21:37]

On 2016/07/13 15:14:46, fs (ooo to July 18) wrote:
> On 2016/07/13 at 15:11:22, foolip wrote:
> > On 2016/07/13 15:11:00, foolip wrote:
> > > I've unchecked CQ. These are tests that cannot be upstreamed and will be
> removed
> > > as soon as the experiment is over, so I'd suggest instead just removing
the
> > > TODOs.
> > 
> > Sorry that I didn't check all the tests when adding the TODOs, there were so
> many :/
> 
> I think removing all traces of video-test.js is worthwhile - leave a tool in
the
> toolbox and someone will undoubtedly use it...

The media fragments tests were left alone, so they're still around. But I guess
if they're the only ones, then video-test.js can be renamed to
media-fragment-test.js and minimized to what is needed. So LGTM then :)

[foolip, 2016-07-13 15:22:11]

The CQ bit was checked by foolip@chromium.org

[commit-bot: I haz the power, 2016-07-13 15:22:54]

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

[Srirama, 2016-07-13 15:45:38]

On 2016/07/13 15:21:37, foolip wrote:
> On 2016/07/13 15:14:46, fs (ooo to July 18) wrote:
> > On 2016/07/13 at 15:11:22, foolip wrote:
> > > On 2016/07/13 15:11:00, foolip wrote:
> > > > I've unchecked CQ. These are tests that cannot be upstreamed and will be
> > removed
> > > > as soon as the experiment is over, so I'd suggest instead just removing
> the
> > > > TODOs.
> > > 
> > > Sorry that I didn't check all the tests when adding the TODOs, there were
so
> > many :/
> > 
> > I think removing all traces of video-test.js is worthwhile - leave a tool in
> the
> > toolbox and someone will undoubtedly use it...
> 
> The media fragments tests were left alone, so they're still around. But I
guess
> if they're the only ones, then video-test.js can be renamed to
> media-fragment-test.js and minimized to what is needed. So LGTM then :)

Agree with both of you. And i have already thought of minimizing it after
finishing all the test cases. There are just a few test cases left.

[commit-bot: I haz the power, 2016-07-13 15:53:12]

Description was changed from

==========
Remove video-test.js dependency from [audio, video]-autoplay-* tests

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

to

==========
Remove video-test.js dependency from [audio, video]-autoplay-* tests

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

[commit-bot: I haz the power, 2016-07-13 15:53:13]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-07-13 15:53:16]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-13 15:56:33]

Description was changed from

==========
Remove video-test.js dependency from [audio, video]-autoplay-* tests

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

to

==========
Remove video-test.js dependency from [audio, video]-autoplay-* tests

BUG=588956

Committed: https://crrev.com/2fce54e4d537bc7a36acc0bd7e514aa5a2818564
Cr-Commit-Position: refs/heads/master@{#405166}
==========

[commit-bot: I haz the power, 2016-07-13 15:56:34]

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