Add wayland specific skips #3575
Conversation
📝 WalkthroughWalkthroughAdds Wayland compatibility notes to pygame display/window stub docstrings and introduces Wayland detection in tests to conditionally skip features not supported by the Wayland video driver. One window position test now uses a dynamic position value. No API signatures or runtime code changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10–15 minutes Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (8)
buildconfig/stubs/pygame/display.pyi (1)
748-750: Docstring note on Wayland support looks goodClear and accurate. No API change; the :exc: markup is correct. Consider mirroring this note anywhere else you document window positioning to keep consistency across the API.
test/surface_test.py (2)
379-383: Confirm version gate matches real sdl2-compat/SDL3 runtime valuesPlease verify that pygame.version.SDL indeed reports >= (2, 32, 50) for all sdl2-compat builds you target and all SDL3 builds, so these skips reliably trigger where intended. If needed, centralize this predicate into a helper for reuse.
412-416: Same verification for second SDL gateAs above, double-check the threshold, and consider a shared helper constant (e.g., IS_SDL3_OR_COMPAT) to avoid divergence across tests.
buildconfig/stubs/pygame/window.pyi (1)
225-227: Wayland compatibility note for always_on_topGood addition; matches observed backend behavior. Suggest aligning wording with other notes (“e.g., Wayland”) for consistency.
test/display_test.py (2)
78-79: Wayland-only skip addedAppropriate skip; consider standardizing the message (“not supported on wayland”) across tests.
415-420: Skip iconify on WaylandReasonable. Same note on message consistency if you care.
test/window_test.py (2)
14-17: Preserve display init state when detecting the backendUnconditionally quitting the display after pygame.init() mutates global state and can impact later tests. Preserve/restore only if we had to init it here.
-pygame.display.init() -is_wayland = pygame.display.get_driver() == "wayland" -pygame.display.quit() +_prev_inited = pygame.display.get_init() +if not _prev_inited: + pygame.display.init() +is_wayland = pygame.display.get_driver() == "wayland" +if not _prev_inited: + pygame.display.quit()
109-109: Capitalize “Wayland” in skip reasons for consistencyMinor wording polish in user-facing skip messages.
-@unittest.skipIf(is_wayland, "not supported on wayland") +@unittest.skipIf(is_wayland, "not supported on Wayland")Also applies to: 157-157, 264-264
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
buildconfig/stubs/pygame/display.pyi(1 hunks)buildconfig/stubs/pygame/window.pyi(2 hunks)test/display_test.py(6 hunks)test/event_test.py(3 hunks)test/surface_test.py(2 hunks)test/window_test.py(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-14T02:59:43.863Z
Learnt from: oddbookworm
PR: pygame-community/pygame-ce#0
File: :0-0
Timestamp: 2025-08-14T02:59:43.863Z
Learning: In pygame-ce's C unit testing framework using Unity, the RUN_TEST_PG_INTERNAL macro automatically handles setUp() and tearDown() calls for each test execution, providing automatic test isolation without requiring explicit reset calls between tests.
Applied to files:
test/window_test.py
🧬 Code graph analysis (3)
test/window_test.py (3)
buildconfig/stubs/pygame/display.pyi (3)
init(105-138)get_driver(345-353)quit(140-148)buildconfig/stubs/pygame/window.pyi (3)
Window(11-500)position(297-305)position(308-308)src_py/__init__.py (1)
Window(380-381)
test/event_test.py (1)
buildconfig/stubs/pygame/display.pyi (3)
init(105-138)get_driver(345-353)quit(140-148)
test/display_test.py (2)
src_c/display.c (1)
display(3884-3929)buildconfig/stubs/pygame/display.pyi (3)
init(105-138)get_driver(345-353)quit(140-148)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: msys2 (mingw64, x86_64)
- GitHub Check: msys2 (ucrt64, ucrt-x86_64)
- GitHub Check: msys2 (clang64, clang-x86_64)
- GitHub Check: build (ubuntu-22.04)
- GitHub Check: build (windows-latest)
- GitHub Check: build (macos-14)
- GitHub Check: build (ubuntu-24.04)
- GitHub Check: dev-check
- GitHub Check: i686
- GitHub Check: AMD64
- GitHub Check: x86_64
- GitHub Check: x86
- GitHub Check: aarch64
- GitHub Check: debug_coverage (ubuntu-24.04, 3.13.5)
- GitHub Check: debug_coverage (ubuntu-24.04, 3.9.23)
- GitHub Check: debug_coverage (ubuntu-24.04, 3.14.0rc1)
- GitHub Check: Debian (Bookworm - 12) [armv7]
- GitHub Check: Debian (Bookworm - 12) [s390x]
- GitHub Check: Debian (Bookworm - 12) [armv6]
- GitHub Check: Debian (Bookworm - 12) [ppc64le]
🔇 Additional comments (8)
buildconfig/stubs/pygame/window.pyi (2)
303-305: Wayland compatibility note for positionLooks correct and consistent with display.set_window_position doc. No further action.
310-314: Expanded opacity doc with Wayland caveatAccurate and helpful. Thanks for making this explicit.
test/display_test.py (3)
528-529: Gamma test skip on Wayland with SDL3/sdl2-compat gateGood gating. Keep this in sync with any future backend behavior changes.
547-548: Tuple gamma test skip mirrors single-value caseLooks consistent with the above.
701-708: Window position test skipped on WaylandMatches the documented limitation; test logic otherwise remains valid.
test/event_test.py (2)
842-847: Wayland skip for set_grab testsAppropriate; grabbing is not consistently supported on Wayland compositors.
913-916: Wayland skip for get_grab symmetry testConsistent with the above; LGTM.
test/window_test.py (1)
169-170: LGTM: more robust position assertionUsing the current position to derive a new one avoids brittle hard-coded coords and should behave consistently across WMs.
ankith26
force-pushed
the
ankith26-wayland-skips
branch
from
91dddfe to
2b6cd9c
Compare
ankith26
changed the title
Add skips to tests that fail on wayland. With this PR, we now have all tests passing on wayland on both sdl2 and sdl2-compat.
Also adds documentation to the API that is known to not work with wayland.