From: Tim Beale <timbeale@catalyst.net.nz>
Date: Thu, 22 Nov 2018 01:35:58 +0000 (+1300)
Subject: tests: Handle backup command exceptions as test failures, not errors
X-Git-Tag: tdb-1.3.17~637
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=70dee4c1c14ea3eb5c8c81887c6caaa6dc288807;p=thirdparty%2Fsamba.git

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 d79fcfbb997..9b30d2efa1e 100644
--- 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 5b68ea92b5e..b9e97eb3369 100644
--- 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):