gh-116303: Skip test module dependent tests if test modules are disabled #116307

erlend-aasland · 2024-03-04T13:35:30Z

Issue: Tests fail if --disable-test-modules is supplied #116303

encukou · 2024-03-04T13:40:09Z

Would it be worth it to skip only the affected assertions, rather than the whole test method?

erlend-aasland · 2024-03-04T13:53:34Z

Would it be worth it to skip only the affected assertions, rather than the whole test method?

Yes, I think so. I'll split them up.

- Lib/test/support/__init__.py - Lib/test/test_audit.py - Lib/test/test_gdb/test_misc.py - Lib/test/test_inspect/test_inspect.py - Lib/test/test_pydoc/test_pydoc.py

erlend-aasland · 2024-03-04T14:04:40Z

@encukou: I started fixing some of the remaining tests, but I won't be back at my laptop until tonight. Feel free to pick this up in the mean time.

erlend-aasland · 2024-03-05T11:03:45Z

@serhiy-storchaka, would you like to join in on finishing this PR? There are still a lot of test failures¹, but I'm slowly getting there. For example, test_importlib still fails; it might be because I introduced a new bug, it might be because we need more skips, or it might be because we uncovered latent test bugs. It is hard to tell right now².

when testing with --disable-test-modules only; unfortunately, they do not show up in CI nor in buildbots, since we do not test with this configure option (yet) ↩
the amount of tests annotated with docstrings and/or sprinkled with print()s do not make this job easier; ideally we should clean up the whole test suite so running python -m test -v generated usable output. test_threading, for example, is sometimes so verbose that it is hard to tell signal from noise. ↩

Lib/test/test_ctypes/test_bitfields.py

serhiy-storchaka · 2024-03-07T10:02:12Z

Lib/test/support/__init__.py

+        return unittest.skip("_testinternalcapi required")
    config = _testinternalcapi.get_config()
    return not bool(config['code_debug_ranges'])


Why does it return a boolean (which is not callable) or a function (which always true)? How this helper is supposed to be used?

serhiy-storchaka · 2024-03-07T10:05:47Z

Lib/test/test_importlib/extension/test_loader.py

@@ -108,7 +108,7 @@ def setUp(self):
            )
        finder = self.machinery.FileFinder(None)
        self.spec = importlib.util.find_spec(self.name)
-        self.assertIsNotNone(self.spec)
+        assert self.spec


Why skip this in optimized mode?

@jaraco requested to leave the assert's for importlib tests, so I reverted the unittest assert style changes. The code is shared with an external repo.

FYI - I only care about the retention of bare asserts in the tests for importlib.metadata and importlib.resources. Unfortunately, the tests for importlib.metadata are interspersed with other importlib tests, so it's not easy to know which are which without looking at the cpython branch of importlib_metadata. I do have plans to move importlib.metadata tests into their own package so they're clearly separated.

vstinner · 2024-03-07T14:38:42Z

About the test_embed bug, it seems like importing _ctypes is enough to trigger the bug which looks like a memory corruption:

vstinner@WIN C:\victor\python\main>type bug.py
import subprocess
program = r"PCbuild\amd64\_testembed_d.exe"
cmd = [program, "test_repeated_init_exec", "import _ctypes"]
subprocess.run(cmd)

vstinner@WIN C:\victor\python\main>python bug.py
Running Debug|x64 interpreter...
--- Loop #1 ---
--- Loop #2 ---
--- Loop #3 ---
--- Loop #4 ---

vstinner@WIN C:\victor\python\main>python bug.py
Running Debug|x64 interpreter...
--- Loop #1 ---
--- Loop #2 ---
--- Loop #3 ---
--- Loop #4 ---

vstinner@WIN C:\victor\python\main>python bug.py
Running Debug|x64 interpreter...
--- Loop #1 ---
--- Loop #2 ---
--- Loop #3 ---
--- Loop #4 ---

vstinner@WIN C:\victor\python\main>python bug.py
Running Debug|x64 interpreter...
--- Loop #1 ---
--- Loop #2 ---
--- Loop #3 ---
--- Loop #4 ---

vstinner@WIN C:\victor\python\main>python bug.py
Running Debug|x64 interpreter...
--- Loop #1 ---
--- Loop #2 ---
Assertion failed: PyUnicode_CheckExact(ep->me_key), file C:\victor\python\main\Objects\dictobject.c, line 1101

vstinner · 2024-03-07T14:45:09Z

About the test_embed bug, it seems like importing _ctypes is enough to trigger the bug which looks like a memory corruption

I can reproduce the bug on the main branch, without these changes. Use this patch to make the bug more likely:

