[bugfix] Add a persistent version of rmtree()

reframe/core/pipeline.py

@@ -1108,7 +1108,7 @@ def cleanup(self, remove_files=False, unload_env=True):
 
         if remove_files:
             self.logger.debug('removing stage directory')
-            shutil.rmtree(self._stagedir)
+            os_ext.rmtree(self._stagedir)
 
         if unload_env:
             self.logger.debug("unloading test's environment")

reframe/core/runtime.py

@@ -5,7 +5,6 @@
 import os
 import functools
 import re
-import shutil
 import socket
 from datetime import datetime
 
@@ -107,7 +106,7 @@ def __init__(self, prefix=None, stagedir=None,
     def _makedir(self, *dirs, wipeout=False):
         ret = os.path.join(*dirs)
         if wipeout:
-            shutil.rmtree(ret, True)
+            os_ext.rmtree(ret, ignore_errors=True)
 
         os.makedirs(ret, exist_ok=True)
         return ret

reframe/utility/os_ext.py

@@ -2,6 +2,7 @@
 # OS and shell utility functions
 #
 
+import errno
 import getpass
 import grp
 import os
@@ -172,6 +173,39 @@ def ignore(dir, contents):
         os.symlink(f, link_name)
 
 
+def rmtree(*args, max_retries=3, **kwargs):
+    """Persistent version of ``shutil.rmtree()``.
+
+    If ``shutil.rmtree()`` with ``ENOTEMPTY``, retry up to ``max_retries``
+    times to delete the directory.
+
+    This version of ``rmtree()`` is mostly provided to work around a race
+    condition between when ``sacct`` reports a job as completed and when the
+    Slurm epilog runs. See https://github.com/eth-cscs/reframe/issues/291 for
+    more information.
+
+    ``args`` and ``kwargs`` are passed through to ``shutil.rmtree()``.
+
+    If ``onerror``  is specified in  ``kwargs`` and is not  :class:`None`, this
+    function is completely equivalent to ``shutil.rmtree()``.
+    """
+    if 'onerror' in kwargs and kwargs['onerror'] is not None:
+        shutil.rmtree(*args, **kwargs)
+        return
+
+    for i in range(max_retries):
+        try:
+            shutil.rmtree(*args, **kwargs)
+            return
+        except OSError as e:
+            if i == max_retries:
+                raise
+            elif e.errno == errno.ENOTEMPTY:
+                pass
+            else:
+                raise
+
+
 def inpath(entry, pathvar):
     """Check if entry is in pathvar. pathvar is a string of the form
     `entry1:entry2:entry3`."""

unittests/test_cli.py

@@ -2,7 +2,6 @@
 import itertools
 import os
 import re
-import shutil
 import sys
 import tempfile
 import unittest
@@ -93,8 +92,8 @@ def setUp(self):
         self.perflogdir = '.rfm-perflogs'
 
     def tearDown(self):
-        shutil.rmtree(self.prefix)
-        shutil.rmtree(self.perflogdir, ignore_errors=True)
+        os_ext.rmtree(self.prefix)
+        os_ext.rmtree(self.perflogdir, ignore_errors=True)
         os_ext.force_remove_file(self.logfile)
 
     def _run_reframe(self):

unittests/test_pipeline.py

@@ -1,9 +1,9 @@
 import os
-import shutil
 import tempfile
 import unittest
 
 import reframe.core.runtime as rt
+import reframe.utility.os_ext as os_ext
 import reframe.utility.sanity as sn
 import unittests.fixtures as fixtures
 from reframe.core.exceptions import (BuildError, PipelineError, ReframeError,
@@ -37,8 +37,8 @@ def setUp(self):
         rt.runtime().resources.prefix = tempfile.mkdtemp(dir='unittests')
 
     def tearDown(self):
-        shutil.rmtree(rt.runtime().resources.prefix, ignore_errors=True)
-        shutil.rmtree('.rfm_testing', ignore_errors=True)
+        os_ext.rmtree(rt.runtime().resources.prefix)
+        os_ext.rmtree('.rfm_testing', ignore_errors=True)
 
     def replace_prefix(self, filename, new_prefix):
         basename = os.path.basename(filename)
@@ -477,7 +477,7 @@ def tearDown(self):
         self.output_file.close()
         os.remove(self.perf_file.name)
         os.remove(self.output_file.name)
-        shutil.rmtree(self.resourcesdir)
+        os_ext.rmtree(self.resourcesdir)
 
     def write_performance_output(self, fp=None, **kwargs):
         if not fp:

unittests/test_policies.py

@@ -1,11 +1,11 @@
 import os
-import shutil
 import tempfile
 import unittest
 
 import reframe.core.runtime as rt
 import reframe.frontend.executors as executors
 import reframe.frontend.executors.policies as policies
+import reframe.utility.os_ext as os_ext
 from reframe.core.exceptions import JobNotStartedError
 from reframe.frontend.loader import RegressionCheckLoader
 from unittests.resources.checks.hellocheck import HelloTest
@@ -27,7 +27,7 @@ def setUp(self):
         rt.runtime().resources.prefix = tempfile.mkdtemp(dir='unittests')
 
     def tearDown(self):
-        shutil.rmtree(rt.runtime().resources.prefix, ignore_errors=True)
+        os_ext.rmtree(rt.runtime().resources.prefix)
 
     def _num_failures_stage(self, stage):
         stats = self.runner.stats

unittests/test_schedulers.py

@@ -1,7 +1,6 @@
 import abc
 import os
 import re
-import shutil
 import tempfile
 import time
 import unittest
@@ -36,7 +35,7 @@ def setUp(self):
         self.parallel_cmd = 'hostname'
 
     def tearDown(self):
-        shutil.rmtree(self.workdir)
+        os_ext.rmtree(self.workdir)
 
     @property
     def commands(self):
@@ -547,7 +546,7 @@ def setUp(self):
         self.testjob._num_tasks = 0
 
     def tearDown(self):
-        shutil.rmtree(self.workdir)
+        os_ext.rmtree(self.workdir)
 
     def test_valid_constraint(self, expected_num_tasks=8):
         self.testjob._sched_reservation = 'Foo'

unittests/test_utility.py

@@ -73,6 +73,26 @@ def test_copytree_src_parent_of_dst(self):
 
         shutil.rmtree(dst_path)
 
+    def _test_rmtree(self, *args, **kwargs):
+        testdir = tempfile.mkdtemp()
+        with open(os.path.join(testdir, 'foo.txt'), 'w') as fp:
+            fp.write('hello\n')
+
+        os_ext.rmtree(testdir, *args, **kwargs)
+        self.assertFalse(os.path.exists(testdir))
+
+    def test_rmtree(self):
+        self._test_rmtree()
+
+    def test_rmtree_onerror(self):
+        self._test_rmtree(onerror=lambda *args: None)
+
+    def test_rmtree_error(self):
+        # Try to remove an inexistent directory
+        testdir = tempfile.mkdtemp()
+        os.rmdir(testdir)
+        self.assertRaises(OSError, os_ext.rmtree, testdir)
+
     def test_inpath(self):
         self.assertTrue(os_ext.inpath('/foo/bin', '/bin:/foo/bin:/usr/bin'))
         self.assertFalse(os_ext.inpath('/foo/bin', '/bin:/usr/local/bin'))