Skip to content

Commit

Permalink
Add retry logic to stabilize flaky UI tests (#3180)
Browse files Browse the repository at this point in the history 
* add retry logic to StartAndVerifyProcessesAreRunning

* revert change that caused builds to fail again

* change requests

* fix spacing

* address comments
  • Loading branch information
@kllysng
kllysng authored Jan 7, 2025
1 parent 2dd962b commit d83d251
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 20 deletions.
9 changes: 4 additions & 5 deletions tests/E2E Tests/WebAppUiTests/B2CWebAppCallsWebApiLocally.cs
View file
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
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
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)
{
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));

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();

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
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

0 comments on commit d83d251

Please sign in to comment.