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

[3.13] gh-128911: Add tests on the PyImport C API (#128915) #128960

Merged
merged 1 commit into from
Jan 17, 2025

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Jan 17, 2025

  • Add Modules/_testlimitedcapi/import.c
  • Add Lib/test/test_capi/test_import.py
  • Remove _testcapi.check_pyimport_addmodule(): tests already covered by newly added tests.

Co-authored-by: Serhiy Storchaka [email protected]
(cherry picked from commit d95ba9f)

@vstinner
Copy link
Member Author

@serhiy-storchaka: In Python 3.13, non-string names lead to crashes. I had to remove these tests:

diff --git a/Lib/test/test_capi/test_import.py b/Lib/test/test_capi/test_import.py
index 94f96728d91..3abd1cd7470 100644
--- a/Lib/test/test_capi/test_import.py
+++ b/Lib/test/test_capi/test_import.py
@@ -70,8 +70,6 @@ def check_addmodule(self, add_module, accept_nonstr=False):
         names = ['nonexistent']
         if accept_nonstr:
             names.append(b'\xff')  # non-UTF-8
-            # PyImport_AddModuleObject() accepts non-string names
-            names.append(tuple(['hashable non-string']))
         for name in names:
             with self.subTest(name=name):
                 self.assertNotIn(name, sys.modules)
@@ -311,9 +309,6 @@ def test_executecodemoduleobject(self):
 
         code = compile('attr = 1', '<test>', 'exec')
         self.assertRaises(TypeError, execute_code_func, [], code, NULL, NULL)
-        nonstring = tuple(['hashable non-string'])
-        self.assertRaises(AttributeError, execute_code_func, nonstring, code, NULL, NULL)
-        sys.modules.pop(nonstring, None)
         # CRASHES execute_code_func(NULL, code, NULL, NULL)
         # CRASHES execute_code_func(name, NULL, NULL, NULL)
 

@vstinner vstinner added the needs backport to 3.12 bug and security fixes label Jan 17, 2025
* Add Modules/_testlimitedcapi/import.c
* Add Lib/test/test_capi/test_import.py
* Remove _testcapi.check_pyimport_addmodule(): tests already covered
  by newly added tests.

Co-authored-by: Serhiy Storchaka <[email protected]>
(cherry picked from commit d95ba9f)
@vstinner vstinner merged commit 34ded1a into python:3.13 Jan 17, 2025
36 checks passed
@vstinner vstinner deleted the test_capi_import13 branch January 17, 2025 21:44
@miss-islington-app
Copy link

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Sorry, @vstinner, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 34ded1a1a10204635cad27830fcbee2f8547e8ed 3.12

@vstinner
Copy link
Member Author

The change cannot be backported to 3.12: Modules/_testlimitedcapi.c doesn't exist in the 3.12 branch.

@vstinner vstinner removed the needs backport to 3.12 bug and security fixes label Jan 17, 2025
serhiy-storchaka pushed a commit to serhiy-storchaka/cpython that referenced this pull request Jan 18, 2025
…nGH-128915) (pythonGH-128960)

pythongh-128911: Add tests on the PyImport C API (pythonGH-128915)

* Add Modules/_testlimitedcapi/import.c
* Add Lib/test/test_capi/test_import.py
* Remove _testcapi.check_pyimport_addmodule(): tests already covered
  by newly added tests.
(cherry picked from commit 34ded1a)

Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: Serhiy Storchaka <[email protected]>
(cherry picked from commit d95ba9f)
@bedevere-app
Copy link

bedevere-app bot commented Jan 18, 2025

GH-128989 is a backport of this pull request to the 3.12 branch.

vstinner pushed a commit that referenced this pull request Jan 19, 2025
) (#128989)

* Add Modules/_testlimitedcapi/import.c
* Add Lib/test/test_capi/test_import.py
* Remove _testcapi.check_pyimport_addmodule(): tests already covered
  by newly added tests.
(cherry picked from commit 34ded1a)

Co-authored-by: Victor Stinner <[email protected]>
Co-authored-by: Serhiy Storchaka <[email protected]>
(cherry picked from commit d95ba9f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant