Upstream IDBCursor.continuePrimaryKey test to WPT.

Upstream IDBCursor.continuePrimaryKey test to WPT.

BUG=683463
Review-Url: https://codereview.chromium.org/2657533002
Cr-Commit-Position: refs/heads/master@{#446194}
Committed: https://chromium.googlesource.com/chromium/src/+/cf18468fc7b170fdee94efffd557f17cdcd1086b

[pwnall, 2017-01-24 02:10:21]

The CQ bit was checked by pwnall@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-24 02:11:27]

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

[pwnall, 2017-01-24 02:17:17]

pwnall@chromium.org changed reviewers:
+ jsbell@chromium.org

[pwnall, 2017-01-24 02:17:17]

PTAL?

This is an upstream from storage/indexeddb/cursor-continueprimarykey.html

I didn't remove the test because
storage/indexeddb/resources/cursor-continueprimarykey.js made me think that our
test possibly runs in workers as well. I can upstream the test using .any.js
instead of .html, but AFAIK we don't support running those tests.

WDYT?

[commit-bot: I haz the power, 2017-01-24 03:25:49]

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

[commit-bot: I haz the power, 2017-01-24 03:25:49]

Dry run: This issue passed the CQ dry run.

[jsbell, 2017-01-24 17:20:38]

On 2017/01/24 02:17:17, pwnall wrote:
> PTAL?
> 
> This is an upstream from storage/indexeddb/cursor-continueprimarykey.html
> 
> I didn't remove the test because
> storage/indexeddb/resources/cursor-continueprimarykey.js made me think that
our
> test possibly runs in workers as well. I can upstream the test using .any.js
> instead of .html, but AFAIK we don't support running those tests.
> 
> WDYT?

Searching doesn't show a reference to that script from a worker test - it
probably just copied the style of an existing test. I'd go ahead and remove.

[pwnall, 2017-01-24 19:38:04]

The CQ bit was checked by pwnall@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-24 19:39:02]

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

[pwnall, 2017-01-24 19:39:27]

On 2017/01/24 17:20:38, jsbell wrote:
> On 2017/01/24 02:17:17, pwnall wrote:
> > PTAL?
> > 
> > This is an upstream from storage/indexeddb/cursor-continueprimarykey.html
> > 
> > I didn't remove the test because
> > storage/indexeddb/resources/cursor-continueprimarykey.js made me think that
> our
> > test possibly runs in workers as well. I can upstream the test using .any.js
> > instead of .html, but AFAIK we don't support running those tests.
> > 
> > WDYT?
> 
> Searching doesn't show a reference to that script from a worker test - it
> probably just copied the style of an existing test. I'd go ahead and remove.

Done.
Can you please take a look at the test migration from js-test to testharness,
and see if it is acceptable?

Thank you!

[commit-bot: I haz the power, 2017-01-24 21:52:37]

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

[commit-bot: I haz the power, 2017-01-24 21:52:38]

Dry run: This issue passed the CQ dry run.

[jsbell, 2017-01-24 21:59:49]

lgtm with a couple nits

https://codereview.chromium.org/2657533002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/IndexedDB/idbcursor-continuePrimaryKey.htm
(right):

https://codereview.chromium.org/2657533002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/IndexedDB/idbcursor-continuePrimaryKey.htm:55:
{ call: (cursor) => { cursor.continue(); },
nit: no need for () around single argument => functions

https://codereview.chromium.org/2657533002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/IndexedDB/idbcursor-continuePrimaryKey.htm:103:
txn.oncomplete = verifyContinueCalls;
Needs t.step_func() wrapper

[pwnall, 2017-01-25 19:40:23]

The CQ bit was checked by pwnall@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-25 19:40:50]

Dry run: 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, 2017-01-25 22:09:02]

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

[commit-bot: I haz the power, 2017-01-25 22:09:03]

Dry run: This issue passed the CQ dry run.

[pwnall, 2017-01-26 00:22:06]

jsbell: Thank you very much for the quick feedback!

Should I CQ these tests, or wait for the outcome of
https://github.com/w3c/web-platform-tests/issues/4622 ?

https://codereview.chromium.org/2657533002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/IndexedDB/idbcursor-continuePrimaryKey.htm
(right):

