From 3e032d3850cc78fc48f061d3b1133664830a17f7 Mon Sep 17 00:00:00 2001 From: Anton Joubert <ajoubert@ska.ac.za> Date: Tue, 7 Jul 2020 12:52:02 +0200 Subject: [PATCH] Fix AttributeError on _set_obs_state When running multiple devices via MultiDeviceTestContext, it could result in failures during initialisation like ``` AttributeError: 'SKABaseDeviceStateModel' object has no attribute '_set_obs_state' ``` Issue was caused by updates to the transitions dict class variable instead of maintaining a unique instance per object. --- .release | 4 +- README.md | 5 ++ src/ska/base/control_model.py | 3 +- src/ska/base/release.py | 2 +- tests/test_control_model.py | 89 +++++++++++++++++++++++++++++++++++ tests/test_obs_device.py | 19 ++++++++ 6 files changed, 118 insertions(+), 4 deletions(-) create mode 100644 tests/test_control_model.py diff --git a/.release b/.release index 5632f945..eb100b2b 100644 --- a/.release +++ b/.release @@ -1,2 +1,2 @@ -release=0.6.1 -tag=lmcbaseclasses-0.6.1 +release=0.6.2 +tag=lmcbaseclasses-0.6.2 diff --git a/README.md b/README.md index fb3c242b..09af3fab 100644 --- a/README.md +++ b/README.md @@ -25,6 +25,11 @@ The lmc-base-classe repository contains set of eight classes as mentioned in SKA ## Version History +#### 0.6.2 +- Fix issue with incorrect updates to transitions dict from inherited devices. + Only noticeable if running multiple devices of different types in the same + the process. + #### 0.6.1 - Add ON state to SKABaseDeviceStateModel. - Move On() and Off() commands to SKABaseDevice. diff --git a/src/ska/base/control_model.py b/src/ska/base/control_model.py index 52328e8a..6f500d55 100644 --- a/src/ska/base/control_model.py +++ b/src/ska/base/control_model.py @@ -355,7 +355,8 @@ class DeviceStateModel: :type initial_state: a state with an entry in the transitions table """ - self._transitions = transitions + # instances may update transitions dict, so need a copy + self._transitions = transitions.copy() self._state = initial_state @property diff --git a/src/ska/base/release.py b/src/ska/base/release.py index 77b97f6e..c5877ac7 100644 --- a/src/ska/base/release.py +++ b/src/ska/base/release.py @@ -7,7 +7,7 @@ """Release information for lmc-base-classes Python Package""" name = """lmcbaseclasses""" -version = "0.6.1" +version = "0.6.2" version_info = version.split(".") description = """A set of generic base devices for SKA Telescope.""" author = "SKA India and SARAO and CSIRO" diff --git a/tests/test_control_model.py b/tests/test_control_model.py new file mode 100644 index 00000000..130077fd --- /dev/null +++ b/tests/test_control_model.py @@ -0,0 +1,89 @@ +"""Tests for ska.base.control_model.""" +import pytest + +from collections import Counter + +from ska.base.control_model import DeviceStateModel +from ska.base.faults import StateModelError + + +class OnOffModel(DeviceStateModel): + __transitions = { + ("ON", "off"): ( + "OFF", + lambda self: self.count_transition("on->off"), + ), + ("OFF", "on"): ( + "ON", + lambda self: self.count_transition("off->on"), + ), + } + + def __init__(self): + super().__init__(self.__transitions, "ON") + self.counter = Counter() + + def count_transition(self, name): + self.counter[name] += 1 + + +def test_initial_state(): + on_off = OnOffModel() + assert on_off.state == "ON" + + +def test_try_valid_action_returns_true_and_does_not_change_state(): + on_off = OnOffModel() + assert on_off.try_action("off") + assert on_off.state == "ON" + + +def test_try_invalid_action_raises_and_does_not_change_state(): + on_off = OnOffModel() + with pytest.raises(StateModelError): + on_off.try_action("on") + assert on_off.state == "ON" + + +def test_valid_state_transitions_succeed(): + on_off = OnOffModel() + + on_off.perform_action("off") + assert on_off.state == "OFF" + on_off.perform_action("on") + assert on_off.state == "ON" + + +def test_invalid_state_transitions_fail(): + on_off = OnOffModel() + + with pytest.raises(StateModelError): + on_off.perform_action("on") + on_off.perform_action("off") + with pytest.raises(StateModelError): + on_off.perform_action("off") + with pytest.raises(StateModelError): + on_off.perform_action("invalid") + + +def test_side_effect_is_called(): + on_off = OnOffModel() + + on_off.perform_action("off") + on_off.perform_action("on") + on_off.perform_action("off") + + assert on_off.counter["on->off"] == 2 + assert on_off.counter["off->on"] == 1 + + +def test_update_transitions_only_applies_to_instances(): + on_off = OnOffModel() + + on_off_updated = OnOffModel() + on_off_updated.update_transitions({("ON", "break"): ("BROKEN", None)}) + + assert on_off.is_action_allowed("off") + assert not on_off.is_action_allowed("break") + assert on_off_updated.is_action_allowed("off") + assert on_off_updated.is_action_allowed("break") diff --git a/tests/test_obs_device.py b/tests/test_obs_device.py index 0c1c2011..0043ed07 100644 --- a/tests/test_obs_device.py +++ b/tests/test_obs_device.py @@ -11,9 +11,12 @@ # Imports import re import pytest + from tango import DevState +from tango.test_context import MultiDeviceTestContext # PROTECTED REGION ID(SKAObsDevice.test_additional_imports) ENABLED START # +from ska.base import SKABaseDevice, SKAObsDevice from ska.base.control_model import ( AdminMode, ControlMode, HealthState, ObsMode, ObsState, SimulationMode, TestMode ) @@ -171,3 +174,19 @@ class TestSKAObsDevice(object): # PROTECTED REGION ID(SKAObsDevice.test_testMode) ENABLED START # assert tango_context.device.testMode == TestMode.NONE # PROTECTED REGION END # // SKAObsDevice.test_testMode + + +def test_multiple_devices_in_same_process(): + + # The order here is important - base class last, so that we can + # test that the subclass isn't breaking anything. + devices_info = ( + {"class": SKAObsDevice, "devices": [{"name": "test/obs/1"}]}, + {"class": SKABaseDevice, "devices": [{"name": "test/base/1"}]}, + ) + + with MultiDeviceTestContext(devices_info, process=False) as context: + proxy1 = context.get_device("test/obs/1") + proxy2 = context.get_device("test/base/1") + assert proxy1.State() == DevState.OFF + assert proxy2.State() == DevState.OFF -- GitLab