Next round of fixes

Author: EmandM

.yamato/console-standalone-test.yml

@@ -88,7 +88,17 @@ console_standalone_test_{{ project.name }}_{{ platform.name }}_{{ editor }}:
     model: {{ platform.model }} # This is set only in platforms where we want non-default model to use (more information in project.metafile)
 {% endif %}
   commands:
-    - UnifiedTestRunner --suite=playmode --platform={{ platform.standalone }} --scripting-backend=il2cpp --testproject={{ project.path }} --artifacts-path=test-results --player-load-path=build/players --fail-on-assert --rerun-strategy=Test --retry={{ num_test_retries }} --timeout={{ test_timeout }}
+    # Console players connect back to UTR's player connection listener on this VM, so the firewall must allow it
+    - command: gsudo NetSh Advfirewall set allprofiles state off
+    # Register the Bokken devkit with the platform's target tools so UTR can deploy and run the prebuilt player
+{% if platform.name == "ps5" %}
+    - command: gsudo prospero-ctrl target add %BOKKEN_DEVICE_IP%
+{% elsif platform.name == "ps4" %}
+    - command: orbis-ctrl add %BOKKEN_DEVICE_IP%
+{% else %}
+    - command: '"%GameDK%/bin/xbconnect" /WS %BOKKEN_DEVICE_IP%'
+{% endif %}
+    - UnifiedTestRunner --suite=playmode --platform={{ platform.standalone }} --scripting-backend=il2cpp --testproject={{ project.path }} --artifacts-path=test-results --player-load-path=build/players --player-connection-ip=%BOKKEN_HOST_IP% --fail-on-assert --rerun-strategy=Test --retry={{ num_test_retries }} --timeout={{ test_timeout }}
   variables:
 # PS4 related
     SCE_ORBIS_SDK_DIR: 'C:\Users\bokken\SCE\ps4_sdk_12_00'

.yamato/desktop-standalone-tests.yml

