Add wayland & sdl2-compat specific skips #3575
Conversation
📝 WalkthroughWalkthroughDocstrings were updated in pygame stubs to note potential incompatibilities on Wayland and possible pygame.error raises. Tests were modified to detect Wayland and skip affected cases. Two surface tests were gated for newer SDL via sdl2-compat. One window position test now initializes with a dynamic position and asserts against it. Changes
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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.
Add skips to tests that fail on sdl2-compat and/or 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.