MM-24611: Fix flaky test TestDeletePreferencesWebsocket (#14399)

After registering the conn in the hub, we proceeded to send a
direct message to the user. We had changed it to send the direct message
in the same hub goroutine that handles the registration. This was the correct
behavior and fixes chances of having panics due to sending to closed channels.

However, often fixing something unearths some deeper underlying bug. This was
such a case :)

The issue was that register channel had a buffer size of 1. And we were sending
a direct message after registration. In the code to send direct message, we
were checking if the user has been registered or not, and if not, then skip it.

Therefore, since the register channel buffer was 1, it could very well be that
the select case would pick up the direct message send case first - in which
case it would not have been registered, and therefore no hello message would be sent.

The fix is to unbuffer the register and unregister channels. There does not seem
to be a valid reason to make these buffered channels. They are meant to be
synchronous operations, because the code following them assumes that the user
has been registered.

While here, we also remove all the time.Sleeps before waiting on the Response channel
because they are not required at all. Waiting on a channel is already blocking.

Co-authored-by: Ben Schumacher <ben.schumacher@mattermost.com>

api4/preference_test.go

@@ -364,7 +364,6 @@ func TestDeletePreferencesWebsocket(t *testing.T) {
 	require.Nil(t, err)
 
 	WebSocketClient.Listen()
-	time.Sleep(300 * time.Millisecond)
 	wsResp := <-WebSocketClient.ResponseChannel
 	require.Equal(t, model.STATUS_OK, wsResp.Status, "should have responded OK to authentication challenge")
 

api4/websocket_test.go

@@ -32,22 +32,18 @@ func TestWebSocket(t *testing.T) {
 
 	WebSocketClient.Listen()
 
-	time.Sleep(300 * time.Millisecond)
 	resp := <-WebSocketClient.ResponseChannel
 	require.Equal(t, resp.Status, model.STATUS_OK, "should have responded OK to authentication challenge")
 
 	WebSocketClient.SendMessage("ping", nil)
-	time.Sleep(300 * time.Millisecond)
 	resp = <-WebSocketClient.ResponseChannel
 	require.Equal(t, resp.Data["text"].(string), "pong", "wrong response")
 
 	WebSocketClient.SendMessage("", nil)
-	time.Sleep(300 * time.Millisecond)
 	resp = <-WebSocketClient.ResponseChannel
 	require.Equal(t, resp.Error.Id, "api.web_socket_router.no_action.app_error", "should have been no action response")
 
 	WebSocketClient.SendMessage("junk", nil)
-	time.Sleep(300 * time.Millisecond)
 	resp = <-WebSocketClient.ResponseChannel
 	require.Equal(t, resp.Error.Id, "api.web_socket_router.bad_action.app_error", "should have been bad action response")
 
@@ -55,23 +51,19 @@ func TestWebSocket(t *testing.T) {
 	req.Seq = 0
 	req.Action = "ping"
 	WebSocketClient.Conn.WriteJSON(req)
-	time.Sleep(300 * time.Millisecond)
 	resp = <-WebSocketClient.ResponseChannel
 	require.Equal(t, resp.Error.Id, "api.web_socket_router.bad_seq.app_error", "should have been bad action response")
 
 	WebSocketClient.UserTyping("", "")
-	time.Sleep(300 * time.Millisecond)
 	resp = <-WebSocketClient.ResponseChannel
 	require.Equal(t, resp.Error.Id, "api.websocket_handler.invalid_param.app_error", "should have been invalid param response")
 	require.Equal(t, resp.Error.DetailedError, "", "detailed error not cleared")
 
 	WebSocketClient.UserTyping(th.BasicChannel.Id, "")
-	time.Sleep(300 * time.Millisecond)
 	resp = <-WebSocketClient.ResponseChannel
 	require.Nil(t, resp.Error)
 
 	WebSocketClient.UserTyping(th.BasicPrivateChannel2.Id, "")
-	time.Sleep(300 * time.Millisecond)
 	resp = <-WebSocketClient.ResponseChannel
 	require.Equal(t, resp.Error.Id, "api.websocket_handler.invalid_param.app_error", "should have been invalid param response")
 	require.Equal(t, resp.Error.DetailedError, "", "detailed error not cleared")
@@ -96,7 +88,6 @@ func TestWebSocketEvent(t *testing.T) {
 
 	WebSocketClient.Listen()
 
-	time.Sleep(300 * time.Millisecond)
 	resp := <-WebSocketClient.ResponseChannel
 	require.Equal(t, resp.Status, model.STATUS_OK, "should have responded OK to authentication challenge")
 
@@ -175,7 +166,6 @@ func TestCreateDirectChannelWithSocket(t *testing.T) {
 	defer WebSocketClient.Close()
 	WebSocketClient.Listen()
 
-	time.Sleep(300 * time.Millisecond)
 	resp := <-WebSocketClient.ResponseChannel
 	require.Equal(t, resp.Status, model.STATUS_OK, "should have responded OK to authentication challenge")
 
@@ -272,7 +262,6 @@ func TestWebSocketStatuses(t *testing.T) {
 	defer WebSocketClient.Close()
 	WebSocketClient.Listen()
 
-	time.Sleep(300 * time.Millisecond)
 	resp := <-WebSocketClient.ResponseChannel
 	require.Equal(t, resp.Status, model.STATUS_OK, "should have responded OK to authentication challenge")