From 6a0fac48fad08e750734b8596cb58360bc7f378e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zolt=C3=A1n=20Leh=C3=B3czky?= <zoltan.lehoczky@lombiq.com>
Date: Fri, 3 Jan 2025 22:28:02 +0100
Subject: [PATCH] Moving test timeout management to UITestExecutionSession

---
 .../Services/UITestExecutionSession.cs        | 27 ++++++++++++++++++-
 Lombiq.Tests.UI/UITestBase.cs                 | 25 +----------------
 2 files changed, 27 insertions(+), 25 deletions(-)

diff --git a/Lombiq.Tests.UI/Services/UITestExecutionSession.cs b/Lombiq.Tests.UI/Services/UITestExecutionSession.cs
index 0a252d21c..5eb168ae3 100644
--- a/Lombiq.Tests.UI/Services/UITestExecutionSession.cs
+++ b/Lombiq.Tests.UI/Services/UITestExecutionSession.cs
@@ -14,6 +14,7 @@
 using System;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.IO;
 using System.Linq;
 using System.Text.Json.Nodes;
@@ -104,7 +105,31 @@ public async Task<bool> ExecuteAsync(int retryCount, string dumpRootPath)
 
             if (_context.IsBrowserConfigured) _context.SetDefaultBrowserSize();
 
-            await _testManifest.TestAsync(_context);
+            var timeout = _configuration.TimeoutConfiguration.TestRunTimeout;
+
+            var timeoutTask = Task.Delay(timeout, _configuration.TestCancellationToken);
+            Debug.WriteLine("Timeout task created: " + timeoutTask.GetHashCode());
+            var testTask = _testManifest.TestAsync(_context);
+
+            await Task.WhenAny(testTask, timeoutTask);
+
+            if (timeoutTask.IsCompleted)
+            {
+                // If the EnterInteractiveModeAsync() extension method has been used, then timeout should be ignored to
+                // make the debugging experience smoother. Note that EnterInteractiveModeAsync() should never be used in
+                // committed tests.
+                if (!ShortcutsUITestContextExtensions.InteractiveModeHasBeenUsed)
+                {
+                    throw new TimeoutException($"The time allotted for the test ({timeout}) was exceeded.");
+                }
+
+                await testTask;
+            }
+
+            // Since the timeout task is not yet completed but the Task.WhenAny has finished, the test task is done in
+            // some way. So it's safe to await it here. It's also necessary to cleanly propagate any exceptions that may
+            // have been thrown inside it.
+            await testTask;
 
             await _context.AssertLogsAsync();
 
diff --git a/Lombiq.Tests.UI/UITestBase.cs b/Lombiq.Tests.UI/UITestBase.cs
index df4e115fb..6473ac9f2 100644
--- a/Lombiq.Tests.UI/UITestBase.cs
+++ b/Lombiq.Tests.UI/UITestBase.cs
@@ -1,4 +1,3 @@
-using Lombiq.Tests.UI.Extensions;
 using Lombiq.Tests.UI.Models;
 using Lombiq.Tests.UI.Services;
 using Lombiq.Tests.UI.Services.GitHub;
@@ -22,7 +21,6 @@ protected async Task ExecuteOrchardCoreTestAsync(
         OrchardCoreUITestExecutorConfiguration configuration)
     {
         var originalTestOutputHelper = _testOutputHelper;
-        var timeout = configuration.TimeoutConfiguration.TestRunTimeout;
 
         Action afterTest = null;
         if (configuration.ExtendGitHubActionsOutput &&
@@ -39,31 +37,10 @@ protected async Task ExecuteOrchardCoreTestAsync(
 
         try
         {
-            var testTask = UITestExecutor.ExecuteOrchardCoreTestAsync(
+            await UITestExecutor.ExecuteOrchardCoreTestAsync(
                 webApplicationInstanceFactory,
                 testManifest,
                 configuration);
-            var timeoutTask = Task.Delay(timeout, configuration.TestCancellationToken);
-
-            await Task.WhenAny(testTask, timeoutTask);
-
-            if (timeoutTask.IsCompleted)
-            {
-                // If the EnterInteractiveModeAsync() extension method has been used, then timeout should be ignored to
-                // make the debugging experience smoother. Note that EnterInteractiveModeAsync() should never be used in
-                // committed tests.
-                if (!ShortcutsUITestContextExtensions.InteractiveModeHasBeenUsed)
-                {
-                    throw new TimeoutException($"The time allotted for the test ({timeout}) was exceeded.");
-                }
-
-                await testTask;
-            }
-
-            // Since the timeout task is not yet completed but the Task.WhenAny has finished, the test task is done in
-            // some way. So it's safe to await it here. It's also necessary to cleanly propagate any exceptions that may
-            // have been thrown inside it.
-            await testTask;
         }
         finally
         {