Merge pull request #93 from Flamefire/more_tests

Add more tests, fix bug in add_to_ld_library_path and typos

Author: AndreasGocht

CMakeLists.txt

@@ -31,7 +31,7 @@ add_custom_target(ScorepModule ALL
 
 enable_testing()
 add_test(NAME ScorepPythonTests
-         COMMAND Python::Interpreter -m pytest test_scorep.py
+         COMMAND Python::Interpreter -m pytest
          WORKING_DIRECTORY ${CMAKE_CURRENT_LIST_DIR}/test
 )
 set(pythonPath ${CMAKE_CURRENT_BINARY_DIR}/site-packages)

DEVELOPING.md

@@ -22,7 +22,7 @@ Although, we do not actively maintain the CMake build system, and will not help
 
 You might find this build system helpful for development, especially if you are doing C/C++ things:
 * Include paths for C++ are correctly searched for and set up for use by IDEs or other tools. For example Visual Studio Code works out of the box, given the appropriate extensions (C++, Python, CMake) are installed.
-* A folder `site-packages` is created in the build folder where the C/C++ extension module and the scorep module are copied to on each build (e.g. `make`-call). Hence it is possible to add that folder to the PYTHONPATH environment variable, build the project and start debugging or execute the tests in test/test_scorep.py.
+* A folder `site-packages` is created in the build folder where the C/C++ extension module and the scorep module are copied to on each build (e.g. `make`-call). Hence it is possible to add that folder to the PYTHONPATH environment variable, build the project and start debugging or execute the tests in test.
 * A `test` target exists which can be run to execute all tests.
 
 Please note, that changes to the Python source files are not reflected in the build folder unless a build is executed.

scorep/__main__.py

@@ -11,7 +11,6 @@ def _err_exit(msg):
 
 
 def scorep_main(argv=None):
-    # print(sys.flags)
     if argv is None:
         argv = sys.argv
 
@@ -61,10 +60,9 @@ def scorep_main(argv=None):
     if len(prog_argv) == 0:
         _err_exit("Did not find a script to run")
 
-    if ("SCOREP_PYTHON_BINDINGS_INITALISED" not in os.environ) or (
-            os.environ["SCOREP_PYTHON_BINDINGS_INITALISED"] != "true"):
+    if os.environ.get("SCOREP_PYTHON_BINDINGS_INITIALISED") != "true":
         scorep.subsystem.init_environment(scorep_config, keep_files)
-        os.environ["SCOREP_PYTHON_BINDINGS_INITALISED"] = "true"
+        os.environ["SCOREP_PYTHON_BINDINGS_INITIALISED"] = "true"
         """
         python -m starts the module as skript. i.e. sys.argv will loke like:
         ['/home/gocht/Dokumente/code/scorep_python/scorep.py', '--mpi', 'mpi_test.py']
@@ -80,7 +78,7 @@ def scorep_main(argv=None):
 
         os.execve(sys.executable, new_args, os.environ)
     else:
-        scorep.subsystem.reset_pereload()
+        scorep.subsystem.reset_preload()
 
     # everything is ready
     sys.argv = prog_argv

scorep/helper.py

@@ -68,15 +68,10 @@ def add_to_ld_library_path(path):
     adds the path to the LD_LIBRARY_PATH.
     @param path path to be added
     """
-    if ("LD_LIBRARY_PATH" not in os.environ):
-        os.environ["LD_LIBRARY_PATH"] = ""
-
-    if (path not in os.environ["LD_LIBRARY_PATH"]):
-        if os.environ["LD_LIBRARY_PATH"] == "":
-            os.environ["LD_LIBRARY_PATH"] = path
-        else:
-            os.environ["LD_LIBRARY_PATH"] = path + \
-                ":" + os.environ["LD_LIBRARY_PATH"]
+    library_path = os.environ.get("LD_LIBRARY_PATH", "")
+    library_paths = library_path.split(":") if library_path else []
+    if path not in library_paths:
+        os.environ["LD_LIBRARY_PATH"] = ':'.join([path] + library_paths)
 
 
 def generate_compile_deps(config=[]):

scorep/instrumenter.py

@@ -47,7 +47,7 @@ def unregister():
     """
     Disables the python-tracing.
     Disabling the python-tracing is more efficient than disable_recording,
-    as python does not longer call the tracing module.
+    as python does no longer call the tracing module.
     However, all the other things that are traced by Score-P will still be recorded.
     Please call register() to enable tracing again.
     """
@@ -61,7 +61,7 @@ class enable():
     with enable(region_name=None):
         do stuff
     ```
-    This overides --no-instrumenter (--nopython leagacy)
+    This overides --noinstrumenter (--nopython legacy)
     If a region name is given, the region the contextmanager is active will be marked in the trace or profile
     """
 
@@ -103,7 +103,7 @@ class disable():
     with disable():
         do stuff
     ```
-    This overides --no-instrumenter (--nopython leagacy)
+    This overides --noinstrumenter (--nopython legacy)
     If a region name is given, the region the contextmanager is active will be marked in the trace or profile
     """
 

scorep/subsystem.py

@@ -18,8 +18,8 @@ def generate_subsystem_lib_name():
 
 def generate_ld_preload(scorep_config):
     """
-    This functions generate a string that needs to be passed to $LD_PRELOAD.
-    After this sting is passed, the tracing needs to be restarted with this $LD_PRELOAD in env.
+    This functions generates a string that needs to be passed to $LD_PRELOAD.
+    After this string is passed, the tracing needs to be restarted with this $LD_PRELOAD in env.
 
     @return ld_preload string which needs to be passed to LD_PRELOAD
     """
@@ -103,15 +103,12 @@ def init_environment(scorep_config=[], keep_files=False):
     """
     Set the inital needed environmet variables, to get everythin up an running.
     As a few variables interact with LD env vars, the programms needs to be restarted after this.
-    The function set the env var `SCOREP_PYTHON_BINDINGS_INITALISED` to true, once it is done with
-    initalising.
 
     @param scorep_config configuration flags for score-p
     @param keep_files whether to keep the generated files, or not.
     """
 
-    if ("LD_PRELOAD" in os.environ) and (
-            "libscorep" in os.environ["LD_PRELOAD"]):
+    if "libscorep" in os.environ.get("LD_PRELOAD", ""):
         raise RuntimeError(
             "Score-P is already loaded. This should not happen at this point")
 
@@ -132,7 +129,7 @@ def init_environment(scorep_config=[], keep_files=False):
     os.environ["LD_PRELOAD"] = preload_str
 
 
-def reset_pereload():
+def reset_preload():
     """
     resets the environment variable `LD_PRELOAD` to the value before init_environment was called.
     """
@@ -152,6 +149,5 @@ def clean_up(keep_files=True):
     if keep_files:
         return
     else:
-        if ("SCOREP_PYTHON_BINDINGS_TEMP_DIR" in os.environ) and (
-                os.environ["SCOREP_PYTHON_BINDINGS_TEMP_DIR"] != ""):
+        if os.environ.get("SCOREP_PYTHON_BINDINGS_TEMP_DIR"):
             shutil.rmtree(os.environ["SCOREP_PYTHON_BINDINGS_TEMP_DIR"])

scorep/user.py

@@ -8,7 +8,7 @@
 def region_begin(name, file_name=None, line_number=None):
     """
     Begin of an User region. If file_name or line_number is None, both will
-    bet determined automatically
+    be determined automatically
     @param name name of the user region
     @param file_name file name of the user region
     @param line_number line number of the user region
@@ -131,7 +131,7 @@ def __exit__(self, exc_type, exc_value, traceback):
 def rewind_begin(name, file_name=None, line_number=None):
     """
     Begin of an User region. If file_name or line_number is None, both will
-    bet determined automatically
+    be determined automatically
     @param name name of the user region
     @param file_name file name of the user region
     @param line_number line number of the user region
@@ -162,7 +162,7 @@ def rewind_end(name, value):
 def oa_region_begin(name, file_name=None, line_number=None):
     """
     Begin of an Online Access region. If file_name or line_number is None, both will
-    bet determined automatically
+    be determined automatically
     @param name name of the user region
     @param file_name file name of the user region
     @param line_number line number of the user region

src/methods.cpp

@@ -151,7 +151,7 @@ extern "C"
         Py_RETURN_NONE;
     }
 
