From 5d6a2bc2aec5cb1044a1964626b1f8ae920d5fab Mon Sep 17 00:00:00 2001 From: aaron-kulkarni Date: Wed, 31 Jul 2024 14:58:10 -0400 Subject: [PATCH] execute_json fixes. 275 remaining --- .../protocols/execution/execute_json_v4.py | 25 ++- .../integration/test_thermocycler.py | 12 +- .../execution/test_execute_json_v3.py | 143 ++++++++---- .../execution/test_execute_json_v4.py | 211 ++++++++++++------ .../execution/test_execute_json_v5.py | 22 +- 5 files changed, 275 insertions(+), 138 deletions(-) diff --git a/api/src/opentrons/protocols/execution/execute_json_v4.py b/api/src/opentrons/protocols/execution/execute_json_v4.py index 6060e82cb47..f3adfc6ab08 100644 --- a/api/src/opentrons/protocols/execution/execute_json_v4.py +++ b/api/src/opentrons/protocols/execution/execute_json_v4.py @@ -8,6 +8,11 @@ ModuleContext, ThermocyclerContext, ) +from opentrons_shared_data.protocol.types import ( + MagneticModuleCommand, + TemperatureModuleCommand, + ThermocyclerCommand, +) from .execute_json_v3 import _delay, _move_to_slot from opentrons.protocols.execution.types import LoadedLabware, Instruments from opentrons_shared_data.protocol.constants import ( @@ -277,21 +282,21 @@ def dispatch_json( params, # type: ignore modules, command_type, # type: ignore - magnetic_module_command_map, + magnetic_module_command_map, # type: ignore[arg-type] ) elif command_type in temperature_module_command_map: handleTemperatureCommand( - params, # type: ignore + params, # type: ignore[arg-type] modules, command_type, # type: ignore - temperature_module_command_map, + temperature_module_command_map, # type: ignore[arg-type] ) elif command_type in thermocycler_module_command_map: handleThermocyclerCommand( params, # type: ignore modules, # type: ignore command_type, # type: ignore - thermocycler_module_command_map, + thermocycler_module_command_map, # type: ignore[arg-type] ) elif command_item["command"] == JsonRobotCommand.delay.value: _delay(context, params) # type: ignore @@ -305,12 +310,12 @@ def handleTemperatureCommand( params: Union["TemperatureParams", "ModuleIDParams"], modules: Dict[str, ModuleContext], command_type: "TemperatureModuleCommandId", - temperature_module_command_map, + temperature_module_command_map: TemperatureModuleCommand, ) -> None: module_id = params["module"] module = modules[module_id] if isinstance(module, TemperatureModuleContext): - temperature_module_command_map[command_type](module, params) + temperature_module_command_map[command_type](module, params) # type: ignore[typeddict-item] else: raise RuntimeError( "Temperature Module does not match " + "TemperatureModuleContext interface" @@ -326,12 +331,12 @@ def handleThermocyclerCommand( ], modules: Dict[str, ThermocyclerContext], command_type: "ThermocyclerCommandId", - thermocycler_module_command_map, + thermocycler_module_command_map: ThermocyclerCommand, ) -> None: module_id = params["module"] module = modules[module_id] if isinstance(module, ThermocyclerContext): - thermocycler_module_command_map[command_type](module, params) + thermocycler_module_command_map[command_type](module, params) # type: ignore[typeddict-item] else: raise RuntimeError( "Thermocycler Module does not match ThermocyclerContext interface" @@ -342,12 +347,12 @@ def handleMagnetCommand( params: Union["ModuleIDParams", "MagneticModuleEngageParams"], modules: Dict[str, ModuleContext], command_type: "MagneticModuleCommandId", - magnetic_module_command_map, + magnetic_module_command_map: MagneticModuleCommand, ) -> None: module_id = params["module"] module = modules[module_id] if isinstance(module, MagneticModuleContext): - magnetic_module_command_map[command_type](module, params) + magnetic_module_command_map[command_type](module, params) # type: ignore[typeddict-item] else: raise RuntimeError( "Magnetic Module does not match MagneticModuleContext interface" diff --git a/api/tests/opentrons/hardware_control/integration/test_thermocycler.py b/api/tests/opentrons/hardware_control/integration/test_thermocycler.py index 1b7292d4275..4a8c3e8b5c8 100644 --- a/api/tests/opentrons/hardware_control/integration/test_thermocycler.py +++ b/api/tests/opentrons/hardware_control/integration/test_thermocycler.py @@ -1,6 +1,6 @@ import asyncio import sys -from typing import AsyncGenerator +from typing import AsyncGenerator, List import anyio import pytest @@ -8,7 +8,7 @@ from opentrons.hardware_control import ExecutionManager from opentrons.hardware_control.emulation.settings import Settings from opentrons.hardware_control.modules import Thermocycler -from opentrons.hardware_control.modules.types import TemperatureStatus +from opentrons.hardware_control.modules.types import TemperatureStatus, ThermocyclerStep from .build_module import build_module @@ -95,7 +95,7 @@ async def test_cycle_temperatures(thermocycler: Thermocycler) -> None: assert thermocycler.total_cycle_count is None assert thermocycler.total_step_count is None - steps = [ + steps: list[ThermocyclerStep] = [ { "temperature": 70.0, }, @@ -164,7 +164,7 @@ async def test_cycle_cannot_be_interrupted_by_pause( # This list must be long enough that we can reliably target a pause somewhere # in the middle of it, but not so long that makes the test take a disruptively # long time. - steps = [ + steps: list[ThermocyclerStep] = [ {"temperature": temp_1, "hold_time_seconds": poll_interval_seconds * 2}, {"temperature": temp_2, "hold_time_seconds": poll_interval_seconds * 2}, {"temperature": temp_1, "hold_time_seconds": poll_interval_seconds * 2}, @@ -227,7 +227,7 @@ async def test_cycle_can_be_blocked_by_preexisting_pause( # as an approximation of asserting that it blocks forever. with pytest.raises(TimeoutError): with anyio.fail_after(0.5): - steps = [{"temperature": temp_2}] + steps: List[ThermocyclerStep] = [{"temperature": temp_2}] await thermocycler.cycle_temperatures(steps=steps, repetitions=1) # Assert that the cycle didn't have any effect. @@ -240,7 +240,7 @@ async def test_cycle_can_be_cancelled( execution_manager: ExecutionManager, ) -> None: """A cycle should be cancellable (even though it isn't pausable).""" - steps = [ + steps: List[ThermocyclerStep] = [ {"temperature": 20.0, "hold_time_minutes": 99999}, ] diff --git a/api/tests/opentrons/protocols/execution/test_execute_json_v3.py b/api/tests/opentrons/protocols/execution/test_execute_json_v3.py index 6043fca0daf..bb875cb999f 100644 --- a/api/tests/opentrons/protocols/execution/test_execute_json_v3.py +++ b/api/tests/opentrons/protocols/execution/test_execute_json_v3.py @@ -1,11 +1,20 @@ -from opentrons.protocol_api.core.legacy.legacy_labware_core import LegacyLabwareCore -from opentrons.protocol_api.core.legacy.legacy_well_core import LegacyWellCore -from opentrons.protocol_api.core.legacy.well_geometry import WellGeometry - from unittest import mock from copy import deepcopy +from typing import Any, Callable, Dict, List, Tuple +import typing +from opentrons.protocol_api.core.well import AbstractWellCore from opentrons_shared_data.labware.types import LabwareDefinition -from opentrons_shared_data.protocol.types import DelayParams +from opentrons_shared_data.protocol.types import ( + BlowoutParams, + DelayParams, + FlowRateParams, + JsonProtocolV3, + MoveToSlotParams, + PipetteAccessParams, + PipetteAccessWithOffsetParams, + StandardLiquidHandlingParams, + TouchTipParams, +) import pytest from opentrons.types import Location, Point, MountType from opentrons.protocols.parse import parse @@ -34,8 +43,10 @@ _get_location_with_offset, load_pipettes_from_json, ) - -from typing import Any, Callable, Tuple +from opentrons.protocol_api.core.core_map import LoadedCoreMap +from opentrons.protocol_api.core.legacy.legacy_labware_core import LegacyLabwareCore +from opentrons.protocol_api.core.legacy.legacy_well_core import LegacyWellCore +from opentrons.protocol_api.core.legacy.well_geometry import WellGeometry def test_load_pipettes_from_json() -> None: @@ -44,27 +55,34 @@ def fake_pipette(name: str, mount: MountType) -> Tuple[str, MountType]: ctx = mock.create_autospec(ProtocolContext) ctx.load_instrument = fake_pipette - protocol = { + protocol: "JsonProtocolV3" = { "pipettes": { - "aID": {"mount": "left", "name": "a"}, - "bID": {"mount": "right", "name": "b"}, + "aID": {"mount": "left", "name": "p10_single"}, + "bID": {"mount": "right", "name": "p50_single"}, } } result = load_pipettes_from_json(ctx, protocol) - assert result == {"aID": ("a", "left"), "bID": ("b", "right")} + assert result == {"aID": ("p10_single", "left"), "bID": ("p50_single", "right")} # type: ignore[comparison-overlap] def test_get_well(minimal_labware_def2: LabwareDefinition) -> None: deck = Location(Point(0, 0, 0), "deck") + mock_core = mock.create_autospec(AbstractWellCore) + mock_map = mock.create_autospec(LoadedCoreMap) + mock_pipette = mock.create_autospec(InstrumentContext) well_name = "A2" some_labware = labware.Labware( core=LegacyLabwareCore(minimal_labware_def2, deck), api_version=MAX_SUPPORTED_VERSION, - protocol_core=None, - core_map=None, + protocol_core=mock_core, + core_map=mock_map, ) loaded_labware = {"someLabwareId": some_labware} - params = {"labware": "someLabwareId", "well": well_name} + params: PipetteAccessParams = { + "labware": "someLabwareId", + "well": well_name, + "pipette": mock_pipette, + } result = _get_well(loaded_labware, params) assert result == some_labware[well_name] @@ -74,7 +92,7 @@ def test_set_flow_rate() -> None: pipette.flow_rate.aspirate = 1 pipette.flow_rate.dispense = 2 pipette.flow_rate.blow_out = 3 - params = {"flowRate": 42} + params: FlowRateParams = {"flowRate": 42} _set_flow_rate(pipette, params) assert pipette.flow_rate.aspirate == 42 @@ -86,7 +104,7 @@ def test_load_labware_from_json_defs( ctx: ProtocolContext, get_labware_fixture: Callable[[str], Any] ) -> None: custom_trough_def = get_labware_fixture("fixture_12_trough") - data = { + data: "JsonProtocolV3" = { "labwareDefinitions": {"someTroughDef": custom_trough_def}, "labware": { "sourcePlateId": { @@ -94,7 +112,7 @@ def test_load_labware_from_json_defs( "definitionId": "someTroughDef", "displayName": "Source (Buffer)", }, - "destPlateId": {"slot": "11", "definitionId": "someTroughDef"}, + "destPlateId": {"slot": "11", "definitionId": "someTroughDef"}, # type: ignore[typeddict-item] }, } loaded_labware = load_labware_from_json_defs(ctx, data) @@ -112,7 +130,13 @@ def test_load_labware_from_json_defs( def test_get_location_with_offset(min_lw2: labware.Labware) -> None: loaded_labware = {"someLabwareId": min_lw2} - params = {"offsetFromBottomMm": 3, "labware": "someLabwareId", "well": "A2"} + mock_pipette = mock.create_autospec(InstrumentContext) + params: PipetteAccessWithOffsetParams = { + "offsetFromBottomMm": 3, + "labware": "someLabwareId", + "well": "A2", + "pipette": mock_pipette, + } result = _get_location_with_offset(loaded_labware, params) assert result == Location(Point(19, 28, 8), min_lw2["A2"]) @@ -121,17 +145,25 @@ def test_get_location_with_offset_fixed_trash( minimal_labware_def2: LabwareDefinition, ) -> None: deck = Location(Point(0, 0, 0), "deck") + mock_core = mock.create_autospec(AbstractWellCore) + mock_map = mock.create_autospec(LoadedCoreMap) + mock_pipette = mock.create_autospec(InstrumentContext) trash_labware_def = deepcopy(minimal_labware_def2) trash_labware_def["parameters"]["quirks"] = ["fixedTrash"] trash_labware = labware.Labware( core=LegacyLabwareCore(trash_labware_def, deck), api_version=MAX_SUPPORTED_VERSION, - protocol_core=None, - core_map=None, + protocol_core=mock_core, + core_map=mock_map, ) loaded_labware = {"someLabwareId": trash_labware} - params = {"offsetFromBottomMm": 3, "labware": "someLabwareId", "well": "A1"} + params: PipetteAccessWithOffsetParams = { + "offsetFromBottomMm": 3, + "labware": "someLabwareId", + "well": "A1", + "pipette": mock_pipette, + } result = _get_location_with_offset(loaded_labware, params) @@ -145,7 +177,7 @@ def test_get_location_with_offset_fixed_trash( ({"wait": 123, "message": "m"}, [mock.call.delay(seconds=123, msg="m")]), ], ) -def test_delay(params: DelayParams, expected) -> None: +def test_delay(params: DelayParams, expected: List[Any]) -> None: mock_context = mock.MagicMock() _delay(mock_context, params) @@ -156,15 +188,17 @@ def test_blowout() -> None: m = mock.MagicMock() m.pipette_mock = mock.create_autospec(InstrumentContext) m.mock_set_flow_rate = mock.MagicMock() + mock_labware = mock.create_autospec(labware.Labware) - params = { + params: BlowoutParams = { "pipette": "somePipetteId", "labware": "someLabwareId", "well": "someWell", + "flowRate": 0, + "offsetFromBottomMm": 0, } instruments = {"somePipetteId": m.pipette_mock} - well = "theWell" - loaded_labware = {"someLabwareId": {"someWell": well}} + loaded_labware: Dict[str, labware.Labware] = {"someLabwareId": mock_labware} flow_rate_patch = "opentrons.protocols.execution.execute_json_v3._set_flow_rate" with mock.patch(flow_rate_patch, new=m.mock_set_flow_rate): @@ -172,64 +206,69 @@ def test_blowout() -> None: assert m.mock_calls == [ mock.call.mock_set_flow_rate(m.pipette_mock, params), - mock.call.pipette_mock.blow_out(well), + mock.call.pipette_mock.blow_out(mock_labware.__getitem__()), ] def test_pick_up_tip() -> None: pipette_mock = mock.create_autospec(InstrumentContext) - params = { + mock_labware = mock.create_autospec(labware.Labware) + params: PipetteAccessParams = { "pipette": "somePipetteId", "labware": "someLabwareId", "well": "someWell", } instruments = {"somePipetteId": pipette_mock} - well = "theWell" - loaded_labware = {"someLabwareId": {"someWell": well}} + loaded_labware: Dict[str, labware.Labware] = {"someLabwareId": mock_labware} _pick_up_tip(instruments, loaded_labware, params) - assert pipette_mock.mock_calls == [mock.call.pick_up_tip(well)] + assert pipette_mock.mock_calls == [ + mock.call.pick_up_tip(mock_labware.__getitem__()) + ] def test_drop_tip() -> None: pipette_mock = mock.create_autospec(InstrumentContext) + mock_labware = mock.create_autospec(labware.Labware) - params = { + params: PipetteAccessParams = { "pipette": "somePipetteId", "labware": "someLabwareId", "well": "someWell", } instruments = {"somePipetteId": pipette_mock} - well = "theWell" - loaded_labware = {"someLabwareId": {"someWell": well}} + loaded_labware = {"someLabwareId": mock_labware} _drop_tip(instruments, loaded_labware, params) - assert pipette_mock.mock_calls == [mock.call.drop_tip(well)] + assert pipette_mock.mock_calls == [mock.call.drop_tip(mock_labware.__getitem__())] def test_air_gap(minimal_labware_def2: LabwareDefinition) -> None: m = mock.MagicMock() m.pipette_mock = mock.create_autospec(InstrumentContext) m.mock_set_flow_rate = mock.MagicMock() + mock_core = mock.create_autospec(LegacyLabwareCore) + mock_map = mock.create_autospec(LoadedCoreMap) deck = Location(Point(0, 0, 0), "deck") well_name = "A2" some_labware = labware.Labware( core=LegacyLabwareCore(minimal_labware_def2, deck), api_version=MAX_SUPPORTED_VERSION, - protocol_core=None, - core_map=None, + protocol_core=mock_core, + core_map=mock_map, ) loaded_labware = {"someLabwareId": some_labware} - params = {"labware": "someLabwareId", "well": well_name} + # params = {"labware": "someLabwareId", "well": well_name} - params = { + params: StandardLiquidHandlingParams = { "pipette": "somePipetteId", "volume": 42, "labware": "someLabwareId", "well": well_name, "offsetFromBottomMm": 12, + "flowRate": 0, } instruments = {"somePipetteId": m.pipette_mock} @@ -261,11 +300,13 @@ def test_aspirate() -> None: ) m.mock_set_flow_rate = mock.MagicMock() - params = { + params: StandardLiquidHandlingParams = { "pipette": "somePipetteId", "volume": 42, "labware": "someLabwareId", "well": "someWell", + "flowRate": 0, + "offsetFromBottomMm": 0, } instruments = {"somePipetteId": m.pipette_mock} @@ -294,11 +335,13 @@ def test_dispense() -> None: ) m.mock_set_flow_rate = mock.MagicMock() - params = { + params: StandardLiquidHandlingParams = { "pipette": "somePipetteId", "volume": 42, "labware": "someLabwareId", "well": "someWell", + "flowRate": 0, + "offsetFromBottomMm": 0, } instruments = {"somePipetteId": m.pipette_mock} @@ -321,8 +364,10 @@ def test_dispense() -> None: def test_touch_tip() -> None: location = Location(Point(1, 2, 3), "deck") + mock_parent = mock.create_autospec(labware.Labware) + mock_core = mock.create_autospec(LegacyLabwareCore) well = labware.Well( - parent=None, + parent=mock_parent, core=LegacyWellCore( well_geometry=WellGeometry( { @@ -335,7 +380,7 @@ def test_touch_tip() -> None: "z": 3, }, parent_point=Point(10, 20, 30), - parent_object=1, + parent_object=mock_core, ), display_name="some well", has_tip=False, @@ -351,10 +396,11 @@ def test_touch_tip() -> None: mock_get_well = mock.MagicMock(return_value=well, name="mock_get_well") mock_set_flow_rate = mock.MagicMock(name="mock_set_flow_rate") - params = { + params: TouchTipParams = { "pipette": "somePipetteId", "labware": "someLabwareId", "well": "someWell", + "offsetFromBottomMm": 0, } instruments = {"somePipetteId": pipette_mock} @@ -393,7 +439,7 @@ def test_move_to_slot() -> None: instruments = {"somePipetteId": pipette_mock} - params = { + params: MoveToSlotParams = { "pipette": "somePipetteId", "slot": "4", "offset": {"x": 10, "y": 11, "z": 12}, @@ -444,7 +490,7 @@ def test_dispatch_json() -> None: context = mock.sentinel.context instruments = mock.sentinel.instruments loaded_labware = mock.sentinel.loaded_labware - dispatch_json(context, protocol_data, instruments, loaded_labware) + dispatch_json(context, protocol_data, instruments, loaded_labware) # type: ignore[arg-type] assert m.mock_calls == [ mock.call._delay(context, "delay_params"), @@ -458,6 +504,7 @@ def test_dispatch_json() -> None: ] +@typing.no_type_check def test_dispatch_json_invalid_command() -> None: protocol_data = { "commands": [ @@ -474,7 +521,11 @@ def test_dispatch_json_invalid_command() -> None: @pytest.mark.ot2_only -def test_papi_execute_json_v3(monkeypatch, ctx, get_json_protocol_fixture) -> None: +def test_papi_execute_json_v3( + monkeypatch: pytest.MonkeyPatch, + ctx: ProtocolContext, + get_json_protocol_fixture: Callable[[str, str, bool], str], +) -> None: protocol_data = get_json_protocol_fixture("3", "testAllAtomicSingleV3", False) protocol = parse(protocol_data, None) ctx.home() diff --git a/api/tests/opentrons/protocols/execution/test_execute_json_v4.py b/api/tests/opentrons/protocols/execution/test_execute_json_v4.py index 39d0d4647ab..c2af08fc7e3 100644 --- a/api/tests/opentrons/protocols/execution/test_execute_json_v4.py +++ b/api/tests/opentrons/protocols/execution/test_execute_json_v4.py @@ -1,6 +1,30 @@ -from unittest import mock +from typing import Callable, Generator, List, Optional import pytest +import typing +from unittest import mock +from opentrons.protocols.execution.dev_types import ( + JsonV4MagneticModuleDispatch, + JsonV4TemperatureModuleDispatch, + JsonV4ThermocyclerDispatch, + PipetteDispatch, +) +from opentrons_shared_data.protocol.types import ( + JsonProtocolV4, + MagneticModuleEngageParams, + ModuleIDParams, + TemperatureParams, + ThermocyclerAwaitBlockTemperatureCommand, + ThermocyclerAwaitLidTemperatureCommand, + ThermocyclerAwaitProfileCommand, + ThermocyclerCommand, + ThermocyclerRunProfileCommand, + ThermocyclerRunProfileParams, + ThermocyclerSetTargetBlockCommand, + ThermocyclerSetTargetBlockParams, + ThermocyclerSetTargetLidCommand, +) from opentrons.protocols.parse import parse + from opentrons.protocols.execution.execute_json_v4 import ( dispatch_json, _engage_magnet, @@ -38,15 +62,15 @@ # autouse set to True to setup/teardown mock after each run @pytest.fixture(autouse=True) -def mockObj(): +def mockObj() -> Generator[mock.Mock, None, None]: m = mock.Mock() yield m m.reset() @pytest.fixture -def pipette_command_map(mockObj): - mock_pipette_command_map = { +def pipette_command_map(mockObj: mock.Mock) -> "PipetteDispatch": + mock_pipette_command_map: "PipetteDispatch" = { JPC.blowout.value: mockObj._blowout, JPC.pickUpTip.value: mockObj._pick_up_tip, JPC.dropTip.value: mockObj._drop_tip, @@ -58,8 +82,8 @@ def pipette_command_map(mockObj): @pytest.fixture -def magnetic_module_command_map(mockObj): - mock_magnetic_module_command_map = { +def magnetic_module_command_map(mockObj: mock.Mock) -> "JsonV4MagneticModuleDispatch": + mock_magnetic_module_command_map: "JsonV4MagneticModuleDispatch" = { JMMC.magneticModuleEngageMagnet.value: mockObj._engage_magnet, JMMC.magneticModuleDisengageMagnet.value: mockObj._disengage_magnet, } @@ -68,8 +92,10 @@ def magnetic_module_command_map(mockObj): @pytest.fixture -def temperature_module_command_map(mockObj): - mock_temperature_module_command_map = { +def temperature_module_command_map( + mockObj: mock.Mock, +) -> "JsonV4TemperatureModuleDispatch": + mock_temperature_module_command_map: "JsonV4TemperatureModuleDispatch" = { JTMC.temperatureModuleSetTargetTemperature.value: mockObj._temperature_module_set_temp, JTMC.temperatureModuleDeactivate.value: mockObj._temperature_module_deactivate, JTMC.temperatureModuleAwaitTemperature.value: mockObj._temperature_module_await_temp, @@ -78,8 +104,8 @@ def temperature_module_command_map(mockObj): @pytest.fixture -def thermocycler_module_command_map(mockObj): - mock_thermocycler_module_command_map = { +def thermocycler_module_command_map(mockObj: mock.Mock) -> "JsonV4ThermocyclerDispatch": + mock_thermocycler_module_command_map: "JsonV4ThermocyclerDispatch" = { JTHC.thermocyclerCloseLid.value: mockObj._thermocycler_close_lid, JTHC.thermocyclerOpenLid.value: mockObj._thermocycler_open_lid, JTHC.thermocyclerDeactivateBlock.value: mockObj._thermocycler_deactivate_block, @@ -97,13 +123,13 @@ def thermocycler_module_command_map(mockObj): return mock_thermocycler_module_command_map -def test_load_modules_from_json(): - def fake_module(model, slot=None): +def test_load_modules_from_json() -> None: + def fake_module(model: str, slot: Optional[str] = None) -> str: return "mock_module_ctx_" + model ctx = mock.create_autospec(ProtocolContext) ctx.load_module.side_effect = fake_module - protocol = { + protocol: "JsonProtocolV4" = { "modules": { "aID": {"slot": "1", "model": "magneticModuleV1"}, "bID": {"slot": "4", "model": "temperatureModuleV2"}, @@ -121,16 +147,16 @@ def fake_module(model, slot=None): # resulting dict should have ModuleContext objects (from calling # load_module) as its values - assert result == { + assert result == { # type: ignore[comparison-overlap] "aID": "mock_module_ctx_magneticModuleV1", "bID": "mock_module_ctx_temperatureModuleV2", "cID": "mock_module_ctx_thermocyclerModuleV1", } -def test_engage_magnet(): +def test_engage_magnet() -> None: module_mock = mock.create_autospec(MagneticModuleContext) - params = { + params: "MagneticModuleEngageParams" = { "module": "someModuleId", "engageHeight": 4.2, } @@ -139,83 +165,86 @@ def test_engage_magnet(): assert module_mock.mock_calls == [mock.call.engage(height_from_base=4.2)] -def test_disengage_magnet(): +def test_disengage_magnet() -> None: module_mock = mock.create_autospec(MagneticModuleContext) - params = {"module": "someModuleId"} + params: "ModuleIDParams" = {"module": "someModuleId"} _disengage_magnet(module_mock, params) assert module_mock.mock_calls == [mock.call.disengage()] -def test_temperature_module_set_temp(): +def test_temperature_module_set_temp() -> None: module_mock = mock.create_autospec(TemperatureModuleContext) - params = {"module": "someModuleId", "temperature": 42.5} + params: "TemperatureParams" = {"module": "someModuleId", "temperature": 42.5} _temperature_module_set_temp(module_mock, params) assert module_mock.mock_calls == [mock.call.start_set_temperature(42.5)] -def test_temperature_module_deactivate(): +def test_temperature_module_deactivate() -> None: module_mock = mock.create_autospec(TemperatureModuleContext) - params = {"module": "someModuleId"} + params: "ModuleIDParams" = {"module": "someModuleId"} _temperature_module_deactivate(module_mock, params) assert module_mock.mock_calls == [mock.call.deactivate()] -def test_temperature_module_await_temp(): +def test_temperature_module_await_temp() -> None: module_mock = mock.create_autospec(TemperatureModuleContext) - params = {"module": "someModuleId", "temperature": 12.3} + params: "TemperatureParams" = {"module": "someModuleId", "temperature": 12.3} _temperature_module_await_temp(module_mock, params) assert module_mock.mock_calls == [mock.call.await_temperature(12.3)] -def test_thermocycler_close_lid(): +def test_thermocycler_close_lid() -> None: module_mock = mock.create_autospec(ThermocyclerContext) - params = {"module": "someModuleId"} + params: "ModuleIDParams" = {"module": "someModuleId"} _thermocycler_close_lid(module_mock, params) assert module_mock.mock_calls == [mock.call.close_lid()] -def test_thermocycler_open_lid(): +def test_thermocycler_open_lid() -> None: module_mock = mock.create_autospec(ThermocyclerContext) - params = {"module": "someModuleId"} + params: "ModuleIDParams" = {"module": "someModuleId"} _thermocycler_open_lid(module_mock, params) assert module_mock.mock_calls == [mock.call.open_lid()] -def test_thermocycler_deactivate_block(): +def test_thermocycler_deactivate_block() -> None: module_mock = mock.create_autospec(ThermocyclerContext) - params = {"module": "someModuleId"} + params: "ModuleIDParams" = {"module": "someModuleId"} _thermocycler_deactivate_block(module_mock, params) assert module_mock.mock_calls == [mock.call.deactivate_block()] -def test_thermocycler_deactivate_lid(): +def test_thermocycler_deactivate_lid() -> None: module_mock = mock.create_autospec(ThermocyclerContext) - params = {"module": "someModuleId"} + params: "ModuleIDParams" = {"module": "someModuleId"} _thermocycler_deactivate_lid(module_mock, params) assert module_mock.mock_calls == [mock.call.deactivate_lid()] -def test_thermocycler_set_block_temperature(): +def test_thermocycler_set_block_temperature() -> None: module_mock = mock.create_autospec(ThermocyclerContext) - params = {"temperature": 42, "module": "someModuleId"} + params: "ThermocyclerSetTargetBlockParams" = { + "temperature": 42, + "module": "someModuleId", + } _thermocycler_set_block_temperature(module_mock, params) assert module_mock.mock_calls == [mock.call.set_block_temperature(42)] -def test_thermocycler_set_lid_temperature(): +def test_thermocycler_set_lid_temperature() -> None: module_mock = mock.create_autospec(ThermocyclerContext) - params = {"module": "someModuleId", "temperature": 42} + params: "TemperatureParams" = {"module": "someModuleId", "temperature": 42} _thermocycler_set_lid_temperature(module_mock, params) assert module_mock.mock_calls == [mock.call.set_lid_temperature(42)] -def test_thermocycler_run_profile(): +def test_thermocycler_run_profile() -> None: module_mock = mock.create_autospec(ThermocyclerContext) - params = { + params: "ThermocyclerRunProfileParams" = { "profile": [ {"temperature": 55, "holdTime": 90}, {"temperature": 65, "holdTime": 30}, @@ -235,13 +264,13 @@ def test_thermocycler_run_profile(): def test_dispatch_json( - monkeypatch, - pipette_command_map, - magnetic_module_command_map, - temperature_module_command_map, - thermocycler_module_command_map, - mockObj, -): + monkeypatch: pytest.MonkeyPatch, + pipette_command_map: "PipetteDispatch", + magnetic_module_command_map: "JsonV4MagneticModuleDispatch", + temperature_module_command_map: "JsonV4TemperatureModuleDispatch", + thermocycler_module_command_map: "JsonV4ThermocyclerDispatch", + mockObj: mock.Mock, +) -> None: monkeypatch.setattr(v4, "_delay", mockObj) monkeypatch.setattr(v4, "_move_to_slot", mockObj) @@ -356,7 +385,7 @@ def test_dispatch_json( dispatch_json( context, - protocol_data, + protocol_data, # type: ignore[arg-type] instruments, loaded_labware, modules, @@ -446,13 +475,14 @@ def test_dispatch_json( ] +@typing.no_type_check def test_dispatch_json_invalid_command( - pipette_command_map, - magnetic_module_command_map, - temperature_module_command_map, - thermocycler_module_command_map, -): - protocol_data = { + pipette_command_map: "PipetteDispatch", + magnetic_module_command_map: "JsonV4MagneticModuleDispatch", + temperature_module_command_map: "JsonV4TemperatureModuleDispatch", + thermocycler_module_command_map: "JsonV4ThermocyclerDispatch", +) -> None: + protocol_data: "JsonProtocolV4" = { "commands": [ {"command": "no_such_command", "params": "foo"}, ] @@ -472,7 +502,11 @@ def test_dispatch_json_invalid_command( @pytest.mark.ot2_only -def test_papi_execute_json_v4(monkeypatch, ctx, get_json_protocol_fixture): +def test_papi_execute_json_v4( + monkeypatch: pytest.MonkeyPatch, + ctx: ProtocolContext, + get_json_protocol_fixture: Callable[[str, str, bool], str], +) -> None: protocol_data = get_json_protocol_fixture("4", "testModulesProtocol", False) protocol = parse(protocol_data, None) ctx.home() @@ -480,14 +514,46 @@ def test_papi_execute_json_v4(monkeypatch, ctx, get_json_protocol_fixture): execute.run_protocol(protocol, ctx) -def test_assert_no_async_tc_behavior(): - setBlock = {"command": "thermocycler/setTargetBlockTemperature"} - awaitBlock = {"command": "thermocycler/awaitBlockTemperature"} - setLid = {"command": "thermocycler/setTargetLidTemperature"} - awaitLid = {"command": "thermocycler/awaitLidTemperature"} - runProfile = {"command": "thermocycler/runProfile"} - awaitProfile = {"command": "thermocycler/awaitProfileComplete"} - anything = {"command": "foo"} # stand-in for some other command +def test_assert_no_async_tc_behavior() -> None: + temp_params: "TemperatureParams" = { + "temperature": 75, + "module": "thermocycler_module_id", + } + set_target_params: "ThermocyclerSetTargetBlockParams" = { + "volume": 100, + **temp_params, + } + run_profile_params: "ThermocyclerRunProfileParams" = { + "module": "thermocycler_module_id", + "volume": 100, + "profile": [], + } + + setBlock: "ThermocyclerSetTargetBlockCommand" = { + "command": "thermocycler/setTargetBlockTemperature", + "params": set_target_params, + } + awaitBlock: "ThermocyclerAwaitBlockTemperatureCommand" = { + "command": "thermocycler/awaitBlockTemperature", + "params": temp_params, + } + setLid: "ThermocyclerSetTargetLidCommand" = { + "command": "thermocycler/setTargetLidTemperature", + "params": temp_params, + } + awaitLid: "ThermocyclerAwaitLidTemperatureCommand" = { + "command": "thermocycler/awaitLidTemperature", + "params": temp_params, + } + runProfile: "ThermocyclerRunProfileCommand" = { + "command": "thermocycler/runProfile", + "params": run_profile_params, + } + awaitProfile: "ThermocyclerAwaitProfileCommand" = { + "command": "thermocycler/awaitProfileComplete", + "params": run_profile_params, + } + anything: "ThermocyclerCommand" = {"command": "foo", "params": {}} # type: ignore[assignment, misc] # stand-in for some other command # no error assert_no_async_tc_behavior( @@ -507,7 +573,7 @@ def test_assert_no_async_tc_behavior(): assert_no_async_tc_behavior([anything, anything]) - setters = [setBlock, setLid, runProfile] + setters: List[ThermocyclerCommand] = [setBlock, setLid, runProfile] for setter in setters: # Should raise if anything except the corresponding await @@ -519,7 +585,7 @@ def test_assert_no_async_tc_behavior(): with pytest.raises(RuntimeError): assert_no_async_tc_behavior([anything, setter]) - awaiters = [awaitBlock, awaitLid, awaitProfile] + awaiters: List[ThermocyclerCommand] = [awaitBlock, awaitLid, awaitProfile] for awaiter in awaiters: # Should raise if anything except the corresponding set @@ -532,15 +598,24 @@ def test_assert_no_async_tc_behavior(): assert_no_async_tc_behavior([awaiter, anything]) -def test_assert_tc_commands_do_not_use_unimplemented_params(): +def test_assert_tc_commands_do_not_use_unimplemented_params() -> None: # should not throw for arbitrary commands assert_tc_commands_do_not_use_unimplemented_params( - [{"command": "foo", "params": {}}] + [{"command": "foo", "params": {}}] # type: ignore[list-item, misc] ) - fail_cases = [ - [{"command": "thermocycler/setTargetBlockTemperature", "params": {"volume": 0}}] + fail_cases: List[List[ThermocyclerSetTargetBlockCommand]] = [ + [ + { + "command": "thermocycler/setTargetBlockTemperature", + "params": { + "volume": 0, + "temperature": 0, + "module": "thermocycler_module_id", + }, + } + ] ] for cmds in fail_cases: with pytest.raises(RuntimeError): - assert_tc_commands_do_not_use_unimplemented_params(cmds) + assert_tc_commands_do_not_use_unimplemented_params(cmds) # type: ignore[arg-type] diff --git a/api/tests/opentrons/protocols/execution/test_execute_json_v5.py b/api/tests/opentrons/protocols/execution/test_execute_json_v5.py index 6b6d935b5fe..496794396b7 100644 --- a/api/tests/opentrons/protocols/execution/test_execute_json_v5.py +++ b/api/tests/opentrons/protocols/execution/test_execute_json_v5.py @@ -1,18 +1,22 @@ from unittest import mock +from opentrons.protocol_api.core.legacy.legacy_labware_core import LegacyLabwareCore from opentrons.protocol_api.core.legacy.legacy_well_core import LegacyWellCore from opentrons.protocol_api.core.legacy.well_geometry import WellGeometry from opentrons.types import Point from opentrons.protocol_api import InstrumentContext, labware, MAX_SUPPORTED_VERSION from opentrons.protocols.execution.execute_json_v5 import _move_to_well +from opentrons_shared_data.protocol.types import MoveToWellParams -def test_move_to_well_with_optional_params(): +def test_move_to_well_with_optional_params() -> None: + mock_parent = mock.create_autospec(labware.Labware) + mock_parent_core = mock.create_autospec(LegacyLabwareCore) pipette_mock = mock.create_autospec(InstrumentContext) instruments = {"somePipetteId": pipette_mock} well = labware.Well( - parent=None, + parent=mock_parent, core=LegacyWellCore( well_geometry=WellGeometry( { @@ -25,7 +29,7 @@ def test_move_to_well_with_optional_params(): "z": 3, }, parent_point=Point(10, 20, 30), - parent_object=1, + parent_object=mock_parent_core, ), display_name="some well", has_tip=False, @@ -36,7 +40,7 @@ def test_move_to_well_with_optional_params(): mock_get_well = mock.MagicMock(return_value=well, name="mock_get_well") - params = { + params: "MoveToWellParams" = { "pipette": "somePipetteId", "labware": "someLabwareId", "well": "someWell", @@ -61,12 +65,14 @@ def test_move_to_well_with_optional_params(): assert mock_get_well.mock_calls == [mock.call(mock.sentinel.loaded_labware, params)] -def test_move_to_well_without_optional_params(): +def test_move_to_well_without_optional_params() -> None: + mock_parent = mock.create_autospec(labware.Labware) + mock_parent_core = mock.create_autospec(LegacyLabwareCore) pipette_mock = mock.create_autospec(InstrumentContext) instruments = {"somePipetteId": pipette_mock} well = labware.Well( - parent=None, + parent=mock_parent, core=LegacyWellCore( well_geometry=WellGeometry( { @@ -79,7 +85,7 @@ def test_move_to_well_without_optional_params(): "z": 3, }, parent_point=Point(10, 20, 30), - parent_object=1, + parent_object=mock_parent_core, ), display_name="some well", has_tip=False, @@ -90,7 +96,7 @@ def test_move_to_well_without_optional_params(): mock_get_well = mock.MagicMock(return_value=well, name="mock_get_well") - params = { + params: "MoveToWellParams" = { "pipette": "somePipetteId", "labware": "someLabwareId", "well": "someWell",