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 Disabling some image tests after updating net9.0 with main #25770

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public MainPage()

popOnAppearing.Appearing += async (sender, args) =>
{
await Task.Yield();
await Task.Delay(50);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could just include a View, like a Label, and wait for it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the test case host app, a new page is being pushed and then immediately popped on the page's Appearing event. As a result, the page's UI does not load, making it impossible to validate views like labels. Here, added a slight delay before popping the page in the Appearing event to avoid exception.

Copy link
Member

Choose a reason for hiding this comment

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

What was the exception? Adding a delay to fix the issue may mean something else went wrong.

Copy link
Member

Choose a reason for hiding this comment

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

Could we wire into an event on the page we're popping back to and add a label?

like

  • push
  • pop from appearing
  • and then incremented a counter or inside "OnNavigatedTo" we could add a label indicating it was popped back to?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The cause of the issue is performing the pop in the Page.Appearing event. As an alternative, I tried this approach:
image
This checks the Modal stack before popping, similar to a push-then-pop pattern. Can we proceed with this approach?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe this should be a different test case? If there is a problem popping from Appearing, do we want to fix that of just say "don't do that"?

We can still use your idea of a label update because the NavigatedTo will fire again on the first/initial page? When you pop the new page, we can increment the counter.

Copy link
Member

Choose a reason for hiding this comment

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

Also, if this is because of animations, then we will have to document that you can't pop in Appearing while pushing.

await popOnAppearing.Navigation.PopModalAsync();
};

Expand All @@ -59,6 +59,7 @@ public MainPage()
}
};

await Task.Delay(50);
await intermediatePage.Navigation.PushModalAsync(popOnAppearing);

await page1.Navigation.PushModalAsync(intermediatePage);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ public Issue14471(TestDevice device) : base(device) { }

[Test]
[Category(UITestCategories.Image)]
[FailsOnAndroidWhenRunningOnXamarinUITest("Suddenly failing. https://github.com/dotnet/maui/issues/24243")]
public void ImageDoesntDisappearWhenNavigatingBack()
{
App.WaitForElement("image");
Expand Down
Loading