Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OSOE-430: Improve stability directly after a page load #316

Merged
merged 7 commits into from
Oct 18, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 59 additions & 9 deletions Lombiq.Tests.UI/Extensions/ExtendedLoggingExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,18 @@
using Lombiq.Tests.UI.Services;
using OpenQA.Selenium;
using System;
using System.Threading;
using System.Threading.Tasks;

namespace Lombiq.Tests.UI.Extensions;

public static class ExtendedLoggingExtensions
{
/// <summary>
/// Used for edge cases like when a scope becomes stale.
/// </summary>
private const int StabilityRetryCount = 3;

public static Task ExecuteLoggedAsync(
this UITestContext context, string operationName, IWebElement element, Func<Task> functionAsync) =>
context.ExecuteSectionAsync(GetLogSection(operationName, element), functionAsync);
Expand Down Expand Up @@ -87,10 +93,41 @@ private static string GetSectionMessage(string operationName, string objectOfOpe
$"{operationName} applied to: {Environment.NewLine}{objectOfOperation}";

private static void ExecuteSection(this UITestContext context, LogSection section, Action action) =>
context.Scope.AtataContext.Log.ExecuteSection(section, action);
context.Scope.AtataContext.Log.ExecuteSection(section, () =>
{
for (int i = 0; i < StabilityRetryCount; i++)
{
var notLast = i < StabilityRetryCount - 1;
try
{
action();
return;
}
catch (StaleElementReferenceException) when (notLast)
{
Thread.Sleep(TimeSpan.FromSeconds(1));
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not throw new InvalidOperationException("Impossible to reach."); here? Like below.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the text says, that's impossible to reach. The other methods need it because they expect to return a value so if I don't throw at the end then I get a compile time error even though it's really not a possible execution path.

});

private static TResult ExecuteSection<TResult>(this UITestContext context, LogSection section, Func<TResult> function) =>
context.Scope.AtataContext.Log.ExecuteSection(section, function);
context.Scope.AtataContext.Log.ExecuteSection(section, () =>
{
for (int i = 0; i < StabilityRetryCount; i++)
{
var notLast = i < StabilityRetryCount - 1;
try
{
return function();
}
catch (StaleElementReferenceException) when (notLast)
{
Thread.Sleep(TimeSpan.FromSeconds(1));
}
}

throw new InvalidOperationException("Impossible to reach.");
});

private static Task ExecuteSectionAsync(this UITestContext context, LogSection section, Func<Task> functionAsync) =>
context.ExecuteSectionAsync(
Expand All @@ -104,12 +141,25 @@ private static Task ExecuteSectionAsync(this UITestContext context, LogSection s
private static async Task<TResult> ExecuteSectionAsync<TResult>(
this UITestContext context, LogSection section, Func<Task<TResult>> functionAsync)
{
// This is somewhat risky. ILogManager is not thread-safe and uses as stack to keep track of sections, so if
// multiple sections are started in concurrent threads, the result will be incorrect. This shouldn't be too much
// of an issue for now though since tests, while async, are single-threaded.
context.Scope.AtataContext.Log.Start(section);
var result = await functionAsync();
context.Scope.AtataContext.Log.EndSection();
return result;
for (int i = 0; i < StabilityRetryCount; i++)
{
var notLast = i < StabilityRetryCount - 1;
try
{
// This is somewhat risky. ILogManager is not thread-safe and uses as stack to keep track of sections, so if
// multiple sections are started in concurrent threads, the result will be incorrect. This shouldn't be too much
// of an issue for now though since tests, while async, are single-threaded.
context.Scope.AtataContext.Log.Start(section);
var result = await functionAsync();
context.Scope.AtataContext.Log.EndSection();
return result;
}
catch (StaleElementReferenceException) when (notLast)
{
await Task.Delay(TimeSpan.FromSeconds(1));
}
}

throw new InvalidOperationException("Impossible to reach.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A custom exception would be better or a constant for the message at least.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be an unreasonable effort for a theoretically impossible scenario.

}
}