test(robot-server): Deduplicate and tidy up integration test fixtures

robot-server/pytest.ini

@@ -2,6 +2,5 @@
 markers =
         ot2_only: Test only functions using the OT2 hardware
         ot3_only: Test only functions using the OT3 hardware
-tavern-global-cfg = tests/integration/common.yaml
 addopts = --color=yes --strict-markers
 asyncio_mode = auto

robot-server/tests/conftest.py

@@ -1,17 +1,11 @@
-import asyncio
-import signal
-import subprocess
-import time
-import sys
 import json
 import os
 import pathlib
-import requests
 import tempfile
 from datetime import datetime, timezone
 from mock import MagicMock
 from pathlib import Path
-from typing import Any, Callable, Generator, Iterator, cast
+from typing import Callable, Generator, Iterator, cast
 from typing_extensions import NoReturn
 from decoy import Decoy
 
@@ -40,7 +34,6 @@
 from robot_server.versioning import API_VERSION_HEADER, LATEST_API_VERSION_HEADER_VALUE
 from robot_server.service.session.manager import SessionManager
 from robot_server.persistence import get_sql_engine, create_sql_engine
-from .integration.robot_client import RobotClient
 from robot_server.health.router import ComponentVersions, get_versions
 
 test_router = routing.APIRouter()
@@ -176,111 +169,6 @@ def server_temp_directory() -> Iterator[str]:
     yield new_dir
 
 
-@pytest.fixture()
-def clean_server_state() -> Iterator[None]:
-    # async fn that does the things below
-    # make a robot client
-    # delete protocols
-    async def _clean_server_state() -> None:
-        port = "31950"
-        async with RobotClient.make(
-            host="http://localhost", port=port, version="*"
-        ) as robot_client:
-            await _delete_all_runs(robot_client)
-            await _delete_all_protocols(robot_client)
-
-    yield
-    asyncio.run(_clean_server_state())
-
-
-# TODO(jbl 2023-05-01) merge this with ot3_run_server, along with clean_server_state and run_server
-@pytest.fixture()
-def ot3_clean_server_state() -> Iterator[None]:
-    # async fn that does the things below
-    # make a robot client
-    # delete protocols
-    async def _clean_server_state() -> None:
-        port = "31960"
-        async with RobotClient.make(
-            host="http://localhost", port=port, version="*"
-        ) as robot_client:
-            await _delete_all_runs(robot_client)
-            await _delete_all_protocols(robot_client)
-
-    yield
-    asyncio.run(_clean_server_state())
-
-
-@pytest.fixture(scope="session")
-def run_server(
-    request_session: requests.Session, server_temp_directory: str
-) -> Iterator["subprocess.Popen[Any]"]:
-    """Run the robot server in a background process."""
-    # In order to collect coverage we run using `coverage`.
-    # `-a` is to append to existing `.coverage` file.
-    # `--source` is the source code folder to collect coverage stats on.
-    with subprocess.Popen(
-        [
-            sys.executable,
-            "-m",
-            "coverage",
-            "run",
-            "-a",
-            "--source",
-            "robot_server",
-            "-m",
-            "uvicorn",
-            "robot_server:app",
-            "--host",
-            "localhost",
-            "--port",
-            "31950",
-        ],
-        env={
-            "OT_ROBOT_SERVER_DOT_ENV_PATH": "dev.env",
-            "OT_API_CONFIG_DIR": server_temp_directory,
-        },
-        stdin=subprocess.DEVNULL,
-        # The server will log to its stdout or stderr.
-        # Let it inherit our stdout and stderr so pytest captures its logs.
-        stdout=None,
-        stderr=None,
-    ) as proc:
-        # Wait for a bit to get started by polling /hcpealth
-        from requests.exceptions import ConnectionError
-
-        while True:
-            try:
-                request_session.get("http://localhost:31950/health")
-            except ConnectionError:
-                pass
-            else:
-                break
-            time.sleep(0.5)
-        request_session.post(
-            "http://localhost:31950/robot/home", json={"target": "robot"}
-        )
-        yield proc
-        proc.send_signal(signal.SIGTERM)
-        proc.wait()
-
-
-async def _delete_all_runs(robot_client: RobotClient) -> None:
-    """Delete all runs on the robot server."""
-    response = await robot_client.get_runs()
-    run_ids = [r["id"] for r in response.json()["data"]]
-    for run_id in run_ids:
-        await robot_client.delete_run(run_id)
-
-
-async def _delete_all_protocols(robot_client: RobotClient) -> None:
-    """Delete all protocols on the robot server"""
-    response = await robot_client.get_protocols()
-    protocol_ids = [p["id"] for p in response.json()["data"]]
-    for protocol_id in protocol_ids:
-        await robot_client.delete_protocol(protocol_id)
-
-
 @pytest.fixture
 def attach_pipettes(server_temp_directory: str) -> Iterator[None]:
     import json

