testing: fixed an issue with the handling of tempdirs (#866)

* testing: changed legacy TestCaseWithTmp to use pytest-based tmpdir_class fixture

* added missing pytest import

* disable ncbi tests again

* make sure module tempdir is nested in session tempdir,
class tempdir in module tempdir, function tempdir in class tempdir

* removed a blank line to force a rebuild

DEVELOPMENT_NOTES.md

@@ -54,6 +54,5 @@ Some TO DO improvements for the future:
    - Second-stage jobs that pull the docker image should utilize quay.io's torrent squashed image pull to reduce the time spent pulling our Docker image (currently about 5 minutes to pull from DockerHub).
    - Alternatively, we can explore creating a minimal docker image that installs only the conda pip packages (and perhaps extremely common conda tools like samtools and Picard) and leaves the rest of the conda tools out, letting them dynamically install themselves as needed using our dynamic tool install code.
 
-
 ### Building documentation
 Documentation is built automatically for certain branches of viral-ngs by [Read the Docs](http://viral-ngs.readthedocs.io/en/latest/). The documentation template files reside within `viral-ngs/docs`, and are formatted in standard docutils [reStructuredText format](http://docutils.sourceforge.net/rst.html). [Pandoc](http://pandoc.org/) may be used for converting from other formats (such as Markdown) to reStructuredText. The `sphinx-argparse` module is used to automatically generate documentation for the argparse parsers used in viral-ngs.

conftest.py

@@ -32,33 +32,51 @@ def pytest_configure(config):
     reporter = FixtureReporter(config)
     config.pluginmanager.register(reporter, 'fixturereporter')
 
+#
+# Fixtures for creating a temp dir at session/module/class/function scope.
+# Unlike pytest's tmpdir fixture, they use tempfile.mkdtemp to create a 
+# tempdir in the most secure/race-condition-free manner possible.
+# Also, since util.file.tmp_dir() is used, the tempdir contens can be
+# preserved for debugging by setting the environment variable VIRAL_NGS_TMP_DIRKEEP.
+#
+
 @contextlib.contextmanager
-def _tmpdir_aux(request, tmpdir_factory, scope, name):
+def _tmpdir_aux(base_dir, scope, name):
     """Create and return a temporary directory; remove it and its contents on context exit."""
-    with util.file.tmp_dir(dir=str(tmpdir_factory.getbasetemp()),
+    with util.file.tmp_dir(dir=base_dir,
                            prefix='test-{}-{}-'.format(scope, name)) as tmpdir:
         yield tmpdir
 
 @pytest.fixture(scope='session')
 def tmpdir_session(request, tmpdir_factory):
     """Create a session-scope temporary directory."""
-    with _tmpdir_aux(request, tmpdir_factory, 'session', id(request.session)) as tmpdir:
+    with _tmpdir_aux(str(tmpdir_factory.getbasetemp()),
+                     'session', id(request.session)) as tmpdir:
         yield tmpdir
 
 @pytest.fixture(scope='module')
-def tmpdir_module(request, tmpdir_factory):
+def tmpdir_module(request, tmpdir_session):
     """Create a module-scope temporary directory."""
-    with _tmpdir_aux(request, tmpdir_factory, 'module', request.module.__name__) as tmpdir:
+    with _tmpdir_aux(tmpdir_session, 'module', request.module.__name__) as tmpdir:
+        yield tmpdir
+
+@pytest.fixture(scope='class')
+def tmpdir_class(request, tmpdir_module):
+    """Create a class-scope temporary directory."""
+    with _tmpdir_aux(tmpdir_module, 'class',
+                     request.cls.__name__ if request.cls else '__noclass__') as tmpdir:
         yield tmpdir
 
 @pytest.fixture(autouse=True)
-def tmpdir_function(request, tmpdir_factory, monkeypatch):
+def tmpdir_function(request, tmpdir_class, monkeypatch):
     """Create a temporary directory and set it to be used by the tempfile module and as the TMPDIR environment variable."""
-    with _tmpdir_aux(request, tmpdir_factory, 'node', request.node.name) as tmpdir:
+    with _tmpdir_aux(tmpdir_class, 'node', request.node.name) as tmpdir:
         monkeypatch.setattr(tempfile, 'tempdir', tmpdir)
         monkeypatch.setenv('TMPDIR', tmpdir)
         yield tmpdir
 
+#############################################################################################
+
 class FixtureReporter:
 
     def __init__(self, config):

test/__init__.py

@@ -12,6 +12,7 @@
 
 # third-party
 import Bio.SeqIO
+import pytest
 
 # intra-project
 import util.file
@@ -80,23 +81,10 @@ def assert_md5_equal_to_line_in_file(testCase, filename, checksum_file, msg=None
 
     testCase.assertEqual(hash_md5.hexdigest(), expected_checksum, msg=msg)
 
+@pytest.mark.usefixtures('tmpdir_class')
 class TestCaseWithTmp(unittest.TestCase):
     'Base class for tests that use tempDir'
 
-    @classmethod
-    def setUpClass(cls):
-        cls._class_tempdir = util.file.set_tmp_dir(cls.__name__)
-
-    def setUp(self):
-        util.file.set_tmp_dir(type(self).__name__)
-
-    @classmethod
-    def tearDownClass(cls):
-        util.file.destroy_tmp_dir(cls._class_tempdir)
-
-    def tearDown(self):
-        util.file.destroy_tmp_dir()
-
     def assertEqualContents(self, f1, f2):
         assert_equal_contents(self, f1, f2)