test refactor

This update collapses all of the time out code into the BaseMultiInstanceTest with an added method WaitForConditionOrTimeOut that leverages from an added TimeOutHelper class.

Author: NoelStephensUnity

com.unity.netcode.gameobjects/Tests/Runtime/BaseMultiInstanceTest.cs

@@ -18,10 +18,70 @@ public abstract class BaseMultiInstanceTest
 
         protected bool m_BypassStartAndWaitForClients = false;
 
+        static protected TimeOutHelper s_GloabalTimeOutHelper = new TimeOutHelper();
+
+        protected const uint k_DefaultTickRate = 30;
+        protected WaitForSeconds m_DefaultWaitForTick = new WaitForSeconds(1.0f / k_DefaultTickRate);
+
+        /// <summary>
+        /// An update to the original MultiInstanceHelpers.WaitForCondition that:
+        ///     -operates at the current tick rate
+        ///     -provides a value of type T to be passed into the checkForCondition function
+        ///     -allows for a unique TimeOutHelper handler (if none then uses the default)
+        ///     -adjusts its yield period to the settings of the m_ServerNetworkManager.NetworkConfig.TickRate
+        /// Notes: This method provides more stability when running integration tests that could
+        /// be impacted by:
+        ///     -how the integration test is being executed (i.e. in editor or in a stand alone build)
+        ///     -potential platform performance issues (i.e. VM is throttled or maxed)
+        /// </summary>
+        /// <typeparam name="T">type of the value to compare</typeparam>
+        /// <param name="checkForCondition">condition checking function that is passed valueToCompare to compare against</param>
+        /// <param name="valueToCompare">the value to compare against</param>
+        /// <param name="timeOutHelper">optional custom time out helper-handler</param>
+        /// <returns></returns>
+        protected IEnumerator WaitForConditionOrTimeOut<T>(Func<T, bool> checkForCondition, T valueToCompare, TimeOutHelper timeOutHelper = null)
+        {
+            if (checkForCondition == null)
+            {
+                throw new ArgumentNullException($"checkForCondition cannot be null!");
+            }
+
+            if (valueToCompare == null)
+            {
+                throw new ArgumentNullException($"The value to be compared cannot be null!");
+            }
+
+            // If none is provided we use the default global time out helper
+            if (timeOutHelper == null)
+            {
+                timeOutHelper = s_GloabalTimeOutHelper;
+            }
+
+            // Start checking for a timeout
+            timeOutHelper.Start();
+            while (!timeOutHelper.HasTimedOut())
+            {
+                // Update and check to see if the condition has been met
+                if (checkForCondition.Invoke(valueToCompare))
+                {
+                    break;
+                }
+
+                // Otherwise wait for 1 tick interval
+                yield return m_DefaultWaitForTick;
+            }
+            // Stop checking for a timeout
+            timeOutHelper.Stop();
+        }
+
         [UnitySetUp]
         public virtual IEnumerator Setup()
         {
             yield return StartSomeClientsAndServerWithPlayers(true, NbClients, _ => { });
+            if (m_ServerNetworkManager != null)
+            {
+                m_DefaultWaitForTick = new WaitForSeconds(1.0f / m_ServerNetworkManager.NetworkConfig.TickRate);
+            }
         }
 
         [UnityTearDown]
@@ -54,9 +114,12 @@ public virtual IEnumerator Teardown()
                 Object.DestroyImmediate(networkObject);
             }
 
-            // wait for next frame so everything is destroyed, so following tests can execute from clean environment
-            int nextFrameNumber = Time.frameCount + 1;
-            yield return new WaitUntil(() => Time.frameCount >= nextFrameNumber);
+            // wait for 1 tick interval so everything is destroyed and any following tests
+            // can execute from clean environment
+            yield return m_DefaultWaitForTick;
+
+            // reset the m_ServerWaitForTick for the next test to initialize
+            m_DefaultWaitForTick = new WaitForSeconds(1.0f / k_DefaultTickRate);
         }
 
         /// <summary>
@@ -170,4 +233,42 @@ public IEnumerator StartSomeClientsAndServerWithPlayers(bool useHost, int nbClie
             }
         }
     }
+
+    /// <summary>
+    /// Can be used independently or a assigned to <see cref="BaseMultiInstanceTest.WaitForConditionOrTimeOut"></see> in the
+    /// event the default timeout period needs to be adjusted
+    /// </summary>
+    public class TimeOutHelper
+    {
+        private const float k_DefaultTimeOutWaitPeriod = 2.0f;
+
+        private float m_MaximumTimeBeforeTimeOut;
+        private float m_TimeOutPeriod;
+
+        private bool m_IsStarted;
+        public bool TimedOut { get; internal set; }
+
+        public void Start()
+        {
+            m_MaximumTimeBeforeTimeOut = Time.realtimeSinceStartup + m_TimeOutPeriod;
+            m_IsStarted = true;
+            TimedOut = false;
+        }
+
+        public void Stop()
+        {
+            TimedOut = HasTimedOut();
+            m_IsStarted = false;
+        }
+
+        public bool HasTimedOut()
+        {
+            return m_IsStarted ? m_MaximumTimeBeforeTimeOut < Time.realtimeSinceStartup : TimedOut;
+        }
+
+        public TimeOutHelper(float timeOutPeriod = k_DefaultTimeOutWaitPeriod)
+        {
+            m_TimeOutPeriod = timeOutPeriod;
+        }
+    }
 }

