fix(api): refactor protocol api integration tests to prevent thread l…

…eakage (#16834)

Fixes an issue found where after a recent mergeback commit into edge, API unit tests were hanging and failing due to thread leakage in protocol api integration tests.

api/tests/opentrons/protocol_api_integration/conftest.py

@@ -0,0 +1,28 @@
+"""Fixtures for protocol api integration tests."""
+
+import pytest
+from _pytest.fixtures import SubRequest
+from typing import Generator
+
+from opentrons import simulate, protocol_api
+from opentrons.protocol_api.core.engine import ENGINE_CORE_API_VERSION
+
+
+@pytest.fixture
+def simulated_protocol_context(
+    request: SubRequest,
+) -> Generator[protocol_api.ProtocolContext, None, None]:
+    """Return a protocol context with requested version and robot."""
+    version, robot_type = request.param
+    context = simulate.get_protocol_api(version=version, robot_type=robot_type)
+    try:
+        yield context
+    finally:
+        if context.api_version >= ENGINE_CORE_API_VERSION:
+            # TODO(jbl, 2024-11-14) this is a hack of a hack to close the hardware and the PE thread when a test is
+            #  complete. At some point this should be replaced with a more holistic way of safely cleaning up these
+            #  threads so they don't leak and cause tests to fail when `get_protocol_api` is called too many times.
+            simulate._LIVE_PROTOCOL_ENGINE_CONTEXTS.close()
+        else:
+            # If this is a non-PE context we need to clean up the hardware thread manually
+            context._hw_manager.hardware.clean_up()

api/tests/opentrons/protocol_api_integration/test_liquid_classes.py

@@ -3,22 +3,30 @@
 from decoy import Decoy
 from opentrons_shared_data.robot.types import RobotTypeEnum
 
-from opentrons import simulate
+from opentrons.protocol_api import ProtocolContext
 from opentrons.config import feature_flags as ff
 
 
 @pytest.mark.ot2_only
+@pytest.mark.parametrize(
+    "simulated_protocol_context", [("2.20", "OT-2")], indirect=True
+)
 def test_liquid_class_creation_and_property_fetching(
-    decoy: Decoy, mock_feature_flags: None
+    decoy: Decoy,
+    mock_feature_flags: None,
+    simulated_protocol_context: ProtocolContext,
 ) -> None:
     """It should create the liquid class and provide access to its properties."""
     decoy.when(ff.allow_liquid_classes(RobotTypeEnum.OT2)).then_return(True)
-    protocol_context = simulate.get_protocol_api(version="2.20", robot_type="OT-2")
-    pipette_left = protocol_context.load_instrument("p20_single_gen2", mount="left")
-    pipette_right = protocol_context.load_instrument("p300_multi", mount="right")
-    tiprack = protocol_context.load_labware("opentrons_96_tiprack_20ul", "1")
+    pipette_left = simulated_protocol_context.load_instrument(
+        "p20_single_gen2", mount="left"
+    )
+    pipette_right = simulated_protocol_context.load_instrument(
+        "p300_multi", mount="right"
+    )
+    tiprack = simulated_protocol_context.load_labware("opentrons_96_tiprack_20ul", "1")
 
-    glycerol_50 = protocol_context.define_liquid_class("fixture_glycerol50")
+    glycerol_50 = simulated_protocol_context.define_liquid_class("fixture_glycerol50")
 
     assert glycerol_50.name == "fixture_glycerol50"
     assert glycerol_50.display_name == "Glycerol 50%"
@@ -50,11 +58,13 @@ def test_liquid_class_creation_and_property_fetching(
         glycerol_50.display_name = "bar"  # type: ignore
 
     with pytest.raises(ValueError, match="Liquid class definition not found"):
-        protocol_context.define_liquid_class("non-existent-liquid")
+        simulated_protocol_context.define_liquid_class("non-existent-liquid")
 
 
-def test_liquid_class_feature_flag() -> None:
+@pytest.mark.parametrize(
+    "simulated_protocol_context", [("2.20", "OT-2")], indirect=True
+)
+def test_liquid_class_feature_flag(simulated_protocol_context: ProtocolContext) -> None:
     """It should raise a not implemented error without the allowLiquidClass flag set."""
-    protocol_context = simulate.get_protocol_api(version="2.20", robot_type="OT-2")
     with pytest.raises(NotImplementedError):
-        protocol_context.define_liquid_class("fixture_glycerol50")
+        simulated_protocol_context.define_liquid_class("fixture_glycerol50")

api/tests/opentrons/protocol_api_integration/test_modules.py

@@ -3,13 +3,17 @@
 import typing
 import pytest
 
-from opentrons import simulate, protocol_api
+from opentrons import protocol_api
 
 
-def test_absorbance_reader_labware_load_conflict() -> None:
+@pytest.mark.parametrize(
+    "simulated_protocol_context", [("2.21", "Flex")], indirect=True
+)
+def test_absorbance_reader_labware_load_conflict(
+    simulated_protocol_context: protocol_api.ProtocolContext,
+) -> None:
     """It should prevent loading a labware onto a closed absorbance reader."""
-    protocol = simulate.get_protocol_api(version="2.21", robot_type="Flex")
-    module = protocol.load_module("absorbanceReaderV1", "A3")
+    module = simulated_protocol_context.load_module("absorbanceReaderV1", "A3")
 
     # The lid should be treated as initially closed.
     with pytest.raises(Exception):
@@ -19,42 +23,52 @@ def test_absorbance_reader_labware_load_conflict() -> None:
     # Should not raise after opening the lid.
     labware_1 = module.load_labware("opentrons_96_wellplate_200ul_pcr_full_skirt")
 
-    protocol.move_labware(labware_1, protocol_api.OFF_DECK)
+    simulated_protocol_context.move_labware(labware_1, protocol_api.OFF_DECK)
 
     # Should raise after closing the lid again.
     module.close_lid()  # type: ignore[union-attr]
     with pytest.raises(Exception):
         module.load_labware("opentrons_96_wellplate_200ul_pcr_full_skirt")
 
 
-def test_absorbance_reader_labware_move_conflict() -> None:
+@pytest.mark.parametrize(
+    "simulated_protocol_context", [("2.21", "Flex")], indirect=True
+)
+def test_absorbance_reader_labware_move_conflict(
+    simulated_protocol_context: protocol_api.ProtocolContext,
+) -> None:
     """It should prevent moving a labware onto a closed absorbance reader."""
-    protocol = simulate.get_protocol_api(version="2.21", robot_type="Flex")
-    module = protocol.load_module("absorbanceReaderV1", "A3")
-    labware = protocol.load_labware("opentrons_96_wellplate_200ul_pcr_full_skirt", "A1")
+    module = simulated_protocol_context.load_module("absorbanceReaderV1", "A3")
+    labware = simulated_protocol_context.load_labware(
+        "opentrons_96_wellplate_200ul_pcr_full_skirt", "A1"
+    )
 
     with pytest.raises(Exception):
         # The lid should be treated as initially closed.
-        protocol.move_labware(labware, module, use_gripper=True)
+        simulated_protocol_context.move_labware(labware, module, use_gripper=True)
 
     module.open_lid()  # type: ignore[union-attr]
     # Should not raise after opening the lid.
-    protocol.move_labware(labware, module, use_gripper=True)
+    simulated_protocol_context.move_labware(labware, module, use_gripper=True)
 
-    protocol.move_labware(labware, "A1", use_gripper=True)
+    simulated_protocol_context.move_labware(labware, "A1", use_gripper=True)
 
     # Should raise after closing the lid again.
     module.close_lid()  # type: ignore[union-attr]
     with pytest.raises(Exception):
-        protocol.move_labware(labware, module, use_gripper=True)
+        simulated_protocol_context.move_labware(labware, module, use_gripper=True)
 
 
-def test_absorbance_reader_read_preconditions() -> None:
+@pytest.mark.parametrize(
+    "simulated_protocol_context", [("2.21", "Flex")], indirect=True
+)
+def test_absorbance_reader_read_preconditions(
+    simulated_protocol_context: protocol_api.ProtocolContext,
+) -> None:
     """Test the preconditions for triggering an absorbance reader read."""
-    protocol = simulate.get_protocol_api(version="2.21", robot_type="Flex")
     module = typing.cast(
         protocol_api.AbsorbanceReaderContext,
-        protocol.load_module("absorbanceReaderV1", "A3"),
+        simulated_protocol_context.load_module("absorbanceReaderV1", "A3"),
     )
 
     with pytest.raises(Exception, match="initialize"):