From b57ec18991fe921ae9fdfc1286b747db1ff4037f Mon Sep 17 00:00:00 2001
From: John Davis <jdavcs@gmail.com>
Date: Tue, 24 Sep 2024 20:43:47 -0400
Subject: [PATCH] Refactor migration testing setup code

- Do not explicitly import fixtures (a little duplication is safer/cleaner)
- Move dangerous fixture into module to prevent accidental usage from other modules
- Ensure only one database is used for all tests in module
---
 .../data/model/migration_fixes/conftest.py    | 44 ++++++++-----------
 .../model/migration_fixes/test_migrations.py  | 31 ++++++++++---
 2 files changed, 45 insertions(+), 30 deletions(-)

diff --git a/test/unit/data/model/migration_fixes/conftest.py b/test/unit/data/model/migration_fixes/conftest.py
index 39ba30f5462a..21edbb3a49fc 100644
--- a/test/unit/data/model/migration_fixes/conftest.py
+++ b/test/unit/data/model/migration_fixes/conftest.py
@@ -1,26 +1,32 @@
-from typing import (
-    Generator,
-    TYPE_CHECKING,
-)
+import tempfile
+from typing import TYPE_CHECKING
 
 import pytest
-from sqlalchemy import (
-    create_engine,
-    text,
-)
+from sqlalchemy import create_engine
 from sqlalchemy.orm import Session
 
-from galaxy import model as m
-
 if TYPE_CHECKING:
     from sqlalchemy.engine import Engine
 
-from galaxy.model.unittest_utils.model_testing_utils import (  # noqa: F401 - url_factory is a fixture we have to import explicitly
-    sqlite_url_factory,
+from galaxy.model.unittest_utils.model_testing_utils import (
+    _generate_unique_database_name,
+    _make_sqlite_db_url,
 )
 
 
-@pytest.fixture()
+@pytest.fixture(scope="module")
+def sqlite_url_factory():
+    """Return a function that generates a sqlite url"""
+
+    def url():
+        database = _generate_unique_database_name()
+        return _make_sqlite_db_url(tmp_dir, database)
+
+    with tempfile.TemporaryDirectory() as tmp_dir:
+        yield url
+
+
+@pytest.fixture(scope="module")
 def db_url(sqlite_url_factory):  # noqa: F811
     return sqlite_url_factory()
 
@@ -33,15 +39,3 @@ def engine(db_url: str) -> "Engine":
 @pytest.fixture
 def session(engine: "Engine") -> Session:
     return Session(engine)
-
-
-@pytest.fixture(autouse=True)
-def clear_database(engine: "Engine") -> "Generator":
-    """Delete all rows from all tables. Called after each test."""
-    yield
-    with engine.begin() as conn:
-        for table in m.mapper_registry.metadata.tables:
-            # Unless db is sqlite, disable foreign key constraints to delete out of order
-            if engine.name != "sqlite":
-                conn.execute(text(f"ALTER TABLE {table} DISABLE TRIGGER ALL"))
-            conn.execute(text(f"DELETE FROM {table}"))
diff --git a/test/unit/data/model/migration_fixes/test_migrations.py b/test/unit/data/model/migration_fixes/test_migrations.py
index bfbc4bc0a9dd..0c6c8979d8cc 100644
--- a/test/unit/data/model/migration_fixes/test_migrations.py
+++ b/test/unit/data/model/migration_fixes/test_migrations.py
@@ -1,20 +1,41 @@
+from typing import (
+    Generator,
+    TYPE_CHECKING,
+)
+
 import pytest
-from sqlalchemy import select
+from sqlalchemy import (
+    select,
+    text,
+)
 
+from galaxy import model as m
 from galaxy.model import (
     GroupRoleAssociation,
     User,
     UserGroupAssociation,
     UserRoleAssociation,
 )
-from galaxy.model.unittest_utils.migration_scripts_testing_utils import (  # noqa: F401 - contains fixtures we have to import explicitly
-    run_command,
-    tmp_directory,
-)
+from galaxy.model.unittest_utils.migration_scripts_testing_utils import run_command
+
+if TYPE_CHECKING:
+    from sqlalchemy.engine import Engine
 
 COMMAND = "manage_db.sh"
 
 
+@pytest.fixture(autouse=True)
+def clear_database(engine: "Engine") -> "Generator":
+    """Delete all rows from all tables. Called after each test."""
+    yield
+    with engine.begin() as conn:
+        for table in m.mapper_registry.metadata.tables:
+            # Unless db is sqlite, disable foreign key constraints to delete out of order
+            if engine.name != "sqlite":
+                conn.execute(text(f"ALTER TABLE {table} DISABLE TRIGGER ALL"))
+            conn.execute(text(f"DELETE FROM {table}"))
+
+
 @pytest.fixture(autouse=True)
 def upgrade_database_after_test():
     """Run after each test for proper cleanup"""