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

Try to fix flaky window decorations tests on Windows. #16597

Merged
merged 1 commit into from
Aug 5, 2024

Conversation

grokys
Copy link
Member

@grokys grokys commented Aug 5, 2024

What does the pull request do?

As mentioned in #16546 (comment), toggling the window decorations on win32 can cause the window to be moved off screen, causing integration test failures. Until this bug is fixed, detect this and move the window to the screen origin (see #11411).

The alternative would be to increase the screen size for integration tests on CI, which would make this problem less likely, but still possible. Unless this hackfix causes other problems it should be more reliable than changing the screen size.

Toggling the window decorations can cause the window to be moved off screen, causing integration test failures. Until this bug is fixed, detect this and move the window to the screen origin. See #11411.
@grokys grokys changed the title Try to fix flaky window decorations tests. Try to fix flaky window decorations tests on Windows. Aug 5, 2024
@avaloniaui-bot
Copy link

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

@grokys grokys marked this pull request as ready for review August 5, 2024 13:19
@MrJul
Copy link
Member

MrJul commented Aug 5, 2024

Regarding the screen size, it's already supposed to be changed:

AgentResolution: '4K'

Not sure if that ever worked, or if CI agents don't support that resolution.

@avaloniaui-bot
Copy link

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

e.Point,
PixelSize.FromSize(ClientSize, DesktopScaling));

if (!screen.WorkingArea.Contains(bounds))
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't only the position be verified instead of the bounds? This might cause an infinite loop if the window somehow doesn't fit into the working area,

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem is when the bottom of the window is moved below the taskbar so we need to verify the bounds (the problem is that this causes the taskbar to be clicked instead of the window).

I guess we should also add a check that the window size is smaller than the working area though to prevent such an infinite loop.

@grokys
Copy link
Member Author

grokys commented Aug 5, 2024

Regarding the screen size, it's already supposed to be changed

Nope, that doesn't seem to have worked as you can see from the recorded videos. I guess we can remove that line.

@maxkatz6 maxkatz6 added the area-infrastructure Issues related to CI/tooling infrastructur label Aug 5, 2024
Copy link
Member

@maxkatz6 maxkatz6 left a comment

Choose a reason for hiding this comment

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

Merging in hopes to get greens (CI greens)

@maxkatz6 maxkatz6 added this pull request to the merge queue Aug 5, 2024
@maxkatz6 maxkatz6 removed this pull request from the merge queue due to a manual request Aug 5, 2024
@maxkatz6 maxkatz6 merged commit c71fa11 into master Aug 5, 2024
12 checks passed
@maxkatz6 maxkatz6 deleted the fixes/flaky-window-decorations-tests branch August 5, 2024 21:40
maxkatz6 pushed a commit that referenced this pull request Aug 12, 2024
      Toggling the window decorations can cause the window to be moved off screen, causing integration test failures. Until this bug is fixed, detect this and move the window to the screen origin. See #11411.
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants