Repository: eclipse-bluechi/bluechi (language: python)

Issue:
Integration tests for uncovered cases
By implementing https://github.com/eclipse-bluechi/bluechi/issues/397 we are able to collect the overall code coverage when all integration tests are run. The latest results were compressed into the following tarball: [2023-12-14-report.tar.gz](https://github.com/eclipse-bluechi/bluechi/files/13684952/report.tar.gz)

Based on this report, the following cases are not yet covered and integration tests for those should be written:
- [x] Agent reconnect (https://github.com/eclipse-bluechi/bluechi/pull/669)
When reconnecting, the agent doesn't fail at least once, even though that this once intended in [test node reconnect](https://github.com/eclipse-bluechi/bluechi/tree/main/tests/tests/tier0/monitor-node-reconnect)
- [x] Agent using a config file from the `-c` cli option
Could be tested for *3* cases - config is valid, invalid (file doesn't exist) and key-value pair is invalid (e.g. `NodeName`, missing the `=` char)
https://github.com/eclipse-bluechi/bluechi/pull/677
https://github.com/eclipse-bluechi/bluechi/pull/680
- [x] Controller using a config file from the `-c` cli option
Could be tested for *3* cases - config is valid, invalid (file doesn't exist) and key-value pair is invalid (e.g. `AllowedNodeNames`, missing the `=` char)
https://github.com/eclipse-bluechi/bluechi/pull/677
https://github.com/eclipse-bluechi/bluechi/pull/680
- [x] Agent connects to controller via `ControllerAddress` 
https://github.com/eclipse-bluechi/bluechi/pull/741
- [x] Function `ReloadUnit` is used
Could be tested by creating two systemd services. The first would define an `ExecReload=systemctl start <other service>`. 
https://github.com/eclipse-bluechi/bluechi/pull/775
- [x] Function `DaemonReload` is used
Could be tested by creating a systemd services manually, running bluechictl daemon-reload and then checking if new unit exists 
https://github.com/eclipse-bluechi/bluechi/pull/818
- [x] Function `ListUnits` (on manager to list all units for all nodes) is used
https://github.com/eclipse-bluechi/bluechi/pull/760
- [x] Disable collecting metrics again
Either the existing test (see [metrics-start-unit test](https://github.com/eclipse-bluechi/bluechi/tree/main/tests/tests/tier0/metrics-start-unit)) can be extended or a new test can be written
- [x] Function `SetLogLevel` is used
This has to be tested for the manager (`org.eclipse.bluechi.Manager SetLogLevel`) and the agent (`org.eclipse.bluechi.Node SetLogLevel`)
  https://github.com/eclipse-bluechi/bluechi/pull/723
  https://github.com/eclipse-bluechi/bluechi/pull/736
- [x] Agent is stopped while Proxy Service(s) are active


*Note:* The list above is not complete. Quite some code parts are error case handling, e.g. for OOM which is really hard to test. 


Discussion / hints:
/cc @ArtiomDivak @mwperina 
@mkemel 

Fix patch:
diff --git a/src/agent/agent.c b/src/agent/agent.c
index cd694025a5..484c5142d2 100644
--- a/src/agent/agent.c
+++ b/src/agent/agent.c
@@ -1563,6 +1563,7 @@ static int agent_method_disable_metrics(sd_bus_message *m, void *userdata, UNUSE
         }
         agent->metrics_enabled = false;
         sd_bus_slot_unrefp(&agent->metrics_slot);
+        agent->metrics_slot = NULL;
         bc_log_debug("Metrics disabled");
         return sd_bus_reply_method_return(m, "");
 }


Test patch:
diff --git a/tests/bluechi_test/bluechictl.py b/tests/bluechi_test/bluechictl.py
index 0782690d66..9aebc0d0d3 100644
--- a/tests/bluechi_test/bluechictl.py
+++ b/tests/bluechi_test/bluechictl.py
@@ -129,3 +129,21 @@ def set_log_level_on_controller(self,
             check_result,
             expected_result
         )
+
+    def enable_metrics(self, check_result: bool = True, expected_result: int = 0) \
+            -> Tuple[Optional[int], Union[Iterator[bytes], Any, Tuple[bytes, bytes]]]:
+        return self._run(
+            "Enabling metrics",
+            "metrics enable",
+            check_result,
+            expected_result
+        )
+
+    def disable_metrics(self, check_result: bool = True, expected_result: int = 0) \
+            -> Tuple[Optional[int], Union[Iterator[bytes], Any, Tuple[bytes, bytes]]]:
+        return self._run(
+            "Disabling metrics",
+            "metrics disable",
+            check_result,
+            expected_result
+        )
diff --git a/tests/tests/tier0/metrics-enable-and-disable/main.fmf b/tests/tests/tier0/metrics-enable-and-disable/main.fmf
new file mode 100644
index 0000000000..4ffd1db695
--- /dev/null
+++ b/tests/tests/tier0/metrics-enable-and-disable/main.fmf
@@ -0,0 +1,2 @@
+summary: Test if metrics reporting is enabled and disabled correctly
+id: ed71f395-b5f9-44a2-abfb-738c77e0539c
diff --git a/tests/tests/tier0/metrics-enable-and-disable/python/metrics_is_enabled.py b/tests/tests/tier0/metrics-enable-and-disable/python/metrics_is_enabled.py
new file mode 100644
index 0000000000..4a7a3287f5
--- /dev/null
+++ b/tests/tests/tier0/metrics-enable-and-disable/python/metrics_is_enabled.py
@@ -0,0 +1,43 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+
+import unittest
+
+from bluechi.api import Metrics, Node, UInt64
+from dasbus.error import DBusError
+from dasbus.loop import EventLoop
+
+
+class TestMetricsIsEnabled(unittest.TestCase):
+
+    def setUp(self) -> None:
+        self.loop = EventLoop()
+        self.metrics = Metrics()
+        self.signal_received = False
+        self.loop_quitter = None
+
+    def test_metrics_is_enabled(self):
+        def on_start_unit_metrics_received(node_name: str,
+                                           job_id: str,
+                                           unit: str,
+                                           job_time_micros: UInt64,
+                                           start_time: UInt64) -> None:
+            if (node_name == "node-foo" and unit == "simple.service"):
+                self.signal_received = True
+                self.loop.quit()
+        try:
+            # This will fail if metrics are disabled
+            self.metrics.on_start_unit_job_metrics(on_start_unit_metrics_received)
+        except DBusError as ex:
+            raise Exception(f"Error subscribing to metrics: {ex}")
+
+        n = Node("node-foo")
+        assert n.start_unit('simple.service', 'replace') != ""
+
+        self.loop.run()
+        assert n.stop_unit('simple.service', 'replace') != ""
+
+        assert self.signal_received
+
+
+if __name__ == "__main__":
+    unittest.main()
diff --git a/tests/tests/tier0/metrics-enable-and-disable/systemd/simple.service b/tests/tests/tier0/metrics-enable-and-disable/systemd/simple.service
new file mode 100644
index 0000000000..acacd10fca
--- /dev/null
+++ b/tests/tests/tier0/metrics-enable-and-disable/systemd/simple.service
@@ -0,0 +1,7 @@
+[Unit]
+Description=Just sleeping
+
+[Service]
+Type=simple
+ExecStart=/bin/true
+RemainAfterExit=yes
diff --git a/tests/tests/tier0/metrics-enable-and-disable/test_metrics_enable_and_disable.py b/tests/tests/tier0/metrics-enable-and-disable/test_metrics_enable_and_disable.py
new file mode 100644
index 0000000000..0aa4a3f653
--- /dev/null
+++ b/tests/tests/tier0/metrics-enable-and-disable/test_metrics_enable_and_disable.py
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+
+import os
+import logging
+from typing import Dict
+
+from bluechi_test.config import BluechiControllerConfig, BluechiAgentConfig
+from bluechi_test.machine import BluechiControllerMachine, BluechiAgentMachine
+from bluechi_test.test import BluechiTest
+
+LOGGER = logging.getLogger(__name__)
+
+node_foo_name = "node-foo"
+simple_service = "simple.service"
+
+
+def exec(ctrl: BluechiControllerMachine, nodes: Dict[str, BluechiAgentMachine]):
+    foo = nodes[node_foo_name]
+
+    source_dir = "systemd"
+    target_dir = os.path.join("/", "etc", "systemd", "system")
+    foo.copy_systemd_service(simple_service, source_dir, target_dir)
+    assert foo.wait_for_unit_state_to_be(simple_service, "inactive")
+
+    result, output = ctrl.run_python(os.path.join("python", "metrics_is_enabled.py"))
+    if result == 0:
+        raise Exception(f"Metrics not disabled: {output}")
+    assert foo.wait_for_unit_state_to_be(simple_service, "inactive")
+
+    ctrl.bluechictl.enable_metrics()
+
+    result, output = ctrl.run_python(os.path.join("python", "metrics_is_enabled.py"))
+    if result != 0:
+        raise Exception(f"Metrics not enabled: {output}")
+    assert foo.wait_for_unit_state_to_be(simple_service, "inactive")
+
+    ctrl.bluechictl.disable_metrics()
+
+    result, output = ctrl.run_python(os.path.join("python", "metrics_is_enabled.py"))
+    if result == 0:
+        raise Exception(f"Metrics not disabled: {output}")
+    assert foo.wait_for_unit_state_to_be(simple_service, "inactive")
+
+
+def test_metrics_enable_and_disable(
+        bluechi_test: BluechiTest,
+        bluechi_ctrl_default_config: BluechiControllerConfig,
+        bluechi_node_default_config: BluechiAgentConfig):
+
+    node_foo_cfg = bluechi_node_default_config.deep_copy()
+    node_foo_cfg.node_name = node_foo_name
+
+    bluechi_ctrl_default_config.allowed_node_names = [node_foo_name]
+
+    bluechi_test.set_bluechi_controller_config(bluechi_ctrl_default_config)
+    bluechi_test.add_bluechi_agent_config(node_foo_cfg)
+
+    bluechi_test.run(exec)