diff --git a/.gitignore b/.gitignore index 62594fc0bc..ac94d9959f 100644 --- a/.gitignore +++ b/.gitignore @@ -23,3 +23,5 @@ virtualenv docs/_build docs/_generated .vscode +.coverage* +coverage*.xml diff --git a/CMakeLists.txt b/CMakeLists.txt index f4ccc837e7..bd728be932 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1019,6 +1019,11 @@ if(NRN_ENABLE_TESTS) if(NOT PYTEST_COV_FOUND) message(STATUS "pytest-cov package not installed. Python coverage will not be generated.") endif() + set(NRN_PYTEST_LAUNCHER -m pytest --capture=tee-sys) + # pytest-cov runs extremely slowly under AddressSanitizer + if(PYTEST_COV_FOUND AND NOT "address" IN_LIST NRN_SANITIZERS_LIST) + list(APPEND NRN_PYTEST_LAUNCHER --cov-report=xml --cov=neuron) + endif() endif() add_dependencies(nrniv_lib copy_share_demo_to_build) # Execute neurondemo as part of the build because it lazily calls nrnivmodl. If we don't do this diff --git a/share/lib/python/neuron/nmodl/ode.py b/share/lib/python/neuron/nmodl/ode.py index dbb4155ac4..59b23175d9 100644 --- a/share/lib/python/neuron/nmodl/ode.py +++ b/share/lib/python/neuron/nmodl/ode.py @@ -5,6 +5,11 @@ # Lesser General Public License. See top-level LICENSE file for details. # *********************************************************************** +# NOTE: because we are testing this via `exec` for the purposes of obtaining +# the correct coverage, do NOT use triple single quotation marks anywhere in +# the below because the code will break (should also be enforced by the +# formatter, but better safe than sorry)! + import re from importlib import import_module diff --git a/src/nmodl/pybind/CMakeLists.txt b/src/nmodl/pybind/CMakeLists.txt index e4af21839c..521f4fa50f 100644 --- a/src/nmodl/pybind/CMakeLists.txt +++ b/src/nmodl/pybind/CMakeLists.txt @@ -29,6 +29,9 @@ if(WIN32) # https://developercommunity.visualstudio.com/t/c-string-literal-max-length-much-shorter-than-docu/758957 string(REGEX REPLACE "\n\n" "\n)jiowi\" R\"jiowi(\n" NMODL_ODE_PY "${NMODL_ODE_PY}") endif() +if(NRN_ENABLE_COVERAGE) + set(NMODL_ODE_PY_PATH "${NMODL_PROJECT_PURELIB_SOURCE_DIR}/ode.py") +endif() configure_file(${CMAKE_CURRENT_SOURCE_DIR}/ode_py.hpp.inc ${CMAKE_CURRENT_BINARY_DIR}/ode_py.hpp @ONLY) @@ -46,6 +49,11 @@ else() target_compile_definitions(pyembed PRIVATE NMODL_STATIC_PYWRAPPER=1) endif() +if(NRN_ENABLE_COVERAGE) + target_compile_definitions(pywrapper PRIVATE NRN_ENABLE_COVERAGE) + target_link_libraries(pywrapper PRIVATE util) +endif() + target_link_libraries(pywrapper PRIVATE fmt::fmt) target_include_directories(pyembed PRIVATE ${PYBIND11_INCLUDE_DIR} ${PYTHON_INCLUDE_DIRS}) diff --git a/src/nmodl/pybind/ode_py.hpp.inc b/src/nmodl/pybind/ode_py.hpp.inc index b6eef4e21e..e3e7371760 100644 --- a/src/nmodl/pybind/ode_py.hpp.inc +++ b/src/nmodl/pybind/ode_py.hpp.inc @@ -15,5 +15,7 @@ namespace nmodl::pybind_wrappers { const std::string ode_py = R"jiowi( @NMODL_ODE_PY@ )jiowi"; - +#ifdef NRN_ENABLE_COVERAGE +const std::string ode_py_path = "@NMODL_ODE_PY_PATH@"; +#endif } diff --git a/src/nmodl/pybind/wrapper.cpp b/src/nmodl/pybind/wrapper.cpp index d59b579d97..2ad95bf7c3 100644 --- a/src/nmodl/pybind/wrapper.cpp +++ b/src/nmodl/pybind/wrapper.cpp @@ -4,27 +4,69 @@ * * SPDX-License-Identifier: Apache-2.0 */ +#include +#include +#include +#include -#include "wrapper.hpp" - -#include "codegen/codegen_naming.hpp" -#include "pybind/pyembed.hpp" +// 3rd party headers #include -#include #include #include -#include -#include +// NMODL headers +#include "codegen/codegen_naming.hpp" +#include "pybind/ode_py.hpp" +#include "pybind/wrapper.hpp" +#include "pybind/pyembed.hpp" +#include "utils/common_utils.hpp" -#include "ode_py.hpp" +namespace fs = std::filesystem; namespace py = pybind11; using namespace py::literals; namespace nmodl { namespace pybind_wrappers { +// This wrapper is used for obtaining better coverage in `ode.py`. +// Since we embed the `ode.py` as a string, there is no way to check what was covered via running +// pytest or similar. Instead, we use the coverage.py API directly. +static void run_python_script(const std::string& script, const py::dict& locals) { +#ifdef NRN_ENABLE_COVERAGE + // to prevent race conditions during testing, we generate a random suffix + const auto& suffix = + nmodl::utils::generate_random_string(20, nmodl::utils::UseNumbersInString::WithoutNumbers); + + py::exec(fmt::format(R"( +import coverage +cov = coverage.Coverage(data_suffix='{}') +cov.start() +)", + suffix), + locals); + const auto& code_with_mapping = std::string("exec(compile(r'''" + ode_py + script + "''', '" + + ode_py_path + "', 'exec'))"); + py::exec(code_with_mapping, locals); +#else + py::exec(ode_py + script, locals); +#endif + +#ifdef NRN_ENABLE_COVERAGE + const auto& path = fs::current_path() / fmt::format("coverage_{}.xml", suffix); + py::exec(fmt::format(R"( +cov.stop() +cov.save() +# Check if we have any coverage data +data = cov.get_data() +if data.measured_files(): + cov.xml_report(outfile='{}') +)", + path.string()), + locals); +#endif +} + std::tuple, std::vector, std::string> call_solve_linear_system(const std::vector& eq_system, const std::vector& state_vars, @@ -57,8 +99,7 @@ except Exception as e: new_local_vars = [""] exception_message = traceback.format_exc() )"; - - py::exec(nmodl::pybind_wrappers::ode_py + script, locals); + run_python_script(script, locals); // returns a vector of solutions, i.e. new statements to add to block: auto solutions = locals["solutions"].cast>(); // and a vector of new local variables that need to be declared in the block: @@ -93,7 +134,7 @@ except Exception as e: exception_message = traceback.format_exc() )"; - py::exec(nmodl::pybind_wrappers::ode_py + script, locals); + run_python_script(script, locals); // returns a vector of solutions, i.e. new statements to add to block: auto solutions = locals["solutions"].cast>(); // may also return a python exception message: @@ -130,7 +171,7 @@ except Exception as e: exception_message = traceback.format_exc() )"; - py::exec(nmodl::pybind_wrappers::ode_py + script, locals); + run_python_script(script, locals); } else if (method == codegen::naming::CNEXP_METHOD) { // replace x' = f(x) differential equation // with analytic solution for x(t+dt) in terms of x(t) @@ -147,7 +188,7 @@ except Exception as e: exception_message = traceback.format_exc() )"; - py::exec(nmodl::pybind_wrappers::ode_py + script, locals); + run_python_script(script, locals); } else { // nothing to do, but the caller should know. return {}; @@ -179,7 +220,7 @@ except Exception as e: exception_message = traceback.format_exc() )"; - py::exec(nmodl::pybind_wrappers::ode_py + script, locals); + run_python_script(script, locals); auto solution = locals["solution"].cast(); auto exception_message = locals["exception_message"].cast(); @@ -223,7 +264,7 @@ except Exception as e: statements, property.has_value() ? fmt::format("{}[{}]", name, property.value()) : name); - py::exec(nmodl::pybind_wrappers::ode_py + script, locals); + run_python_script(script, locals); auto solution = locals["solution"].cast(); auto exception_message = locals["exception_message"].cast(); diff --git a/test/nmodl/transpiler/unit/CMakeLists.txt b/test/nmodl/transpiler/unit/CMakeLists.txt index 27dcd175aa..10086da0a5 100644 --- a/test/nmodl/transpiler/unit/CMakeLists.txt +++ b/test/nmodl/transpiler/unit/CMakeLists.txt @@ -211,8 +211,22 @@ endif() # ============================================================================= if(NRN_ENABLE_PYTHON) if(NOT (CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" AND NRN_SANITIZERS)) - add_test(NAME Ode COMMAND ${PYTHON_EXECUTABLE} -m pytest ${CMAKE_CURRENT_SOURCE_DIR}/ode) - set_tests_properties(Ode PROPERTIES ENVIRONMENT "PYTHONPATH=${NMODL_TEST_PYTHONPATH}") + # Most of the Python tests added by `add_nrn_test` run in their own directories. Unfortunately, + # pytest-cov does not seem to have an option to specify the output file (it _always_ outputs to + # `coverage.xml`), which means that the various tests here may clobber each other's coverage + # reports. The way out of this is to a) either specify `WORKING_DIRECTORY` of each test, or b) + # use a coverage config file (see + # https://coverage.readthedocs.io/en/latest/config.html#xml-output); the latter requires far + # more work than the former, so we use option a) in the below. + file(MAKE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/ode) + add_test( + NAME Ode + COMMAND ${PYTHON_EXECUTABLE} ${NRN_PYTEST_LAUNCHER} ${CMAKE_CURRENT_SOURCE_DIR}/ode + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/ode) + set_tests_properties( + Ode + PROPERTIES ENVIRONMENT + "PYTHONPATH=${NMODL_TEST_PYTHONPATH};NEURONHOME=${PROJECT_BINARY_DIR}/share/nrn") cpp_cc_configure_sanitizers(TEST Ode PRELOAD) endif() @@ -220,11 +234,17 @@ if(NRN_ENABLE_PYTHON) # Apple Clang and ASAN do not play along nicely with NMODL's Python bindings, so we skip these # tests if(NOT (CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" AND NRN_SANITIZERS)) - add_test(NAME Pybind COMMAND ${PYTHON_EXECUTABLE} -m pytest - ${CMAKE_CURRENT_SOURCE_DIR}/pybind) + file(MAKE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/pybind) + add_test( + NAME Pybind + COMMAND ${PYTHON_EXECUTABLE} ${NRN_PYTEST_LAUNCHER} ${CMAKE_CURRENT_SOURCE_DIR}/pybind + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/pybind) set_tests_properties( - Pybind PROPERTIES ENVIRONMENT - "PYTHONPATH=${NMODL_TEST_PYTHONPATH};NMODLHOME=${PROJECT_BINARY_DIR}") + Pybind + PROPERTIES + ENVIRONMENT + "PYTHONPATH=${NMODL_TEST_PYTHONPATH};NMODLHOME=${PROJECT_BINARY_DIR};NEURONHOME=${PROJECT_BINARY_DIR}/share/nrn" + ) cpp_cc_configure_sanitizers(TEST Pybind PRELOAD) endif() endif()