Filter annoying psycopg connection closed warning

This warning doesn't show up except when running pytest with the `-W error` option. It's handled and doesn't indicate a real problem.
It's quite unpredictable which test it will happen in.

Also try to use context managers as much as possible and fix a couple other test issues.

hera_mc/db_check.py

@@ -61,20 +61,20 @@ def is_valid_database(base, session):
 
     engine = session.get_bind()
     try:  # This tries thrice with 5sec sleeps in between
-        iengine = inspect(engine)
+        insp = inspect(engine)
     except OperationalError:  # pragma: no cover
         import time
 
         time.sleep(5)
         try:
-            iengine = inspect(engine)
+            insp = inspect(engine)
         except OperationalError:
             time.sleep(5)
-            iengine = inspect(engine)
+            insp = inspect(engine)
 
     errors = False
 
-    tables = iengine.get_table_names()
+    tables = insp.get_table_names()
 
     # Go through all SQLAlchemy models
 
@@ -86,15 +86,15 @@ def is_valid_database(base, session):
             #              'autoincrement': True, 'nullable': False,
             #              'type': INTEGER(), 'name': 'id'}]
 
-            columns = [c["name"] for c in iengine.get_columns(table)]
+            columns = [c["name"] for c in insp.get_columns(table)]
             mapper = inspect(klass)
 
             for column in mapper.columns:
                 # Assume normal flat column
                 if column.key not in columns:
                     logger.error(
-                        f"Model {klass} declares column {column.key} which does not "
-                        "exist in database {engine}"
+                        f"Model {klass} declares column {column.key} which does "
+                        f"not exist in database {engine}"
                     )
                     errors = True
             # TODO: Add validity checks for relations

hera_mc/tests/conftest.py

@@ -78,26 +78,21 @@ def setup_and_teardown_package():
 @pytest.fixture(scope="function")
 def mcsession(setup_and_teardown_package):
     test_db, _ = setup_and_teardown_package
-    test_conn = test_db.engine.connect()
-    test_trans = test_conn.begin()
-    test_session = mc.MCSession(bind=test_conn)
-
-    yield test_session
-
-    test_session.close()
-    # rollback - everything that happened with the
-    # Session above (including calls to commit())
-    # is rolled back.
-    with warnings.catch_warnings():
-        # If an error was raised, rollback may have already been called. If so, this
-        # will give a warning which we filter out here.
-        warnings.filterwarnings(
-            "ignore", "transaction already deassociated from connection"
-        )
-        test_trans.rollback()
-
-    # return connection to the Engine
-    test_conn.close()
+    with test_db.engine.connect() as test_conn:
+        with test_conn.begin() as test_trans:
+            with mc.MCSession(bind=test_conn) as test_session:
+                yield test_session
+
+            # rollback - everything that happened with the
+            # Session above (including calls to commit())
+            # is rolled back.
+            with warnings.catch_warnings():
+                # If an error was raised, rollback may have already been called. If so, this
+                # will give a warning which we filter out here.
+                warnings.filterwarnings(
+                    "ignore", "transaction already deassociated from connection"
+                )
+                test_trans.rollback()
 
     # delete the hookup cache file
     from .. import cm_hookup

hera_mc/tests/test_autocorrelations.py

@@ -15,6 +15,11 @@
 
 from ..tests import TEST_DEFAULT_REDIS_HOST, requires_default_redis, requires_redis
 
+# Sometimes a connection is closed, which is handled and doesn't produce an error
+# or even a warning under normal testing. But for the warnings test where we
+# pass `-W error`, the warning causes an error so we filter it out here.
+pytestmark = pytest.mark.filterwarnings("ignore:connection:ResourceWarning:psycopg")
+
 standard_query_time = Time(
     datetime.datetime(2016, 1, 5, 20, 44, 52, 741137), format="datetime"
 )

hera_mc/tests/test_cm_transfer.py

@@ -12,6 +12,11 @@
 from ..cm_partconnect import Connections
 from ..mc import AutomappedDB
 
+# Sometimes a connection is closed, which is handled and doesn't produce an error
+# or even a warning under normal testing. But for the warnings test where we
+# pass `-W error`, the warning causes an error so we filter it out here.
+pytestmark = pytest.mark.filterwarnings("ignore:connection:ResourceWarning:psycopg")
+
 
 def test_classTime():
     pytest.raises(ValueError, cm_transfer.CMVersion.create, None, None)

hera_mc/tests/test_cm_utils.py

@@ -14,6 +14,11 @@
 
 from hera_mc import cm_utils, mc
 
+# Sometimes a connection is closed, which is handled and doesn't produce an error
+# or even a warning under normal testing. But for the warnings test where we
+# pass `-W error`, the warning causes an error so we filter it out here.
+pytestmark = pytest.mark.filterwarnings("ignore:connection:ResourceWarning:psycopg")
+
 
 def test_log():
     from argparse import Namespace

hera_mc/tests/test_connections.py

@@ -10,6 +10,11 @@
 
 from .. import cm_handling, cm_partconnect
 
+# Sometimes a connection is closed, which is handled and doesn't produce an error
+# or even a warning under normal testing. But for the warnings test where we
+# pass `-W error`, the warning causes an error so we filter it out here.
+pytestmark = pytest.mark.filterwarnings("ignore:connection:ResourceWarning:psycopg")
+
 
 @pytest.fixture(scope="function")
 def conns(mcsession):

hera_mc/tests/test_correlator.py

@@ -34,6 +34,11 @@
     requires_redis,
 )
 
+# Sometimes a connection is closed, which is handled and doesn't produce an error
+# or even a warning under normal testing. But for the warnings test where we
+# pass `-W error`, the warning causes an error so we filter it out here.
+pytestmark = pytest.mark.filterwarnings("ignore:connection:ResourceWarning:psycopg")
+
 TEST_TIME1 = Time("2016-01-10 01:15:23", scale="utc")
 TEST_TIME2 = TEST_TIME1 + TimeDelta(120.0, format="sec")
 

hera_mc/tests/test_daemon_status.py

@@ -10,6 +10,11 @@
 
 from ..daemon_status import DaemonStatus
 
+# Sometimes a connection is closed, which is handled and doesn't produce an error
+# or even a warning under normal testing. But for the warnings test where we
+# pass `-W error`, the warning causes an error so we filter it out here.
+pytestmark = pytest.mark.filterwarnings("ignore:connection:ResourceWarning:psycopg")
+
 
 @pytest.fixture(scope="function")
 def daemon_data():

hera_mc/tests/test_db_check.py

@@ -13,6 +13,11 @@
 from .. import mc
 from ..db_check import check_connection, is_valid_database
 
+# Sometimes a connection is closed, which is handled and doesn't produce an error
+# or even a warning under normal testing. But for the warnings test where we
+# pass `-W error`, the warning causes an error so we filter it out here.
+pytestmark = pytest.mark.filterwarnings("ignore:connection:ResourceWarning:psycopg")
+
 
 def gen_test_model():
     Base = declarative_base()
@@ -95,6 +100,8 @@ def test_validity_pass():
         assert is_valid_database(Base, session) is True
     finally:
         Base.metadata.drop_all(engine)
+        session.close()
+        conn.close()
 
 
 def test_validity_table_missing():
@@ -113,6 +120,8 @@ def test_validity_table_missing():
         pass
 
     assert is_valid_database(Base, session) is False
+    session.close()
+    conn.close()
 
 
 def test_validity_column_missing():
@@ -138,6 +147,7 @@ def test_validity_column_missing():
         Session = sessionmaker(bind=engine)
         session = Session()
         assert is_valid_database(Base, session) is False
+        session.close()
 
 
 def test_validity_pass_relationship():
@@ -168,6 +178,8 @@ def test_validity_pass_relationship():
         assert is_valid_database(Base, session) is True
     finally:
         Base.metadata.drop_all(engine)
+        session.close()
+        conn.close()
 
 
 def test_validity_pass_declarative():
