Re: [whatwg/streams] WritableStream abort logic clean up (#655)

tyoshino commented on this pull request.



> +  }).then(() => {
+    assert_array_equals(events, [], 'writePromise, abortPromise and writer.closed must be not rejected yet');
+
+    rejectWrite(error2);
+
+    return Promise.all([
+      promise_rejects(t, error2, writePromise,
+                      'writePromise must reject with the error returned from the sink\'s write method'),
+      promise_rejects(t, error2, abortPromise,
+                      'abortPromise must reject with the error returned from the sink\'s write method'),
+      promise_rejects(t, error2, writer.closed,
+                      'writer.closed must reject with the error returned from the sink\'s write method'),
+      flushAsyncEvents()
+    ]);
+  }).then(() => {
+    const writePromise3 = writer.write('a');

Yeah, added.

> +        events, [],
+        'writePromise, abortPromise and writer.closed must be not fulfilled/rejected yet even after '
+            + 'controller.error() call');
+
+    resolveWrite();
+
+    return Promise.all([
+      writePromise,
+      promise_rejects(t, error2, abortPromise,
+                      'abortPromise must reject with the error passed to the controller\'s error method'),
+      promise_rejects(t, error2, writer.closed,
+                      'writer.closed must reject with the error passed to the controller\'s error method'),
+      flushAsyncEvents()
+    ]);
+  }).then(() => {
+    const writePromise4 = writer.write('a');

Done

> +    });
+
+    abortPromise = writer.abort(error1);
+    abortPromise.catch(() => {
+      events.push('abortPromise');
+    });
+
+    const writePromise2 = writer.write('a');
+
+    return Promise.all([
+      promise_rejects(t, new TypeError(), writePromise2, 'writePromise2 must reject with an error indicating abort'),
+      promise_rejects(t, new TypeError(), writer.ready, 'writer.ready must reject with an error indicating abort'),
+      flushAsyncEvents()
+    ]);
+  }).then(() => {
+    assert_array_equals(events, [], 'writePromise, abortPromise and writer.closed must be not fulfilled/rejected yet');

Done

> +
+    controller.error(error2);
+
+    const writePromise2 = writer.write('a');
+
+    return Promise.all([
+      promise_rejects(t, new TypeError(), writePromise2,
+                      'writePromise2 must reject with an error indicating the stream has already been errored'),
+      promise_rejects(t, error2, writer.ready,
+                      'writer.ready must reject with the error passed to the controller\'s error method'),
+      flushAsyncEvents()
+    ]);
+  }).then(() => {
+    assert_array_equals(events, [], 'writePromise and writer.closed must be rejected yet');
+
+    abortPromise = writer.abort(error1);

Thanks for finding this. Yes, it's important. Actually, in this test case, abort() fails immediately since it checks the state.

> +      events.push('writePromise');
+    });
+
+    controller.error(error2);
+
+    const writePromise2 = writer.write('a');
+
+    return Promise.all([
+      promise_rejects(t, new TypeError(), writePromise2,
+                      'writePromise2 must reject with an error indicating the stream has already been errored'),
+      promise_rejects(t, error2, writer.ready,
+                      'writer.ready must reject with the error passed to the controller\'s error method'),
+      flushAsyncEvents()
+    ]);
+  }).then(() => {
+    assert_array_equals(events, [], 'writePromise and writer.closed must be rejected yet');

Fixed. Also changed the phrase to "fulfilled/rejected". writePromise will fulfill in this test case.

> @@ -213,4 +219,96 @@ promise_test(() => {
   });
 }, 'the promise returned by async abort during close should resolve');
 
+// Though the order of the promises is not important, we're checking it for interoperability. We can change the order

Thanks. Updated.

> @@ -109,6 +109,25 @@ function IsWritableStreamLocked(stream) {
   return true;
 }
 
+function WritableStreamEnsureReadyPromiseRejectedWith(stream, error, isPending) {

Good catch. Fixed.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/streams/pull/655