Skip to content

ctypes.Union & ctypes.Structure: common features should be tested on both types #124570

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

Closed
encukou opened this issue Sep 26, 2024 · 2 comments
Closed
Assignees

Comments

@encukou
Copy link
Member

encukou commented Sep 26, 2024

Currently, Lib/test/test_ctypes/test_structure.py is very long, but Lib/test/test_ctypes/test_union.py only has recent additions.
A lot of the Structure tests apply to Union as well, and should be run on both. My plan is:

  • Move tests of common features to Lib/test/test_ctypes/test_structunion.py, and use the “common base class + two subclasses” pattern used for tests of similar classes
  • Keep the Structure-specific tests in Lib/test/test_ctypes/test_structure.py

Linked PRs

@encukou encukou self-assigned this Sep 26, 2024
@rruuaanng
Copy link
Contributor

It seems like the workload is quite heavy. I don't think it's necessary just to organize an almost stable and unchanging module.

encukou added a commit to encukou/cpython that referenced this issue Oct 4, 2024
- Move some Structure tests to test_structunion; use a common base
  test class + two subclasses to run them on Union too
- Remove test_union for now as it's redundant

Note: `test_simple_structs` & `test_simple_unions` are in the common
file because they share `formats`.
@encukou
Copy link
Member Author

encukou commented Oct 4, 2024

But ctypes isn't stable and unchanging. In 3.14 the struct/union packing algorithm is replaced and moved to Python code. That on its own deserves better Union tests.

encukou added a commit that referenced this issue Oct 10, 2024
- Move some Structure tests to test_structunion; use a common base
  test class + two subclasses to run them on Union too
- Remove test_union for now as it's redundant

Note: `test_simple_structs` & `test_simple_unions` are in the common
file because they share `formats`.
@encukou encukou closed this as completed Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants