Add pytest unit tests (#251)

* Refactor TokenFile, add tests

* Add api.py tests

* Add errorbuilder.py tests

* Remove invalid assertions

* Test tokenfile class methods on class

In the app, we use these methods on the class itself, not instances of it.

* Add unit test workflow job

* Add fault tolerance for invalid token file

* Refactor TokenFile.validate_tokens

---------

Co-authored-by: Terje Kvernes <terjekv@users.noreply.github.com>

.github/workflows/test.yml

@@ -38,7 +38,6 @@ jobs:
         python-version:
           - "3.11"
           - "3.12"
-
     steps:
       - uses: actions/checkout@v4
       - name: Install uv
@@ -49,6 +48,27 @@ jobs:
         run: |
           uv venv
           uv pip install tox-uv tox-gh-actions 
-      - name: Test with tox
+      - name: Test building with tox
         run: uv run tox r
-
+
+  unit:
+    name: Unit tests
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        python-version:
+          - "3.11"
+          - "3.12"
+    steps:
+      - uses: actions/checkout@v4
+      - name: Install uv
+        uses: astral-sh/setup-uv@v2
+      - name: Set up Python ${{ matrix.python-version }}
+        run: uv python install ${{ matrix.python-version }}
+      - name: Install dependencies
+        run: |
+          uv venv
+          uv pip install -U -e ".[test]"
+      - name: Run unittest
+        run: uv run pytest

mreg_cli/tokenfile.py

@@ -5,9 +5,9 @@
 import json
 import os
 import sys
-from typing import Optional
+from typing import Any, Optional, Self
 
-from pydantic import BaseModel
+from pydantic import BaseModel, TypeAdapter, ValidationError
 
 # The contents of the token file is:
 
@@ -28,47 +28,61 @@ class Token(BaseModel):
     username: str
 
 
+TokenList = TypeAdapter(list[Token])
+
+
 class TokenFile:
     """A class for managing tokens in a JSON file."""
 
     tokens_path: str = os.path.join(os.getenv("HOME", ""), ".mreg-cli_auth_token.json")
 
-    def __init__(self, tokens: Optional[list[dict[str, str]]] = None):
+    def __init__(self, tokens: Any = None):
         """Initialize the TokenFile instance."""
-        self.tokens = [Token(**token) for token in tokens] if tokens else []
-
-    @classmethod
-    def _load_tokens(cls) -> "TokenFile":
-        """Load tokens from a JSON file, returning a new instance of TokenFile."""
-        try:
-            with open(cls.tokens_path, "r") as file:
-                data = json.load(file)
-                return TokenFile(tokens=data["tokens"])
-        except (FileNotFoundError, KeyError):
-            return TokenFile(tokens=[])
-
-    @classmethod
-    def _set_file_permissions(cls, mode: int) -> None:
+        self.tokens = self._validate_tokens(tokens)
+
+    def _validate_tokens(self, tokens: Any) -> list[Token]:
+        """Convert deserialized JSON to list of Token objects."""
+        if tokens:
+            try:
+                return TokenList.validate_python(tokens)
+            except ValidationError as e:
+                print(
+                    f"Failed to validate tokens from token file {self.tokens_path}: {e}",
+                    file=sys.stderr,
+                )
+        return []
+
+    def _set_file_permissions(self, mode: int) -> None:
         """Set the file permissions for the token file."""
         try:
-            os.chmod(cls.tokens_path, mode)
+            os.chmod(self.tokens_path, mode)
         except PermissionError:
-            print("Failed to set permissions on " + cls.tokens_path, file=sys.stderr)
+            print(f"Failed to set permissions on {self.tokens_path}", file=sys.stderr)
         except FileNotFoundError:
             pass
 
-    @classmethod
-    def _save_tokens(cls, tokens: "TokenFile") -> None:
+    def save(self) -> None:
         """Save tokens to a JSON file."""
-        with open(cls.tokens_path, "w") as file:
-            json.dump({"tokens": [token.model_dump() for token in tokens.tokens]}, file, indent=4)
+        with open(self.tokens_path, "w") as file:
+            json.dump({"tokens": [token.model_dump() for token in self.tokens]}, file, indent=4)
+        self._set_file_permissions(0o600)
 
-        cls._set_file_permissions(0o600)
+    @classmethod
+    def load(cls) -> Self:
+        """Load tokens from a JSON file, returning a new instance of TokenFile."""
+        try:
+            with open(cls.tokens_path, "r") as file:
+                data = json.load(file)
+                return cls(tokens=data.get("tokens"))
+        except (FileNotFoundError, KeyError, json.JSONDecodeError) as e:
+            if isinstance(e, json.JSONDecodeError):
+                print(f"Failed to decode JSON in tokens file {cls.tokens_path}", file=sys.stderr)
+            return cls(tokens=[])
 
     @classmethod
     def get_entry(cls, username: str, url: str) -> Optional[Token]:
         """Retrieve a token by username and URL."""
-        tokens_file = cls._load_tokens()
+        tokens_file = cls.load()
         for token in tokens_file.tokens:
             if token.url == url and token.username == username:
                 return token
@@ -77,13 +91,11 @@ def get_entry(cls, username: str, url: str) -> Optional[Token]:
     @classmethod
     def set_entry(cls, username: str, url: str, new_token: str) -> None:
         """Update or add a token based on the URL and username."""
-        tokens_file = cls._load_tokens()
+        tokens_file = cls.load()
         for token in tokens_file.tokens:
             if token.url == url and token.username == username:
                 token.token = new_token
-                cls._save_tokens(tokens_file)
-                return
-
+                return tokens_file.save()
         # If not found, add a new token
         tokens_file.tokens.append(Token(token=new_token, url=url, username=username))
-        cls._save_tokens(tokens_file)
+        tokens_file.save()

pyproject.toml

@@ -49,7 +49,17 @@ dependencies = [
 dynamic = ["version"]
 
 [project.optional-dependencies]
-dev = ["ruff", "tox-uv", "rich", "setuptools", "setuptools-scm", "build"]
+test = ["pytest", "inline-snapshot", "pytest-httpserver"]
+dev = [
+    "mreg-cli[test]",
+    "ruff",
+    "tox-uv",
+    "rich",
+    "setuptools",
+    "setuptools-scm",
+    "build",
+    "pyinstaller",
+]
 
 [project.urls]
 Repository = 'https://github.com/unioslo/mreg-cli/'

tests/conftest.py

@@ -0,0 +1,37 @@
+from __future__ import annotations
+
+import os
+from typing import Iterator
+
+import pytest
+from pytest_httpserver import HTTPServer
+
+from mreg_cli.config import MregCliConfig
+
+
+@pytest.fixture(autouse=True)
+def set_url_env(httpserver: HTTPServer) -> Iterator[None]:
+    """Set the config URL to the test HTTP server URL."""
+    conf = MregCliConfig()
+    pre_override_conf = conf._config_cmd.copy()  # pyright: ignore[reportPrivateUsage]
+    conf._config_cmd["url"] = httpserver.url_for("/")  # pyright: ignore[reportPrivateUsage]
+    yield
+    conf._config_cmd = pre_override_conf  # pyright: ignore[reportPrivateUsage]
+
+
+@pytest.fixture(autouse=True if os.environ.get("PYTEST_HTTPSERVER_STRICT") else False)
+def check_assertions(httpserver: HTTPServer) -> Iterator[None]:
+    """Ensure all HTTP server assertions are checked after the test."""
+    # If the HTTP server raises errors or has failed assertions in its handlers
+    # themselves, we want to raise an exception to fail the test.
+    #
+    # The `check_assertions` method will raise an exception if there are
+    # if any tests have HTTP test server errors.
+    # See: https://pytest-httpserver.readthedocs.io/en/latest/tutorial.html#handling-test-errors
+    #      https://pytest-httpserver.readthedocs.io/en/latest/howto.html#using-custom-request-handler
+    #
+    # If a test has an assertion or handler error that is expected, it should
+    # call `httpserver.clear_assertions()` and/or `httpserver.clear_handler_errors()` as needed.
+    yield
+    httpserver.check_assertions()
+    httpserver.check_handler_errors()

tests/test_errorbuilder.py

@@ -0,0 +1,56 @@
+from __future__ import annotations
+
+import pytest
+
+from mreg_cli.errorbuilder import (
+    ErrorBuilder,
+    FallbackErrorBuilder,
+    FilterErrorBuilder,
+    build_error_message,
+    get_builder,
+)
+
+
+@pytest.mark.parametrize(
+    "command, exc_or_str, expected",
+    [
+        (
+            r"permission label_add 192.168.0.0/24 oracle-group ^(db|cman)ora.*\.example\.com$ oracle",
+            "failed to compile regex",
+            FilterErrorBuilder,
+        ),
+        (
+            r"permission label_add other_error",
+            "Other error message",
+            FallbackErrorBuilder,
+        ),
+    ],
+)
+def test_get_builder(command: str, exc_or_str: str, expected: type[ErrorBuilder]) -> None:
+    builder = get_builder(command, exc_or_str)
+    assert builder.__class__ == expected
+    assert builder.get_underline(0, 0) == ""
+    assert builder.get_underline(0, 10) == "^^^^^^^^^^"
+    assert builder.get_underline(5, 10) == "     ^^^^^"
+
+
+@pytest.mark.parametrize(
+    "command, exc_or_str, expected",
+    [
+        (
+            r"permission label_add 192.168.0.0/24 oracle-group ^(db|cman)ora.*\.example\.com$ oracle",
+            r"Unable to compile regex 'cman)ora.*\.example\.com$ oracle'",
+            r"""Unable to compile regex 'cman)ora.*\.example\.com$ oracle'
+permission label_add 192.168.0.0/24 oracle-group ^(db|cman)ora.*\.example\.com$ oracle
+                                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+                                                 └ Consider enclosing this part in quotes.""",
+        ),
+        (
+            r"permission label_add other_error",
+            "Other error message",
+            "Other error message",
+        ),
+    ],
+)
+def test_build_error_message(command: str, exc_or_str: str, expected: str) -> None:
+    assert build_error_message(command, exc_or_str) == expected