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

ReadableStream: Add tests for shutting down when readable becomes errored with preventAbort = true #9591

Conversation

Projects
None yet
5 participants
@MattiasBuelens
Copy link
Contributor

MattiasBuelens commented Feb 20, 2018

After looking at the new tests introduced in #5636 and #9573, I found that there was a small mismatch between the tests for forward close propagation and forward error propagation.

For closing, we have 4 tests for the shutdown behavior:

  1. Closing must be propagated forward: shutdown must not occur until the final write completes
  2. Closing must be propagated forward: shutdown must not occur until the final write completes; preventClose = true
  3. Closing must be propagated forward: shutdown must not occur until the final write completes; becomes closed after first write
  4. Closing must be propagated forward: shutdown must not occur until the final write completes; becomes closed after first write; preventClose = true

For erroring, we only have 2:

  1. Errors must be propagated forward: shutdown must not occur until the final write completes
  2. Errors must be propagated forward: abort should not happen until all queued writes complete

This PR reworks the forward error propagation tests, so we now have similar variants:

  1. Errors must be propagated forward: shutdown must not occur until the final write completes
  2. Errors must be propagated forward: shutdown must not occur until the final write completes; preventAbort = true
  3. Errors must be propagated forward: shutdown must not occur until the final write completes; becomes errored after first write
  4. Errors must be propagated forward: shutdown must not occur until the final write completes; becomes errored after first write; preventAbort = true
@w3c-bots

This comment has been minimized.

Copy link

w3c-bots commented Feb 20, 2018

Build PASSED

Started: 2018-02-26 21:53:42
Finished: 2018-02-26 22:03:17

Failing Jobs

  • safari:11.0

Unstable Results

Browser: "Safari 11.0" (failures allowed)

View in: WPT PR Status | TravisCI

Test Subtest Results Messages
/streams/piping/close-propagation-forward.sharedworker.html   TIMEOUT: 1
OK: 9
  close-propagation-forward.js shared worker wrapper file FAIL: 9
ReferenceError: Can't find variable: SharedWorker
@domenic
Copy link
Member

domenic left a comment

LGTM; will let @ricea have a look too if he wants, or just merge if he's busy :)

@ricea

ricea approved these changes Feb 26, 2018

Copy link
Contributor

ricea left a comment

lgtm

return pipePromise;
}).then(() => flushAsyncEvents()).then(() => {
assert_array_equals(ws.events, ['write', 'a', 'write', 'b'],
'all chunks must have been written, but abort must not have happened yet');

This comment has been minimized.

Copy link
@ricea

ricea Feb 26, 2018

Contributor

Since abort will never happen, the word "yet" doesn't belong here.

This comment has been minimized.

Copy link
@MattiasBuelens

MattiasBuelens Feb 26, 2018

Author Contributor

Good catch, this should actually be done for all preventAbort = true and preventClose = true tests. I'll look into this later today.

@ricea ricea merged commit 5154532 into web-platform-tests:master Feb 27, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@MattiasBuelens MattiasBuelens deleted the MattiasBuelens:readable-stream-pipeto-error-propagation-forward-prevent-abort branch Feb 27, 2018

kereliuk added a commit to kereliuk/web-platform-tests that referenced this pull request Feb 28, 2018

ReadableStream piping: Add tests for shutting down when readable beco…
…mes errored with preventAbort = true (web-platform-tests#9591)

For closing, we had 4 tests for the shutdown behavior, but for erroring we only had two.

Rework the forward error propagation tests, so we now have similar variants:

* Errors must be propagated forward: shutdown must not occur until the final write completes
* Errors must be propagated forward: shutdown must not occur until the final write completes; preventAbort = true
* Errors must be propagated forward: shutdown must not occur until the final write completes; becomes errored after first write
* Errors must be propagated forward: shutdown must not occur until the final write completes; becomes errored after first write; preventAbort = true
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.