From 84f760bbbe5c57393bad74e6d5bcb2f7691e735a Mon Sep 17 00:00:00 2001
From: Hannes Feldt <feldt@astron.nl>
Date: Wed, 12 Jul 2023 14:31:30 +0000
Subject: [PATCH] L2SS-1388: Lofar station client intermittent unit test
 test_lazy_write_scalar

---
 README.md                       |   3 +-
 VERSION                         |   2 +-
 lofar_station_client/devices.py | 180 ++++++++++++++++++--------------
 tests/test_devices.py           |  56 +++++-----
 4 files changed, 139 insertions(+), 102 deletions(-)

diff --git a/README.md b/README.md
index a57bd01..7b17590 100644
--- a/README.md
+++ b/README.md
@@ -105,12 +105,13 @@ tox -e debug tests.requests.test_prometheus
 ```
 
 ## Releasenotes
+- 0.14.7 - Refactor LazyDeviceProxy
 - 0.14.6 - Removed deprecated StationSSTCollector
 - 0.14.5 - Added `gn_indices` and support for global node indices > 16.
 - 0.14.4 - Fixed bug on `writer_version` retrieval
 - 0.14.3 - Added `rcu_pcb_id` and `rcu_pcb_version` to Hdf5 file header
 - 0.14.2 - Added `station_name` attribute to Hdf5 file header
-- 0.14.1 - Added `beamlet.subband_select_RW` attribute to BSTHdf5Writer 
+- 0.14.1 - Added `beamlet.subband_select_RW` attribute to BSTHdf5Writer
 - 0.14  - Added new attributes to statistics HDF file as well as documentation
 - 0.13  - Added lazy connection behavior to `devices.LofarDeviceProxy` class
 - 0.12.
diff --git a/VERSION b/VERSION
index 226468e..c24a395 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-0.14.6
+0.14.7
diff --git a/lofar_station_client/devices.py b/lofar_station_client/devices.py
index b494688..dac55bf 100644
--- a/lofar_station_client/devices.py
+++ b/lofar_station_client/devices.py
@@ -1,116 +1,144 @@
 """ Enhanced interfaces towards the station's Tango devices. """
 
+#  Copyright (C) 2023 ASTRON (Netherlands Institute for Radio Astronomy)
+#  SPDX-License-Identifier: Apache-2.0
+
 # inconsistent-return-statements
 # pylint: disable=R1710
 
 import ast
-from functools import lru_cache
 import logging
+from functools import lru_cache
 
 import numpy
-
-from tango import DeviceProxy, DevFailed
+from tango import DevFailed, DeviceProxy
 from tango import ExtractAs
 
 logger = logging.getLogger()
 
 
-class LofarDeviceProxy(DeviceProxy):
-    """A LOFAR-specific tango.DeviceProxy that provides
-    a richer experience."""
+LAZY_DEVICE_PROXY_SLOTS = ["_create_instance", "connected", "_instance"]
+LAZY_DEVICE_PROXY_MEMBERS = LAZY_DEVICE_PROXY_SLOTS + ["_connect"]
 
-    def __init__(self, *args):
-        """Do not connect on device construction"""
-        self.__dict__["dev_name"] = str(args[0])
-        self.dev_name = str(args[0])
-        self.__dict__["connected"] = False
-        self.connected = False
-        logger.info("LOFARDeviceProxy %s not connected", self.dev_name)
 
-    def __str__(self):
-        if not self.connected:
-            return f"<LOFARDeviceProxy({self.dev_name})"
-        return super().__str__()
+class LazyDeviceProxy:
+    """A wrapper class that allows the regular DeviceProxy to only open the connection
+    when needed and not during creation."""
 
-    __repr__ = __str__
-    repr = __str__
+    __slots__ = LAZY_DEVICE_PROXY_SLOTS
+
+    def __init__(self, cls, *args, **kwargs):
+        self._create_instance = lambda: cls(*args, **kwargs)
+        self.connected = False
+        self._instance = None
 
-    def connect(self):
+    def _connect(self):
         """Try to estabilish a connection when a device operation is called"""
         if not self.connected:
             try:
-                super().__init__(self.dev_name)
-                self.__dict__["connected"] = True
+                self._instance = self._create_instance()
                 self.connected = True
-            except DevFailed as excep:
-                self.__dict__["connected"] = False
+            except DevFailed as ex:
                 self.connected = False
-                reason = excep.args[0].desc.replace("\n", " ")
+                reason = ex.args[0].desc.replace("\n", " ")
                 logger.warning("LOFARDeviceProxy not connected: %s", reason)
 
+    def __getattribute__(self, name):
+        if name in LAZY_DEVICE_PROXY_MEMBERS:
+            return object.__getattribute__(self, name)
+        self._connect()
+        return getattr(object.__getattribute__(self, "_instance"), name)
+
+    def __delattr__(self, name):
+        self._connect()
+        delattr(object.__getattribute__(self, "_instance"), name)
+
+    def __setattr__(self, name, value):
+        if name in LAZY_DEVICE_PROXY_MEMBERS:
+            object.__setattr__(self, name, value)
+        else:
+            self._connect()
+            setattr(object.__getattribute__(self, "_instance"), name, value)
+
+    def __nonzero__(self):
+        self._connect()
+        return bool(object.__getattribute__(self, "_instance"))
+
+    def __str__(self):
+        self._connect()
+        return str(object.__getattribute__(self, "_instance"))
+
+    def __repr__(self):
+        self._connect()
+        return repr(object.__getattribute__(self, "_instance"))
+
+
+def lazy(cls):
+    """A decorator that allows the regular DeviceProxy to only open the connection
+    when needed and not during creation."""
+    return lambda *args, **kwargs: LazyDeviceProxy(cls, *args, **kwargs)
+
+
+@lazy
+class LofarDeviceProxy(DeviceProxy):
+    """A LOFAR-specific tango.DeviceProxy that provides
+    a richer experience."""
+
     @lru_cache()
     def get_attribute_config(self, name):
         """Get cached attribute configurations, as they are not expected to change."""
-        self.connect()
-        if self.connected:
-            return super().get_attribute_config(name)
+        return super().get_attribute_config(name)
 
     @lru_cache()
     def get_attribute_shape(self, name):
         """Get the shape of the requested attribute, as a tuple."""
-        self.connect()
-        if self.connected:
-            config = self.get_attribute_config(name)
-
-            if config.format and config.format[0] == "(":
-                # For >2D arrays, the "format" property describes actual
-                # the dimensions as a tuple (x, y, z, ...),
-                # so reshape the value accordingly.
-                shape = ast.literal_eval(config.format)
-            elif config.max_dim_y > 0:
-                # 2D array
-                shape = (config.max_dim_y, config.max_dim_x)
-            elif config.max_dim_x > 1:
-                # 1D array
-                shape = (config.max_dim_x,)
-            else:
-                # scalar
-                shape = ()
-
-            return shape
+        config = self.get_attribute_config(name)
+
+        if config.format and config.format[0] == "(":
+            # For >2D arrays, the "format" property describes actual
+            # the dimensions as a tuple (x, y, z, ...),
+            # so reshape the value accordingly.
+            shape = ast.literal_eval(config.format)
+        elif config.max_dim_y > 0:
+            # 2D array
+            shape = (config.max_dim_y, config.max_dim_x)
+        elif config.max_dim_x > 1:
+            # 1D array
+            shape = (config.max_dim_x,)
+        else:
+            # scalar
+            shape = ()
+
+        return shape
 
     def read_attribute(self, name, extract_as=ExtractAs.Numpy):
         """Read an attribute from the server."""
-        self.connect()
-        if self.connected:
-            attr = super().read_attribute(name, extract_as)
+        attr = super().read_attribute(name, extract_as)
 
-            # convert non-scalar values into their actual shape
-            shape = self.get_attribute_shape(name)
-            if shape != ():
-                attr.value = attr.value.reshape(shape)
+        # convert non-scalar values into their actual shape
+        shape = self.get_attribute_shape(name)
+        if shape != ():
+            attr.value = attr.value.reshape(shape)
 
-            return attr
+        return attr
 
     def write_attribute(self, name, value):
         """Write an attribute to the server."""
-        self.connect()
-        if self.connected:
-            config = self.get_attribute_config(name)
-            shape = self.get_attribute_shape(name)
-
-            # 2D arrays also represent arrays of higher dimensionality. reshape them
-            # to fit their original Tango shape before writing.
-            if shape != ():
-                value = numpy.array(value)
-
-                if value.shape != shape:
-                    raise ValueError(
-                        f"Invalid shape. Given: {value.shape} Expected: {shape}"
-                    )
-
-                if len(shape) > 2:
-                    # >2D arrays collapse into 2D
-                    value = value.reshape((config.max_dim_y, config.max_dim_x))
-
-            return super().write_attribute(name, value)
+        config = self.get_attribute_config(name)
+        shape = self.get_attribute_shape(name)
+
+        # 2D arrays also represent arrays of higher dimensionality. reshape them
+        # to fit their original Tango shape before writing.
+        if shape != ():
+            value = numpy.array(value)
+
+            if value.shape != shape:
+                raise ValueError(
+                    f"Invalid shape. Given: {value.shape} Expected: {shape}"
+                )
+
+            if len(shape) > 2:
+                # >2D arrays collapse into 2D
+                value = value.reshape((config.max_dim_y, config.max_dim_x))
+
+        return super().write_attribute(name, value)
diff --git a/tests/test_devices.py b/tests/test_devices.py
index 780a85c..b653e74 100644
--- a/tests/test_devices.py
+++ b/tests/test_devices.py
@@ -1,11 +1,15 @@
 #  Copyright (C) 2023 ASTRON (Netherlands Institute for Radio Astronomy)
 #  SPDX-License-Identifier: Apache-2.0
+from unittest import mock
+from unittest.mock import MagicMock
 
 import numpy
 from tango import DevState, DevFailed
 from tango.server import Device, attribute, AttrWriteType
 from tango.test_context import MultiDeviceTestContext
 
+import lofar_station_client
+from lofar_station_client import devices
 from lofar_station_client.devices import LofarDeviceProxy
 from tests import base
 
@@ -76,7 +80,7 @@ class LofarDeviceProxyTest(base.TestCase):
 
         cls.context.start()
         cls.proxy = LofarDeviceProxy(cls.context.get_device_access("STAT/MyDevice/1"))
-        cls.proxy.connect()  # necessary in the DeviceTestContext
+        # cls.proxy.connect()  # necessary in the DeviceTestContext
 
     @classmethod
     def tearDownClass(cls):
@@ -139,6 +143,7 @@ class LofarDeviceProxyTest(base.TestCase):
         self.assertEqual((2, 3, 4), value.shape)
 
 
+@mock.patch("lofar_station_client.devices.LofarDeviceProxy")
 class LazyLofarDeviceProxyTest(base.TestCase):
     TEST_DEVICE_INFO = [
         {
@@ -156,44 +161,47 @@ class LazyLofarDeviceProxyTest(base.TestCase):
         )
 
         cls.context.start()
-        cls.proxy = LofarDeviceProxy(cls.context.get_device_access("STAT/MyDevice/2"))
-        cls.proxy.connected = False
+        cls.device_name = cls.context.get_device_access("STAT/MyDevice/2")
 
     @classmethod
     def tearDownClass(cls):
         # In Python3.8+, we can use addClassCleanup instead
         cls.context.stop()
 
-    @classmethod
-    def connect(cls):
-        # Simulate that a connection with the DB has been estabilished,
-        # i.e. Device has been created in the TangoDB
-        cls.proxy.connect()
+    def test_lazy_read_scalar(self, dev_proxy_mock):
+        mock = MagicMock()
+        mock.side_effect = DevFailed(type("", (object,), {"desc": "something"}))
+        dev_proxy_mock.side_effect = devices.lazy(mock)
 
-    @classmethod
-    def disconnect(cls):
-        # Simulate a disconnected device
-        cls.proxy.connected = False
+        proxy = lofar_station_client.devices.LofarDeviceProxy(self.device_name)
 
-    def test_lazy_read_scalar(self):
-        self.disconnect()
         # DeviceProxy not yet initialised
         with self.assertRaises(AttributeError):
-            value = self.proxy.scalar
+            _ = proxy.scalar
+
         # Simulate connection with DB
-        self.connect()
-        value = self.proxy.scalar
+        mock.side_effect = None
+        mock.return_value = type("", (object,), {"scalar": True})
+        value = proxy.scalar
         self.assertEqual(bool, type(value))
 
-    def test_lazy_write_scalar(self):
-        self.disconnect()
+    def test_lazy_write_scalar(self, dev_proxy_mock):
+        mock = MagicMock()
+        mock.side_effect = DevFailed(type("", (object,), {"desc": "something"}))
+        dev_proxy_mock.side_effect = devices.lazy(mock)
+
+        proxy = lofar_station_client.devices.LofarDeviceProxy(self.device_name)
+
         with self.assertRaises(AttributeError):
-            self.proxy.scalar = True
+            proxy.scalar = True
+
         # Simulate connection with DB
-        self.connect()
-        self.assertEqual(False, self.proxy.scalar)
-        self.proxy.scalar = True
-        self.assertEqual(True, self.proxy.scalar)
+        mock.side_effect = None
+        mock.return_value = type("", (object,), {"scalar": False})
+
+        self.assertEqual(False, proxy.scalar)
+        proxy.scalar = True
+        self.assertEqual(True, proxy.scalar)
 
 
 class RecvDeviceTest(MyDevice):
-- 
GitLab