From e140f3596b4499e291b201b1bb08fc318960663f Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Tue, 15 Jul 2025 18:02:03 +0100 Subject: [PATCH 01/13] Added backend server endpoint to check whether a multigrid controller exists --- src/murfey/server/api/instrument.py | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/src/murfey/server/api/instrument.py b/src/murfey/server/api/instrument.py index e4acae82c..3e1886419 100644 --- a/src/murfey/server/api/instrument.py +++ b/src/murfey/server/api/instrument.py @@ -4,7 +4,7 @@ import datetime import logging from pathlib import Path -from typing import Annotated, List, Optional +from typing import Annotated, Any, List, Optional from urllib.parse import quote import aiohttp @@ -165,6 +165,30 @@ async def start_multigrid_watcher(session_id: MurfeySessionID, db=murfey_db): return data +@router.post("/sessions/{session_id}/multigrid_controller/status") +async def check_multigrid_controller_exists(session_id: MurfeySessionID, db=murfey_db): + session = db.exec(select(Session).where(Session.id == session_id)).one() + instrument_name = session.instrument_name + machine_config = get_machine_config(instrument_name=instrument_name)[ + instrument_name + ] + if machine_config.instrument_server_url: + log.debug( + f"Submitting request to inspect multigrid controller for session {session_id}" + ) + async with aiohttp.ClientSession() as clientsession: + async with clientsession.post( + f"{machine_config.instrument_server_url}{url_path_for('api.router', 'check_multigrid_controller_exists', session_id=session_id)}", + headers={ + "Authorization": f"Bearer {instrument_server_tokens[session_id]['access_token']}" + }, + ) as resp: + data: dict[str, Any] = await resp.json() + else: + data = {"detail": "No instrument server URL found"} + return data + + class ProvidedProcessingParameters(BaseModel): dose_per_frame: float extract_downscale: bool = True From fb84819bfc08cc25392b48e61f995fa04240709b Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Tue, 15 Jul 2025 18:02:30 +0100 Subject: [PATCH 02/13] Added instrument server endpoint to check whether a multigrid controller exists --- src/murfey/instrument_server/api.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/murfey/instrument_server/api.py b/src/murfey/instrument_server/api.py index afd890255..3b9db5166 100644 --- a/src/murfey/instrument_server/api.py +++ b/src/murfey/instrument_server/api.py @@ -223,6 +223,15 @@ def update_multigrid_controller_visit_end_time( return {"success": True} +@router.post("/sessions/{session_id}/multigrid_controller/status") +def check_multigrid_controller_exists( + session_id: MurfeySessionID, +): + if controllers.get(session_id, None) is not None: + return {"exists": True} + return {"exists": False} + + class RsyncerSource(BaseModel): source: Path From 310dc1e300bd5f4022823a8ba69031bece950782 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Tue, 15 Jul 2025 18:03:28 +0100 Subject: [PATCH 03/13] Updated route manifest --- src/murfey/util/route_manifest.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/murfey/util/route_manifest.yaml b/src/murfey/util/route_manifest.yaml index 03bfb2af7..e5d8e7272 100644 --- a/src/murfey/util/route_manifest.yaml +++ b/src/murfey/util/route_manifest.yaml @@ -43,6 +43,11 @@ murfey.instrument_server.api.router: path_params: [] methods: - POST + - path: /sessions/{session_id}/multigrid_controller/status + function: check_multigrid_controller_exists + path_params: [] + methods: + - POST - path: /sessions/{session_id}/stop_rsyncer function: stop_rsyncer path_params: [] @@ -503,6 +508,11 @@ murfey.server.api.instrument.router: path_params: [] methods: - POST + - path: /instrument_server/sessions/{session_id}/multigrid_controller/status + function: check_multigrid_controller_exists + path_params: [] + methods: + - POST - path: /instrument_server/sessions/{session_id}/provided_processing_parameters function: pass_proc_params_to_instrument_server path_params: [] From ab16492b5a379e37d8fc637b370ac24384f15c81 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Tue, 15 Jul 2025 18:32:15 +0100 Subject: [PATCH 04/13] Forgot to change the request method to be used --- src/murfey/instrument_server/api.py | 2 +- src/murfey/server/api/instrument.py | 2 +- src/murfey/util/route_manifest.yaml | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/murfey/instrument_server/api.py b/src/murfey/instrument_server/api.py index 3b9db5166..ed8b4100b 100644 --- a/src/murfey/instrument_server/api.py +++ b/src/murfey/instrument_server/api.py @@ -223,7 +223,7 @@ def update_multigrid_controller_visit_end_time( return {"success": True} -@router.post("/sessions/{session_id}/multigrid_controller/status") +@router.get("/sessions/{session_id}/multigrid_controller/status") def check_multigrid_controller_exists( session_id: MurfeySessionID, ): diff --git a/src/murfey/server/api/instrument.py b/src/murfey/server/api/instrument.py index 3e1886419..e2cb48d09 100644 --- a/src/murfey/server/api/instrument.py +++ b/src/murfey/server/api/instrument.py @@ -165,7 +165,7 @@ async def start_multigrid_watcher(session_id: MurfeySessionID, db=murfey_db): return data -@router.post("/sessions/{session_id}/multigrid_controller/status") +@router.get("/sessions/{session_id}/multigrid_controller/status") async def check_multigrid_controller_exists(session_id: MurfeySessionID, db=murfey_db): session = db.exec(select(Session).where(Session.id == session_id)).one() instrument_name = session.instrument_name diff --git a/src/murfey/util/route_manifest.yaml b/src/murfey/util/route_manifest.yaml index e5d8e7272..a92821c19 100644 --- a/src/murfey/util/route_manifest.yaml +++ b/src/murfey/util/route_manifest.yaml @@ -47,7 +47,7 @@ murfey.instrument_server.api.router: function: check_multigrid_controller_exists path_params: [] methods: - - POST + - GET - path: /sessions/{session_id}/stop_rsyncer function: stop_rsyncer path_params: [] @@ -512,7 +512,7 @@ murfey.server.api.instrument.router: function: check_multigrid_controller_exists path_params: [] methods: - - POST + - GET - path: /instrument_server/sessions/{session_id}/provided_processing_parameters function: pass_proc_params_to_instrument_server path_params: [] From 119ec67bcd6bb113f7c38c936ba5a43118d787d1 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Tue, 15 Jul 2025 19:15:18 +0100 Subject: [PATCH 05/13] Added debug log --- src/murfey/server/api/instrument.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/murfey/server/api/instrument.py b/src/murfey/server/api/instrument.py index e2cb48d09..959352d84 100644 --- a/src/murfey/server/api/instrument.py +++ b/src/murfey/server/api/instrument.py @@ -186,6 +186,7 @@ async def check_multigrid_controller_exists(session_id: MurfeySessionID, db=murf data: dict[str, Any] = await resp.json() else: data = {"detail": "No instrument server URL found"} + log.debug(f"Received response: {data}") return data From 0f8064d57a26899b51ef16140f871bb348c002cf Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Tue, 15 Jul 2025 19:18:22 +0100 Subject: [PATCH 06/13] Forgot to replace another 'post' with 'get' --- src/murfey/server/api/instrument.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/murfey/server/api/instrument.py b/src/murfey/server/api/instrument.py index 959352d84..949d28137 100644 --- a/src/murfey/server/api/instrument.py +++ b/src/murfey/server/api/instrument.py @@ -177,7 +177,7 @@ async def check_multigrid_controller_exists(session_id: MurfeySessionID, db=murf f"Submitting request to inspect multigrid controller for session {session_id}" ) async with aiohttp.ClientSession() as clientsession: - async with clientsession.post( + async with clientsession.get( f"{machine_config.instrument_server_url}{url_path_for('api.router', 'check_multigrid_controller_exists', session_id=session_id)}", headers={ "Authorization": f"Bearer {instrument_server_tokens[session_id]['access_token']}" From 239d7e932a2984769ccfe635b80b7a763a33da9a Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Thu, 17 Jul 2025 12:07:25 +0100 Subject: [PATCH 07/13] Rearannged functions by purpose --- src/murfey/instrument_server/api.py | 16 +++--- src/murfey/server/api/instrument.py | 86 ++++++++++++++--------------- 2 files changed, 51 insertions(+), 51 deletions(-) diff --git a/src/murfey/instrument_server/api.py b/src/murfey/instrument_server/api.py index ed8b4100b..ca31f9c21 100644 --- a/src/murfey/instrument_server/api.py +++ b/src/murfey/instrument_server/api.py @@ -215,14 +215,6 @@ def stop_multigrid_watcher(session_id: MurfeySessionID, label: str): return {"success": True} -@router.post("/sessions/{session_id}/multigrid_controller/visit_end_time") -def update_multigrid_controller_visit_end_time( - session_id: MurfeySessionID, end_time: datetime -): - controllers[session_id].update_visit_time(end_time) - return {"success": True} - - @router.get("/sessions/{session_id}/multigrid_controller/status") def check_multigrid_controller_exists( session_id: MurfeySessionID, @@ -232,6 +224,14 @@ def check_multigrid_controller_exists( return {"exists": False} +@router.post("/sessions/{session_id}/multigrid_controller/visit_end_time") +def update_multigrid_controller_visit_end_time( + session_id: MurfeySessionID, end_time: datetime +): + controllers[session_id].update_visit_time(end_time) + return {"success": True} + + class RsyncerSource(BaseModel): source: Path diff --git a/src/murfey/server/api/instrument.py b/src/murfey/server/api/instrument.py index 949d28137..9f2860f44 100644 --- a/src/murfey/server/api/instrument.py +++ b/src/murfey/server/api/instrument.py @@ -101,6 +101,31 @@ async def check_if_session_is_active( return {"active": response.status == 200} +@router.get("/sessions/{session_id}/multigrid_controller/status") +async def check_multigrid_controller_exists(session_id: MurfeySessionID, db=murfey_db): + session = db.exec(select(Session).where(Session.id == session_id)).one() + instrument_name = session.instrument_name + machine_config = get_machine_config(instrument_name=instrument_name)[ + instrument_name + ] + if machine_config.instrument_server_url: + log.debug( + f"Submitting request to inspect multigrid controller for session {session_id}" + ) + async with aiohttp.ClientSession() as clientsession: + async with clientsession.get( + f"{machine_config.instrument_server_url}{url_path_for('api.router', 'check_multigrid_controller_exists', session_id=session_id)}", + headers={ + "Authorization": f"Bearer {instrument_server_tokens[session_id]['access_token']}" + }, + ) as resp: + data: dict[str, Any] = await resp.json() + else: + data = {"detail": "No instrument server URL found"} + log.debug(f"Received response: {data}") + return data + + @router.post("/sessions/{session_id}/multigrid_watcher") async def setup_multigrid_watcher( session_id: MurfeySessionID, watcher_spec: MultigridWatcherSetup, db=murfey_db @@ -165,28 +190,33 @@ async def start_multigrid_watcher(session_id: MurfeySessionID, db=murfey_db): return data -@router.get("/sessions/{session_id}/multigrid_controller/status") -async def check_multigrid_controller_exists(session_id: MurfeySessionID, db=murfey_db): - session = db.exec(select(Session).where(Session.id == session_id)).one() - instrument_name = session.instrument_name +@router.post("/sessions/{session_id}/multigrid_controller/visit_end_time") +async def update_visit_end_time( + session_id: MurfeySessionID, end_time: datetime.datetime, db=murfey_db +): + # Load data for session + session_entry = db.exec(select(Session).where(Session.id == session_id)).one() + instrument_name = session_entry.instrument_name + + # Update visit end time in database + session_entry.visit_end_time = end_time + db.add(session_entry) + db.commit() + + # Update the multigrid controller + data = {} machine_config = get_machine_config(instrument_name=instrument_name)[ instrument_name ] if machine_config.instrument_server_url: - log.debug( - f"Submitting request to inspect multigrid controller for session {session_id}" - ) async with aiohttp.ClientSession() as clientsession: - async with clientsession.get( - f"{machine_config.instrument_server_url}{url_path_for('api.router', 'check_multigrid_controller_exists', session_id=session_id)}", + async with clientsession.post( + f"{machine_config.instrument_server_url}{url_path_for('api.router', 'update_multigrid_controller_visit_end_time', session_id=session_id)}?end_time={quote(end_time.isoformat())}", headers={ "Authorization": f"Bearer {instrument_server_tokens[session_id]['access_token']}" }, ) as resp: - data: dict[str, Any] = await resp.json() - else: - data = {"detail": "No instrument server URL found"} - log.debug(f"Received response: {data}") + data = await resp.json() return data @@ -422,36 +452,6 @@ async def finalise_session(session_id: MurfeySessionID, db=murfey_db): return data -@router.post("/sessions/{session_id}/multigrid_controller/visit_end_time") -async def update_visit_end_time( - session_id: MurfeySessionID, end_time: datetime.datetime, db=murfey_db -): - # Load data for session - session_entry = db.exec(select(Session).where(Session.id == session_id)).one() - instrument_name = session_entry.instrument_name - - # Update visit end time in database - session_entry.visit_end_time = end_time - db.add(session_entry) - db.commit() - - # Update the multigrid controller - data = {} - machine_config = get_machine_config(instrument_name=instrument_name)[ - instrument_name - ] - if machine_config.instrument_server_url: - async with aiohttp.ClientSession() as clientsession: - async with clientsession.post( - f"{machine_config.instrument_server_url}{url_path_for('api.router', 'update_multigrid_controller_visit_end_time', session_id=session_id)}?end_time={quote(end_time.isoformat())}", - headers={ - "Authorization": f"Bearer {instrument_server_tokens[session_id]['access_token']}" - }, - ) as resp: - data = await resp.json() - return data - - @router.post("/sessions/{session_id}/abandon_session") async def abandon_session(session_id: MurfeySessionID, db=murfey_db): data = {} From 03799a8c46544b7058423bc2d2e4142a7bd3ee2a Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Thu, 17 Jul 2025 13:19:39 +0100 Subject: [PATCH 08/13] Added test for backend endpoint to check for existence of multigrid controller --- tests/server/api/test_instrument.py | 130 ++++++++++++++++++++++++++++ 1 file changed, 130 insertions(+) create mode 100644 tests/server/api/test_instrument.py diff --git a/tests/server/api/test_instrument.py b/tests/server/api/test_instrument.py new file mode 100644 index 000000000..b8c83ae8f --- /dev/null +++ b/tests/server/api/test_instrument.py @@ -0,0 +1,130 @@ +from typing import Literal +from unittest import mock +from unittest.mock import AsyncMock, MagicMock + +from fastapi import FastAPI +from fastapi.testclient import TestClient +from pytest_mock import MockerFixture + +from murfey.server.api.auth import validate_frontend_session_access, validate_token +from murfey.server.api.instrument import router as backend_router +from murfey.server.murfey_db import murfey_db_session +from murfey.util.api import url_path_for + + +def mock_aiohttp_clientsession( + mocker: MockerFixture, + method: Literal["get", "post", "delete"] = "get", + json_data={}, + status=200, +): + """ + Helper function to patch a aiohttp.ClientSession GET request. This returns a + mocked async context manager with a mocked response that, in turn, returns + the given JSON data and status. + + Returns the mocked ClientSession, which can then be inspected to assert that + the expected calls were made. + """ + + # Mock out the async response + mock_response = MagicMock() + mock_response.json = AsyncMock(return_value=json_data) + mock_response.status = status + + # Mock out the context manager returned by clientsession.get() + mock_context_manager = MagicMock() + mock_context_manager.__aenter__ = AsyncMock(return_value=mock_response) + mock_context_manager.__aexit__ = AsyncMock(return_value=None) + + # Mock the client session + mock_clientsession = MagicMock() + mock_clientsession.__aenter__ = AsyncMock(return_value=mock_clientsession) + mock_clientsession.__aexit__ = AsyncMock(return_value=None) + + # Assign the context manager to the request method being tested + getattr(mock_clientsession, method.lower()).return_value = mock_context_manager + + # Patch 'aiohttp.ClientSession' to return the mocked client session + mocker.patch("aiohttp.ClientSession", return_value=mock_clientsession) + + return mock_clientsession, mock_response + + +def test_check_multigrid_controller_exists(mocker: MockerFixture): + # Set up the objects to mock + instrument_name = "test" + session_id = 1 + instrment_server_url = "https://murfey.instrument-server.test" + + # Override the database session generator + mock_session = MagicMock() + mock_session.instrument_name = instrument_name + mock_query_result = MagicMock() + mock_query_result.one.return_value = mock_session + mock_db_session = MagicMock() + mock_db_session.exec.return_value = mock_query_result + + def mock_get_db_session(): + yield mock_db_session + + # Mock the machine config + mock_machine_config = MagicMock() + mock_machine_config.instrument_server_url = instrment_server_url + mock_get_machine_config = mocker.patch( + "murfey.server.api.instrument.get_machine_config" + ) + mock_get_machine_config.return_value = { + instrument_name: mock_machine_config, + } + + # Mock the instrument server tokens dictionary + mock_tokens = mocker.patch( + "murfey.server.api.instrument.instrument_server_tokens", + {session_id: {"access_token": mock.sentinel}}, + ) + + # Mock out the async GET request in the endpoint + mock_clientsession, _ = mock_aiohttp_clientsession( + mocker, + method="get", + json_data={"exists": True}, + status=200, + ) + + # Set up the backend server + backend_app = FastAPI() + + # Override validation and database dependencies + backend_app.dependency_overrides[validate_token] = lambda: None + backend_app.dependency_overrides[validate_frontend_session_access] = ( + lambda: session_id + ) + backend_app.dependency_overrides[murfey_db_session] = mock_get_db_session + backend_app.include_router(backend_router) + backend_server = TestClient(backend_app) + + # Construct the URL paths for poking and sending to + backend_url_path = url_path_for( + "api.instrument.router", + "check_multigrid_controller_exists", + session_id=session_id, + ) + client_url_path = url_path_for( + "api.router", + "check_multigrid_controller_exists", + session_id=session_id, + ) + + # Poke the backend + response = backend_server.get(backend_url_path) + + # Check that the expected calls were made + mock_db_session.exec.assert_called_once() + mock_get_machine_config.assert_called_once_with(instrument_name=instrument_name) + mock_clientsession.get.assert_called_once_with( + f"{instrment_server_url}{client_url_path}", + headers={"Authorization": f"Bearer {mock_tokens[session_id]['access_token']}"}, + ) + assert response.status_code == 200 + assert response.json() == {"exists": True} From 9b9c738efc54ce0cc68b8010f44126c2ee933470 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Thu, 17 Jul 2025 13:44:52 +0100 Subject: [PATCH 09/13] Added test for instrument server endpoint to check for existence of multigrid controller for a given session id --- tests/instrument_server/test_api.py | 45 +++++++++++++++++++++++++---- 1 file changed, 39 insertions(+), 6 deletions(-) diff --git a/tests/instrument_server/test_api.py b/tests/instrument_server/test_api.py index 785f1e812..262e3b2e7 100644 --- a/tests/instrument_server/test_api.py +++ b/tests/instrument_server/test_api.py @@ -1,18 +1,33 @@ from pathlib import Path from typing import Optional -from unittest.mock import ANY, Mock, patch +from unittest.mock import ANY, MagicMock, Mock, patch from urllib.parse import urlparse +from fastapi import FastAPI +from fastapi.testclient import TestClient from pytest import mark +from pytest_mock import MockerFixture -from murfey.instrument_server.api import ( - GainReference, - _get_murfey_url, - upload_gain_reference, -) +from murfey.instrument_server.api import GainReference, _get_murfey_url +from murfey.instrument_server.api import router as client_router +from murfey.instrument_server.api import upload_gain_reference, validate_session_token from murfey.util import posix_path from murfey.util.api import url_path_for + +def set_up_test_client(session_id: Optional[int] = None): + """ + Helper function to set up a test client for the instrument server with validation + checks disabled. + """ + # Set up the instrument server + client_app = FastAPI() + if session_id: + client_app.dependency_overrides[validate_session_token] = lambda: session_id + client_app.include_router(client_router) + return TestClient(client_app) + + test_get_murfey_url_params_matrix = ( # Server URL to use ("default",), @@ -57,6 +72,24 @@ def test_get_murfey_url( assert parsed_server.path == parsed_original.path +def test_check_multigrid_controller_exists(mocker: MockerFixture): + session_id = 1 + + # Patch out the multigrid controllers that have been stored in memory + mocker.patch("murfey.instrument_server.api.controllers", {session_id: MagicMock()}) + + # Set up the test client + client_server = set_up_test_client(session_id=session_id) + url_path = url_path_for( + "api.router", "check_multigrid_controller_exists", session_id=session_id + ) + response = client_server.get(url_path) + + # Check that the result is as expected + assert response.status_code == 200 + assert response.json() == {"exists": True} + + test_upload_gain_reference_params_matrix = ( # Rsync URL settings ("http://1.1.1.1",), # When rsync_url is provided From fda4f56f89c1b0c0c5e23c4639e0f0fb0c31c4ec Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Thu, 17 Jul 2025 13:47:01 +0100 Subject: [PATCH 10/13] Use 'pytest.mark' instead of just 'mark' for greater clarity in decorator --- tests/instrument_server/test_api.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/instrument_server/test_api.py b/tests/instrument_server/test_api.py index 262e3b2e7..61f4d0e63 100644 --- a/tests/instrument_server/test_api.py +++ b/tests/instrument_server/test_api.py @@ -3,9 +3,9 @@ from unittest.mock import ANY, MagicMock, Mock, patch from urllib.parse import urlparse +import pytest from fastapi import FastAPI from fastapi.testclient import TestClient -from pytest import mark from pytest_mock import MockerFixture from murfey.instrument_server.api import GainReference, _get_murfey_url @@ -38,7 +38,7 @@ def set_up_test_client(session_id: Optional[int] = None): ) -@mark.parametrize("test_params", test_get_murfey_url_params_matrix) +@pytest.mark.parametrize("test_params", test_get_murfey_url_params_matrix) def test_get_murfey_url( test_params: tuple[str], mock_client_configuration, # From conftest.py @@ -98,7 +98,7 @@ def test_check_multigrid_controller_exists(mocker: MockerFixture): ) -@mark.parametrize("test_params", test_upload_gain_reference_params_matrix) +@pytest.mark.parametrize("test_params", test_upload_gain_reference_params_matrix) @patch("murfey.instrument_server.api.subprocess") @patch("murfey.instrument_server.api.tokens") @patch("murfey.instrument_server.api._get_murfey_url") From 5993c232173d74d8d98a57d9470ec3301e899d14 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Thu, 17 Jul 2025 13:48:20 +0100 Subject: [PATCH 11/13] Use MagicMock consistently in test module --- tests/instrument_server/test_api.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/instrument_server/test_api.py b/tests/instrument_server/test_api.py index 61f4d0e63..214b22740 100644 --- a/tests/instrument_server/test_api.py +++ b/tests/instrument_server/test_api.py @@ -1,6 +1,6 @@ from pathlib import Path from typing import Optional -from unittest.mock import ANY, MagicMock, Mock, patch +from unittest.mock import ANY, MagicMock, patch from urllib.parse import urlparse import pytest @@ -128,12 +128,12 @@ def test_upload_gain_reference( mock_machine_config["rsync_url"] = rsync_url_setting # Assign expected values to the mock objects - mock_response = Mock() + mock_response = MagicMock() mock_response.status_code = 200 mock_response.json.return_value = mock_machine_config mock_request.get.return_value = mock_response mock_get_server_url.return_value = server_url - mock_subprocess.run.return_value = Mock(returncode=0) + mock_subprocess.run.return_value = MagicMock(returncode=0) # Construct payload and pass request to function gain_ref_file = f"{gain_ref_dir}/gain.mrc" From 99f216f3a9c3576fffcd9b7040767c0496d72b76 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Thu, 17 Jul 2025 13:55:45 +0100 Subject: [PATCH 12/13] Switched to using 'mocker' within test function instead of stacking multiple 'mock.patch' decorators --- tests/instrument_server/test_api.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/tests/instrument_server/test_api.py b/tests/instrument_server/test_api.py index 214b22740..ee78fd742 100644 --- a/tests/instrument_server/test_api.py +++ b/tests/instrument_server/test_api.py @@ -99,24 +99,22 @@ def test_check_multigrid_controller_exists(mocker: MockerFixture): @pytest.mark.parametrize("test_params", test_upload_gain_reference_params_matrix) -@patch("murfey.instrument_server.api.subprocess") -@patch("murfey.instrument_server.api.tokens") -@patch("murfey.instrument_server.api._get_murfey_url") -@patch("murfey.instrument_server.api.requests") def test_upload_gain_reference( - mock_request, - mock_get_server_url, - mock_tokens, - mock_subprocess, + mocker: MockerFixture, test_params: tuple[Optional[str]], ): - # Unpack test parameters and define other ones (rsync_url_setting,) = test_params server_url = "http://0.0.0.0:8000" instrument_name = "murfey" session_id = 1 + # Mock out objects + mock_request = mocker.patch("murfey.instrument_server.api.requests") + mock_get_server_url = mocker.patch("murfey.instrument_server.api._get_murfey_url") + mock_subprocess = mocker.patch("murfey.instrument_server.api.subprocess") + mocker.patch("murfey.instrument_server.api.tokens", {session_id: ANY}) + # Create a mock machine config base on the test params rsync_module = "data" gain_ref_dir = "C:/ProgramData/Gatan/Gain Reference" From fe10c6217ef23f6c6be1f3d9f2b8f9c18828dc21 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Thu, 17 Jul 2025 14:05:58 +0100 Subject: [PATCH 13/13] Use the FastAPI TestClient to test the 'upload_gain_reference' endpoint, now that we know how to do so --- tests/instrument_server/test_api.py | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/tests/instrument_server/test_api.py b/tests/instrument_server/test_api.py index ee78fd742..310479fd6 100644 --- a/tests/instrument_server/test_api.py +++ b/tests/instrument_server/test_api.py @@ -8,9 +8,9 @@ from fastapi.testclient import TestClient from pytest_mock import MockerFixture -from murfey.instrument_server.api import GainReference, _get_murfey_url +from murfey.instrument_server.api import _get_murfey_url from murfey.instrument_server.api import router as client_router -from murfey.instrument_server.api import upload_gain_reference, validate_session_token +from murfey.instrument_server.api import validate_session_token from murfey.util import posix_path from murfey.util.api import url_path_for @@ -105,7 +105,7 @@ def test_upload_gain_reference( ): # Unpack test parameters and define other ones (rsync_url_setting,) = test_params - server_url = "http://0.0.0.0:8000" + server_url = "https://murfey.server.test" instrument_name = "murfey" session_id = 1 @@ -142,13 +142,18 @@ def test_upload_gain_reference( "visit_path": visit_path, "gain_destination_dir": gain_dest_dir, } - result = upload_gain_reference( + + # Set up instrument server test client + client_server = set_up_test_client(session_id=session_id) + + # Poke the endpoint with the expected data + url_path = url_path_for( + "api.router", + "upload_gain_reference", instrument_name=instrument_name, session_id=session_id, - gain_reference=GainReference( - **payload, - ), ) + response = client_server.post(url_path, json=payload) # Check that the machine config request was called machine_config_url = f"{server_url}{url_path_for('session_control.router', 'machine_info_by_instrument', instrument_name=instrument_name)}" @@ -176,4 +181,4 @@ def test_upload_gain_reference( ) # Check that the function ran through to completion successfully - assert result == {"success": True} + assert response.json() == {"success": True}