Upstream service worker prototype test to WPT

Upstream service worker prototype test to WPT

Recent improvements to the Web Platform Test's `idlharness.js` utility
introduced tests for immutable prototype objects [1] [2].

Update WPT's existing interface tests (and Chromium "expectations"
files) to use `idlharness.js` and remove the Service-Worker-specific
`interfaces.js` utility (which is far less thorough). Refactor tests
across two files to avoid duplicating sub-test names within the same
test.

[1] https://github.com/w3c/web-platform-tests/commit/55f606ec31b88e7ab7689c2ab3e630cfd2348fae
[2] https://github.com/w3c/web-platform-tests/commit/13b0e2d8d609603b0230ba950404ea21c149f0db

BUG=688116
R=falken@chromium.org

[mike3, 2017-06-12 19:26:03]

Mind taking a look, Matt?

[falken, 2017-06-13 06:03:42]

falken@chromium.org changed reviewers:
+ yukishiino@chromium.org

[falken, 2017-06-13 06:03:43]

I think I might not be the best to review this. Adding yukishiino@ who reviewed
the original https://codereview.chromium.org/2452073002.

https://codereview.chromium.org/2933913002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/interfaces-idls.js
(right):

https://codereview.chromium.org/2933913002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/interfaces-idls.js:14:
`;
how do you decide what to put in idls.untested?

https://codereview.chromium.org/2933913002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/interfaces-idls.js:130:
`;
Do you get idls.tested by copying everything from
https://w3c.github.io/ServiceWorker/ that has Exposed=ServiceWorker in it?

[Yuki, 2017-06-13 08:33:29]

yukishiino@chromium.org changed reviewers:
+ domenic@chromium.org

[Yuki, 2017-06-13 08:33:30]

This only changes WPTs and I'm not a right reviewer, neither.  domenic@, would
you mind reviewing this CL or recommend someone else?

Just in case, I'm personally fine with this change.

[falken, 2017-06-13 08:42:54]

On 2017/06/13 08:33:30, Yuki wrote:
> This only changes WPTs and I'm not a right reviewer, neither.  domenic@, would
> you mind reviewing this CL or recommend someone else?
> 
> Just in case, I'm personally fine with this change.

Thanks! It's not WPT that I'm wrong for, it's that I don't really understand the
original test: https://codereview.chromium.org/2452073002 since I'm not an IDL
expert.

I don't see how WPT changes things. You can consider the new tests to be inside
LayoutTests like any other test.

[Yuki, 2017-06-13 09:09:36]

On 2017/06/13 08:42:54, falken wrote:
> On 2017/06/13 08:33:30, Yuki wrote:
> > This only changes WPTs and I'm not a right reviewer, neither.  domenic@,
would
> > you mind reviewing this CL or recommend someone else?
> > 
> > Just in case, I'm personally fine with this change.
> 
> Thanks! It's not WPT that I'm wrong for, it's that I don't really understand
the
> original test: https://codereview.chromium.org/2452073002 since I'm not an IDL
> expert.
> 
> I don't see how WPT changes things. You can consider the new tests to be
inside
> LayoutTests like any other test.

Ah, I see.  Now I've reviewed about "immutable prototype".  I took a look at
https://github.com/w3c/web-platform-tests/commit/55f606ec31b88e7ab7689c2ab3e6...
and found that, IIUC, this is NOT testing all the inherited interfaces, so there
is room to improve.  The original test (Chromium-local test) is testing all the
inherited interfaces.

Having said that, given that WPTs are now available to test immutable prototype
exotic objects, then this CL looks good to use an existing WPT framework.

My opinion is a) this CL lgtm, and b) we should improve WPTs to test all the
inherited interfaces (in a follow-up CL).

c.f.
https://heycam.github.io/webidl/#interface-prototype-object
The second last paragraph describes "immutable prototype exotic object", and
https://heycam.github.io/webidl/#dfn-inherited-interfaces
is the definition of "inherited interface".

[domenic, 2017-06-13 15:49:34]

NOT l-g-t-m due to removed coverage.

https://codereview.chromium.org/2933913002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/immutable-prototype-serviceworker.html
(left):

https://codereview.chromium.org/2933913002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/serviceworker/immutable-prototype-serviceworker.html:1:
<!DOCTYPE html>
We should not delete this file as doing so removes coverage. We could upstream
it instead.

[mike3, 2017-06-30 22:12:51]

The test named `immutable-prototype-serviceworker.html` concerned details that
I would have expected to be covered by WPT's `idlharness.js` library. However,
that library did not previously assert expectations regarding immutable
prototype objects. I thought that instead of simply migrating the test, a
better solution would be to extend `idlharness.js`. This would improve coverage
for every relevant platform object/prototype object that was already under
test. I submitted [1] and [2] to implement this.

With those patches in place (and by introducing a dedicated WebIDL test for an
instance of ServiceWorkerGlobalScope), I believe this patch makes
`immutable-prototype-serviceworker.html` redundant. At Domenic's request, I
have updated this patch to migrate the test to WPT (instead of removing it
outright).

[1] https://github.com/w3c/web-platform-tests/pull/5788
[2] https://github.com/w3c/web-platform-tests/pull/6262

[mike3, 2017-06-30 22:17:43]

Description was changed from

==========
Upstream service worker prototype test to WPT

A recent improvement to the Web Platform Test's `idlharness.js` utility
introduced tests for immutable prototype objects [1].

Update WPT's existing interface tests (and Chromium "expectations"
files) to use `idlharness.js` and remove the Service-Worker-specific
`interfaces.js` utility (which is far less thorough). Refactor tests
across two files to avoid duplicating sub-test names within the same
test. Remove the Chromium test dedicated to prototype immutability
because this detail is now asserted by `idlharness.js`.

[1]
https://github.com/w3c/web-platform-tests/commit/55f606ec31b88e7ab7689c2ab3e6...

BUG=688116
R=falken@chromium.org
==========

to

==========
Upstream service worker prototype test to WPT

Recent improvements to the Web Platform Test's `idlharness.js` utility
introduced tests for immutable prototype objects [1] [2].

Update WPT's existing interface tests (and Chromium "expectations"
files) to use `idlharness.js` and remove the Service-Worker-specific
`interfaces.js` utility (which is far less thorough). Refactor tests
across two files to avoid duplicating sub-test names within the same
test.

[1]
https://github.com/w3c/web-platform-tests/commit/55f606ec31b88e7ab7689c2ab3e6...
[2]
https://github.com/w3c/web-platform-tests/commit/13b0e2d8d609603b0230ba950404...

BUG=688116
R=falken@chromium.org
==========

[domenic, 2017-06-30 22:43:00]

On 2017/06/30 at 22:12:51, mike wrote:
> The test named `immutable-prototype-serviceworker.html` concerned details that
> I would have expected to be covered by WPT's `idlharness.js` library. However,
> that library did not previously assert expectations regarding immutable
> prototype objects. I thought that instead of simply migrating the test, a
> better solution would be to extend `idlharness.js`. This would improve
coverage
> for every relevant platform object/prototype object that was already under
> test. I submitted [1] and [2] to implement this.
> 
> With those patches in place (and by introducing a dedicated WebIDL test for an
> instance of ServiceWorkerGlobalScope), I believe this patch makes
> `immutable-prototype-serviceworker.html` redundant. At Domenic's request, I
> have updated this patch to migrate the test to WPT (instead of removing it
> outright).
> 
> [1] https://github.com/w3c/web-platform-tests/pull/5788
> [2] https://github.com/w3c/web-platform-tests/pull/6262

To be clear, it's not redundant, as outlined in
https://github.com/w3c/web-platform-tests/pull/6262#pullrequestreview-44669998
("But it is not testing the rest of the objects in the prototype chain...").

Nevertheless, lgtm since this is no longer removing coverage.

[falken, 2017-07-30 14:18:05]

Hi Mike, this lgtm to me too. Shall we put it in the CQ? In case it doesn't
apply to ToT and you aren't working on it anymore, any objection to me fixing
conflicts and reuploading it?

[falken, 2017-08-02 06:06:10]

The CQ bit was checked by falken@chromium.org

[falken, 2017-08-02 06:06:11]

The patchset sent to the CQ was uploaded after l-g-t-m from
yukishiino@chromium.org
Link to the patchset: https://codereview.chromium.org/2933913002/#ps20001
(title: "Migrate test for immutable prototype")

[commit-bot: I haz the power, 2017-08-02 06:06:19]

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-08-02 07:19:29]

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

[commit-bot: I haz the power, 2017-08-02 07:19:30]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[falken, 2017-08-03 02:36:19]

Rebaselined and reuploaded at https://chromium-review.googlesource.com/c/598829

[badaczanala, 2017-08-03 07:12:48]

badaczanala@gmail.com changed reviewers:
+ badaczanala@gmail.com

[badaczanala, 2017-08-03 07:12:49]

lgtm