From 67cc0366cad9134ce621b0134228b674bb684c1c Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Wed, 9 Apr 2025 15:32:04 +0100
Subject: [PATCH 01/27] Captured post request when registering tomography
 preprocessing parameters; leave 'None' as-is when preparing the JSON data in
 '_start_dc'

---
 src/murfey/client/multigrid_control.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/murfey/client/multigrid_control.py b/src/murfey/client/multigrid_control.py
index a52bb0430..ab1bade3d 100644
--- a/src/murfey/client/multigrid_control.py
+++ b/src/murfey/client/multigrid_control.py
@@ -403,7 +403,8 @@ def _start_dc(self, json, from_form: bool = False):
         # it is then necessary to extract the data from the message
         if from_form:
             json = json.get("form", {})
-            json = {k: str(v) for k, v in json.items()}
+            # Safely convert all entries into strings, but leave None as-is
+            json = {k: str(v) if v is not None else None for k, v in json.items()}
         self._environment.data_collection_parameters = {
             k: None if v == "None" else v for k, v in json.items()
         }
@@ -411,7 +412,7 @@ def _start_dc(self, json, from_form: bool = False):
         context = self.analysers[source]._context
         if isinstance(context, TomographyContext):
             if from_form:
-                requests.post(
+                capture_post(
                     f"{self._environment.url.geturl()}/clients/{self._environment.client_id}/tomography_processing_parameters",
                     json=json,
                 )
@@ -442,7 +443,7 @@ def _start_dc(self, json, from_form: bool = False):
                 )
                 eer_fractionation_file = eer_response.json()["eer_fractionation_file"]
                 json.update({"eer_fractionation_file": eer_fractionation_file})
-            requests.post(
+            capture_post(
                 f"{self._environment.url.geturl()}/sessions/{self._environment.murfey_session}/tomography_preprocessing_parameters",
                 json=json,
             )

From aa690ff30afe98feabdc0569329e3a155eccf449 Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Wed, 9 Apr 2025 15:35:22 +0100
Subject: [PATCH 02/27] Fixed logic when parsing MachineConfig for rsync URL

---
 src/murfey/instrument_server/api.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/murfey/instrument_server/api.py b/src/murfey/instrument_server/api.py
index cb67ea25f..445faa59b 100644
--- a/src/murfey/instrument_server/api.py
+++ b/src/murfey/instrument_server/api.py
@@ -351,7 +351,11 @@ def upload_gain_reference(
         )
 
     # Return the rsync URL if set, otherwise assume you are syncing via Murfey
-    rsync_url = urlparse(str(machine_config.get("rsync_url", _get_murfey_url())))
+    rsync_url = urlparse(
+        str(machine_config["rsync_url"])
+        if machine_config.get("rsync_url", "")
+        else _get_murfey_url()
+    )
     rsync_module = machine_config.get("rsync_module", "data")
     rsync_path = f"{rsync_url.hostname}::{rsync_module}/{safe_visit_path}/{safe_destination_dir}/{secure_filename(gain_reference.gain_path.name)}"
 

From 360a345e2ef8199357d1fc3e2aa6894fb10c1b43 Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Wed, 9 Apr 2025 15:38:27 +0100
Subject: [PATCH 03/27] Allow 'None' in 'dose_per_frame' key in 'Base'
 sub-class

---
 src/murfey/util/models.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/murfey/util/models.py b/src/murfey/util/models.py
index d72eff927..c000de7c7 100644
--- a/src/murfey/util/models.py
+++ b/src/murfey/util/models.py
@@ -368,7 +368,7 @@ class PreprocessingParametersTomo(BaseModel):
     eer_fractionation: int
 
     class Base(BaseModel):
-        dose_per_frame: float
+        dose_per_frame: Optional[float]  # Doesn't match the model?
         gain_ref: Optional[str]
         manual_tilt_offset: float
         eer_fractionation: int

From 245ee6a70df2250b173ec1965c4817e10101ebc7 Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Wed, 9 Apr 2025 17:17:40 +0100
Subject: [PATCH 04/27] Added unit test for 'upload_gain_reference()' API
 endpoint

---
 tests/instrument_server/test_api.py | 89 +++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)
 create mode 100644 tests/instrument_server/test_api.py

