[whatwg/streams] Tests contain non-zero timers (#548)

Short timers make tests flaky. From the [Testharness.js API Documentation](http://testthewebforward.org/docs/testharness-library.html):

> Note that timeouts generally need to be a few seconds long in order to produce stable results in all test environments.

Long timeouts slow down all developers by making the the tests take longer to run. They also waste resources on automatic testing infrastructure.

Mostly timers are being used to ensure that the implementation never takes some action that it shouldn't. For example (from [writable-streams/start.js](https://github.com/whatwg/streams/blob/master/reference-implementation/to-upstream-wpts/writable-streams/start.js)):

```javascript
promise_test(() => {
  const ws = recordingWritableStream({
    start() {
      return Promise.reject();
    }
  });

  // Wait and verify that write or close aren't called.
  return delay(100)
      .then(() => assert_array_equals(ws.events, [], 'write and close should not be called'));
}, 'underlying sink\'s write or close should not be invoked if the promise returned by start is rejected');
```

This test never[1] flakes on a correct implementation. Suppose hypothetically that there existed an implementation which called `close()` here after a asynchronously performing an action which takes an arbitrary amount of time. Then this test would be flaky on that implementation: it would fail when the action took <100ms, and pass otherwise.

I propose that we make the simplifying assumption that no reasonable implementation will do the wrong thing after a delay. We can then disregard the above hypothetical implementation as being unreasonable, and so consider its false positive on the above test to be acceptable.

I furthermore propose that we consider two times around the event loop to be long enough to see whether  an implementation is going to do the wrong thing or not. My reasoning is that the implementation may run a callback after the user code, which then posts another async callback which finally does the wrong thing.

I think we should have a function `flushAsyncEvents()` which is equivalent to `delay(0).then(delay(0))`. The above test would then look like

```javascript
promise_test(() => {
  const ws = recordingWritableStream({
    start() {
      return Promise.reject();
    }
  });

  // Verify that write or close aren't called.
  return flushAsyncEvents()
      .then(() => assert_array_equals(ws.events, [], 'write and close should not be called'));
}, 'underlying sink\'s write or close should not be invoked if the promise returned by start is rejected');
```


[1] Test frameworks usually have a hard timeout, which means that even passing tests will fail occasionally. The longer a test takes, the more likely it is to hit the hard timeout. This is seen in practice on the Blink layout tests.

-- 
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/issues/548