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

Split up _testinternalcapi.c #108777

Closed
colesbury opened this issue Sep 1, 2023 · 7 comments
Closed

Split up _testinternalcapi.c #108777

colesbury opened this issue Sep 1, 2023 · 7 comments
Labels
tests Tests in the Lib/test dir topic-C-API

Comments

@colesbury
Copy link
Contributor

colesbury commented Sep 1, 2023

#93649 and associated PRs split up _testcapimodule.c because the file is large and hard to maintain. I'm proposing splitting the tests for the internal C API (Modules/_testinternalcapi.c) in the same manner for the same reasons. Additionally, I expect the PRs related to PEP 703 to require additional internal C API tests, and I'd like to avoid further cluttering _testinternalcapi.c.

cc @encukou @vstinner

Linked PRs

@colesbury colesbury added tests Tests in the Lib/test dir topic-C-API labels Sep 1, 2023
@rhettinger
Copy link
Contributor

I don't think this is worthwhile. In some ways, having lots of smaller files is its own bit of hassle. Our editors are powerful enough to make it easy to work with a single large file.

@colesbury
Copy link
Contributor Author

To be clear, I am not proposing breaking up _testinternalcapi.c entirely, only where there's enough related tests to make it worthwhile. I'm not proposing lots of smaller files. I'm proposing a few smaller files that contain test code for related bits of the internal C API, such as for testing the locks and other synchronization primitives in #108724.

@erlend-aasland
Copy link
Contributor

#93649 and associated PRs split up _testcapimodule.c because the file is large and hard to maintain. I'm proposing splitting the tests for the internal C API (Modules/_testinternalcapi.c) in the same manner for the same reasons. Additionally, I expect the PRs related to PEP 703 to require additional internal C API tests, and I'd like to avoid further cluttering _testinternalcapi.c.

Sounds reasonable to me; there's been various efforts the last years that have split up large and unmaintainable files (both .py and .c files) into maintainable chunks.

@vstinner
Copy link
Member

vstinner commented Sep 1, 2023

Can you give a prototype (list of sections) of how you would like to split this C extension? Does it change the Python test_capi test package?

@colesbury
Copy link
Contributor Author

For now, I'd just move the 12 _PyTime tests to their own file. And I'd put the lock-related tests for #108724 in their own file. It would not change the Python test_capi package.

@erlend-aasland
Copy link
Contributor

For now, I'd just move the 12 _PyTime tests to their own file.

Do you want to keep this open?

@colesbury
Copy link
Contributor Author

I think we can close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Tests in the Lib/test dir topic-C-API
Projects
None yet
Development

No branches or pull requests

4 participants