@@ -194,6 +206,8 @@ def test_validity_pass_declarative():
         assert is_valid_database(Base, session) is True
     finally:
         Base.metadata.drop_all(engine)
+        session.close()
+        conn.close()
 
 
 def test_check_connection(tmpdir):

hera_mc/tests/test_default_db_schema.py

@@ -5,21 +5,26 @@
 """
 Test that default database matches code schema.
 """
+import pytest
 from sqlalchemy.orm import sessionmaker
 
 from .. import MCDeclarativeBase, mc
 from ..db_check import is_valid_database
 
+# Sometimes a connection is closed, which is handled and doesn't produce an error
+# or even a warning under normal testing. But for the warnings test where we
+# pass `-W error`, the warning causes an error so we filter it out here.
+pytestmark = pytest.mark.filterwarnings("ignore:connection:ResourceWarning:psycopg")
+
 
 def test_default_db_schema():
     # this test will fail if the default database schema does not match the code schema
 
     default_db = mc.connect_to_mc_db(None)
     engine = default_db.engine
-    conn = engine.connect()
-    conn.begin()
-
-    Session = sessionmaker(bind=engine)
-    session = Session()
 
-    assert is_valid_database(MCDeclarativeBase, session) is True
+    with engine.connect() as conn:
+        with conn.begin():
+            Session = sessionmaker(bind=engine)
+            with Session() as session:
+                assert is_valid_database(MCDeclarativeBase, session) is True

hera_mc/tests/test_geo_location.py

@@ -8,6 +8,11 @@
 
 from .. import cm_active, cm_partconnect, geo_handling, geo_location, geo_sysdef
 
+# Sometimes a connection is closed, which is handled and doesn't produce an error
+# or even a warning under normal testing. But for the warnings test where we
+# pass `-W error`, the warning causes an error so we filter it out here.
+pytestmark = pytest.mark.filterwarnings("ignore:connection:ResourceWarning:psycopg")
+
 
 @pytest.fixture(scope="function")
 def geo_handle(mcsession):

hera_mc/tests/test_librarian.py

@@ -18,6 +18,11 @@
     LibStatus,
 )
 
+# Sometimes a connection is closed, which is handled and doesn't produce an error
+# or even a warning under normal testing. But for the warnings test where we
+# pass `-W error`, the warning causes an error so we filter it out here.
+pytestmark = pytest.mark.filterwarnings("ignore:connection:ResourceWarning:psycopg")
+
 
 @pytest.fixture(scope="module")
 def status():

hera_mc/tests/test_node.py

@@ -13,6 +13,11 @@
 from .. import node
 from ..tests import TEST_DEFAULT_REDIS_HOST, requires_redis
 
+# Sometimes a connection is closed, which is handled and doesn't produce an error
+# or even a warning under normal testing. But for the warnings test where we
+# pass `-W error`, the warning causes an error so we filter it out here.
+pytestmark = pytest.mark.filterwarnings("ignore:connection:ResourceWarning:psycopg")
+
 
 @pytest.fixture(scope="module")
 def nodelist():

hera_mc/tests/test_observations.py

@@ -13,6 +13,11 @@
 from .. import geo_handling, utils
 from ..observations import Observation, allowed_tags
 
+# Sometimes a connection is closed, which is handled and doesn't produce an error
+# or even a warning under normal testing. But for the warnings test where we
+# pass `-W error`, the warning causes an error so we filter it out here.
+pytestmark = pytest.mark.filterwarnings("ignore:connection:ResourceWarning:psycopg")
+
 
 def test_new_obs(mcsession):
     t1 = Time("2016-01-10 01:15:23", scale="utc")

hera_mc/tests/test_parts.py

@@ -19,6 +19,11 @@
     cm_utils,
 )
 
+# Sometimes a connection is closed, which is handled and doesn't produce an error
+# or even a warning under normal testing. But for the warnings test where we
+# pass `-W error`, the warning causes an error so we filter it out here.
+pytestmark = pytest.mark.filterwarnings("ignore:connection:ResourceWarning:psycopg")
+
 
 @pytest.fixture(scope="function")
 def parts(mcsession):
