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

Refactor integration test app paging #16551

Merged
merged 3 commits into from
Aug 3, 2024
Merged

Conversation

grokys
Copy link
Member

@grokys grokys commented Jul 31, 2024

What does the pull request do?

#15542 started causing integration tests to fail on Windows and I had no idea why until I managed to get a screen recording (see #16546). Turns out that our integration tests run on a really small screen and adding the "Embedding" tab caused our tabs to expand to 3 columns, leaving little room for the content:

image

Besides that, all tabs being defined in the MainWindow.xaml file was getting a bit unwieldy anyway.

This PR refactors the IntegrationTestApp to use a ListBox to handle the paging (which will ensure that the tabs don't grow horizontally) and moves each page into its own view (to ease of mantenence):

image

Because paging now potentially requires scrolling to offscreen elements, I had to add support for the UIA IsOffscreen property in order for WinAppDriver to scroll elements into view.

It also moves selecting the page in the test project to a base class as we may need to handle scrolling manually on macOS at some point (Appium on macOS doesn't scroll elements into view automatically) - this isn't a problem yet as the items fit comfortably on-screen on macOS.

grokys added 2 commits July 31, 2024 13:21
Use a `ListBox` to switch pages instead of a `TabControl`: the `TabControl` didn't adapt well to smaller screen sizes, and the `MainWindow` was getting unwieldy anyway.
Move logic for selecting the page to a base class as we may need to handle scrolling manually on macOS at some point (Appium on macOS doesn't scroll elements into view automatically).
@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0050772-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0050776-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@grokys grokys force-pushed the fixes/integration-test-paging branch 2 times, most recently from 8091168 to 0c4f6b5 Compare July 31, 2024 22:12
This is needed in order for controls to be scrolled into view using WinAppDriver. The default is the same as WPF and the default value is overridden in the same controls as WPF (where present).
@grokys grokys force-pushed the fixes/integration-test-paging branch from 0c4f6b5 to 0e7aad2 Compare July 31, 2024 23:49
@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0050792-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@grokys grokys marked this pull request as ready for review August 2, 2024 08:28
Copy link
Member

@MrJul MrJul left a comment

Choose a reason for hiding this comment

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

LGTM! Hopefully integration tests will be more stable with these changes.

A quick question about the AutomationProperties.IsOffscreenBehavior property, as I noticed it's been overridden to FromClip in several places. Why isn't FromClip the default? It seems to represent "the right thing" when thinking about offscreen elements.

@MrJul MrJul added this pull request to the merge queue Aug 3, 2024
@MrJul MrJul added area-infrastructure Issues related to CI/tooling infrastructur enhancement labels Aug 3, 2024
Merged via the queue into master with commit fa8b173 Aug 3, 2024
12 checks passed
@MrJul MrJul deleted the fixes/integration-test-paging branch August 3, 2024 09:34
@grokys
Copy link
Member Author

grokys commented Aug 4, 2024

Why isn't FromClip the default? It seems to represent "the right thing" when thinking about offscreen elements

It's a very good question and I don't know the answer; WPF and UWP do it like this so I just copied them, which is what we generally do as a default these days.

@grokys grokys added the backport-candidate-11.1.x Consider this PR for backporting to 11.1 branch label Aug 5, 2024
@maxkatz6 maxkatz6 added the customer-priority Issue reported by a customer with a support agreement. label Aug 12, 2024
maxkatz6 pushed a commit that referenced this pull request Aug 12, 2024
* Refactor IntegrationTestApp.

Use a `ListBox` to switch pages instead of a `TabControl`: the `TabControl` didn't adapt well to smaller screen sizes, and the `MainWindow` was getting unwieldy anyway.

* Update tests to use new pager.

Move logic for selecting the page to a base class as we may need to handle scrolling manually on macOS at some point (Appium on macOS doesn't scroll elements into view automatically).

* Add AutomationPeer.IsOffscreen.

This is needed in order for controls to be scrolled into view using WinAppDriver. The default is the same as WPF and the default value is overridden in the same controls as WPF (where present).
#Conflicts:
#	samples/IntegrationTestApp/App.axaml.cs
#	samples/IntegrationTestApp/MainWindow.axaml
#	samples/IntegrationTestApp/MainWindow.axaml.cs
#	samples/IntegrationTestApp/TopmostWindowTest.axaml.cs
#	src/Avalonia.Controls/TreeViewItem.cs
#	tests/Avalonia.IntegrationTests.Appium/ContextMenuTests.cs
#	tests/Avalonia.IntegrationTests.Appium/PointerTests.cs
#	tests/Avalonia.IntegrationTests.Appium/ScreenTests.cs
#	tests/Avalonia.IntegrationTests.Appium/TrayIconTests.cs
#	tests/Avalonia.IntegrationTests.Appium/WindowDecorationsTests.cs
#	tests/Avalonia.IntegrationTests.Appium/WindowTests.cs
#	tests/Avalonia.IntegrationTests.Appium/WindowTests_MacOS.cs
@maxkatz6 maxkatz6 added backported-11.1.x and removed backport-candidate-11.1.x Consider this PR for backporting to 11.1 branch labels Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Issues related to CI/tooling infrastructur backported-11.1.x customer-priority Issue reported by a customer with a support agreement. enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants