We have several wait_for_* methods that by default return bool value (timeout was exceeded or not). The proper usage of this methods usualy is like this:
self.assertTrue(nginx.wait_for_connections())
Here we have problems when we forgot self.assertTrue - it can lead to hard to detect bugs (now some part of invocations of this methods haven't assert).
Proposal:
Introduce strict parameter, and check the timeout inside the method by default:
def wait_for_connection_close(self, timeout=5, strict=True):
"""
Try to use strict mode whenever it's possible
to prevent tests from hard to detect errors.
"""
timeout = adjust_timeout_for_tcp_segmentation(timeout)
timeout_not_exceeded = util.wait_until(
lambda: not self.connection_is_closed(),
timeout,
abort_cond=lambda: self.state != stateful.STATE_STARTED,
)
if strict:
assert (
timeout_not_exceeded != False
), f"Timeout exceeded while waiting connection close: {timeout}"
return timeout_not_exceeded
Now we have this implemented in BaseDeproxyClient, but we should implement this in all related wait_for_* methods and switch default strict value to True (after refactoring all affected tests).
Problem:
We have several
wait_for_*
methods that by default return bool value (timeout was exceeded or not). The proper usage of this methods usualy is like this:Here we have problems when we forgot
self.assertTrue
- it can lead to hard to detect bugs (now some part of invocations of this methods haven't assert).Proposal:
Introduce
strict
parameter, and check the timeout inside the method by default:Now we have this implemented in BaseDeproxyClient, but we should implement this in all related
wait_for_*
methods and switch defaultstrict
value to True (after refactoring all affected tests).