From d22d48552fe573e89a55ca60afc2a0eefc775810 Mon Sep 17 00:00:00 2001 From: Simon Lemay Date: Thu, 28 Oct 2021 13:29:49 -0400 Subject: [PATCH 1/2] fix: UTP adapter tests failing on consoles A lot of tests were waiting on the Connect event on the server side as a proxy of the connection being fully established. But that's not correct. A Connect event on the server does not mean that the UTP Connection Accept message has been received and processed by the client. This lead to some timing issues where we'd try to e.g. send data on the client side before the connection is fully established. --- .../Tests/Runtime/ConnectionTests.cs | 43 ++++++++++++------- .../Tests/Runtime/TransportTests.cs | 23 ++++++---- 2 files changed, 42 insertions(+), 24 deletions(-) diff --git a/com.unity.netcode.adapter.utp/Tests/Runtime/ConnectionTests.cs b/com.unity.netcode.adapter.utp/Tests/Runtime/ConnectionTests.cs index df2bb2e8d7..4d19fd2183 100644 --- a/com.unity.netcode.adapter.utp/Tests/Runtime/ConnectionTests.cs +++ b/com.unity.netcode.adapter.utp/Tests/Runtime/ConnectionTests.cs @@ -17,6 +17,23 @@ public class ConnectionTests private List m_ServerEvents; private List[] m_ClientsEvents = new List[k_NumClients]; + private IEnumerator WaitForAllClientsConnected() + { + for (int i = 0; i < k_NumClients; i++) + { + if (m_ClientsEvents[i].Count == 0) + { + yield return WaitForNetworkEvent(NetworkEvent.Connect, m_ClientsEvents[i]); + } + } + + // Check that all clients received the correct event. + Assert.True(m_ClientsEvents.All(evs => evs[0].Type == NetworkEvent.Connect)); + + // Check that server received all Connect events. + Assert.AreEqual(k_NumClients, m_ServerEvents.Count); + } + [UnityTearDown] public IEnumerator Cleanup() { @@ -56,11 +73,11 @@ public IEnumerator ConnectSingleClient() m_Server.StartServer(); m_Clients[0].StartClient(); - yield return WaitForNetworkEvent(NetworkEvent.Connect, m_ServerEvents); + yield return WaitForNetworkEvent(NetworkEvent.Connect, m_ClientsEvents[0]); - // Check we've received Connect event on client too. - Assert.AreEqual(1, m_ClientsEvents[0].Count); - Assert.AreEqual(NetworkEvent.Connect, m_ClientsEvents[0][0].Type); + // Check we've received Connect event on server too. + Assert.AreEqual(1, m_ServerEvents.Count); + Assert.AreEqual(NetworkEvent.Connect, m_ServerEvents[0].Type); yield return null; } @@ -79,11 +96,7 @@ public IEnumerator ConnectMultipleClients() m_Clients[i].StartClient(); } - yield return WaitForNetworkEvent(NetworkEvent.Connect, m_ServerEvents); - - // Check that every client also received a Connect event. - Assert.True(m_ClientsEvents.All(evs => evs.Count == 1)); - Assert.True(m_ClientsEvents.All(evs => evs[0].Type == NetworkEvent.Connect)); + yield return WaitForAllClientsConnected(); yield return null; } @@ -98,7 +111,7 @@ public IEnumerator ServerDisconnectSingleClient() m_Server.StartServer(); m_Clients[0].StartClient(); - yield return WaitForNetworkEvent(NetworkEvent.Connect, m_ServerEvents); + yield return WaitForNetworkEvent(NetworkEvent.Connect, m_ClientsEvents[0]); m_Server.DisconnectRemoteClient(m_ServerEvents[0].ClientID); @@ -120,7 +133,7 @@ public IEnumerator ServerDisconnectMultipleClients() m_Clients[i].StartClient(); } - yield return WaitForNetworkEvent(NetworkEvent.Connect, m_ServerEvents); + yield return WaitForAllClientsConnected(); // Disconnect a single client. m_Server.DisconnectRemoteClient(m_ServerEvents[0].ClientID); @@ -157,7 +170,7 @@ public IEnumerator ClientDisconnectSingleClient() m_Server.StartServer(); m_Clients[0].StartClient(); - yield return WaitForNetworkEvent(NetworkEvent.Connect, m_ServerEvents); + yield return WaitForNetworkEvent(NetworkEvent.Connect, m_ClientsEvents[0]); m_Clients[0].DisconnectLocalClient(); @@ -177,7 +190,7 @@ public IEnumerator ClientDisconnectMultipleClients() m_Clients[i].StartClient(); } - yield return WaitForNetworkEvent(NetworkEvent.Connect, m_ServerEvents); + yield return WaitForAllClientsConnected(); // Disconnect a single client. m_Clients[0].DisconnectLocalClient(); @@ -209,7 +222,7 @@ public IEnumerator RepeatedServerDisconnectsNoop() m_Server.StartServer(); m_Clients[0].StartClient(); - yield return WaitForNetworkEvent(NetworkEvent.Connect, m_ServerEvents); + yield return WaitForNetworkEvent(NetworkEvent.Connect, m_ClientsEvents[0]); m_Server.DisconnectRemoteClient(m_ServerEvents[0].ClientID); @@ -241,7 +254,7 @@ public IEnumerator RepeatedClientDisconnectsNoop() m_Server.StartServer(); m_Clients[0].StartClient(); - yield return WaitForNetworkEvent(NetworkEvent.Connect, m_ServerEvents); + yield return WaitForNetworkEvent(NetworkEvent.Connect, m_ClientsEvents[0]); m_Clients[0].DisconnectLocalClient(); diff --git a/com.unity.netcode.adapter.utp/Tests/Runtime/TransportTests.cs b/com.unity.netcode.adapter.utp/Tests/Runtime/TransportTests.cs index d685ce6983..7705ef1c1f 100644 --- a/com.unity.netcode.adapter.utp/Tests/Runtime/TransportTests.cs +++ b/com.unity.netcode.adapter.utp/Tests/Runtime/TransportTests.cs @@ -54,7 +54,7 @@ public IEnumerator PingPong() m_Server.StartServer(); m_Client1.StartClient(); - yield return WaitForNetworkEvent(NetworkEvent.Connect, m_ServerEvents); + yield return WaitForNetworkEvent(NetworkEvent.Connect, m_Client1Events); var ping = new ArraySegment(Encoding.ASCII.GetBytes("ping")); m_Client1.Send(m_Client1.ServerClientId, ping, NetworkDelivery.ReliableSequenced); @@ -86,7 +86,7 @@ public IEnumerator PingPongSimultaneous() m_Server.StartServer(); m_Client1.StartClient(); - yield return WaitForNetworkEvent(NetworkEvent.Connect, m_ServerEvents); + yield return WaitForNetworkEvent(NetworkEvent.Connect, m_Client1Events); var ping = new ArraySegment(Encoding.ASCII.GetBytes("ping")); m_Server.Send(m_ServerEvents[0].ClientID, ping, NetworkDelivery.ReliableSequenced); @@ -120,7 +120,7 @@ public IEnumerator FilledSendQueueSingleSend() m_Server.StartServer(); m_Client1.StartClient(); - yield return WaitForNetworkEvent(NetworkEvent.Connect, m_ServerEvents); + yield return WaitForNetworkEvent(NetworkEvent.Connect, m_Client1Events); var payload = new ArraySegment(new byte[UnityTransport.InitialBatchQueueSize]); m_Client1.Send(m_Client1.ServerClientId, payload, NetworkDelivery.ReliableFragmentedSequenced); @@ -139,7 +139,7 @@ public IEnumerator FilledSendQueueMultipleSends() m_Server.StartServer(); m_Client1.StartClient(); - yield return WaitForNetworkEvent(NetworkEvent.Connect, m_ServerEvents); + yield return WaitForNetworkEvent(NetworkEvent.Connect, m_Client1Events); var numSends = UnityTransport.InitialBatchQueueSize / 1024; @@ -170,7 +170,7 @@ public IEnumerator MultipleSendsSingleFrame() m_Server.StartServer(); m_Client1.StartClient(); - yield return WaitForNetworkEvent(NetworkEvent.Connect, m_ServerEvents); + yield return WaitForNetworkEvent(NetworkEvent.Connect, m_Client1Events); var data1 = new ArraySegment(new byte[] { 11 }); m_Client1.Send(m_Client1.ServerClientId, data1, NetworkDelivery.ReliableSequenced); @@ -201,7 +201,11 @@ public IEnumerator SendMultipleClients() m_Client1.StartClient(); m_Client2.StartClient(); - yield return WaitForNetworkEvent(NetworkEvent.Connect, m_ServerEvents); + yield return WaitForNetworkEvent(NetworkEvent.Connect, m_Client1Events); + if (m_Client2Events.Count == 0) + { + yield return WaitForNetworkEvent(NetworkEvent.Connect, m_Client2Events); + } // Ensure we got both Connect events. Assert.AreEqual(2, m_ServerEvents.Count); @@ -239,9 +243,10 @@ public IEnumerator ReceiveMultipleClients() m_Client2.StartClient(); yield return WaitForNetworkEvent(NetworkEvent.Connect, m_Client1Events); - - // Ensure we got the Connect event on the other client too. - Assert.AreEqual(1, m_Client2Events.Count); + if (m_Client2Events.Count == 0) + { + yield return WaitForNetworkEvent(NetworkEvent.Connect, m_Client2Events); + } var data1 = new ArraySegment(new byte[] { 11 }); m_Client1.Send(m_Client1.ServerClientId, data1, NetworkDelivery.ReliableSequenced); From 095ed99f21ee23814c5af5346f9f0f147316e733 Mon Sep 17 00:00:00 2001 From: Simon Lemay Date: Tue, 2 Nov 2021 13:51:19 -0400 Subject: [PATCH 2/2] Use Assert.That instead of Assert.True --- .../Tests/Runtime/ConnectionTests.cs | 6 +++--- .../Tests/Runtime/TransportTests.cs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/com.unity.netcode.adapter.utp/Tests/Runtime/ConnectionTests.cs b/com.unity.netcode.adapter.utp/Tests/Runtime/ConnectionTests.cs index 4d19fd2183..09f27cf36c 100644 --- a/com.unity.netcode.adapter.utp/Tests/Runtime/ConnectionTests.cs +++ b/com.unity.netcode.adapter.utp/Tests/Runtime/ConnectionTests.cs @@ -28,7 +28,7 @@ private IEnumerator WaitForAllClientsConnected() } // Check that all clients received the correct event. - Assert.True(m_ClientsEvents.All(evs => evs[0].Type == NetworkEvent.Connect)); + Assert.That(m_ClientsEvents.All(evs => evs[0].Type == NetworkEvent.Connect)); // Check that server received all Connect events. Assert.AreEqual(k_NumClients, m_ServerEvents.Count); @@ -154,8 +154,8 @@ public IEnumerator ServerDisconnectMultipleClients() yield return new WaitForSeconds(MaxNetworkEventWaitTime); // Check that all clients got a Disconnect event. - Assert.True(m_ClientsEvents.All(evs => evs.Count == 2)); - Assert.True(m_ClientsEvents.All(evs => evs[1].Type == NetworkEvent.Disconnect)); + Assert.That(m_ClientsEvents.All(evs => evs.Count == 2)); + Assert.That(m_ClientsEvents.All(evs => evs[1].Type == NetworkEvent.Disconnect)); yield return null; } diff --git a/com.unity.netcode.adapter.utp/Tests/Runtime/TransportTests.cs b/com.unity.netcode.adapter.utp/Tests/Runtime/TransportTests.cs index 7705ef1c1f..83c31c322a 100644 --- a/com.unity.netcode.adapter.utp/Tests/Runtime/TransportTests.cs +++ b/com.unity.netcode.adapter.utp/Tests/Runtime/TransportTests.cs @@ -225,7 +225,7 @@ public IEnumerator SendMultipleClients() byte c1Data = m_Client1Events[1].Data.First(); byte c2Data = m_Client2Events[1].Data.First(); - Assert.True((c1Data == 11 && c2Data == 22) || (c1Data == 22 && c2Data == 11)); + Assert.That((c1Data == 11 && c2Data == 22) || (c1Data == 22 && c2Data == 11)); yield return null; } @@ -262,7 +262,7 @@ public IEnumerator ReceiveMultipleClients() byte sData1 = m_ServerEvents[2].Data.First(); byte sData2 = m_ServerEvents[3].Data.First(); - Assert.True((sData1 == 11 && sData2 == 22) || (sData1 == 22 && sData2 == 11)); + Assert.That((sData1 == 11 && sData2 == 22) || (sData1 == 22 && sData2 == 11)); yield return null; }