OSOE-530: Better docs, fixing retires on stale elements

Lombiq.Tests.UI/Extensions/ReliabilityUITestContextExtensions.cs

@@ -74,13 +74,14 @@ public static Task DoWithRetriesOrFailAsync(
             interval ?? context.Configuration.TimeoutConfiguration.RetryInterval);
 
     /// <summary>
-    /// Executes the async process and retries if an element becomes stale ( <see
+    /// Executes the async process and retries if an element becomes stale (throws <see
     /// cref="StaleElementReferenceException"/>). If the operation didn't succeed then throws a <see
     /// cref="TimeoutException"/>.
     ///
     /// In situations like a DataTable load it is possible that the page will change during execution of multiple long
-    /// running operations such as GetAll, causing stale virtual DOM. Such change tends to be near instantaneous and
-    /// only happens once at a time so this should pass by the 2nd try.
+    /// running operations such as <see cref="ElementRetrievalUITestContextExtensions.GetAll(UITestContext, By)"/>,
+    /// causing stale virtual DOM. Such change tends to be near instantaneous and only happens once at a time so this
+    /// should pass by the 2nd try.
     /// </summary>
     /// <param name="processAsync">
     /// The long running operation that may execute during DOM change and should be retried. Should return <see
@@ -109,7 +110,8 @@ public static Task RetryIfStaleOrFailAsync(
             interval ?? context.Configuration.TimeoutConfiguration.RetryInterval);
 
     /// <summary>
-    /// Executes the process and retries until no element is stale ( <see cref="StaleElementReferenceException"/>).
+    /// Executes the process and retries until no element is stale (throws <see
+    /// cref="StaleElementReferenceException"/>).
     ///
     /// If the operation didn't succeed then throws a <see cref="TimeoutException"/>.
     /// </summary>

Lombiq.Tests.UI/Helpers/ReliabilityHelper.cs

@@ -7,29 +7,33 @@ namespace Lombiq.Tests.UI.Helpers;
 
 public static class ReliabilityHelper
 {
-    private static readonly Func<Func<Task<bool>>, Func<Task<bool>>> _retryIfStaleProcess = innerProcess => () =>
+    private static readonly Func<Func<Task<bool>>, Func<Task<bool>>> _retryIfStaleProcess = innerProcess => async () =>
     {
         try
         {
-            return innerProcess();
+            // This needs to use await instead of returning the Task directly, because only this way can we catch the
+            // exception below.
+            return await innerProcess();
         }
         catch (StaleElementReferenceException)
         {
             // When navigating away this exception will be thrown for all old element references. Not nice to use
             // exceptions but there doesn't seem to be a better way to do this.
-            return Task.FromResult(false);
+            return false;
         }
     };
 
-    private static readonly Func<Func<Task<bool>>, Func<Task<bool>>> _retryIfNotStaleProcess = innerProcess => () =>
+    private static readonly Func<Func<Task<bool>>, Func<Task<bool>>> _retryIfNotStaleProcess = innerProcess => async () =>
     {
         try
         {
-            return innerProcess();
+            // This needs to use await instead of returning the Task directly, because only this way can we catch the
+            // exception below.
+            return await innerProcess();
         }
         catch (StaleElementReferenceException)
         {
-            return Task.FromResult(true);
+            return true;
         }
     };