Improve the loading of GMT's shared library

1. Need to catch `GMTCLibError` error from calling `check_libgmt()`
2. Add a new parameter `lib_fullnames` (default to `clib_full_names()`) to
   `load_libgmt()` so that we can test more cases
3. Skip a library path if it's known to fail in previous tries
4. Improve the error message, because each library path may fail due to
   different reasons.
5. Add more tests

Author: seisman

pygmt/clib/loading.py

@@ -14,13 +14,18 @@
 from pygmt.exceptions import GMTCLibError, GMTCLibNotFoundError, GMTOSError
 
 
-def load_libgmt():
+def load_libgmt(lib_fullnames=None):
     """
     Find and load ``libgmt`` as a :py:class:`ctypes.CDLL`.
 
-    By default, will look for the shared library in the directory specified by
-    the environment variable ``GMT_LIBRARY_PATH``. If it's not set, will let
-    ctypes try to find the library.
+    Will look for the GMT shared library in the directories determined by
+    clib_full_names().
+
+    Parameters
+    ----------
+    lib_fullnames : list of str or None
+        List of possible full names of GMT's shared library. If ``None``, will
+        default to ``clib_full_names()``.
 
     Returns
     -------
@@ -33,22 +38,29 @@ def load_libgmt():
         If there was any problem loading the library (couldn't find it or
         couldn't access the functions).
     """
-    lib_fullnames = []
+    if lib_fullnames is None:
+        lib_fullnames = clib_full_names()
+
     error = True
-    for libname in clib_full_names():
-        lib_fullnames.append(libname)
+    error_msg = []
+    failing_libs = []
+    for libname in lib_fullnames:
         try:
+            if libname in failing_libs:  # libname is known to fail, so skip it
+                continue
             libgmt = ctypes.CDLL(libname)
             check_libgmt(libgmt)
             error = False
             break
-        except OSError as err:
-            error = err
+        except (OSError, GMTCLibError) as err:
+            error_msg.append(
+                f"Error loading the GMT shared library '{libname}'.\n{err}"
+            )
+            failing_libs.append(libname)
+
     if error:
-        raise GMTCLibNotFoundError(
-            "Error loading the GMT shared library "
-            f"{', '.join(lib_fullnames)}.\n {error}."
-        )
+        raise GMTCLibNotFoundError("\n".join(error_msg))
+
     return libgmt
 
 

pygmt/tests/test_clib_loading.py

@@ -1,6 +1,7 @@
 """
 Test the functions that load libgmt.
 """
+import ctypes
 import shutil
 import subprocess
 import sys
@@ -12,15 +13,23 @@
 from pygmt.exceptions import GMTCLibError, GMTCLibNotFoundError, GMTOSError
 
 
+class FakedLibGMT:  # pylint: disable=too-few-public-methods
+    """
+    Class for faking a GMT library.
+    """
+
+    def __init__(self, name):
+        self._name = name
+
+    def __str__(self):
+        return self._name
+
+
 def test_check_libgmt():
     """
     Make sure check_libgmt fails when given a bogus library.
     """
