tests: Handle backup command exceptions as test failures, not errors

If the backup command fails (i.e. throws an exception), we want the test
to fail. This makes it easier to mark tests as 'knownfail' (because we
can't knownfail test errors).

In theory, this should just involve updating run_cmd() to catch any
exceptions from the command and then call self.fail().

However, if the backup command fails, it can leave behind files in the
targetdir. Partly this is intentional, as these files may provide clues
to users as to why the command failed. However, in selftest, it causes
the TestCaseInTempDir._remove_tempdir() assertion to fire. Because this
assert actually gets run as part of the teardown, the assertion gets
treated as an error rather than a failure (and so we can't knownfail the
backup tests). To get around this, we remove any files in the tempdir
prior to calling self.fail().

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

diff --git a/python/samba/tests/__init__.py b/python/samba/tests/__init__.py
index d79fcfbb997466bb0b7f4cb0854e3b6b61fd98d7..9b30d2efa1ed55452acda15126c13dcdb032bd4a 100644 (file)
--- a/python/samba/tests/__init__.py
+++ b/python/samba/tests/__init__.py
@@ -294,6 +294,7 @@ class TestCaseInTempDir(TestCase):
         self.addCleanup(self._remove_tempdir)
 
     def _remove_tempdir(self):
+        # Note asserting here is treated as an error rather than a test failure
         self.assertEquals([], os.listdir(self.tempdir))
         os.rmdir(self.tempdir)
         self.tempdir = None

diff --git a/python/samba/tests/domain_backup.py b/python/samba/tests/domain_backup.py
index 5b68ea92b5e043784619d7d1c249f269ab68f6c7..b9e97eb3369e032c5bddf2209237a2ee4ea94168 100644 (file)
--- a/python/samba/tests/domain_backup.py
+++ b/python/samba/tests/domain_backup.py
@@ -360,6 +360,11 @@ class DomainBackupBase(SambaToolCmdTest, TestCaseInTempDir):
                             not in owner.extended_str(),
                             "%s found as FSMO %s role owner" % (server, role))
 
+    def cleanup_tempdir(self):
+        for filename in os.listdir(self.tempdir):
+            filepath = os.path.join(self.tempdir, filename)
+            shutil.rmtree(filepath)
+
     def run_cmd(self, args):
         """Executes a samba-tool backup/restore command"""
 
@@ -369,7 +374,14 @@ class DomainBackupBase(SambaToolCmdTest, TestCaseInTempDir):
         # settings can bleed from one test case to another).
         cmd = " ".join(args)
         print("Executing: samba-tool %s" % cmd)
-        out = self.check_output("samba-tool " + cmd)
+        try:
+            out = self.check_output("samba-tool " + cmd)
+        except BlackboxProcessError as e:
+            # if the command failed, it may have left behind temporary files.
+            # We're going to fail the test, but first cleanup any temp files so
+            # that we skip the TestCaseInTempDir._remove_tempdir() assertions
+            self.cleanup_tempdir()
+            self.fail("Error calling samba-tool: %s" % e)
         print(out)
 
     def create_backup(self, extra_args=None):