Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TST: Implement test IOC #34

Merged
merged 6 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions conda-recipe/meta.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ test:
imports:
- {{ import_name }}
requires:
- caproto
- coverage
- numpy
- pytest
- pytest-asyncio
- pytest-qt
Expand Down
2 changes: 2 additions & 0 deletions dev-requirements.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# These are required for developing the package (running the tests) but not
# necessarily required for _using_ it.
caproto
tangkong marked this conversation as resolved.
Show resolved Hide resolved
coverage
numpy
pytest
pytest-asyncio
pytest-cov
Expand Down
23 changes: 23 additions & 0 deletions docs/source/upcoming_release_notes/34-ioc_infrastructure.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
34 ioc infrastructure
#################

API Breaks
----------
- N/A

Features
--------
- implement fixture for running IOCs that can be queried for integration tests
- implement module that can run IOCs for demos

Bugfixes
--------
- don't change control_layer/core.py:SHIMS when creating dummy CLs

Maintenance
-----------
- define linac testing structure outside of fixture

Contributors
------------
- shilorigins
2 changes: 1 addition & 1 deletion superscore/control_layers/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class ControlLayer:
def __init__(self, *args, shims: Optional[List[str]] = None, **kwargs):
if shims is None:
# load all available shims
self.shims = SHIMS
self.shims = SHIMS.copy()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the record, I think this is fine and no changes need to be made. But I do want to clarify:

Was this changed to provide assurance that we couldn't accidentally clobber the global SHIMS? I think my intent was for there to be only one shim of each type per session, on the off chance we started to track running coroutines or caching values. I didn't implement any of that, so there's no reason to revert this right now.

I also thought the way you fixed this in the test suite would have been sufficient, but better safe than sorry haha

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is redundant for fixing the issue, but I figured it would help avoid similar mistakes in the future. If we want to configure shims for a session, as you say, we can revert this line later.

logger.debug('No shims specified, loading all available communication '
f'shims: {list(self.shims.keys())}')
else:
Expand Down
20 changes: 16 additions & 4 deletions superscore/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@
from superscore.control_layers.core import ControlLayer
from superscore.model import (Collection, Parameter, Readback, Root, Setpoint,
Snapshot)
from superscore.tests.ioc import IOCFactory


@pytest.fixture(scope='function')
def linac_backend():
def linac_data():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason not to make this a fixture as well? I suppose if I ever find myself wanting to use it as a fixture we could add the decorator.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marking it as a fixture makes it difficult to use outside of a pytest context, which I ran into when trying to run the test IOC. You can still import and call it in other fixtures, it just won't happen automatically. If we do want fixture controls, I suppose we could make a fixture that simply calls and returns linac_data to get the benefits of both options.

lasr_gunb_pv1 = Parameter(
uuid="5544c58f-88b6-40aa-9076-f180a44908f5",
pv_name="LASR:GUNB:TEST1",
Expand Down Expand Up @@ -615,6 +615,12 @@ def linac_backend():
]
)

return all_col, all_snapshot


@pytest.fixture(scope='function')
def linac_backend():
all_col, all_snapshot = linac_data()
return TestBackend([all_col, all_snapshot])


Expand Down Expand Up @@ -736,8 +742,7 @@ def monitor(self, *args, **kwargs):
@pytest.fixture(scope='function')
def dummy_cl() -> ControlLayer:
cl = ControlLayer()
cl.shims['ca'] = DummyShim()
cl.shims['pva'] = DummyShim()
cl.shims = {protocol: DummyShim() for protocol in ['ca', 'pva']}
return cl


