diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index e8776221..2d11c990 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -69,8 +69,13 @@ jobs: run: xpython --version - name: Test xeus-python C++ - run: ./test_xeus_python - timeout-minutes: 4 + run: | + for i in $(seq 1 10); + do + echo "tests run $i" + ./test_xeus_python + done + timeout-minutes: 40 working-directory: build/test - name: Test xeus-python Python @@ -139,8 +144,10 @@ jobs: - name: Test xeus-python C++ shell: cmd /C call {0} run: | - micromamba activate xeus-python - test_xeus_python - timeout-minutes: 4 + for /L %%i in (1,1,20) do ( + echo tests run %%i + test_xeus_python + ) + timeout-minutes: 40 working-directory: build\test diff --git a/.gitignore b/.gitignore index 041239f7..97eb65ac 100644 --- a/.gitignore +++ b/.gitignore @@ -44,6 +44,7 @@ docs/build/ # Build directory build/ +/build-*/ # generated kernel specs /share/jupyter/kernels/xpython/kernel.json diff --git a/environment-dev.yml b/environment-dev.yml index 7c8335ec..e1483547 100644 --- a/environment-dev.yml +++ b/environment-dev.yml @@ -25,3 +25,7 @@ dependencies: - jupyter_kernel_test<0.8 - doctest - pluggy=1.3 + - libboost + - libboost-devel + - libboost-headers + diff --git a/include/xeus-python/xdebugger.hpp b/include/xeus-python/xdebugger.hpp index 6c7743ae..d5464932 100644 --- a/include/xeus-python/xdebugger.hpp +++ b/include/xeus-python/xdebugger.hpp @@ -20,6 +20,7 @@ #include #include #include +#include #include "nlohmann/json.hpp" #include "pybind11/pybind11.h" diff --git a/src/main.cpp b/src/main.cpp index 8862a319..95af2803 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -77,15 +77,32 @@ int main(int argc, char* argv[]) PyConfig_InitPythonConfig(&config); // config.isolated = 1; + constexpr std::string_view python_path_help = "PYTHONHOME or PYTHON_EXECUTABLE environment variables can be used to specify the correct path"; + const auto fail_with_error_message = [](const auto&... message_parts) { + std::stringstream message_stream; + ((message_stream << message_parts), ...); + auto message = message_stream.str(); + std::cerr << message << std::endl; + throw std::runtime_error(std::move(message)); + }; + // Setting Program Name static const std::string executable(xpyt::get_python_path()); static const std::wstring wexecutable(executable.cbegin(), executable.cend()); + if (!std::filesystem::exists(wexecutable)) + { + fail_with_error_message("cannot find python executable, tried '", executable, "' - ", python_path_help); + } config.program_name = const_cast(wexecutable.c_str()); // Setting Python Home static const std::string pythonhome{ xpyt::get_python_prefix() }; - static const std::wstring wstr(pythonhome.cbegin(), pythonhome.cend());; - config.home = const_cast(wstr.c_str()); + static const std::wstring wpythonhome(pythonhome.cbegin(), pythonhome.cend()); + if (!std::filesystem::exists(wpythonhome)) + { + fail_with_error_message("cannot find python home directory, tried '", pythonhome, "' - ", python_path_help); + } + config.home = const_cast(wpythonhome.c_str()); xpyt::print_pythonhome(); // Implicitly pre-initialize Python diff --git a/src/xdebugger.cpp b/src/xdebugger.cpp index 58d534da..bc5f987f 100644 --- a/src/xdebugger.cpp +++ b/src/xdebugger.cpp @@ -72,6 +72,9 @@ namespace xpyt debugger::~debugger() { + // release/destroy the debugger python object while GIL is acquired + pybind11::gil_scoped_acquire gil_lock; + m_pydebugger.dec_ref(); } nl::json debugger::inspect_variables_request(const nl::json& message) @@ -286,7 +289,7 @@ namespace xpyt py::gil_scoped_acquire acquire; py::module xeus_python_shell = py::module::import("xeus_python_shell.debugger"); m_pydebugger = xeus_python_shell.attr("XDebugger")(); - + // Get debugpy version std::string expression = "debugpy.__version__"; std::string version = (eval(py::str(expression))).cast(); diff --git a/src/xinterpreter.cpp b/src/xinterpreter.cpp index 548935e1..4ec49c5d 100644 --- a/src/xinterpreter.cpp +++ b/src/xinterpreter.cpp @@ -342,7 +342,6 @@ namespace xpyt // Reset traceback m_ipython_shell.attr("last_error") = py::none(); - try { exec(py::str(code)); @@ -350,21 +349,45 @@ namespace xpyt } catch (py::error_already_set& e) { - // This will grab the latest traceback and set shell.last_error - m_ipython_shell.attr("showtraceback")(); - - py::list pyerror = m_ipython_shell.attr("last_error"); - - xerror error = extract_error(pyerror); + try{ + // This will grab the latest traceback and set shell.last_error + m_ipython_shell.attr("showtraceback")(); + py::list pyerror = m_ipython_shell.attr("last_error"); + xerror error = extract_error(pyerror); - publish_execution_error(error.m_ename, error.m_evalue, error.m_traceback); - error.m_traceback.resize(1); - error.m_traceback[0] = code; + publish_execution_error(error.m_ename, error.m_evalue, error.m_traceback); - return xeus::create_error_reply(error.m_ename, error.m_evalue, error.m_traceback); + error.m_traceback.resize(1); + error.m_traceback[0] = code; + return xeus::create_error_reply(error.m_ename, error.m_evalue, error.m_traceback); + } + catch (py::error_already_set& e) + { + std::cerr << "an error occurred during error handling: "<()); + } + catch(std::exception& e) + { + std::cerr << "a standard exception occurred during error handling: "<()); + } + catch (...) + { + std::cerr << "an unknown error occurred during error handling"<()); + } + } + catch(std::exception& e) + { + return xeus::create_error_reply("Exception", e.what(), std::vector()); + } + catch (...) + { + return xeus::create_error_reply("UnknownError", "", std::vector()); } } + void interpreter::set_request_context(xeus::xrequest_context context) { py::gil_scoped_acquire acquire; diff --git a/src/xpaths.cpp b/src/xpaths.cpp index 9ccc468d..3fbd84a2 100644 --- a/src/xpaths.cpp +++ b/src/xpaths.cpp @@ -52,6 +52,13 @@ namespace xpyt std::string get_python_path() { + const char* python_exe_environment = std::getenv("PYTHON_EXECUTABLE"); + if (python_exe_environment != nullptr && std::strlen(python_exe_environment) != 0) + { + static const std::string python_exe_path = python_exe_environment; + return python_exe_path; + } + std::string python_prefix = get_python_prefix(); #ifdef _WIN32 if (python_prefix.back() != '\\') diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index d5fafe2c..5aceaf62 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -45,6 +45,9 @@ endif() find_package(Threads) find_package(doctest) +cmake_policy(SET CMP0167 OLD) # use find boost from old cmake +find_package(Boost REQUIRED COMPONENTS process filesystem) + include_directories(${GTEST_INCLUDE_DIRS} SYSTEM) set(XEUS_PYTHON_TESTS @@ -70,7 +73,7 @@ set_target_properties(test_xeus_python PROPERTIES ) include_directories(${PYTHON_INCLUDE_DIRS}) -target_link_libraries(test_xeus_python ${PYTHON_LIBRARIES} xeus-zmq doctest::doctest ${CMAKE_THREAD_LIBS_INIT}) +target_link_libraries(test_xeus_python ${PYTHON_LIBRARIES} xeus-zmq doctest::doctest Boost::headers Boost::filesystem Boost::process ${CMAKE_THREAD_LIBS_INIT}) target_include_directories(test_xeus_python PRIVATE ${XEUS_PYTHON_INCLUDE_DIR}) add_custom_target(xtest COMMAND test_xeus_python DEPENDS test_xeus_python) diff --git a/test/test_debugger.cpp b/test/test_debugger.cpp index 23e515f3..888a1b5b 100644 --- a/test/test_debugger.cpp +++ b/test/test_debugger.cpp @@ -8,8 +8,10 @@ * The full license is in the file LICENSE, distributed with this software. * ****************************************************************************/ + #include "doctest/doctest.h" + #include #include #include @@ -35,6 +37,12 @@ #include #endif +#ifndef WIN32_LEAN_AND_MEAN +#define WIN32_LEAN_AND_MEAN +#endif +#include +#include + /*********************************** * Should be moved in a utils file * ***********************************/ @@ -361,7 +369,7 @@ nl::json make_exception_breakpoint_request(int seq) })}, {"breakMode", "always"} }; - nl::json options = nl::json::array({except_option, except_option}); + nl::json options = nl::json::array({ except_option, except_option }); nl::json req = { {"type", "request"}, {"seq", seq}, @@ -383,8 +391,8 @@ class debugger_client public: debugger_client(xeus::xcontext& context, - const std::string& connection_file, - const std::string& log_file); + const std::string& connection_file, + const std::string& log_file); bool test_init(); bool test_disconnect(); @@ -430,10 +438,10 @@ class debugger_client }; debugger_client::debugger_client(xeus::xcontext& context, - const std::string& connection_file, - const std::string& log_file) + const std::string& connection_file, + const std::string& log_file) : m_client(context, "debugger_client", - std::get(xeus::load_configuration(connection_file)), log_file) + std::get(xeus::load_configuration(connection_file)), log_file) { } @@ -471,7 +479,7 @@ bool debugger_client::print_code_variable(const std::string& expected, int& seq) ++seq; nl::json json1 = m_client.receive_on_control(); - if(json1["content"]["body"]["stackFrames"].empty()) + if (json1["content"]["body"]["stackFrames"].empty()) { m_client.send_on_control("debug_request", make_stacktrace_request(seq, 1)); ++seq; @@ -491,11 +499,11 @@ bool debugger_client::print_code_variable(const std::string& expected, int& seq) const auto& ar = json3["content"]["body"]["variables"]; bool var_found = false; std::string name, value; - for(auto it = ar.begin(); it != ar.end() && !var_found; ++it) + for (auto it = ar.begin(); it != ar.end() && !var_found; ++it) { auto d = *it; name = d["name"]; - if(name == "i") + if (name == "i") { var_found = true; value = d["value"]; @@ -745,7 +753,7 @@ bool debugger_client::test_inspect_variables() auto check_var = [&vars](const std::string& name, const std::string& value) { auto x = std::find_if(vars.begin(), vars.end(), [&name](const nl::json& var) { return var.is_object() && var.value("name", "") == name; - }); + }); if (x == vars.end()) { std::cout << "missing " << name << std::endl; @@ -753,7 +761,7 @@ bool debugger_client::test_inspect_variables() } nl::json var = *x; return var["value"] == value && var["variablesReference"] == 0; - }; + }; bool res = check_var("i", "4") && check_var("j", "8") && check_var("k", "5"); return res; @@ -858,7 +866,7 @@ bool debugger_client::test_variables() ++seq; nl::json json1 = m_client.receive_on_control(); - if(json1["content"]["body"]["stackFrames"].empty()) + if (json1["content"]["body"]["stackFrames"].empty()) { m_client.send_on_control("debug_request", make_stacktrace_request(seq, 1)); ++seq; @@ -909,7 +917,7 @@ bool debugger_client::test_copy_to_globals() m_client.send_on_control("debug_request", make_stacktrace_request(seq, 1)); ++seq; nl::json json1 = m_client.receive_on_control(); - if(json1["content"]["body"]["stackFrames"].empty()) + if (json1["content"]["body"]["stackFrames"].empty()) { m_client.send_on_control("debug_request", make_stacktrace_request(seq, 1)); ++seq; @@ -935,7 +943,7 @@ bool debugger_client::test_copy_to_globals() ++seq; nl::json json3 = m_client.receive_on_control(); nl::json local_var = {}; - for (auto &var: json3["content"]["body"]["variables"]){ + for (auto& var : json3["content"]["body"]["variables"]) { if (var["evaluateName"] == local_var_name) { local_var = var; } @@ -952,7 +960,7 @@ bool debugger_client::test_copy_to_globals() ++seq; nl::json json4 = m_client.receive_on_control(); nl::json global_var = {}; - for (auto &var: json4["content"]["body"]["variables"]){ + for (auto& var : json4["content"]["body"]["variables"]) { if (var["evaluateName"] == global_var_name) { global_var = var; } @@ -1034,7 +1042,7 @@ std::string debugger_client::get_external_path() void debugger_client::dump_external_file() { static bool already_dumped = false; - if(!already_dumped) + if (!already_dumped) { std::ofstream out(get_external_path()); out << make_external_code() << std::endl; @@ -1065,6 +1073,8 @@ class timer { public: + struct timeout : std::runtime_error { using std::runtime_error::runtime_error; }; + timer(); ~timer(); @@ -1106,10 +1116,11 @@ void timer::notify_done() void timer::run_timer() { std::unique_lock lk(m_mcv); - if (!m_cv.wait_for(lk, std::chrono::seconds(20), [this]() { return m_done; })) + if (!m_cv.wait_for(lk, std::chrono::seconds(60), [this]() { return m_done; })) { - std::clog << "Unit test time out !!" << std::endl; - std::terminate(); + constexpr auto message = "Unit test time out !!"; + std::clog << message << std::endl; + throw timeout{ message }; // same as calling terminate if unhandled, but some impl will also display the error in the console } } @@ -1139,7 +1150,7 @@ void dump_connection_file() "kernel_name": "xcpp" } )"; - if(!dumped) + if (!dumped) { std::ofstream out(KERNEL_JSON); out << connection_file; @@ -1147,19 +1158,102 @@ void dump_connection_file() } } -void start_kernel() +struct KernelProcess { - dump_connection_file(); - std::string cmd = "xpython -f " + KERNEL_JSON + "&"; - int ret2 = std::system(cmd.c_str()); - std::this_thread::sleep_for(2s); -} + struct error : std::runtime_error { + using std::runtime_error::runtime_error; + }; + + KernelProcess() + { + running.emplace_back(m_impl); + + std::cout << "-> xpython sub-process started, waiting for ready message ..." << std::endl; + + constexpr std::string_view ready_message = "Run with XEUS"; // we expect this to appear in the error output of xpython once it's ready + std::string err_output; + try + { + boost::asio::read_until(m_impl->err_pipe, boost::asio::dynamic_buffer(err_output), ready_message); + } + catch (const std::exception& ex) + { + std::stringstream msgbuilder; + msgbuilder << "xpython startup failed: " << ex.what(); + msgbuilder << "\nxpython's standard error output : " << err_output; + auto message = msgbuilder.str(); + std::cerr << message << std::endl; + throw error(message); + } + std::cout << "-> xpython ready, proceeding with the test" << std::endl; + } + + ~KernelProcess() + { + if (m_impl) + { + std::cout << "-> xpython - stopping ..." << std::endl; + m_impl.reset(); + std::cout << "-> xpython - done" << std::endl; + } + } + + private: + struct Impl + { + bool _ = [] { dump_connection_file(); return true; }(); + boost::asio::io_context ctx; + boost::asio::readable_pipe err_pipe{ ctx }; + boost::filesystem::path xpython_path = boost::process::environment::find_executable("xpython"); + boost::process::process process{ ctx, xpython_path, { "-f" , KERNEL_JSON }, boost::process::process_stdio{{}, {}, err_pipe} }; + }; + + std::shared_ptr m_impl = std::make_shared(); + + struct on_program_exit + { + // Makes sure sub-processes are all destroyed even if an abort occcurs. + // Will not be invoked on quick_exit however. + ~on_program_exit() + { + const auto still_running_count = std::count_if(running.begin(), running.end(), [](const auto& maybe_impl) { return !maybe_impl.expired(); }); + if (still_running_count == 0) + { + return; + } + + std::cerr << "=> test program exiting (after `main()`): explicitly terminate remaining sub-processes (" << still_running_count << ") ..." << std::endl; + for (auto& maybe_process : running) + { + if (auto process_impl = maybe_process.lock()) + { + std::cerr << "=> request exit politely : " << process_impl->xpython_path << "(" << process_impl->process.id() << ") ..." << std::endl; + process_impl->process.request_exit(); + std::this_thread::sleep_for(std::chrono::seconds(2)); + if (process_impl->process.running()) + { + std::cerr << "=> still running, force terminate : " << process_impl->xpython_path << "(" << process_impl->process.id() << ") ..." << std::endl; + process_impl->process.terminate(); + process_impl->process.wait(); + } + std::cerr << "=> terminate " << process_impl->xpython_path << "(" << process_impl->process.id() << ") - DONE" << std::endl; + } + } + std::cerr << "=> test program exiting : explicitly terminate remaining sub-processes - DONE" << std::endl; + } + }; + static std::vector> running; + static on_program_exit run_on_program_exit; +}; + +std::vector> KernelProcess::running; +KernelProcess::on_program_exit KernelProcess::run_on_program_exit; TEST_SUITE("debugger") { TEST_CASE("init") { - start_kernel(); + KernelProcess xpython_process; timer t; auto context_ptr = xeus::make_zmq_context(); { @@ -1176,7 +1270,7 @@ TEST_SUITE("debugger") TEST_CASE("disconnect") { - start_kernel(); + KernelProcess xpython_process; timer t; auto context_ptr = xeus::make_zmq_context(); { @@ -1192,7 +1286,7 @@ TEST_SUITE("debugger") TEST_CASE("attach") { - start_kernel(); + KernelProcess xpython_process; timer t; auto context_ptr = xeus::make_zmq_context(); { @@ -1209,7 +1303,7 @@ TEST_SUITE("debugger") TEST_CASE("multisession") { - start_kernel(); + KernelProcess xpython_process; timer t; auto context_ptr = xeus::make_zmq_context(); { @@ -1228,7 +1322,7 @@ TEST_SUITE("debugger") TEST_CASE("set_external_breakpoints") { - start_kernel(); + KernelProcess xpython_process; timer t; auto context_ptr = xeus::make_zmq_context(); { @@ -1247,7 +1341,7 @@ TEST_SUITE("debugger") /* TEST_CASE("external_next_continue") { - start_kernel(); + KernelProcess xpython_process; timer t; auto context_ptr = xeus::make_zmq_context(); { @@ -1264,7 +1358,7 @@ TEST_SUITE("debugger") */ TEST_CASE("set_breakpoints") { - start_kernel(); + KernelProcess xpython_process; timer t; auto context_ptr = xeus::make_zmq_context(); { @@ -1281,7 +1375,7 @@ TEST_SUITE("debugger") TEST_CASE("set_exception_breakpoints") { - start_kernel(); + KernelProcess xpython_process; timer t; auto context_ptr = xeus::make_zmq_context(); { @@ -1298,7 +1392,7 @@ TEST_SUITE("debugger") TEST_CASE("source") { - start_kernel(); + KernelProcess xpython_process; timer t; auto context_ptr = xeus::make_zmq_context(); { @@ -1317,7 +1411,7 @@ TEST_SUITE("debugger") /* TEST_CASE("next_continue") { - start_kernel(); + KernelProcess xpython_process; timer t; auto context_ptr = xeus::make_zmq_context(); { @@ -1337,7 +1431,7 @@ TEST_SUITE("debugger") /* TEST_CASE("stepin") { - start_kernel(); + KernelProcess xpython_process; timer t; auto context_ptr = xeus::make_zmq_context(); { @@ -1355,7 +1449,7 @@ TEST_SUITE("debugger") TEST_CASE("stack_trace") { - start_kernel(); + KernelProcess xpython_process; timer t; auto context_ptr = xeus::make_zmq_context(); { @@ -1372,7 +1466,7 @@ TEST_SUITE("debugger") TEST_CASE("debug_info") { - start_kernel(); + KernelProcess xpython_process; timer t; auto context_ptr = xeus::make_zmq_context(); { @@ -1389,7 +1483,7 @@ TEST_SUITE("debugger") TEST_CASE("inspect_variables") { - start_kernel(); + KernelProcess xpython_process; timer t; auto context_ptr = xeus::make_zmq_context(); { @@ -1404,28 +1498,28 @@ TEST_SUITE("debugger") } } -// TODO: Get test_rich_inspect_variables to work -/* - TEST_CASE("rich_inspect_variables") - { - start_kernel(); - timer t; - auto context_ptr = xeus::make_zmq_context(); + // TODO: Get test_rich_inspect_variables to work + /* + TEST_CASE("rich_inspect_variables") { - debugger_client deb(*context_ptr, KERNEL_JSON, "debugger_rich_inspect_variables.log"); - deb.start(); - bool res = deb.test_rich_inspect_variables(); - deb.shutdown(); - std::this_thread::sleep_for(2s); - CHECK(res); - t.notify_done(); + KernelProcess xpython_process; + timer t; + auto context_ptr = xeus::make_zmq_context(); + { + debugger_client deb(*context_ptr, KERNEL_JSON, "debugger_rich_inspect_variables.log"); + deb.start(); + bool res = deb.test_rich_inspect_variables(); + deb.shutdown(); + std::this_thread::sleep_for(2s); + CHECK(res); + t.notify_done(); + } } - } -*/ + */ TEST_CASE("variables") { - start_kernel(); + KernelProcess xpython_process; timer t; auto context_ptr = xeus::make_zmq_context(); { @@ -1442,7 +1536,7 @@ TEST_SUITE("debugger") TEST_CASE("copy_to_globals") { - start_kernel(); + KernelProcess xpython_process; timer t; auto context_ptr = xeus::make_zmq_context(); { @@ -1456,4 +1550,4 @@ TEST_SUITE("debugger") t.notify_done(); } } -} +} \ No newline at end of file