From 33d1964f86886f7279cb0a874fcab3b7fda01f71 Mon Sep 17 00:00:00 2001 From: Jens <1742418+1cu@users.noreply.github.com> Date: Wed, 12 Nov 2025 21:29:51 +0100 Subject: [PATCH] feat: speed up and stabilise test suite (#676) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## ℹ️ 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. ## 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 --- .github/workflows/build.yml | 7 +- docs/TESTING.md | 9 +- pdm.lock | 28 +++++- pyproject.toml | 5 ++ tests/conftest.py | 12 ++- .../test_web_scraping_mixin_integration.py | 2 + tests/smoke/test_smoke_health.py | 89 ++++++++++++++++--- tests/unit/test_update_checker.py | 52 +++++++++-- tests/unit/test_web_scraping_mixin.py | 1 + 9 files changed, 181 insertions(+), 24 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 2083a02..8ff4a5a 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -10,9 +10,8 @@ on: # https://docs.github.com/en/actions/reference/workflows-and-actions/events # https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows#schedule - cron: '0 15 1 * *' push: - branches-ignore: # build all branches except: - - 'dependabot/**' # prevent GHA triggered twice (once for commit to the branch and once for opening/syncing the PR) - - 'dependencies/pdm' # prevent GHA triggered twice (once for commit to the branch and once for opening/syncing the PR) + branches: + - main tags-ignore: # don't build tags - '**' paths-ignore: @@ -55,6 +54,8 @@ jobs: build: ########################################################### + if: github.event_name != 'push' || github.ref == 'refs/heads/main' + permissions: packages: write diff --git a/docs/TESTING.md b/docs/TESTING.md index 544b9f5..d9f7630 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -62,6 +62,13 @@ This project uses a layered testing approach, with a focus on reliability and fa - **All tests in order:** - Run with: `pdm run test` (runs unit, then integration, then smoke) +### Parallel Execution and Slow-Test Tracking + +- `pytest-xdist` runs every invocation with `-n auto`, so the suite is split across CPU cores automatically. +- Pytest now reports the slowest 25 tests (`--durations=25 --durations-min=0.5`), making regressions easy to spot in CI logs. +- Long-running scenarios are tagged with `@pytest.mark.slow` (smoke CLI checks and browser integrations). Keep them in CI, but skip locally via `pytest -m "not slow"` when you only need a quick signal. +- Coverage commands (`pdm run test:cov`, etc.) remain compatible—`pytest-cov` merges the per-worker data transparently. + ### CI Test Order - CI runs unit tests first, then integration tests, then smoke tests. @@ -99,4 +106,4 @@ See also: `pyproject.toml` for test script definitions and `.github/workflows/bu ### When to Use Which - **CI:** Composite groups are preferred for CI to catch critical failures early and avoid wasting resources. -- **Local development:** You may prefer a unified run (`pdm run test`) to see all failures at once. Both options can be provided in `pyproject.toml` for flexibility. \ No newline at end of file +- **Local development:** You may prefer a unified run (`pdm run test`) to see all failures at once. Both options can be provided in `pyproject.toml` for flexibility. diff --git a/pdm.lock b/pdm.lock index a0c1b6b..defb438 100644 --- a/pdm.lock +++ b/pdm.lock @@ -5,7 +5,7 @@ groups = ["default", "dev"] strategy = ["inherit_metadata"] lock_version = "4.5.0" -content_hash = "sha256:db686d496c2229b3bd3d33e5cddb92024bb15b0641c8cad58af92e97bab507b8" +content_hash = "sha256:2786f3461151a94a8f76ed4ca2b2f3d8d7242d0004e17f1e1ae326b0d6529acd" [[metadata.targets]] requires_python = ">=3.10,<3.15" @@ -553,6 +553,17 @@ files = [ {file = "exceptiongroup-1.3.0.tar.gz", hash = "sha256:b241f5885f560bc56a59ee63ca4c6a8bfa46ae4ad651af316d4e81817bb9fd88"}, ] +[[package]] +name = "execnet" +version = "2.1.1" +requires_python = ">=3.8" +summary = "execnet: rapid multi-Python deployment" +groups = ["dev"] +files = [ + {file = "execnet-2.1.1-py3-none-any.whl", hash = "sha256:26dee51f1b80cebd6d0ca8e74dd8745419761d3bef34163928cbebbdc4749fdc"}, + {file = "execnet-2.1.1.tar.gz", hash = "sha256:5189b52c6121c24feae288166ab41b32549c7e2348652736540b9e6e7d4e72e3"}, +] + [[package]] name = "filelock" version = "3.20.0" @@ -1360,6 +1371,21 @@ files = [ {file = "pytest_rerunfailures-16.1.tar.gz", hash = "sha256:c38b266db8a808953ebd71ac25c381cb1981a78ff9340a14bcb9f1b9bff1899e"}, ] +[[package]] +name = "pytest-xdist" +version = "3.8.0" +requires_python = ">=3.9" +summary = "pytest xdist plugin for distributed testing, most importantly across multiple CPUs" +groups = ["dev"] +dependencies = [ + "execnet>=2.1", + "pytest>=7.0.0", +] +files = [ + {file = "pytest_xdist-3.8.0-py3-none-any.whl", hash = "sha256:202ca578cfeb7370784a8c33d6d05bc6e13b4f25b5053c30a152269fd10f0b88"}, + {file = "pytest_xdist-3.8.0.tar.gz", hash = "sha256:7e578125ec9bc6050861aa93f2d59f1d8d085595d6551c2c90b6f4fad8d3a9f1"}, +] + [[package]] name = "pywin32-ctypes" version = "0.2.3" diff --git a/pyproject.toml b/pyproject.toml index 617cf68..d82eac6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -51,6 +51,7 @@ dev = [ "pip-audit", "pytest>=8.3.4", "pytest-asyncio>=0.25.3", + "pytest-xdist>=3.6.1", "pytest-rerunfailures", "pytest-cov>=6.0.0", "ruff", @@ -347,8 +348,12 @@ addopts = """ --strict-markers --doctest-modules --cov-report=term-missing + -n auto + --durations=25 + --durations-min=0.5 """ markers = [ + "slow: marks a test as long running", "smoke: marks a test as a high-level smoke test (critical path, no mocks)", "itest: marks a test as an integration test (i.e. a test with external dependencies)", "asyncio: mark test as async" diff --git a/tests/conftest.py b/tests/conftest.py index bc21b3f..3c8eceb 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -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 # ============================================================================ diff --git a/tests/integration/test_web_scraping_mixin_integration.py b/tests/integration/test_web_scraping_mixin_integration.py index 90fe214..9b6f048 100644 --- a/tests/integration/test_web_scraping_mixin_integration.py +++ b/tests/integration/test_web_scraping_mixin_integration.py @@ -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 diff --git a/tests/smoke/test_smoke_health.py b/tests/smoke/test_smoke_health.py index 7c26d59..861633e 100644 --- a/tests/smoke/test_smoke_health.py +++ b/tests/smoke/test_smoke_health.py @@ -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": diff --git a/tests/unit/test_update_checker.py b/tests/unit/test_update_checker.py index 21910f0..32e4747 100644 --- a/tests/unit/test_update_checker.py +++ b/tests/unit/test_update_checker.py @@ -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) diff --git a/tests/unit/test_web_scraping_mixin.py b/tests/unit/test_web_scraping_mixin.py index d205a3d..fad2456 100644 --- a/tests/unit/test_web_scraping_mixin.py +++ b/tests/unit/test_web_scraping_mixin.py @@ -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), \