https://codereview.chromium.org/2657533002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/IndexedDB/idbcursor-continuePrimaryKey.htm:55:
{ call: (cursor) => { cursor.continue(); },
On 2017/01/24 21:59:49, jsbell wrote:
> nit: no need for () around single argument => functions

Done.

https://codereview.chromium.org/2657533002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/IndexedDB/idbcursor-continuePrimaryKey.htm:103:
txn.oncomplete = verifyContinueCalls;
On 2017/01/24 21:59:49, jsbell wrote:
> Needs t.step_func() wrapper

Done.

[jsbell, 2017-01-26 01:20:00]

On 2017/01/26 00:22:06, pwnall wrote:
> jsbell: Thank you very much for the quick feedback!
> 
> Should I CQ these tests, or wait for the outcome of
> https://github.com/w3c/web-platform-tests/issues/4622 ?
> 

If you can just make sure the test fails promptly (and doesn't timeout) in
stable w/o the experimental flag, I think it's okay to upstream.

That wpt issue seems to be pointing at some issue with testharness, rather than
an issue with the test.

[pwnall, 2017-01-26 01:29:32]

The CQ bit was checked by pwnall@chromium.org

[pwnall, 2017-01-26 01:29:33]

The patchset sent to the CQ was uploaded after l-g-t-m from jsbell@chromium.org
Link to the patchset: https://codereview.chromium.org/2657533002/#ps40001
(title: "Addressed feedback.")

[commit-bot: I haz the power, 2017-01-26 01:30:15]

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, 2017-01-26 01:36:48]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1485394172991130,
"parent_rev": "cac0db7f354b8187b4932f1131d3701c5197d71b", "commit_rev":
"cf18468fc7b170fdee94efffd557f17cdcd1086b"}

[commit-bot: I haz the power, 2017-01-26 01:37:23]

Description was changed from

==========
Upstream IDBCursor.continuePrimaryKey test to WPT.

BUG=683463
==========

to

==========
Upstream IDBCursor.continuePrimaryKey test to WPT.

BUG=683463

Review-Url: https://codereview.chromium.org/2657533002
Cr-Commit-Position: refs/heads/master@{#446194}
Committed:
https://chromium.googlesource.com/chromium/src/+/cf18468fc7b170fdee94efffd557...
==========

[commit-bot: I haz the power, 2017-01-26 01:37:24]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/cf18468fc7b170fdee94efffd557...

[pwnall, 2017-01-26 02:35:10]

On 2017/01/26 01:20:00, jsbell wrote:
> On 2017/01/26 00:22:06, pwnall wrote:
> > jsbell: Thank you very much for the quick feedback!
> > 
> > Should I CQ these tests, or wait for the outcome of
> > https://github.com/w3c/web-platform-tests/issues/4622 ?
> > 
> 
> If you can just make sure the test fails promptly (and doesn't timeout) in
> stable w/o the experimental flag, I think it's okay to upstream.

Closing the loop -- the test completes immediately without any console output.
The document output includes "Harness: the test ran to completion."

[jeffcarp, 2017-01-26 05:31:57]

On 2017/01/26 at 02:35:10, pwnall wrote:
> On 2017/01/26 01:20:00, jsbell wrote:
> > On 2017/01/26 00:22:06, pwnall wrote:
> > > jsbell: Thank you very much for the quick feedback!
> > > 
> > > Should I CQ these tests, or wait for the outcome of
> > > https://github.com/w3c/web-platform-tests/issues/4622 ?
> > > 
> > 
> > If you can just make sure the test fails promptly (and doesn't timeout) in
> > stable w/o the experimental flag, I think it's okay to upstream.
> 
> Closing the loop -- the test completes immediately without any console output.
The document output includes "Harness: the test ran to completion."

Upstreamed in https://github.com/w3c/web-platform-tests/pull/4631

(also: would it be useful if the bot posted status notifications like this to
the CL?)

[jsbell, 2017-01-26 18:49:03]

On 2017/01/26 05:31:57, jeffcarp wrote:

> Upstreamed in https://github.com/w3c/web-platform-tests/pull/4631
> 
> (also: would it be useful if the bot posted status notifications like this to
> the CL?)

Yes!

[pwnall, 2017-01-27 02:21:12]

On 2017/01/26 18:49:03, jsbell wrote:
> On 2017/01/26 05:31:57, jeffcarp wrote:
> 
> > Upstreamed in https://github.com/w3c/web-platform-tests/pull/4631
> > 
> > (also: would it be useful if the bot posted status notifications like this
to
> > the CL?)
> 
> Yes!

An enthusiastic "Yes!" from me too.