From 23bfe9af7c0b763d0a702c3ff4f35b8913230451 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sun, 21 Dec 2025 19:19:41 -0800 Subject: [PATCH 1/4] Improve C++ test infrastructure and disable hanging test This commit improves the C++ test infrastructure to ensure test output is visible in CI logs, and disables a test that hangs on free-threaded Python 3.14+. Changes: ## CI/test infrastructure improvements - .github/workflows: Added `timeout-minutes: 3` to all C++ test steps to prevent indefinite hangs. - tests/**/CMakeLists.txt: Added `USES_TERMINAL` to C++ test targets (cpptest, test_cross_module_rtti, test_pure_cpp) to ensure output is shown immediately rather than buffered and possibly lost on crash/timeout. - tests/test_with_catch/catch.cpp: Added a custom Catch2 progress reporter with timestamps, Python version info, and a SIGTERM handler to make test execution and failures clearly visible in CI logs. ## Disabled hanging test - The "Move Subinterpreter" test is disabled on free-threaded Python 3.14+ due to a hang in Py_EndInterpreter() when the subinterpreter is destroyed from a different thread than it was created on. Work on fixing the underlying issue will continue under PR #5940. Context: We were in the dark for months (since we started testing with Python 3.14t) because CI logs gave no clue about the root cause of hangs. This led to ignoring intermittent hangs (mostly on macOS). Our hand was forced only with the Python 3.14.1 release, when hangs became predictable on all platforms. For the full development history of these changes, see PR #5933. --- .github/workflows/ci.yml | 15 +++ .github/workflows/reusable-standard.yml | 1 + .github/workflows/upstream.yml | 2 + tests/pure_cpp/CMakeLists.txt | 4 +- tests/test_cross_module_rtti/CMakeLists.txt | 4 +- tests/test_with_catch/CMakeLists.txt | 4 +- tests/test_with_catch/catch.cpp | 115 ++++++++++++++++++ tests/test_with_catch/test_subinterpreter.cpp | 5 +- 8 files changed, 146 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c5a200e32e..4800b9c25c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -229,6 +229,7 @@ jobs: run: cmake --build . --target pytest - name: Compiled tests + timeout-minutes: 3 run: cmake --build . --target cpptest - name: Interface test @@ -334,6 +335,7 @@ jobs: run: cmake --build --preset default --target pytest - name: C++ tests + timeout-minutes: 3 run: cmake --build --preset default --target cpptest - name: Visibility test @@ -393,6 +395,7 @@ jobs: run: cmake --build build --target pytest - name: C++ tests + timeout-minutes: 3 run: cmake --build build --target cpptest - name: Interface test @@ -516,6 +519,7 @@ jobs: run: cmake --build build --target pytest - name: C++ tests + timeout-minutes: 3 run: cmake --build build --target cpptest - name: Interface test @@ -570,6 +574,7 @@ jobs: run: cmake --build build --target pytest - name: C++ tests + timeout-minutes: 3 run: cmake --build build --target cpptest - name: Interface test @@ -652,6 +657,7 @@ jobs: cmake --build build-11 --target check - name: C++ tests C++11 + timeout-minutes: 3 run: | set +e; source /opt/intel/oneapi/setvars.sh; set -e cmake --build build-11 --target cpptest @@ -689,6 +695,7 @@ jobs: cmake --build build-17 --target check - name: C++ tests C++17 + timeout-minutes: 3 run: | set +e; source /opt/intel/oneapi/setvars.sh; set -e cmake --build build-17 --target cpptest @@ -760,6 +767,7 @@ jobs: run: cmake --build build --target pytest - name: C++ tests + timeout-minutes: 3 run: cmake --build build --target cpptest - name: Interface test @@ -1000,6 +1008,7 @@ jobs: run: cmake --build build --target pytest - name: C++20 tests + timeout-minutes: 3 run: cmake --build build --target cpptest -j 2 - name: Interface test C++20 @@ -1076,6 +1085,7 @@ jobs: run: cmake --build build --target pytest -j 2 - name: C++11 tests + timeout-minutes: 3 run: PYTHONHOME=/${{matrix.sys}} PYTHONPATH=/${{matrix.sys}} cmake --build build --target cpptest -j 2 - name: Interface test C++11 @@ -1100,6 +1110,7 @@ jobs: run: cmake --build build2 --target pytest -j 2 - name: C++14 tests + timeout-minutes: 3 run: PYTHONHOME=/${{matrix.sys}} PYTHONPATH=/${{matrix.sys}} cmake --build build2 --target cpptest -j 2 - name: Interface test C++14 @@ -1124,6 +1135,7 @@ jobs: run: cmake --build build3 --target pytest -j 2 - name: C++17 tests + timeout-minutes: 3 run: PYTHONHOME=/${{matrix.sys}} PYTHONPATH=/${{matrix.sys}} cmake --build build3 --target cpptest -j 2 - name: Interface test C++17 @@ -1195,6 +1207,7 @@ jobs: run: cmake --build . --target pytest -j 2 - name: C++ tests + timeout-minutes: 3 run: cmake --build . --target cpptest -j 2 - name: Interface test @@ -1257,6 +1270,7 @@ jobs: run: cmake --build . --target pytest -j 2 - name: C++ tests + timeout-minutes: 3 run: cmake --build . --target cpptest -j 2 - name: Interface test @@ -1329,6 +1343,7 @@ jobs: run: cmake --build build --target pytest -j 2 - name: C++ tests + timeout-minutes: 3 run: PYTHONHOME=/clangarm64 PYTHONPATH=/clangarm64 cmake --build build --target cpptest -j 2 - name: Interface test diff --git a/.github/workflows/reusable-standard.yml b/.github/workflows/reusable-standard.yml index 96b14bdfba..56d92e2779 100644 --- a/.github/workflows/reusable-standard.yml +++ b/.github/workflows/reusable-standard.yml @@ -83,6 +83,7 @@ jobs: run: cmake --build build --target pytest - name: C++ tests + timeout-minutes: 3 run: cmake --build build --target cpptest - name: Interface test diff --git a/.github/workflows/upstream.yml b/.github/workflows/upstream.yml index 15ede7a856..890ae0b6fd 100644 --- a/.github/workflows/upstream.yml +++ b/.github/workflows/upstream.yml @@ -66,6 +66,7 @@ jobs: run: cmake --build build11 --target pytest -j 2 - name: C++11 tests + timeout-minutes: 3 run: cmake --build build11 --target cpptest -j 2 - name: Interface test C++11 @@ -87,6 +88,7 @@ jobs: run: cmake --build build17 --target pytest - name: C++17 tests + timeout-minutes: 3 run: cmake --build build17 --target cpptest # Third build - C++17 mode with unstable ABI diff --git a/tests/pure_cpp/CMakeLists.txt b/tests/pure_cpp/CMakeLists.txt index 1150cb405e..d2757db766 100644 --- a/tests/pure_cpp/CMakeLists.txt +++ b/tests/pure_cpp/CMakeLists.txt @@ -15,6 +15,8 @@ target_link_libraries(smart_holder_poc_test PRIVATE pybind11::headers Catch2::Ca add_custom_target( test_pure_cpp COMMAND "$" - WORKING_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}") + WORKING_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}" + USES_TERMINAL # Ensures output is shown immediately (not buffered and possibly lost on crash) +) add_dependencies(check test_pure_cpp) diff --git a/tests/test_cross_module_rtti/CMakeLists.txt b/tests/test_cross_module_rtti/CMakeLists.txt index 97d2c780cb..c9b95bfba1 100644 --- a/tests/test_cross_module_rtti/CMakeLists.txt +++ b/tests/test_cross_module_rtti/CMakeLists.txt @@ -60,7 +60,9 @@ add_custom_target( test_cross_module_rtti COMMAND "$" DEPENDS test_cross_module_rtti_main - WORKING_DIRECTORY "$") + WORKING_DIRECTORY "$" + USES_TERMINAL # Ensures output is shown immediately (not buffered and possibly lost on crash) +) set_target_properties(test_cross_module_rtti_bindings PROPERTIES LIBRARY_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}") diff --git a/tests/test_with_catch/CMakeLists.txt b/tests/test_with_catch/CMakeLists.txt index 136537e67f..e6a9f67aa7 100644 --- a/tests/test_with_catch/CMakeLists.txt +++ b/tests/test_with_catch/CMakeLists.txt @@ -47,7 +47,9 @@ add_custom_target( cpptest COMMAND "$" DEPENDS test_with_catch - WORKING_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}") + WORKING_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}" + USES_TERMINAL # Ensures output is shown immediately (not buffered and possibly lost on crash) +) pybind11_add_module(external_module THIN_LTO external_module.cpp) set_target_properties(external_module PROPERTIES LIBRARY_OUTPUT_DIRECTORY diff --git a/tests/test_with_catch/catch.cpp b/tests/test_with_catch/catch.cpp index 5bd8b3880e..89fec39c37 100644 --- a/tests/test_with_catch/catch.cpp +++ b/tests/test_with_catch/catch.cpp @@ -3,6 +3,17 @@ #include +#include +#include +#include +#include +#include +#include + +#ifndef _WIN32 +# include +#endif + // Silence MSVC C++17 deprecation warning from Catch regarding std::uncaught_exceptions (up to // catch 2.0.1; this should be fixed in the next catch release after 2.0.1). PYBIND11_WARNING_DISABLE_MSVC(4996) @@ -13,11 +24,109 @@ PYBIND11_WARNING_DISABLE_MSVC(4996) #endif #define CATCH_CONFIG_RUNNER +#define CATCH_CONFIG_DEFAULT_REPORTER "progress" #include namespace py = pybind11; +// Simple progress reporter that prints a line per test case. +namespace { + +class ProgressReporter : public Catch::StreamingReporterBase { +public: + using StreamingReporterBase::StreamingReporterBase; + + static std::string getDescription() { return "Simple progress reporter (one line per test)"; } + + void testCaseStarting(Catch::TestCaseInfo const &testInfo) override { + print_python_version_once(); + auto &os = Catch::cout(); + os << "[ RUN ] " << testInfo.name << '\n'; + os.flush(); + } + + void testCaseEnded(Catch::TestCaseStats const &stats) override { + bool failed = stats.totals.assertions.failed > 0; + auto &os = Catch::cout(); + os << (failed ? "[ FAILED ] " : "[ OK ] ") << stats.testInfo.name << '\n'; + os.flush(); + } + + void noMatchingTestCases(std::string const &spec) override { + auto &os = Catch::cout(); + os << "[ NO TEST ] no matching test cases for spec: " << spec << '\n'; + os.flush(); + } + + void reportInvalidArguments(std::string const &arg) override { + auto &os = Catch::cout(); + os << "[ ERROR ] invalid Catch2 arguments: " << arg << '\n'; + os.flush(); + } + + void assertionStarting(Catch::AssertionInfo const &) override {} + + bool assertionEnded(Catch::AssertionStats const &) override { return false; } + +private: + void print_python_version_once() { + if (printed_) { + return; + } + printed_ = true; + auto &os = Catch::cout(); + os << "[ PYTHON ] " << Py_GetVersion() << '\n'; + os.flush(); + } + + bool printed_ = false; +}; + +} // namespace + +CATCH_REGISTER_REPORTER("progress", ProgressReporter) + +namespace { + +std::string get_utc_timestamp() { + auto now = std::chrono::system_clock::now(); + auto time_t_now = std::chrono::system_clock::to_time_t(now); + auto ms = std::chrono::duration_cast(now.time_since_epoch()) % 1000; + + std::tm utc_tm{}; +#if defined(_WIN32) + gmtime_s(&utc_tm, &time_t_now); +#else + gmtime_r(&time_t_now, &utc_tm); +#endif + + std::ostringstream oss; + oss << std::put_time(&utc_tm, "%Y-%m-%d %H:%M:%S") << '.' << std::setfill('0') << std::setw(3) + << ms.count() << 'Z'; + return oss.str(); +} + +#ifndef _WIN32 +// Signal handler to print a message when the process is terminated. +// Uses only async-signal-safe functions. +void termination_signal_handler(int sig) { + const char *msg = "[ SIGNAL ] Process received SIGTERM\n"; + // write() is async-signal-safe, unlike std::cout + ssize_t written = write(STDOUT_FILENO, msg, strlen(msg)); + (void) written; // suppress "unused variable" warnings + // Re-raise with default handler to get proper exit status + std::signal(sig, SIG_DFL); + std::raise(sig); +} +#endif + +} // namespace + int main(int argc, char *argv[]) { +#ifndef _WIN32 + std::signal(SIGTERM, termination_signal_handler); +#endif + // Setup for TEST_CASE in test_interpreter.cpp, tagging on a large random number: std::string updated_pythonpath("pybind11_test_with_catch_PYTHONPATH_2099743835476552"); const char *preexisting_pythonpath = getenv("PYTHONPATH"); @@ -35,9 +144,15 @@ int main(int argc, char *argv[]) { setenv("PYTHONPATH", updated_pythonpath.c_str(), /*replace=*/1); #endif + std::cout << "[ STARTING ] " << get_utc_timestamp() << '\n'; + std::cout.flush(); + py::scoped_interpreter guard{}; auto result = Catch::Session().run(argc, argv); + std::cout << "[ DONE ] " << get_utc_timestamp() << " (result " << result << ")\n"; + std::cout.flush(); + return result < 0xff ? result : 0xff; } diff --git a/tests/test_with_catch/test_subinterpreter.cpp b/tests/test_with_catch/test_subinterpreter.cpp index 3c7c35be19..749cc45a89 100644 --- a/tests/test_with_catch/test_subinterpreter.cpp +++ b/tests/test_with_catch/test_subinterpreter.cpp @@ -90,7 +90,10 @@ TEST_CASE("Single Subinterpreter") { unsafe_reset_internals_for_single_interpreter(); } -# if PY_VERSION_HEX >= 0x030D0000 +// "Move Subinterpreter" test is disabled on free-threaded Python 3.14+ due to a hang +// in Py_EndInterpreter() when the subinterpreter is destroyed from a different thread +// than it was created on. See: https://github.com/pybind/pybind11/pull/5940 +# if PY_VERSION_HEX >= 0x030D0000 && !(PY_VERSION_HEX >= 0x030E0000 && defined(Py_GIL_DISABLED)) TEST_CASE("Move Subinterpreter") { std::unique_ptr sub(new py::subinterpreter(py::subinterpreter::create())); From 4aa1e0d6dfa0693eb07235fce3e58c11f3c2ffb1 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sun, 21 Dec 2025 19:33:11 -0800 Subject: [PATCH 2/4] Add test summary to progress reporter Print the total number of test cases and assertions at the end of the test run, making it easy to spot if tests are disabled or added. Example output: [ PASSED ] 20 test cases, 1589 assertions. --- tests/test_with_catch/catch.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/test_with_catch/catch.cpp b/tests/test_with_catch/catch.cpp index 89fec39c37..7f59b77ac9 100644 --- a/tests/test_with_catch/catch.cpp +++ b/tests/test_with_catch/catch.cpp @@ -68,6 +68,21 @@ class ProgressReporter : public Catch::StreamingReporterBase { bool assertionEnded(Catch::AssertionStats const &) override { return false; } + void testRunEnded(Catch::TestRunStats const &stats) override { + auto &os = Catch::cout(); + auto passed = stats.totals.testCases.passed; + auto failed = stats.totals.testCases.failed; + auto total = passed + failed; + auto assertions = stats.totals.assertions.passed + stats.totals.assertions.failed; + if (failed == 0) { + os << "[ PASSED ] " << total << " test cases, " << assertions << " assertions.\n"; + } else { + os << "[ FAILED ] " << failed << " of " << total << " test cases, " << assertions + << " assertions.\n"; + } + os.flush(); + } + private: void print_python_version_once() { if (printed_) { From 4fa88e65e88151e82610afa1ffde0e1e9f1c3f76 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sun, 21 Dec 2025 19:40:55 -0800 Subject: [PATCH 3/4] Add PYBIND11_CATCH2_SKIP_IF macro to skip tests at runtime Catch2 v2 doesn't have native skip support (v3 does with SKIP()). This macro allows tests to be skipped with a visible message while still appearing in the test list. Use this for the Move Subinterpreter test on free-threaded Python 3.14+ so it shows as skipped rather than being conditionally compiled out. Example output: [ RUN ] Move Subinterpreter [ SKIPPED ] Skipped on free-threaded Python 3.14+ (see PR #5940) [ OK ] Move Subinterpreter --- tests/test_with_catch/catch.cpp | 2 ++ tests/test_with_catch/catch_skip.h | 16 ++++++++++++++++ tests/test_with_catch/test_subinterpreter.cpp | 14 ++++++++++---- 3 files changed, 28 insertions(+), 4 deletions(-) create mode 100644 tests/test_with_catch/catch_skip.h diff --git a/tests/test_with_catch/catch.cpp b/tests/test_with_catch/catch.cpp index 7f59b77ac9..8959593184 100644 --- a/tests/test_with_catch/catch.cpp +++ b/tests/test_with_catch/catch.cpp @@ -25,6 +25,8 @@ PYBIND11_WARNING_DISABLE_MSVC(4996) #define CATCH_CONFIG_RUNNER #define CATCH_CONFIG_DEFAULT_REPORTER "progress" +#include "catch_skip.h" + #include namespace py = pybind11; diff --git a/tests/test_with_catch/catch_skip.h b/tests/test_with_catch/catch_skip.h new file mode 100644 index 0000000000..01c02b8b3c --- /dev/null +++ b/tests/test_with_catch/catch_skip.h @@ -0,0 +1,16 @@ +// Macro to skip a test at runtime with a visible message. +// Catch2 v2 doesn't have native skip support (v3 does with SKIP()). +// The test will count as "passed" in totals, but the output clearly shows it was skipped. + +#pragma once + +#include + +#define PYBIND11_CATCH2_SKIP_IF(condition, reason) \ + do { \ + if (condition) { \ + Catch::cout() << "[ SKIPPED ] " << reason << '\n'; \ + Catch::cout().flush(); \ + return; \ + } \ + } while (0) diff --git a/tests/test_with_catch/test_subinterpreter.cpp b/tests/test_with_catch/test_subinterpreter.cpp index 749cc45a89..26e0597582 100644 --- a/tests/test_with_catch/test_subinterpreter.cpp +++ b/tests/test_with_catch/test_subinterpreter.cpp @@ -6,6 +6,8 @@ // catch 2.0.1; this should be fixed in the next catch release after 2.0.1). PYBIND11_WARNING_DISABLE_MSVC(4996) +# include "catch_skip.h" + # include # include # include @@ -90,11 +92,15 @@ TEST_CASE("Single Subinterpreter") { unsafe_reset_internals_for_single_interpreter(); } -// "Move Subinterpreter" test is disabled on free-threaded Python 3.14+ due to a hang -// in Py_EndInterpreter() when the subinterpreter is destroyed from a different thread -// than it was created on. See: https://github.com/pybind/pybind11/pull/5940 -# if PY_VERSION_HEX >= 0x030D0000 && !(PY_VERSION_HEX >= 0x030E0000 && defined(Py_GIL_DISABLED)) +# if PY_VERSION_HEX >= 0x030D0000 TEST_CASE("Move Subinterpreter") { + // Test is skipped on free-threaded Python 3.14+ due to a hang in Py_EndInterpreter() + // when the subinterpreter is destroyed from a different thread than it was created on. + // See: https://github.com/pybind/pybind11/pull/5940 +# if PY_VERSION_HEX >= 0x030E0000 && defined(Py_GIL_DISABLED) + PYBIND11_CATCH2_SKIP_IF(true, "Skipped on free-threaded Python 3.14+ (see PR #5940)"); +# endif + std::unique_ptr sub(new py::subinterpreter(py::subinterpreter::create())); // on this thread, use the subinterpreter and import some non-trivial junk From d3227ce59fb489ad150c046e0e2c5bbfeb0c39f3 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sun, 21 Dec 2025 20:27:56 -0800 Subject: [PATCH 4/4] Fix clang-tidy bugprone-macro-parentheses warning in PYBIND11_CATCH2_SKIP_IF --- tests/test_with_catch/catch_skip.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_with_catch/catch_skip.h b/tests/test_with_catch/catch_skip.h index 01c02b8b3c..72ffdb62b6 100644 --- a/tests/test_with_catch/catch_skip.h +++ b/tests/test_with_catch/catch_skip.h @@ -9,7 +9,7 @@ #define PYBIND11_CATCH2_SKIP_IF(condition, reason) \ do { \ if (condition) { \ - Catch::cout() << "[ SKIPPED ] " << reason << '\n'; \ + Catch::cout() << "[ SKIPPED ] " << (reason) << '\n'; \ Catch::cout().flush(); \ return; \ } \