robot-server/tests/integration/conftest.py

@@ -1,19 +1,22 @@
+import asyncio
+import contextlib
 import json
 import time
 from pathlib import Path
-from typing import Any, Dict, Iterator
+from typing import Any, Dict, Generator
 
 import pytest
 import requests
 
 from robot_server.versioning import API_VERSION_HEADER, LATEST_API_VERSION_HEADER_VALUE
 
 from .dev_server import DevServer
+from .robot_client import RobotClient
 
 
-# Must match our Tavern config in common.yaml.
-_SESSION_SERVER_HOST = "http://localhost"
-_SESSION_SERVER_PORT = "31950"
+_SESSION_SERVER_SCHEME = "http://"
+_SESSION_SERVER_HOST = "localhost"
+_OT2_SESSION_SERVER_PORT = "31950"
 _OT3_SESSION_SERVER_PORT = "31960"
 
 
@@ -35,71 +38,82 @@ def pytest_tavern_beta_after_every_response(
     print(json.dumps(response.json(), indent=4))
 
 
-@pytest.fixture(scope="session")
-def request_session() -> requests.Session:
-    session = requests.Session()
-    session.headers.update({API_VERSION_HEADER: LATEST_API_VERSION_HEADER_VALUE})
-    return session
+@pytest.fixture
+def ot2_server_set_disable_fast_analysis(
+    ot2_server_base_url: str,
+) -> Generator[None, None, None]:
+    """For integration tests that need to set then clear the
+    disableFastProtocolUpload feature flag"""
+    url = f"{ot2_server_base_url}/settings"
+    data = {"id": "disableFastProtocolUpload", "value": True}
+    with _requests_session() as requests_session:
+        requests_session.post(url, json=data)
+        yield None
+        data["value"] = None
+        requests.post(url, json=data)
+
+
+@pytest.fixture
+def ot2_server_base_url(_ot2_session_server: str) -> Generator[str, None, None]:
+    """Return the URL for a running dev server.
+
+    Because it can take several seconds to start up and shut down, one server is shared across all
+    tests in the test session. This fixture softly "resets" it after each test by deleting all
+    runs and protocols, which provides good enough isolation in practice.
+    """
+    yield _ot2_session_server
+    _clean_server_state(_ot2_session_server)
+
+
+@pytest.fixture
+def ot3_server_base_url(_ot3_session_server: str) -> Generator[str, None, None]:
+    """Like `ot2_server_base_url()`, but the server is configured as an OT-3 instead of an OT-2."""
+    yield _ot3_session_server
+    _clean_server_state(_ot3_session_server)
 
 
 @pytest.fixture(scope="session")
-def run_server(
-    request_session: requests.Session,
-    server_temp_directory: str,
-) -> Iterator[None]:
-    """Run the robot server in a background process."""
+def _ot2_session_server(server_temp_directory: str) -> Generator[str, None, None]:
+    base_url = (
+        f"{_SESSION_SERVER_SCHEME}{_SESSION_SERVER_HOST}:{_OT2_SESSION_SERVER_PORT}"
+    )
     with DevServer(
-        port=_SESSION_SERVER_PORT,
+        port=_OT2_SESSION_SERVER_PORT,
         ot_api_config_dir=Path(server_temp_directory),
     ) as dev_server:
         dev_server.start()
-
-        # Wait for a bit to get started by polling /hcpealth
-        from requests.exceptions import ConnectionError
-
-        while True:
-            try:
-                health_response = request_session.get(
-                    f"{_SESSION_SERVER_HOST}:{_SESSION_SERVER_PORT}/health"
-                )
-            except ConnectionError:
-                # The server isn't up yet to accept requests. Keep polling.
-                pass
-            else:
-                if health_response.status_code == 503:
-                    # The server is accepting requests but reporting not ready. Keep polling.
-                    pass
-                else:
-                    # The server's replied with something other than a busy indicator. Stop polling.
-                    break
-
-            time.sleep(0.1)
-
-        yield
+        _wait_until_ready(base_url)
+        yield base_url
 
 
 @pytest.fixture(scope="session")
-def ot3_run_server(
-    request_session: requests.Session,
-    server_temp_directory: str,
-) -> Iterator[None]:
-    """Run the robot server in a background process."""
+def _ot3_session_server(server_temp_directory: str) -> Generator[str, None, None]:
+    base_url = (
+        f"{_SESSION_SERVER_SCHEME}{_SESSION_SERVER_HOST}:{_OT3_SESSION_SERVER_PORT}"
+    )
     with DevServer(
         port=_OT3_SESSION_SERVER_PORT,
         is_ot3=True,
         ot_api_config_dir=Path(server_temp_directory),
     ) as dev_server:
         dev_server.start()
+        _wait_until_ready(base_url)
+        yield base_url
 
-        # Wait for a bit to get started by polling /hcpealth
-        from requests.exceptions import ConnectionError
 
+@contextlib.contextmanager
+def _requests_session() -> Generator[requests.Session, None, None]:
+    with requests.Session() as session:
+        session.headers.update({API_VERSION_HEADER: LATEST_API_VERSION_HEADER_VALUE})
+        yield session
+
+
+def _wait_until_ready(base_url: str) -> None:
+    with _requests_session() as requests_session:
         while True:
             try:
-                health_response = request_session.get(
-                    f"{_SESSION_SERVER_HOST}:{_OT3_SESSION_SERVER_PORT}/health"
-                )
-            except ConnectionError:
+                health_response = requests_session.get(f"{base_url}/health")
+            except requests.ConnectionError:
                 # The server isn't up yet to accept requests. Keep polling.
                 pass
             else:
@@ -108,34 +122,32 @@ def ot3_run_server(
                     pass
                 else:
                     # The server's replied with something other than a busy indicator. Stop polling.
-                    break
+                    return
 
             time.sleep(0.1)
 
-        yield
 
+def _clean_server_state(base_url: str) -> None:
+    async def _clean_server_state_async() -> None:
+        async with RobotClient.make(base_url=base_url, version="*") as robot_client:
+            # Delete runs first because protocols can't be deleted if a run refers to them.
+            await _delete_all_runs(robot_client)
+            await _delete_all_protocols(robot_client)
 
-@pytest.fixture(scope="session")
-def session_server_host(run_server: object) -> str:
-    """Return the host of the running session-scoped dev server."""
-    return _SESSION_SERVER_HOST
+    asyncio.run(_clean_server_state_async())
 
 
-@pytest.fixture(scope="session")
-def session_server_port(run_server: object) -> str:
-    """Return the port of the running session-scoped dev server."""
-    return _SESSION_SERVER_PORT
+async def _delete_all_runs(robot_client: RobotClient) -> None:
+    """Delete all runs on the robot server."""
+    response = await robot_client.get_runs()
+    run_ids = [r["id"] for r in response.json()["data"]]
+    for run_id in run_ids:
+        await robot_client.delete_run(run_id)
 
 
-@pytest.fixture
-def set_disable_fast_analysis(
-    request_session: requests.Session,
-) -> Iterator[None]:
-    """For integration tests that need to set then clear the
-    disableFastProtocolUpload feature flag"""
-    url = "http://localhost:31950/settings"
-    data = {"id": "disableFastProtocolUpload", "value": True}
-    request_session.post(url, json=data)
-    yield None
-    data["value"] = None
-    request_session.post(url, json=data)
+async def _delete_all_protocols(robot_client: RobotClient) -> None:
+    """Delete all protocols on the robot server"""
+    response = await robot_client.get_protocols()
+    protocol_ids = [p["id"] for p in response.json()["data"]]
+    for protocol_id in protocol_ids:
+        await robot_client.delete_protocol(protocol_id)