Refine tests and comments

.gitignore

@@ -1,6 +1,4 @@
 # This is a template `.gitignore` file for snaps
-# This file is managed by bootstack-charms-spec and should not be modified
-# within individual snap repos. https://launchpad.net/bootstack-charms-spec
 
 # Byte-compiled / optimized / DLL files
 __pycache__/

pyproject.toml

@@ -1,7 +1,3 @@
-# This is a template `pyproject.toml` file for snaps
-# This file is managed by bootstack-charms-spec and should not be modified
-# within individual snap repos. https://launchpad.net/bootstack-charms-spec
-
 [tool.flake8]
 ignore = ["C901", "D100", "D101", "D102", "D103", "W503", "W504"]
 exclude = ['.eggs', '.git', '.tox', '.venv', '.build', 'build', 'report']

snap/hooks/configure

@@ -5,10 +5,10 @@
 # `sudo snap get dcgm`.
 if [ -z "$(snapctl get nv-hostengine-port)" ]; then
     # Setting to empty string for the nv-hostengine binary to use the default port. (5555)
-    snapctl set nv-hostengine-port=""
+    snapctl set nv-hostengine-port="5555"
 fi
 
 if [ -z "$(snapctl get dcgm-exporter-address)" ]; then
     # Setting to empty string for the dcgm-exporter binary to use the default address. (:9400)
-    snapctl set dcgm-exporter-address=""
+    snapctl set dcgm-exporter-address=":9400"
 fi

snap/snapcraft.yaml

@@ -25,7 +25,9 @@ apps:
     plugs:
       - network-bind
       - opengl
-    daemon: simple
+    daemon: simple 
+    # As this is a dcgm snap, not the dcgm-exporter snap,
+    # user might not be interested in running dcgm-exporter, so disable it by default
     install-mode: disable
     restart-condition: on-failure
   dcgmi:

tests/functional/conftest.py

@@ -1,20 +1,6 @@
 import subprocess
 
 import pytest
-from tenacity import retry, stop_after_delay, wait_fixed
-
-
-@retry(wait=wait_fixed(5), stop=stop_after_delay(20))
-def check_dcgm_exporter_service():
-    dcgm_exporter_service = "dcgm.dcgm-exporter"
-
-    result = subprocess.run(
-        f"sudo snap services {dcgm_exporter_service}".split(),
-        check=True,
-        capture_output=True,
-        text=True,
-    )
-    assert " active" in result.stdout.strip(), f"{dcgm_exporter_service} service is not active"
 
 
 @pytest.fixture(scope="session", autouse=True)
@@ -30,7 +16,6 @@ def install_dcgm_snap():
     )
 
     subprocess.run("sudo snap start dcgm.dcgm-exporter".split(), check=True)
-    check_dcgm_exporter_service()
 
     yield
 

tests/functional/requirments.txt → tests/functional/requirements.txt

@@ -1,2 +1 @@
 tenacity
-pyyaml

tests/functional/test_snap_dcgm.py

@@ -1,66 +1,78 @@
 import json
 import subprocess
 import urllib.request
-from time import sleep
 
-from tenacity import retry, stop_after_delay, wait_fixed
+import pytest
+from tenacity import retry, stop_after_delay, wait_fixed, Retrying
 
 
 @retry(wait=wait_fixed(5), stop=stop_after_delay(30))
 def test_dcgm_exporter():
     """Test of the dcgm-exporter service and its endpoint."""
+    dcgm_exporter_service = "snap.dcgm.dcgm-exporter"
     endpoint = "http://localhost:9400/metrics"
 
+    assert 0 == subprocess.call(
+        f"sudo systemctl is-active --quiet {dcgm_exporter_service}".split()
+    ), f"{dcgm_exporter_service} is not running"
+
     # Check the exporter endpoint, will raise an exception if the endpoint is not reachable
-    urllib.request.urlopen(endpoint)
+    response = urllib.request.urlopen(endpoint)
+
+    # The output of the exporter endpoint is not tested
+    # as in a virtual environment it will not have any GPU metrics
+    assert 200 == response.getcode(), "DCGM exporter endpoint returned an error"
 
 
 def test_dcgm_nv_hostengine():
     """Check the dcgm-nv-hostengine service."""
-    nv_hostengine_service = "dcgm.nv-hostengine"
+    nv_hostengine_service = "snap.dcgm.nv-hostengine"
+    nv_hostengine_port = 5555
 
-    service = subprocess.run(
-        f"snap services {nv_hostengine_service}".split(),
-        check=True,
-        capture_output=True,
-        text=True,
-    )
+    assert 0 == subprocess.call(
+        f"sudo systemctl is-active --quiet {nv_hostengine_service}".split()
+    ), f"{nv_hostengine_service} is not running"
 
-    assert " active" in service.stdout.strip(), f"{nv_hostengine_service} service is not active"
+    assert 0 == subprocess.call(
+        f"nc -z localhost {nv_hostengine_port}".split()
+    ), f"{nv_hostengine_service} is not listening on port {nv_hostengine_port}"
 
 
 def test_dcgmi():
     """Test of the dcgmi command."""
     result = subprocess.run(
         "dcgm.dcgmi discovery -l".split(), check=True, capture_output=True, text=True
     )
-    assert "GPU ID" in result.stdout.strip(), "DCGMI is not working"
 
+    # Test if the command is working and outputs a table with the GPU ID
+    # The table will be empty in a virtual environment, but the command should still work
+    assert "GPU ID" in result.stdout.strip(), "DCGMI didn't produce the expected table"
 
-def test_dcgm_bind_configs():
-    """Test snap port configuratin."""
-    services = ["dcgm.dcgm-exporter", "dcgm.nv-hostengine"]
-    configs = ["dcgm-exporter-address", "nv-hostengine-port"]
-    new_values = [":9466", "5666"]
 
+bind_test_data = [
+    ("dcgm.dcgm-exporter", "dcgm-exporter-address", ":9466"),
+    ("dcgm.nv-hostengine", "nv-hostengine-port", "5566"),
+]
+
+
+@pytest.mark.parametrize("service, config, new_value", bind_test_data)
+def test_dcgm_bind_config(service: str, config: str, new_value: str):
+    """Test snap bind configuration."""
     result = subprocess.run(
         "sudo snap get dcgm -d".split(), check=True, capture_output=True, text=True
     )
     dcgm_snap_config = json.loads(result.stdout.strip())
-    assert all(config in dcgm_snap_config for config in configs), "Missing snap configuration keys"
+    assert config in dcgm_snap_config, f"{config} is not in the snap configuration"
 
-    for config, new_value in zip(configs, new_values):
-        subprocess.run(
-            f"sudo snap set dcgm {config}={new_value}".split(), check=True
-        ), f"Failed to set {config}"
+    assert 0 == subprocess.call(
+        f"sudo snap set dcgm {config}={new_value}".split()
+    ), f"Failed to set {config} to {new_value}"
 
     # restart the service to apply the new configuration
-    for service in services:
-        subprocess.run(f"sudo snap restart {service}".split(), check=True)
-
-    sleep(5)
+    subprocess.run(f"sudo snap restart {service}".split(), check=True)
 
-    for service, port in zip(services, new_values):
-        subprocess.run(
-            f"sudo lsof -i :{port.lstrip(':')}".split(), check=True
-        ), f"{service} port is not listening"
+    for attempt in Retrying(wait=wait_fixed(2), stop=stop_after_delay(10)):
+        with attempt:
+            assert 0 == subprocess.call(
+                f"nc -z localhost {new_value.lstrip(':')}".split()
+            ), f"{service} is not listening on {new_value}"

tox.ini

@@ -1,7 +1,3 @@
-# This is a template `tox.ini` file for snaps
-# This file is managed by bootstack-charms-spec and should not be modified
-# within individual snap repos. https://launchpad.net/bootstack-charms-spec
-
 [tox]
 skipsdist=True
 envlist = lint, unit, func
@@ -39,7 +35,7 @@ commands =
 [testenv:func]
 deps =
     pytest
-    -r {toxinidir}/tests/functional/requirments.txt
+    -r {toxinidir}/tests/functional/requirements.txt
 passenv =
     TEST_*
 commands =