From 280a72cba0af1ab1a58398bccd15c28b5fdf0f5a Mon Sep 17 00:00:00 2001
From: Jens Bergmann <1742418+1cu@users.noreply.github.com>
Date: Mon, 14 Jul 2025 12:38:23 +0200
Subject: [PATCH] feat: Refactor and expand CLI smoke tests for
 subcommand/config coverage (#581)

---
 tests/smoke/test_smoke_health.py | 174 +++++++++++++++++--------------
 tests/unit/test_config_model.py  |  16 ++-
 tests/unit/test_init.py          |  26 +++++
 3 files changed, 134 insertions(+), 82 deletions(-)

diff --git a/tests/smoke/test_smoke_health.py b/tests/smoke/test_smoke_health.py
index 578526e..b1f479e 100644
--- a/tests/smoke/test_smoke_health.py
+++ b/tests/smoke/test_smoke_health.py
@@ -6,16 +6,18 @@ Minimal smoke tests: post-deployment health checks for kleinanzeigen-bot.
 These tests verify that the most essential components are operational.
 """
 
-import logging
+import json
+import re
 import subprocess  # noqa: S404
 import sys
 from pathlib import Path
+from typing import Callable
 
 import pytest
+from ruyaml import YAML
 
 from kleinanzeigen_bot.model.config_model import Config
-from kleinanzeigen_bot.utils import i18n
-from tests.conftest import DummyBrowser, DummyPage, SmokeKleinanzeigenBot
+from tests.conftest import SmokeKleinanzeigenBot
 
 
 def run_cli_subcommand(args:list[str], cwd:str | None = None) -> subprocess.CompletedProcess[str]:
@@ -37,85 +39,95 @@ def test_app_starts(smoke_bot:SmokeKleinanzeigenBot) -> None:
 
 
 @pytest.mark.smoke
-def test_config_loads() -> None:
-    """Smoke: Minimal config loads successfully."""
-    minimal_cfg = {
-        "ad_defaults": {"contact": {"name": "dummy", "zipcode": "12345"}},
-        "login": {"username": "dummy", "password": "dummy"},
-        "publishing": {"delete_old_ads": "BEFORE_PUBLISH", "delete_old_ads_by_title": False},
-    }
-    config = Config.model_validate(minimal_cfg)
-    assert config.login.username == "dummy"
-    assert config.login.password == "dummy"  # noqa: S105
+@pytest.mark.parametrize("subcommand", [
+    "--help",
+    "help",
+    "version",
+])
+def test_cli_subcommands_no_config(subcommand:str, tmp_path:Path) -> None:
+    """
+    Smoke: CLI subcommands that do not require a config file (--help, help, version).
+    """
+    args = [subcommand]
+    result = run_cli_subcommand(args, cwd = str(tmp_path))
+    assert result.returncode == 0
+    out = (result.stdout + "\n" + result.stderr).lower()
+    if subcommand in {"--help", "help"}:
+        assert "usage" in out or "help" in out, f"Expected help text in CLI output.\n{out}"
+    elif subcommand == "version":
+        assert re.match(r"^\s*\d{4}\+\w+", result.stdout.strip()), f"Output does not look like a version string: {result.stdout}"
 
 
 @pytest.mark.smoke
-def test_logger_initializes(tmp_path:Path, caplog:pytest.LogCaptureFixture) -> None:
-    """Smoke: Logger can be initialized and used, robust to pytest log capture."""
-    log_path = tmp_path / "smoke_test.log"
-    logger_name = "smoke_test_logger_unique"
-    logger = logging.getLogger(logger_name)
-    logger.setLevel(logging.DEBUG)
-    logger.propagate = False
-    # Remove all handlers to start clean
-    for h in list(logger.handlers):
-        logger.removeHandler(h)
-    # Create and attach a file handler
-    handle = logging.FileHandler(str(log_path), encoding = "utf-8")
-    handle.setLevel(logging.DEBUG)
-    formatter = logging.Formatter("%(levelname)s:%(name)s:%(message)s")
-    handle.setFormatter(formatter)
-    logger.addHandler(handle)
-    # Log a message
-    logger.info("Smoke test log message")
-    # Flush and close the handler
-    handle.flush()
-    handle.close()
-    # Remove the handler from the logger
-    logger.removeHandler(handle)
-    assert log_path.exists()
-    with open(log_path, "r", encoding = "utf-8") as f:
-        contents = f.read()
-    assert "Smoke test log message" in contents
-
-
-@pytest.mark.smoke
-def test_translation_system_healthy() -> None:
-    """Smoke: Translation system loads and retrieves a known key."""
-    # Use a known string that should exist in translations (fallback to identity)
-    en = i18n.translate("Login", None)
-    assert isinstance(en, str)
-    assert len(en) > 0
-    # Switch to German and test
-    i18n.set_current_locale(i18n.Locale("de"))
-    de = i18n.translate("Login", None)
-    assert isinstance(de, str)
-    assert len(de) > 0
-    # Reset locale
-    i18n.set_current_locale(i18n.Locale("en"))
-
-
-@pytest.mark.smoke
-def test_dummy_browser_session() -> None:
-    """Smoke: Dummy browser session can be created and closed."""
-    browser = DummyBrowser()
-    page = browser.page
-    assert isinstance(page, DummyPage)
-    browser.stop()  # Should not raise
-
-
-@pytest.mark.smoke
-def test_cli_entrypoint_help_runs() -> None:
-    """Smoke: CLI entry point runs with --help and exits cleanly (subprocess)."""
-    result = run_cli_subcommand(["--help"])
-    assert result.returncode in {0, 1}, f"CLI exited with unexpected code: {result.returncode}\nstdout: {result.stdout}\nstderr: {result.stderr}"
-    assert "Usage" in result.stdout or "usage" in result.stdout or "help" in result.stdout.lower(), f"No help text in CLI output: {result.stdout}"
-
-
-@pytest.mark.smoke
-def test_cli_create_config_creates_file(tmp_path:Path) -> None:
-    """Smoke: CLI 'create-config' creates a config.yaml file in the current directory."""
+def test_cli_subcommands_create_config_creates_file(tmp_path:Path) -> None:
+    """
+    Smoke: CLI 'create-config' creates a config.yaml file in the current directory.
+    """
     result = run_cli_subcommand(["create-config"], cwd = str(tmp_path))
-    config_path = tmp_path / "config.yaml"
-    assert result.returncode == 0, f"CLI exited with code {result.returncode}\nstdout: {result.stdout}\nstderr: {result.stderr}"
-    assert config_path.exists(), "config.yaml was not created by create-config command"
+    config_file = tmp_path / "config.yaml"
+    assert result.returncode == 0
+    assert config_file.exists(), "config.yaml was not created by create-config command"
+    out = (result.stdout + "\n" + result.stderr).lower()
+    assert "saving" in out, f"Expected saving message in CLI output.\n{out}"
+    assert "config.yaml" in out, f"Expected config.yaml in CLI output.\n{out}"
+
+
+@pytest.mark.smoke
+def test_cli_subcommands_create_config_fails_if_exists(tmp_path:Path) -> None:
+    """
+    Smoke: CLI 'create-config' does not overwrite config.yaml if it already exists.
+    """
+    config_file = tmp_path / "config.yaml"
+    config_file.write_text("# dummy config\n", encoding = "utf-8")
+    result = run_cli_subcommand(["create-config"], cwd = str(tmp_path))
+    assert result.returncode == 0
+    assert config_file.exists(), "config.yaml was deleted or not present after second create-config run"
+    out = (result.stdout + "\n" + result.stderr).lower()
+    assert (
+        "already exists" in out or "not overwritten" in out or "saving" in out
+    ), f"Expected message about existing config in CLI output.\n{out}"
+
+
+@pytest.mark.smoke
+@pytest.mark.parametrize(("subcommand", "output_check"), [
+    ("verify", "verify"),
+    ("update-check", "update"),
+    ("update-content-hash", "update-content-hash"),
+])
+@pytest.mark.parametrize(("config_ext", "serializer"), [
+    ("yaml", None),
+    ("yml", None),
+    ("json", json.dumps),
+])
+def test_cli_subcommands_with_config_formats(
+    subcommand:str,
+    output_check:str,
+    config_ext:str,
+    serializer:Callable[[dict[str, object]], str] | None,
+    tmp_path:Path,
+    test_bot_config:Config,
+) -> None:
+    """
+    Smoke: CLI subcommands that require a config file, tested with all supported formats.
+    """
+    config_path = tmp_path / f"config.{config_ext}"
+    try:
+        config_dict = test_bot_config.model_dump()
+    except AttributeError:
+        config_dict = test_bot_config.dict()
+    if config_ext in {"yaml", "yml"}:
+        yaml = YAML(typ = "unsafe", pure = True)
+        with open(config_path, "w", encoding = "utf-8") as f:
+            yaml.dump(config_dict, f)
+    elif serializer is not None:
+        config_path.write_text(serializer(config_dict), encoding = "utf-8")
+    args = [subcommand, "--config", str(config_path)]
+    result = run_cli_subcommand(args, cwd = str(tmp_path))
+    assert result.returncode == 0
+    out = (result.stdout + "\n" + result.stderr).lower()
+    if subcommand == "verify":
+        assert "no configuration errors found" in out, f"Expected 'no configuration errors found' in output for 'verify'.\n{out}"
+    elif subcommand == "update-content-hash":
+        assert "no active ads found" in out, f"Expected 'no active ads found' in output for 'update-content-hash'.\n{out}"
+    elif subcommand == "update-check":
+        assert result.returncode == 0
diff --git a/tests/unit/test_config_model.py b/tests/unit/test_config_model.py
index 999134f..aa4789f 100644
--- a/tests/unit/test_config_model.py
+++ b/tests/unit/test_config_model.py
@@ -1,7 +1,7 @@
 # SPDX-FileCopyrightText: © Sebastian Thomschke and contributors
 # SPDX-License-Identifier: AGPL-3.0-or-later
 # SPDX-ArtifactOfProjectHomePage: https://github.com/Second-Hand-Friends/kleinanzeigen-bot/
-from kleinanzeigen_bot.model.config_model import AdDefaults
+from kleinanzeigen_bot.model.config_model import AdDefaults, Config
 
 
 def test_migrate_legacy_description_prefix() -> None:
@@ -60,3 +60,17 @@ def test_migrate_legacy_description_suffix() -> None:
             "suffix": "Legacy Suffix"
         }
     }).description_suffix == "Legacy Suffix"
+
+
+def test_minimal_config_validation() -> None:
+    """
+    Unit: Minimal config validation.
+    """
+    minimal_cfg = {
+        "ad_defaults": {"contact": {"name": "dummy", "zipcode": "12345"}},
+        "login": {"username": "dummy", "password": "dummy"},
+        "publishing": {"delete_old_ads": "BEFORE_PUBLISH", "delete_old_ads_by_title": False},
+    }
+    config = Config.model_validate(minimal_cfg)
+    assert config.login.username == "dummy"
+    assert config.login.password == "dummy"  # noqa: S105
diff --git a/tests/unit/test_init.py b/tests/unit/test_init.py
index 54c3267..3df61bb 100644
--- a/tests/unit/test_init.py
+++ b/tests/unit/test_init.py
@@ -1358,3 +1358,29 @@ class TestKleinanzeigenBotChangedAds:
 
                 # The changed ad should be loaded with 'due' selector because it's due for republication
                 assert len(ads_to_publish) == 1
+
+
+def test_file_logger_writes_message(tmp_path:Path, caplog:pytest.LogCaptureFixture) -> None:
+    """
+    Unit: Logger can be initialized and used, robust to pytest log capture.
+    """
+    log_path = tmp_path / "logger_test.log"
+    logger_name = "logger_test_logger_unique"
+    logger = logging.getLogger(logger_name)
+    logger.setLevel(logging.DEBUG)
+    logger.propagate = False
+    for h in list(logger.handlers):
+        logger.removeHandler(h)
+    handle = logging.FileHandler(str(log_path), encoding = "utf-8")
+    handle.setLevel(logging.DEBUG)
+    formatter = logging.Formatter("%(levelname)s:%(name)s:%(message)s")
+    handle.setFormatter(formatter)
+    logger.addHandler(handle)
+    logger.info("Logger test log message")
+    handle.flush()
+    handle.close()
+    logger.removeHandler(handle)
+    assert log_path.exists()
+    with open(log_path, "r", encoding = "utf-8") as f:
+        contents = f.read()
+    assert "Logger test log message" in contents