Merge pull request #452 from lsst/tickets/DM-28042

DM-28042: configure test temporary directory location via envvar

SConstruct

@@ -2,7 +2,15 @@
 from lsst.sconsUtils import scripts
 from lsst.sconsUtils import state
 
+import os
+
 scripts.BasicSConstruct("daf_butler", disableCc=True)
 mypy = state.env.Command("mypy.log", "python/lsst/daf/butler",
                          "mypy python/lsst 2>&1 | tee -a mypy.log")
 state.env.Alias("mypy", mypy)
+
+# Propagate environment variables used only by this package through SCons.
+envvars = ["DAF_BUTLER_TEST_TMP"]
+for e in envvars:
+    if e in os.environ:
+        state.env["ENV"][e] = os.environ[e]

python/lsst/daf/butler/tests/utils.py

@@ -19,13 +19,20 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+from __future__ import annotations
 
-import astropy
-from astropy.table import Table as AstropyTable
-from astropy.utils.diff import report_diff_values
+__all__ = ()
+
+from contextlib import contextmanager
 import io
 import os
+import shutil
+import tempfile
+from typing import Optional
 
+import astropy
+from astropy.table import Table as AstropyTable
+from astropy.utils.diff import report_diff_values
 
 from .. import (
     Butler,
@@ -36,6 +43,67 @@
 from ..registry import CollectionType
 
 
+def makeTestTempDir(default_base: str) -> str:
+    """Create a temporary directory for test usage.
+
+    The directory will be created within ``DAF_BUTLER_TEST_TMP`` if that
+    environment variable is set, falling back to ``default_base`` if it is
+    not.
+
+    Parameters
+    ----------
+    default_base : `str`
+        Default parent directory.
+
+    Returns
+    -------
+    dir : `str`
+        Name of the new temporary directory.
+    """
+    base = os.environ.get("DAF_BUTLER_TEST_TMP", default_base)
+    return tempfile.mkdtemp(dir=base)
+
+
+def removeTestTempDir(root: Optional[str]) -> None:
+    """Attempt to remove a temporary test directory, but do not raise if
+    unable to.
+
+    Unlike `tempfile.TemporaryDirectory`, this passes ``ignore_errors=True``
+    to ``shutil.rmtree`` at close, making it safe to use on NFS.
+
+    Parameters
+    ----------
+    root : `str`, optional
+        Name of the directory to be removed.  If `None`, nothing will be done.
+    """
+    if root is not None and os.path.exists(root):
+        shutil.rmtree(root, ignore_errors=True)
+
+
+@contextmanager
+def safeTestTempDir(default_base: str) -> str:
+    """Return a context manager that creates a temporary directory and then
+    attempts to remove it.
+
+    Parameters
+    ----------
+    default_base : `str`
+        Default parent directory, forwarded to `makeTestTempDir`.
+
+    Returns
+    -------
+    context : `contextlib.ContextManager`
+        A context manager that returns the new directory name on ``__enter__``
+        and removes the temporary directory (via `removeTestTempDir`) on
+        ``__exit__``.
+    """
+    root = makeTestTempDir(default_base)
+    try:
+        yield root
+    finally:
+        removeTestTempDir(root)
+
+
 class ButlerTestHelper:
     """Mixin with helpers for unit tests."""
 

requirements.txt

@@ -17,5 +17,9 @@ pyarrow >= 0.16
 responses == 0.12.0
 urllib3 == 1.25
 
+# We don't use mock directly, but something else (moto?) does and is
+# incompatible with mock == 4.0.3.
+mock == 4.0.2
+
 # These are required by lsst.utils
 psutil >= 5.7

tests/test_astropyTableFormatter.py

@@ -23,14 +23,13 @@
 """
 
 import unittest
-import tempfile
 import os
-import shutil
 import numpy
 
 from astropy.table import Table
 
 from lsst.daf.butler import Butler, DatasetType
+from lsst.daf.butler.tests.utils import makeTestTempDir, removeTestTempDir
 
 TESTDIR = os.path.abspath(os.path.dirname(__file__))
 
@@ -40,7 +39,7 @@ class AstropyTableFormatterTestCase(unittest.TestCase):
     """
 
     def setUp(self):
-        self.root = tempfile.mkdtemp(dir=TESTDIR)
+        self.root = makeTestTempDir(TESTDIR)
         Butler.makeRepo(self.root)
         ints = [1, 2, 3]
         names = ['one', 'two', 'three']
@@ -49,8 +48,7 @@ def setUp(self):
                            names=['ints', 'names', 'transcendentals'])
 
     def tearDown(self):
