Override does not change the order of inherited accessibles.

except when explicitely mentioned with reorder=True
improved test_modules for unique accessibles, as this
was related with Accessible.ctr

Change-Id: I61877de9300bb0297c88a6c44bb265c634937856
Reviewed-on: https://forge.frm2.tum.de/review/19693
Tested-by: JenkinsCodeReview <bjoern_pedersen@frm2.tum.de>
Reviewed-by: Enrico Faulhaber <enrico.faulhaber@frm2.tum.de>

secop/modules.py

@@ -214,10 +214,10 @@ class Module(object):
 
     def early_init(self):
         # may be overriden in derived classes to init stuff
-        self.log.debug('empty early_init()')
+        self.log.debug('empty %s.early_init()' % self.__class__.__name__)
 
     def init_module(self):
-        self.log.debug('empty init_module()')
+        self.log.debug('empty %s.init_module()' % self.__class__.__name__)
 
     def start_module(self, started_callback):
         '''runs after init of all modules
@@ -226,7 +226,7 @@ class Module(object):
         or, if not implmemented, immediately
         '''
 
-        self.log.debug('empty start_module()')
+        self.log.debug('empty %s.start_module()' % self.__class__.__name__)
         started_callback(self)
 
 

secop/params.py

@@ -48,7 +48,6 @@ class Accessible(CountedObj):
     def copy(self):
         # return a copy of ourselfs
         props = self.__dict__.copy()
-        props.pop('ctr')
         return type(self)(**props)
 
     def exported_properties(self):
@@ -155,10 +154,12 @@ class Override(CountedObj):
     """Stores the overrides to be applied to a Parameter
 
     note: overrides are applied by the metaclass during class creating
+    reorder= True: use position of Override instead of inherited for the order
     """
-    def __init__(self, description="", **kwds):
+    def __init__(self, description="", reorder=False, **kwds):
         super(Override, self).__init__()
         self.kwds = kwds
+        self.reorder = reorder
         # allow to override description without keyword
         if description:
             self.kwds['description'] = description
@@ -177,7 +178,8 @@ class Override(CountedObj):
                     raise ProgrammingError( "%s is not a valid %s property" %
                                            (key, type(obj).__name__))
             props.update(self.kwds)
