DM-35901: Add new test case for importing every python file in a package

.github/workflows/build.yaml

@@ -13,7 +13,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        python-version: ["3.8", "3.9", "3.10", "3.11"]
+        python-version: ["3.9", "3.10", "3.11"]
 
     steps:
       - uses: actions/checkout@v3
@@ -96,7 +96,7 @@ jobs:
       - name: Set up Python
         uses: actions/setup-python@v4
         with:
-          python-version: 3.8
+          python-version: "3.10"
 
       - name: Install dependencies
         run: |

doc/changes/DM-35901.feature.rst

@@ -0,0 +1,3 @@
+Added ``lsst.utils.tests.ImportTestCase`` class.
+This test case can be used to force the import of every file in a package.
+This can be very useful for spotting obvious problems if a package does not export every file by default.

doc/changes/DM-35901.removal.rst

@@ -0,0 +1 @@
+Dropped support for Python 3.8.

pyproject.toml

@@ -15,7 +15,6 @@ classifiers = [
     "License :: OSI Approved :: BSD License",
     "Operating System :: OS Independent",
     "Programming Language :: Python :: 3",
-    "Programming Language :: Python :: 3.8",
     "Programming Language :: Python :: 3.9",
     "Programming Language :: Python :: 3.10",
     "Programming Language :: Python :: 3.11",

python/lsst/utils/packages.py

@@ -534,7 +534,7 @@ def write(self, filename: str) -> None:
             ff.write(self.toBytes(self.formats[ext]))
 
     def __str__(self) -> str:
-        ss = "%s({\n" % self.__class__.__name__
+        ss = self.__class__.__name__ + "({\n"
         # Sort alphabetically by module name, for convenience in reading
         ss += ",\n".join(f"{prod!r}:{self[prod]!r}" for prod in sorted(self))
         ss += ",\n})"

python/lsst/utils/tests.py

@@ -17,6 +17,7 @@
     "init",
     "MemoryTestCase",
     "ExecutablesTestCase",
+    "ImportTestCase",
     "getTempFilePath",
     "TestCase",
     "assertFloatsAlmostEqual",
@@ -31,6 +32,7 @@
 import contextlib
 import functools
 import gc
+import importlib.resources as resources
 import inspect
 import itertools
 import os
@@ -41,12 +43,14 @@
 import tempfile
 import unittest
 import warnings
-from collections.abc import Callable, Iterator, Mapping, Sequence
-from typing import Any
+from collections.abc import Callable, Iterable, Iterator, Mapping, Sequence
+from typing import Any, ClassVar
 
 import numpy
 import psutil
 
+from .doImport import doImport
+
 # Initialize the list of open files to an empty set
 open_files = set()
 
@@ -160,7 +164,7 @@
         diff = now_open.difference(open_files)
         if diff:
             for f in diff:
-                print("File open: %s" % f)
+                print(f"File open: {f}")
             self.fail("Failed to close %d file%s" % (len(diff), "s" if len(diff) != 1 else ""))
 
 
@@ -331,6 +335,67 @@
             cls._build_test_method(e, ref_dir)
 
 
+class ImportTestCase(unittest.TestCase):
+    """Test that the named packages can be imported and all files within
+    that package.
+
+    The test methods are created dynamically. Callers must subclass this
+    method and define the ``PACKAGES`` property.
+    """
+
+    PACKAGES: ClassVar[Iterable[str]] = ()
+    """Packages to be imported."""
+
+    _n_registered = 0
+    """Number of packages registered for testing by this class."""
+
+    def _test_no_packages_registered_for_import_testing(self) -> None:
+        """Test when no packages have been registered.
+
+        Without this, if no packages have been listed no tests will be
+        registered and the test system will not report on anything. This
+        test fails and reports why.
+        """
+        raise AssertionError("No packages registered with import test. Was the PACKAGES property set?")
+
+    def __init_subclass__(cls, **kwargs: Any) -> None:
+        """Create the test methods based on the content of the ``PACKAGES``
+        class property.
+        """
+        super().__init_subclass__(**kwargs)
+
+        for mod in cls.PACKAGES:
+            test_name = "test_import_" + mod.replace(".", "_")
+
+            def test_import(*args: Any) -> None:
+                self = args[0]
+                self.assertImport(mod)
+
+            test_import.__name__ = test_name
+            setattr(cls, test_name, test_import)
+            cls._n_registered += 1
+
+        # If there are no packages listed that is likely a mistake and
+        # so register a failing test.
+        if cls._n_registered == 0:
+            setattr(cls, "test_no_packages_registered", cls._test_no_packages_registered_for_import_testing)
+
+    def assertImport(self, root_pkg):
+        for file in resources.files(root_pkg).iterdir():
+            file = file.name
+            if not file.endswith(".py"):
+                continue
+            if file.startswith("__"):
+                continue
+            root, _ = os.path.splitext(file)
+            module_name = f"{root_pkg}.{root}"
+            with self.subTest(module=module_name):
+                try:
+                    doImport(module_name)
+                except ImportError as e:
+                    raise AssertionError(f"Error importing module {module_name}: {e}") from e
+
+
 @contextlib.contextmanager
 def getTempFilePath(ext: str, expectOutput: bool = True) -> Iterator[str]:
     """Return a path suitable for a temporary file and try to delete the
@@ -687,17 +752,14 @@
             if rtol is None:
                 errMsg = [f"{lhs} {cmpStr} {rhs}; diff={absDiff} with atol={atol}"]
             elif atol is None:
-                errMsg = [
-                    "%s %s %s; diff=%s/%s=%s with rtol=%s"
-                    % (lhs, cmpStr, rhs, absDiff, relTo, absDiff / relTo, rtol)
-                ]
+                errMsg = [f"{lhs} {cmpStr} {rhs}; diff={absDiff}/{relTo}={absDiff / relTo} with rtol={rtol}"]
             else:
                 errMsg = [
-                    "%s %s %s; diff=%s/%s=%s with rtol=%s, atol=%s"
-                    % (lhs, cmpStr, rhs, absDiff, relTo, absDiff / relTo, rtol, atol)
+                    f"{lhs} {cmpStr} {rhs}; diff={absDiff}/{relTo}={absDiff / relTo} "
+                    f"with rtol={rtol}, atol={atol}"
                 ]
         else:
-            errMsg = ["%d/%d elements %s with rtol=%s, atol=%s" % (bad.sum(), bad.size, failStr, rtol, atol)]
+            errMsg = [f"{bad.sum()}/{bad.size} elements {failStr} with rtol={rtol}, atol={atol}"]
             if plotOnFailure:
                 if len(lhs.shape) != 2 or len(rhs.shape) != 2:
                     raise ValueError("plotOnFailure is only valid for 2-d arrays")

tests/test_import.py

@@ -0,0 +1,32 @@
+# This file is part of utils.
+#
+# Developed for the LSST Data Management System.
+# This product includes software developed by the LSST Project
+# (http://www.lsst.org).
+# See the COPYRIGHT file at the top-level directory of this distribution
+# for details of code ownership.
+#
+# Use of this source code is governed by a 3-clause BSD-style
+# license that can be found in the LICENSE file.
+
+import unittest
+
+from lsst.utils.tests import ImportTestCase
+
+
+class TestImports(ImportTestCase):
+    """Test we can run the import tests.
+
+    All the code in utils is being tested and imported already but this
+    will confirm the test infrastructure works.
+    """
+
+    PACKAGES = ("lsst.utils",)
+
+    def test_counter(self):
+        """Test that the expected number of packages were registered."""
+        self.assertEqual(self._n_registered, 1)
+
+
+if __name__ == "__main__":
+    unittest.main()