diff --git a/Programs/_testembed.c b/Programs/_testembed.c
index 30998bf80f..e8f36ae442 100644
--- a/Programs/_testembed.c
+++ b/Programs/_testembed.c
@@ -38,7 +38,7 @@ char **main_argv;
 /* Use path starting with "./" avoids a search along the PATH */
 #define PROGRAM_NAME L"./_testembed"

-#define INIT_LOOPS 4
+#define INIT_LOOPS 12

 // Ignore Py_DEPRECATED() compiler warnings: deprecated functions are
 // tested on purpose here.

Example of crash:

vstinner@WIN C:\victor\python\main>python bug.py    
Running Debug|x64 interpreter...
--- Loop #1 ---
--- Loop #2 ---
--- Loop #3 ---
--- Loop #4 ---
--- Loop #5 ---
--- Loop #6 ---
--- Loop #7 ---
--- Loop #8 ---
--- Loop #9 ---
--- Loop #10 ---
--- Loop #11 ---
--- Loop #12 ---

vstinner@WIN C:\victor\python\main>python bug.py
Running Debug|x64 interpreter...
--- Loop #1 ---
--- Loop #2 ---
--- Loop #3 ---
Assertion failed: PyUnicode_CheckExact(ep->me_key), file C:\victor\python\main\Objects\dictobject.c, line 1101

vstinner · 2024-03-07T15:33:52Z

I can reproduce the bug on the main branch, without these changes.

I tried to dig into Python history: the bug exists since July 1st, 2023 at least. I failed to go further in the past with git bisect, around June 1st, 2023, the test fails at importing _ctypes at the second loop iteration!

vstinner@WIN C:\victor\python\main>python bug.py  
Running Debug|x64 interpreter...
   == Process #1 ===
--- Loop #1 ---
--- Loop #2 ---
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ModuleNotFoundError: No module named '_ctypes'
=> exitcode 1

Latest version of my bug.py:

import subprocess
program = r"PCbuild\amd64\_testembed_d.exe"
cmd = [program, "test_repeated_init_exec", "import _ctypes"]

for i in range(1, 11):
    print(f"   == Process #{i} ===")
    proc = subprocess.run(cmd)
    exitcode = proc.returncode
    print(f"=> exitcode {exitcode}")
    if exitcode:
        break

vstinner · 2024-03-07T15:43:12Z

I created #116467 for the test_embed crash: Initialize/Finalize Python multiple time and import _ctypes each time lead to a memory corruption.

serhiy-storchaka · 2024-03-07T15:43:31Z

@vstinner, could you please open a separate issue for this and copy the results of researches there?

erlend-aasland · 2024-03-07T22:41:07Z

Thanks for your help, everyone. I'm going to close this PR and split it up in multiple parts.

gh-116307: Create a new import helper 'isolated modules' and use that instead of 'Clean Import' to ensure that tests from importlib_resources don't leave modules in sys.modules.

…ythonGH-116680) (cherry picked from commit a254807) Co-authored-by: Jason R. Coombs <jaraco@jaraco.com> pythongh-116307: Create a new import helper 'isolated modules' and use that instead of 'Clean Import' to ensure that tests from importlib_resources don't leave modules in sys.modules.

