Timeout support in WebSocketTestSession

[YuriyGuts]

Hi! I'm trying to test a WebSocket endpoint with TestClient, which opens a WebSocketTestSession under the hood. Between test runs, I'm also deliberately trying to "break" the endpoint so see if the tests catch the errors properly.

One thing I've noticed is that WebSocketTestSession does not support timeouts. Under the hood, it seems to read/write to queues using the blocking mode (without the timeout parameter):

starlette/starlette/testclient.py

Lines 145 to 146 in 2168e47

	def send(self, message: Message) -> None:
	self._receive_queue.put(message)

starlette/starlette/testclient.py

Lines 165 to 166 in 2168e47

	def receive(self) -> Message:
	message = self._send_queue.get()

This, unfortunately, makes the unit tests hang infinitely if the app under test does not behave properly.

Would adding and passing the timeout kwarg to WebSocketTestSession methods be a reasonable fix for this issue?

[mbergen]

I'd really like a timeout option when testing websockets, so i created #2329 as potential fix.
It adds the option to every receive method instead of to the session to allow more granular control about different calls.

[adriangb]

I'd recommend you use pure-ASGI when you want to test timeouts. Or connect over a socket and use a real httpx instance, it will help make sure your catching the right errors.

[mbergen]

It's not really that we want to test timeouts, it is more that we want the test to fail if the SUT does not respond (in some margin, i.e. timeout).
For example, i can have a test that expects to receive a message on the websocket. As long as the SUT behaves, it will get one, but as soon as a breaking change of the SUT happens, this test will hang forever (or until pytest runs into a timeout).

[YuriyGuts]

Yes, precisely ^ Timeouts in tests would serve the purpose of not making the test suite hang forever if a breaking change is made to the SUT.

[adriangb]

That's fair but you can use a pytest extension that just generally fails if a test takes more than X seconds. Maybe not as precise as you want w.r.t to where exactly the failure occurred but it will also protect you against other reasons a SUT might just hang / run forever and should be enough for you to go and look at logs or investigate locally in more detail.

[mbergen]

That's how i'm currently working around having no timeout option on the WebSocketTestSession::receive, but i fail to understand why you are opposed to adding this feature.

In addition, some of my tests have longer sections unrelated to the websocket itself, so i have to account for them in the generic timeout and that is rather high. That means when it's actually the websocket thats blocking and running into the timeout, the tests waits for way longer than it needs to (e.g. whole test needs timeouts of >60s, whereas for most websocket receives i could already timeout after 1 second).

[serans1]

The objection to add such a trivial request is not clear. especially when the fix is so simple and straight forward since queues are supporting timeout.
I have implanted this and running like that locally, works great.
python unit test do not support timeouts (another unclear behavior for testing environment which expects to hang), this makes life much harder and for no apparent reason.