Merge pull request #47 from labthings/improved-test-coverage

Improved test coverage

Author: jtc42

.coveragerc

@@ -1,7 +1,7 @@
 [run]
 branch = True
 source = ./labthings
-omit = .venv/*
+omit = .venv/*, labthings/server/wsgi/*, , labthings/server/monkey/*
 concurrency = greenlet
 
 [report]

.gitignore

@@ -52,6 +52,7 @@ coverage.xml
 .hypothesis/
 .pytest_cache/
 coverage_html_report/
+prof/
 
 # Translations
 *.mo

labthings/server/decorators.py

@@ -8,7 +8,7 @@
 
 from marshmallow import Schema as _Schema
 
-from .spec.utilities import update_spec
+from .spec.utilities import update_spec, tag_spec
 from .schema import TaskSchema, Schema, FieldSchema
 from .fields import Field
 from .view import View
@@ -100,8 +100,7 @@ def ThingAction(viewcls: View):
         View: View class with Action spec tags
     """
     # Update Views API spec
-    update_spec(viewcls, {"tags": ["actions"]})
-    update_spec(viewcls, {"_groups": ["actions"]})
+    tag_spec(viewcls, "actions")
     return viewcls
 
 
@@ -173,8 +172,7 @@ def wrapped(*args, **kwargs):
         viewcls.put = property_notify(viewcls.put)
 
     # Update Views API spec
-    update_spec(viewcls, {"tags": ["properties"]})
-    update_spec(viewcls, {"_groups": ["properties"]})
+    tag_spec(viewcls, "properties")
     return viewcls
 
 
@@ -281,16 +279,11 @@ def __call__(self, f):
 
 class Tag:
     def __init__(self, tags):
-        if isinstance(tags, str):
-            self.tags = [tags]
-        elif isinstance(tags, list) and all([isinstance(e, str) for e in tags]):
-            self.tags = tags
-        else:
-            raise TypeError("Tags must be a string or list of strings")
+        self.tags = tags
 
     def __call__(self, f):
         # Pass params to call function attribute for external access
-        update_spec(f, {"tags": self.tags})
+        tag_spec(f, self.tags)
         return f
 
 

labthings/server/default_views/root.py

@@ -0,0 +1,7 @@
+from ..find import current_labthing
+from ..view import View
+
+
+class RootView(View):
+    def get(self):
+        return current_labthing().thing_description.to_dict()

labthings/server/default_views/sockets.py

@@ -0,0 +1,16 @@
+from ..sockets import SocketSubscriber, socket_handler_loop
+from ..find import current_labthing
+
+import logging
+
+
+def socket_handler(ws):
+    # Create a socket subscriber
+    wssub = SocketSubscriber(ws)
+    current_labthing().subscribers.add(wssub)
+    logging.info(f"Added subscriber {wssub}")
+    # Start the socket connection handler loop
+    socket_handler_loop(ws)
+    # Remove the subscriber once the loop returns
+    current_labthing().subscribers.remove(wssub)
+    logging.info(f"Removed subscriber {wssub}")

labthings/server/find.py

@@ -1,5 +1,6 @@
 import logging
 from flask import current_app
+import weakref
 
 from .names import EXTENSION_NAME
 
@@ -13,14 +14,15 @@ def current_labthing(app=None):
     # reach the Flask app object. Just using current_app returns
     # a wrapper, which breaks it's use in Task threads
     if not app:
-        app = current_app._get_current_object()  # skipcq: PYL-W0212
-    if not app:
-        return None
-    logging.debug("Active app extensions:")
-    logging.debug(app.extensions)
-    logging.debug("Active labthing:")
-    logging.debug(app.extensions.get(EXTENSION_NAME))
-    return app.extensions.get(EXTENSION_NAME, None)
+        try:
+            app = current_app._get_current_object()  # skipcq: PYL-W0212
+        except RuntimeError:
+            return None
+    ext = app.extensions.get(EXTENSION_NAME, None)
+    if isinstance(ext, weakref.ref):
+        return ext()
+    else:
+        return ext
 
 
 def registered_extensions(labthing_instance=None):

labthings/server/labthing.py

@@ -17,14 +17,17 @@
 from .spec.utilities import get_spec
 from .spec.td import ThingDescription
 from .decorators import tag
-from .sockets import Sockets, SocketSubscriber, socket_handler_loop
+from .sockets import Sockets
 
 from .default_views.extensions import ExtensionList
 from .default_views.tasks import TaskList, TaskView
 from .default_views.docs import docs_blueprint, SwaggerUIView
+from .default_views.root import RootView
+from .default_views.sockets import socket_handler
 
 from ..core.utilities import get_docstring
 
+import weakref
 import logging
 
 
@@ -72,7 +75,7 @@ def __init__(
 
         # Logging handler
         # TODO: Add cleanup code
-        self.log_handler = LabThingLogger(self)
+        self.log_handler = LabThingLogger()
         logging.getLogger().addHandler(self.log_handler)
 
         self.spec = APISpec(
@@ -97,14 +100,23 @@ def description(self, description: str):
         self.spec.description = description
 
     @property
-    def title(self,):
+    def title(self):
         return self._title
 
     @title.setter
     def title(self, title: str):
         self._title = title
         self.spec.title = title
 
+    @property
+    def safe_title(self):
+        title = self.title
+        if not title:
+            title = "unknown"
+        title = title.replace(" ", "")
+        title = title.lower()
+        return title
+
     @property
     def version(self,):
         return str(self._version)
@@ -119,11 +131,9 @@ def version(self, version: str):
     def init_app(self, app):
         self.app = app
 
-        app.teardown_appcontext(self.teardown)
-
         # Register Flask extension
         app.extensions = getattr(app, "extensions", {})
-        app.extensions[EXTENSION_NAME] = self
+        app.extensions[EXTENSION_NAME] = weakref.ref(self)
 
         # Flask error formatter
         if self.format_flask_exceptions:
@@ -145,12 +155,9 @@ def init_app(self, app):
         self.sockets = Sockets(app)
         self._create_base_sockets()
 
-    def teardown(self, exception):
-        pass
-
     def _create_base_routes(self):
         # Add root representation
-        self.app.add_url_rule(self._complete_url("/", ""), "root", self.root)
+        self.add_view(RootView, "/", endpoint="root")
         # Add thing descriptions
         self.app.register_blueprint(
             docs_blueprint, url_prefix=f"{self.url_prefix}/docs"
@@ -166,19 +173,7 @@ def _create_base_routes(self):
         self.add_view(TaskView, "/tasks/<task_id>", endpoint=TASK_ENDPOINT)
 
     def _create_base_sockets(self):
-        self.sockets.add_url_rule(self._complete_url("", ""), self._socket_handler)
-
-    def _socket_handler(self, ws):
-        # Create a socket subscriber
-        wssub = SocketSubscriber(ws)
-        self.subscribers.add(wssub)
-        logging.info(f"Added subscriber {wssub}")
-        # Start the socket connection handler loop
-        socket_handler_loop(ws)
-        # Remove the subscriber once the loop returns
-        self.subscribers.remove(wssub)
-        logging.info(f"Removed subscriber {wssub}")
-        logging.debug(list(self.subscribers))
+        self.sockets.add_view(self._complete_url("", ""), socket_handler)
 
     # Device stuff
 
@@ -300,17 +295,19 @@ def _register_view(self, app, view, *urls, endpoint=None, **kwargs):
 
         # There might be a better way to do this than _rules_by_endpoint,
         # but I can't find one so this will do for now. Skipping PYL-W0212
+        # FIXME: There is a MASSIVE memory leak or something going on in APISpec!
+        # This is grinding tests to a halt, and is really annoying... Should be fixed.
         flask_rules = app.url_map._rules_by_endpoint.get(endpoint)  # skipcq: PYL-W0212
         for flask_rule in flask_rules:
             self.spec.path(**rule_to_apispec_path(flask_rule, view, self.spec))
 
         # Handle resource groups listed in API spec
         view_spec = get_spec(view)
-        view_groups = view_spec.get("_groups", [])
-        if "actions" in view_groups:
+        view_tags = view_spec.get("tags", set())
+        if "actions" in view_tags:
             self.thing_description.action(flask_rules, view)
             self._action_views[view.endpoint] = view
-        if "properties" in view_groups:
+        if "properties" in view_tags:
             self.thing_description.property(flask_rules, view)
             self._property_views[view.endpoint] = view
 
@@ -336,8 +333,3 @@ def add_root_link(self, view, rel, kwargs=None, params=None):
         if params is None:
             params = {}
         self.thing_description.add_link(view, rel, kwargs=kwargs, params=params)
-
-    # Description
-    def root(self):
-        """Root representation"""
-        return self.thing_description.to_dict()

labthings/server/logging.py

@@ -1,17 +1,18 @@
+from .find import current_labthing
+
 from logging import StreamHandler
 import datetime
 
 
 class LabThingLogger(StreamHandler):
-    def __init__(self, labthing):
-        StreamHandler.__init__(self)
-        self.labthing = labthing
+    def __init__(self, *args, **kwargs):
+        StreamHandler.__init__(self, *args, **kwargs)
 
     def emit(self, record):
         log_event = self.rest_format_record(record)
 
         # Broadcast to subscribers
-        subscribers = getattr(self.labthing, "subscribers", [])
+        subscribers = getattr(current_labthing(), "subscribers", [])
         for sub in subscribers:
             sub.event_notify(log_event)
 

labthings/server/monkey.py

@@ -1,3 +1,18 @@
 from gevent.monkey import patch_all
 
 __all__ = ["patch_all"]
+
+"""
+NOTE: THIS FILE IS EXCLUDED FROM OUR UNIT TESTS.
+
+MONKEY PATCHING IN THE MIDDLE OF A TEST SUITE RUNNING
+MAY CAUSE PROBLEMS.
+
+GIVEN THAT THIS MODULE IS SIMPLY A PROXY FOR GEVENTS
+MONKEY PATCHER, TESTSING IS FAIRLY REDUNDANT ANYWAY.
+
+THIS SHOULD BE PERIODICALLY REVISITED TO MAKE SURE ITS
+STILL TRUE.
+
+THANKS
+"""

labthings/server/representations.py

@@ -10,6 +10,8 @@ class LabThingsJSONEncoder(JSONEncoder):
     """
 
     def default(self, o):
+        if isinstance(o, set):
+            return list(o)
         return JSONEncoder.default(self, o)