-    # create a fake library with a "_name" property
-    def libgmt():
-        pass
-
-    libgmt._name = "/path/to/libgmt.so"  # pylint: disable=protected-access
+    libgmt = FakedLibGMT("/path/to/libgmt.so")
     msg = (
         # pylint: disable=protected-access
         f"Error loading '{libgmt._name}'. "
@@ -33,6 +42,22 @@ def libgmt():
         check_libgmt(libgmt)
 
 
+def test_clib_names():
+    """
+    Make sure we get the correct library name for different OS names.
+    """
+    for linux in ["linux", "linux2", "linux3"]:
+        assert clib_names(linux) == ["libgmt.so"]
+    assert clib_names("darwin") == ["libgmt.dylib"]
+    assert clib_names("win32") == ["gmt.dll", "gmt_w64.dll", "gmt_w32.dll"]
+    for freebsd in ["freebsd10", "freebsd11", "freebsd12"]:
+        assert clib_names(freebsd) == ["libgmt.so"]
+    with pytest.raises(GMTOSError):
+        clib_names("meh")
+
+
+###############################################################################
+# Tests for load_libgmt
 def test_load_libgmt():
     """
     Test that loading libgmt works and doesn't crash.
@@ -64,19 +89,85 @@ def test_load_libgmt_with_a_bad_library_path(monkeypatch):
     assert check_libgmt(load_libgmt()) is None
 
 
-def test_clib_names():
+def test_load_libgmt_with_broken_libraries(monkeypatch):
     """
-    Make sure we get the correct library name for different OS names.
+    Test load_libgmt still works when a broken library is found.
     """
-    for linux in ["linux", "linux2", "linux3"]:
-        assert clib_names(linux) == ["libgmt.so"]
-    assert clib_names("darwin") == ["libgmt.dylib"]
-    assert clib_names("win32") == ["gmt.dll", "gmt_w64.dll", "gmt_w32.dll"]
-    for freebsd in ["freebsd10", "freebsd11", "freebsd12"]:
-        assert clib_names(freebsd) == ["libgmt.so"]
-    with pytest.raises(GMTOSError):
-        clib_names("meh")
+    # load the GMT library before mocking the ctypes.CDLL function
+    loaded_libgmt = load_libgmt()
+
+    def mock_ctypes_cdll_return(libname):
+        """
+        Mock the return value of ctypes.CDLL.
+
+        Parameters
+        ----------
+        libname : str or FakedLibGMT or ctypes.CDLL
+            Path to the GMT library, a faked GMT library or a working library
+            loaded as ctypes.CDLL.
+
+        Return
+        ------
+        object
+            Either the loaded GMT library or the faked GMT library.
+        """
+        if isinstance(libname, FakedLibGMT):
+            # libname is a faked GMT library, return the faked library
+            return libname
+        if isinstance(libname, str):
+            # libname is an invalid library path in str type,
+            # raise OSError like the original ctypes.CDLL
+            raise OSError(f"Unable to find '{libname}'")
+        # libname is a loaded GMT library
+        return loaded_libgmt
+
+    with monkeypatch.context() as mpatch:
+        # pylint: disable=protected-access
+        # mock the ctypes.CDLL using mock_ctypes_cdll_return()
+        mpatch.setattr(ctypes, "CDLL", mock_ctypes_cdll_return)
+
+        faked_libgmt1 = FakedLibGMT("/path/to/faked/libgmt1.so")
+        faked_libgmt2 = FakedLibGMT("/path/to/faked/libgmt2.so")
+
+        # case 1: two broken libraries
+        # Raise the GMTCLibNotFoundError exception
+        # The error message should contains information of both libraries
+        lib_fullnames = [faked_libgmt1, faked_libgmt2]
+        msg_regex = (
+            fr"Error loading the GMT shared library '{faked_libgmt1._name}'.\n"
+            fr"Error loading '{faked_libgmt1._name}'. Couldn't access.*\n"
+            fr"Error loading the GMT shared library '{faked_libgmt2._name}'.\n"
+            fr"Error loading '{faked_libgmt2._name}'. Couldn't access.*"
+        )
+        with pytest.raises(GMTCLibNotFoundError, match=msg_regex):
+            load_libgmt(lib_fullnames=lib_fullnames)
+
+        # case 2: broken library + invalid path
+        lib_fullnames = [faked_libgmt1, "/invalid/path/to/libgmt.so"]
+        msg_regex = (
+            fr"Error loading the GMT shared library '{faked_libgmt1._name}'.\n"
+            fr"Error loading '{faked_libgmt1._name}'. Couldn't access.*\n"
+            "Error loading the GMT shared library '/invalid/path/to/libgmt.so'.\n"
+            "Unable to find '/invalid/path/to/libgmt.so'"
+        )
+        with pytest.raises(GMTCLibNotFoundError, match=msg_regex):
+            load_libgmt(lib_fullnames=lib_fullnames)
+
+        # case 3: broken library + invalid path + working library
+        lib_fullnames = [faked_libgmt1, "/invalid/path/to/libgmt.so", loaded_libgmt]
+        assert check_libgmt(load_libgmt(lib_fullnames=lib_fullnames)) is None
+
+        # case 4: invalid path + broken library + working library
+        lib_fullnames = ["/invalid/path/to/libgmt.so", faked_libgmt1, loaded_libgmt]
+        assert check_libgmt(load_libgmt(lib_fullnames=lib_fullnames)) is None
+
+        # case 5: working library + broken library + invalid path
+        lib_fullnames = [loaded_libgmt, faked_libgmt1, "/invalid/path/to/libgmt.so"]
+        assert check_libgmt(load_libgmt(lib_fullnames=lib_fullnames)) is None
 
+        # case 6: repeated broken library + working library
+        lib_fullnames = [faked_libgmt1, faked_libgmt1, loaded_libgmt]
+        assert check_libgmt(load_libgmt(lib_fullnames=lib_fullnames)) is None
 
 ###############################################################################
 # Tests for clib_full_names