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

Fix BlazorWebView tests on Windows #16487

Merged
merged 1 commit into from Aug 3, 2023
Merged

Conversation

Eilon
Copy link
Member

@Eilon Eilon commented Aug 1, 2023

It seems that the test sometimes waits for an event that already happened, so it doesn't see it happen, and fails. I added a check to evaluate the state of the test to see if it doesn't need to wait for the event and allows the test to continue.

Fixes #16187

It seems that the test sometimes waits for an event that already happened, so it doesn't see it happen, and fails. I added a check to evaluate the state of the test to see if it doesn't need to wait for the event and allows the test to continue.

Fixes #16187
@Eilon Eilon requested a review from a team as a code owner August 1, 2023 21:37
@@ -27,6 +27,10 @@ public void EnsureHandlerCreated(Action<MauiAppBuilder> additionalCreationAction
var appBuilder = MauiApp
.CreateBuilder();

appBuilder.Services.AddSingleton<IDispatcherProvider>(svc => TestDispatcher.Provider);
Copy link
Member Author

Choose a reason for hiding this comment

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

This was to fix some issue that only happens in some environments. I updated this setup code to match what other tests do.

// It's possible that the DOMContentLoaded event won't fire because it's already loaded by the time we got here. To check
// for that, we inspect an arbitrary custom HTML element attribute to see if we can find it. If we can find it, then surely
// the DOM content is loaded, so we can continue with the test.
var testHtmlLoadedAttributeValue = await wv2.CoreWebView2.ExecuteScriptAsync("(document.head.attributes['testhtmlloaded']?.value === 'true')");
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the actual fix.

@Eilon Eilon added the area/blazor 🕸️ Blazor Hybrid / Desktop, BlazorWebView label Aug 1, 2023
@Eilon
Copy link
Member Author

Eilon commented Aug 3, 2023

@dotnet/dotnet-maui-blazor-eng - need one of you to sign off as well.

@Eilon Eilon merged commit 79117f9 into main Aug 3, 2023
39 checks passed
@Eilon Eilon deleted the eilon/fix-blazorwebview-tests branch August 3, 2023 15:58
Copy link
Member

@SteveSandersonMS SteveSandersonMS left a comment

Choose a reason for hiding this comment

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

I understand the DOMContentLoaded fix but don't know what the other changes are for. It sounds like you know what you're doing though.

@Eilon
Copy link
Member Author

Eilon commented Aug 3, 2023

I understand the DOMContentLoaded fix but don't know what the other changes are for. It sounds like you know what you're doing though.

The other fixes were to simply make the tests work on Windows at all. These tests previously never ran on Windows (MAUI tests only ran on Android/iOS devices). At some point various MAUI test projects were updated to work on Windows, but not the Blazor ones. Part of the changes in this PR are to bring the Blazor tests up-to-date with other MAUI tests (I just copied their setup code).

@Eilon
Copy link
Member Author

Eilon commented Aug 3, 2023

@SteveSandersonMS said:

... It sounds like you know what you're doing though.

The highest compliment 😁 But maybe I have you fooled??

@github-actions github-actions bot locked and limited conversation to collaborators Dec 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/blazor 🕸️ Blazor Hybrid / Desktop, BlazorWebView platform/windows 🪟
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Windows, Blazor] Review currently failing Windows tests
6 participants