Title: Improve Testing For Importing Multiple Extension Modules From One File (and Deal With _testimportmultiple.c) · Issue #124978 · python/cpython · GitHub
Open Graph Title: Improve Testing For Importing Multiple Extension Modules From One File (and Deal With _testimportmultiple.c) · Issue #124978 · python/cpython
X Title: Improve Testing For Importing Multiple Extension Modules From One File (and Deal With _testimportmultiple.c) · Issue #124978 · python/cpython
Description: (low priority) The capability of loading multiple extension modules from a single .so file was added over a decade ago. AFAIK the feature wasn't actually tested at all for several years. The PEP 489 implementation added some test coverag...
Open Graph Description: (low priority) The capability of loading multiple extension modules from a single .so file was added over a decade ago. AFAIK the feature wasn't actually tested at all for several years. The PEP 48...
X Description: (low priority) The capability of loading multiple extension modules from a single .so file was added over a decade ago. AFAIK the feature wasn't actually tested at all for several years. The PE...
Opengraph URL: https://github.com/python/cpython/issues/124978
X: @github
Domain: github.com
{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"Improve Testing For Importing Multiple Extension Modules From One File (and Deal With _testimportmultiple.c)","articleBody":"(low priority)\r\n\r\nThe capability of loading multiple extension modules from a single .so file was added over a decade ago. AFAIK the feature wasn't actually tested at all for several years. The PEP 489 implementation added some test coverage, but only indirectly, superficially, and (perhaps) unintentionally. Even then, the feature wasn't tested for single-phase init modules until a couple years ago and, again, only indirectly, superficially, and unintentionally. (FWIW, I'm not sure the feature is meaningfully documented at all.)\r\n\r\nHere's the related history:\r\n\u003cdetails\u003e\r\n\u003csummary\u003e(expand)\u003c/summary\u003e\r\n\r\n* 2012 (3.4) - `Modules/_testimportmultiple.c` added (6b2cbeba58aeb0755bdefd02fb51a80ec66d6144); not actually used?\r\n* 2015 (3.5) - `Modules/_testmultiphase.c` added for [PEP 489](https://peps.python.org/pep-0489/) (d5cacbb1d9c3edc02bf0ba01702e7c06da5bc318); contained multiple modules (like `_testimportmultiple), but actually used in tests\r\n* 2022 (3.12) - `Modules/_testsinglephase.c` added ([gh-99039](https://github.com/python/cpython/pull/99039)); so we could stop using the `_testcapi` module to exercise singlephase init in tests; contained the impl. of the one module\r\n* 2023 (3.12) - `Modules/_testsinglephase.c` now implements multiple extension modules ([gh-101891](https://github.com/python/cpython/pull/101891))\r\n\r\n\u003c/details\u003e\r\n\r\nAt the time we added the feature originally, we also added an example implementation: `Modules/_testimportmultiple.c`. However, it looks like it hasn't ever actually been used in any tests. (Perhaps I'm missing something.) We should use it and its modules to test the feature, or drop the file.\r\n\r\nWith all that in mind, we should consider improving test coverage of the feature, and use `_testimportmultiple.c` (or drop it).\r\n\r\nTo be clear, sorting this out is fairly **low priority**. We have *basic* coverage of the feature, I'm not sure it's very widely used, and the status quo isn't causing any problems.\r\n\r\n(Honestly, I've created this issue only because I noticed `_testimportmultiple.c` isn't used anywhere and because of how clunky tests using `_testsinglephae.c` and `_testmultiphase.c` are to both understand and to add. However, while looking closer, I realized the feature wasn't explicitly tested, nor thoroughly.)\r\n\r\n----\r\n\r\nHere's how I think we should proceed:\r\n\r\n1. determine what aspects of the feature need to be tested (see below)\r\n1. create the new `Modules/_testimportmultiple` directory\r\n1. for each test case:\r\n a. implement the test (e.g. in `Lib/test/test_import/__init__.py` or `Lib/test/test_import/test_ext_multiple_in_file.py`)\r\n b. add the new .c file (see [the devguide](https://devguide.python.org/developer-workflow/extension-modules/#adding-an-extension-module-to-cpython)) under `Modules/_testimportmultiple/`\r\n c. implement the relevant modules there\r\n1. delete `Modules/_testimportmultiple.c`\r\n\r\n----\r\n\r\nWe should be sure the following is tested:\r\n\r\n* multiphase-init\r\n * 2 modules in the same file (1 direct, 1 indirect)\r\n * load direct only\r\n * load indirect only\r\n * load direct then indirect\r\n * load indirect then direct\r\n * 3 modules in the same file (1 direct, 2 indirect)\r\n * similar permutations, including both indirect-only, indirect-indirect-only, indirect-direct-indirect, etc.\r\n * 1 indirect module without a direct one (init func name does not match filename\r\n* singlephase-init\r\n * same as multi-phase init\r\n * 2 modules in same file that share some non-exported state stored in a global variable\r\n* singlephase-init and multiphase-init modules in the same file\r\n\r\nWe should be sure that the new tests include whatever `_testsinglephase.c` and `_testmultiphase.c` currently cover specific to multiple-modules-in-a-file.\r\n\r\nThe modules for each case could all be in a single file, i.e. the existing `Modules/_testimportmultiple.c`, but it probably makes more sense to implement each case in its own file (in a new `Modules/_testimportmultiple` directory).\r\n\r\n----\r\n\r\nAgain, the feature is actually tested currently, albeit indirectly and only superficially, through `Modules/_testmultiphase.c` and `Modules/_testsinglephase.c`. However, as far as I know, there isn't any need for the multiple-modules-in-one-file approach in either case. Once we're directly testing the feature using `_testimportmultiple.c`, we can stop testing it indirectly with the other two. See [gh-124983](https://github.com/python/cpython/issues/124983).","author":{"url":"https://github.com/ericsnowcurrently","@type":"Person","name":"ericsnowcurrently"},"datePublished":"2024-10-04T18:02:06.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":2},"url":"https://github.com/124978/cpython/issues/124978"}
| route-pattern | /_view_fragments/issues/show/:user_id/:repository/:id/issue_layout(.:format) |
| route-controller | voltron_issues_fragments |
| route-action | issue_layout |
| fetch-nonce | v2:cf6bb6f4-1a45-a0dc-30b3-cdeebd2892d7 |
| current-catalog-service-hash | 81bb79d38c15960b92d99bca9288a9108c7a47b18f2423d0f6438c5b7bcd2114 |
| request-id | AD20:9F1EC:20712:2DCB7:696A0B80 |
| html-safe-nonce | bb49d064bbcc0bf09fd779c0548ff9e0dedf3dc42bb8cc4e2180d42f598d53ac |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiJBRDIwOjlGMUVDOjIwNzEyOjJEQ0I3OjY5NkEwQjgwIiwidmlzaXRvcl9pZCI6IjY2NDQ5NjA1MzA2NzU2NjU3OTIiLCJyZWdpb25fZWRnZSI6ImlhZCIsInJlZ2lvbl9yZW5kZXIiOiJpYWQifQ== |
| visitor-hmac | 7884233d1dcc83900c015904eef8220bd3c9527786cbdadfe631dfb8c5a6e43b |
| hovercard-subject-tag | issue:2566976315 |
| github-keyboard-shortcuts | repository,issues,copilot |
| google-site-verification | Apib7-x98H0j5cPqHWwSMm6dNU4GmODRoqxLiDzdx9I |
| octolytics-url | https://collector.github.com/github/collect |
| analytics-location | / |
| fb:app_id | 1401488693436528 |
| apple-itunes-app | app-id=1477376905, app-argument=https://github.com/_view_fragments/issues/show/python/cpython/124978/issue_layout |
| twitter:image | https://opengraph.githubassets.com/53e89717b874e4f56cd0141af6f05f5b3741d46c0bede90c0b3f6cc757236eb2/python/cpython/issues/124978 |
| twitter:card | summary_large_image |
| og:image | https://opengraph.githubassets.com/53e89717b874e4f56cd0141af6f05f5b3741d46c0bede90c0b3f6cc757236eb2/python/cpython/issues/124978 |
| og:image:alt | (low priority) The capability of loading multiple extension modules from a single .so file was added over a decade ago. AFAIK the feature wasn't actually tested at all for several years. The PEP 48... |
| og:image:width | 1200 |
| og:image:height | 600 |
| og:site_name | GitHub |
| og:type | object |
| og:author:username | ericsnowcurrently |
| hostname | github.com |
| expected-hostname | github.com |
| None | 699227a00bbb7fe1eec276d2ae1c3a93068bc5ba483bd9dc4b2a27a8f4f2f595 |
| turbo-cache-control | no-preview |
| go-import | github.com/python/cpython git https://github.com/python/cpython.git |
| octolytics-dimension-user_id | 1525981 |
| octolytics-dimension-user_login | python |
| octolytics-dimension-repository_id | 81598961 |
| octolytics-dimension-repository_nwo | python/cpython |
| octolytics-dimension-repository_public | true |
| octolytics-dimension-repository_is_fork | false |
| octolytics-dimension-repository_network_root_id | 81598961 |
| octolytics-dimension-repository_network_root_nwo | python/cpython |
| turbo-body-classes | logged-out env-production page-responsive |
| disable-turbo | false |
| browser-stats-url | https://api.github.com/_private/browser/stats |
| browser-errors-url | https://api.github.com/_private/browser/errors |
| release | 7266b2d935baa1c6474b16dd9feaa5ca30607261 |
| ui-target | full |
| theme-color | #1e2327 |
| color-scheme | light dark |
Links:
Viewport: width=device-width