diff --git a/tests/instrument_server/test_api.py b/tests/instrument_server/test_api.py
new file mode 100644
index 000000000..7bc3ae2ed
--- /dev/null
+++ b/tests/instrument_server/test_api.py
@@ -0,0 +1,89 @@
+from typing import Optional
+from unittest.mock import Mock, patch
+from urllib.parse import urlparse
+
+from fastapi import FastAPI
+from fastapi.testclient import TestClient
+from pytest import mark
+
+from murfey.instrument_server.api import router
+
+app = FastAPI()
+app.include_router(router)
+client = TestClient(app)
+
+
+test_upload_gain_reference_params_matrix = (
+    # Rsync URL | Rsync module | Gain reference directory |
+    (
+        "http://1.1.1.1",
+        "data",
+        "/c/ProgramData/Gatan/Gain Reference",
+    ),  # When rsync_url is provided
+    (
+        "",
+        "data",
+        "/c/ProgramData/Gatan/Gain Reference",
+    ),  # When rsync_url is blank
+    (
+        None,
+        "data",
+        "/c/ProgramData/Gatan/Gain Reference",
+    ),  # When rsync_url not provided
+)
+
+
+@mark.parametrize("test_params", test_upload_gain_reference_params_matrix)
+@patch("murfey.instrument_server.api.subprocess.run")
+@patch("murfey.instrument_server.api.urlparse", wraps=urlparse)
+@patch("murfey.instrument_server.api._get_murfey_url")
+@patch("murfey.instrument_server.api.requests.get")
+def test_upload_gain_reference(
+    mock_get,
+    mock_server_url,
+    spy_parse,
+    mock_run,
+    test_params: tuple[Optional[str], str, str],
+):
+
+    # Create a mock machine config base on the test params
+    rsync_url, rsync_module, gain_ref_dir = test_params
+    server_url = "http://0.0.0.0:8000"
+    mock_machine_config = {
+        "rsync_module": rsync_module,
+        "gain_reference_directory": gain_ref_dir,
+    }
+    if rsync_url is not None:
+        mock_machine_config["rsync_url"] = rsync_url
+
+    # Assign expected values to the mock objects
+    mock_get.return_value = Mock(status_code=200, json=lambda: mock_machine_config)
+    mock_server_url.return_value = server_url
+    mock_run.return_value = Mock(returncode=0)
+
+    # Construct payload and submit post request
+    payload = {
+        "gain_path": f"{gain_ref_dir}/gain.mrc",
+        "visit_path": "2025/aa00000-0",
+        "gain_destination_dir": "processing",
+    }
+    response = client.post(
+        "/instruments/m02/session/1/upload_gain_reference", json=payload
+    )
+
+    # Check that the machine config request was called
+    mock_get.assert_called_once()
+
+    # If no rsync_url key is provided, or rsync_url key is empty,
+    # This should default to the Murfey URL
+    if not rsync_url:
+        assert spy_parse.return_value == urlparse(server_url)
+    else:
+        assert spy_parse.return_value == urlparse(rsync_url)
+
+    # Check that the subprocess was run
+    mock_run.assert_called_once()
+
+    # Check that the endpoint function ran through to completion successfully
+    assert response.status_code == 200
+    assert response.json() == {"success": True}

From ed388440bb862671f8b0231d8cf10c7cccd4e405 Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Wed, 9 Apr 2025 17:18:31 +0100
Subject: [PATCH 05/27] Adjusted location of comments

---
 tests/instrument_server/test_api.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/instrument_server/test_api.py b/tests/instrument_server/test_api.py
index 7bc3ae2ed..52c2a0213 100644
--- a/tests/instrument_server/test_api.py
+++ b/tests/instrument_server/test_api.py
@@ -16,20 +16,20 @@
 test_upload_gain_reference_params_matrix = (
     # Rsync URL | Rsync module | Gain reference directory |
     (
-        "http://1.1.1.1",
+        "http://1.1.1.1",  # When rsync_url is provided
         "data",
         "/c/ProgramData/Gatan/Gain Reference",
-    ),  # When rsync_url is provided
+    ),
     (
-        "",
+        "",  # When rsync_url is blank
         "data",
         "/c/ProgramData/Gatan/Gain Reference",
-    ),  # When rsync_url is blank
+    ),
     (
-        None,
+        None,  # When rsync_url not provided
         "data",
         "/c/ProgramData/Gatan/Gain Reference",
-    ),  # When rsync_url not provided
+    ),
 )
 
 

From a3cfb85e0633eadcb817467cc698fe5c827139d5 Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Wed, 9 Apr 2025 17:29:43 +0100
Subject: [PATCH 06/27] Fixed which function/module layer to mock

---
 tests/instrument_server/test_api.py | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/tests/instrument_server/test_api.py b/tests/instrument_server/test_api.py
index 52c2a0213..76b8419eb 100644
--- a/tests/instrument_server/test_api.py
+++ b/tests/instrument_server/test_api.py
@@ -34,15 +34,15 @@
 
 
 @mark.parametrize("test_params", test_upload_gain_reference_params_matrix)
-@patch("murfey.instrument_server.api.subprocess.run")
+@patch("murfey.instrument_server.api.subprocess")
 @patch("murfey.instrument_server.api.urlparse", wraps=urlparse)
 @patch("murfey.instrument_server.api._get_murfey_url")
-@patch("murfey.instrument_server.api.requests.get")
+@patch("murfey.instrument_server.api.requests")
 def test_upload_gain_reference(
-    mock_get,
-    mock_server_url,
+    mock_request,
+    mock_get_server_url,
     spy_parse,
-    mock_run,
+    mock_subprocess,
     test_params: tuple[Optional[str], str, str],
 ):
 
