Revert "Remove leftover fixture"

This reverts commit db8140d.

Revert "Revert change to how port is defined in testing"

This reverts commit f62977c.

Revert "Redo how default db credentials are obtained"

This reverts commit 667d9dc.

Revert "Ignore unused parameter"

This reverts commit efcd9a7.

Revert "Linter fix"

This reverts commit cf0653a.

Revert "Add test for db.install.install_mathesar"

This reverts commit 1518211.

conftest.py

@@ -21,20 +21,25 @@
 from fixtures.utils import create_scoped_fixtures
 
 
-# TODO BUG engine cache uses db_names to key databases: might cause problems at
-# some point: better to index using DbCredentials
 def engine_cache(request):
+    import logging
+    logger = logging.getLogger(f'engine_cache-{request.scope}')
+    logger.debug('enter')
     db_names_to_engines = {}
 
     def _get(db_name):
         engine = db_names_to_engines.get(db_name)
+        logger.debug(f'getting engine for {db_name}')
         if engine is None:
+            logger.debug(f'creating engine for {db_name}')
             engine = _create_engine(db_name)
             db_names_to_engines[db_name] = engine
         return engine
     yield _get
     for db_name, engine in db_names_to_engines.items():
+        logger.debug(f'cleaning up engine for {db_name}')
         engine.dispose()
+    logger.debug('exit')
 
 
 # defines:

db/credentials.py

@@ -7,10 +7,6 @@
 # information around as 5 separate parameters.
 #
 # Usable as a cache key when caching database-specific things (like SA engines).
-#
-# Notice how custom class methods are defined. Whether they're static or not
-# has to be inferred, though if a method's docstring doesn't mention it,
-# they're probably meant to be instance methods.
 DbCredentials = namedtuple(
     'DbCredentials',
     [
@@ -52,23 +48,3 @@ def _get_root(credentials):
 
 
 DbCredentials.get_root = _get_root
-
-
-def _from_engine(engine):
-    """
-    Creates DbCredentials from SA Engine's URL.
-
-    Static method. Probably shouldn't ever be used in production. Can be useful
-    in testing.
-    """
-    url = engine.url
-    return DbCredentials(
-        username=url.username,
-        password=url.password,
-        hostname=url.host,
-        db_name=url.database,
-        port=url.port,
-    )
-
-
-DbCredentials.from_engine = _from_engine

db/install.py

@@ -7,20 +7,14 @@
 from db.types import install as types_install
 
 
-# TODO would be nice to use this in conftest.py to setup our tests, but it's a
-# bit too complicated for that: as of writing, it might create a db, has logic
-# for interactive user input, etc.; if someone's interested in splitting this
-# up, would be nice!
 def install_mathesar(
-    credentials, skip_confirm=True
+    credentials, skip_confirm
 ):
     """Create database and install Mathesar on it."""
     db_name = credentials.db_name
     hostname = credentials.hostname
     user_db_engine = engine.create_future_engine(
         credentials,
-        # TODO explain why a custom timeout is needed; this keeps this method
-        # from being compatible with cached engines, used in tests, etc.
         connect_args={"connect_timeout": 10}
     )
     try:

db/tests/test_utils.py

@@ -1,11 +1,7 @@
-import pytest
 from sqlalchemy import MetaData, Column, String, Table
 
 from db.columns.utils import get_enriched_column_table
 from db.metadata import get_empty_metadata
-from db.install import install_mathesar
-from conftest import _drop_database
-from db.credentials import DbCredentials
 
 
 def test_get_enriched_column_table(engine):
@@ -20,25 +16,3 @@ def test_get_enriched_column_table_no_engine():
     table = Table("testtable", MetaData(), Column(abc, String), Column('def', String))
     enriched_table = get_enriched_column_table(table, metadata=get_empty_metadata())
     assert enriched_table.columns[abc].engine is None
-
-
-@pytest.fixture
-def install_fixture(FUN_engine_cache, SES_engine_cache, get_uid):
-    db_name = get_uid()
-    engine = FUN_engine_cache(db_name)
-    credentials = DbCredentials.from_engine(engine)
-    # Will create a db
-    # Note, does not use cached engines, may leak
-    install_mathesar(credentials)
-    yield
-    _drop_database(engine, SES_engine_cache)
-
-
-def test_install(install_fixture):  # noqa: F841
-    """
-    Tests that the instralling fixture (db.install.install_mathesar) doesn't
-    panic. As of time of writing, our tests use a different set of routines for
-    setting up a database (would be nice to fix that), so need this test to
-    have full(-er?) coverage.
-    """
-    pass