Rework coverage for NMODL ODE solver and pybind (#3544)

* Rework coverage

* Use readlink -f for getting the abs path to modfile

* Okay I give up on nrnivmodl

Put back whatever hackery it used before to collect the mod files, if
some of the files are missing coverage I really don't care.

* Try using a different coverage database file

* Update gitignore

also add comment about using single triple quotes

---------

Co-authored-by: Goran Jelic-Cizmek <goran.jelic-cizmek@epfl.ch>
Co-authored-by: nrnhines <michael.hines@yale.edu>

Author: 3 people

.gitignore

@@ -23,3 +23,5 @@ virtualenv
 docs/_build
 docs/_generated
 .vscode
+.coverage*
+coverage*.xml

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

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
 

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})

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
 }

src/nmodl/pybind/wrapper.cpp

@@ -4,27 +4,69 @@
  *
  * SPDX-License-Identifier: Apache-2.0
  */
+#include <filesystem>
+#include <optional>
+#include <set>
+#include <vector>
 
-#include "wrapper.hpp"
-
-#include "codegen/codegen_naming.hpp"
-#include "pybind/pyembed.hpp"
+// 3rd party headers
 #include <fmt/format.h>
-#include <optional>
 #include <pybind11/embed.h>
 #include <pybind11/stl.h>
 
-#include <set>
-#include <vector>
+// 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>, std::vector<std::string>, std::string>
 call_solve_linear_system(const std::vector<std::string>& eq_system,
                          const std::vector<std::string>& 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<std::vector<std::string>>();
     // 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<std::vector<std::string>>();
     // 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<std::string>();
     auto exception_message = locals["exception_message"].cast<std::string>();
@@ -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<std::string>();
     auto exception_message = locals["exception_message"].cast<std::string>();

test/nmodl/transpiler/unit/CMakeLists.txt

@@ -211,20 +211,40 @@ 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()
 
   if(NMODL_ENABLE_PYTHON_BINDINGS)
     # 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()