From f3d66db81be7f92a1b4bd9ed03693a3776be0b78 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Mon, 11 Jul 2022 16:20:52 +0200 Subject: [PATCH 1/2] bpo-94751: Install, import and run the test C++ extension (MVP) - Run test functions on import (yes, this can definitely be improved) - Fudge setuptools metadata (name & version) to make the extension installable - Install and import the extension in test_cppext --- Lib/test/_testcppext.cpp | 10 ++++++++ Lib/test/setup_testcppext.py | 2 +- Lib/test/test_cppext.py | 45 +++++++++++++++++++++++------------- 3 files changed, 40 insertions(+), 17 deletions(-) diff --git a/Lib/test/_testcppext.cpp b/Lib/test/_testcppext.cpp index b6d35407a61edc..77927b4f4f4214 100644 --- a/Lib/test/_testcppext.cpp +++ b/Lib/test/_testcppext.cpp @@ -128,6 +128,9 @@ static PyMethodDef _testcppext_methods[] = { {"add", _testcppext_add, METH_VARARGS, _testcppext_add_doc}, {"test_api_casts", test_api_casts, METH_NOARGS, _Py_NULL}, {"test_unicode", test_unicode, METH_NOARGS, _Py_NULL}, + // Note: _testcppext_exec currently runs all test functions directly. + // When adding a new one, add a call there. + {_Py_NULL, _Py_NULL, 0, _Py_NULL} /* sentinel */ }; @@ -138,6 +141,13 @@ _testcppext_exec(PyObject *module) if (PyModule_AddIntMacro(module, __cplusplus) < 0) { return -1; } + + PyObject *result; + result = PyObject_CallMethod(module, "test_api_casts", ""); + if (!result) return -1; + result = PyObject_CallMethod(module, "test_unicode", ""); + if (!result) return -1; + return 0; } diff --git a/Lib/test/setup_testcppext.py b/Lib/test/setup_testcppext.py index 892e24a6376c5e..ae81e33e23b6bd 100644 --- a/Lib/test/setup_testcppext.py +++ b/Lib/test/setup_testcppext.py @@ -46,7 +46,7 @@ def main(): sources=[SOURCE], language='c++', extra_compile_args=cppflags) - setup(name=name, ext_modules=[cpp_ext]) + setup(name='internal' + name, version='0.0', ext_modules=[cpp_ext]) if __name__ == "__main__": diff --git a/Lib/test/test_cppext.py b/Lib/test/test_cppext.py index 8673911ecfae5d..ad8b1251f877b8 100644 --- a/Lib/test/test_cppext.py +++ b/Lib/test/test_cppext.py @@ -17,22 +17,22 @@ @support.requires_subprocess() class TestCPPExt(unittest.TestCase): def test_build_cpp11(self): - self.check_build(False) + self.check_build(False, '_testcpp11ext') def test_build_cpp03(self): - self.check_build(True) + self.check_build(True, '_testcpp03ext') # With MSVC, the linker fails with: cannot open file 'python311.lib' # https://github.com/python/cpython/pull/32175#issuecomment-1111175897 @unittest.skipIf(MS_WINDOWS, 'test fails on Windows') # the test uses venv+pip: skip if it's not available @support.requires_venv_with_pip() - def check_build(self, std_cpp03): + def check_build(self, std_cpp03, extension_name): # Build in a temporary directory with os_helper.temp_cwd(): - self._check_build(std_cpp03) + self._check_build(std_cpp03, extension_name) - def _check_build(self, std_cpp03): + def _check_build(self, std_cpp03, extension_name): venv_dir = 'env' verbose = support.verbose @@ -52,22 +52,35 @@ def _check_build(self, std_cpp03): else: python = os.path.join(venv_dir, 'bin', python_exe) + def run_cmd(operation, cmd): + if verbose: + print('Run:', ' '.join(cmd)) + subprocess.run(cmd, check=True) + else: + proc = subprocess.run(cmd, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + text=True) + if proc.returncode: + print(proc.stdout, end='') + self.fail( + f"{operation} failed with exit code {proc.returncode}") + # Build the C++ extension cmd = [python, '-X', 'dev', SETUP_TESTCPPEXT, 'build_ext', '--verbose'] if std_cpp03: cmd.append('-std=c++03') - if verbose: - print('Run:', ' '.join(cmd)) - subprocess.run(cmd, check=True) - else: - proc = subprocess.run(cmd, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - text=True) - if proc.returncode: - print(proc.stdout, end='') - self.fail(f"Build failed with exit code {proc.returncode}") + run_cmd('Build', cmd) + + # Install the C++ extension + cmd = [python, '-X', 'dev', + SETUP_TESTCPPEXT, 'install'] + run_cmd('Install', cmd) + + # Import the C++ extension + cmd = [python, '-X', 'dev', '-c', f"import {extension_name}"] + run_cmd('Import', cmd) if __name__ == "__main__": From a615602b77e12d6d24fa95bf13743e9a2b740537 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Mon, 11 Jul 2022 17:01:56 +0200 Subject: [PATCH 2/2] Fix refcounting --- Lib/test/_testcppext.cpp | 4 ++++ Lib/test/test_cppext.py | 14 +++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/Lib/test/_testcppext.cpp b/Lib/test/_testcppext.cpp index 77927b4f4f4214..be7538826b6c4b 100644 --- a/Lib/test/_testcppext.cpp +++ b/Lib/test/_testcppext.cpp @@ -143,10 +143,14 @@ _testcppext_exec(PyObject *module) } PyObject *result; + result = PyObject_CallMethod(module, "test_api_casts", ""); if (!result) return -1; + Py_DECREF(result); + result = PyObject_CallMethod(module, "test_unicode", ""); if (!result) return -1; + Py_DECREF(result); return 0; } diff --git a/Lib/test/test_cppext.py b/Lib/test/test_cppext.py index ad8b1251f877b8..2c54d337116911 100644 --- a/Lib/test/test_cppext.py +++ b/Lib/test/test_cppext.py @@ -78,8 +78,20 @@ def run_cmd(operation, cmd): SETUP_TESTCPPEXT, 'install'] run_cmd('Install', cmd) + # Do a reference run. Until we test that running python + # doesn't leak references (gh-94755), run it so one can manually check + # -X showrefcount results against this baseline. + cmd = [python, + '-X', 'dev', + '-X', 'showrefcount', + '-c', 'pass'] + run_cmd('Reference run', cmd) + # Import the C++ extension - cmd = [python, '-X', 'dev', '-c', f"import {extension_name}"] + cmd = [python, + '-X', 'dev', + '-X', 'showrefcount', + '-c', f"import {extension_name}"] run_cmd('Import', cmd)