Re: [whatwg/streams] Port writable stream abort tests to wpt (#540)

domenic requested changes on this pull request.

In general very nice, and the porting work is super-appreciated.  The main things are using promise_rejects and recordingWritableStream to clean up the tests.

>  }, 'Aborting a WritableStream should cause the writer\'s fulfilled ready promise to reset to a rejected one');
+
+function promise_fulfills(expectedValue, promise, msg) {

This is not necessary. Inside your promise_test, just do

```js
return promise.then(value => {
  assert_equals(value, expectedValue, msg);
});
```

the test will fail if it returns a rejected promise.

> +  const writer = ws.getWriter();
+  writer.releaseLock();
+
+  const abortPromise = writer.abort();
+  return abortPromise.then(() => {
+    throw new Error('abortPromise fulfilled unexpectedly');
+  },
+  r => {
+    assert_equals(r.constructor.toString(), TypeError.toString(), 'abort() should reject with a TypeError');
+  });
+}, 'abort() on a released writer rejects');
+
+promise_test(() => {
+  const ws = new WritableStream({
+    write() {
+      throw new Error('Unexpected write() call');

It's a decent bit of work, but I think these sorts of tests would be improved by using the recordingWritableStream, and then checking the recorded events in a final `.then` block. An example: https://github.com/whatwg/streams/blob/ff1d3c9af8a01f4b03a508281844b72a92b533c7/reference-implementation/to-upstream-wpts/writable-streams/close.js#L65-L81

> +  });
+
+  return delay(0).then(() => {
+    const writer = ws.getWriter();
+
+    writer.abort();
+    writer.write(1);
+    writer.write(2);
+  })
+  .then(() => delay(100));
+}, 'Aborting a WritableStream immediately prevents future writes');
+
+promise_test(() => {
+  let writeCount = 0;
+
+  const ws = new WritableStream({

Another good example of where recordingWritableStream would help you. You wouldn't need writeCount or any asserts, just check the events in a final `.then` block.

> +      () => { throw new Error('closing should not succeed'); },
+      r => assert_equals(r.constructor.toString(), TypeError.toString(), 'closing should reject with the given reason')
+    ),
+    writer.abort().then(
+      () => { throw new Error('aborting a second time should not succeed'); },
+      r => assert_equals(r.constructor.toString(), TypeError.toString(), 'aborting a second time should reject ' +
+        'with the given reason')
+    ),
+    writer.closed.then(
+      () => { throw new Error('closed fulfilled unexpectedly'); },
+      r => assert_equals(r.constructor.toString(), TypeError.toString(), 'closed should reject with a TypeError')
+    )
+  ]);
+}, 'Aborting a WritableStream puts it in an errored state, with stored error equal to the abort reason');
+
+test(() => {

This will need to be a promise_test when it moves to use assert_promise_rejects

> +
+  return writer.closed.then(
+    () => { throw new Error('the stream should not close successfully'); },
+    r => {
+      assert_equals(r.constructor.toString(), TypeError.toString(), 'the stream should be errored with a TypeError');
+    }
+  );
+}, 'Closing a WritableStream and aborting it while it closes causes the stream to error');
+
+test(() => {
+  const ws = new WritableStream();
+  const writer = ws.getWriter();
+
+  writer.close();
+
+  setTimeout(() => {

Use promise_test and delay()

-- 
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/540#pullrequestreview-4352595