Merge pull request #104 from pymeasure/increase-test-coverage

Increase test coverage

pyleco/management/starter.py

@@ -35,7 +35,7 @@
 if __name__ != "__main__":
     from ..utils.message_handler import MessageHandler
     from ..utils.parser import parser, parse_command_line_parameters
-else:
+else:  # pragma: no cover
     from pyleco.utils.message_handler import MessageHandler
     from pyleco.utils.parser import parser, parse_command_line_parameters
 
@@ -119,9 +119,7 @@ def __init__(
             self.folder_name = "test_tasks"
 
         log.info(f"Starter started with tasks in folder '{self.directory}'.")
-        if tasks is not None:
-            for task in tasks:
-                self.start_task(task)
+        self.start_tasks(tasks or ())
 
     def register_rpc_methods(self) -> None:
         super().register_rpc_methods()
@@ -140,21 +138,13 @@ def _listen_close(self, waiting_time: Optional[int] = None) -> None:
         log.info("Starter stopped.")
 
     def stop_all_tasks(self) -> None:
+        self.started_tasks = {}
         keys = list(self.threads.keys())
         for name in keys:
             # set all stop signals
             self.events[name].set()
         for name in keys:
-            # wait for threads to have stopped
-            thread = self.threads[name]
-            thread.join(2)
-            if thread.is_alive():
-                log.warning(f"Task '{name}' did not stop in time!")
-                # TODO add possibility to stop thread otherwise.
-            try:
-                del self.threads[name]
-            except Exception as exc:
-                log.exception(f"Deleting task {name} failed", exc_info=exc)
+            self.wait_for_stopped_thread(name)
 
     def heartbeat(self) -> None:
         """Check installed tasks at heartbeating."""
@@ -205,6 +195,9 @@ def stop_task(self, name: str) -> None:
             return
         log.info(f"Stopping task '{name}'.")
         self.events[name].set()
+        self.wait_for_stopped_thread(name)
+
+    def wait_for_stopped_thread(self, name: str) -> None:
         thread = self.threads[name]
         thread.join(timeout=2)
         if thread.is_alive():
@@ -265,9 +258,9 @@ def list_tasks(self) -> list[dict[str, str]]:
             if name.endswith(".py") and not name == "__init__.py":
                 with open(f"{self.directory}/{name}", "r") as file:
                     # Search for the first line with triple quotes
-                    i = 0
-                    while not file.readline().strip() == '"""' and i < 10:
-                        i += 1
+                    for i in range(10):
+                        if file.readline().strip() == '"""':
+                            break
                     tooltip = file.readline()  # first line after line with triple quotes
                 tasks.append({"name": name.replace(".py", ""), "tooltip": tooltip})
         log.debug(f"Tasks found: {tasks}.")

pyleco/management/test_tasks/failing_task.py

@@ -0,0 +1,3 @@
+# it shall fail.
+
+raise ValueError("I fail for testing.")

pyleco/management/test_tasks/no_task.py

@@ -0,0 +1,3 @@
+"""
+Task which can be imported, but not started as method `task` is missing.
+"""

pyproject.toml

@@ -88,6 +88,8 @@ omit = [
     # Omit LECO definitions
     "pyleco/errors.py",
     "pyleco/core/leco_protocols.py",
-    # exclude import file
+    # omit import file
     "pyleco/json_utils/rpc_server.py",
+    # omit files for testing only
+    "pyleco/management/test_tasks/*",
 ]

tests/acceptance_tests/test_starter_live.py

@@ -67,7 +67,7 @@ def director():
         thread.daemon = True
         thread.start()
     sleep(1)
-    director = StarterDirector(actor="starter", port=PORT)
+    director = StarterDirector(actor="starter", port=PORT, timeout=5)
     yield director
     log.info("Tearing down")
     stop_event.set()
@@ -82,9 +82,21 @@ def test_sign_in(director: StarterDirector):
 
 
 def test_tasks_listing(director: StarterDirector):
-    assert director.list_tasks() == [{
-        "name": "test_task",
-        "tooltip": "Example scheme for an Actor for pymeasure instruments. 'test_task'\n"}]
+    tasks = director.list_tasks()
+    expected_tasks = [
+        {"name": "failing_task", "tooltip": ""},
+        {
+            "name": "no_task",
+            "tooltip": "Task which can be imported, but not started as method `task` is missing.\n",
+        },
+        {
+            "name": "test_task",
+            "tooltip": "Example scheme for an Actor for pymeasure instruments. 'test_task'\n",
+        },
+    ]
+    for t in expected_tasks:
+        assert t in tasks
+    assert len(tasks) == len(expected_tasks), "More tasks present than expected."
 
 
 def test_start_task(director: StarterDirector):
@@ -95,8 +107,15 @@ def test_start_task(director: StarterDirector):
 
 
 def test_stop_task(director: StarterDirector):
-    sleep(1)
     director.stop_tasks("test_task")
     status = Status(director.status_tasks("test_task").get("test_task", 0))
     assert Status.STARTED not in status
     assert Status.RUNNING not in status
+
+
+def test_start_task_again(director: StarterDirector):
+    director.start_tasks(["test_task", "failing_task", "no_task"])
+    status = Status(director.status_tasks("test_task")["test_task"])
+    assert Status.STARTED in status
+    assert Status.RUNNING in status
+    director.stop_tasks(["test_task", "no_task"])

tests/management/test_data_logger.py

@@ -321,6 +321,9 @@ class Test_last:
     def test_last_returns_last_value(self, data_logger: DataLogger):
         assert data_logger.last([1, 2, 3, 4, 5]) == 5
 
+    def test_return_single_value(self, data_logger: DataLogger):
+        assert data_logger.last(5) == 5  # type: ignore
+
     def test_empty_list_returns_nan(self, data_logger: DataLogger):
         assert isnan(data_logger.last([]))
 

tests/management/test_starter.py

@@ -63,6 +63,17 @@ def test_sanitize_tasks(tasks):
         assert isinstance(t, str)
 
 
+@pytest.mark.parametrize("tasks, invalid_task_name", (
+        (5, 5),
+        (("valid", 6), 6),
+        ([["list"], "abc"], "['list']")),
+)
+def test_invalid_tasks(tasks, invalid_task_name, caplog):
+    assert sanitize_tasks(tasks) == ()
+    assert caplog.messages == [f"Invalid task name '{invalid_task_name}' received."]
+
+
+
 def test_init(starter: Starter):
     assert starter.started_tasks == {}
     assert starter.threads == {}
@@ -220,3 +231,20 @@ def test_restart_tasks(starter: Starter):
     starter.restart_tasks(["a", "b"])
     assert starter.stop_task.call_args_list == [call("a"), call("b")]
     assert starter.start_task.call_args_list == [call("a"), call("b")]
+
+
+def test_stop_all_tasks(starter: Starter):
+    # arrange
+    starter.started_tasks["t1"] = Status.STARTED
+    starter.threads["t1"] = FakeThread(alive=True)  # type: ignore
+    event = starter.events["t1"] = SimpleEvent()  # type: ignore
+    # act
+    starter.stop_all_tasks()
+    assert "t1" not in starter.threads
+    assert "t1" not in starter.started_tasks
+    assert event.is_set() is True
+
+
+def test_list_tasks_failing(starter: Starter):
+    starter.directory = "/abcdefghijklmno"
+    assert starter.list_tasks() == []

tests/utils/test_extended_message_handler.py

@@ -42,7 +42,6 @@ def handler():
     handler.stop_event = SimpleEvent()
     handler.subscriber = FakeSocket(2)  # type: ignore
     handler.handle_subscription_message = MagicMock()  # it is not defined
-    handler.handle_subscription_message.assert_called_once_with
     return handler
 
 
@@ -54,6 +53,12 @@ def test_read_subscription_message_calls_handle(handler: ExtendedMessageHandler)
     handler.handle_subscription_message.assert_called_once_with(message)  # type: ignore
 
 
+def test_handle_subscription_message_raises_not_implemented():
+    handler = ExtendedMessageHandler(name="handler", context=FakeContext())  # type: ignore
+    with pytest.raises(NotImplementedError):
+        handler.handle_subscription_message(DataMessage(b"topic"))
+
+
 def test_read_subscription_message_calls_handle_legacy(handler: ExtendedMessageHandler):
     message = DataMessage("", data="[]", message_type=234)
     handler.handle_full_legacy_subscription_message = MagicMock()  # type: ignore[method-assign]
@@ -69,6 +74,15 @@ def test_subscribe_single(handler: ExtendedMessageHandler):
     assert handler._subscriptions == [b"topic"]
 
 
+def test_subscribe_single_again(handler: ExtendedMessageHandler, caplog: pytest.LogCaptureFixture):
+    # arrange
+    handler.subscribe_single(b"topic")
+    caplog.set_level(10)
+    # act
+    handler.subscribe_single(b"topic")
+    assert caplog.messages[-1] == f"Already subscribed to {b'topic'!r}."
+
+
 @pytest.mark.parametrize("topics, result", (
         ("topic", [b"topic"]),  # single string
         (["topic1", "topic2"], [b"topic1", b"topic2"]),  # list of strings
@@ -129,3 +143,7 @@ def test_handle_unknown_message_type(self, handler_hfl: ExtendedMessageHandler):
             handler_hfl.handle_full_legacy_subscription_message(
                 DataMessage("topic", data="", message_type=210)
             )
+
+    def test_handle_subscription_data(self, handler: ExtendedMessageHandler):
+        with pytest.raises(NotImplementedError):
+            handler.handle_subscription_data({})