tests: Rework backup test inheritance to make LP constraints clearer

The backup tests have a special constraint where we always want to use
check_output() over runcmd(). The reason is we need the samba-tool
backup/restore commands executed in a separate process. Otherwise the
global underlying LoadParm can accumulate settings from earlier test
case runs.

We can avoid someone in future inadvertently running runcmd() by
mistake, by simply changing the inheritance so we no longer inherit from
SambaToolCmdTest (so the runcmd functions are no longer present).

The comment explaining this has been moved to the top of the file.

Note that the TestCaseInTempDir inheritance was redundant.
BlackboxTestCase inherits from TestCaseInTempDir (and SambaToolCmdTest
was inheriting from BlackboxTestCase).

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>

Autobuild-User(master): Tim Beale <timbeale@samba.org>
Autobuild-Date(master): Tue Nov 27 06:57:03 CET 2018 on sn-devel-144

2 files changed, 21 insertions, 14 deletions

diff --git a/python/samba/tests/domain_backup.py b/python/samba/tests/domain_backup.py
index 7147f0fdfe4..2a5df739d2c 100644
--- a/python/samba/tests/domain_backup.py
+++ b/python/samba/tests/domain_backup.py
@@ -18,9 +18,8 @@ from samba import provision, param
 import tarfile
 import os
 import shutil
-from samba.tests.samba_tool.base import SambaToolCmdTest
-from samba.tests import (TestCaseInTempDir, env_loadparm, create_test_ou,
-                         BlackboxProcessError)
+from samba.tests import (env_loadparm, create_test_ou, BlackboxProcessError,
+                         BlackboxTestCase, connect_samdb)
 import ldb
 from samba.samdb import SamDB
 from samba.auth import system_session
@@ -39,8 +38,14 @@ def get_prim_dom(secrets_path, lp):
                               scope=ldb.SCOPE_SUBTREE,
                               expression="(objectClass=kerberosSecret)")
 
-
-class DomainBackupBase(SambaToolCmdTest, TestCaseInTempDir):
+# The backup tests require that a completely clean LoadParm object gets used
+# for the restore. Otherwise the same global LP gets re-used, and the LP
+# settings can bleed from one test case to another.
+# To do this, these tests should use check_output(), which executes the command
+# in a separate process (as opposed to runcmd(), runsubcmd()).
+# So although this is a samba-tool test, we don't inherit from SambaToolCmdTest
+# so that we never inadvertently use .runcmd() by accident.
+class DomainBackupBase(BlackboxTestCase):
 
     def setUp(self):
         super(DomainBackupBase, self).setUp()
@@ -50,8 +55,7 @@ class DomainBackupBase(SambaToolCmdTest, TestCaseInTempDir):
                                        os.environ["DC_PASSWORD"])
 
         # LDB connection to the original server being backed up
-        self.ldb = self.getSamDB("-H", "ldap://%s" % server,
-                                 self.user_auth)
+        self.ldb = connect_samdb("ldap://%s" % server)
         self.new_server = "BACKUPSERV"
         self.server = server.upper()
         self.base_cmd = None
@@ -368,13 +372,10 @@ class DomainBackupBase(SambaToolCmdTest, TestCaseInTempDir):
     def run_cmd(self, args):
         """Executes a samba-tool backup/restore command"""
 
-        # we use check_output() here to execute the command because we want the
-        # command run in a separate process. This means a completely clean
-        # LoadParm object gets used for the restore (otherwise the global LP
-        # settings can bleed from one test case to another).
         cmd = " ".join(args)
         print("Executing: samba-tool %s" % cmd)
         try:
+            # note: it's important we run the cmd in a separate process here
             out = self.check_output("samba-tool " + cmd)
         except BlackboxProcessError as e:
             # if the command failed, it may have left behind temporary files.
diff --git a/python/samba/tests/domain_backup_offline.py b/python/samba/tests/domain_backup_offline.py
index f5fa1561de2..8b7209ec24d 100644
--- a/python/samba/tests/domain_backup_offline.py
+++ b/python/samba/tests/domain_backup_offline.py
@@ -19,11 +19,17 @@ import tarfile
 import os
 import shutil
 import tempfile
-from samba.tests.samba_tool.base import SambaToolCmdTest
-from samba.tests import TestCaseInTempDir
+from samba.tests import BlackboxTestCase
 from samba.netcmd import CommandError
 
-class DomainBackupOfflineCmp(SambaToolCmdTest, TestCaseInTempDir):
+# The backup tests require that a completely clean LoadParm object gets used
+# for the restore. Otherwise the same global LP gets re-used, and the LP
+# settings can bleed from one test case to another.
+# To do this, these tests should use check_output(), which executes the command
+# in a separate process (as opposed to runcmd(), runsubcmd()).
+# So although this is a samba-tool test, we don't inherit from SambaToolCmdTest
+# so that we never inadvertently use .runcmd() by accident.
+class DomainBackupOfflineCmp(BlackboxTestCase):
 
     def test_domain_backup_offline_untar_tdb(self):
         self.untar_testcase('tdb')