Fixup Fixtures Use in db test classes.

With Fixture objects and test cases it is important to upcall first
rather than later to allow base classes to configure their internal
test tracking state. Failing to do so allows bugs like this one to
occur - when failure occurs early, the state required to gather
environmental details about the failure is not prepared, and the
test machinery will raise secondary exceptions as it tries to do that.
This may even obscure the original error.

I've taken a fairly minimal approach here - I think it would be
possible and likely even desirable to get rid of the test classes
altogether - this module appears to have very little, if any, actual
test code - its all glue for managing the test database connections.
However I know zzzeek is working on a fairly large reorg in this
area and I don't want to conceptually clash with that - we can do
further cleanups later.

Co-Authored-By: Robert Collins <rbtcollins@hp.com>

Change-Id: Ic9f24aa2352d097a10be8420b701fa9f588b21b0
Closes-Bug: #1330763

1 files changed, 20 insertions, 27 deletions

diff --git a/oslo/db/sqlalchemy/test_base.py b/oslo/db/sqlalchemy/test_base.py
index 849e726..ace4e94 100644
--- a/oslo/db/sqlalchemy/test_base.py
+++ b/oslo/db/sqlalchemy/test_base.py
@@ -26,6 +26,7 @@ except ImportError:
                     ' test-requirements')
 
 import six
+import testtools
 
 from oslo.db.sqlalchemy import provision
 from oslo.db.sqlalchemy import session
@@ -49,13 +50,11 @@ class DbFixture(fixtures.Fixture):
 
         self.test = test
 
-    def cleanUp(self):
-        self.test.engine.dispose()
-
     def setUp(self):
         super(DbFixture, self).setUp()
 
         self.test.engine = session.create_engine(self._get_uri())
+        self.addCleanup(self.test.engine.dispose)
         self.test.sessionmaker = session.get_maker(self.test.engine)
 
 
@@ -110,8 +109,21 @@ class OpportunisticFixture(DbFixture):
 
     DRIVER = abc.abstractproperty(lambda: None)
     DBNAME = PASSWORD = USERNAME = 'openstack_citest'
+    _uri = None
+
+    def _get_uri(self):
+        if self._uri is not None:
+            return self._uri
+
+        credentials = {
+            'backend': self.DRIVER,
+            'user': self.USERNAME,
+            'passwd': self.PASSWORD,
+            'database': self.DBNAME}
+        if self.DRIVER and not utils.is_backend_avail(**credentials):
+            msg = '%s backend is not available.' % self.DRIVER
+            raise testtools.testcase.TestSkipped(msg)
 
-    def setUp(self):
         self._provisioning_engine = provision.get_engine(
             utils.get_connect_string(backend=self.DRIVER,
                                      user=self.USERNAME,
@@ -119,15 +131,9 @@ class OpportunisticFixture(DbFixture):
                                      database=self.DBNAME)
         )
         self._uri = provision.create_database(self._provisioning_engine)
-
-        super(OpportunisticFixture, self).setUp()
-
-    def cleanUp(self):
-        super(OpportunisticFixture, self).cleanUp()
-
-        provision.drop_database(self._provisioning_engine, self._uri)
-
-    def _get_uri(self):
+        self.addCleanup(
+            provision.drop_database, self._provisioning_engine, self._uri)
+        self.addCleanup(setattr, self, '_uri', None)
         return self._uri
 
 
@@ -136,24 +142,11 @@ class OpportunisticTestCase(DbTestCase):
     """Base test case to use default CI databases.
 
     The subclasses of the test case are running only when openstack_citest
-    database is available otherwise a tests will be skipped.
+    database is available otherwise tests will be skipped.
     """
 
     FIXTURE = abc.abstractproperty(lambda: None)
 
-    def setUp(self):
-        credentials = {
-            'backend': self.FIXTURE.DRIVER,
-            'user': self.FIXTURE.USERNAME,
-            'passwd': self.FIXTURE.PASSWORD,
-            'database': self.FIXTURE.DBNAME}
-
-        if self.FIXTURE.DRIVER and not utils.is_backend_avail(**credentials):
-            msg = '%s backend is not available.' % self.FIXTURE.DRIVER
-            return self.skip(msg)
-
-        super(OpportunisticTestCase, self).setUp()
-
 
 class MySQLOpportunisticFixture(OpportunisticFixture):
     DRIVER = 'mysql'