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.

Sign up for GitHub

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

Add retry logic to stabilize flaky UI tests #3180

Merged
merged 5 commits into from
Jan 7, 2025
Merged

Add retry logic to stabilize flaky UI tests #3180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e73255b
add retry logic to StartAndVerifyProcessesAreRunning
kllysng Jan 6, 2025
d82fdea
revert change that caused builds to fail again
kllysng Jan 6, 2025
9dbcc4c
change requests
kllysng Jan 7, 2025
204e4aa
fix spacing
kllysng Jan 7, 2025
e11c87e
address comments
kllysng Jan 7, 2025
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
9 changes: 4 additions & 5 deletions tests/E2E Tests/WebAppUiTests/B2CWebAppCallsWebApiLocally.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using System.Diagnostics;
using System.IO;
using System.Runtime.Versioning;
using System.Threading;
using System.Threading.Tasks;
using Azure.Identity;
using Microsoft.Identity.Lab.Api;
Expand Down Expand Up @@ -42,7 +41,7 @@ public B2CWebAppCallsWebApiLocally(ITestOutputHelper output)

[Fact]
[SupportedOSPlatform("windows")]
public async Task Susi_B2C_LocalAccount_TodoAppFucntionsCorrectlyAsync()
public async Task Susi_B2C_LocalAccount_TodoAppFunctionsCorrectlyAsync()
{
// Web app and api environmental variable setup.
DefaultAzureCredential azureCred = new();
Expand Down Expand Up @@ -77,11 +76,11 @@ public async Task Susi_B2C_LocalAccount_TodoAppFucntionsCorrectlyAsync()
{
// Start the web app and api processes.
// The delay before starting client prevents transient devbox issue where the client fails to load the first time after rebuilding.
serviceProcess = UiTestHelpers.StartProcessLocally(_testAssemblyPath, _devAppPath + TC.s_todoListServicePath, TC.s_todoListServiceExe, serviceEnvVars);
serviceProcess = UiTestHelpers.StartProcessLocally(_testAssemblyPath, _devAppPath + TC.s_todoListServicePath, TC.s_todoListServiceExe, _output, serviceEnvVars);
await Task.Delay(3000);
clientProcess = UiTestHelpers.StartProcessLocally(_testAssemblyPath, _devAppPath + TC.s_todoListClientPath, TC.s_todoListClientExe, clientEnvVars);
clientProcess = UiTestHelpers.StartProcessLocally(_testAssemblyPath, _devAppPath + TC.s_todoListClientPath, TC.s_todoListClientExe, _output, clientEnvVars, 5);

if (!UiTestHelpers.ProcessesAreAlive(new List<Process>() { clientProcess, serviceProcess }))
if (!UiTestHelpers.ProcessesAreAlive([clientProcess, serviceProcess]))
{
Assert.Fail(TC.WebAppCrashedString);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/E2E Tests/WebAppUiTests/TestingWebAppLocally.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ private async Task ExecuteWebAppCallsGraphFlowAsync(string upn, string credentia

try
{
process = UiTestHelpers.StartProcessLocally(_uiTestAssemblyLocation, _devAppPath, _devAppExecutable, clientEnvVars);
process = UiTestHelpers.StartProcessLocally(_uiTestAssemblyLocation, _devAppPath, _devAppExecutable, _output, clientEnvVars);

if (!UiTestHelpers.ProcessIsAlive(process))
{ Assert.Fail(TC.WebAppCrashedString); }
Expand Down
37 changes: 29 additions & 8 deletions tests/E2E Tests/WebAppUiTests/UiTestHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
using Azure.Core;
using Azure.Security.KeyVault.Secrets;
using Microsoft.Identity.Web.Test.Common;
using Microsoft.IdentityModel.Tokens;
using Microsoft.Playwright;
using Xunit.Abstractions;

Expand Down Expand Up @@ -142,8 +141,15 @@ await page.Context.Tracing.StartAsync(new()
/// <param name="executableName">The name of the executable that launches the process.</param>
/// <param name="portNumber">The port for the process to listen on.</param>
/// <param name="isHttp">If the launch URL is http or https. Default is https.</param>
/// <param name="maxRetries">Optionally, maximum number of retries if the process exited prematurely.</param>
/// <returns>The started process.</returns>
public static Process StartProcessLocally(string testAssemblyLocation, string appLocation, string executableName, Dictionary<string, string>? environmentVariables = null)
public static Process StartProcessLocally(
string testAssemblyLocation,
string appLocation,
string executableName,
ITestOutputHelper output,
Dictionary<string, string>? environmentVariables = null,
int maxRetries = 0)
kllysng marked this conversation as resolved.
Show resolved Hide resolved
{
string applicationWorkingDirectory = GetApplicationWorkingDirectory(testAssemblyLocation, appLocation);
ProcessStartInfo processStartInfo = new ProcessStartInfo(applicationWorkingDirectory + executableName)
Expand All @@ -161,14 +167,27 @@ public static Process StartProcessLocally(string testAssemblyLocation, string ap
}
}

Process? process = Process.Start(processStartInfo);
var currentAttempt = 1;
Process? process;
do
{
Thread.Sleep(1000 * currentAttempt++); // linear backoff
process = Process.Start(processStartInfo);
} while (currentAttempt++ <= maxRetries && ProcessIsAlive(process));
kllysng marked this conversation as resolved.
Show resolved Hide resolved

if (process == null)
{
throw new Exception($"Could not start process {executableName}");
}
else
{
// Log the output and error streams
process.OutputDataReceived += (sender, e) => output.WriteLine(e.Data);
process.ErrorDataReceived += (sender, e) => output.WriteLine(e.Data);

process.BeginOutputReadLine();
process.BeginErrorReadLine();
kllysng marked this conversation as resolved.
Show resolved Hide resolved

return process;
}
}
Expand Down Expand Up @@ -275,9 +294,9 @@ public static bool ProcessesAreAlive(List<Process> processes)
/// </summary>
/// <param name="process">Process to check</param>
/// <returns>True if alive false if not</returns>
public static bool ProcessIsAlive(Process process)
public static bool ProcessIsAlive(Process? process)
{
return !process.HasExited;
return process != null && !process.HasExited;
}

/// <summary>
Expand Down Expand Up @@ -310,7 +329,7 @@ internal static async Task<string> GetValueFromKeyvaultWitDefaultCredsAsync(Uri
return (await client.GetSecretAsync(keyvaultSecretName)).Value.Value;
}

internal static bool StartAndVerifyProcessesAreRunning(List<ProcessStartOptions> processDataEntries, out Dictionary<string, Process> processes)
internal static bool StartAndVerifyProcessesAreRunning(List<ProcessStartOptions> processDataEntries, ITestOutputHelper output, out Dictionary<string, Process> processes)
{
processes = new Dictionary<string, Process>();

Expand All @@ -321,6 +340,7 @@ internal static bool StartAndVerifyProcessesAreRunning(List<ProcessStartOptions>
processDataEntry.TestAssemblyLocation,
processDataEntry.AppLocation,
processDataEntry.ExecutableName,
output,
processDataEntry.EnvironmentVariables);

processes.Add(processDataEntry.ExecutableName, process);
Expand All @@ -332,7 +352,7 @@ internal static bool StartAndVerifyProcessesAreRunning(List<ProcessStartOptions>
{
if (!UiTestHelpers.ProcessesAreAlive(processes.Values.ToList()))
{
RestartProcesses(processes, processDataEntries);
RestartProcesses(processes, processDataEntries , output);
}
}

Expand All @@ -344,7 +364,7 @@ internal static bool StartAndVerifyProcessesAreRunning(List<ProcessStartOptions>
return true;
}

static void RestartProcesses(Dictionary<string, Process> processes, List<ProcessStartOptions> processDataEntries)
static void RestartProcesses(Dictionary<string, Process> processes, List<ProcessStartOptions> processDataEntries, ITestOutputHelper output)
{
//attempt to restart failed processes
foreach (KeyValuePair<string, Process> processEntry in processes)
Expand All @@ -356,6 +376,7 @@ static void RestartProcesses(Dictionary<string, Process> processes, List<Process
processDataEntry.TestAssemblyLocation,
processDataEntry.AppLocation,
processDataEntry.ExecutableName,
output,
processDataEntry.EnvironmentVariables);
Thread.Sleep(5000);

Expand Down
10 changes: 4 additions & 6 deletions tests/E2E Tests/WebAppUiTests/WebAppCallsApiCallsGraphLocally.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,14 @@
using System.Collections.Generic;
using System.IO;
using System.Runtime.Versioning;
using System.Threading;
using System.Text;
using System.Threading.Tasks;
using Microsoft.Identity.Lab.Api;
using TC = Microsoft.Identity.Web.Test.Common.TestConstants;
using Microsoft.Playwright;
using Xunit;
using Xunit.Abstractions;
using Process = System.Diagnostics.Process;
using System.Linq;
using System.Text;
using TC = Microsoft.Identity.Web.Test.Common.TestConstants;

namespace WebAppUiTests
#if !FROM_GITHUB_ACTION
Expand Down Expand Up @@ -82,7 +80,7 @@ public async Task ChallengeUser_MicrosoftIdFlow_LocalApp_ValidEmailPasswordCreds
var serviceProcessOptions = new ProcessStartOptions(_testAssemblyLocation, _devAppPath + TC.s_todoListServicePath, TC.s_todoListServiceExe, serviceEnvVars);
var clientProcessOptions = new ProcessStartOptions(_testAssemblyLocation, _devAppPath + TC.s_todoListClientPath, TC.s_todoListClientExe, clientEnvVars);

bool areProcessesRunning = UiTestHelpers.StartAndVerifyProcessesAreRunning(new List<ProcessStartOptions> { /*grpcProcessOptions,*/ serviceProcessOptions, clientProcessOptions }, out processes);
bool areProcessesRunning = UiTestHelpers.StartAndVerifyProcessesAreRunning(new List<ProcessStartOptions> { /*grpcProcessOptions,*/ serviceProcessOptions, clientProcessOptions }, _output, out processes);

if (!areProcessesRunning)
{
Expand Down Expand Up @@ -219,7 +217,7 @@ public async Task ChallengeUser_MicrosoftIdFlow_LocalApp_ValidEmailPasswordCreds
// The delay before starting client prevents transient devbox issue where the client fails to load the first time after rebuilding.
var serviceProcessOptions = new ProcessStartOptions(_testAssemblyLocation, _devAppPathCiam + TC.s_myWebApiPath, TC.s_myWebApiExe, serviceEnvVars);
var clientProcessOptions = new ProcessStartOptions(_testAssemblyLocation, _devAppPathCiam + TC.s_myWebAppPath, TC.s_myWebAppExe, clientEnvVars);
bool areProcessesRunning = UiTestHelpers.StartAndVerifyProcessesAreRunning(new List<ProcessStartOptions> { serviceProcessOptions, clientProcessOptions }, out processes);
bool areProcessesRunning = UiTestHelpers.StartAndVerifyProcessesAreRunning(new List<ProcessStartOptions> { serviceProcessOptions, clientProcessOptions }, _output, out processes);

if (!areProcessesRunning)
{
Expand Down
Loading