diff --git a/api/release-notes.md b/api/release-notes.md index f48d7343dab..92eea73d543 100644 --- a/api/release-notes.md +++ b/api/release-notes.md @@ -36,7 +36,7 @@ Flex touchscreen - Manage instruments: View information about connected pipettes and the gripper. Attach, detach, or recalibrate instruments. - Robot settings: Customize the behavior of your Flex, including the LED and touchscreen displays. -Flex features +Flex features - Analyze and run protocols that use the Flex robot, Flex pipettes, and Flex tip racks. - Move labware around the deck automatically with the Flex Gripper. @@ -65,7 +65,6 @@ Python API features Some protocols can't be simulated with the `opentrons_simulate` command-line tool: - JSON protocols created or modified with Protocol Designer v6.0.0 or higher. -- Python protocols specifying an `apiLevel` of 2.14 or higher. --- diff --git a/api/src/opentrons/execute.py b/api/src/opentrons/execute.py index 5a3f30743bb..15f0acf84c2 100644 --- a/api/src/opentrons/execute.py +++ b/api/src/opentrons/execute.py @@ -11,15 +11,12 @@ import contextlib import logging import os -from pathlib import Path import sys -import tempfile from typing import ( TYPE_CHECKING, BinaryIO, Callable, Dict, - Generator, List, Optional, TextIO, @@ -35,7 +32,6 @@ from opentrons.hardware_control import ( API as OT2API, - HardwareControlAPI, ThreadManagedHardware, ThreadManager, ) @@ -59,23 +55,16 @@ Config, DeckType, EngineStatus, - ErrorOccurrence as ProtocolEngineErrorOccurrence, create_protocol_engine, create_protocol_engine_in_thread, ) from opentrons.protocol_engine.types import PostRunHardwareState -from opentrons.protocol_reader import ProtocolReader, ProtocolSource +from opentrons.protocol_reader import ProtocolSource from opentrons.protocol_runner import create_protocol_runner -from .util.entrypoint_util import ( - FoundLabware, - find_jupyter_labware, - labware_from_paths, - datafiles_from_paths, - copy_file_like, -) +from .util import entrypoint_util if TYPE_CHECKING: from opentrons_shared_data.labware.dev_types import ( @@ -172,7 +161,7 @@ def get_protocol_api( if extra_labware is None: extra_labware = { uri: details.definition - for uri, details in (find_jupyter_labware() or {}).items() + for uri, details in (entrypoint_util.find_jupyter_labware() or {}).items() } robot_type = _get_robot_type() @@ -191,6 +180,8 @@ def get_protocol_api( ) else: if bundled_labware is not None: + # Protocol Engine has a deep assumption that standard labware definitions are always + # implicitly loadable. raise NotImplementedError( f"The bundled_labware argument is not currently supported for Python protocols" f" with apiLevel {ENGINE_CORE_API_VERSION} or newer." @@ -363,21 +354,20 @@ def execute( # noqa: C901 stack_logger.propagate = propagate_logs stack_logger.setLevel(getattr(logging, log_level.upper(), logging.WARNING)) - contents = protocol_file.read() - # TODO(mm, 2023-10-02): Switch this truthy check to `is not None` # to match documented behavior. # See notes in https://github.com/Opentrons/opentrons/pull/13107 if custom_labware_paths: - extra_labware = labware_from_paths(custom_labware_paths) + extra_labware = entrypoint_util.labware_from_paths(custom_labware_paths) else: - extra_labware = find_jupyter_labware() or {} + extra_labware = entrypoint_util.find_jupyter_labware() or {} if custom_data_paths: - extra_data = datafiles_from_paths(custom_data_paths) + extra_data = entrypoint_util.datafiles_from_paths(custom_data_paths) else: extra_data = {} + contents = protocol_file.read() try: protocol = parse.parse( contents, @@ -422,10 +412,8 @@ def execute( # noqa: C901 ) protocol_file.seek(0) _run_file_pe( - protocol_file=protocol_file, - protocol_name=protocol_name, - extra_labware=extra_labware, - hardware_api=_get_global_hardware_controller(_get_robot_type()).wrapped(), + protocol=protocol, + hardware_api=_get_global_hardware_controller(_get_robot_type()), emit_runlog=emit_runlog, ) @@ -493,8 +481,8 @@ def main() -> int: emit_runlog=printer, ) return 0 - except _ProtocolEngineExecuteError as error: - # _ProtocolEngineExecuteError is a wrapper that's meaningless to the CLI user. + except entrypoint_util.ProtocolEngineExecuteError as error: + # This exception is a wrapper that's meaningless to the CLI user. # Take the actual protocol problem out of it and just print that. print(error.to_stderr_string(), file=sys.stderr) return 1 @@ -502,40 +490,6 @@ def main() -> int: # Just let Python show a traceback. -class _ProtocolEngineExecuteError(Exception): - def __init__(self, errors: List[ProtocolEngineErrorOccurrence]) -> None: - """Raised when there was any fatal error running a protocol through Protocol Engine. - - Protocol Engine reports errors as data, not as exceptions. - But the only way for `execute()` to signal problems to its caller is to raise something. - So we need this class to wrap them. - - Params: - errors: The errors that Protocol Engine reported. - """ - # Show the full error details if this is part of a traceback. Don't try to summarize. - super().__init__(errors) - self._error_occurrences = errors - - def to_stderr_string(self) -> str: - """Return a string suitable as the stderr output of the `opentrons_execute` CLI. - - This summarizes from the full error details. - """ - # It's unclear what exactly we should extract here. - # - # First, do we print the first element, or the last, or all of them? - # - # Second, do we print the .detail? .errorCode? .errorInfo? .wrappedErrors? - # By contract, .detail seems like it would be insufficient, but experimentally, - # it includes a lot, like: - # - # ProtocolEngineError [line 3]: Error 4000 GENERAL_ERROR (ProtocolEngineError): - # UnexpectedProtocolError: Labware "fixture_12_trough" not found with version 1 - # in namespace "fixture". - return self._error_occurrences[0].detail - - def _create_live_context_non_pe( api_version: APIVersion, hardware_api: ThreadManagedHardware, @@ -640,24 +594,22 @@ def _run_file_non_pe( def _run_file_pe( - protocol_file: Union[BinaryIO, TextIO], - protocol_name: str, - extra_labware: Dict[str, FoundLabware], - hardware_api: HardwareControlAPI, + protocol: Protocol, + hardware_api: ThreadManagedHardware, emit_runlog: Optional[_EmitRunlogCallable], ) -> None: """Run a protocol file with Protocol Engine.""" async def run(protocol_source: ProtocolSource) -> None: protocol_engine = await create_protocol_engine( - hardware_api=hardware_api, + hardware_api=hardware_api.wrapped(), config=_get_protocol_engine_config(), ) protocol_runner = create_protocol_runner( protocol_config=protocol_source.config, protocol_engine=protocol_engine, - hardware_api=hardware_api, + hardware_api=hardware_api.wrapped(), ) unsubscribe = protocol_runner.broker.subscribe( @@ -671,13 +623,11 @@ async def run(protocol_source: ProtocolSource) -> None: unsubscribe() if result.state_summary.status != EngineStatus.SUCCEEDED: - raise _ProtocolEngineExecuteError(result.state_summary.errors) + raise entrypoint_util.ProtocolEngineExecuteError( + result.state_summary.errors + ) - with _adapt_protocol_source( - protocol_file=protocol_file, - protocol_name=protocol_name, - extra_labware=extra_labware, - ) as protocol_source: + with entrypoint_util.adapt_protocol_source(protocol) as protocol_source: asyncio.run(run(protocol_source)) @@ -697,39 +647,6 @@ def _get_protocol_engine_config() -> Config: ) -@contextlib.contextmanager -def _adapt_protocol_source( - protocol_file: Union[BinaryIO, TextIO], - protocol_name: str, - extra_labware: Dict[str, FoundLabware], -) -> Generator[ProtocolSource, None, None]: - """Create a `ProtocolSource` representing input protocol files.""" - with tempfile.TemporaryDirectory() as temporary_directory: - # It's not well-defined in our customer-facing interfaces whether the supplied protocol_name - # should be just the filename part, or a path with separators. In case it contains stuff - # like "../", sanitize it to just the filename part so we don't save files somewhere bad. - safe_protocol_name = Path(protocol_name).name - - temp_protocol_file = Path(temporary_directory) / safe_protocol_name - - # FIXME(mm, 2023-06-26): Copying this file is pure overhead, and it introduces encoding - # hazards. Remove this when we can parse JSONv6+ and PAPIv2.14+ protocols without going - # through the filesystem. https://opentrons.atlassian.net/browse/RSS-281 - copy_file_like(source=protocol_file, destination=temp_protocol_file) - - custom_labware_files = [labware.path for labware in extra_labware.values()] - - protocol_source = asyncio.run( - ProtocolReader().read_saved( - files=[temp_protocol_file] + custom_labware_files, - directory=None, - files_are_prevalidated=False, - ) - ) - - yield protocol_source - - def _get_global_hardware_controller(robot_type: RobotType) -> ThreadManagedHardware: # Build a hardware controller in a worker thread, which is necessary # because ipython runs its notebook in asyncio but the notebook diff --git a/api/src/opentrons/protocols/execution/execute_python.py b/api/src/opentrons/protocols/execution/execute_python.py index 1d01a7120cd..51955b35766 100644 --- a/api/src/opentrons/protocols/execution/execute_python.py +++ b/api/src/opentrons/protocols/execution/execute_python.py @@ -47,10 +47,19 @@ def run_python(proto: PythonProtocol, context: ProtocolContext): # If the protocol is written correctly, it will have defined a function # like run(context: ProtocolContext). If so, that function is now in the # current scope. + + # TODO(mm, 2023-10-11): This coupling to opentrons.protocols.parse is fragile. + # Can we get the correct filename directly from proto.contents? if proto.filename and proto.filename.endswith("zip"): + # The ".zip" extension needs to match what opentrons.protocols.parse recognizes as a bundle, + # and the "protocol.ot2.py" fallback needs to match what opentrons.protocol.py sets as the + # AST filename. filename = "protocol.ot2.py" else: + # "" needs to match what opentrons.protocols.parse sets as the fallback + # AST filename. filename = proto.filename or "" + try: _runfunc_ok(new_globs.get("run")) except SyntaxError as se: diff --git a/api/src/opentrons/protocols/parse.py b/api/src/opentrons/protocols/parse.py index c757ab6d26d..ee868912ed7 100644 --- a/api/src/opentrons/protocols/parse.py +++ b/api/src/opentrons/protocols/parse.py @@ -217,11 +217,16 @@ def _parse_python( extra_labware: Optional[Dict[str, "LabwareDefinition"]] = None, ) -> PythonProtocol: """Parse a protocol known or at least suspected to be python""" - filename_checked = filename or "" - if filename_checked.endswith(".zip"): + if filename is None: + # The fallback "" needs to match what opentrons.protocols.execution.execute_python + # looks for when it extracts tracebacks. + ast_filename = "" + elif filename.endswith(".zip"): + # The extension ".zip" and the fallback "protocol.ot2.py" need to match what + # opentrons.protocols.execution.execute_python looks for when it extracts tracebacks. ast_filename = "protocol.ot2.py" else: - ast_filename = filename_checked + ast_filename = filename # todo(mm, 2021-09-13): By default, ast.parse will inherit compiler options # and future features from this module. This may not be appropriate. @@ -244,7 +249,7 @@ def _parse_python( static_info = _extract_static_python_info(parsed) protocol = compile(parsed, filename=ast_filename, mode="exec") - version = _get_version(static_info, parsed, filename_checked) + version = _get_version(static_info, parsed, ast_filename) robot_type = _robot_type_from_static_python_info(static_info) if version >= APIVersion(2, 0): @@ -257,7 +262,7 @@ def _parse_python( result = PythonProtocol( text=protocol_contents, - filename=getattr(protocol, "co_filename", ""), + filename=filename, contents=protocol, metadata=static_info.metadata, api_level=version, diff --git a/api/src/opentrons/protocols/types.py b/api/src/opentrons/protocols/types.py index 45b60c8f4ab..792951efbfa 100644 --- a/api/src/opentrons/protocols/types.py +++ b/api/src/opentrons/protocols/types.py @@ -32,11 +32,22 @@ class StaticPythonInfo: @dataclass(frozen=True) class _ProtocolCommon: text: str + filename: Optional[str] + """The original name of the main protocol file, if it had a name. + + For JSON protocols, this will be the name of the .json file. + For Python protocols, this will be the name of the .py file. + For bundled protocols, this will be the name of the .zip file. + + This can be `None` if, for example, we've parsed the protocol from an in-memory text stream. + """ + # TODO(mm, 2023-06-22): Move api_level out of _ProtocolCommon and into PythonProtocol. # JSON protocols do not have an intrinsic api_level, especially since JSONv6, # where they are no longer executed via the Python Protocol API. api_level: "APIVersion" + robot_type: RobotType diff --git a/api/src/opentrons/simulate.py b/api/src/opentrons/simulate.py index bbf3ae73f85..9670bab2d8a 100644 --- a/api/src/opentrons/simulate.py +++ b/api/src/opentrons/simulate.py @@ -4,12 +4,17 @@ a protocol from the command line. """ import argparse +import asyncio +import atexit +from contextlib import ExitStack, contextmanager import sys import logging import os import pathlib import queue from typing import ( + TYPE_CHECKING, + Generator, Any, Dict, List, @@ -22,6 +27,8 @@ ) from typing_extensions import Literal +from opentrons_shared_data.robot.dev_types import RobotType + import opentrons from opentrons import should_use_ot3 from opentrons.hardware_control import ( @@ -31,7 +38,16 @@ ) from opentrons.hardware_control.simulator_setup import load_simulator -from opentrons.protocol_api import MAX_SUPPORTED_VERSION +from opentrons.protocol_api.core.engine import ENGINE_CORE_API_VERSION +from opentrons.protocol_api.protocol_context import ProtocolContext +from opentrons.protocol_engine import create_protocol_engine +from opentrons.protocol_engine.create_protocol_engine import ( + create_protocol_engine_in_thread, +) +from opentrons.protocol_engine.state.config import Config +from opentrons.protocol_engine.types import DeckType, EngineStatus, PostRunHardwareState +from opentrons.protocol_reader.protocol_source import ProtocolSource +from opentrons.protocol_runner.protocol_runner import create_protocol_runner from opentrons.protocols.duration import DurationEstimator from opentrons.protocols.execution import execute from opentrons.legacy_broker import LegacyBroker @@ -40,31 +56,27 @@ from opentrons.commands import types as command_types from opentrons.protocols import parse, bundle -from opentrons.protocols.types import PythonProtocol, BundleContents +from opentrons.protocols.types import ( + ApiDeprecationError, + Protocol, + PythonProtocol, + BundleContents, +) from opentrons.protocols.api_support.deck_type import ( - guess_from_global_config as guess_deck_type_from_global_config, + for_simulation as deck_type_for_simulation, ) from opentrons.protocols.api_support.types import APIVersion -from opentrons_shared_data.labware.dev_types import LabwareDefinition -from opentrons_shared_data.robot.dev_types import RobotType +from opentrons_shared_data.labware.labware_definition import LabwareDefinition -from .util.entrypoint_util import ( - find_jupyter_labware, - labware_from_paths, - datafiles_from_paths, -) +from .util import entrypoint_util + +if TYPE_CHECKING: + from opentrons_shared_data.labware.dev_types import ( + LabwareDefinition as LabwareDefinitionDict, + ) # See Jira RCORE-535. -_PYTHON_TOO_NEW_MESSAGE = ( - "Python protocols with apiLevels higher than 2.13" - " cannot currently be simulated with" - " the opentrons_simulate command-line tool," - " the opentrons.simulate.simulate() function," - " or the opentrons.simulate.get_protocol_api() function." - " Use a lower apiLevel" - " or use the Opentrons App instead." -) _JSON_TOO_NEW_MESSAGE = ( "Protocols created by recent versions of Protocol Designer" " cannot currently be simulated with" @@ -74,6 +86,22 @@ ) +# When a ProtocolContext is using a ProtocolEngine to control the robot, +# it requires some long-lived resources. There's a background thread, +# an asyncio event loop in that thread, and some ProtocolEngine-controlled background tasks in that +# event loop. +# +# When we're executing a protocol file beginning-to-end, we can clean up those resources after it +# completes. However, when someone gets a live ProtocolContext through get_protocol_api(), we have +# no way of knowing when they're done with it. So, as a hack, we keep these resources open +# indefinitely, letting them leak. +# +# We keep this at module scope so that the contained context managers aren't garbage-collected. +# If they're garbage collected, they can close their resources prematurely. +# https://stackoverflow.com/a/69155026/497934 +_LIVE_PROTOCOL_ENGINE_CONTEXTS = ExitStack() + + # TODO(mm, 2023-10-05): Deduplicate this with opentrons.protocols.parse(). _UserSpecifiedRobotType = Literal["OT-2", "Flex"] """The user-facing robot type specifier. @@ -82,11 +110,17 @@ """ -class AccumulatingHandler(logging.Handler): +# TODO(mm, 2023-10-05): Type _SimulateResultRunLog more precisely by using TypedDicts from +# opentrons.commands. +_SimulateResultRunLog = List[Mapping[str, Any]] +_SimulateResult = Tuple[_SimulateResultRunLog, Optional[BundleContents]] + + +class _AccumulatingHandler(logging.Handler): def __init__( self, level: str, - command_queue: "queue.Queue[Any]", + command_queue: "queue.Queue[object]", ) -> None: """Create the handler @@ -96,82 +130,91 @@ def __init__( self._command_queue = command_queue super().__init__(level) - def emit(self, record: Any) -> None: + def emit(self, record: object) -> None: self._command_queue.put(record) -class CommandScraper: - """An object that handles scraping the broker for commands - - This should be instantiated with the logger to integrate - messages from (e.g. ``logging.getLogger('opentrons')``), the - level to scrape, and the opentrons broker object to subscribe to. - - The :py:attr:`commands` property contains the list of commands - and log messages integrated together. Each element of the list is - a dict following the pattern in the docs of :py:obj:`simulate`. - """ - +class _CommandScraper: def __init__( self, logger: logging.Logger, level: str, broker: LegacyBroker ) -> None: - """Build the scraper. + """An object that handles scraping the broker for commands and integrating log messages + with them. - :param logger: The :py:class:`logging.logger` to scrape - :param level: The log level to scrape - :param broker: Which broker to subscribe to + Params: + logger: The logger to integrate messages from, e.g. ``logging.getLogger("opentrons")``. + level: The log level to scrape. + broker: The broker to subscribe to for commands. """ self._logger = logger + self._level = level self._broker = broker - self._queue = queue.Queue() # type: ignore - if level != "none": - level = getattr(logging, level.upper(), logging.WARNING) - self._logger.setLevel(level) - self._handler: Optional[AccumulatingHandler] = AccumulatingHandler( - level, self._queue - ) - logger.addHandler(self._handler) - else: - self._handler = None - self._depth = 0 - self._commands: List[Mapping[str, Any]] = [] - self._unsub = self._broker.subscribe( - command_types.COMMAND, self._command_callback - ) + self._commands: _SimulateResultRunLog = [] @property - def commands(self) -> List[Mapping[str, Mapping[str, Any]]]: - """The list of commands. See :py:obj:`simulate`""" + def commands(self) -> _SimulateResultRunLog: + """The list of commands scraped while `.scrape()` was open, integrated with log messages. + + See :py:obj:`simulate` for the return type. + """ return self._commands - def __del__(self) -> None: - if getattr(self, "_handler", None): - try: - self._logger.removeHandler(self._handler) # type: ignore - except Exception: - pass - if hasattr(self, "_unsub"): - self._unsub() - - def _command_callback(self, message: command_types.CommandMessage) -> None: - """The callback subscribed to the broker""" - payload = message["payload"] - if message["$"] == "before": - self._commands.append( - {"level": self._depth, "payload": payload, "logs": []} + @contextmanager + def scrape(self) -> Generator[None, None, None]: + """While this context manager is open, scrape the broker for commands and integrate log + messages with them. The accumulated commands will be accessible through `.commands`. + """ + log_queue: "queue.Queue[object]" = queue.Queue() + + depth = 0 + + def handle_command(message: command_types.CommandMessage) -> None: + """The callback that we will subscribe to the broker.""" + nonlocal depth + payload = message["payload"] + if message["$"] == "before": + self._commands.append({"level": depth, "payload": payload, "logs": []}) + depth += 1 + else: + while not log_queue.empty(): + self._commands[-1]["logs"].append(log_queue.get()) + depth = max(depth - 1, 0) + + if self._level != "none": + # The simulation entry points probably leave logging unconfigured, so the level will be + # Python's default. Set it to what the user asked to make sure we see the expected + # records. + # + # TODO(mm, 2023-10-03): This is a bit too intrusive for something whose job is just to + # "scrape." The entry point function should be responsible for setting the underlying + # logger's level. + level = getattr(logging, self._level.upper(), logging.WARNING) + self._logger.setLevel(level) + + log_handler: Optional[_AccumulatingHandler] = _AccumulatingHandler( + level, log_queue ) - self._depth += 1 else: - while not self._queue.empty(): - self._commands[-1]["logs"].append(self._queue.get()) - self._depth = max(self._depth - 1, 0) + log_handler = None + + with ExitStack() as exit_stack: + if log_handler is not None: + self._logger.addHandler(log_handler) + exit_stack.callback(self._logger.removeHandler, log_handler) + + unsubscribe_from_broker = self._broker.subscribe( + command_types.COMMAND, handle_command + ) + exit_stack.callback(unsubscribe_from_broker) + + yield def get_protocol_api( version: Union[str, APIVersion], - bundled_labware: Optional[Dict[str, LabwareDefinition]] = None, + bundled_labware: Optional[Dict[str, "LabwareDefinitionDict"]] = None, bundled_data: Optional[Dict[str, bytes]] = None, - extra_labware: Optional[Dict[str, LabwareDefinition]] = None, + extra_labware: Optional[Dict[str, "LabwareDefinitionDict"]] = None, hardware_simulator: Optional[ThreadManagedHardware] = None, # Additional arguments are kw-only to make mistakes harder in environments without # type checking, like Jupyter Notebook. @@ -235,28 +278,52 @@ def get_protocol_api( # function. parsed_robot_type = parse.robot_type_from_python_identifier(robot_type) _validate_can_simulate_for_robot_type(parsed_robot_type) + deck_type = deck_type_for_simulation(parsed_robot_type) if extra_labware is None: extra_labware = { uri: details.definition - for uri, details in (find_jupyter_labware() or {}).items() + for uri, details in (entrypoint_util.find_jupyter_labware() or {}).items() } - checked_hardware = _check_hardware_simulator(hardware_simulator, parsed_robot_type) - return _build_protocol_context( - version=checked_version, - hardware_simulator=checked_hardware, - bundled_labware=bundled_labware, - bundled_data=bundled_data, - extra_labware=extra_labware, + checked_hardware = _make_hardware_simulator( + override=hardware_simulator, robot_type=parsed_robot_type ) + if checked_version < ENGINE_CORE_API_VERSION: + context = _create_live_context_non_pe( + api_version=checked_version, + deck_type=deck_type, + hardware_api=checked_hardware, + bundled_labware=bundled_labware, + bundled_data=bundled_data, + extra_labware=extra_labware, + ) + else: + if bundled_labware is not None: + # Protocol Engine has a deep assumption that standard labware definitions are always + # implicitly loadable. + raise NotImplementedError( + f"The bundled_labware argument is not currently supported for Python protocols" + f" with apiLevel {ENGINE_CORE_API_VERSION} or newer." + ) + context = _create_live_context_pe( + api_version=checked_version, + robot_type=parsed_robot_type, + deck_type=deck_type, + hardware_api=checked_hardware, + bundled_data=bundled_data, + extra_labware=extra_labware, + ) + + return context + -def _check_hardware_simulator( - hardware_simulator: Optional[ThreadManagedHardware], robot_type: RobotType +def _make_hardware_simulator( + override: Optional[ThreadManagedHardware], robot_type: RobotType ) -> ThreadManagedHardware: - if hardware_simulator: - return hardware_simulator + if override: + return override elif robot_type == "OT-3 Standard": # Local import because this isn't available on OT-2s. from opentrons.hardware_control.ot3api import OT3API @@ -266,34 +333,25 @@ def _check_hardware_simulator( return ThreadManager(OT2API.build_hardware_simulator) -def _build_protocol_context( - version: APIVersion, - hardware_simulator: ThreadManagedHardware, - bundled_labware: Optional[Dict[str, LabwareDefinition]], - bundled_data: Optional[Dict[str, bytes]], - extra_labware: Optional[Dict[str, LabwareDefinition]], -) -> protocol_api.ProtocolContext: - """Internal version of :py:meth:`get_protocol_api` that allows deferring - version specification for use with - :py:meth:`.protocol_api.execute.run_protocol` - """ - try: - context = protocol_api.create_protocol_context( - api_version=version, - hardware_api=hardware_simulator, - # FIXME(2022-12-02): Instead of guessing, - # match this to the robot type declared by the protocol. - # https://opentrons.atlassian.net/browse/RSS-156 - deck_type=guess_deck_type_from_global_config(), - bundled_labware=bundled_labware, - bundled_data=bundled_data, - extra_labware=extra_labware, - use_simulating_core=True, +@contextmanager +def _make_hardware_simulator_cm( + config_file_path: Optional[pathlib.Path], robot_type: RobotType +) -> Generator[ThreadManagedHardware, None, None]: + if config_file_path is not None: + result = ThreadManager( + load_simulator, + pathlib.Path(config_file_path), ) - except protocol_api.ProtocolEngineCoreRequiredError as e: - raise NotImplementedError(_PYTHON_TOO_NEW_MESSAGE) from e # See Jira RCORE-535. - context.home() - return context + try: + yield result + finally: + result.clean_up() + else: + result = _make_hardware_simulator(override=None, robot_type=robot_type) + try: + yield result + finally: + result.clean_up() def _get_current_robot_type() -> Optional[RobotType]: @@ -329,7 +387,7 @@ def bundle_from_sim( From a protocol, and the context that has finished simulating that protocol, determine what needs to go in a bundle for the protocol. """ - bundled_labware: Dict[str, LabwareDefinition] = {} + bundled_labware: Dict[str, "LabwareDefinitionDict"] = {} for lw in context.loaded_labwares.values(): if ( isinstance(lw, opentrons.protocol_api.labware.Labware) @@ -345,7 +403,7 @@ def bundle_from_sim( ) -def simulate( # noqa: C901 +def simulate( protocol_file: Union[BinaryIO, TextIO], file_name: Optional[str] = None, custom_labware_paths: Optional[List[str]] = None, @@ -354,7 +412,7 @@ def simulate( # noqa: C901 hardware_simulator_file_path: Optional[str] = None, duration_estimator: Optional[DurationEstimator] = None, log_level: str = "warning", -) -> Tuple[List[Mapping[str, Any]], Optional[BundleContents]]: +) -> _SimulateResult: """ Simulate the protocol itself. @@ -422,30 +480,22 @@ def simulate( # noqa: C901 """ stack_logger = logging.getLogger("opentrons") stack_logger.propagate = propagate_logs - - contents = protocol_file.read() + # _CommandScraper will set the level of this logger. # TODO(mm, 2023-10-02): Switch this truthy check to `is not None` # to match documented behavior. # See notes in https://github.com/Opentrons/opentrons/pull/13107 if custom_labware_paths: - extra_labware = labware_from_paths(custom_labware_paths) + extra_labware = entrypoint_util.labware_from_paths(custom_labware_paths) else: - extra_labware = find_jupyter_labware() or {} + extra_labware = entrypoint_util.find_jupyter_labware() or {} if custom_data_paths: - extra_data = datafiles_from_paths(custom_data_paths) + extra_data = entrypoint_util.datafiles_from_paths(custom_data_paths) else: extra_data = {} - hardware_simulator = None - - if hardware_simulator_file_path: - hardware_simulator = ThreadManager( - load_simulator, - pathlib.Path(hardware_simulator_file_path), - ) - + contents = protocol_file.read() try: protocol = parse.parse( contents, @@ -462,42 +512,43 @@ def simulate( # noqa: C901 else: raise - bundle_contents: Optional[BundleContents] = None - - # we want a None literal rather than empty dict so get_protocol_api - # will look for custom labware if this is a robot - gpa_extras = getattr(protocol, "extra_labware", None) or None - - try: - context = get_protocol_api( - getattr(protocol, "api_level", MAX_SUPPORTED_VERSION), - bundled_labware=getattr(protocol, "bundled_labware", None), - bundled_data=getattr(protocol, "bundled_data", None), - hardware_simulator=hardware_simulator, - extra_labware=gpa_extras, - robot_type="Flex" if protocol.robot_type == "OT-3 Standard" else "OT-2", - ) - except protocol_api.ProtocolEngineCoreRequiredError as e: - raise NotImplementedError(_PYTHON_TOO_NEW_MESSAGE) from e # See Jira RCORE-535. - - broker = context.broker - scraper = CommandScraper(stack_logger, log_level, broker) - if duration_estimator: - broker.subscribe(command_types.COMMAND, duration_estimator.on_message) - - try: - execute.run_protocol(protocol, context) - if ( - isinstance(protocol, PythonProtocol) - and protocol.api_level >= APIVersion(2, 0) - and protocol.bundled_labware is None - and allow_bundle() - ): - bundle_contents = bundle_from_sim(protocol, context) - finally: - context.cleanup() - - return scraper.commands, bundle_contents + if protocol.api_level < APIVersion(2, 0): + raise ApiDeprecationError(version=protocol.api_level) + + _validate_can_simulate_for_robot_type(protocol.robot_type) + + with _make_hardware_simulator_cm( + config_file_path=( + None + if hardware_simulator_file_path is None + else pathlib.Path(hardware_simulator_file_path) + ), + robot_type=protocol.robot_type, + ) as hardware_simulator: + if protocol.api_level < ENGINE_CORE_API_VERSION: + return _run_file_non_pe( + protocol=protocol, + hardware_api=hardware_simulator, + logger=stack_logger, + level=log_level, + duration_estimator=duration_estimator, + ) + else: + # TODO(mm, 2023-07-06): Once these NotImplementedErrors are resolved, consider removing + # the enclosing if-else block and running everything through _run_file_pe() for simplicity. + if custom_data_paths: + raise NotImplementedError( + f"The custom_data_paths argument is not currently supported for Python protocols" + f" with apiLevel {ENGINE_CORE_API_VERSION} or newer." + ) + protocol_file.seek(0) + return _run_file_pe( + protocol=protocol, + robot_type=protocol.robot_type, + hardware_api=hardware_simulator, + stack_logger=stack_logger, + log_level=log_level, + ) def format_runlog(runlog: List[Mapping[str, Any]]) -> str: @@ -687,6 +738,187 @@ def _get_bundle_dest( return None +def _create_live_context_non_pe( + api_version: APIVersion, + hardware_api: ThreadManagedHardware, + deck_type: str, + extra_labware: Optional[Dict[str, "LabwareDefinitionDict"]], + bundled_labware: Optional[Dict[str, "LabwareDefinitionDict"]], + bundled_data: Optional[Dict[str, bytes]], +) -> ProtocolContext: + """Return a live ProtocolContext. + + This controls the robot through the older infrastructure, instead of through Protocol Engine. + """ + assert api_version < ENGINE_CORE_API_VERSION + return protocol_api.create_protocol_context( + api_version=api_version, + deck_type=deck_type, + hardware_api=hardware_api, + bundled_labware=bundled_labware, + bundled_data=bundled_data, + extra_labware=extra_labware, + ) + + +def _create_live_context_pe( + api_version: APIVersion, + hardware_api: ThreadManagedHardware, + robot_type: RobotType, + deck_type: str, + extra_labware: Dict[str, "LabwareDefinitionDict"], + bundled_data: Optional[Dict[str, bytes]], +) -> ProtocolContext: + """Return a live ProtocolContext that controls the robot through ProtocolEngine.""" + assert api_version >= ENGINE_CORE_API_VERSION + + global _LIVE_PROTOCOL_ENGINE_CONTEXTS + pe, loop = _LIVE_PROTOCOL_ENGINE_CONTEXTS.enter_context( + create_protocol_engine_in_thread( + hardware_api=hardware_api.wrapped(), + config=_get_protocol_engine_config(robot_type), + drop_tips_after_run=False, + post_run_hardware_state=PostRunHardwareState.STAY_ENGAGED_IN_PLACE, + ) + ) + + # `async def` so we can use loop.run_coroutine_threadsafe() to wait for its completion. + # Non-async would use call_soon_threadsafe(), which makes the waiting harder. + async def add_all_extra_labware() -> None: + for labware_definition_dict in extra_labware.values(): + labware_definition = LabwareDefinition.parse_obj(labware_definition_dict) + pe.add_labware_definition(labware_definition) + + # Add extra_labware to ProtocolEngine, being careful not to modify ProtocolEngine from this + # thread. See concurrency notes in ProtocolEngine docstring. + future = asyncio.run_coroutine_threadsafe(add_all_extra_labware(), loop) + future.result() + + return protocol_api.create_protocol_context( + api_version=api_version, + hardware_api=hardware_api, + deck_type=deck_type, + protocol_engine=pe, + protocol_engine_loop=loop, + bundled_data=bundled_data, + ) + + +def _run_file_non_pe( + protocol: Protocol, + hardware_api: ThreadManagedHardware, + logger: logging.Logger, + level: str, + duration_estimator: Optional[DurationEstimator], +) -> _SimulateResult: + """Run a protocol file without Protocol Engine, with the older infrastructure instead.""" + if isinstance(protocol, PythonProtocol): + extra_labware = protocol.extra_labware + bundled_labware = protocol.bundled_labware + bundled_data = protocol.bundled_data + else: + # JSON protocols do have "bundled labware" embedded in them, but those aren't represented in + # the parsed Protocol object and we don't need to create the ProtocolContext with them. + # execute_apiv2.run_protocol() will pull them out of the JSON and load them into the + # ProtocolContext. + extra_labware = None + bundled_labware = None + bundled_data = None + + context = _create_live_context_non_pe( + api_version=protocol.api_level, + hardware_api=hardware_api, + deck_type=deck_type_for_simulation(protocol.robot_type), + extra_labware=extra_labware, + bundled_labware=bundled_labware, + bundled_data=bundled_data, + ) + + scraper = _CommandScraper(logger=logger, level=level, broker=context.broker) + if duration_estimator: + context.broker.subscribe(command_types.COMMAND, duration_estimator.on_message) + + context.home() + with scraper.scrape(): + try: + execute.run_protocol(protocol, context) + if ( + isinstance(protocol, PythonProtocol) + and protocol.api_level >= APIVersion(2, 0) + and protocol.bundled_labware is None + and allow_bundle() + ): + bundle_contents: Optional[BundleContents] = bundle_from_sim( + protocol, context + ) + else: + bundle_contents = None + + finally: + context.cleanup() + + return scraper.commands, bundle_contents + + +def _run_file_pe( + protocol: Protocol, + robot_type: RobotType, + hardware_api: ThreadManagedHardware, + stack_logger: logging.Logger, + log_level: str, +) -> _SimulateResult: + """Run a protocol file with Protocol Engine.""" + + async def run(protocol_source: ProtocolSource) -> _SimulateResult: + protocol_engine = await create_protocol_engine( + hardware_api=hardware_api.wrapped(), + config=_get_protocol_engine_config(robot_type), + ) + + protocol_runner = create_protocol_runner( + protocol_config=protocol_source.config, + protocol_engine=protocol_engine, + hardware_api=hardware_api.wrapped(), + ) + + scraper = _CommandScraper(stack_logger, log_level, protocol_runner.broker) + with scraper.scrape(): + result = await protocol_runner.run(protocol_source) + + if result.state_summary.status != EngineStatus.SUCCEEDED: + raise entrypoint_util.ProtocolEngineExecuteError( + result.state_summary.errors + ) + + # We don't currently support returning bundle contents from protocols run through + # Protocol Engine. To get them, bundle_from_sim() requires direct access to the + # ProtocolContext, which opentrons.protocol_runner does not grant us. + bundle_contents = None + + return scraper.commands, bundle_contents + + with entrypoint_util.adapt_protocol_source(protocol) as protocol_source: + return asyncio.run(run(protocol_source)) + + +def _get_protocol_engine_config(robot_type: RobotType) -> Config: + """Return a Protocol Engine config to execute protocols on this device.""" + return Config( + robot_type=robot_type, + deck_type=DeckType(deck_type_for_simulation(robot_type)), + ignore_pause=True, + use_virtual_pipettes=True, + use_virtual_modules=True, + use_virtual_gripper=True, + ) + + +@atexit.register +def _clear_live_protocol_engine_contexts() -> None: + global _LIVE_PROTOCOL_ENGINE_CONTEXTS + _LIVE_PROTOCOL_ENGINE_CONTEXTS.close() + + # Note - this script is also set up as a setuptools entrypoint and thus does # an absolute minimum of work since setuptools does something odd generating # the scripts @@ -703,15 +935,21 @@ def main() -> int: # TODO(mm, 2022-12-01): Configure the DurationEstimator with the correct deck type. duration_estimator = DurationEstimator() if args.estimate_duration else None # type: ignore[no-untyped-call] - runlog, maybe_bundle = simulate( - protocol_file=args.protocol, - file_name=args.protocol.name, - custom_labware_paths=args.custom_labware_path, - custom_data_paths=(args.custom_data_path + args.custom_data_file), - duration_estimator=duration_estimator, - hardware_simulator_file_path=getattr(args, "custom_hardware_simulator_file"), - log_level=args.log_level, - ) + try: + runlog, maybe_bundle = simulate( + protocol_file=args.protocol, + file_name=args.protocol.name, + custom_labware_paths=args.custom_labware_path, + custom_data_paths=(args.custom_data_path + args.custom_data_file), + duration_estimator=duration_estimator, + hardware_simulator_file_path=getattr( + args, "custom_hardware_simulator_file" + ), + log_level=args.log_level, + ) + except entrypoint_util.ProtocolEngineExecuteError as error: + print(error.to_stderr_string(), file=sys.stderr) + return 1 if maybe_bundle: bundle_name = getattr(args, "bundle", None) diff --git a/api/src/opentrons/util/entrypoint_util.py b/api/src/opentrons/util/entrypoint_util.py index eb2d03bc629..442b0686ebe 100644 --- a/api/src/opentrons/util/entrypoint_util.py +++ b/api/src/opentrons/util/entrypoint_util.py @@ -1,18 +1,34 @@ """ opentrons.util.entrypoint_util: functions common to entrypoints """ +import asyncio +import contextlib from dataclasses import dataclass +import json import logging from json import JSONDecodeError import pathlib -import shutil -from typing import BinaryIO, Dict, Sequence, TextIO, Optional, Union, TYPE_CHECKING +import tempfile +from typing import ( + Dict, + Generator, + List, + Optional, + Sequence, + Union, + TYPE_CHECKING, +) from jsonschema import ValidationError # type: ignore from opentrons.config import IS_ROBOT, JUPYTER_NOTEBOOK_LABWARE_DIR from opentrons.protocol_api import labware from opentrons.calibration_storage import helpers +from opentrons.protocol_engine.errors.error_occurrence import ( + ErrorOccurrence as ProtocolEngineErrorOccurrence, +) +from opentrons.protocol_reader import ProtocolReader, ProtocolSource +from opentrons.protocols.types import JsonProtocol, Protocol, PythonProtocol if TYPE_CHECKING: from opentrons_shared_data.labware.dev_types import LabwareDefinition @@ -107,35 +123,96 @@ def datafiles_from_paths(paths: Sequence[Union[str, pathlib.Path]]) -> Dict[str, return datafiles -# HACK(mm, 2023-06-29): This function is attempting to do something fundamentally wrong. -# Remove it when we fix https://opentrons.atlassian.net/browse/RSS-281. -def copy_file_like(source: Union[BinaryIO, TextIO], destination: pathlib.Path) -> None: - """Copy a file-like object to a path. +@contextlib.contextmanager +def adapt_protocol_source(protocol: Protocol) -> Generator[ProtocolSource, None, None]: + """Convert a `Protocol` to a `ProtocolSource`. - Limitations: - If `source` is text, the new file's encoding may not correctly match its original encoding. - This can matter if it's a Python file and it has an encoding declaration - (https://docs.python.org/3.7/reference/lexical_analysis.html#encoding-declarations). - Also, its newlines may get translated. - """ - # When we read from the source stream, will it give us bytes, or text? - try: - # Experimentally, this is present (but possibly None) on text-mode streams, - # and not present on binary-mode streams. - getattr(source, "encoding") - except AttributeError: - source_is_text = False - else: - source_is_text = True + `Protocol` and `ProtocolSource` do basically the same thing. `Protocol` is the traditional + interface. `ProtocolSource` is the newer, but not necessarily better, interface that's required + to run stuff through Protocol Engine. Ideally, the two would be unified. Until then, we have + this shim. - if source_is_text: - destination_mode = "wt" + This is a context manager because it needs to keep some temp files around. + """ + # ProtocolReader needs to know the filename of the main protocol file so it can infer from its + # extension whether it's a JSON or Python protocol. But that filename doesn't necessarily exist, + # like when a user passes a text stream to opentrons.simulate.simulate(). As a hack, work + # backwards and synthesize a dummy filename with the correct extension. + if protocol.filename is not None: + # We were given a filename, so no need to guess. + # + # It's not well-defined in our customer-facing interfaces whether the supplied protocol_name + # should be just the filename part, or a path with separators. In case it contains stuff + # like "../", sanitize it to just the filename part so we don't save files somewhere bad. + main_file_name = pathlib.Path(protocol.filename).name + elif isinstance(protocol, JsonProtocol): + main_file_name = "protocol.json" else: - destination_mode = "wb" - - with open( - destination, - mode=destination_mode, - ) as destination_file: - # Use copyfileobj() to limit memory usage. - shutil.copyfileobj(fsrc=source, fdst=destination_file) + main_file_name = "protocol.py" + + with tempfile.TemporaryDirectory() as temporary_directory: + # FIXME(mm, 2023-06-26): Copying these files is pure overhead, and it introduces encoding + # hazards. Remove this when we can parse JSONv6+ and PAPIv2.14+ protocols without going + # through the filesystem. https://opentrons.atlassian.net/browse/RSS-281 + + main_file = pathlib.Path(temporary_directory) / main_file_name + main_file.write_text(protocol.text, encoding="utf-8") + + labware_files: List[pathlib.Path] = [] + if isinstance(protocol, PythonProtocol) and protocol.extra_labware is not None: + for labware_index, labware_definition in enumerate( + protocol.extra_labware.values() + ): + new_labware_file = ( + pathlib.Path(temporary_directory) / f"{labware_index}.json" + ) + new_labware_file.write_text( + json.dumps(labware_definition), encoding="utf-8" + ) + labware_files.append(new_labware_file) + + protocol_source = asyncio.run( + ProtocolReader().read_saved( + files=[main_file] + labware_files, + directory=None, + files_are_prevalidated=False, + ) + ) + + yield protocol_source + + +class ProtocolEngineExecuteError(Exception): + def __init__(self, errors: List[ProtocolEngineErrorOccurrence]) -> None: + """Raised when there was any fatal error running a protocol through Protocol Engine. + + Protocol Engine reports errors as data, not as exceptions. + But the only way for `opentrons.execute.execute()` and `opentrons.simulate.simulate()` + to signal problems to their callers is to raise something. + So we need this class to wrap them. + + Params: + errors: The errors that Protocol Engine reported. + """ + # Show the full error details if this is part of a traceback. Don't try to summarize. + super().__init__(errors) + + self._error_occurrences = errors + + def to_stderr_string(self) -> str: + """Return a string suitable as the stderr output of the `opentrons_execute` CLI. + + This summarizes from the full error details. + """ + # It's unclear what exactly we should extract here. + # + # First, do we print the first element, or the last, or all of them? + # + # Second, do we print the .detail? .errorCode? .errorInfo? .wrappedErrors? + # By contract, .detail seems like it would be insufficient, but experimentally, + # it includes a lot, like: + # + # ProtocolEngineError [line 3]: Error 4000 GENERAL_ERROR (ProtocolEngineError): + # UnexpectedProtocolError: Labware "fixture_12_trough" not found with version 1 + # in namespace "fixture". + return self._error_occurrences[0].detail diff --git a/api/tests/opentrons/conftest.py b/api/tests/opentrons/conftest.py index b25eb9049f7..e1d45591f4e 100755 --- a/api/tests/opentrons/conftest.py +++ b/api/tests/opentrons/conftest.py @@ -129,16 +129,11 @@ def protocol_file() -> str: @pytest.fixture() def protocol(protocol_file: str) -> Generator[Protocol, None, None]: - root = protocol_file - filename = os.path.join(os.path.dirname(__file__), "data", root) - - file = open(filename) - text = "".join(list(file)) - file.seek(0) - - yield Protocol(text=text, filename=filename, filelike=file) - - file.close() + filename = os.path.join(os.path.dirname(__file__), "data", protocol_file) + with open(filename, encoding="utf-8") as file: + text = file.read() + file.seek(0) + yield Protocol(text=text, filename=filename, filelike=file) @pytest.fixture() diff --git a/api/tests/opentrons/protocols/test_parse.py b/api/tests/opentrons/protocols/test_parse.py index 54899de6117..cc86621601a 100644 --- a/api/tests/opentrons/protocols/test_parse.py +++ b/api/tests/opentrons/protocols/test_parse.py @@ -426,14 +426,19 @@ def test_parse_python_details( assert parsed.text == protocol_source assert isinstance(parsed.text, str) - fname = filename if filename is not None else "" - - assert parsed.filename == fname + assert parsed.filename == filename + assert parsed.contents.co_filename == ( + filename if filename is not None else "" + ) assert parsed.api_level == expected_api_level assert expected_robot_type == expected_robot_type assert parsed.metadata == expected_metadata - assert parsed.contents == compile(protocol_source, filename=fname, mode="exec") + assert parsed.contents == compile( + protocol_source, + filename="", + mode="exec", + ) @pytest.mark.parametrize( @@ -481,7 +486,7 @@ def test_parse_bundle_details(get_bundle_fixture: Callable[..., Any]) -> None: parsed = parse(fixture["binary_zipfile"], filename) assert isinstance(parsed, PythonProtocol) - assert parsed.filename == "protocol.ot2.py" + assert parsed.filename == filename assert parsed.bundled_labware == fixture["bundled_labware"] assert parsed.bundled_python == fixture["bundled_python"] assert parsed.bundled_data == fixture["bundled_data"] diff --git a/api/tests/opentrons/test_simulate.py b/api/tests/opentrons/test_simulate.py index 4336df3b116..e6c5920e520 100644 --- a/api/tests/opentrons/test_simulate.py +++ b/api/tests/opentrons/test_simulate.py @@ -5,13 +5,14 @@ import json import textwrap from pathlib import Path -from typing import TYPE_CHECKING, Callable, Generator, TextIO, cast +from typing import TYPE_CHECKING, Callable, Generator, List, TextIO, cast import pytest from opentrons_shared_data import get_shared_data_root, load_shared_data from opentrons import simulate, protocols +from opentrons.protocol_api.core.engine import ENGINE_CORE_API_VERSION from opentrons.protocols.types import ApiDeprecationError from opentrons.protocols.api_support.types import APIVersion from opentrons.protocols.execution.errors import ExceptionInProtocolError @@ -24,13 +25,7 @@ HERE = Path(__file__).parent -@pytest.fixture( - params=[ - APIVersion(2, 0), - # TODO(mm, 2023-07-14): Enable this for https://opentrons.atlassian.net/browse/RSS-268. - # ENGINE_CORE_API_VERSION, - ] -) +@pytest.fixture(params=[APIVersion(2, 0), ENGINE_CORE_API_VERSION]) def api_version(request: pytest.FixtureRequest) -> APIVersion: """Return an API version to test with. @@ -44,28 +39,68 @@ def api_version(request: pytest.FixtureRequest) -> APIVersion: "protocol_file", [ "testosaur_v2.py", - # TODO(mm, 2023-07-14): Resolve this xfail. https://opentrons.atlassian.net/browse/RSS-268 pytest.param( "testosaur_v2_14.py", - marks=pytest.mark.xfail(strict=True, raises=NotImplementedError), + marks=pytest.mark.xfail( + strict=True, + reason=( + "We can't currently get bundle contents" + " from protocols run through Protocol Engine." + ), + ), ), ], ) -def test_simulate_function_apiv2( +def test_simulate_function_apiv2_bundle( protocol: Protocol, protocol_file: str, monkeypatch: pytest.MonkeyPatch, ) -> None: - """Test `simulate()` with a Python file.""" + """Test that `simulate()` returns the expected bundle contents from a Python file.""" monkeypatch.setenv("OT_API_FF_allowBundleCreation", "1") - runlog, bundle = simulate.simulate(protocol.filelike, protocol.filename) - assert isinstance(bundle, protocols.types.BundleContents) - assert [item["payload"]["text"] for item in runlog] == [ - "Picking up tip from A1 of Opentrons 96 Tip Rack 1000 µL on 1", - "Aspirating 100.0 uL from A1 of Corning 96 Well Plate 360 µL Flat on 2 at 500.0 uL/sec", - "Dispensing 100.0 uL into B1 of Corning 96 Well Plate 360 µL Flat on 2 at 1000.0 uL/sec", - "Dropping tip into H12 of Opentrons 96 Tip Rack 1000 µL on 1", - ] + _, bundle_contents = simulate.simulate(protocol.filelike, protocol.filename) + assert isinstance(bundle_contents, protocols.types.BundleContents) + + +@pytest.mark.parametrize("protocol_file", ["testosaur_v2.py", "testosaur_v2_14.py"]) +def test_simulate_without_filename(protocol: Protocol, protocol_file: str) -> None: + """`simulate()` should accept a protocol without a filename.""" + simulate.simulate(protocol.filelike) # Should not raise. + + +@pytest.mark.parametrize( + ("protocol_file", "expected_entries"), + [ + ( + "testosaur_v2.py", + [ + "Picking up tip from A1 of Opentrons 96 Tip Rack 1000 µL on 1", + "Aspirating 100.0 uL from A1 of Corning 96 Well Plate 360 µL Flat on 2 at 500.0 uL/sec", + "Dispensing 100.0 uL into B1 of Corning 96 Well Plate 360 µL Flat on 2 at 1000.0 uL/sec", + "Dropping tip into H12 of Opentrons 96 Tip Rack 1000 µL on 1", + ], + ), + ( + "testosaur_v2_14.py", + # FIXME(2023-10-04): This run log is wrong. It should match the one above. + # https://opentrons.atlassian.net/browse/RSS-368 + [ + "Picking up tip from A1 of None", + "Aspirating 100.0 uL from A1 of None at 500.0 uL/sec", + "Dispensing 100.0 uL into B1 of None at 1000.0 uL/sec", + "Dropping tip into H12 of None", + ], + ), + ], +) +def test_simulate_function_apiv2_run_log( + protocol: Protocol, + protocol_file: str, + expected_entries: List[str], +) -> None: + """Test that `simulate()` returns the expected run log from a Python file.""" + run_log, _ = simulate.simulate(protocol.filelike, protocol.filename) + assert [item["payload"]["text"] for item in run_log] == expected_entries def test_simulate_function_json( diff --git a/api/tests/opentrons/util/test_entrypoint_util.py b/api/tests/opentrons/util/test_entrypoint_util.py index 4e82ce0e80f..c30351dec3b 100644 --- a/api/tests/opentrons/util/test_entrypoint_util.py +++ b/api/tests/opentrons/util/test_entrypoint_util.py @@ -1,17 +1,13 @@ -import io import json import os from pathlib import Path from typing import Callable -import pytest - from opentrons_shared_data.labware.dev_types import LabwareDefinition as LabwareDefDict from opentrons.util.entrypoint_util import ( FoundLabware, labware_from_paths, datafiles_from_paths, - copy_file_like, ) @@ -79,70 +75,3 @@ def test_datafiles_from_paths(tmp_path: Path) -> None: "test1": "wait theres a second file???".encode(), "test-file": "this isnt even in a directory".encode(), } - - -class TestCopyFileLike: - """Tests for `copy_file_like()`.""" - - @pytest.fixture(params=["abc", "µ"]) - def source_text(self, request: pytest.FixtureRequest) -> str: - return request.param # type: ignore[attr-defined,no-any-return] - - @pytest.fixture - def source_bytes(self, source_text: str) -> bytes: - return b"\x00\x01\x02\x03\x04" - - @pytest.fixture - def source_path(self, tmp_path: Path) -> Path: - return tmp_path / "source" - - @pytest.fixture - def destination_path(self, tmp_path: Path) -> Path: - return tmp_path / "destination" - - def test_from_text_file( - self, - source_text: str, - source_path: Path, - destination_path: Path, - ) -> None: - """Test that it correctly copies from a text-mode `open()`.""" - source_path.write_text(source_text) - - with open( - source_path, - mode="rt", - ) as source_file: - copy_file_like(source=source_file, destination=destination_path) - - assert destination_path.read_text() == source_text - - def test_from_binary_file( - self, - source_bytes: bytes, - source_path: Path, - destination_path: Path, - ) -> None: - """Test that it correctly copies from a binary-mode `open()`.""" - source_path.write_bytes(source_bytes) - - with open(source_path, mode="rb") as source_file: - copy_file_like(source=source_file, destination=destination_path) - - assert destination_path.read_bytes() == source_bytes - - def test_from_stringio(self, source_text: str, destination_path: Path) -> None: - """Test that it correctly copies from an `io.StringIO`.""" - stringio = io.StringIO(source_text) - - copy_file_like(source=stringio, destination=destination_path) - - assert destination_path.read_text() == source_text - - def test_from_bytesio(self, source_bytes: bytes, destination_path: Path) -> None: - """Test that it correctly copies from an `io.BytesIO`.""" - bytesio = io.BytesIO(source_bytes) - - copy_file_like(source=bytesio, destination=destination_path) - - assert destination_path.read_bytes() == source_bytes