TCK: Receptacle#expectError timeout approach #451
Comments
Scottmitch
added a commit
to Scottmitch/reactive-streams-jvm
that referenced
this issue
Apr 18, 2019
Motivation: `defaultTimeoutMillis` is typically used as a "max time to wait until an expected event occurs". For example this value is used as the argument to a `ArrayBlockingQueue#poll(timeout)` [1][2][3] method call and therefore when things work as expected you rarely wait this full duration. However [Receptacle#expectError](/~https://github.com/reactive-streams/reactive-streams-jvm/blob/master/tck/src/main/java/org/reactivestreams/tck/TestEnvironment.java#L1030) does a `Thread.sleep(..)` and then checks if there is something present in the error list (which is a [CopyOnWriteArrayList](/~https://github.com/reactive-streams/reactive-streams-jvm/blob/master/tck/src/main/java/org/reactivestreams/tck/TestEnvironment.java#L45)). The value for the sleep is typically derived from [defaultTimeoutMillis](/~https://github.com/reactive-streams/reactive-streams-jvm/blob/master/tck/src/main/java/org/reactivestreams/tck/TestEnvironment.java#L521-L526) which leads to unconditional delays. This makes it challenging to use `defaultTimeoutMillis` in tests to specify "maximum amount of time to wait until some event occurs". The impacts are if `defaultTimeoutMillis` is set very small you are more likely to see timeouts (especially on over utilized CI servers), but if `defaultTimeoutMillis` the tests take a long time to complete. [1] /~https://github.com/reactive-streams/reactive-streams-jvm/blob/master/tck/src/main/java/org/reactivestreams/tck/TestEnvironment.java#L1019 [2] /~https://github.com/reactive-streams/reactive-streams-jvm/blob/master/tck/src/main/java/org/reactivestreams/tck/TestEnvironment.java#L978 [3] /~https://github.com/reactive-streams/reactive-streams-jvm/blob/master/tck/src/main/java/org/reactivestreams/tck/TestEnvironment.java#L990 Modifications: - TestEnvironment now supports an additional poll timeout parameter to be used when there is no explicit async event to trigger off of. Result: TestEnvironment's defaultTimeoutMillis can be used as "max time until some event occurs" where defaultPollTimeoutMillis provides the polling interval to check for an event if there is no explicit async event to trigger off of. Fixes reactive-streams#451
Scottmitch
added a commit
to Scottmitch/reactive-streams-jvm
that referenced
this issue
Apr 18, 2019
Motivation: `defaultTimeoutMillis` is typically used as a "max time to wait until an expected event occurs". For example this value is used as the argument to a `ArrayBlockingQueue#poll(timeout)` [1][2][3] method call and therefore when things work as expected you rarely wait this full duration. However [Receptacle#expectError](/~https://github.com/reactive-streams/reactive-streams-jvm/blob/master/tck/src/main/java/org/reactivestreams/tck/TestEnvironment.java#L1030) does a `Thread.sleep(..)` and then checks if there is something present in the error list (which is a [CopyOnWriteArrayList](/~https://github.com/reactive-streams/reactive-streams-jvm/blob/master/tck/src/main/java/org/reactivestreams/tck/TestEnvironment.java#L45)). The value for the sleep is typically derived from [defaultTimeoutMillis](/~https://github.com/reactive-streams/reactive-streams-jvm/blob/master/tck/src/main/java/org/reactivestreams/tck/TestEnvironment.java#L521-L526) which leads to unconditional delays. This makes it challenging to use `defaultTimeoutMillis` in tests to specify "maximum amount of time to wait until some event occurs". The impacts are if `defaultTimeoutMillis` is set very small you are more likely to see timeouts (especially on over utilized CI servers), but if `defaultTimeoutMillis` the tests take a long time to complete. [1] /~https://github.com/reactive-streams/reactive-streams-jvm/blob/master/tck/src/main/java/org/reactivestreams/tck/TestEnvironment.java#L1019 [2] /~https://github.com/reactive-streams/reactive-streams-jvm/blob/master/tck/src/main/java/org/reactivestreams/tck/TestEnvironment.java#L978 [3] /~https://github.com/reactive-streams/reactive-streams-jvm/blob/master/tck/src/main/java/org/reactivestreams/tck/TestEnvironment.java#L990 Modifications: - TestEnvironment now supports an additional poll timeout parameter to be used when there is no explicit async event to trigger off of. Result: TestEnvironment's defaultTimeoutMillis can be used as "max time until some event occurs" where defaultPollTimeoutMillis provides the polling interval to check for an event if there is no explicit async event to trigger off of. Fixes reactive-streams#451
|
PR #452 submitted to address this issue. |
viktorklang
pushed a commit
that referenced
this issue
May 6, 2019
* TCK: Receptacle#expectError timeout approach Motivation: `defaultTimeoutMillis` is typically used as a "max time to wait until an expected event occurs". For example this value is used as the argument to a `ArrayBlockingQueue#poll(timeout)` [1][2][3] method call and therefore when things work as expected you rarely wait this full duration. However [Receptacle#expectError](/~https://github.com/reactive-streams/reactive-streams-jvm/blob/master/tck/src/main/java/org/reactivestreams/tck/TestEnvironment.java#L1030) does a `Thread.sleep(..)` and then checks if there is something present in the error list (which is a [CopyOnWriteArrayList](/~https://github.com/reactive-streams/reactive-streams-jvm/blob/master/tck/src/main/java/org/reactivestreams/tck/TestEnvironment.java#L45)). The value for the sleep is typically derived from [defaultTimeoutMillis](/~https://github.com/reactive-streams/reactive-streams-jvm/blob/master/tck/src/main/java/org/reactivestreams/tck/TestEnvironment.java#L521-L526) which leads to unconditional delays. This makes it challenging to use `defaultTimeoutMillis` in tests to specify "maximum amount of time to wait until some event occurs". The impacts are if `defaultTimeoutMillis` is set very small you are more likely to see timeouts (especially on over utilized CI servers), but if `defaultTimeoutMillis` the tests take a long time to complete. [1] /~https://github.com/reactive-streams/reactive-streams-jvm/blob/master/tck/src/main/java/org/reactivestreams/tck/TestEnvironment.java#L1019 [2] /~https://github.com/reactive-streams/reactive-streams-jvm/blob/master/tck/src/main/java/org/reactivestreams/tck/TestEnvironment.java#L978 [3] /~https://github.com/reactive-streams/reactive-streams-jvm/blob/master/tck/src/main/java/org/reactivestreams/tck/TestEnvironment.java#L990 Modifications: - TestEnvironment now supports an additional poll timeout parameter to be used when there is no explicit async event to trigger off of. Result: TestEnvironment's defaultTimeoutMillis can be used as "max time until some event occurs" where defaultPollTimeoutMillis provides the polling interval to check for an event if there is no explicit async event to trigger off of. Fixes #451 * review round 1 * revert FlowPublisherVerification -> PublisherVerification change in doc * review comments from ktoso
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
defaultTimeoutMillisis typically used as a "max time to wait until an expected event occurs". For example this value is used as the argument to aArrayBlockingQueue#poll(timeout)[1][2][3] method call and therefore when things work as expected you rarely wait this full duration. However Receptacle#expectError does aThread.sleep(..)and then checks if there is something present in the error list (which is a CopyOnWriteArrayList). The value for the sleep is typically derived from defaultTimeoutMillis which leads to unconditional delays. This makes it challenging to usedefaultTimeoutMillisin tests to specify "maximum amount of time to wait until some event occurs". The impacts are ifdefaultTimeoutMillisis set very small you are more likely to see timeouts (especially on over utilized CI servers), but if is largedefaultTimeoutMillisthe tests take a long time to complete.[1] /~https://github.com/reactive-streams/reactive-streams-jvm/blob/master/tck/src/main/java/org/reactivestreams/tck/TestEnvironment.java#L1019
[2] /~https://github.com/reactive-streams/reactive-streams-jvm/blob/master/tck/src/main/java/org/reactivestreams/tck/TestEnvironment.java#L978
[3] /~https://github.com/reactive-streams/reactive-streams-jvm/blob/master/tck/src/main/java/org/reactivestreams/tck/TestEnvironment.java#L990
The text was updated successfully, but these errors were encountered: