Don't fail ci on playwright test setup failure

[adamint]

We shouldn't fail CI build if we are unable to set up and see the dashboard in a certain amount of time (5000ms).

@radical let me know if a better way around this is just to increase the timeout

fixes #4851

Microsoft Reviewers: Open in CodeFlow

[radical]

Could you please help me understand the fix here. What exception is being thrown here? And how does this try/catch fix the issue?

[adamint]

There is a timeout waiting for an element on the dashboard to be ready; I am unable to reproduce it locally. If this occurs during test setup, and not during test execution, we can skip the test, since this seems to occur rarely.

[radical]

This would skip the test, and we'll be missing the coverage it provides.

Looking at the trace from the issue:

  - LocatorAssertions.ToBeVisibleAsync with timeout 5000ms
  - waiting for GetByText("TestResource")
  Stack Trace:
     at Microsoft.Playwright.Core.AssertionsBase.ExpectImplAsync(String expression, FrameExpectOptions expectOptions, Object expected, String message) in /_/src/Playwright/Core/AssertionsBase.cs:line 84
   at Microsoft.Playwright.Core.AssertionsBase.ExpectImplAsync(String expression, ExpectedTextValue[] expectedText, Object expected, String message, FrameExpectOptions options) in /_/src/Playwright/Core/AssertionsBase.cs:line 64
   at Aspire.Dashboard.Tests.Integration.Playwright.PlaywrightFixture.GoToHomeAndWaitForDataGridLoad(IPage page) in /_/tests/Aspire.Dashboard.Tests/Integration/Playwright/PlaywrightFixture.cs:line 28
   at Aspire.Dashboard.Tests.Integration.Playwright.AppBarTests.<AppBar_Change_Theme>b__1_0(IPage page) in /_/tests/Aspire.Dashboard.Tests/Integration/Playwright/AppBarTests.cs:line 25
   at Aspire.Dashboard.Tests.Integration.Playwright.PlaywrightTestsBase.RunTestAsync(Func`2 test) in /_/tests/Aspire.Dashboard.Tests/Integration/Playwright/PlaywrightTestsBase.cs:line 27
   at Aspire.Dashboard.Tests.Integration.Playwright.PlaywrightTestsBase.RunTestAsync(Func`2 test) in /_/tests/Aspire.Dashboard.Tests/Integration/Playwright/PlaywrightTestsBase.cs:line 31
   at Aspire.Dashboard.Tests.Integration.Playwright.AppBarTests.AppBar_Change_Theme() in /_/tests/Aspire.Dashboard.Tests/Integration/Playwright/AppBarTests.cs:line 23
--- End of stack trace from previous location ---

The failure is when GoToHomeAndWaitForDataGridLoad is waiting for the data grid to show up on the page, and it times out. This could be because the page was loading slowly, or something failed on the page causing the grid to never show up. I would suggest checking for that.

aspire/tests/Aspire.Dashboard.Tests/Integration/Playwright/PlaywrightFixture.cs

Lines 25 to 31 in f943b52

	public async Task GoToHomeAndWaitForDataGridLoad(IPage page)
	{
	await page.GotoAsync("/");
	await Assertions
	.Expect(page.GetByText(MockDashboardClient.TestResource1.DisplayName))
	.ToBeVisibleAsync();
	}

Maybe take a screenshot when it times out like this. See

aspire/tests/Aspire.Workload.Tests/StarterTemplateRunTestsBase.cs

Lines 80 to 81 in f943b52

	string screenshotPath = Path.Combine(logPath, "webfrontend-fail.png");
	await page.ScreenshotAsync(new PageScreenshotOptions { Path = screenshotPath });

Also, that method is directly trying to open the dashboard url, but it could take time to become available too. We should wait till the url becomes available, see

aspire/tests/Shared/WorkloadTesting/AspireProject.cs

Lines 298 to 304 in f943b52

	CancellationTokenSource cts = new();
	cts.CancelAfter(TimeSpan.FromSeconds(DashboardAvailabilityTimeoutSecs));
	await WaitForDashboardToBeAvailableAsync(dashboardUrlToUse, _testOutput, cts.Token).ConfigureAwait(false);
	var dashboardPage = await context.NewPageWithLoggingAsync(_testOutput);
	await dashboardPage.GotoAsync(dashboardUrlToUse);

[adamint]

I will re-add a wait function, I had removed it after I received feedback to do so.

[adamint]

Actually, just increasing the timeout significantly might be good enough.

[radical]

This is only set for helix jobs. On the build machine we want to use artifacts/log directory, which gets collected into Artifacts shown in azdo.

[adamint]

BuildEnvironment uses artifacts/logs, I'll do the same

[radical]

We should let the test fail, and the screenshot can be used for debugging the failure. I don't think we need to split this into two steps.
If the failure is because we need to wait longer then a longer timeout should be fine. If it's not that then maybe something is failing on the page.

[radical]

This path won't get picked up by azdo. If we place the screenshot in artifacts/log then it will get collected.

[adamint]

So correct me if I'm wrong, but then logs just needs to become log? As AppContext.BaseDirectory will be artifacts? @radical

[radical]

AppContext.BaseDirectory would be a path like artifacts/bin/Aspire.Dashboard.Tests/Debug/net8.0/ IIUC.

[adamint]

I see. Changed

[radical]

This will create a artifacts/log path under your bindir.

aspire/tests/Aspire.Hosting.Tests/MSBuildTests.cs

Lines 126 to 137 in 4938cb3

	private static string GetRepoRoot()
	{
	string directory = AppContext.BaseDirectory;
	// To support git worktrees, check for either a directory or a file named ".git"
	while (directory != null && !Directory.Exists(Path.Combine(directory, ".git")) && !File.Exists(Path.Combine(directory, ".git")))
	{
	directory = Directory.GetParent(directory)!.FullName;
	}
	return directory!;
	}

This method can be used to find the repo root, and then + "/artifacts/log/" should get you the correct path. It might be useful to move this method somewhere usable by other tests too.

Also, try inducing a failure and check that you can retrieve the screenshot from the logs.