improve error messages in get_class

- make clear which part of the spec is invalid
- raise the same error message when a module is loaded already or not
- check that the return type is a class
- catch properly the edge case with no dot at all
+ add a test

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

frappy/lib/__init__.py

@@ -248,27 +248,33 @@ def get_class(spec):
        a class (or any object) and optionally names of attributes
     :return: the object
     """
-    error = None
-    for maxsplit in range(1, len(spec)):
-        # len(spec) is high enough for all cases
+    for maxsplit in range(1, 999):
         module, *attrs = spec.rsplit('.', maxsplit)
+        if not attrs:
+            raise ValueError(f'{spec!r} needs a dot')
         try:
             obj = importlib.import_module(module)
             break
-        except ImportError as e:
-            if error is None:
-                error = module, e
+        except ModuleNotFoundError:
             if '.' in module:
                 continue
             raise
-    for na, attr in enumerate(attrs):
-        try:
+    try:
+        for na, attr in enumerate(attrs):
             obj = getattr(obj, attr)
-        except AttributeError:
-            if error is not None:
-                raise ImportError(f'{error[1]} during import {error[0]}') from None
-            raise AttributeError(f'{".".join(attrs[:na+1])!r} not found in {module!r}') from None
-    return obj
+        if isinstance(obj, type):
+            return obj
+    except AttributeError:
+        pass
+    base = '.'.join([module] + attrs[:na])
+    if na + 1 == len(attrs):
+        # when attr is a module, there are 2 cases:
+        # - the module is not yet loaded (AttributeError caught above)
+        # - the type check failed
+        # we should raise the same error in both cases
+        raise AttributeError(f'No class {attr!r} found in {base!r}')
+    # we do not know if attr was meant to be a module or an object
+    raise AttributeError(f'{attr!r} not found in {base!r}')
 
 
 def mkthread(func, *args, **kwds):

test/test_lib.py

@@ -21,7 +21,7 @@
 
 import pytest
 
-from frappy.lib import parse_host_port, merge_status
+from frappy.lib import parse_host_port, merge_status, get_class
 
 
 @pytest.mark.parametrize('hostport, defaultport, result', [
@@ -61,3 +61,43 @@ def test_parse_host(hostport, defaultport, result):
 ])
 def test_merge_status(args, result):
     assert merge_status(*args) == result
+
+
+def test_get_class():
+    assert isinstance(get_class('frappy.modules.Readable'), type)
+    # frappy_demo.lakeshore is not yet loaded here
+    with pytest.raises(AttributeError) as e:
+        get_class('frappy_demo.lakeshore')
+        errormsg = str(e)
+    assert isinstance(get_class('frappy_demo.lakeshore.TemperatureSensor'), type)
+    # frappy_demo.lakeshore is now loaded, check that we get the same error message
+    with pytest.raises(AttributeError) as e:
+        get_class('frappy_demo.lakeshore')
+        assert str(e) == errormsg
+
+
+@pytest.mark.parametrize('spec, exc', [
+    ('frappy.bad',
+     AttributeError("No class 'bad' found in 'frappy'")),
+    ('frappy.bad.Bad',
+     AttributeError("'bad' not found in 'frappy'")),
+    ('frappy.modules.Bad',
+     AttributeError("No class 'Bad' found in 'frappy.modules'")),
+    ('bad.modules',
+     ModuleNotFoundError("No module named 'bad'")),
+    ('frappy.modules.Readable.bad',
+     AttributeError("No class 'bad' found in 'frappy.modules.Readable'" )),
+    ('cfg.demo_cfg.Mod',
+     NameError("name 'Node' is not defined")),
+    ('bad.Class',
+     ModuleNotFoundError("No module named 'bad'")),
+    ('bad',
+     ValueError("'bad' needs a dot")),
+    ('frappy',
+     ValueError("'frappy' needs a dot")),
+])
+def test_get_class_failed(spec, exc):
+    with pytest.raises(Exception) as e:
+        get_class(spec)
+    assert e.type == type(exc)
+    assert repr(e.value) == repr(exc)