-        if os.path.exists(self.root):
-            shutil.rmtree(self.root, ignore_errors=True)
+        removeTestTempDir(self.root)
         del self.table
 
     def testAstropyTableFormatter(self):

tests/test_butler.py

@@ -71,6 +71,7 @@ def mock_s3(cls):
 from lsst.daf.butler.core._butlerUri.http import isWebdavEndpoint
 
 from lsst.daf.butler.tests import MultiDetectorFormatter, MetricsExample
+from lsst.daf.butler.tests.utils import makeTestTempDir, removeTestTempDir, safeTestTempDir
 
 TESTDIR = os.path.abspath(os.path.dirname(__file__))
 
@@ -142,8 +143,7 @@ def assertGetComponents(self, butler, datasetRef, components, reference, collect
             self.assertEqual(result_deferred, result)
 
     def tearDown(self):
-        if self.root is not None and os.path.exists(self.root):
-            shutil.rmtree(self.root, ignore_errors=True)
+        removeTestTempDir(self.root)
 
     def runPutGetTest(self, storageClass, datasetTypeName):
         # New datasets will be added to run and tag, but we will only look in
@@ -413,7 +413,7 @@ class ButlerTests(ButlerPutGetTests):
 
     def setUp(self):
         """Create a new butler root for each test."""
-        self.root = tempfile.mkdtemp(dir=TESTDIR)
+        self.root = makeTestTempDir(TESTDIR)
         Butler.makeRepo(self.root, config=Config(self.configFile))
         self.tmpConfigFile = os.path.join(self.root, "butler.yaml")
 
@@ -984,7 +984,7 @@ def runImportExportTest(self, storageClass):
         skymapRecord = {"name": "example_skymap", "hash": (50).to_bytes(8, byteorder="little")}
         exportButler.registry.insertDimensionData("skymap", skymapRecord)
         # Export and then import datasets.
-        with tempfile.TemporaryDirectory() as exportDir:
+        with safeTestTempDir(TESTDIR) as exportDir:
             exportFile = os.path.join(exportDir, "exports.yaml")
             with exportButler.export(filename=exportFile, directory=exportDir, transfer="auto") as export:
                 export.saveDatasets(datasets)
@@ -999,7 +999,7 @@ def runImportExportTest(self, storageClass):
                 # Save some dimension records directly.
                 export.saveDimensionData("skymap", [skymapRecord])
             self.assertTrue(os.path.exists(exportFile))
-            with tempfile.TemporaryDirectory() as importDir:
+            with safeTestTempDir(TESTDIR) as importDir:
                 # We always want this to be a local posix butler
                 Butler.makeRepo(importDir, config=Config(os.path.join(TESTDIR, "config/basic/butler.yaml")))
                 # Calling script.butlerImport tests the implementation of the
@@ -1046,7 +1046,7 @@ def testExportTransferCopy(self):
                             f"Checking path {path}")
 
         for transfer in ("copy", "link", "symlink", "relsymlink"):
-            with tempfile.TemporaryDirectory(dir=TESTDIR) as exportDir:
+            with safeTestTempDir(TESTDIR) as exportDir:
                 with exportButler.export(directory=exportDir, format="yaml",
                                          transfer=transfer) as export:
                     export.saveDatasets(datasets)
