From 4aec05c86b071d5efcf3642964509be3620cf69c Mon Sep 17 00:00:00 2001 From: Markus Zolliker Date: Wed, 29 Jan 2025 09:00:08 +0100 Subject: [PATCH] core: simplify test for methods names The test for method names 'read_' and 'write_' 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 Reviewed-by: Markus Zolliker --- frappy/modulebase.py | 9 ++--- frappy/rwhandler.py | 3 -- frappy/simulation.py | 1 + test/test_all_modules.py | 76 ++++++++++++++++++++++++++++++++++++++++ test/test_modules.py | 23 ------------ 5 files changed, 80 insertions(+), 32 deletions(-) create mode 100644 test/test_all_modules.py diff --git a/frappy/modulebase.py b/frappy/modulebase.py index 7ac6741..670015f 100644 --- a/frappy/modulebase.py +++ b/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: diff --git a/frappy/rwhandler.py b/frappy/rwhandler.py index 4192547..5837e2f 100644 --- a/frappy/rwhandler.py +++ b/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_ or write_ method without parameter - # 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 diff --git a/frappy/simulation.py b/frappy/simulation.py index 0b790b0..5693263 100644 --- a/frappy/simulation.py +++ b/frappy/simulation.py @@ -142,4 +142,5 @@ class SimDrivable(SimReadable, Drivable): @Command def stop(self): + """set target to value""" self.target = self.value diff --git a/test/test_all_modules.py b/test/test_all_modules.py new file mode 100644 index 0000000..027ece4 --- /dev/null +++ b/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 +# +# ***************************************************************************** +"""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 diff --git a/test/test_modules.py b/test/test_modules.py index b9069ba..86982b3 100644 --- a/test/test_modules.py +++ b/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)