Re: [whatwg/streams] WPT-ify most of the remaining tape tests (#604)

ricea commented on this pull request.

I've reviewed all the diffs and everything looks good.

Please take a look at my comments.

> +
+promise_test(t => {
+  const sizes = [NaN, -Infinity, Infinity, -1];
+  return Promise.all(sizes.map(size => {
+    let theError;
+    const ws = new WritableStream({}, {
+      size() {
+        return size;
+      },
+      highWaterMark: 5
+    });
+
+    const writer = ws.getWriter();
+
+    return Promise.all([
+      promise_rejects(t, new RangeError(), writer.write('a').catch(r => {

Use promise_rejects is just making this more complicated and harder to understand, so I suggest doing something closer to the original. For example (untested):

```javascript
return writer.write('a').then(
    () => assert_unreached('write should not fulfill');
    r => {
      assert_equals(r.name, 'RangeError', `write should reject with a RangeError for ${size}`);
      let theError = r;
      return writer.closed.then(
          () => assert_unreached('closed should not fulfill'),
          e => assert_equals(e, theError, `closed should reject with the same error for ${size}`));
      ));
```

This removes the implicit test that the write() promise fulfills before the closed promise.  If you are feeling energetic you could add an explicit test of that fact.

> +
+const thrownError = new Error('bad things are happening!');
+thrownError.name = 'error1';
+
+promise_test(t => {
+  const results = [];
+
+  const ts = new TransformStream({
+    transform() {
+      throw thrownError;
+    }
+  });
+
+  const reader = ts.readable.getReader();
+
+  results.push(promise_rejects(t, thrownError, reader.read(),

I agree with @domenic that the |results| array doesn't really help with readability.

> +  });
+
+  const reader = ts.readable.getReader();
+
+  results.push(promise_rejects(t, thrownError, reader.read(),
+                               'readable\'s read should reject with the thrown error'));
+
+  results.push(promise_rejects(t, thrownError, reader.closed,
+                               'readable\'s closed should be rejected with the thrown error'));
+
+  const writer = ts.writable.getWriter();
+
+  results.push(promise_rejects(t, thrownError, writer.closed,
+                               'writable\'s closed should be rejected with the thrown error'));
+
+  writer.write('a');

It's odd that this call to write() is just sitting here with no obvious purpose. I know you didn't add it, and I'm not asking you to remove it. I'm just making a random observation.

-- 
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/604#pullrequestreview-7780361