@@ -49,7 +49,7 @@ desktop_standalone_build_{{ project.name }}_{{ platform.name }}_{{ backend }}_{{
 {% endif %}
   commands:
     - unity-downloader-cli --fast --wait -u {{ editor }} -c Editor {% if backend == "il2cpp" %} -c il2cpp {% endif %}{% if backend == "coreclr" %}{% unless coreclr_runtime_editors contains editor %} -c {{ platform.coreclr_component }} {% endunless %}{% endif %} --arch {{ platform.architecture }}
-    - UnifiedTestRunner --suite=playmode --platform={{ platform.standalone }} --editor-location=.Editor --testproject={{ project.path }} --scripting-backend={{ backend }} --testfilter="Unity.Netcode.RuntimeTests.*;TestProject.RuntimeTests.*" --player-save-path=build/players --artifacts-path=artifacts --extra-editor-arg=-batchmode --extra-editor-arg=-nographics --reruncount=1 --clean-library-on-rerun --build-only --timeout={{ test_timeout }}
+    - UnifiedTestRunner --suite=playmode --platform={{ platform.standalone }} --editor-location=.Editor --testproject={{ project.path }} --scripting-backend={{ backend }} --testfilter="Unity.Netcode.RuntimeTests.*;TestProject.RuntimeTests.*" --playergraphicsapi=Null --player-save-path=build/players --artifacts-path=artifacts --extra-editor-arg=-batchmode --extra-editor-arg=-nographics --reruncount=1 --clean-library-on-rerun --build-only --timeout={{ test_timeout }}
   artifacts:
     players:
       paths:

com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/IntegrationTestSceneHandler.cs

@@ -372,7 +372,8 @@ private bool ExcludeSceneFromSynchronizationCheck(Scene scene)
 
         public void ClearSceneTracking(NetworkManager networkManager)
         {
-            SceneNameToSceneHandles.Clear();
+            // Only clear the calling NetworkManager's tracking, other NetworkManagers may still be tracking their own scenes
+            SceneNameToSceneHandles.Remove(networkManager);
         }
 
         public void StopTrackingScene(NetworkSceneHandle handle, string name, NetworkManager networkManager)

com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs

@@ -831,6 +831,8 @@ public IEnumerator SetUp()
             // In addition to setting the number of clients in the OneTimeSetup, we need to re-apply the number of clients for each unique test
             // in the event that a previous test stopped a client.
             m_NumberOfClients = NumberOfClients;
+            // Clear any time out state left by a previous test so it cannot fail this test's assertions
+            s_GlobalTimeoutHelper.TimedOut = false;
             VerboseDebugLog.Clear();
             VerboseDebug($"Entering {nameof(SetUp)}");
             NetcodeLogAssert = new NetcodeLogAssert();
@@ -1261,7 +1263,7 @@ protected void CreateAndStartNewClientWithTimeTravel()
 
             // Wait for the new client to connect
             var connected = WaitForClientsConnectedOrTimeOutWithTimeTravel();
-            AssertOnTimeout($"{nameof(CreateAndStartNewClientWithTimeTravel)} timed out waiting for all clients to be connected!\n {m_InternalErrorLog}");
+            Assert.IsTrue(connected, $"{nameof(CreateAndStartNewClientWithTimeTravel)} timed out waiting for all clients to be connected!\n {m_InternalErrorLog}");
 
             OnNewClientStartedAndConnected(networkManager);
             if (!connected)
@@ -1644,9 +1646,9 @@ protected void StartServerAndClientsWithTimeTravel()
                 if (!m_BypassConnectionTimeout)
                 {
                     // Wait for all clients to connect
-                    WaitForClientsConnectedOrTimeOutWithTimeTravel();
+                    var connected = WaitForClientsConnectedOrTimeOutWithTimeTravel();
 
-                    AssertOnTimeout($"{nameof(StartServerAndClients)} timed out waiting for all clients to be connected!\n {m_InternalErrorLog}");
+                    Assert.IsTrue(connected, $"{nameof(StartServerAndClients)} timed out waiting for all clients to be connected!\n {m_InternalErrorLog}");
 
                     if (m_UseHost || authorityManager.IsHost)
                     {
@@ -1669,8 +1671,8 @@ protected void StartServerAndClientsWithTimeTravel()
                     {
                         foreach (var networkManager in m_NetworkManagers)
                         {
-                            WaitForConditionOrTimeOutWithTimeTravel(() => AllPlayerObjectClonesSpawned(networkManager));
-                            AssertOnTimeout($"{nameof(CreateAndStartNewClient)} timed out waiting for all sessions to spawn Client-{networkManager.LocalClientId}'s player object!");
+                            var spawned = WaitForConditionOrTimeOutWithTimeTravel(() => AllPlayerObjectClonesSpawned(networkManager));
+                            Assert.IsTrue(spawned, $"{nameof(CreateAndStartNewClient)} timed out waiting for all sessions to spawn Client-{networkManager.LocalClientId}'s player object!");
                         }
                     }
 
@@ -1771,7 +1773,7 @@ protected void ShutdownAndCleanUp()
             }
             catch (Exception e)
             {
-                throw e;
+                Debug.LogException(e);
             }
             finally
             {
@@ -1909,6 +1911,9 @@ public IEnumerator TearDown()
                 {
                     ShutdownAndCleanUp();
                 }
+
+                // Wait until all the scene from this test are unloaded before starting the next test.
+                yield return WaitForRemainingScenesUnloaded();
             }
 
             if (m_EnableTimeTravel)
@@ -2975,22 +2980,72 @@ protected void AssertOnTimeout(string timeOutErrorMessage, TimeoutHelper assigne
         }
 
 
+        private const float k_SceneUnloadTimeout = 5.0f;
+        private readonly List<AsyncOperation> m_PendingSceneUnloads = new List<AsyncOperation>();
+
         private void UnloadRemainingScenes()
         {
             // Unload any remaining scenes loaded but the test runner scene
             // Note: Some tests only unload the server-side instance, and this
             // just assures no currently loaded scenes will impact the next test
+            Scene testRunnerScene = default;
             for (int i = 0; i < SceneManager.sceneCount; i++)
             {
                 var scene = SceneManager.GetSceneAt(i);
-                if (!scene.IsValid() || !scene.isLoaded || scene.name.Contains(NetcodeIntegrationTestHelpers.FirstPartOfTestRunnerSceneName) || !OnCanSceneCleanUpUnload(scene))
+                if (scene.name.Contains(NetcodeIntegrationTestHelpers.FirstPartOfTestRunnerSceneName)) {
+                    testRunnerScene = scene;
+                    continue;
+                }
+                if (!scene.IsValid() || !scene.isLoaded || !OnCanSceneCleanUpUnload(scene))
                 {
                     continue;
                 }
 
                 VerboseDebug($"Unloading scene {scene.name}-{scene.handle}");
                 var asyncOperation = SceneManager.UnloadSceneAsync(scene);
+                if (asyncOperation != null)
+                {
+                    m_PendingSceneUnloads.Add(asyncOperation);
+                }
+            }
+
+            // Ensure the test runner scene is reset to be the active scene.
+            if (testRunnerScene.IsValid())
+            {
+                if (SceneManager.GetActiveScene() != testRunnerScene)
+                {
+                    SceneManager.SetActiveScene(testRunnerScene);
+                }
+
+            }
+        }
+
+        /// <summary>
+        /// Waits (in real time) for all scenes left over by the test to finish unloading.
+        /// Scenes that finish loading after tear down started are also unloaded.
+        /// </summary>
+        private IEnumerator WaitForRemainingScenesUnloaded()
+        {
+            var timeOut = Time.realtimeSinceStartup + k_SceneUnloadTimeout;
+            while (true)
+            {
+                // Pick up any scenes that finished loading after the clean up started
+                UnloadRemainingScenes();
+
+                if (m_PendingSceneUnloads.All(c => c.isDone))
+                {
+                    break;
+                }
+
+                if (Time.realtimeSinceStartup >= timeOut)
+                {
+                    Debug.LogWarning($"[{nameof(WaitForRemainingScenesUnloaded)}] Timed out waiting for scenes to unload!");
+                    break;
+                }
+                yield return null;
             }
+
+            m_PendingSceneUnloads.Clear();
         }
 
         private StringBuilder m_WaitForLog = new StringBuilder();

com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTestHelpers.cs

@@ -478,16 +478,29 @@ public static void Destroy()
 
             try
             {
-                // Shutdown the server which forces clients to disconnect
+                NetworkManager sessionAuthority = null;
+                // Shutdown clients first
                 foreach (var networkManager in NetworkManagerInstances)
                 {
-                    if (networkManager != null && networkManager.IsListening)
+                    if (networkManager.LocalClient.IsSessionOwner || networkManager.IsServer)
                     {
-                        networkManager?.Shutdown();
+                        sessionAuthority = networkManager;
+                    }
+                    else
+                    {
+                        networkManager.Shutdown();
                         s_Hooks.Remove(networkManager);
                     }
                 }
 
+                // Shutdown the server/session owner last
+                // This avoids session owner election when running CMB tests
+                if (sessionAuthority != null)
+                {
+                    sessionAuthority.Shutdown();
+                    s_Hooks.Remove(sessionAuthority);
+                }
+
                 // Destroy the network manager instances
                 foreach (var networkManager in NetworkManagerInstances)
                 {