re: #34422 (review)
Using a context that silently polls the debug log until a hidden timeout is hit, is brittle and confusing.
Wow, thank you noticing this and identifying the problem! But I don’t think this is right.
There is literally no timeout specified on this line of code, nor should there be. The assert_debug_log function did not take a timeout option when it was introduced in #14024 and tests should not need to hardcode timeout values every time they take an action that does not complete immediately.
I think test framework should provide some default timeout controlling how long tests are allowed to wait idly before they are considered failed. Specifically, it would seem nice to replace the rpc_timeout variable with a generic failure_timeout variable, and also stop multiplying it by timeout_factor up front, instead applying the factor when RPC calls are made. This is probably better discussed elsewhere but I feel like the recent change in #34581 adding timeout=2 values everywhere is not the best long term approach.
For now, though it is probably easiest to add timeout=2 here as you suggest to fix the silent conflict with #34581.
For a quick and dirty fix, you can set timeout=2 (or whatever). However, my recommendation would be to wait on a “real” condition in the inner context. e.g
0with assert_debug_log([disconnect_msg]):
1 initiate_disconnect()
2 self.wait_until(disconnect)
This suggestion doesn’t actually make sense. The test is calling waitNext, then disconnecting the IPC client, then waiting for the server to detect that the IPC client has disconnected, then it is calling generate.
The point of the disconnected_log_check is not to make sure something is logged, but to delay the generate call after until the server processes the client disconnect, to make the test deterministic and be sure it is checking the right thing.
