Fix infotag loss bug #105
Fix infotag loss bug #105
Conversation
jacomago
force-pushed
the
fix-infotag-loss-bug
branch
3 times, most recently
from
e70c345 to
65cee1b
Compare
|
Current problem with this version is that owner is specified twice in cfstore, once from config, another from the ioc itself. Kind of makes sense, but means my check on the properties doesn't work in the merge_property_list. Need a cleverer check for the owner. |
|
Realised should have all of the property names that receiver deals with anyway. The required property list. Just need to pass that as an extra argument |
jacomago
force-pushed
the
fix-infotag-loss-bug
branch
from
65cee1b to
2c3d66d
Compare
jacomago
requested review from
anderslindho,
mdavidsaver,
ralphlange,
shroffk,
simon-ess and
tynanford
server/tests/client_checks.py
Outdated
| if prop["name"] in properties_to_match: | ||
| if prop not in channel1["properties"]: | ||
| assert False, f"Property {prop} not found in channel {channel1['name']}" |
There was a problem hiding this comment.
If you skip the log message you can simplify this to an all or any check
| log_thread.start() | ||
|
|
||
|
|
||
| class TestRemoveProperty: |
| EXPECTED_DEFAULT_CHANNELS = 32 | ||
|
|
||
|
|
||
| @pytest.fixture(scope="class") |
There was a problem hiding this comment.
You could also do module scope and skip the class, since it's only two tests in this file
| def create_channel(name: str, owner: str, properties: list[dict[str, str]]): | ||
| return { | ||
| "name": name, | ||
| "owner": owner, | ||
| "properties": properties, | ||
| } | ||
|
|
||
|
|
||
| def create_property(owner: str, name: str, value: str): | ||
| return { | ||
| "name": name, | ||
| "owner": owner, | ||
| "value": value, | ||
| } |
There was a problem hiding this comment.
Extremely naïve question, but this looks a lot like you just want a dataclass, doesn't it? Or is that a "python is too old issue" or something else?
There was a problem hiding this comment.
More I thought adding it would be another refactor.
| """ | ||
| newPropNames = [p["name"] for p in newProperties] | ||
| for oldProperty in oldProperties: | ||
| for oldProperty in channel["properties"]: |
There was a problem hiding this comment.
Is there any chance that properties does not exist as a key?
There was a problem hiding this comment.
Not here... I hope. I think raising exception is pretty suitable response if it happens. Which is the current behaviour.
jacomago
force-pushed
the
fix-infotag-loss-bug
branch
2 times, most recently
from
ac31111 to
8de97cc
Compare
jacomago
force-pushed
the
fix-infotag-loss-bug
branch
3 times, most recently
from
b1de169 to
4474792
Compare
Bug can be reproduced manually as follows 1. Create recceiver with infotag (X) set to be updated in channelfinder. 2. Add IOC with PV with infotag X set to A 3. Channelfinder has PV with X=A 4. Reboot recceiver with infotag X removed 5. Channelfinder has PV with X=A still even after IOC update Or by 1. Create recceiver with infotag (X) set to be updated in channelfinder. 2. Add IOC with PV with infotag X set to A 3. Channelfinder has PV with X=A 4. Reboot IOC with PV with infotag X removed 5. Channelfinder has PV with X=A still
jacomago
force-pushed
the
fix-infotag-loss-bug
branch
2 times, most recently
from
0167001 to
44539ec
Compare
To find source of problem
Essentially when merging property lists, should only take the old data when setting the channel to be inactive. Otherwise only the new data owned by the recceiver is relevant
jacomago
force-pushed
the
fix-infotag-loss-bug
branch
from
44539ec to
d17610d
Compare
|
3 participants
Adds a test for and fixes issue #92