Expand Down Expand Up @@ -772,3 +777,10 @@ def sample_client(
client.cl = dummy_cl

return client


@pytest.fixture(scope='module')
def linac_ioc():
_, snapshot = linac_data()
with IOCFactory.from_entries(snapshot.children)(prefix="SCORETEST:") as ioc:
yield ioc
1 change: 1 addition & 0 deletions superscore/tests/ioc/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
from .ioc_factory import IOCFactory # noqa: F401
71 changes: 71 additions & 0 deletions superscore/tests/ioc/ioc_factory.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
from multiprocessing import Process
from typing import Iterable, Mapping, Union

from caproto.server import PVGroup, pvproperty
from caproto.server import run as run_ioc
from epicscorelibs.ca import dbr

from superscore.model import Entry, Nestable, Parameter, Readback, Setpoint


class TempIOC(PVGroup):
"""
Makes PVs accessible via EPICS when running. Instances automatically start
and stop running when used as a context manager, and are thus suitable for
use in tests.
"""
def __enter__(self):
self.running_process = Process(
target=run_ioc,
args=(self.pvdb,),
daemon=True,
)
self.running_process.start()
return self

def __exit__(self, exc_type, exc_value, traceback):
pass


class IOCFactory:
"""
Generates TempIOC subclasses bound to a set of PVs.
"""
@staticmethod
def from_entries(entries: Iterable[Entry], **ioc_options) -> PVGroup:
"""
Defines and instantiates a TempIOC subclass containing all PVs reachable
from entries.
"""
attrs = IOCFactory.prepare_attrs(entries)
IOC = type("IOC", (TempIOC,), attrs)
return IOC

@staticmethod
def collect_pvs(entries: Iterable[Entry]) -> Iterable[Union[Parameter, Setpoint, Readback]]:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've written this function a couple of times at this point (or at least similar looking ones), we should find a good way to DRY this out (later, not this PR)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. As a note for the future PR, the reason I didn't do it here is because it requires untangling backend calls in the client version, which are required for resolving UUIDs.

"""Returns a collection of all PVs reachable from entries"""
pvs = []
q = entries.copy()
while len(q) > 0:
entry = q.pop()
if isinstance(entry, Nestable):
q.extend(entry.children)
else:
pvs.append(entry)
return pvs

@staticmethod
def prepare_attrs(entries: Iterable[Entry]) -> Mapping[str, pvproperty]:
"""
Turns a collecton of PVs into a Mapping from attribute names to
caproto.pvproperties. The mapping is suitable for passing into a type()
call as the dict arg.
"""
pvs = IOCFactory.collect_pvs(entries)
attrs = {}
for entry in pvs:
value = entry.data if isinstance(entry, (Setpoint, Readback)) else None
pv = pvproperty(name=entry.pv_name, doc=entry.description, value=value, dtype=dbr.DBR_STRING if isinstance(entry.data, str) else None)
attr = "".join([c.lower() for c in entry.pv_name if c != ':'])
attrs[attr] = pv
return attrs
15 changes: 15 additions & 0 deletions superscore/tests/ioc/linac.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
from caproto.server import ioc_arg_parser, run

from superscore.tests.conftest import linac_data
from superscore.tests.ioc import IOCFactory

if __name__ == '__main__':
_, snapshot = linac_data()
LinacIOC = IOCFactory.from_entries(snapshot.children)

ioc_options, run_options = ioc_arg_parser(
default_prefix='SCORETEST:',
desc="IOC evoking the structure of a linac",
)
ioc = LinacIOC(**ioc_options)
run(ioc.pvdb, **run_options)
20 changes: 20 additions & 0 deletions superscore/tests/test_ioc.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
from superscore.control_layers.core import ControlLayer


def test_ioc(linac_ioc):
cl = ControlLayer()
assert cl.get("SCORETEST:MGNT:GUNB:TEST0").data == 1
cl.put("SCORETEST:MGNT:GUNB:TEST0", 0)
assert cl.get("SCORETEST:MGNT:GUNB:TEST0").data == 0

assert cl.get("SCORETEST:VAC:GUNB:TEST1").data == "Ion Pump"
cl.put("SCORETEST:VAC:GUNB:TEST1", "new value")
assert cl.get("SCORETEST:VAC:GUNB:TEST1").data == "new value"

assert cl.get("SCORETEST:LASR:GUNB:TEST2").data == 5
cl.put("SCORETEST:LASR:GUNB:TEST2", 10)
assert cl.get("SCORETEST:LASR:GUNB:TEST2").data == 10

assert cl.get("SCORETEST:LASR:IN10:TEST0").data == 645.26
cl.put("SCORETEST:LASR:IN10:TEST0", 600.0)
assert cl.get("SCORETEST:LASR:IN10:TEST0").data == 600.0