replace flaky integration test with stable unit tests

robot-server/robot_server/protocols/analysis_store.py

@@ -325,6 +325,9 @@ async def matching_rtp_values_in_analysis(
                 analysis_summary.id
             )
         )
+        # We already make sure that the protocol has an analysis associated with before
+        # checking the RTP values so this assert should never raise.
+        # It is only added for type checking.
         assert (
             rtp_values_and_defaults_in_last_analysis is not None
         ), "This protocol has no analysis associated with it."

robot-server/tests/integration/http_api/protocols/test_analyses.tavern.yaml

@@ -26,22 +26,6 @@ stages:
             - id: !anystr
               status: pending
 
-  - name: Upload a protocol
-    request:
-      url: '{ot2_server_base_url}/protocols'
-      method: POST
-      files:
-        files: 'tests/integration/protocols/basic_transfer_standalone.py'
-    response:
-      strict:
-        - json:off
-      status_code: 503
-      json:
-        errors:
-          - id: "LastAnalysisPending"
-            title: "Last Analysis Still Pending."
-            errorCode: '4000'
-
   - name: Check that the analysis summary is present in /protocols/:id; retry until it says it's completed
     max_retries: 5
     delay_after: 1

robot-server/tests/protocols/test_analysis_store.py

@@ -35,6 +35,7 @@
 from robot_server.protocols.analysis_store import (
     AnalysisStore,
     AnalysisNotFoundError,
+    AnalysisIsPendingError,
     _CURRENT_ANALYZER_VERSION,
 )
 from robot_server.protocols.completed_analysis_store import (
@@ -475,3 +476,13 @@ async def test_matching_default_rtp_values_in_analysis_with_no_client_rtp_values
         )
         is True
     )
+
+
+async def test_matching_default_rtp_values_in_analysis_with_pending_analysis(
+    subject: AnalysisStore, protocol_store: ProtocolStore
+) -> None:
+    """It should raise an error if analysis is pending."""
+    with pytest.raises(AnalysisIsPendingError):
+        await subject.matching_rtp_values_in_analysis(
+            AnalysisSummary(id="analysis-id", status=AnalysisStatus.PENDING), {}
+        )

robot-server/tests/protocols/test_protocols_router.py

@@ -1,5 +1,6 @@
 """Tests for the /protocols router."""
 import io
+
 import pytest
 from datetime import datetime
 from decoy import Decoy, matchers
@@ -24,7 +25,11 @@
 from robot_server.errors.error_responses import ApiError
 from robot_server.service.json_api import SimpleEmptyBody, MultiBodyMeta
 from robot_server.service.task_runner import TaskRunner
-from robot_server.protocols.analysis_store import AnalysisStore, AnalysisNotFoundError
+from robot_server.protocols.analysis_store import (
+    AnalysisStore,
+    AnalysisNotFoundError,
+    AnalysisIsPendingError,
+)
 from robot_server.protocols.protocol_analyzer import ProtocolAnalyzer
 from robot_server.protocols.protocol_auto_deleter import ProtocolAutoDeleter
 from robot_server.protocols.analysis_models import (
@@ -977,6 +982,101 @@ async def test_create_existing_protocol_with_same_run_time_params(
     )
 
 
+async def test_create_existing_protocol_with_pending_analysis_raises(
+    decoy: Decoy,
+    protocol_store: ProtocolStore,
+    analysis_store: AnalysisStore,
+    protocol_reader: ProtocolReader,
+    file_reader_writer: FileReaderWriter,
+    file_hasher: FileHasher,
+    protocol_analyzer: ProtocolAnalyzer,
+    task_runner: TaskRunner,
+    protocol_auto_deleter: ProtocolAutoDeleter,
+) -> None:
+    """It should raise an error if protocol has existing pending analysis."""
+    protocol_directory = Path("/dev/null")
+    content = bytes("some_content", encoding="utf-8")
+    uploaded_file = io.BytesIO(content)
+
+    protocol_file = UploadFile(filename="foo.json", file=uploaded_file)
+    buffered_file = BufferedFile(name="blah", contents=content, path=None)
+
+    protocol_source = ProtocolSource(
+        directory=Path("/dev/null"),
+        main_file=Path("/dev/null/foo.json"),
+        files=[
+            ProtocolSourceFile(
+                path=Path("/dev/null/foo.json"),
+                role=ProtocolFileRole.MAIN,
+            )
+        ],
+        metadata={"this_is_fake_metadata": True},
+        robot_type="OT-2 Standard",
+        config=JsonProtocolConfig(schema_version=123),
+        content_hash="a_b_c",
+    )
+
+    stored_protocol_resource = ProtocolResource(
+        protocol_id="protocol-id",
+        created_at=datetime(year=2020, month=1, day=1),
+        source=protocol_source,
+        protocol_key="dummy-key-222",
+    )
+
+    analysis_summaries = [
+        AnalysisSummary(
+            id="analysis-id",
+            status=AnalysisStatus.PENDING,
+        ),
+    ]
+
+    decoy.when(
+        await file_reader_writer.read(
+            # TODO(mm, 2024-02-07): Recent FastAPI upgrades mean protocol_file.filename
+            # is typed as possibly None. Investigate whether that can actually happen in
+            # practice and whether we need to account for it.
+            files=[protocol_file]  # type: ignore[list-item]
+        )
+    ).then_return([buffered_file])
+
+    decoy.when(await file_hasher.hash(files=[buffered_file])).then_return("a_b_c")
+    decoy.when(protocol_store.get_id_by_hash("a_b_c")).then_return("the-og-proto-id")
+    decoy.when(protocol_store.get(protocol_id="the-og-proto-id")).then_return(
+        stored_protocol_resource
+    )
+    decoy.when(
+        analysis_store.get_summaries_by_protocol(protocol_id="the-og-proto-id")
+    ).then_return(analysis_summaries)
+    decoy.when(
+        await analysis_store.matching_rtp_values_in_analysis(
+            analysis_summaries[-1], {"vol": 123, "dry_run": True, "mount": "left"}
+        )
+    ).then_raise(AnalysisIsPendingError("a-id"))
+
+    with pytest.raises(ApiError) as exc_info:
+        await create_protocol(
+            files=[protocol_file],
+            key="dummy-key-111",
+            run_time_parameter_values='{"vol": 123, "dry_run": true, "mount": "left"}',
+            protocol_directory=protocol_directory,
+            protocol_store=protocol_store,
+            analysis_store=analysis_store,
+            file_reader_writer=file_reader_writer,
+            protocol_reader=protocol_reader,
+            file_hasher=file_hasher,
+            protocol_analyzer=protocol_analyzer,
+            task_runner=task_runner,
+            protocol_auto_deleter=protocol_auto_deleter,
+            robot_type="OT-2 Standard",
+            protocol_id="protocol-id",
+            analysis_id="analysis-id",
+            created_at=datetime(year=2021, month=1, day=1),
+        )
+
+    assert exc_info.value.status_code == 503
+    assert exc_info.value.content["errors"][0]["id"] == "LastAnalysisPending"
+
+
 async def test_create_protocol_not_readable(
     decoy: Decoy,
     file_reader_writer: FileReaderWriter,