core: simplify test for methods names

The test for method names 'read_<param>' and 'write_<param>'
without a defined parameter is simplified. We do not check
anymore method names from base classes. Base classes
inheriting from HasAccessible are checked anyway at the
place they are defined.

+ add a test for it
+ move some tests to a new file test_all_modules.py, as
  test_modules.py is getting too long
+ fix missing doc string (frappy.simulation.SimDrivable.stop)

Change-Id: Id8a9afe5c977ae3b1371bd40c6da52be2fc79eb9
Reviewed-on: https://forge.frm2.tum.de/review/c/secop/frappy/+/35503
Tested-by: Jenkins Automated Tests <pedersen+jenkins@frm2.tum.de>
Reviewed-by: Markus Zolliker <markus.zolliker@psi.ch>

frappy/modulebase.py

@@ -60,14 +60,12 @@ class HasAccessibles(HasProperties):
     (so the dispatcher will get notified of changed values)
     """
     isWrapped = False
-    checkedMethods = ()
 
     @classmethod
     def __init_subclass__(cls):  # pylint: disable=too-many-branches
         super().__init_subclass__()
         if cls.isWrapped:
             return
-        cls.checkedMethods = set(cls.checkedMethods)  # might be initial empty tuple
         # merge accessibles from all sub-classes, treat overrides
         # for now, allow to use also the old syntax (parameters/commands dict)
         accessibles = OrderedDict()  # dict of accessibles
@@ -200,16 +198,15 @@ class HasAccessibles(HasProperties):
                 new_wfunc.__module__ = cls.__module__
                 cls.wrappedAttributes[wname] = new_wfunc
 
-        cls.checkedMethods.update(cls.wrappedAttributes)
-
         # check for programming errors
-        for attrname in cls.__dict__:
+        for attrname, func in cls.__dict__.items():
             prefix, _, pname = attrname.partition('_')
             if not pname:
                 continue
             if prefix == 'do':
                 raise ProgrammingError(f'{cls.__name__!r}: old style command {attrname!r} not supported anymore')
-            if prefix in ('read', 'write') and attrname not in cls.checkedMethods:
+            if (prefix in ('read', 'write') and attrname not in cls.wrappedAttributes
+                    and not hasattr(func, 'poll')):  # may be a handler, which always has a poll attribute
                 raise ProgrammingError(f'{cls.__name__}.{attrname} defined, but {pname!r} is no parameter')
 
         try:

frappy/rwhandler.py

@@ -102,9 +102,6 @@ class Handler:
         """create the wrapped read_* or write_* methods"""
         # at this point, this 'method_names' entry is no longer used -> delete
         self.method_names.discard((self.func.__module__, self.func.__qualname__))
-        # avoid complain about read_<name> or write_<name> method without parameter <name>
-        # can not use .add() here, as owner.checkedMethods might be an empty tuple
-        owner.checkedMethods = set(owner.checkedMethods) | {name}
         for key in self.keys:
             wrapped = self.wrap(key)
             method_name = self.prefix + key

frappy/simulation.py

@@ -142,4 +142,5 @@ class SimDrivable(SimReadable, Drivable):
 
     @Command
     def stop(self):
+        """set target to value"""
         self.target = self.value

test/test_all_modules.py

@@ -0,0 +1,76 @@
+# *****************************************************************************
+#
+# This program is free software; you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the Free Software
+# Foundation; either version 2 of the License, or (at your option) any later
+# version.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
+# details.
+#
+# You should have received a copy of the GNU General Public License along with
+# this program; if not, write to the Free Software Foundation, Inc.,
+# 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+#
+# Module authors:
+#   Markus Zolliker <markus.zolliker@psi.ch>
+#
+# *****************************************************************************
+"""tests for probable implementation errors."""
+
+import sys
+import importlib
+from glob import glob
+import pytest
+from frappy.core import Module, Drivable
+from frappy.errors import ProgrammingError
+
+
+all_drivables = set()
+for pyfile in glob('frappy_*/*.py') + glob('frappy/*.py'):
+    module = pyfile[:-3].replace('/', '.')
+    try:
+        importlib.import_module(module)
+    except Exception as e:
+        print(module, e)
+        continue
+    for obj_ in sys.modules[module].__dict__.values():
+        if isinstance(obj_, type) and issubclass(obj_, Drivable):
+            all_drivables.add(obj_)
+
+
+@pytest.mark.parametrize('modcls', all_drivables)
+def test_stop_doc(modcls):
+    # make sure that implemented stop methods have a doc string
+    if (modcls.stop.description == Drivable.stop.description
+            and modcls.stop.func != Drivable.stop.func):
+        assert modcls.stop.func.__doc__  # stop method needs a doc string
+
+
+def test_bad_method_test():
+    with pytest.raises(ProgrammingError):
+        class Mod1(Module):  # pylint: disable=unused-variable
+            def read_param(self):
+                pass
+
+    with pytest.raises(ProgrammingError):
+        class Mod2(Module):  # pylint: disable=unused-variable
+            def write_param(self):
+                pass
+
+    with pytest.raises(ProgrammingError):
+        class Mod3(Module):  # pylint: disable=unused-variable
+            def do_cmd(self):
+                pass
+
+    # no complain in this case
+    # checking this would make code to check much more complicated.
+    # in the rare cases used it might even be intentional
+    class Mixin:
+        def read_param(self):
+            pass
+
+    class ModTest(Mixin, Module):  # pylint: disable=unused-variable
+        pass

test/test_modules.py

@@ -23,8 +23,6 @@
 
 import sys
 import threading
-import importlib
-from glob import glob
 import pytest
 
 from frappy.datatypes import BoolType, FloatRange, StringType, IntRange, ScaledInteger
@@ -922,27 +920,6 @@ def test_interface_classes(bases, iface_classes):
     assert m.interface_classes == iface_classes
 
 
-all_drivables = set()
-for pyfile in glob('frappy_*/*.py'):
-    module = pyfile[:-3].replace('/', '.')
-    try:
-        importlib.import_module(module)
-    except Exception as e:
-        print(module, e)
-        continue
-    for obj_ in sys.modules[module].__dict__.values():
-        if isinstance(obj_, type) and issubclass(obj_, Drivable):
-            all_drivables.add(obj_)
-
-
-@pytest.mark.parametrize('modcls', all_drivables)
-def test_stop_doc(modcls):
-    # make sure that implemented stop methods have a doc string
-    if (modcls.stop.description == Drivable.stop.description
-            and modcls.stop.func != Drivable.stop.func):
-        assert modcls.stop.func.__doc__  # stop method needs a doc string
-
-
 def test_write_error():
     updates = {}
     srv = ServerStub(updates)