@@ -1089,7 +1089,7 @@ class ButlerExplicitRootTestCase(PosixDatastoreButlerTestCase):
     fullConfigKey = None
 
     def setUp(self):
-        self.root = tempfile.mkdtemp(dir=TESTDIR)
+        self.root = makeTestTempDir(TESTDIR)
 
         # Make a new repository in one place
         self.dir1 = os.path.join(self.root, "dir1")
@@ -1119,8 +1119,8 @@ class ButlerMakeRepoOutfileTestCase(ButlerPutGetTests, unittest.TestCase):
     configFile = os.path.join(TESTDIR, "config/basic/butler.yaml")
 
     def setUp(self):
-        self.root = tempfile.mkdtemp(dir=TESTDIR)
-        self.root2 = tempfile.mkdtemp(dir=TESTDIR)
+        self.root = makeTestTempDir(TESTDIR)
+        self.root2 = makeTestTempDir(TESTDIR)
 
         self.tmpConfigFile = os.path.join(self.root2, "different.yaml")
         Butler.makeRepo(self.root, config=Config(self.configFile),
@@ -1149,8 +1149,8 @@ class ButlerMakeRepoOutfileDirTestCase(ButlerMakeRepoOutfileTestCase):
     configFile = os.path.join(TESTDIR, "config/basic/butler.yaml")
 
     def setUp(self):
-        self.root = tempfile.mkdtemp(dir=TESTDIR)
-        self.root2 = tempfile.mkdtemp(dir=TESTDIR)
+        self.root = makeTestTempDir(TESTDIR)
+        self.root2 = makeTestTempDir(TESTDIR)
 
         self.tmpConfigFile = self.root2
         Butler.makeRepo(self.root, config=Config(self.configFile),
@@ -1169,8 +1169,8 @@ class ButlerMakeRepoOutfileUriTestCase(ButlerMakeRepoOutfileTestCase):
     configFile = os.path.join(TESTDIR, "config/basic/butler.yaml")
 
     def setUp(self):
-        self.root = tempfile.mkdtemp(dir=TESTDIR)
-        self.root2 = tempfile.mkdtemp(dir=TESTDIR)
+        self.root = makeTestTempDir(TESTDIR)
+        self.root2 = makeTestTempDir(TESTDIR)
 
         self.tmpConfigFile = ButlerURI(os.path.join(self.root2, "something.yaml")).geturl()
         Butler.makeRepo(self.root, config=Config(self.configFile),
@@ -1235,7 +1235,7 @@ def setUp(self):
         config.update({"datastore": {"datastore": {"root": rooturi}}})
 
         # need local folder to store registry database
-        self.reg_dir = tempfile.mkdtemp(dir=TESTDIR)
+        self.reg_dir = makeTestTempDir(TESTDIR)
         config["registry", "db"] = f"sqlite:///{self.reg_dir}/gen3.sqlite3"
 
         # MOTO needs to know that we expect Bucket bucketname to exist
@@ -1367,7 +1367,7 @@ def setUp(self):
         config.update({"datastore": {"datastore": {"root": self.rooturi}}})
 
         # need local folder to store registry database
-        self.reg_dir = tempfile.mkdtemp(dir=TESTDIR)
+        self.reg_dir = makeTestTempDir(TESTDIR)
         config["registry", "db"] = f"sqlite:///{self.reg_dir}/gen3.sqlite3"
 
         self.datastoreStr = f"datastore={self.root}"

tests/test_cliCmdPruneCollection.py

@@ -25,14 +25,18 @@
 from astropy.table import Table
 from numpy import array
 import os
-import shutil
-import tempfile
 import unittest
 
 from lsst.daf.butler import Butler
 from lsst.daf.butler.cli.butler import cli as butlerCli
 from lsst.daf.butler.cli.utils import clickResultMsg, LogCliRunner
-from lsst.daf.butler.tests.utils import ButlerTestHelper, MetricTestRepo, readTable
+from lsst.daf.butler.tests.utils import (
+    ButlerTestHelper,
+    makeTestTempDir,
+    MetricTestRepo,
+    readTable,
+    removeTestTempDir,
+)
 
 
 TESTDIR = os.path.abspath(os.path.dirname(__file__))
@@ -94,13 +98,12 @@ class PruneCollectionExecutionTest(unittest.TestCase, ButlerTestHelper):
     def setUp(self):
         self.runner = LogCliRunner()
 
-        self.root = tempfile.mkdtemp(dir=TESTDIR)
+        self.root = makeTestTempDir(TESTDIR)
         self.testRepo = MetricTestRepo(self.root,
                                        configFile=os.path.join(TESTDIR, "config/basic/butler.yaml"))
 
     def tearDown(self):
-        if os.path.exists(self.root):
-            shutil.rmtree(self.root, ignore_errors=True)
+        removeTestTempDir(self.root)
 
     def testPruneRun(self):
         result = self.runner.invoke(butlerCli, ["query-collections", self.root])

tests/test_cliCmdQueryDataIds.py

@@ -25,13 +25,11 @@
 from astropy.table import Table as AstropyTable
 from numpy import array
 import os
-import shutil
-import tempfile
 import unittest
 
 from lsst.daf.butler import Butler
 from lsst.daf.butler import script
-from lsst.daf.butler.tests.utils import ButlerTestHelper, MetricTestRepo
+from lsst.daf.butler.tests.utils import ButlerTestHelper, makeTestTempDir, MetricTestRepo, removeTestTempDir
 
 
 TESTDIR = os.path.abspath(os.path.dirname(__file__))
@@ -52,13 +50,12 @@ def _queryDataIds(repo, dimensions=(), collections=(), datasets=None, where=None
                                    where=where)
 
     def setUp(self):
-        self.root = tempfile.mkdtemp(dir=TESTDIR)
+        self.root = makeTestTempDir(TESTDIR)
         self.repo = MetricTestRepo(root=self.root,
                                    configFile=os.path.join(TESTDIR, "config/basic/butler.yaml"))
 
     def tearDown(self):
-        if os.path.exists(self.root):
-            shutil.rmtree(self.root, ignore_errors=True)
+        removeTestTempDir(self.root)
 
     def testDimensions(self):
         """Test getting a dimension."""

tests/test_cliCmdQueryDatasets.py

@@ -25,14 +25,12 @@
 from astropy.table import Table as AstropyTable
 from numpy import array
 import os
-import shutil
-import tempfile
 import unittest
 
 from lsst.daf.butler import StorageClassFactory
 from lsst.daf.butler import script
 from lsst.daf.butler.tests import addDatasetType
-from lsst.daf.butler.tests.utils import ButlerTestHelper, MetricTestRepo
+from lsst.daf.butler.tests.utils import ButlerTestHelper, makeTestTempDir, MetricTestRepo, removeTestTempDir
 
 
 TESTDIR = os.path.abspath(os.path.dirname(__file__))
@@ -50,13 +48,12 @@ def _queryDatasets(repo, glob=(), collections=(), where="", find_first=False, sh
         return script.queryDatasets(repo, glob, collections, where, find_first, show_uri)
 
     def setUp(self):
-        self.root = tempfile.mkdtemp(dir=TESTDIR)
+        self.root = makeTestTempDir(TESTDIR)
         self.testRepo = MetricTestRepo(self.root,
                                        configFile=os.path.join(TESTDIR, "config/basic/butler.yaml"))
 
     def tearDown(self):
-        if os.path.exists(self.root):
-            shutil.rmtree(self.root, ignore_errors=True)
+        removeTestTempDir(self.root)
 
     def testShowURI(self):
         """Test for expected output with show_uri=True."""