-    static PyObject* get_expiriment_dir_name(PyObject* self, PyObject* args)
+    static PyObject* get_experiment_dir_name(PyObject* self, PyObject* args)
     {
 
         return PyUnicode_FromString(SCOREP_GetExperimentDirName());
@@ -169,7 +169,7 @@ extern "C"
         { "parameter_int", parameter_int, METH_VARARGS, "User parameter int." },
         { "parameter_uint", parameter_uint, METH_VARARGS, "User parameter uint." },
         { "parameter_string", parameter_string, METH_VARARGS, "User parameter string." },
-        { "get_expiriment_dir_name", get_expiriment_dir_name, METH_VARARGS,
+        { "get_experiment_dir_name", get_experiment_dir_name, METH_VARARGS,
           "Get the Score-P experiment dir." },
         { NULL, NULL, 0, NULL } /* Sentinel */
     };

test/cases/mpi.py

@@ -1,10 +1,13 @@
 #!/usr/bin/env python
 
+import scorep
 from mpi4py import MPI
 import numpy as np
 import mpi4py
+import instrumentation2
 mpi4py.rc.thread_level = "funneled"
 
+scorep.instrumenter.register()
 
 comm = mpi4py.MPI.COMM_WORLD
 
@@ -14,7 +17,9 @@
 N = 5
 if comm.rank == 0:
     A = np.arange(N, dtype=np.float64)    # rank 0 has proper data
+    instrumentation2.baz()
 else:
+    instrumentation2.bar()
     A = np.empty(N, dtype=np.float64)     # all other just an empty array
 
 # Broadcast A from rank 0 to everybody

test/test_helper.py

@@ -0,0 +1,24 @@
+import os
+import scorep.helper
+
+
+def test_add_to_ld_library_path(monkeypatch):
+    # Previous value: Empty
+    monkeypatch.setenv('LD_LIBRARY_PATH', '')
+    scorep.helper.add_to_ld_library_path('/my/path')
+    assert os.environ['LD_LIBRARY_PATH'] == '/my/path'
+    # Don't add duplicates
+    scorep.helper.add_to_ld_library_path('/my/path')
+    assert os.environ['LD_LIBRARY_PATH'] == '/my/path'
+    # Prepend
+    scorep.helper.add_to_ld_library_path('/new/folder')
+    assert os.environ['LD_LIBRARY_PATH'] == '/new/folder:/my/path'
+    # also no duplicates:
+    for p in ('/my/path', '/new/folder'):
+        scorep.helper.add_to_ld_library_path(p)
+        assert os.environ['LD_LIBRARY_PATH'] == '/new/folder:/my/path'
+
+    # Add parent folder of existing one
+    monkeypatch.setenv('LD_LIBRARY_PATH', '/some/folder:/parent/sub')
+    scorep.helper.add_to_ld_library_path('/parent')
+    assert os.environ['LD_LIBRARY_PATH'] == '/parent:/some/folder:/parent/sub'