Skip to content

fix: assert uniqueness by origin (#3298)#4428

Merged
freakboy3742 merged 3 commits into
beeware:mainfrom
audreydng:3298-linux-wayland-duplicate-monitors
May 29, 2026
Merged

fix: assert uniqueness by origin (#3298)#4428
freakboy3742 merged 3 commits into
beeware:mainfrom
audreydng:3298-linux-wayland-duplicate-monitors

Conversation

@audreydng
Copy link
Copy Markdown
Contributor

@audreydng audreydng commented May 27, 2026

Fix test_screens to handle duplicate monitor names on Linux Wayland by asserting uniqueness on origin instead of name, and skip the screen[0] == (0, 0) check on Linux Wayland since it is not guaranteed.

Fixes #3298

PR Checklist:

  • I will abide by the BeeWare Code of Conduct
  • I have read and have followed the CONTRIBUTING.md file
  • This PR was generated or assisted using an AI tool.

@audreydng audreydng force-pushed the 3298-linux-wayland-duplicate-monitors branch 2 times, most recently from 8c6cbbb to 7af239c Compare May 27, 2026 20:48
@audreydng audreydng force-pushed the 3298-linux-wayland-duplicate-monitors branch from 1584703 to 9f1d405 Compare May 27, 2026 20:53
Copy link
Copy Markdown
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Looks like you've found a good workaround to the problem described by the original ticket.

The only feedback on the PR is that we try to avoid having platform-specific details in the testbed tests themselves, instead describing platform-specific details in terms of properties that individual probes can enable or disable as required.

However, in this case, I don't think there's a need for that. As noted inline, the key detail we need to assert is that the monitor origins are unique - there's no hard requirement that one of them is at (0,0).

So - if you can remove the "optional (0,0)" test, and remove the reference to Wayland in the test comment, this should be good to land!

@audreydng audreydng force-pushed the 3298-linux-wayland-duplicate-monitors branch from 09a2fc9 to 9cfd80a Compare May 28, 2026 07:43
@audreydng
Copy link
Copy Markdown
Contributor Author

Thanks for the PR. Looks like you've found a good workaround to the problem described by the original ticket.

The only feedback on the PR is that we try to avoid having platform-specific details in the testbed tests themselves, instead describing platform-specific details in terms of properties that individual probes can enable or disable as required.

However, in this case, I don't think there's a need for that. As noted inline, the key detail we need to assert is that the monitor origins are unique - there's no hard requirement that one of them is at (0,0).

So - if you can remove the "optional (0,0)" test, and remove the reference to Wayland in the test comment, this should be good to land!

Thanks for the feedback! I've removed the (0, 0) test.

@audreydng audreydng force-pushed the 3298-linux-wayland-duplicate-monitors branch 2 times, most recently from 932aa06 to 9ecff5e Compare May 28, 2026 07:55
@audreydng audreydng changed the title fix: assert uniqueness by origin, skip (0,0) check on Wayland (#3298) fix: assert uniqueness by origin (#3298) May 28, 2026
@audreydng audreydng force-pushed the 3298-linux-wayland-duplicate-monitors branch from 9ecff5e to 09cf370 Compare May 28, 2026 07:58
@audreydng audreydng force-pushed the 3298-linux-wayland-duplicate-monitors branch from 09cf370 to a2d8613 Compare May 28, 2026 08:03
@audreydng audreydng requested a review from freakboy3742 May 28, 2026 08:10
@audreydng audreydng force-pushed the 3298-linux-wayland-duplicate-monitors branch from a02b9c6 to a2d8613 Compare May 28, 2026 18:20
Copy link
Copy Markdown
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

Thanks for that update - there was also a need to simplify the test comment; I've pushed an update to that effect (plus a simplification of the change note).

Thanks for the fix!

@freakboy3742 freakboy3742 merged commit e4f3d43 into beeware:main May 29, 2026
64 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Linux Wayland: Test suite fails with two identical monitors

2 participants