1230148 - import and uplift streams spec wpt tests

Status: RESOLVED FIXED

(Testing :: web-platform-tests, defect)

Description

[Ben Kelly [:bkelly, not reviewing]]

The streams spec has a nice test suite written in wpt format.  We should import and uplift it to the wpt repo as our first step to implementation.

https://github.com/whatwg/streams/tree/master/reference-implementation/web-platform-tests

The webkit teams has also shown interest in using these tests:

https://github.com/whatwg/streams/issues/411

Comment 1

[Yury Delendik (:yury)]

Some tests trip on "SyntaxError: missing = in const declaration" in `for (const i of [1,2])` statements. bug 449811 shall address that.

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

We could also just change the tests not to use const for now.

Comment 3

[Yury Delendik (:yury)]

WebKit team imported that to the "streams-api" folder (per https://github.com/whatwg/streams/issues/411#issuecomment-162652583), shall we choose the same name?

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

I guess we can keep the "streams-api".  I know from experience we'll end up typing this a lot, though.  I would have preferred just "streams".

In case you haven't worked with out wpt tests before, you can just put them in:

  testing/web-platform/tests/streams-api

I would like to keep the disabled until we actually have some code to test, though.  That can be done with a "disable: true" line in:

  testing/web-platform/meta/streams-api/__dir__.ini

After adding the tests you need to run something like this to teach wpt about the new tests:

  ./mach web-platform-tests --manifest-update streams-api

Comment 5

[Yury Delendik (:yury)]

Attachment: add-streams-wpt.patch

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0eec199091a1

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

Thanks!

Comment 7

[Yury Delendik (:yury)]

Attachment: add-streams-wpt.patch

Comment 8

[Yury Delendik (:yury)]

Attachment: Import and uplift streams spec wpt tests

Comment 9

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8705174 [details] [diff] [review]
Import and uplift streams spec wpt tests

Review of attachment 8705174 [details] [diff] [review]:
-----------------------------------------------------------------

I did not look at the test content in detail since we're basically just importing from the spec.  I agree with disabling for now, but also keeping expected failure meta data.  Lets land this so it can get upstreamed and we can iterate from there.  Thanks! r=me

Comment 10

[Yury Delendik (:yury)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=572ff9913665

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/31a86d5e5ffa

Comment 12

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/31a86d5e5ffa

Comment 13

[Nigel Babu [:nigelb]]

Landing this has lead to an interesting situation.

WPT e10s (3) on linux x64 debug has become extremely unstable. Any idea what's causing this?

See: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&filter-searchStr=a667f0f51aec3d6841635efb6fe10984380a43a3&fromchange=2ee573ab8547&selectedJob=19568537

Comment 14

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Nigel Babu [:nigelb] from comment #13)
> WPT e10s (3) on linux x64 debug has become extremely unstable. Any idea
> what's causing this?

The streams tests are in W(7), not W(3).  Also, they are disabled and you can see in the log they are being skipped.  The only way this landing could affect W(3) is through chunking changes.

Comment 15

[Yury Delendik (:yury)]

(In reply to Ben Kelly [:bkelly] from comment #14)
> (In reply to Nigel Babu [:nigelb] from comment #13)
> > WPT e10s (3) on linux x64 debug has become extremely unstable. Any idea
> > what's causing this?
>
> The only way this landing could affect W(3) is through chunking changes.

I concur with this assumption. I looked at the log files before and after landing -- the /html/dom/reflection-*.html files were in different chunk [W(4) ? and now they are included in W(3)] for some reason and I didn't see we run W(4) anywhere to check we did not fail this test before.

Comment 16

[James Graham [:jgraham]]

The fact that W4 is missing is due to the fact that SETA identified it as a chunk that doesn't fail very often, so we don't run it very often. But it should be run every 10 pushes or so iirc. Of course this whole SETA thing breaks down when chunks change.

I assume the instability is now covered by the other bug?