com.unity.netcode.gameobjects/Tests/Runtime/NetworkBehaviourUpdaterTests.cs

@@ -1,5 +1,6 @@
 using System.Collections;
 using System.Collections.Generic;
+using System.Linq;
 using NUnit.Framework;
 using UnityEngine;
 using UnityEngine.TestTools;
@@ -153,9 +154,7 @@ public class NetworkBehaviourUpdaterTests : BaseMultiInstanceTest
     {
         // Go ahead and create maximum number of clients (not all tests will use them)
         protected override int NbClients => 2;
-        private const float k_TimeOutWaitPeriod = 2.0f;
         public const int NetVarValueToSet = 1;
-        private static float s_TimeOutPeriod;
         private static List<GameObject> s_ClientSpawnedNetworkObjects = new List<GameObject>();
         private List<NetworkManager> m_ActiveClientsForCurrentTest;
 
@@ -168,31 +167,9 @@ public static void ClientSideNotifyObjectSpawned(GameObject objectSpaned)
             if (!s_ClientSpawnedNetworkObjects.Contains(objectSpaned))
             {
                 s_ClientSpawnedNetworkObjects.Add(objectSpaned);
-                // As long as we are getting notified the clients are spawning objects
-                // then bump up the timeout period
-                AdvanceTimeOutPeriod();
             }
         }
 
-        /// <summary>
-        /// This will simply advance the timeout period
-        /// Note: When ClientSideNotifyObjectSpawned is invoked this will get
-        /// called to handle any potential latencies due to poor performance or
-        /// the like.
-        /// </summary>
-        private static void AdvanceTimeOutPeriod()
-        {
-            s_TimeOutPeriod = Time.realtimeSinceStartup + k_TimeOutWaitPeriod;
-        }
-
-        /// <summary>
-        /// Checks if the timeout period has elapsed
-        /// </summary>
-        private static bool HasTimedOut()
-        {
-            return s_TimeOutPeriod <= Time.realtimeSinceStartup;
-        }
-
         public override IEnumerator Setup()
         {
             // This makes sure the server and client are not started during the setup
@@ -315,22 +292,10 @@ public IEnumerator BehaviourUpdaterAllTests([Values] bool useHost,
             // wait until all objects are spawned on the clients
             if (numberOfObjectsToSpawnOnClients > 0)
             {
-                var allClientsSpawnedObjects = false;
-                // Reset the time out to be k_TimeOutWaitPeriod + the current time
-                AdvanceTimeOutPeriod();
-
                 // Waits for all clients to spawn the NetworkObjects
-                while (!allClientsSpawnedObjects && !HasTimedOut())
-                {
-                    allClientsSpawnedObjects = numberOfObjectsToSpawnOnClients == s_ClientSpawnedNetworkObjects.Count;
-                    yield return new WaitForSeconds(tickInterval);
-                }
-
-                Assert.True(!HasTimedOut(), $"Timed out waiting for clients to report spawning objects! " +
+                yield return WaitForConditionOrTimeOut((c) => c == s_ClientSpawnedNetworkObjects.Count, numberOfObjectsToSpawnOnClients);
+                Assert.IsFalse(s_GloabalTimeOutHelper.TimedOut, $"Timed out waiting for clients to report spawning objects! " +
                     $"Total reported client-side spawned objects {s_ClientSpawnedNetworkObjects.Count}");
-
-                // This really should never fail as it should timeout first
-                Assert.True(allClientsSpawnedObjects, "Not all clients spawned their objects!");
             }
 
             // Once all clients have spawned the NetworkObjects, set the network variables for
@@ -371,30 +336,10 @@ public IEnumerator BehaviourUpdaterAllTests([Values] bool useHost,
                     }
                 }
 
-                var allClientsCompleted = false;
-                AdvanceTimeOutPeriod();
-
-                // Wait until all clients have had their NetworkVariables updated
-                while (!allClientsCompleted && !HasTimedOut())
-                {
-                    var completedCount = 0;
-                    foreach (var clientNetVarContainer in clientSideNetVarContainers)
-                    {
-                        if (clientNetVarContainer.HaveAllValuesChanged(NetVarValueToSet))
-                        {
-                            completedCount++;
-                        }
-                    }
-
-                    allClientsCompleted = completedCount == clientSideNetVarContainers.Count;
-
-                    yield return new WaitForSeconds(tickInterval);
-                }
-
-                Assert.True(!HasTimedOut(), $"Timed out waiting for client side NetVarContainers to report all NetworkVariables have been updated!");
-
-                // This really should never fail as it should timeout first
-                Assert.True(allClientsCompleted, "Not all client side NetworkVariables reported they were updated with target value!");
+                yield return WaitForConditionOrTimeOut((c) =>
+                clientSideNetVarContainers.Where(d =>
+                d.HaveAllValuesChanged(c)).Count() == clientSideNetVarContainers.Count, NetVarValueToSet);
+                Assert.IsFalse(s_GloabalTimeOutHelper.TimedOut, $"Timed out waiting for client side NetVarContainers to report all NetworkVariables have been updated!");
             }
 
             Object.DestroyImmediate(prefabToSpawn);