Fix E-stopped runs being marked as stopped instead of failed.

We permanently set the run result to `FAILED` or `STOPPED` when a stop is requested, not just when the engine is `.finish()`'d. This seems premature to me, but, taking it for granted, it means we need to make sure that we choose `FAILED` if the stop was requested because of an E-stop. Also, because of that premature setting of the run result, we need to make sure that when the E-stop-induced `FinishAction` comes in, the run result already being set doesn't stop it from setting the run error.
Opentrons · Apr 19, 2024 · bec9461 · bec9461
1 parent e7c936e
commit bec9461
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 10 deletions.
diff --git a/api/src/opentrons/protocol_engine/state/commands.py b/api/src/opentrons/protocol_engine/state/commands.py
@@ -345,9 +345,11 @@ def handle_action(self, action: Action) -> None:  # noqa: C901
         elif isinstance(action, StopAction):
             if not self._state.run_result:
                 self._state.queue_status = QueueStatus.PAUSED
-                self._state.run_result = RunResult.STOPPED
                 if action.from_estop:
                     self._state.stopped_by_estop = True
+                    self._state.run_result = RunResult.FAILED
+                else:
+                    self._state.run_result = RunResult.STOPPED
 
         elif isinstance(action, FinishAction):
             if not self._state.run_result:
@@ -361,12 +363,12 @@ def handle_action(self, action: Action) -> None:  # noqa: C901
                 else:
                     self._state.run_result = RunResult.STOPPED
 
-                if action.error_details:
-                    self._state.run_error = self._map_run_exception_to_error_occurrence(
-                        action.error_details.error_id,
-                        action.error_details.created_at,
-                        action.error_details.error,
-                    )
+            if not self._state.run_error and action.error_details:
+                self._state.run_error = self._map_run_exception_to_error_occurrence(
+                    action.error_details.error_id,
+                    action.error_details.created_at,
+                    action.error_details.error,
+                )
 
         elif isinstance(action, HardwareStoppedAction):
             self._state.queue_status = QueueStatus.PAUSED

diff --git a/api/tests/opentrons/protocol_engine/state/test_command_store_old.py b/api/tests/opentrons/protocol_engine/state/test_command_store_old.py
@@ -600,8 +600,13 @@ def test_command_store_handles_finish_action_with_stopped() -> None:
     assert subject.state.run_result == RunResult.STOPPED
 
 
-@pytest.mark.parametrize("from_estop", [True, False])
-def test_command_store_handles_stop_action(from_estop: bool) -> None:
+@pytest.mark.parametrize(
+    ["from_estop", "expected_run_result"],
+    [(True, RunResult.FAILED), (False, RunResult.STOPPED)],
+)
+def test_command_store_handles_stop_action(
+    from_estop: bool, expected_run_result: RunResult
+) -> None:
     """It should mark the engine as non-gracefully stopped on StopAction."""
     subject = CommandStore(is_door_open=False, config=_make_config())
 
@@ -615,7 +620,7 @@ def test_command_store_handles_stop_action(from_estop: bool) -> None:
     assert subject.state == CommandState(
         command_history=CommandHistory(),
         queue_status=QueueStatus.PAUSED,
-        run_result=RunResult.STOPPED,
+        run_result=expected_run_result,
         run_completed_at=None,
         is_door_blocking=False,
         run_error=None,