@@ -57,9 +57,12 @@ def test_upload_gain_reference(
         mock_machine_config["rsync_url"] = rsync_url
 
     # Assign expected values to the mock objects
-    mock_get.return_value = Mock(status_code=200, json=lambda: mock_machine_config)
-    mock_server_url.return_value = server_url
-    mock_run.return_value = Mock(returncode=0)
+    mock_request.get.return_value = Mock(
+        status_code=200,
+        json=lambda: mock_machine_config,
+    )
+    mock_get_server_url.return_value = server_url
+    mock_subprocess.run.return_value = Mock(returncode=0)
 
     # Construct payload and submit post request
     payload = {
@@ -72,7 +75,7 @@ def test_upload_gain_reference(
     )
 
     # Check that the machine config request was called
-    mock_get.assert_called_once()
+    mock_request.get.assert_called_once()
 
     # If no rsync_url key is provided, or rsync_url key is empty,
     # This should default to the Murfey URL
@@ -82,7 +85,7 @@ def test_upload_gain_reference(
         assert spy_parse.return_value == urlparse(rsync_url)
 
     # Check that the subprocess was run
-    mock_run.assert_called_once()
+    mock_subprocess.run.assert_called_once()
 
     # Check that the endpoint function ran through to completion successfully
     assert response.status_code == 200

From 898efd39a01dc14bc1b30bc6566073497c6745bf Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Wed, 9 Apr 2025 17:37:52 +0100
Subject: [PATCH 07/27] Tried fixing the 'requests.get' mock

---
 tests/instrument_server/test_api.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/instrument_server/test_api.py b/tests/instrument_server/test_api.py
index 76b8419eb..dbf076ae1 100644
--- a/tests/instrument_server/test_api.py
+++ b/tests/instrument_server/test_api.py
@@ -57,10 +57,10 @@ def test_upload_gain_reference(
         mock_machine_config["rsync_url"] = rsync_url
 
     # Assign expected values to the mock objects
-    mock_request.get.return_value = Mock(
-        status_code=200,
-        json=lambda: mock_machine_config,
-    )
+    mock_response = Mock()
+    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)
 

From a5a30a0edd97cd4ad07d52abe6bcf4e293efb99f Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Wed, 9 Apr 2025 17:46:28 +0100
Subject: [PATCH 08/27] Typo in URL path to endpoint to test

---
 tests/instrument_server/test_api.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/instrument_server/test_api.py b/tests/instrument_server/test_api.py
index dbf076ae1..737f1ee9f 100644
--- a/tests/instrument_server/test_api.py
+++ b/tests/instrument_server/test_api.py
@@ -71,7 +71,7 @@ def test_upload_gain_reference(
         "gain_destination_dir": "processing",
     }
     response = client.post(
-        "/instruments/m02/session/1/upload_gain_reference", json=payload
+        "/instruments/m02/sessions/1/upload_gain_reference", json=payload
     )
 
     # Check that the machine config request was called

From 914e9973f76c36f396b5f2110d9cdc3c7426c806 Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Wed, 9 Apr 2025 17:54:59 +0100
Subject: [PATCH 09/27] Commented out 'mock_request' to see if test function
 actually runs

---
 tests/instrument_server/test_api.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/instrument_server/test_api.py b/tests/instrument_server/test_api.py
index 737f1ee9f..2b45e486f 100644
--- a/tests/instrument_server/test_api.py
+++ b/tests/instrument_server/test_api.py
@@ -75,7 +75,7 @@ def test_upload_gain_reference(
     )
 
     # Check that the machine config request was called
-    mock_request.get.assert_called_once()
+    # mock_request.get.assert_called_once()
 
     # If no rsync_url key is provided, or rsync_url key is empty,
     # This should default to the Murfey URL

From f242cd1056018214607cb9541e4f376c7e9dcab2 Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Wed, 9 Apr 2025 18:04:43 +0100
Subject: [PATCH 10/27] Adjusted assertion logic for urlparse result

---
 tests/instrument_server/test_api.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/instrument_server/test_api.py b/tests/instrument_server/test_api.py
index 2b45e486f..478da3387 100644
--- a/tests/instrument_server/test_api.py
+++ b/tests/instrument_server/test_api.py
@@ -35,7 +35,7 @@
 
 @mark.parametrize("test_params", test_upload_gain_reference_params_matrix)
 @patch("murfey.instrument_server.api.subprocess")
-@patch("murfey.instrument_server.api.urlparse", wraps=urlparse)
+@patch("murfey.instrument_server.api.urlparse", wrap=urlparse)
 @patch("murfey.instrument_server.api._get_murfey_url")
 @patch("murfey.instrument_server.api.requests")
 def test_upload_gain_reference(
@@ -79,10 +79,10 @@ def test_upload_gain_reference(
 
     # If no rsync_url key is provided, or rsync_url key is empty,
     # This should default to the Murfey URL
-    if not rsync_url:
-        assert spy_parse.return_value == urlparse(server_url)
-    else:
-        assert spy_parse.return_value == urlparse(rsync_url)
+    expected_urlparse = urlparse(server_url) if not rsync_url else urlparse(rsync_url)
+    assert expected_urlparse.scheme == spy_parse.return_value.scheme
+    assert expected_urlparse.netloc == spy_parse.return_value.netloc
+    assert expected_urlparse.path == spy_parse.return_value.path
 
     # Check that the subprocess was run
     mock_subprocess.run.assert_called_once()

From fb62ccca391baaa7bd41a9e4a4dddb5136302897 Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Wed, 9 Apr 2025 18:12:13 +0100
Subject: [PATCH 11/27] Adjusted assertion logic for urlparse return value

---
 tests/instrument_server/test_api.py | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tests/instrument_server/test_api.py b/tests/instrument_server/test_api.py
index 478da3387..447ea65ab 100644
--- a/tests/instrument_server/test_api.py
+++ b/tests/instrument_server/test_api.py
@@ -35,7 +35,7 @@
 
 @mark.parametrize("test_params", test_upload_gain_reference_params_matrix)
 @patch("murfey.instrument_server.api.subprocess")
-@patch("murfey.instrument_server.api.urlparse", wrap=urlparse)
+@patch("murfey.instrument_server.api.urlparse", wraps=urlparse)
 @patch("murfey.instrument_server.api._get_murfey_url")
 @patch("murfey.instrument_server.api.requests")
 def test_upload_gain_reference(
@@ -79,10 +79,11 @@ def test_upload_gain_reference(
 
     # If no rsync_url key is provided, or rsync_url key is empty,
     # This should default to the Murfey URL
+    returned_urlparse = spy_parse.return_value
     expected_urlparse = urlparse(server_url) if not rsync_url else urlparse(rsync_url)
-    assert expected_urlparse.scheme == spy_parse.return_value.scheme
-    assert expected_urlparse.netloc == spy_parse.return_value.netloc
-    assert expected_urlparse.path == spy_parse.return_value.path
+    assert expected_urlparse.scheme == returned_urlparse.scheme
+    assert expected_urlparse.netloc == returned_urlparse.netloc
+    assert expected_urlparse.path == returned_urlparse.path
 
     # Check that the subprocess was run
     mock_subprocess.run.assert_called_once()

From 2a489e7545fe1e791681d0e4f26d7f7d0d3be0b5 Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Wed, 9 Apr 2025 18:58:39 +0100
Subject: [PATCH 12/27] Check if endpoint is being poked correctly first

---
 tests/instrument_server/test_api.py | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/tests/instrument_server/test_api.py b/tests/instrument_server/test_api.py
index 447ea65ab..96e14f11e 100644
--- a/tests/instrument_server/test_api.py
+++ b/tests/instrument_server/test_api.py
@@ -74,19 +74,19 @@ def test_upload_gain_reference(
         "/instruments/m02/sessions/1/upload_gain_reference", json=payload
     )
 
-    # Check that the machine config request was called
+    # # Check that the machine config request was called
     # mock_request.get.assert_called_once()
 
-    # If no rsync_url key is provided, or rsync_url key is empty,
-    # This should default to the Murfey URL
-    returned_urlparse = spy_parse.return_value
-    expected_urlparse = urlparse(server_url) if not rsync_url else urlparse(rsync_url)
-    assert expected_urlparse.scheme == returned_urlparse.scheme
-    assert expected_urlparse.netloc == returned_urlparse.netloc
-    assert expected_urlparse.path == returned_urlparse.path
+    # # If no rsync_url key is provided, or rsync_url key is empty,
+    # # This should default to the Murfey URL
+    # returned_urlparse = spy_parse.return_value
+    # expected_urlparse = urlparse(server_url) if not rsync_url else urlparse(rsync_url)
+    # assert expected_urlparse.scheme == returned_urlparse.scheme
+    # assert expected_urlparse.netloc == returned_urlparse.netloc
+    # assert expected_urlparse.path == returned_urlparse.path
 
-    # Check that the subprocess was run
-    mock_subprocess.run.assert_called_once()
+    # # Check that the subprocess was run
+    # mock_subprocess.run.assert_called_once()
 
     # Check that the endpoint function ran through to completion successfully
     assert response.status_code == 200

From 73beba4f119233825d07c283dd9ecd5ae9282082 Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Thu, 10 Apr 2025 11:18:26 +0100
Subject: [PATCH 13/27] Try testing the function by itself, and not as a
 FastAPI call

---
 tests/instrument_server/test_api.py | 100 ++++++++++++++--------------
 1 file changed, 49 insertions(+), 51 deletions(-)

diff --git a/tests/instrument_server/test_api.py b/tests/instrument_server/test_api.py
index 96e14f11e..5b7dd452e 100644
--- a/tests/instrument_server/test_api.py
+++ b/tests/instrument_server/test_api.py
@@ -1,60 +1,43 @@
+from pathlib import Path
 from typing import Optional
 from unittest.mock import Mock, patch
 from urllib.parse import urlparse
 
-from fastapi import FastAPI
-from fastapi.testclient import TestClient
 from pytest import mark
 
-from murfey.instrument_server.api import router
-
-app = FastAPI()
-app.include_router(router)
-client = TestClient(app)
-
+from murfey.instrument_server.api import GainReference, upload_gain_reference
+from murfey.util import posix_path
 
 test_upload_gain_reference_params_matrix = (
-    # Rsync URL | Rsync module | Gain reference directory |
-    (
-        "http://1.1.1.1",  # When rsync_url is provided
-        "data",
-        "/c/ProgramData/Gatan/Gain Reference",
-    ),
-    (
-        "",  # When rsync_url is blank
-        "data",
-        "/c/ProgramData/Gatan/Gain Reference",
-    ),
-    (
-        None,  # When rsync_url not provided
-        "data",
-        "/c/ProgramData/Gatan/Gain Reference",
-    ),
+    # Rsync URL
+    ("http://1.1.1.1",),  # When rsync_url is provided
+    ("",),  # When rsync_url is blank
+    (None,),  # When rsync_url not provided
 )
 
 
 @mark.parametrize("test_params", test_upload_gain_reference_params_matrix)
 @patch("murfey.instrument_server.api.subprocess")
-@patch("murfey.instrument_server.api.urlparse", wraps=urlparse)
 @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,
-    spy_parse,
     mock_subprocess,
     test_params: tuple[Optional[str], str, str],
 ):
 
     # Create a mock machine config base on the test params
-    rsync_url, rsync_module, gain_ref_dir = test_params
+    rsync_url_setting, rsync_module, gain_ref_dir = test_params
     server_url = "http://0.0.0.0:8000"
+    rsync_module = "data"
+    gain_ref_dir = "C:/ProgramData/Gatan/Gain Reference"
     mock_machine_config = {
         "rsync_module": rsync_module,
         "gain_reference_directory": gain_ref_dir,
     }
-    if rsync_url is not None:
-        mock_machine_config["rsync_url"] = rsync_url
+    if rsync_url_setting is not None:
+        mock_machine_config["rsync_url"] = rsync_url_setting
 
     # Assign expected values to the mock objects
     mock_response = Mock()
@@ -62,32 +45,47 @@ def test_upload_gain_reference(
     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 = Mock(
+        returncode=0, stderr="An error has occurred."
+    )
 
-    # Construct payload and submit post request
+    # Construct payload and pass request to function
+    gain_ref_file = f"{gain_ref_dir}/gain.mrc"
+    visit_path = "2025/aa00000-0"
+    gain_dest_dir = "processing"
     payload = {
-        "gain_path": f"{gain_ref_dir}/gain.mrc",
-        "visit_path": "2025/aa00000-0",
-        "gain_destination_dir": "processing",
+        "gain_path": gain_ref_file,
+        "visit_path": visit_path,
+        "gain_destination_dir": gain_dest_dir,
     }
-    response = client.post(
-        "/instruments/m02/sessions/1/upload_gain_reference", json=payload
+    result = upload_gain_reference(
+        instrument_name="murfey",
+        session_id=1,
+        gain_reference=GainReference(
+            **payload,
+        ),
     )
 
-    # # Check that the machine config request was called
-    # mock_request.get.assert_called_once()
+    # Check that the machine config request was called
+    mock_request.get.assert_called_once()
 
-    # # If no rsync_url key is provided, or rsync_url key is empty,
-    # # This should default to the Murfey URL
-    # returned_urlparse = spy_parse.return_value
-    # expected_urlparse = urlparse(server_url) if not rsync_url else urlparse(rsync_url)
-    # assert expected_urlparse.scheme == returned_urlparse.scheme
-    # assert expected_urlparse.netloc == returned_urlparse.netloc
-    # assert expected_urlparse.path == returned_urlparse.path
-
-    # # Check that the subprocess was run
-    # mock_subprocess.run.assert_called_once()
+    # Check that the subprocess was run with the expected arguments
+    # If no rsync_url key is provided, or rsync_url key is empty,
+    # It should default to the server URL
+    expected_rsync_url = (
+        urlparse(server_url) if not rsync_url_setting else urlparse(rsync_url_setting)
+    )
+    expected_rsync_path = f"{expected_rsync_url.hostname}::{rsync_module}/{visit_path}/{gain_dest_dir}/gain.mrc"
+    expected_rsync_cmd = [
+        "rsync",
+        posix_path(Path(gain_ref_file)),
+        expected_rsync_path,
+    ]
+    mock_subprocess.run.assert_called_once_with(
+        expected_rsync_cmd,
+        capture_output=True,
+        text=True,
+    )
 
-    # Check that the endpoint function ran through to completion successfully
-    assert response.status_code == 200
-    assert response.json() == {"success": True}
+    # Check that the function ran through to completion successfully
+    assert result == {"success": True}

From 4a1548e8d0b88b7e284d9240cee177fe843c7494 Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Thu, 10 Apr 2025 11:23:44 +0100
Subject: [PATCH 14/27] Forgot remove extra variables after changing length of
 parameter tuple being passed in

---
 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 5b7dd452e..7d36203b8 100644
--- a/tests/instrument_server/test_api.py
+++ b/tests/instrument_server/test_api.py
@@ -9,7 +9,7 @@
 from murfey.util import posix_path
 
 test_upload_gain_reference_params_matrix = (
-    # Rsync URL
+    # Rsync URL settings
     ("http://1.1.1.1",),  # When rsync_url is provided
     ("",),  # When rsync_url is blank
     (None,),  # When rsync_url not provided
@@ -24,11 +24,11 @@ def test_upload_gain_reference(
     mock_request,
     mock_get_server_url,
     mock_subprocess,
-    test_params: tuple[Optional[str], str, str],
+    test_params: tuple[Optional[str]],
 ):
 
     # Create a mock machine config base on the test params
-    rsync_url_setting, rsync_module, gain_ref_dir = test_params
+    (rsync_url_setting,) = test_params
     server_url = "http://0.0.0.0:8000"
     rsync_module = "data"
     gain_ref_dir = "C:/ProgramData/Gatan/Gain Reference"

From cfc48cb9f796fe9fb6a5cd97404e0d21cd1b8c74 Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Thu, 10 Apr 2025 11:47:44 +0100
Subject: [PATCH 15/27] Check contents of request.get() call

---
 tests/instrument_server/test_api.py | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/tests/instrument_server/test_api.py b/tests/instrument_server/test_api.py
index 7d36203b8..26e76d652 100644
--- a/tests/instrument_server/test_api.py
+++ b/tests/instrument_server/test_api.py
@@ -18,18 +18,24 @@
 
 @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,
     test_params: tuple[Optional[str]],
 ):
 
-    # Create a mock machine config base on the test params
+    # 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
+
+    # Create a mock machine config base on the test params
     rsync_module = "data"
     gain_ref_dir = "C:/ProgramData/Gatan/Gain Reference"
     mock_machine_config = {
@@ -48,7 +54,9 @@ def test_upload_gain_reference(
     mock_subprocess.run.return_value = Mock(
         returncode=0, stderr="An error has occurred."
     )
-
+    mock_tokens = {
+        session_id: "hello",
+    }
     # Construct payload and pass request to function
     gain_ref_file = f"{gain_ref_dir}/gain.mrc"
     visit_path = "2025/aa00000-0"
@@ -59,15 +67,19 @@ def test_upload_gain_reference(
         "gain_destination_dir": gain_dest_dir,
     }
     result = upload_gain_reference(
-        instrument_name="murfey",
-        session_id=1,
+        instrument_name=instrument_name,
+        session_id=session_id,
         gain_reference=GainReference(
             **payload,
         ),
     )
 
     # Check that the machine config request was called
-    mock_request.get.assert_called_once()
+    machine_config_url = f"{server_url}/instruments/{instrument_name}/machine"
+    mock_request.get.assert_called_once_with(
+        machine_config_url,
+        headers={"Authorization": f"Bearer {mock_tokens[session_id]}"},
+    )
 
     # Check that the subprocess was run with the expected arguments
     # If no rsync_url key is provided, or rsync_url key is empty,

From 25725e3572aa73d615f636bb136dffca2b0a8c50 Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Thu, 10 Apr 2025 11:55:26 +0100
Subject: [PATCH 16/27] Try using unittest.mock.ANY to skip token
 authentication check

---
 tests/instrument_server/test_api.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/instrument_server/test_api.py b/tests/instrument_server/test_api.py
index 26e76d652..42c8bb8a6 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 Mock, patch
+from unittest.mock import ANY, Mock, patch
 from urllib.parse import urlparse
 
 from pytest import mark
@@ -54,9 +54,9 @@ def test_upload_gain_reference(
     mock_subprocess.run.return_value = Mock(
         returncode=0, stderr="An error has occurred."
     )
-    mock_tokens = {
-        session_id: "hello",
-    }
+    # mock_tokens = {
+    #     session_id: "hello",
+    # }
     # Construct payload and pass request to function
     gain_ref_file = f"{gain_ref_dir}/gain.mrc"
     visit_path = "2025/aa00000-0"
@@ -78,7 +78,7 @@ def test_upload_gain_reference(
     machine_config_url = f"{server_url}/instruments/{instrument_name}/machine"
     mock_request.get.assert_called_once_with(
         machine_config_url,
-        headers={"Authorization": f"Bearer {mock_tokens[session_id]}"},
+        headers={"Authorization": ANY},
     )
 
     # Check that the subprocess was run with the expected arguments

From 26b0b174a2a610e019ab71bbfa2dc026afe9fa52 Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Thu, 10 Apr 2025 12:08:05 +0100
Subject: [PATCH 17/27] Added fixture to simulate a pre-loaded client-side
 configuration ConfigParser object

---
 tests/conftest.py | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/tests/conftest.py b/tests/conftest.py
index 6672188ba..9176d86a1 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -1,4 +1,5 @@
 import json
+from configparser import ConfigParser
 
 import pytest
 from sqlmodel import Session
@@ -21,6 +22,20 @@ def start_postgres():
         murfey_db.commit()
 
 
+@pytest.fixture()
+def mock_client_configuration() -> ConfigParser:
+    """
+    Returns the client-side configuration file as a pre-loaded ConfigParser object.
+    """
+    config = ConfigParser()
+    config["Murfey"] = {
+        "instrument_name": "murfey",
+        "server": "http://0.0.0.0:8000",
+        "token": "pneumonoultramicroscopicsilicovolcanoconiosis",
+    }
+    return config
+
+
 @pytest.fixture()
 def mock_security_configuration(tmp_path):
     config_file = tmp_path / mock_security_config_name

From a4a274b04b26253b094e91dde4299c6bcfbc43e8 Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Thu, 10 Apr 2025 12:24:45 +0100
Subject: [PATCH 18/27] Added test for '_get_murfey_url' function

---
 tests/instrument_server/test_api.py | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/tests/instrument_server/test_api.py b/tests/instrument_server/test_api.py
index 42c8bb8a6..75434b805 100644
--- a/tests/instrument_server/test_api.py
+++ b/tests/instrument_server/test_api.py
@@ -5,9 +5,25 @@
 
 from pytest import mark
 
-from murfey.instrument_server.api import GainReference, upload_gain_reference
+from murfey.instrument_server.api import (
+    GainReference,
+    _get_murfey_url,
+    upload_gain_reference,
+)
 from murfey.util import posix_path
 
+
+def test_get_murfey_url(
+    mock_client_configuration,  # From conftest.py
+):
+    # Mock the module-wide config variable with the fixture value
+    # The fixture is only loaded within the test function, so this patch
+    # has to happen inside the function instead of as a decorator
+    with patch("murfey.instrument_server.api.config", mock_client_configuration):
+        known_server = _get_murfey_url()
+        assert known_server == mock_client_configuration["Murfey"].get("server")
+
+
 test_upload_gain_reference_params_matrix = (
     # Rsync URL settings
     ("http://1.1.1.1",),  # When rsync_url is provided

From 7d08e2ed06a4798f8e24fb93dae4a3c17128f0e3 Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Thu, 10 Apr 2025 12:52:00 +0100
Subject: [PATCH 19/27] Parametrised the '_get_murfey_url' test

---
 tests/instrument_server/test_api.py | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/tests/instrument_server/test_api.py b/tests/instrument_server/test_api.py
index 75434b805..b79c96c17 100644
--- a/tests/instrument_server/test_api.py
+++ b/tests/instrument_server/test_api.py
@@ -12,16 +12,40 @@
 )
 from murfey.util import posix_path
 
+test_get_murfey_url_params_matrix = (
+    # Server URL to use
+    ("default",),
+    ("0.0.0.0:8000",),
+    ("murfey_server",),
+    ("http://murfey_server:8000",),
+    ("http://murfey_server:8080/api",),
+)
+
 
+@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
 ):
+    # Unpack test_params
+    (server_url_to_test,) = test_params
+
+    # Replace the server URL from the fixture with other ones for testing
+    if server_url_to_test != "default":
+        mock_client_configuration["Murfey"]["server"] = server_url_to_test
+
     # Mock the module-wide config variable with the fixture value
     # The fixture is only loaded within the test function, so this patch
     # has to happen inside the function instead of as a decorator
     with patch("murfey.instrument_server.api.config", mock_client_configuration):
         known_server = _get_murfey_url()
-        assert known_server == mock_client_configuration["Murfey"].get("server")
+        parsed_server = urlparse(known_server)
+        parsed_original = urlparse(
+            str(mock_client_configuration["Murfey"].get("server"))
+        )
+        assert parsed_server.scheme in ("http", "https")
+        assert parsed_server.netloc == parsed_original.netloc
+        assert parsed_server.path == parsed_original.path
 
 
 test_upload_gain_reference_params_matrix = (

From c196e6bf6b0806cd1a9529b20bd5a28a65b43a3b Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Thu, 10 Apr 2025 12:54:12 +0100
Subject: [PATCH 20/27] Code block shouldn't have to be indented

---
 tests/instrument_server/test_api.py | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/instrument_server/test_api.py b/tests/instrument_server/test_api.py
index b79c96c17..1b0467ab2 100644
--- a/tests/instrument_server/test_api.py
+++ b/tests/instrument_server/test_api.py
@@ -39,13 +39,13 @@ def test_get_murfey_url(
     # has to happen inside the function instead of as a decorator
     with patch("murfey.instrument_server.api.config", mock_client_configuration):
         known_server = _get_murfey_url()
-        parsed_server = urlparse(known_server)
-        parsed_original = urlparse(
-            str(mock_client_configuration["Murfey"].get("server"))
-        )
-        assert parsed_server.scheme in ("http", "https")
-        assert parsed_server.netloc == parsed_original.netloc
-        assert parsed_server.path == parsed_original.path
+
+    # Check that the components of the result match those in the config
+    parsed_server = urlparse(known_server)
+    parsed_original = urlparse(str(mock_client_configuration["Murfey"].get("server")))
+    assert parsed_server.scheme in ("http", "https")
+    assert parsed_server.netloc == parsed_original.netloc
+    assert parsed_server.path == parsed_original.path
 
 
 test_upload_gain_reference_params_matrix = (

From bd8883f2664e94904dd9a532ee7c3f6dd813dd26 Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Thu, 10 Apr 2025 13:02:43 +0100
Subject: [PATCH 21/27] Added 'http://' to URL test cases with no schemes when
 performing comparison at the end

---
 tests/instrument_server/test_api.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/instrument_server/test_api.py b/tests/instrument_server/test_api.py
index 1b0467ab2..6da464e1d 100644
--- a/tests/instrument_server/test_api.py
+++ b/tests/instrument_server/test_api.py
@@ -42,7 +42,10 @@ def test_get_murfey_url(
 
     # Check that the components of the result match those in the config
     parsed_server = urlparse(known_server)
-    parsed_original = urlparse(str(mock_client_configuration["Murfey"].get("server")))
+    original_url = str(mock_client_configuration["Murfey"].get("server"))
+    if not original_url.startswith(("http://", "https://")):
+        original_url = f"http://{original_url}"
+    parsed_original = urlparse(original_url)
     assert parsed_server.scheme in ("http", "https")
     assert parsed_server.netloc == parsed_original.netloc
     assert parsed_server.path == parsed_original.path

From b27585c829368e5b44d7702f536d6f532310470d Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Thu, 10 Apr 2025 13:06:04 +0100
Subject: [PATCH 22/27] 'tokens' variable should not require mocking anymore

---
 tests/instrument_server/test_api.py | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/tests/instrument_server/test_api.py b/tests/instrument_server/test_api.py
index 6da464e1d..808023b89 100644
--- a/tests/instrument_server/test_api.py
+++ b/tests/instrument_server/test_api.py
@@ -61,13 +61,11 @@ def test_get_murfey_url(
 
 @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,
     test_params: tuple[Optional[str]],
 ):
@@ -97,9 +95,7 @@ def test_upload_gain_reference(
     mock_subprocess.run.return_value = Mock(
         returncode=0, stderr="An error has occurred."
     )
-    # mock_tokens = {
-    #     session_id: "hello",
-    # }
+
     # Construct payload and pass request to function
     gain_ref_file = f"{gain_ref_dir}/gain.mrc"
     visit_path = "2025/aa00000-0"

From 75c8e41dfbdecde7b70c1267deb130428ccd1d23 Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Thu, 10 Apr 2025 13:09:32 +0100
Subject: [PATCH 23/27] Turns out the token still needs to be mocked, oops

---
 tests/instrument_server/test_api.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/instrument_server/test_api.py b/tests/instrument_server/test_api.py
index 808023b89..54cb84096 100644
--- a/tests/instrument_server/test_api.py
+++ b/tests/instrument_server/test_api.py
@@ -61,11 +61,13 @@ def test_get_murfey_url(
 
 @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,
     test_params: tuple[Optional[str]],
 ):

From 87e91f64e7cc2a97b4529b353182b43c338325a8 Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Thu, 10 Apr 2025 14:13:27 +0100
Subject: [PATCH 24/27] Removed residual comment

---
 src/murfey/util/models.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/murfey/util/models.py b/src/murfey/util/models.py
index c000de7c7..b9a5e60b3 100644
--- a/src/murfey/util/models.py
+++ b/src/murfey/util/models.py
@@ -368,7 +368,7 @@ class PreprocessingParametersTomo(BaseModel):
     eer_fractionation: int
 
     class Base(BaseModel):
-        dose_per_frame: Optional[float]  # Doesn't match the model?
+        dose_per_frame: Optional[float]
         gain_ref: Optional[str]
         manual_tilt_offset: float
         eer_fractionation: int

From 6bcb2b5e1191ffeae6f1593958713c888e27fd06 Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Thu, 10 Apr 2025 14:15:26 +0100
Subject: [PATCH 25/27] Does 'stderr' need to be set if that branch of logic is
 not tested here?

---
 tests/instrument_server/test_api.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tests/instrument_server/test_api.py b/tests/instrument_server/test_api.py
index 54cb84096..7c5765dc8 100644
--- a/tests/instrument_server/test_api.py
+++ b/tests/instrument_server/test_api.py
@@ -94,9 +94,7 @@ def test_upload_gain_reference(
     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, stderr="An error has occurred."
-    )
+    mock_subprocess.run.return_value = Mock(returncode=0)
 
     # Construct payload and pass request to function
     gain_ref_file = f"{gain_ref_dir}/gain.mrc"

From 6cc5c532bb0a2fe4fad60762037998ed870cd8bf Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Thu, 10 Apr 2025 14:28:20 +0100
Subject: [PATCH 26/27] Test for 'port' and 'hostname' attributes as well

---
 tests/instrument_server/test_api.py | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tests/instrument_server/test_api.py b/tests/instrument_server/test_api.py
index 7c5765dc8..0cc1ba50c 100644
--- a/tests/instrument_server/test_api.py
+++ b/tests/instrument_server/test_api.py
@@ -40,13 +40,17 @@ def test_get_murfey_url(
     with patch("murfey.instrument_server.api.config", mock_client_configuration):
         known_server = _get_murfey_url()
 
-    # Check that the components of the result match those in the config
-    parsed_server = urlparse(known_server)
+    # Prepend 'http://' to config URLs that don't have it for the comparison
+    # Otherwise, urlparse stores it under the 'path' attribute
     original_url = str(mock_client_configuration["Murfey"].get("server"))
     if not original_url.startswith(("http://", "https://")):
         original_url = f"http://{original_url}"
     parsed_original = urlparse(original_url)
+    parsed_server = urlparse(known_server)
+    # Check that the components of the result match those in the config
     assert parsed_server.scheme in ("http", "https")
+    assert parsed_server.hostname == parsed_original.hostname
+    assert parsed_server.port == parsed_original.port
     assert parsed_server.netloc == parsed_original.netloc
     assert parsed_server.path == parsed_original.path
 

From cc6477ee775434425ad0fd17fcfc1d8626c63d9f Mon Sep 17 00:00:00 2001
From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk>
Date: Thu, 10 Apr 2025 14:29:38 +0100
Subject: [PATCH 27/27] Rearranged code in blocks according to purpose

---
 tests/instrument_server/test_api.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/instrument_server/test_api.py b/tests/instrument_server/test_api.py
index 0cc1ba50c..b4fa9cd73 100644
--- a/tests/instrument_server/test_api.py
+++ b/tests/instrument_server/test_api.py
@@ -45,9 +45,10 @@ def test_get_murfey_url(
     original_url = str(mock_client_configuration["Murfey"].get("server"))
     if not original_url.startswith(("http://", "https://")):
         original_url = f"http://{original_url}"
+
+    # Check that the components of the result match those in the config
     parsed_original = urlparse(original_url)
     parsed_server = urlparse(known_server)
-    # Check that the components of the result match those in the config
     assert parsed_server.scheme in ("http", "https")
     assert parsed_server.hostname == parsed_original.hostname
     assert parsed_server.port == parsed_original.port