… tests (pythonGH-116680) (python#116684) pythongh-116307: Proper fix for 'mod' leaking across importlib tests (pythonGH-116680) (cherry picked from commit a254807) pythongh-116307: Create a new import helper 'isolated modules' and use that instead of 'Clean Import' to ensure that tests from importlib_resources don't leave modules in sys.modules. Co-authored-by: Jason R. Coombs <jaraco@jaraco.com>

#116694) [3.11] gh-116307: Proper fix for 'mod' leaking across importlib tests (GH-116680) (cherry picked from commit a254807) gh-116307: Create a new import helper 'isolated modules' and use that instead of 'Clean Import' to ensure that tests from importlib_resources don't leave modules in sys.modules.

… tests (pythonGH-116680) (cherry picked from commit a254807) Co-authored-by: Jason R. Coombs <jaraco@jaraco.com> pythongh-116307: Create a new import helper 'isolated modules' and use that instead of 'Clean Import' to ensure that tests from importlib_resources don't leave modules in sys.modules.

…ython#116680) pythongh-116307: Create a new import helper 'isolated modules' and use that instead of 'Clean Import' to ensure that tests from importlib_resources don't leave modules in sys.modules.

pythongh-116303: Skip some sqlite3 tests if testcapi is unavailable

9b3f0f7

erlend-aasland requested a review from berkerpeksag as a code owner March 4, 2024 13:35

erlend-aasland added skip news needs backport to 3.12 bug and security fixes labels Mar 4, 2024

bedevere-app bot added the awaiting core review label Mar 4, 2024

bedevere-app bot mentioned this pull request Mar 4, 2024

Tests fail if --disable-test-modules is supplied #116303

Closed

bedevere-app bot added the tests Tests in the Lib/test dir label Mar 4, 2024

erlend-aasland added the needs backport to 3.11 only security fixes label Mar 4, 2024

erlend-aasland changed the title ~~gh-116303: Skip some sqlite3 tests if testcapi is unavailable~~ gh-116303: Skip test module dependent tests if test modules are disabled Mar 4, 2024

erlend-aasland marked this pull request as draft March 4, 2024 13:54

bedevere-app bot removed the awaiting core review label Mar 4, 2024

Skip more tests

0f6f74e

- Lib/test/support/__init__.py - Lib/test/test_audit.py - Lib/test/test_gdb/test_misc.py - Lib/test/test_inspect/test_inspect.py - Lib/test/test_pydoc/test_pydoc.py

erlend-aasland force-pushed the sqlite/testcapi branch from ee6047c to 0f6f74e Compare March 4, 2024 14:03

erlend-aasland added 10 commits March 4, 2024 15:07

Split up sqlite3 tests

2f750c3

Amend previous commit

7c4477d

Pull in main

a9c6f62

Use import_helper iso. requires_limited_api

cdc6df4

More skips

d05dcd7

Fix gc and weakref tests

cb7cff4

Remove local debug stuff

ad5e3b6

Fix test_os

cdb8bf4

Fixup stable_abi.py

e4a30b0

Fixup run_in_subinterp

fabb007

erlend-aasland added 2 commits March 5, 2024 12:08

Fixup test_call

c51dafa

Fixup test_audit

c3f9c99

erlend-aasland commented Mar 5, 2024

View reviewed changes

Lib/test/test_ctypes/test_bitfields.py Outdated Show resolved Hide resolved

Revert another spurious test_importlib change

13c4829

erlend-aasland requested a review from jaraco March 6, 2024 23:42

Fix test_embed

31e19a7

jaraco approved these changes Mar 7, 2024

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting changes labels Mar 7, 2024

erlend-aasland added 2 commits March 7, 2024 10:12

Pull in main

45e0586

Remove useless import from test_embed

a148fb5

serhiy-storchaka reviewed Mar 7, 2024

View reviewed changes

erlend-aasland closed this Mar 7, 2024

erlend-aasland deleted the sqlite/testcapi branch March 7, 2024 22:41

This was referenced Mar 12, 2024

ghpython-116307: Proper fix for 'mod' leaking across importlib tests #116678

Closed

gh-116307: Proper fix for 'mod' leaking across importlib tests #116680

Merged

miss-islington mentioned this pull request Mar 12, 2024

[3.12] gh-116307: Proper fix for 'mod' leaking across importlib tests (GH-116680) #116684

Merged

jaraco mentioned this pull request Mar 13, 2024

[3.11] gh-116307: Proper fix for 'mod' leaking across importlib tests… #116694

Merged

erlend-aasland mentioned this pull request Mar 28, 2024

gh-116303: Skip test module dependent tests if test modules are unavailable #117341

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-116303: Skip test module dependent tests if test modules are disabled #116307

gh-116303: Skip test module dependent tests if test modules are disabled #116307

erlend-aasland commented Mar 4, 2024 •

edited by bedevere-app bot

encukou commented Mar 4, 2024

erlend-aasland commented Mar 4, 2024

erlend-aasland commented Mar 4, 2024

erlend-aasland commented Mar 5, 2024

serhiy-storchaka Mar 7, 2024

serhiy-storchaka Mar 7, 2024

erlend-aasland Mar 7, 2024

jaraco Mar 7, 2024

vstinner commented Mar 7, 2024

vstinner commented Mar 7, 2024

vstinner commented Mar 7, 2024

vstinner commented Mar 7, 2024

serhiy-storchaka commented Mar 7, 2024

erlend-aasland commented Mar 7, 2024

gh-116303: Skip test module dependent tests if test modules are disabled #116307

gh-116303: Skip test module dependent tests if test modules are disabled #116307

Conversation

erlend-aasland commented Mar 4, 2024 • edited by bedevere-app bot

encukou commented Mar 4, 2024

erlend-aasland commented Mar 4, 2024

erlend-aasland commented Mar 4, 2024

erlend-aasland commented Mar 5, 2024

Footnotes

serhiy-storchaka Mar 7, 2024

Choose a reason for hiding this comment

serhiy-storchaka Mar 7, 2024

Choose a reason for hiding this comment

erlend-aasland Mar 7, 2024

Choose a reason for hiding this comment

jaraco Mar 7, 2024

Choose a reason for hiding this comment

vstinner commented Mar 7, 2024

vstinner commented Mar 7, 2024

vstinner commented Mar 7, 2024

vstinner commented Mar 7, 2024

serhiy-storchaka commented Mar 7, 2024

erlend-aasland commented Mar 7, 2024

erlend-aasland commented Mar 4, 2024 •

edited by bedevere-app bot