mirror of
https://github.com/Second-Hand-Friends/kleinanzeigen-bot.git
synced 2026-03-12 02:31:45 +01:00
feat: speed up and stabilise test suite (#676)
## ℹ️ Description *Provide a concise summary of the changes introduced in this pull request.* - Link to the related issue(s): Issue # - Describe the motivation and context for this change. Refactors the test harness for faster and more reliable feedback: adds deterministic time freezing for update checks, accelerates and refactors smoke tests to run in-process, defaults pytest to xdist with durations tracking, and adjusts CI triggers so PRs run the test matrix only once. ## 📋 Changes Summary - add pytest-xdist + durations reporting defaults, force deterministic locale and slow markers, and document the workflow adjustments - run smoke tests in-process (no subprocess churn), mock update checks/logging, and mark slow specs appropriately - deflake update check interval tests by freezing datetime and simplify FixedDateTime helper - limit GitHub Actions `push` trigger to `main` so feature branches rely on the single pull_request run ### ⚙️ Type of Change Select the type(s) of change(s) included in this pull request: - [ ] 🐞 Bug fix (non-breaking change which fixes an issue) - [x] ✨ New feature (adds new functionality without breaking existing usage) - [ ] 💥 Breaking change (changes that might break existing user setups, scripts, or configurations) ## ✅ Checklist Before requesting a review, confirm the following: - [x] I have reviewed my changes to ensure they meet the project's standards. - [x] I have tested my changes and ensured that all tests pass (`pdm run test`). - [x] I have formatted the code (`pdm run format`). - [x] I have verified that linting passes (`pdm run lint`). - [x] I have updated documentation where necessary. By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Ensure tests run in a consistent English locale and restore prior locale after each run * Mark integration scraping tests as slow for clearer categorization * Replace subprocess-based CLI tests with an in-process runner that returns structured results and captures combined stdout/stderr/logs; disable update checks during smoke tests * Freeze current time in update-check tests for deterministic assertions * Add mock for process enumeration in web‑scraping unit tests to stabilize macOS-specific warnings <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -15,6 +15,7 @@ Fixture Organization:
|
||||
- Test data fixtures: Shared test data (description_test_cases)
|
||||
"""
|
||||
import os
|
||||
from collections.abc import Iterator
|
||||
from typing import Any, Final, cast
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
@@ -23,7 +24,7 @@ import pytest
|
||||
from kleinanzeigen_bot import KleinanzeigenBot
|
||||
from kleinanzeigen_bot.model.ad_model import Ad
|
||||
from kleinanzeigen_bot.model.config_model import Config
|
||||
from kleinanzeigen_bot.utils import loggers
|
||||
from kleinanzeigen_bot.utils import i18n, loggers
|
||||
from kleinanzeigen_bot.utils.web_scraping_mixin import Browser
|
||||
|
||||
loggers.configure_console_logging()
|
||||
@@ -198,6 +199,15 @@ def silence_nodriver_logs() -> None:
|
||||
loggers.get_logger("nodriver").setLevel(loggers.WARNING)
|
||||
|
||||
|
||||
@pytest.fixture(autouse = True)
|
||||
def force_english_locale() -> Iterator[None]:
|
||||
"""Ensure tests run with a deterministic English locale."""
|
||||
previous_locale = i18n.get_current_locale()
|
||||
i18n.set_current_locale(i18n.Locale("en", "US", "UTF-8"))
|
||||
yield
|
||||
i18n.set_current_locale(previous_locale)
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# Smoke Test Fixtures - Special fixtures for smoke tests
|
||||
# ============================================================================
|
||||
|
||||
@@ -10,6 +10,8 @@ import pytest
|
||||
from kleinanzeigen_bot.utils.misc import ensure
|
||||
from kleinanzeigen_bot.utils.web_scraping_mixin import WebScrapingMixin
|
||||
|
||||
pytestmark = pytest.mark.slow
|
||||
|
||||
# Configure logging for integration tests
|
||||
# The main bot already handles nodriver logging via silence_nodriver_logs fixture
|
||||
# and pytest handles verbosity with -v flag automatically
|
||||
|
||||
@@ -6,28 +6,91 @@ Minimal smoke tests: post-deployment health checks for kleinanzeigen-bot.
|
||||
These tests verify that the most essential components are operational.
|
||||
"""
|
||||
|
||||
import contextlib
|
||||
import io
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
import re
|
||||
import subprocess # noqa: S404
|
||||
import sys
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
from typing import Callable
|
||||
from typing import Any, Callable
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
from ruyaml import YAML
|
||||
|
||||
import kleinanzeigen_bot
|
||||
from kleinanzeigen_bot.model.config_model import Config
|
||||
from kleinanzeigen_bot.utils.i18n import get_current_locale, set_current_locale
|
||||
from tests.conftest import SmokeKleinanzeigenBot
|
||||
|
||||
pytestmark = pytest.mark.slow
|
||||
|
||||
def run_cli_subcommand(args:list[str], cwd:str | None = None) -> subprocess.CompletedProcess[str]:
|
||||
|
||||
@dataclass(slots = True)
|
||||
class CLIResult:
|
||||
returncode:int
|
||||
stdout:str
|
||||
stderr:str
|
||||
|
||||
|
||||
def invoke_cli(args:list[str], cwd:Path | None = None) -> CLIResult:
|
||||
"""
|
||||
Run the kleinanzeigen-bot CLI as a subprocess with the given arguments.
|
||||
Returns the CompletedProcess object.
|
||||
Run the kleinanzeigen-bot CLI in-process and capture stdout/stderr.
|
||||
"""
|
||||
cli_module = "kleinanzeigen_bot.__main__"
|
||||
cmd = [sys.executable, "-m", cli_module] + args
|
||||
return subprocess.run(cmd, check = False, capture_output = True, text = True, cwd = cwd) # noqa: S603
|
||||
stdout = io.StringIO()
|
||||
stderr = io.StringIO()
|
||||
previous_cwd:Path | None = None
|
||||
previous_locale = get_current_locale()
|
||||
|
||||
def capture_register(func:Callable[..., object], *_cb_args:Any, **_cb_kwargs:Any) -> Callable[..., object]:
|
||||
return func
|
||||
|
||||
log_capture = io.StringIO()
|
||||
log_handler = logging.StreamHandler(log_capture)
|
||||
log_handler.setLevel(logging.DEBUG)
|
||||
|
||||
def build_result(exit_code:object) -> CLIResult:
|
||||
if exit_code is None:
|
||||
normalized = 0
|
||||
elif isinstance(exit_code, int):
|
||||
normalized = exit_code
|
||||
else:
|
||||
normalized = 1
|
||||
combined_stderr = stderr.getvalue() + log_capture.getvalue()
|
||||
return CLIResult(normalized, stdout.getvalue(), combined_stderr)
|
||||
|
||||
try:
|
||||
if cwd is not None:
|
||||
previous_cwd = Path.cwd()
|
||||
os.chdir(os.fspath(cwd))
|
||||
logging.getLogger().addHandler(log_handler)
|
||||
with contextlib.ExitStack() as stack:
|
||||
stack.enter_context(patch("kleinanzeigen_bot.atexit.register", capture_register))
|
||||
stack.enter_context(contextlib.redirect_stdout(stdout))
|
||||
stack.enter_context(contextlib.redirect_stderr(stderr))
|
||||
try:
|
||||
kleinanzeigen_bot.main(["kleinanzeigen-bot", *args])
|
||||
except SystemExit as exc:
|
||||
return build_result(exc.code)
|
||||
return build_result(0)
|
||||
finally:
|
||||
logging.getLogger().removeHandler(log_handler)
|
||||
log_handler.close()
|
||||
if previous_cwd is not None:
|
||||
os.chdir(previous_cwd)
|
||||
set_current_locale(previous_locale)
|
||||
|
||||
|
||||
@pytest.fixture(autouse = True)
|
||||
def disable_update_checker(monkeypatch:pytest.MonkeyPatch) -> None:
|
||||
"""Prevent smoke tests from hitting GitHub for update checks."""
|
||||
|
||||
def _no_update(*_args:object, **_kwargs:object) -> None:
|
||||
return None
|
||||
|
||||
monkeypatch.setattr("kleinanzeigen_bot.update_checker.UpdateChecker.check_for_updates", _no_update)
|
||||
|
||||
|
||||
@pytest.mark.smoke
|
||||
@@ -50,7 +113,7 @@ 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, diagnose).
|
||||
"""
|
||||
args = [subcommand]
|
||||
result = run_cli_subcommand(args, cwd = str(tmp_path))
|
||||
result = invoke_cli(args, cwd = tmp_path)
|
||||
assert result.returncode == 0
|
||||
out = (result.stdout + "\n" + result.stderr).lower()
|
||||
if subcommand in {"--help", "help"}:
|
||||
@@ -66,7 +129,7 @@ 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))
|
||||
result = invoke_cli(["create-config"], cwd = tmp_path)
|
||||
config_file = tmp_path / "config.yaml"
|
||||
assert result.returncode == 0
|
||||
assert config_file.exists(), "config.yaml was not created by create-config command"
|
||||
@@ -82,7 +145,7 @@ def test_cli_subcommands_create_config_fails_if_exists(tmp_path:Path) -> None:
|
||||
"""
|
||||
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))
|
||||
result = invoke_cli(["create-config"], cwd = 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()
|
||||
@@ -126,7 +189,7 @@ def test_cli_subcommands_with_config_formats(
|
||||
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))
|
||||
result = invoke_cli(args, cwd = tmp_path)
|
||||
assert result.returncode == 0
|
||||
out = (result.stdout + "\n" + result.stderr).lower()
|
||||
if subcommand == "verify":
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from datetime import datetime, timedelta, timezone
|
||||
from datetime import datetime, timedelta, timezone, tzinfo
|
||||
from typing import TYPE_CHECKING
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
@@ -18,11 +18,47 @@ import requests
|
||||
if TYPE_CHECKING:
|
||||
from pytest_mock import MockerFixture
|
||||
|
||||
from kleinanzeigen_bot.model import update_check_state as update_check_state_module
|
||||
from kleinanzeigen_bot.model.config_model import Config
|
||||
from kleinanzeigen_bot.model.update_check_state import UpdateCheckState
|
||||
from kleinanzeigen_bot.update_checker import UpdateChecker
|
||||
|
||||
|
||||
def _freeze_update_state_datetime(monkeypatch:pytest.MonkeyPatch, fixed_now:datetime) -> None:
|
||||
"""Patch UpdateCheckState to return a deterministic datetime.now/utcnow."""
|
||||
|
||||
class FixedDateTime(datetime):
|
||||
@classmethod
|
||||
def now(cls, tz:tzinfo | None = None) -> "FixedDateTime":
|
||||
base = fixed_now.replace(tzinfo = None) if tz is None else fixed_now.astimezone(tz)
|
||||
return cls(
|
||||
base.year,
|
||||
base.month,
|
||||
base.day,
|
||||
base.hour,
|
||||
base.minute,
|
||||
base.second,
|
||||
base.microsecond,
|
||||
tzinfo = base.tzinfo
|
||||
)
|
||||
|
||||
@classmethod
|
||||
def utcnow(cls) -> "FixedDateTime":
|
||||
base = fixed_now.astimezone(timezone.utc).replace(tzinfo = None)
|
||||
return cls(
|
||||
base.year,
|
||||
base.month,
|
||||
base.day,
|
||||
base.hour,
|
||||
base.minute,
|
||||
base.second,
|
||||
base.microsecond
|
||||
)
|
||||
|
||||
datetime_module = getattr(update_check_state_module, "datetime")
|
||||
monkeypatch.setattr(datetime_module, "datetime", FixedDateTime)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def config() -> Config:
|
||||
return Config.model_validate({
|
||||
@@ -256,10 +292,12 @@ class TestUpdateChecker:
|
||||
# Should not raise an exception
|
||||
state.save(state_file)
|
||||
|
||||
def test_update_check_state_interval_units(self) -> None:
|
||||
def test_update_check_state_interval_units(self, monkeypatch:pytest.MonkeyPatch) -> None:
|
||||
"""Test that different interval units are handled correctly."""
|
||||
state = UpdateCheckState()
|
||||
now = datetime.now(timezone.utc)
|
||||
fixed_now = datetime(2025, 1, 15, 8, 0, tzinfo = timezone.utc)
|
||||
_freeze_update_state_datetime(monkeypatch, fixed_now)
|
||||
now = fixed_now
|
||||
|
||||
# Test seconds (should always be too short, fallback to 7d, only 2 days elapsed, so should_check is False)
|
||||
state.last_check = now - timedelta(seconds = 30)
|
||||
@@ -303,10 +341,14 @@ class TestUpdateChecker:
|
||||
state.last_check = now - timedelta(days = 6)
|
||||
assert state.should_check("1z") is False
|
||||
|
||||
def test_update_check_state_interval_validation(self) -> None:
|
||||
def test_update_check_state_interval_validation(self, monkeypatch:pytest.MonkeyPatch) -> None:
|
||||
"""Test that interval validation works correctly."""
|
||||
state = UpdateCheckState()
|
||||
now = datetime.now(timezone.utc)
|
||||
fixed_now = datetime(2025, 1, 1, 12, 0, tzinfo = timezone.utc)
|
||||
|
||||
_freeze_update_state_datetime(monkeypatch, fixed_now)
|
||||
|
||||
now = fixed_now
|
||||
state.last_check = now - timedelta(days = 1)
|
||||
|
||||
# Test minimum value (1d)
|
||||
|
||||
@@ -1393,6 +1393,7 @@ class TestWebScrapingDiagnostics:
|
||||
|
||||
with patch("os.path.exists", return_value = True), \
|
||||
patch("os.access", return_value = True), \
|
||||
patch("psutil.process_iter", return_value = []), \
|
||||
patch("kleinanzeigen_bot.utils.web_scraping_mixin.net.is_port_open", return_value = False), \
|
||||
patch("platform.system", return_value = "Darwin"), \
|
||||
patch("kleinanzeigen_bot.utils.web_scraping_mixin._is_admin", return_value = False), \
|
||||
|
||||
Reference in New Issue
Block a user