Standardise config client and fix tests dependent on order they are ran

docs/how-to/write-tests.md

@@ -121,6 +121,253 @@ async def test_my_device_read(sim_my_device: MyDevice, run_engine: RunEngine) ->
     )
 ```
 
+## Mocking `ConfigClient`
+
+Beamlines and devices may depend on the daq-config-server `ConfigClient` to retrieve configuration files. These files are often stored on the DLS filesystem and contain configuration data that determines device behaviour.
+
+For example, an insertion device may use a calibration file containing polynomial coefficients that convert photon energy into insertion device gap for different polarisation modes. A device may retrieve this information using a `ConfigClient`:
+
+```python
+from pathlib import Path
+
+from daq_config_server import ConfigClient
+
+from dodal.device_manager import DeviceManager
+from dodal.devices.my_device import MyDevice
+
+LOOKUPTABLE_DIR = (
+    "/dls_sw/iXX/software/gda/workspace_git/"
+    "gda-diamond.git/configurations/iXX/lookupTables"
+)
+LOOKUP_FILE_NAME = "JIDEnergy2GapCalibrations.csv"
+
+devices = DeviceManager()
+
+
+@devices.fixture
+def config_client() -> ConfigClient:
+    return ConfigClient()
+
+
+@devices.factory()
+def my_device(config_client: ConfigClient) -> MyDevice:
+    return MyDevice(
+        config_client=config_client,
+        path=Path(LOOKUPTABLE_DIR, LOOKUP_FILE_NAME),
+    )
+```
+
+### Directly mocking return values
+
+If a test does not need to exercise file parsing logic, it is often simpler to mock the ConfigClient response directly rather than creating a test file and configuring a parser.
+
+```python
+from unittest.mock import MagicMock
+
+from daq_config_server.models.lookup_tables.insertion_device import (
+    UndulatorEnergyGapLookupTable,
+)
+
+@pytest.fixture
+def mock_config_client():
+    mock_config_client = MagicMock()
+    mock_config_client.get_file_contents = MagicMock(
+        return_value=UndulatorEnergyGapLookupTable(
+            rows=[
+                [5700, 5.4606],
+                [7000, 6.045],
+                [9700, 6.404],
+            ],
+        )
+    )
+    return mock_config_client
+```
+This approach is appropriate when:
+
+* The test only cares about the behaviour of the device or component consuming the configuration data.
+* The contents of the configuration file are not relevant to the test.
+* The parsing logic is tested elsewhere.
+
+In these cases, mocking the return value directly avoids unnecessary coupling to file formats or test data files.
+
+Use the below method if you need the data to come from a file for a unit test.
+
+### Using the mock_config_client fixture to read files
+Unit tests should not communicate with the real daq-config-server or read files from the DLS filesystem.
+
+Dodal provides a `mock_config_client` fixture in `dodal.testing.fixtures.config_client`. This fixture replaces the real service with a lightweight implementation that reads test files directly from the local filesystem.
+
+Most tests only need to depend on this fixture:
+
+```python
+import pytest
+from pathlib import Path
+
+from daq_config_server import ConfigClient
+from dodal.devices.my_device import MyDevice
+from ophyd_async.core import init_devices
+
+
+@pytest.fixture
+def my_device(mock_config_client: ConfigClient) -> MyDevice:
+    with init_devices(mock=True):
+        return MyDevice(
+            config_client=mock_config_client,
+            path=Path("tests/test_data/example_lookup_table.json"),
+        )
+```
+
+### Default file parsing behaviour
+
+The mocked `ConfigClient.get_file_contents()` supports the same return types commonly used by production code:
+
+| Requested type         | Behaviour                                                         |
+| ---------------------- | ----------------------------------------------------------------- |
+| `str`                  | Returns the raw file contents                                     |
+| `dict`                 | Parses the file as JSON and returns a dictionary                  |
+| `ConfigModel` subclass | Parses the file as JSON and validates it using `model_validate()` |
+
+For example, if the requested return type is a `ConfigModel` subclass:
+
+```python
+config_client.get_file_contents(
+    path,
+    desired_return_type=MyConfigModel,
+)
+```
+
+the mock implementation will perform:
+
+```python
+MyConfigModel.model_validate(json.loads(contents))
+```
+
+### Registering a custom parser
+
+Some configuration file formats are not JSON-based and require custom parsing logic. For these cases, tests can register a parser using `MOCK_CONFIG_CLIENT_PATH_TO_MODEL_CONVERSION`.
+
+This registry maps a file path to a function that converts the file contents into a `ConfigModel`.
+
+For example:
+
+```python
+import pytest
+from daq_config_server import ConfigClient
+from daq_config_server.models.lookup_tables.insertion_device import (
+    parse_i09_hu_undulator_energy_gap_lut,
+)
+
+from dodal.testing.fixtures.config_client import (
+    MOCK_CONFIG_CLIENT_PATH_TO_MODEL_CONVERSION,
+)
+from tests.devices.beamlines.i09_1_shared.test_data import (
+    TEST_HARD_UNDULATOR_LUT,
+)
+
+
+@pytest.fixture
+def mock_config_client(mock_config_client: ConfigClient):
+    MOCK_CONFIG_CLIENT_PATH_TO_MODEL_CONVERSION[
+        TEST_HARD_UNDULATOR_LUT
+    ] = parse_i09_hu_undulator_energy_gap_lut
+
+    yield mock_config_client
+
+    MOCK_CONFIG_CLIENT_PATH_TO_MODEL_CONVERSION.pop(
+        TEST_HARD_UNDULATOR_LUT,
+        None,
+    )
+```
+
+When `get_file_contents()` is called for `TEST_HARD_UNDULATOR_LUT`, the registered parser will be used instead of the default JSON parsing behaviour.
+
+This mechanism exists as a temporary workaround for configuration formats that are not yet directly supported by daq-config-server.
+
+## Testing beamline module imports and device creation
+
+The tests in `tests/common/beamlines/test_device_instantiation.py` validate that every beamline module can be imported and that all device factories successfully construct devices in a test environment.
+
+During import and device creation, many beamlines load configuration files using either:
+
+* `ConfigClient.get_file_contents()`
+* Module-level path constants that point to files on the DLS filesystem
+
+For example:
+
+```python
+BEAMLINE_PARAMETERS_PATH = (
+    "/dls_sw/i03/software/daq_configuration/json/beamlineParameters.json"
+)
+```
+
+These files are not available in unit test environments and must not be accessed during tests.
+
+To allow beamline import and device creation tests to run without access to the DLS filesystem, the `module_and_devices_for_beamline` fixture performs two forms of mocking:
+
+1. Replaces `ConfigClient.get_file_contents()` with the `mock_config_client` implementation.
+2. Replaces known module-level file path constants with paths to test data files.
+
+### Mocking `ConfigClient`
+
+Before importing the beamline module, the fixture replaces the production implementation:
+
+```python
+ConfigClient.get_file_contents = mock_config_client.get_file_contents
+```
+
+This ensures that any configuration requested through the daq-config-server client is loaded from local test files rather than the real service.
+
+### Mocking beamline file paths
+
+Some beamlines contain module-level constants that directly reference files on the DLS filesystem. These constants are replaced after module import using the `MOCK_ATTRIBUTES_TABLE`.
+
+For example:
+
+```python
+MOCK_ATTRIBUTES_TABLE = {
+    "i03": {
+        "BEAMLINE_PARAMETERS_PATH": TEST_BEAMLINE_PARAMETERS_TXT,
+        "DAQ_CONFIGURATION_PATH": MOCK_DAQ_CONFIG_PATH,
+        "ZOOM_PARAMS_FILE": TEST_OAV_ZOOM_LEVELS,
+        "DISPLAY_CONFIG": TEST_DISPLAY_CONFIG,
+    },
+}
+```
+
+When the `i03` beamline is tested, these attributes are overwritten with paths to local test files:
+
+```python
+mock_beamline_module_filepaths("i03", beamline_module)
+```
+
+This allows device factories to load representative configuration data without accessing the real DLS filesystem.
+
+### Adding support for a new beamline
+
+If the device creation tests fail because a beamline attempts to access a file under `/dls` or `/dls_sw`, identify the module attribute containing the path and add an entry to `MOCK_ATTRIBUTES_TABLE`.
+
+For example:
+
+```python
+MOCK_ATTRIBUTES_TABLE = { 
+    ...
+    "iXX: {
+        "MY_CONFIGURATION_FILE": TEST_MY_CONFIGURATION_FILE
+    },
+    ...
+}
+```
+
+The replacement file should be committed to the test suite and contain the minimum configuration required for the beamline to initialise successfully.
+
+As a rule of thumb:
+
+* Use `mock_config_client` when configuration is loaded through `ConfigClient`.
+* Use `MOCK_ATTRIBUTES_TABLE` when a beamline module contains hard-coded filesystem paths.
+* If both mechanisms are used by a beamline, both may need to be configured for the tests to pass.
+
+
+
 ## Test performance and reliability
 
 Dodal has well over 1000 unit tests and developers will run the full unit test suite frequently on their local 

pyproject.toml

@@ -24,16 +24,16 @@ dependencies = [
     "requests",
     "graypy",
     "pydantic>=2.0",
-    "opencv-python-headless",                                                                   # For pin-tip detection.
+    "opencv-python-headless",          # For pin-tip detection.
     "numpy",
     "aiofiles",
     "aiohttp",
     "redis",
     "scanspec>=0.7.3",
     "pyzmq==27.1.0",
     "deepdiff",
-    "daq-config-server >= 1.3.6", # For getting Configuration settings.
-    "mysql-connector-python == 9.5.0",                                                          # Can unpin once https://github.com/DiamondLightSource/ispyb-api/pull/244 is merged and released
+    "daq-config-server >= 1.3.6",      # For getting Configuration settings.
+    "mysql-connector-python == 9.5.0", # Can unpin once https://github.com/DiamondLightSource/ispyb-api/pull/244 is merged and released
 ]
 
 dynamic = ["version"]
@@ -113,9 +113,10 @@ typeCheckingMode = "standard"
 reportMissingImports = false  # Ignore missing stubs in imported modules
 
 [tool.pytest.ini_options]
-# Run pytest with all our checkers, and don't spam us with massive tracebacks on error
+# Run pytest with all our checkers
 asyncio_mode = "auto"
 timeout = 1
+timeout_method = "signal" # Alllow us to see full stack trace of where it timed out to make debugging easier.
 markers = [
     "requires: marks tests as requiring other infrastructure",
     "skip_in_pycharm: marks test as not working in pycharm testrunner",

src/dodal/beamlines/i09_1_shared.py

@@ -1,6 +1,5 @@
 from daq_config_server import ConfigClient
 
-from dodal.common.beamlines.beamline_utils import get_config_client, set_config_client
 from dodal.device_manager import DeviceManager
 from dodal.devices.beamlines.i09_1_shared import (
     HardEnergy,
@@ -30,10 +29,14 @@
 
 devices = DeviceManager()
 
-set_config_client(ConfigClient())
 LOOK_UPTABLE_FILE = "/dls_sw/i09-1/software/gda/workspace_git/gda-diamond.git/configurations/i09-1-shared/lookupTables/IIDCalibrationTable.txt"
 
 
+@devices.fixture
+def iconfig_client() -> ConfigClient:
+    return ConfigClient()
+
+
 @devices.factory()
 def psi1() -> HutchShutter:
     return HutchShutter(I_PREFIX.beamline_prefix)
@@ -60,12 +63,14 @@ def ienergy_order() -> UndulatorOrder:
 
 @devices.factory()
 def iidenergy(
-    ienergy_order: UndulatorOrder, iid: UndulatorInMm
+    ienergy_order: UndulatorOrder,
+    iid: UndulatorInMm,
+    iconfig_client: ConfigClient,
 ) -> HardInsertionDeviceEnergy:
     return HardInsertionDeviceEnergy(
         undulator_order=ienergy_order,
         undulator=iid,
-        config_server=get_config_client(),
+        config_server=iconfig_client,
         filepath=LOOK_UPTABLE_FILE,
         gap_to_energy_func=calculate_energy_i09_hu,
         energy_to_gap_func=calculate_gap_i09_hu,

src/dodal/beamlines/i09_2_shared.py

@@ -25,7 +25,6 @@
 from dodal.devices.pgm import PlaneGratingMonochromator
 from dodal.utils import BeamlinePrefix, get_beamline_name
 
-J09_CONF_CLIENT = ConfigClient(url="https://daq-config.diamond.ac.uk")
 LOOK_UPTABLE_DIR = "/dls_sw/i09-2/software/gda/workspace_git/gda-diamond.git/configurations/i09-2-shared/lookupTables/"
 GAP_LOOKUP_FILE_NAME = "JIDEnergy2GapCalibrations.csv"
 PHASE_LOOKUP_FILE_NAME = "JIDEnergy2PhaseCalibrations.csv"
@@ -37,6 +36,11 @@
 devices = DeviceManager()
 
 
+@devices.fixture
+def jconfig_client() -> ConfigClient:
+    return ConfigClient()
+
+
 @devices.factory()
 def psk1() -> HutchShutter:
     return HutchShutter(K_PREFIX.beamline_prefix)
@@ -75,18 +79,19 @@ def jid(jgap: UndulatorGap, jphase: UndulatorPhaseAxes) -> Apple2[UndulatorPhase
 @devices.factory()
 def jidcontroller(
     jid: Apple2[UndulatorPhaseAxes],
+    jconfig_client: ConfigClient,
 ) -> Apple2EnforceLHMoveController[UndulatorPhaseAxes]:
     """J09 insertion device controller."""
     return Apple2EnforceLHMoveController[UndulatorPhaseAxes](
         apple2=jid,
         gap_energy_motor_lut=ConfigServerEnergyMotorLookup(
             lut_config=LookupTableColumnConfig(poly_deg=J09_GAP_POLY_DEG_COLUMNS),
-            config_client=J09_CONF_CLIENT,
+            config_client=jconfig_client,
             path=Path(LOOK_UPTABLE_DIR, GAP_LOOKUP_FILE_NAME),
         ),
         phase_energy_motor_lut=ConfigServerEnergyMotorLookup(
             lut_config=LookupTableColumnConfig(poly_deg=J09_PHASE_POLY_DEG_COLUMNS),
-            config_client=J09_CONF_CLIENT,
+            config_client=jconfig_client,
             path=Path(LOOK_UPTABLE_DIR, PHASE_LOOKUP_FILE_NAME),
         ),
         units="keV",