Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update WritableStream tests #4587

Merged
merged 1 commit into from Jan 25, 2017

Conversation

tyoshino
Copy link
Contributor

Spec change: whatwg/streams#655

@wpt-pr-bot
Copy link
Collaborator

@tyoshino
Copy link
Contributor Author

The spec side change is also still under review.

@tyoshino
Copy link
Contributor Author

Rebased.

The last commit before rebase was: a81b8b3

@tyoshino
Copy link
Contributor Author

Rebased again onto fb0caa1 to sync with the point specified for the submodule in the streams repo.

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after the one passedError/thrownError thing is fixed.

@@ -164,6 +164,30 @@ promise_test(t => {
.then(() => promise_rejects(t, new TypeError(), writer.close(), 'close() should be rejected'));
}, 'when sink\'s write throws an error, the stream should become errored and the promise should reject');

promise_test(t => {
const passedError = new Error('horrible things');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since both of these have error.name === 'Error' the promise_rejects tests here are no good. Let's stick to the error1 + error2 pattern.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Removed passedError and thrownError from all the tests in this file and switched to use error1/error2 in a48c302

@tyoshino
Copy link
Contributor Author

tyoshino commented Jan 25, 2017

Rebased and squashed. The last commit before that was f5c2f75.

- Fixes the test description of one test
- Modified some tests to check promise fulfillment/rejection order
- Added tests to check the order and timing of promise fulfillment/rejection when writer.abort() is called while there's a pending writer.write() or writer.close()
- Added tests to check that writer.releaseLock() doesn't hit assertions when called while there's a pending writer.write() or writer.close()
- Added tests to check an error from which of controller.error(), sink.write()/close() or writer.abort()) is used for rejecting writer.write()/close()

Corresponding spec side change: whatwg/streams#655
@tyoshino
Copy link
Contributor Author

Squashed and updated the commit message. The last commit before this was a48c302.

@tyoshino
Copy link
Contributor Author

Hi Domenic, it looks I need write access to commit. Can I get it or could you please merge this?

@domenic
Copy link
Member

domenic commented Jan 25, 2017

@sideshowbarker can you help give @tyoshino write access for the future? And @ricea while we're at it.

@domenic domenic merged commit 906f481 into web-platform-tests:master Jan 25, 2017
@tyoshino tyoshino deleted the updatewritablestream branch January 25, 2017 06:29
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.

None yet

3 participants