Merge "Fix interference in db unit test"

4 files changed, 30 insertions, 18 deletions

diff --git a/nova/tests/fixtures/nova.py b/nova/tests/fixtures/nova.py
index 0acc3fa151..f7e272e9f2 100644
--- a/nova/tests/fixtures/nova.py
+++ b/nova/tests/fixtures/nova.py
@@ -204,6 +204,18 @@ class DatabasePoisonFixture(fixtures.Fixture):
             '_create_session',
             self._poison_configure))
 
+        # NOTE(gibi): not just _create_session indicates a manipulation on the
+        # DB but actually any operation that actually initializes (starts) a
+        # transaction factory. If a test does this without using the Database
+        # fixture then that test i) actually a database test and should declare
+        # it so ii) actually manipulates a global state without proper cleanup
+        # and test isolation. This could lead that later tests are failing with
+        # the error: oslo_db.sqlalchemy.enginefacade.AlreadyStartedError: this
+        # TransactionFactory is already started
+        self.useFixture(fixtures.MonkeyPatch(
+           'oslo_db.sqlalchemy.enginefacade._TransactionFactory._start',
+           self._poison_configure))
+
     def _poison_configure(self, *a, **k):
         # If you encounter this error, you might be tempted to just not
         # inherit from NoDBTestCase. Bug #1568414 fixed a few hundred of these
diff --git a/nova/tests/unit/db/api/test_migrations.py b/nova/tests/unit/db/api/test_migrations.py
index 10c0c8feb9..1b14d569db 100644
--- a/nova/tests/unit/db/api/test_migrations.py
+++ b/nova/tests/unit/db/api/test_migrations.py
@@ -275,11 +275,14 @@ class NovaMigrationsWalk(
                 self._migrate_up(connection, revision)
 
     def test_db_version_alembic(self):
-        migration.db_sync(database='api')
+        engine = enginefacade.writer.get_engine()
 
-        script = alembic_script.ScriptDirectory.from_config(self.config)
-        head = script.get_current_head()
-        self.assertEqual(head, migration.db_version(database='api'))
+        with mock.patch.object(migration, '_get_engine', return_value=engine):
+            migration.db_sync(database='api')
+
+            script = alembic_script.ScriptDirectory.from_config(self.config)
+            head = script.get_current_head()
+            self.assertEqual(head, migration.db_version(database='api'))
 
 
 class TestMigrationsWalkSQLite(
diff --git a/nova/tests/unit/db/main/test_migrations.py b/nova/tests/unit/db/main/test_migrations.py
index 478920c248..cb253dfc5b 100644
--- a/nova/tests/unit/db/main/test_migrations.py
+++ b/nova/tests/unit/db/main/test_migrations.py
@@ -308,11 +308,14 @@ class NovaMigrationsWalk(
                 self._migrate_up(connection, revision)
 
     def test_db_version_alembic(self):
-        migration.db_sync(database='main')
+        engine = enginefacade.writer.get_engine()
 
-        script = alembic_script.ScriptDirectory.from_config(self.config)
-        head = script.get_current_head()
-        self.assertEqual(head, migration.db_version(database='main'))
+        with mock.patch.object(migration, '_get_engine', return_value=engine):
+            migration.db_sync(database='main')
+
+            script = alembic_script.ScriptDirectory.from_config(self.config)
+            head = script.get_current_head()
+            self.assertEqual(head, migration.db_version(database='main'))
 
 
 class TestMigrationsWalkSQLite(
diff --git a/nova/tests/unit/db/test_migration.py b/nova/tests/unit/db/test_migration.py
index 7251b174b1..6657bc48e0 100644
--- a/nova/tests/unit/db/test_migration.py
+++ b/nova/tests/unit/db/test_migration.py
@@ -17,33 +17,27 @@ import os
 import urllib
 
 from alembic.runtime import migration as alembic_migration
-import fixtures
 from migrate import exceptions as migrate_exceptions
 from migrate.versioning import api as migrate_api
 import mock
-from oslo_db.sqlalchemy import enginefacade
 
 from nova.db.api import api as api_db_api
 from nova.db.main import api as main_db_api
 from nova.db import migration
 from nova import exception
 from nova import test
+from nova.tests import fixtures as nova_fixtures
 
 
 class TestDBURL(test.NoDBTestCase):
+    USES_DB_SELF = True
 
     def test_db_sync_with_special_symbols_in_connection_string(self):
         qargs = 'read_default_group=data with/a+percent_%-and%20symbols!'
         url = f"sqlite:///:memory:?{qargs}"
         self.flags(connection=url, group='database')
-        # since the engine.url is immutable it will never get updated
-        # once its created so reusing the engine instance would break
-        # this test.
-        engine = enginefacade.writer.get_engine()
-        self.useFixture(
-            fixtures.MonkeyPatch(
-                'nova.db.migration._get_engine',
-                mock.Mock(return_value=engine)))
+        self.useFixture(nova_fixtures.Database())
+
         alembic_config = migration._find_alembic_conf()
         with mock.patch.object(
                 migration, '_find_alembic_conf', return_value=alembic_config):