Skip to content

Upstream transform streams tests#8090

Merged
ricea merged 4 commits intoweb-platform-tests:masterfrom
ricea:transform-streams-import-tests
Nov 9, 2017
Merged

Upstream transform streams tests#8090
ricea merged 4 commits intoweb-platform-tests:masterfrom
ricea:transform-streams-import-tests

Conversation

@ricea
Copy link
Contributor

@ricea ricea commented Nov 7, 2017

Copy the tests for transform streams from the whatwg/streams repository.

TransformStream has now been added to the Streams Standard so it seems
reasonable to move these tests to the web-platform-tests repository. A follow-on
change will remove them from the whatwg/streams repository.

See whatwg/streams#851.

There are no changes to the test Javascript files. They have all been
reviewed in their old location. All the HTML files have been regenerated by
generate-test-wrappers.js.

recording-streams.js has an additional "recordingTransformStream" function which
is used in some of the new tests. test-utils.js has an additional
"constructorThrowsForAll" test helper.

@ghost
Copy link

ghost commented Nov 7, 2017

Build PASSED

Started: 2017-11-09 08:39:34
Finished: 2017-11-09 09:23:30

Failing Jobs

  • safari:10.0
  • MicrosoftEdge:14.14393

View more information about this build on:

@ricea
Copy link
Contributor Author

ricea commented Nov 7, 2017

Failed due to lint errors:

streams/transform-streams/general.js:125: setTimeout used; step_timeout should typically be used instead (SET TIMEOUT)
ERROR:lint:streams/transform-streams/general.js:125: setTimeout used; step_timeout should typically be used instead (SET TIMEOUT)
streams/transform-streams/general.js:147: setTimeout used; step_timeout should typically be used instead (SET TIMEOUT)
ERROR:lint:streams/transform-streams/general.js:147: setTimeout used; step_timeout should typically be used instead (SET TIMEOUT)
streams/transform-streams/general.js:148: setTimeout used; step_timeout should typically be used instead (SET TIMEOUT)
ERROR:lint:streams/transform-streams/general.js:148: setTimeout used; step_timeout should typically be used instead (SET TIMEOUT)
streams/transform-streams/general.js:244: setTimeout used; step_timeout should typically be used instead (SET TIMEOUT)
ERROR:lint:streams/transform-streams/general.js:244: setTimeout used; step_timeout should typically be used instead (SET TIMEOUT)
streams/transform-streams/general.js:245: setTimeout used; step_timeout should typically be used instead (SET TIMEOUT)
ERROR:lint:streams/transform-streams/general.js:245: setTimeout used; step_timeout should typically be used instead (SET TIMEOUT)
streams/transform-streams/general.js:246: setTimeout used; step_timeout should typically be used instead (SET TIMEOUT)
ERROR:lint:streams/transform-streams/general.js:246: setTimeout used; step_timeout should typically be used instead (SET TIMEOUT)

I think I will probably fix this over at whatwg/streams so we can review the fix in isolation, then merge it back here.

@domenic
Copy link
Member

domenic commented Nov 7, 2017

Hmm, I think I'd rather fix them here, or at least ensure we don't merge to whatwg/streams before verifying here that they pass lint.

Anyway, I'll approve this now so that when we do get lint passing, we can merge regardless of timezones. The substance of these have all seen review over in whatwg/streams.

@ricea
Copy link
Contributor Author

ricea commented Nov 8, 2017

I did the minimal change (replacing setTimeout with delay), so we can proceed with upstreaming the tests.

Once we've done the move, I'd like to follow-up with rewriting those tests to not use non-zero delays.

@ricea
Copy link
Contributor Author

ricea commented Nov 8, 2017

The Firefox Nightly stability test is failing on Travis. It looks like an issue with the Firefox nightly rather than the tests themselves.

ricea added 3 commits November 9, 2017 14:46
Copy the tests for transform streams from the whatwg/streams repository.

TransformStream has now been added to the Streams Standard so it seems
reasonable to move these tests to the web-platform-tests repository. A follow-on
change will remove them from the whatwg/streams repository.

There are no changes to the test Javascript files. They have all been
reviewed in their old location. All the HTML files have been regenerated by
generate-test-wrappers.js.

recording-streams.js has an additional "recordingTransformStream" function which
is used in some of the new tests. test-utils.js has an additional
"constructorThrowsForAll" test helper.
The test added in whatwg/streams#852 was missing
from the previous commit. Add it.
@ricea ricea force-pushed the transform-streams-import-tests branch from 7fa7e2c to a4e177c Compare November 9, 2017 05:46
@ricea
Copy link
Contributor Author

ricea commented Nov 9, 2017

The Firefox Nightly stability test is still broken today. I will try to get it working on my own machine.

@ricea
Copy link
Contributor Author

ricea commented Nov 9, 2017

I've looked at the other stability tests. Safari and Edge are both failing with infrastructure errors. The Chrome test is complaining that lipfuzz.html has an error. But it just reports "TransformStream is not defined" like all the other tests. From a stability point of view, it's 100% stable.

My theory for what is happening with the Firefox bot is that the sheer volume of errors it produces is hitting some limit which prevents the test from passing. I could probably mitigate this by splitting into one PR per file, but I'd rather not do that. It there someone I can ask to get an override for the CI build failure?

Because the stringification of [] and [''] is the same, the test name
"testing """ was used twice. Include input array length in the test name
to make them unique.
@ricea
Copy link
Contributor Author

ricea commented Nov 9, 2017

Okay, so there was a problem with lipfuzz.js after all.

@ricea ricea merged commit b855fd8 into web-platform-tests:master Nov 9, 2017
@ricea ricea deleted the transform-streams-import-tests branch November 9, 2017 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants