Organise testing utils #567
Conversation
- and sort test utils into submodules
…ource/dodal into 564_organise_testing_utils
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #567 +/- ##
==========================================
+ Coverage 92.78% 93.20% +0.42%
==========================================
Files 99 104 +5
Lines 3769 4062 +293
==========================================
+ Hits 3497 3786 +289
- Misses 272 276 +4 ☔ View full report in Codecov by Sentry. |
|
|
||
|
|
||
| async def get_mock_device() -> Synchrotron: | ||
| async with DeviceCollector(mock=True): |
There was a problem hiding this comment.
Should: I think from looking at the docs DeviceCollector doesn't need to be async e.g. you can just do:
with DeviceCollector(mock=True):
device = Synchrotron()Then the whole function can be synchronous too. This is true in multiple places in this PR, can you do the same there?
src/dodal/testing_utils/constants.py
Outdated
| SYNCHR_RING_CURRENT = 0.556677 | ||
| SYNCHR_USER_COUNTDOWN = 55.0 | ||
| SYNCHR_START_COUNTDOWN = 66.0 | ||
| SYNCHR_END_COUNTDOWN = 77.0 | ||
| SYNCHR_BEAM_ENERGY = 3.0158 | ||
| SYNCHR_MODE = "Injection" | ||
| SYNCHR_NUMBER = "number" | ||
| SYNCHR_STRING = "string" |
There was a problem hiding this comment.
Could: I sort of feel these should be with the device too but I don't want to pollute the device namespace too much. Maybe we add the convention of as well as get_mock_device you also have a dict/dataclass of TESTING_CONSTS?
| async def mock_aperturescatterguard_with_call_log(): | ||
| call_log = MagicMock() | ||
| ap_sg = await get_mock_ap_sg() | ||
| with ( |
There was a problem hiding this comment.
Should: patch_ophyd_async_motor isn't a context manager
There was a problem hiding this comment.
yes it is, it's type-hinted wrong in ophyd-async, fix has been merged but not released, doesn't affect functionality
| patch_ophyd_async_motor(ap_sg.aperture.y, call_log=call_log), | ||
| patch_ophyd_async_motor(ap_sg.aperture.z, call_log=call_log), | ||
| patch_ophyd_async_motor(ap_sg.scatterguard.x, call_log=call_log), | ||
| patch_ophyd_async_motor(ap_sg.scatterguard.y, call_log=call_log), |
There was a problem hiding this comment.
Should: Can we do this patching in get_mock_ap_sg?
There was a problem hiding this comment.
no, it leads to circular import errors if we import test utils to device classes and then device classes to test utils
|
|
||
|
|
||
| @pytest.fixture | ||
| def mock_eiger_and_stage(mock_eiger): |
There was a problem hiding this comment.
Nit: Rename to something like mock_eiger_with_noop_stage?
There was a problem hiding this comment.
haha, that's actually what I had first and I changed it to this
dperl-dls
force-pushed
the
564_organise_testing_utils
branch
from
dc9e8ea
to
34edbc3
Compare
dperl-dls
force-pushed
the
564_organise_testing_utils
branch
from
34edbc3
to
6aa2310
Compare
|
never mind, this is too hard, I now think it's better to just copy and paste stuff everywhere |
Fixes #564
Moves many mock device test fixtures and utility functions to a central place under
src/, so that they can easily be reused by application codeInstructions to reviewer on how to test:
Checks for reviewer