Convert appcache http tests to testharness.js

Convert appcache video http tests to testharness.js

Cleaning appcache video http tests 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/b9860113bedaeef0f76a8e699ba7551e6bdd0d08
Cr-Commit-Position: refs/heads/master@{#412502}

[Srirama, 2016-08-16 14:08:42]

Description was changed from

==========
fix


fix

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

to

==========
Convert appcache http tests to testharness.js

Cleaning appcache http tests to use
testharness.js instead of video-test.js.
This will enable to upstream these tests to web-platform-tests.

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

[Srirama, 2016-08-16 14:09:58]

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

[Srirama, 2016-08-16 14:09:59]

PTAL

[fs, 2016-08-16 14:32:35]

LGTM w/ the below addressed

> "...appcache http tests..."

Could you add "video" somewhere in there to make that clear?

https://codereview.chromium.org/2252663002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/http/tests/appcache/video.html (right):

https://codereview.chromium.org/2252663002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/appcache/video.html:11:
applicationCache.onnoupdate = t.unreached_func();
So, these two were hooked up differently before (updateready, test1) - I don't
see how we'd get an 'updateready' so that change seems fine, but I guess we
could get a 'noupdate' if running the test twice or so? I'm not an really an
expert on appcache though, so I'd suggest either checking that dispatch orders
are ok and 'noupdate' can be avoided (I don't see that it should fail the test?)
- maybe you checked this already. Or just do what the test did previously (which
is not much additional burden.)

[Srirama, 2016-08-17 07:34:47]

On 2016/08/16 14:32:35, fs wrote:
> LGTM w/ the below addressed
> 
> > "...appcache http tests..."
> 
> Could you add "video" somewhere in there to make that clear?
> 
>
https://codereview.chromium.org/2252663002/diff/1/third_party/WebKit/LayoutTe...
> File third_party/WebKit/LayoutTests/http/tests/appcache/video.html (right):
> 
>
https://codereview.chromium.org/2252663002/diff/1/third_party/WebKit/LayoutTe...
> third_party/WebKit/LayoutTests/http/tests/appcache/video.html:11:
> applicationCache.onnoupdate = t.unreached_func();
> So, these two were hooked up differently before (updateready, test1) - I don't
> see how we'd get an 'updateready' so that change seems fine, but I guess we
> could get a 'noupdate' if running the test twice or so? I'm not an really an
> expert on appcache though, so I'd suggest either checking that dispatch orders
> are ok and 'noupdate' can be avoided (I don't see that it should fail the
test?)
> - maybe you checked this already. Or just do what the test did previously
(which
> is not much additional burden.)

You are right, "noupdate" is fired from second time and this difference in event
behavior is also one of the reason i failed using 3 separate async_tests.
Looks like there are more problems here. From 2nd run onwards it fails to
identify "asyc_test" itself. Probably we need to add testharness.js and
testharnessreport.js also to the manifest? I tried adding
"../resources/testharness.js" but that doesn't seem to recognize the path.

[Srirama, 2016-08-17 07:35:02]

Description was changed from

==========
Convert appcache http tests to testharness.js

Cleaning appcache http tests to use
testharness.js instead of video-test.js.
This will enable to upstream these tests to web-platform-tests.

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

to

==========
Convert appcache video http tests to testharness.js

Cleaning appcache video http tests to use
testharness.js instead of video-test.js.
This will enable to upstream these tests to web-platform-tests.

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

[fs, 2016-08-17 07:47:29]

On 2016/08/17 at 07:34:47, srirama.m wrote:
> On 2016/08/16 14:32:35, fs wrote:
> > LGTM w/ the below addressed
> > 
> > > "...appcache http tests..."
> > 
> > Could you add "video" somewhere in there to make that clear?
> > 
> >
https://codereview.chromium.org/2252663002/diff/1/third_party/WebKit/LayoutTe...
> > File third_party/WebKit/LayoutTests/http/tests/appcache/video.html (right):
> > 
> >
https://codereview.chromium.org/2252663002/diff/1/third_party/WebKit/LayoutTe...
> > third_party/WebKit/LayoutTests/http/tests/appcache/video.html:11:
> > applicationCache.onnoupdate = t.unreached_func();
> > So, these two were hooked up differently before (updateready, test1) - I
don't
> > see how we'd get an 'updateready' so that change seems fine, but I guess we
> > could get a 'noupdate' if running the test twice or so? I'm not an really an
> > expert on appcache though, so I'd suggest either checking that dispatch
orders
> > are ok and 'noupdate' can be avoided (I don't see that it should fail the
test?)
> > - maybe you checked this already. Or just do what the test did previously
(which
> > is not much additional burden.)
> 
> You are right, "noupdate" is fired from second time and this difference in
event behavior is also one of the reason i failed using 3 separate async_tests.
> Looks like there are more problems here. From 2nd run onwards it fails to
identify "asyc_test" itself. Probably we need to add testharness.js and
testharnessreport.js also to the manifest? I tried adding
"../resources/testharness.js" but that doesn't seem to recognize the path.

Try using /resources/testharness.js as the path

[Srirama, 2016-08-17 10:44:24]

On 2016/08/17 07:47:29, fs wrote:
> On 2016/08/17 at 07:34:47, srirama.m wrote:
> > On 2016/08/16 14:32:35, fs wrote:
> > > LGTM w/ the below addressed
> > > 
> > > > "...appcache http tests..."
> > > 
> > > Could you add "video" somewhere in there to make that clear?
> > > 
> > >
>
https://codereview.chromium.org/2252663002/diff/1/third_party/WebKit/LayoutTe...
> > > File third_party/WebKit/LayoutTests/http/tests/appcache/video.html
(right):
> > > 
> > >
>
https://codereview.chromium.org/2252663002/diff/1/third_party/WebKit/LayoutTe...
> > > third_party/WebKit/LayoutTests/http/tests/appcache/video.html:11:
> > > applicationCache.onnoupdate = t.unreached_func();
> > > So, these two were hooked up differently before (updateready, test1) - I
> don't
> > > see how we'd get an 'updateready' so that change seems fine, but I guess
we
> > > could get a 'noupdate' if running the test twice or so? I'm not an really
an
> > > expert on appcache though, so I'd suggest either checking that dispatch
> orders
> > > are ok and 'noupdate' can be avoided (I don't see that it should fail the
> test?)
> > > - maybe you checked this already. Or just do what the test did previously
> (which
> > > is not much additional burden.)
> > 
> > You are right, "noupdate" is fired from second time and this difference in
> event behavior is also one of the reason i failed using 3 separate
async_tests.
> > Looks like there are more problems here. From 2nd run onwards it fails to
> identify "asyc_test" itself. Probably we need to add testharness.js and
> testharnessreport.js also to the manifest? I tried adding
> "../resources/testharness.js" but that doesn't seem to recognize the path.
> 
> Try using /resources/testharness.js as the path

I have already tried that and the test is timing out from 2nd iteration, so i
thought path is still a problem, but the problem is with "noupdate" event not
being handled. Its fine now after adding "noupdate" event. Thanks.

[fs, 2016-08-17 10:47:34]

On 2016/08/17 at 10:44:24, srirama.m wrote:
> On 2016/08/17 07:47:29, fs wrote:
> > On 2016/08/17 at 07:34:47, srirama.m wrote:
> > > On 2016/08/16 14:32:35, fs wrote:
> > > > LGTM w/ the below addressed
> > > > 
> > > > > "...appcache http tests..."
> > > > 
> > > > Could you add "video" somewhere in there to make that clear?
> > > > 
> > > >
> >
https://codereview.chromium.org/2252663002/diff/1/third_party/WebKit/LayoutTe...
> > > > File third_party/WebKit/LayoutTests/http/tests/appcache/video.html
(right):
> > > > 
> > > >
> >
https://codereview.chromium.org/2252663002/diff/1/third_party/WebKit/LayoutTe...
> > > > third_party/WebKit/LayoutTests/http/tests/appcache/video.html:11:
> > > > applicationCache.onnoupdate = t.unreached_func();
> > > > So, these two were hooked up differently before (updateready, test1) - I
> > don't
> > > > see how we'd get an 'updateready' so that change seems fine, but I guess
we
> > > > could get a 'noupdate' if running the test twice or so? I'm not an
really an
> > > > expert on appcache though, so I'd suggest either checking that dispatch
> > orders
> > > > are ok and 'noupdate' can be avoided (I don't see that it should fail
the
> > test?)
> > > > - maybe you checked this already. Or just do what the test did
previously
> > (which
> > > > is not much additional burden.)
> > > 
> > > You are right, "noupdate" is fired from second time and this difference in
> > event behavior is also one of the reason i failed using 3 separate
async_tests.
> > > Looks like there are more problems here. From 2nd run onwards it fails to
> > identify "asyc_test" itself. Probably we need to add testharness.js and
> > testharnessreport.js also to the manifest? I tried adding
> > "../resources/testharness.js" but that doesn't seem to recognize the path.
> > 
> > Try using /resources/testharness.js as the path
> 
> I have already tried that and the test is timing out from 2nd iteration, so i
thought path is still a problem, but the problem is with "noupdate" event not
being handled. Its fine now after adding "noupdate" event. Thanks.

Thanks. LGTM.

[Srirama, 2016-08-17 10:49:29]

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

[commit-bot: I haz the power, 2016-08-17 10:49:40]

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-08-17 12:03:49]

Description was changed from

==========
Convert appcache video http tests to testharness.js

Cleaning appcache video http tests to use
testharness.js instead of video-test.js.
This will enable to upstream these tests to web-platform-tests.

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

to

==========
Convert appcache video http tests to testharness.js

Cleaning appcache video http tests 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-08-17 12:03:49]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-17 12:06:14]

Description was changed from

==========
Convert appcache video http tests to testharness.js

Cleaning appcache video http tests to use
testharness.js instead of video-test.js.
This will enable to upstream these tests to web-platform-tests.

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

to

==========
Convert appcache video http tests to testharness.js

Cleaning appcache video http tests 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/b9860113bedaeef0f76a8e699ba7551e6bdd0d08
Cr-Commit-Position: refs/heads/master@{#412502}
==========

[commit-bot: I haz the power, 2016-08-17 12:06:16]

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