-            props['ctr'] = self.ctr
+            if self.reorder:
+                props['ctr'] = self.ctr
             return type(obj)(**props)
         else:
             raise ProgrammingError(

test/test_modules.py

@@ -25,9 +25,9 @@ from __future__ import division, print_function
 # no fixtures needed
 import pytest
 
-from secop.datatypes import BoolType, EnumType
+from secop.datatypes import BoolType, EnumType, FloatRange
 from secop.metaclass import ModuleMeta
-from secop.modules import Communicator, Drivable, Module, Readable, Writable
+from secop.modules import Communicator, Drivable, Module
 from secop.params import Command, Override, Parameter
 
 try:
@@ -60,8 +60,9 @@ def test_Communicator():
     q.get()
 
 def test_ModuleMeta():
-    newclass = ModuleMeta.__new__(ModuleMeta, 'TestReadable', (Drivable, Writable, Readable, Module), {
+    newclass1 = ModuleMeta.__new__(ModuleMeta, 'TestDrivable', (Drivable,), {
         "parameters" : {
+            'pollinterval': Override(reorder=True),
             'param1' : Parameter('param1', datatype=BoolType(), default=False),
             'param2': Parameter('param2', datatype=BoolType(), default=True),
         },
@@ -83,12 +84,19 @@ def test_ModuleMeta():
         "read_value": lambda self, *args: True,
         "init": lambda self, *args: [None for self.accessibles['value'].datatype in [EnumType('value', OK=1, Bad=2)]],
     })
-    # every cmd/param has to be collected to accessibles
+    # first inherited accessibles, then Overrides with reorder=True and new accessibles
-    assert newclass.accessibles
+    sortcheck1 = ['value', 'status', 'target', 'pollinterval',
-    with pytest.raises(AttributeError):
+                 'param1', 'param2', 'cmd', 'a1', 'a2', 'cmd2']
-        assert newclass.commands
+
-    with pytest.raises(AttributeError):
+    newclass2 = ModuleMeta.__new__(ModuleMeta, 'UpperClass', (newclass1,), {
-        assert newclass.parameters
+        "accessibles": {
+            'cmd2': Override('another stuff'),
+            'a1': Override(datatype=FloatRange(), reorder=True),
+            'b2': Parameter('a2', datatype=BoolType(), default=True),
+        },
+    })
+    sortcheck2 = ['value', 'status', 'target', 'pollinterval',
+                 'param1', 'param2', 'cmd', 'a2', 'cmd2', 'a1', 'b2']
 
     logger = type('LoggerStub', (object,), dict(
         debug = lambda self, *a: print(*a),
@@ -103,21 +111,45 @@ def test_ModuleMeta():
         dispatcher = dispatcher,
     ))()
 
-    o1 = newclass('o1', logger, {}, srv)
+    params_found = set() # set of instance accessibles
-    o2 = newclass('o2', logger, {}, srv)
+    objects = []
-    params_found= set()
+
-    ctr_found = set()
+    for newclass, sortcheck in [(newclass1, sortcheck1), (newclass2, sortcheck2)]:
-    for obj in [o1, o2]:
+        o1 = newclass('o1', logger, {}, srv)
-        for n, o in obj.accessibles.items():
+        o2 = newclass('o2', logger, {}, srv)
-            print(n)
+        for obj in [o1, o2]:
+            objects.append(obj)
+            ctr_found = set()
+            for n, o in obj.accessibles.items():
+                # check that instance accessibles are unique objects
+                assert o not in params_found
+                params_found.add(o)
+                assert o.ctr not in ctr_found
+                ctr_found.add(o.ctr)
+            check_order = [(obj.accessibles[n].ctr, n) for n in sortcheck]
+            assert check_order == sorted(check_order)
+
+    # check on the level of classes
+    # this checks newclass1 too, as it is inherited by newclass2
+    for baseclass in newclass2.__mro__:
+        # every cmd/param has to be collected to accessibles
+        acs = getattr(baseclass, 'accessibles', None)
+        if issubclass(baseclass, Module):
+            assert acs is not None
+        else: # do not check object or mixin
+            acs = {}
+        with pytest.raises(AttributeError):
+            assert baseclass.commands
+        with pytest.raises(AttributeError):
+            assert baseclass.parameters
+        for n, o in acs.items():
+            # check that class accessibles are not reused as instance accessibles
             assert o not in params_found
-            params_found.add(o)
+
-            assert o.ctr not in ctr_found
+    for o in objects:
-            ctr_found.add(o.ctr)
+        o.early_init()
-    o1.early_init()
+    for o in objects:
-    o2.early_init()
+        o.init_module()
-    o1.init_module()
-    o2.init_module()
     q = queue.Queue()
-    o1.start_module(q.put)
+    for o in objects:
-    o2.start_module(q.put)
+        o.start_module(q.put)

test/test_params.py

@@ -49,10 +49,19 @@ def test_Parameter():
 
 def test_Override():
     p = Parameter('description1', datatype=BoolType, default=False)
-    o = Override(default=True)
+    o = Override(default=True, reorder=True)
     assert o.ctr != p.ctr
     q = o.apply(p)
     assert q.ctr == o.ctr  # override shall be useable to influence the order, hence copy the ctr value
     assert q.ctr != p.ctr
     assert o.ctr != p.ctr
     assert q != p
+
+    p2 = Parameter('description2', datatype=BoolType, default=False)
+    o2 = Override(default=True)
+    assert o2.ctr != p2.ctr
+    q2 = o2.apply(p2)
+    assert q2.ctr != o2.ctr
+    assert q2.ctr == p2.ctr
+    assert o2.ctr != p2.ctr
+    assert q2 != p2