@@ -106,12 +111,18 @@ def test_apriori(mcsession):
     assert active.apriori["HH700:A"].status == "not_connected"
 
 
-def test_duplicate(mcsession):
+@pytest.mark.parametrize(
+    ("val", "msg"),
+    [
+        ("up", "Duplicate active port HH700:A:GROUND"),
+        ("down", "Duplicate active port NBP700:A:E1"),
+    ],
+)
+def test_duplicate_errors(mcsession, val, msg):
     active = cm_active.ActiveData(mcsession)
-    active.pytest_param = "up"
-    pytest.raises(ValueError, active.load_connections)
-    active.pytest_param = "down"
-    pytest.raises(ValueError, active.load_connections)
+    active.pytest_param = val
+    with pytest.raises(ValueError, match=msg):
+        active.load_connections()
 
 
 def test_rosetta(mcsession, capsys):
@@ -139,8 +150,11 @@ def test_rosetta(mcsession, capsys):
     )
     active.load_rosetta(at_date)
     assert int(active.rosetta["SNPC000709"].stop_gpstime) == 1280278818
-    # Add a test part to fail on update part
 
+
+def test_rosetta_update_error(mcsession):
+    # Add a test part to fail on update part
+    stop_at = Time("2020-08-01 01:00:00", scale="utc")
     rose = cm_partconnect.PartRosetta()
     rose.hpn = "SNPC000701"
     rose.syspn = "heraNode0Snap701"
@@ -149,12 +163,16 @@ def test_rosetta(mcsession, capsys):
     mcsession.commit()
     with pytest.raises(
         ValueError,
-        match="Multiple rosetta relationships active" " for heraNode0Snap701",
+        match="Multiple rosetta relationships active for heraNode0Snap701",
     ):
         cm_partconnect.update_part_rosetta(
             "SNPC000701", "heraNode0Snap701", stop_at, None, session=mcsession
         )
+
+
+def test_rosetta_load_error(mcsession):
     # Add a test part to fail on load active
+    active = cm_active.ActiveData(mcsession)
     rose = cm_partconnect.PartRosetta()
     rose.hpn = "SNPC000701"
     rose.syspn = "heraNode700Snap700"

hera_mc/tests/test_qm.py

@@ -16,6 +16,11 @@
 from hera_qm.firstcal_metrics import get_firstcal_metrics_dict  # noqa
 from hera_qm.utils import get_metrics_dict  # noqa
 
+# Sometimes a connection is closed, which is handled and doesn't produce an error
+# or even a warning under normal testing. But for the warnings test where we
+# pass `-W error`, the warning causes an error so we filter it out here.
+pytestmark = pytest.mark.filterwarnings("ignore:connection:ResourceWarning:psycopg")
+
 
 @pytest.fixture(scope="module")
 def metrics_dict():

hera_mc/tests/test_rtp.py

@@ -25,6 +25,11 @@
     RTPTaskResourceRecord,
 )
 
+# Sometimes a connection is closed, which is handled and doesn't produce an error
+# or even a warning under normal testing. But for the warnings test where we
+# pass `-W error`, the warning causes an error so we filter it out here.
+pytestmark = pytest.mark.filterwarnings("ignore:connection:ResourceWarning:psycopg")
+
 
 @pytest.fixture(scope="module")
 def observation():

hera_mc/tests/test_server_status.py

@@ -13,6 +13,11 @@
 from ..librarian import LibServerStatus
 from ..rtp import RTPServerStatus
 
+# Sometimes a connection is closed, which is handled and doesn't produce an error
+# or even a warning under normal testing. But for the warnings test where we
+# pass `-W error`, the warning causes an error so we filter it out here.
+pytestmark = pytest.mark.filterwarnings("ignore:connection:ResourceWarning:psycopg")
+
 
 @pytest.fixture(scope="module")
 def status():