Flaky stoppable tests
#6963
Assignees
Comments
glasser
added a commit
that referenced
this issue
Dec 7, 2022
We've had a lot of issues where `stoppable` tests are flaky because they depend on measuring the time that something takes and hoping it's relatively close to a particular timeout that's supposed to be "controlling" the observed behavior. This PR refactors the internal Stoppable object so that instead of taking a millisecond timeout value, it takes a DOM-style AbortSignal. This object is only used by ApolloServerPluginDrainHttpServer; we move the setTimeout from inside Stoppable into that plugin, which now creates an AbortController polyfilled from `node-abort-controller` (like we already are in the usage reporting plugin; note that once we drop Node v14 support we can switch to the built-in implementation). Now the Stoppable test can control the grace period directly via AbortSignals rather than indirectly based on timeouts. This lets us drop all of the time measurements from the test. There still are some delays but they are just of the form "wait some time and double-check that nothing happened": the worst case scenario here is that a test passes that should fail because the thing would have happened if you'd waited slightly longer, but there shouldn't be any spurious failures. Fixes #6963.
glasser
added a commit
that referenced
this issue
Dec 7, 2022
We've had a lot of issues where `stoppable` tests are flaky because they depend on measuring the time that something takes and hoping it's relatively close to a particular timeout that's supposed to be "controlling" the observed behavior. This PR refactors the internal Stoppable object so that instead of taking a millisecond timeout value, it takes a DOM-style AbortSignal. This object is only used by ApolloServerPluginDrainHttpServer; we move the setTimeout from inside Stoppable into that plugin, which now creates an AbortController polyfilled from `node-abort-controller` (like we already are in the usage reporting plugin; note that once we drop Node v14 support we can switch to the built-in implementation). Now the Stoppable test can control the grace period directly via AbortSignals rather than indirectly based on timeouts. This lets us drop all of the time measurements from the test. There still are some delays but they are just of the form "wait some time and double-check that nothing happened": the worst case scenario here is that a test passes that should fail because the thing would have happened if you'd waited slightly longer, but there shouldn't be any spurious failures. Fixes #6963.
glasser
added a commit
that referenced
this issue
Dec 8, 2022
We've had a lot of issues where `stoppable` tests are flaky because they depend on measuring the time that something takes and hoping it's relatively close to a particular timeout that's supposed to be "controlling" the observed behavior. This PR refactors the internal Stoppable object so that instead of taking a millisecond timeout value, it takes a DOM-style AbortSignal. This object is only used by ApolloServerPluginDrainHttpServer; we move the setTimeout from inside Stoppable into that plugin, which now creates an AbortController polyfilled from `node-abort-controller` (like we already are in the usage reporting plugin; note that once we drop Node v14 support we can switch to the built-in implementation). Now the Stoppable test can control the grace period directly via AbortSignals rather than indirectly based on timeouts. This lets us drop all of the time measurements from the test. There still are some delays but they are just of the form "wait some time and double-check that nothing happened": the worst case scenario here is that a test passes that should fail because the thing would have happened if you'd waited slightly longer, but there shouldn't be any spurious failures. Fixes #6963. Co-authored-by: Trevor Scheer <trevor.scheer@gmail.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Investigate flaky tests in
packages/server/src/__tests__/plugin/drainHttpServer/stoppable.test.tsPossibly resolvable with mocked timers.
The text was updated successfully, but these errors were encountered: