Windows: Fix active_quic_listener_test#13918
Windows: Fix active_quic_listener_test#13918sunjayBhatia wants to merge 1 commit intoenvoyproxy:masterfrom greenhouse-org:windows-fix-active-quic-listener-test
Conversation
Rerun event loop until all client connections are consumed by the event loop. Running a non-blocking event loop, all outstanding socket events (especially on Windows) may not be picked up before the dispatcher exits. Another alternative would be to run the run the dispatcher in blocking mode and make it exit in a mocked callback on the last request processing. This is doable for the non-TLS quic path, but not as straightforward when testing a quic version with TLS with the network filter chain setup etc. Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
|
@envoyproxy/windows-dev |
I remember now! we tested this earlier on before your PR was further along and saw it still failing (it is passing now, confirmed), sounds good, will hold off on these sorts of flaky/failing tests for a bit until your PR is reviewed and merged |
|
I ended up removing the |
|
@davinci26 the general idea around this kind of change still maybe stands, technically non-blocking mode doesn't guarantee every event is picked up, the assumption that it does maybe should be reviewed at some point and test structure at large refactored, especially if we continue to see windows flakiness |
Commit Message:
Windows: Fix active_quic_listener_test
Rerun event loop until all client connections are consumed by the event
loop. Running a non-blocking event loop, all outstanding socket events
(especially on Windows) may not be picked up before the dispatcher
exits.
Additional Description:
Another alternative would be to run the run the dispatcher in blocking
mode and make it exit in a mocked callback on the last request
processing. This is doable for the non-TLS quic path, but not as
straightforward when testing a quic version with TLS with the network
filter chain setup etc.
Risk Level: Low
Testing: Locally on Windows + RBE, modifies unit test
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A