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

Increase BlazorWebView test timeouts #4543

Merged
merged 3 commits into from
Feb 8, 2022
Merged

Conversation

SteveSandersonMS
Copy link
Member

Increases BlazorWebView test timeouts to 7.5s. Previously they were 500ms (Windows) or 2s (Android/iOS). Hopefully this will mitigate test flakiness, but we have to try it to see.

@SteveSandersonMS SteveSandersonMS requested a review from a team as a code owner February 7, 2022 13:50
@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@@ -9,8 +9,8 @@ namespace Microsoft.Maui.MauiBlazorWebView.DeviceTests
{
public static class WebViewHelpers
{
const int MaxWaitTimes = 10;
const int WaitTimeInMS = 200;
const int MaxWaitTimes = 30;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use TimeSpan instead of these?

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const int MaxWaitTimes = 30;
static readonly TimeSpan MaxWaitTimes = TimeSpan.FromSeconds(30);

Copy link
Member Author

Choose a reason for hiding this comment

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

Many things about how these tests are written could be refactored and cleaned up. The way the timeout is represented is one of the least of the changes :) I know they were originally written only as a proof-of-concept that such tests are possible and can be made to run in CI.

So I'd rather wait until we make the test infrastructure nice here before being concerned with such a detail.

Copy link
Member

@Eilon Eilon left a comment

Choose a reason for hiding this comment

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

No feelings of mine can be hurt regarding this 😄

@Eilon Eilon merged commit 8c3fb0f into main Feb 8, 2022
@Eilon Eilon deleted the stevesa/increase-test-timeouts branch February 8, 2022 04:16
@Eilon
Copy link
Member

Eilon commented Feb 8, 2022

I went ahead and merged so that by morning time we can hopefully see if reliability has gone up. Thank you @SteveSandersonMS !

@samhouts samhouts added the area-testing Unit tests, device tests label Jul 11, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Dec 21, 2023
@samhouts samhouts added the fixed-in-6.0.200-preview.14.2 Look for this fix in 6.0.200-preview.14.2! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-testing Unit tests, device tests fixed-in-6.0.200-preview.14.2 Look for this fix in 6.0.200-preview.14.2!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants