From dcb5a37468e800875d5208b4392291a3cdc18b20 Mon Sep 17 00:00:00 2001 From: Mike Cousins Date: Tue, 9 Nov 2021 15:34:52 -0500 Subject: [PATCH] refactor(robot-server): consolidate runs into a single Run model (#8685) --- .../robot_server/runs/router/base_router.py | 24 +-- robot-server/robot_server/runs/run_models.py | 152 ++++++--------- robot-server/robot_server/runs/run_store.py | 5 +- robot-server/robot_server/runs/run_view.py | 77 ++------ .../tests/runs/router/test_actions_router.py | 14 +- .../tests/runs/router/test_base_router.py | 113 ++++------- .../tests/runs/router/test_commands_router.py | 26 ++- robot-server/tests/runs/test_run_store.py | 28 ++- robot-server/tests/runs/test_run_view.py | 179 +++--------------- 9 files changed, 178 insertions(+), 440 deletions(-) diff --git a/robot-server/robot_server/runs/router/base_router.py b/robot-server/robot_server/runs/router/base_router.py index a9e7ca09863..3431c36bc28 100644 --- a/robot-server/robot_server/runs/router/base_router.py +++ b/robot-server/robot_server/runs/router/base_router.py @@ -26,9 +26,9 @@ get_protocol_store, ) -from ..run_store import RunStore, RunNotFoundError +from ..run_store import RunStore, RunResource, RunNotFoundError from ..run_view import RunView -from ..run_models import Run, RunCreateData, ProtocolRunCreateData, RunUpdate +from ..run_models import Run, RunCreate, RunUpdate from ..engine_store import EngineStore, EngineConflictError from ..dependencies import get_run_store, get_engine_store @@ -88,7 +88,7 @@ class AllRunsLinks(BaseModel): }, ) async def create_run( - request_body: Optional[RequestModel[RunCreateData]] = None, + request_body: Optional[RequestModel[RunCreate]] = None, run_view: RunView = Depends(RunView), run_store: RunStore = Depends(get_run_store), engine_store: EngineStore = Depends(get_engine_store), @@ -109,12 +109,10 @@ async def create_run( created_at: Timestamp to attach to created run. task_runner: Background task runner. """ - create_data = request_body.data if request_body is not None else None + protocol_id = request_body.data.protocolId if request_body is not None else None protocol_resource = None - if isinstance(create_data, ProtocolRunCreateData): - protocol_id = create_data.createParams.protocolId - + if protocol_id is not None: try: protocol_resource = protocol_store.get(protocol_id=protocol_id) except ProtocolNotFoundError as e: @@ -132,10 +130,12 @@ async def create_run( # them in the run resource task_runner.run(engine_store.runner.join) - run = run_view.as_resource( + run = RunResource( run_id=run_id, + protocol_id=protocol_id, created_at=created_at, - create_data=create_data, + is_current=True, + actions=[], ) run_store.upsert(run=run) @@ -166,7 +166,7 @@ async def get_runs( """Get all runs. Args: - run_view: Run model construction interface. + run_view: Run model manipulation interface. run_store: Run storage interface. engine_store: ProtocolEngine storage and control. """ @@ -210,7 +210,7 @@ async def get_run( Args: runId: Run ID pulled from URL. - run_view: Run model construction interface. + run_view: Run model manipulation interface. run_store: Run storage interface. engine_store: ProtocolEngine storage and control. """ @@ -290,7 +290,7 @@ async def update_run( Args: runId: Run ID pulled from URL. request_body: Update data from request body. - run_view: Run model manipulation. + run_view: Run model manipulation interface. run_store: Run storage interface. engine_store: ProtocolEngine storage and control. """ diff --git a/robot-server/robot_server/runs/run_models.py b/robot-server/robot_server/runs/run_models.py index 9e861e96aa4..4e4f1da808b 100644 --- a/robot-server/robot_server/runs/run_models.py +++ b/robot-server/robot_server/runs/run_models.py @@ -1,9 +1,7 @@ """Request and response models for run resources.""" -from enum import Enum from datetime import datetime from pydantic import BaseModel, Field -from typing import List, Optional, Union -from typing_extensions import Literal +from typing import List, Optional from opentrons.protocol_engine import ( CommandStatus, @@ -17,13 +15,6 @@ from .action_models import RunAction -class RunType(str, Enum): - """All available run types.""" - - BASIC = "basic" - PROTOCOL = "protocol" - - class RunCommandSummary(ResourceModel): """A stripped down model of a full Command for usage in a Run response.""" @@ -32,11 +23,38 @@ class RunCommandSummary(ResourceModel): status: CommandStatus = Field(..., description="Execution status of the command.") -class _AbstractRun(ResourceModel): - """Base run resource model.""" +class LabwareOffsetVector(BaseModel): + """An offset to apply to labware, in deck coordinates.""" + + x: float + y: float + z: float + + +class LabwareOffset(BaseModel): + """An offset that the robot adds to a pipette's position when it moves to a labware. + + During the run, if a labware is loaded whose definition URI and location + both match what's found here, the given offset will be added to all + pipette movements that use that labware as a reference point. + """ + + id: str = Field(..., description="Unique labware offset record identifier.") + definitionUri: str = Field(..., description="The URI for the labware's definition.") + location: LabwareLocation = Field( + ..., + description="Where the labware is located on the robot.", + ) + offset: LabwareOffsetVector = Field( + ..., + description="The offset applied to matching labware.", + ) + + +class Run(ResourceModel): + """Run resource model.""" id: str = Field(..., description="Unique run identifier.") - runType: RunType = Field(..., description="Specific run type.") createdAt: datetime = Field(..., description="When the run was created") status: RunStatus = Field(..., description="Execution status of the run") current: bool = Field( @@ -62,95 +80,48 @@ class _AbstractRun(ResourceModel): ..., description="Labware that has been loaded into the run.", ) + labwareOffsets: List[LabwareOffset] = Field( + default_factory=list, + description="Labware offsets to apply as labware are loaded.", + ) + protocolId: Optional[str] = Field( + None, + description=( + "Protocol resource being run, if any. If not present, the run may" + " still be used to execute protocol commands over HTTP." + ), + ) -class LabwareOffsetVector(BaseModel): - """An offset to apply to labware, in deck coordinates.""" - - x: float - y: float - z: float - - -class LabwareOffset(BaseModel): - """An offset that the robot adds to a pipette's position when it moves to a labware. - - During the run, if a labware is loaded whose definition URI and location - both match what's found here, the given offset will be added to all - pipette movements that use that labware as a reference point. - """ +class LabwareOffsetCreate(BaseModel): + """Create request data for a labware offset.""" definitionUri: str = Field(..., description="The URI for the labware's definition.") location: LabwareLocation = Field( - ..., description="Where the labware is located on the robot." + ..., + description="Where the labware is located on the robot.", ) offset: LabwareOffsetVector = Field( - ..., description="The offset applied to matching labware." - ) - - -class BasicRunCreateParams(BaseModel): - """Creation parameters for a basic run.""" - - labwareOffsets: List[LabwareOffset] = Field(default_factory=list) - - -class BasicRunCreateData(BaseModel): - """Creation request data for a basic run.""" - - runType: Literal[RunType.BASIC] = Field( - RunType.BASIC, - description="The run type to create.", - ) - # TODO(mc, 2021-05-25): how hard would it be to rename this field to `config`? - createParams: BasicRunCreateParams = Field( - default_factory=BasicRunCreateParams, - description="Parameters to set run behaviors at creation time.", - ) - - -class BasicRun(_AbstractRun): - """A run to execute commands without a previously loaded protocol file.""" - - runType: Literal[RunType.BASIC] = RunType.BASIC - - createParams: BasicRunCreateParams - - -class ProtocolRunCreateParams(BaseModel): - """Creation parameters for a protocol run.""" - - protocolId: str = Field( ..., - description="Unique identifier of the protocol this run will execute.", + description="The offset applied to matching labware.", ) - labwareOffsets: List[LabwareOffset] = Field(default_factory=list) +class RunCreate(BaseModel): + """Create request data for a new run.""" -class ProtocolRunCreateData(BaseModel): - """Creation request data for a protocol run.""" - - runType: Literal[RunType.PROTOCOL] = Field( - RunType.PROTOCOL, - description="The run type to create.", + protocolId: Optional[str] = Field( + None, + description="Protocol resource ID that this run will be using, if applicable.", ) - # TODO(mc, 2021-05-25): how hard would it be to rename this field to `config`? - createParams: ProtocolRunCreateParams = Field( - ..., - description="Parameters to set run behaviors at creation time.", + labwareOffsets: List[LabwareOffsetCreate] = Field( + default_factory=list, + description="Labware offsets to apply as labware are loaded.", ) -class ProtocolRun(_AbstractRun): - """A run to execute commands with a previously loaded protocol file.""" - - runType: Literal[RunType.PROTOCOL] = RunType.PROTOCOL - createParams: ProtocolRunCreateParams - - class RunUpdate(BaseModel): - """Update request data for a run.""" + """Update request data for an existing run.""" current: Optional[bool] = Field( None, @@ -159,14 +130,3 @@ class RunUpdate(BaseModel): " Setting `current` to `false` will deactivate the run." ), ) - - -RunCreateData = Union[ - BasicRunCreateData, - ProtocolRunCreateData, -] - -Run = Union[ - BasicRun, - ProtocolRun, -] diff --git a/robot-server/robot_server/runs/run_store.py b/robot-server/robot_server/runs/run_store.py index 256396f5ded..bde5018314e 100644 --- a/robot-server/robot_server/runs/run_store.py +++ b/robot-server/robot_server/runs/run_store.py @@ -1,9 +1,8 @@ """Runs' in-memory store.""" from dataclasses import dataclass, replace from datetime import datetime -from typing import Dict, List +from typing import Dict, List, Optional -from .run_models import RunCreateData from .action_models import RunAction @@ -16,7 +15,7 @@ class RunResource: """ run_id: str - create_data: RunCreateData + protocol_id: Optional[str] created_at: datetime actions: List[RunAction] is_current: bool diff --git a/robot-server/robot_server/runs/run_view.py b/robot-server/robot_server/runs/run_view.py index 742a9aa7130..d1309212773 100644 --- a/robot-server/robot_server/runs/run_view.py +++ b/robot-server/robot_server/runs/run_view.py @@ -1,7 +1,7 @@ """Run response model factory.""" from dataclasses import replace from datetime import datetime -from typing import List, Optional, Tuple +from typing import List, Tuple from opentrons.protocol_engine import ( Command as ProtocolEngineCommand, @@ -12,16 +12,7 @@ from .run_store import RunResource from .action_models import RunAction, RunActionCreateData -from .run_models import ( - Run, - RunCreateData, - BasicRun, - BasicRunCreateData, - ProtocolRun, - ProtocolRunCreateData, - RunCommandSummary, - RunUpdate, -) +from .run_models import Run, RunUpdate, RunCommandSummary class RunView: @@ -31,31 +22,6 @@ class RunView: immutable. """ - @staticmethod - def as_resource( - run_id: str, - created_at: datetime, - create_data: Optional[RunCreateData], - ) -> RunResource: - """Create a new run resource instance from its create data. - - Arguments: - run_id: Unique identifier. - created_at: Resource creation timestamp. - create_data: Data used to create the run. - - Returns: - The run in its internal resource representation, for use in - the `RunStore` and other classes. - """ - return RunResource( - run_id=run_id, - created_at=created_at, - create_data=create_data or BasicRunCreateData(), - actions=[], - is_current=True, - ) - @staticmethod def with_update(run: RunResource, update: RunUpdate) -> RunResource: """Update a run resource with update request data. @@ -123,36 +89,19 @@ def as_response( Returns: Run response model representing the same resource. """ - create_data = run.create_data command_summaries = [ RunCommandSummary(id=c.id, commandType=c.commandType, status=c.status) for c in commands ] - if isinstance(create_data, BasicRunCreateData): - return BasicRun( - id=run.run_id, - createParams=create_data.createParams, - createdAt=run.created_at, - current=run.is_current, - actions=run.actions, - commands=command_summaries, - pipettes=pipettes, - labware=labware, - status=engine_status, - ) - - if isinstance(create_data, ProtocolRunCreateData): - return ProtocolRun( - id=run.run_id, - createParams=create_data.createParams, - createdAt=run.created_at, - current=run.is_current, - actions=run.actions, - commands=command_summaries, - pipettes=pipettes, - labware=labware, - status=engine_status, - ) - - raise ValueError(f"Invalid run resource {run}") + return Run( + id=run.run_id, + protocolId=run.protocol_id, + createdAt=run.created_at, + current=run.is_current, + actions=run.actions, + commands=command_summaries, + pipettes=pipettes, + labware=labware, + status=engine_status, + ) diff --git a/robot-server/tests/runs/router/test_actions_router.py b/robot-server/tests/runs/router/test_actions_router.py index 490537526dd..45960d91fe7 100644 --- a/robot-server/tests/runs/router/test_actions_router.py +++ b/robot-server/tests/runs/router/test_actions_router.py @@ -8,7 +8,7 @@ from tests.helpers import verify_response from opentrons.protocol_engine.errors import ProtocolEngineStoppedError -from robot_server.runs.run_models import BasicRunCreateData + from robot_server.runs.run_view import RunView from robot_server.runs.engine_store import EngineStore from robot_server.runs.run_store import ( @@ -33,7 +33,7 @@ prev_run = RunResource( run_id="run-id", - create_data=BasicRunCreateData(), + protocol_id=None, created_at=datetime(year=2021, month=1, day=1), actions=[], is_current=True, @@ -70,7 +70,7 @@ def test_create_play_action( next_run = RunResource( run_id="run-id", - create_data=BasicRunCreateData(), + protocol_id=None, created_at=datetime(year=2021, month=1, day=1), actions=[action], is_current=True, @@ -136,7 +136,7 @@ def test_create_run_action_without_runner( next_run = RunResource( run_id="run-id", - create_data=BasicRunCreateData(), + protocol_id=None, created_at=datetime(year=2021, month=1, day=1), actions=[actions], is_current=True, @@ -179,7 +179,7 @@ def test_create_run_action_not_current( """It should 409 if the run is not current.""" prev_run = RunResource( run_id="run-id", - create_data=BasicRunCreateData(), + protocol_id=None, created_at=datetime(year=2021, month=1, day=1), actions=[], is_current=False, @@ -217,7 +217,7 @@ def test_create_pause_action( next_run = RunResource( run_id="run-id", - create_data=BasicRunCreateData(), + protocol_id=None, created_at=datetime(year=2021, month=1, day=1), actions=[action], is_current=True, @@ -259,7 +259,7 @@ async def test_create_stop_action( next_run = RunResource( run_id="run-id", - create_data=BasicRunCreateData(), + protocol_id=None, created_at=datetime(year=2021, month=1, day=1), actions=[action], is_current=True, diff --git a/robot-server/tests/runs/router/test_base_router.py b/robot-server/tests/runs/router/test_base_router.py index a37eeb0951f..928450c8964 100644 --- a/robot-server/tests/runs/router/test_base_router.py +++ b/robot-server/tests/runs/router/test_base_router.py @@ -22,17 +22,7 @@ ) from robot_server.runs.run_view import RunView - -from robot_server.runs.run_models import ( - RunCommandSummary, - BasicRun, - BasicRunCreateData, - BasicRunCreateParams, - ProtocolRun, - ProtocolRunCreateData, - ProtocolRunCreateParams, - RunUpdate, -) +from robot_server.runs.run_models import RunCommandSummary, Run, RunCreate, RunUpdate from robot_server.runs.engine_store import ( EngineStore, @@ -66,17 +56,18 @@ async def test_create_run( """It should be able to create a basic run.""" run_id = "run-id" run_created_at = datetime(year=2021, month=1, day=1) - run = RunResource( + + expected_run = RunResource( run_id=run_id, created_at=run_created_at, - create_data=BasicRunCreateData(), + protocol_id=None, actions=[], is_current=True, ) - expected_response = BasicRun( + expected_response = Run( id=run_id, + protocolId=None, createdAt=run_created_at, - createParams=BasicRunCreateParams(), status=pe_types.EngineStatus.READY_TO_RUN, current=True, actions=[], @@ -95,17 +86,9 @@ async def test_create_run( pe_types.EngineStatus.READY_TO_RUN ) - decoy.when( - run_view.as_resource( - run_id=run_id, - created_at=run_created_at, - create_data=BasicRunCreateData(), - ) - ).then_return(run) - decoy.when( run_view.as_response( - run=run, + run=expected_run, commands=[], pipettes=[], labware=[], @@ -114,7 +97,7 @@ async def test_create_run( ).then_return(expected_response) result = await create_run( - request_body=RequestModel(data=BasicRunCreateData()), + request_body=RequestModel(data=RunCreate()), run_view=run_view, run_store=run_store, engine_store=engine_store, @@ -127,7 +110,7 @@ async def test_create_run( decoy.verify( task_runner.run(engine_store.runner.join), - run_store.upsert(run=run), + run_store.upsert(run=expected_run), ) @@ -140,14 +123,12 @@ async def test_create_protocol_run( task_runner: TaskRunner, ) -> None: """It should be able to create a protocol run.""" - run_id = "run-id" run_created_at = datetime(year=2021, month=1, day=1) + run = RunResource( - run_id=run_id, + run_id="run-id", + protocol_id="protocol-id", created_at=run_created_at, - create_data=ProtocolRunCreateData( - createParams=ProtocolRunCreateParams(protocolId="protocol-id") - ), actions=[], is_current=True, ) @@ -157,12 +138,12 @@ async def test_create_protocol_run( created_at=datetime(year=2022, month=2, day=2), files=[], ) - expected_response = ProtocolRun( - id=run_id, + expected_response = Run( + id="run-id", + protocolId="protocol-id", createdAt=run_created_at, status=pe_types.EngineStatus.READY_TO_RUN, current=True, - createParams=ProtocolRunCreateParams(protocolId="protocol-id"), actions=[], commands=[], pipettes=[], @@ -173,18 +154,8 @@ async def test_create_protocol_run( protocol_resource ) - decoy.when( - run_view.as_resource( - run_id=run_id, - created_at=run_created_at, - create_data=ProtocolRunCreateData( - createParams=ProtocolRunCreateParams(protocolId="protocol-id") - ), - ) - ).then_return(run) - engine_state = decoy.mock(cls=StateView) - decoy.when(await engine_store.create(run_id=run_id)).then_return(engine_state) + decoy.when(await engine_store.create(run_id="run-id")).then_return(engine_state) decoy.when(engine_state.commands.get_all()).then_return([]) decoy.when(engine_state.pipettes.get_all()).then_return([]) @@ -204,17 +175,13 @@ async def test_create_protocol_run( ).then_return(expected_response) result = await create_run( - request_body=RequestModel( - data=ProtocolRunCreateData( - createParams=ProtocolRunCreateParams(protocolId="protocol-id") - ) - ), + request_body=RequestModel(data=RunCreate(protocolId="protocol-id")), run_view=run_view, run_store=run_store, engine_store=engine_store, protocol_store=protocol_store, task_runner=task_runner, - run_id=run_id, + run_id="run-id", created_at=run_created_at, ) @@ -238,11 +205,7 @@ async def test_create_protocol_run_missing_protocol( with pytest.raises(ApiError) as exc_info: await create_run( - request_body=RequestModel( - data=ProtocolRunCreateData( - createParams=ProtocolRunCreateParams(protocolId="protocol-id") - ) - ), + request_body=RequestModel(data=RunCreate(protocolId="protocol-id")), protocol_store=protocol_store, ) @@ -270,11 +233,11 @@ async def test_get_run( engine_store: EngineStore, ) -> None: """It should be able to get a run by ID.""" - created_at = datetime.now() - create_data = BasicRunCreateData() + created_at = datetime(year=2021, month=1, day=1) + run = RunResource( run_id="run-id", - create_data=create_data, + protocol_id=None, created_at=created_at, actions=[], is_current=False, @@ -300,9 +263,9 @@ async def test_get_run( mount=MountType.LEFT, ) - expected_response = BasicRun( + expected_response = Run( id="run-id", - createParams=BasicRunCreateParams(), + protocolId=None, createdAt=created_at, status=pe_types.EngineStatus.READY_TO_RUN, current=False, @@ -385,7 +348,7 @@ async def test_get_runs_not_empty( run_1 = RunResource( run_id="unique-id-1", - create_data=BasicRunCreateData(), + protocol_id=None, created_at=created_at_1, actions=[], is_current=False, @@ -393,15 +356,15 @@ async def test_get_runs_not_empty( run_2 = RunResource( run_id="unique-id-2", - create_data=BasicRunCreateData(), + protocol_id=None, created_at=created_at_2, actions=[], is_current=True, ) - response_1 = BasicRun( + response_1 = Run( id="unique-id-1", - createParams=BasicRunCreateParams(), + protocolId=None, createdAt=created_at_1, status=pe_types.EngineStatus.SUCCEEDED, current=False, @@ -411,9 +374,9 @@ async def test_get_runs_not_empty( labware=[], ) - response_2 = BasicRun( + response_2 = Run( id="unique-id-2", - createParams=BasicRunCreateParams(), + protocolId=None, createdAt=created_at_2, status=pe_types.EngineStatus.READY_TO_RUN, current=True, @@ -557,7 +520,7 @@ async def test_update_run_to_not_current( """It should update a run to no longer be current.""" run_resource = RunResource( run_id="run-id", - create_data=BasicRunCreateData(), + protocol_id=None, created_at=datetime(year=2021, month=1, day=1), actions=[], is_current=True, @@ -565,15 +528,15 @@ async def test_update_run_to_not_current( updated_resource = RunResource( run_id="run-id", - create_data=BasicRunCreateData(), + protocol_id=None, created_at=datetime(year=2021, month=1, day=1), actions=[], is_current=False, ) - expected_response = BasicRun( + expected_response = Run( id="run-id", - createParams=BasicRunCreateParams(), + protocolId=None, createdAt=datetime(year=2021, month=1, day=1), status=pe_types.EngineStatus.SUCCEEDED, current=False, @@ -633,15 +596,15 @@ async def test_update_current_to_current_noop( """It should noop if updating the current run to current: true.""" run_resource = RunResource( run_id="run-id", - create_data=BasicRunCreateData(), + protocol_id=None, created_at=datetime(year=2021, month=1, day=1), actions=[], is_current=True, ) - expected_response = BasicRun( + expected_response = Run( id="run-id", - createParams=BasicRunCreateParams(), + protocolId=None, createdAt=datetime(year=2021, month=1, day=1), status=pe_types.EngineStatus.SUCCEEDED, current=True, @@ -699,7 +662,7 @@ async def test_update_to_current_conflict( """It should 409 if attempting to update a not current run.""" run_resource = RunResource( run_id="run-id", - create_data=BasicRunCreateData(), + protocol_id=None, created_at=datetime(year=2021, month=1, day=1), actions=[], is_current=False, diff --git a/robot-server/tests/runs/router/test_commands_router.py b/robot-server/tests/runs/router/test_commands_router.py index f015b9a7c7f..0ada83925e1 100644 --- a/robot-server/tests/runs/router/test_commands_router.py +++ b/robot-server/tests/runs/router/test_commands_router.py @@ -14,11 +14,7 @@ from robot_server.errors import ApiError from robot_server.service.json_api import RequestModel, ResponseModel -from robot_server.runs.run_models import ( - BasicRun, - BasicRunCreateParams, - RunCommandSummary, -) +from robot_server.runs.run_models import Run, RunCommandSummary from robot_server.runs.engine_store import EngineStore from robot_server.runs.router.commands_router import ( post_run_command, @@ -33,9 +29,9 @@ async def test_post_run_command(decoy: Decoy, engine_store: EngineStore) -> None data=pe_commands.PauseData(message="Hello") ) - run = BasicRun( + run = Run( id="run-id", - createParams=BasicRunCreateParams(), + protocolId=None, createdAt=datetime(year=2021, month=1, day=1), status=EngineStatus.RUNNING, current=True, @@ -75,9 +71,9 @@ async def test_post_run_command_not_current( data=pe_commands.PauseData(message="Hello") ) - run = BasicRun( + run = Run( id="run-id", - createParams=BasicRunCreateParams(), + protocolId=None, createdAt=datetime(year=2021, month=1, day=1), status=EngineStatus.RUNNING, current=False, @@ -106,9 +102,9 @@ async def test_get_run_commands() -> None: status=CommandStatus.RUNNING, ) - run = BasicRun( + run = Run( id="run-id", - createParams=BasicRunCreateParams(), + protocolId=None, createdAt=datetime(year=2021, month=1, day=1), status=EngineStatus.RUNNING, current=True, @@ -141,9 +137,9 @@ async def test_get_run_command_by_id( data=pe_commands.MoveToWellData(pipetteId="a", labwareId="b", wellName="c"), ) - run = BasicRun( + run = Run( id="run-id", - createParams=BasicRunCreateParams(), + protocolId=None, createdAt=datetime(year=2021, month=1, day=1), status=EngineStatus.RUNNING, current=True, @@ -174,9 +170,9 @@ async def test_get_run_command_missing_command( """It should 404 if you attempt to get a non-existent command.""" key_error = pe_errors.CommandDoesNotExistError("oh no") - run = BasicRun( + run = Run( id="run-id", - createParams=BasicRunCreateParams(), + protocolId=None, createdAt=datetime(year=2021, month=1, day=1), status=EngineStatus.RUNNING, current=True, diff --git a/robot-server/tests/runs/test_run_store.py b/robot-server/tests/runs/test_run_store.py index e9dbdc6eacc..cb6505e784e 100644 --- a/robot-server/tests/runs/test_run_store.py +++ b/robot-server/tests/runs/test_run_store.py @@ -2,20 +2,14 @@ import pytest from datetime import datetime -from robot_server.runs.run_models import BasicRunCreateData - -from robot_server.runs.run_store import ( - RunStore, - RunResource, - RunNotFoundError, -) +from robot_server.runs.run_store import RunStore, RunResource, RunNotFoundError def test_add_run() -> None: - """It should be able to create a basic run from a None data argument.""" + """It should be able to add a new run to the store.""" run = RunResource( run_id="run-id", - create_data=BasicRunCreateData(), + protocol_id=None, created_at=datetime.now(), actions=[], is_current=True, @@ -31,14 +25,14 @@ def test_update_run() -> None: """It should be able to update a run in the store.""" run = RunResource( run_id="identical-run-id", - create_data=BasicRunCreateData(), + protocol_id=None, created_at=datetime(year=2021, month=1, day=1, hour=1, minute=1, second=1), actions=[], is_current=True, ) updated_run = RunResource( run_id="identical-run-id", - create_data=BasicRunCreateData(), + protocol_id=None, created_at=datetime(year=2022, month=2, day=2, hour=2, minute=2, second=2), actions=[], is_current=True, @@ -56,7 +50,7 @@ def test_get_run() -> None: """It can get a previously stored run entry.""" run = RunResource( run_id="run-id", - create_data=BasicRunCreateData(), + protocol_id=None, created_at=datetime.now(), actions=[], is_current=False, @@ -82,14 +76,14 @@ def test_get_all_runs() -> None: """It can get all created runs.""" run_1 = RunResource( run_id="run-id-1", - create_data=BasicRunCreateData(), + protocol_id=None, created_at=datetime.now(), actions=[], is_current=False, ) run_2 = RunResource( run_id="run-id-2", - create_data=BasicRunCreateData(), + protocol_id=None, created_at=datetime.now(), actions=[], is_current=True, @@ -108,7 +102,7 @@ def test_remove_run() -> None: """It can remove and return a previously stored run entry.""" run = RunResource( run_id="run-id", - create_data=BasicRunCreateData(), + protocol_id=None, created_at=datetime.now(), actions=[], is_current=True, @@ -135,7 +129,7 @@ def test_add_run_current_run_deactivates() -> None: """Adding a current run should mark all others as not current.""" run_1 = RunResource( run_id="run-id-1", - create_data=BasicRunCreateData(), + protocol_id=None, created_at=datetime.now(), actions=[], is_current=True, @@ -143,7 +137,7 @@ def test_add_run_current_run_deactivates() -> None: run_2 = RunResource( run_id="run-id-2", - create_data=BasicRunCreateData(), + protocol_id=None, created_at=datetime.now(), actions=[], is_current=True, diff --git a/robot-server/tests/runs/test_run_view.py b/robot-server/tests/runs/test_run_view.py index b46a05713ed..c9276c31305 100644 --- a/robot-server/tests/runs/test_run_view.py +++ b/robot-server/tests/runs/test_run_view.py @@ -1,5 +1,4 @@ """Tests for the run resource model builder.""" -import pytest from datetime import datetime from opentrons.types import DeckSlotName, MountType @@ -15,19 +14,7 @@ from robot_server.runs.run_store import RunResource from robot_server.runs.run_view import RunView -from robot_server.runs.run_models import ( - Run, - BasicRun, - BasicRunCreateData, - BasicRunCreateParams, - ProtocolRun, - ProtocolRunCreateData, - ProtocolRunCreateParams, - RunCommandSummary, - LabwareOffset, - LabwareOffsetVector, - RunUpdate, -) +from robot_server.runs.run_models import Run, RunUpdate, RunCommandSummary from robot_server.runs.action_models import ( RunAction, @@ -36,137 +23,16 @@ ) -def test_create_run_resource_from_none() -> None: - """It should create a basic run from create_data=None.""" - created_at = datetime.now() - create_data = None - - subject = RunView() - result = subject.as_resource( - run_id="run-id", - created_at=created_at, - create_data=create_data, - ) - - assert result == RunResource( - run_id="run-id", - create_data=BasicRunCreateData(), - created_at=created_at, - actions=[], - is_current=True, - ) - - -def test_create_run_resource() -> None: - """It should create a run with create_data specified.""" - created_at = datetime.now() - create_data = BasicRunCreateData() - - subject = RunView() - result = subject.as_resource( - run_id="run-id", created_at=created_at, create_data=create_data - ) - - assert result == RunResource( - run_id="run-id", - create_data=create_data, - created_at=created_at, - actions=[], - is_current=True, - ) - - -def test_create_protocol_run_resource() -> None: - """It should create a protocol run resource view.""" - created_at = datetime.now() - create_data = ProtocolRunCreateData( - createParams=ProtocolRunCreateParams(protocolId="protocol-id") - ) - - subject = RunView() - result = subject.as_resource( - run_id="run-id", created_at=created_at, create_data=create_data - ) - - assert result == RunResource( +def test_to_response() -> None: + """It should create the correct type of run.""" + run_resource = RunResource( run_id="run-id", - create_data=create_data, - created_at=created_at, + protocol_id=None, + created_at=datetime(year=2021, month=1, day=1), actions=[], is_current=True, ) - -current_time = datetime.now() - - -@pytest.mark.parametrize( - ("run_resource", "expected_response"), - ( - ( - RunResource( - run_id="run-id", - create_data=BasicRunCreateData( - createParams=BasicRunCreateParams( - labwareOffsets=[ - LabwareOffset( - definitionUri="my-labware-definition-uri", - location=DeckSlotLocation(slot=DeckSlotName.SLOT_1), - offset=LabwareOffsetVector(x=1, y=2, z=3), - ) - ] - ) - ), - created_at=current_time, - actions=[], - is_current=True, - ), - BasicRun( - id="run-id", - createdAt=current_time, - status=EngineStatus.READY_TO_RUN, - current=True, - createParams=BasicRunCreateParams( - labwareOffsets=[ - LabwareOffset( - definitionUri="my-labware-definition-uri", - location=DeckSlotLocation(slot=DeckSlotName.SLOT_1), - offset=LabwareOffsetVector(x=1, y=2, z=3), - ) - ] - ), - actions=[], - commands=[], - pipettes=[], - labware=[], - ), - ), - ( - RunResource( - run_id="run-id", - create_data=ProtocolRunCreateData( - createParams=ProtocolRunCreateParams(protocolId="protocol-id") - ), - created_at=current_time, - actions=[], - is_current=False, - ), - ProtocolRun( - id="run-id", - createdAt=current_time, - status=EngineStatus.READY_TO_RUN, - current=False, - createParams=ProtocolRunCreateParams(protocolId="protocol-id"), - actions=[], - commands=[], - pipettes=[], - labware=[], - ), - ), - ), -) -def test_to_response(run_resource: RunResource, expected_response: Run) -> None: - """It should create the correct type of run.""" subject = RunView() result = subject.as_response( run=run_resource, @@ -175,14 +41,25 @@ def test_to_response(run_resource: RunResource, expected_response: Run) -> None: labware=[], engine_status=EngineStatus.READY_TO_RUN, ) - assert result == expected_response + + assert result == Run( + id="run-id", + protocolId=None, + createdAt=datetime(year=2021, month=1, day=1), + status=EngineStatus.READY_TO_RUN, + current=True, + actions=[], + commands=[], + pipettes=[], + labware=[], + ) def test_to_response_maps_commands() -> None: """It should map ProtocolEngine commands to RunCommandSummary models.""" run_resource = RunResource( run_id="run-id", - create_data=BasicRunCreateData(), + protocol_id="protocol-id", created_at=datetime(year=2021, month=1, day=1), actions=[], is_current=True, @@ -214,9 +91,9 @@ def test_to_response_maps_commands() -> None: engine_status=EngineStatus.RUNNING, ) - assert result == BasicRun( + assert result == Run( id="run-id", - createParams=BasicRunCreateParams(), + protocolId="protocol-id", createdAt=datetime(year=2021, month=1, day=1), status=EngineStatus.RUNNING, current=True, @@ -242,7 +119,7 @@ def test_to_response_adds_equipment() -> None: """It should add ProtocolEngine equipment to Session response model.""" run_resource = RunResource( run_id="run-id", - create_data=BasicRunCreateData(), + protocol_id=None, created_at=datetime(year=2021, month=1, day=1), actions=[], is_current=True, @@ -270,9 +147,9 @@ def test_to_response_adds_equipment() -> None: engine_status=EngineStatus.RUNNING, ) - assert result == BasicRun( + assert result == Run( id="run-id", - createParams=BasicRunCreateParams(), + protocolId=None, createdAt=datetime(year=2021, month=1, day=1), status=EngineStatus.RUNNING, current=True, @@ -289,7 +166,7 @@ def test_create_action(current_time: datetime) -> None: run = RunResource( run_id="run-id", - create_data=BasicRunCreateData(), + protocol_id="protocol-id", created_at=run_created_at, actions=[], is_current=True, @@ -315,7 +192,7 @@ def test_create_action(current_time: datetime) -> None: assert run_result == RunResource( run_id="run-id", - create_data=BasicRunCreateData(), + protocol_id="protocol-id", created_at=run_created_at, actions=[action_result], is_current=True, @@ -326,7 +203,7 @@ def test_with_update() -> None: """It should update a run resource to not current.""" run = RunResource( run_id="run-id", - create_data=BasicRunCreateData(), + protocol_id="protocol-id", created_at=datetime(year=2021, month=1, day=1), actions=[], is_current=True, @@ -338,7 +215,7 @@ def test_with_update() -> None: assert result == RunResource( run_id="run-id", - create_data=BasicRunCreateData(), + protocol_id="protocol-id", created_at=datetime(year=2021, month=1, day=1), actions=[], is_current=False,