From 8ea41d323081ea1aa490c32654b8e1e17ba02dec Mon Sep 17 00:00:00 2001 From: Jens <1742418+1cu@users.noreply.github.com> Date: Sun, 21 Dec 2025 16:45:20 +0100 Subject: [PATCH] fix: compare updates via release tag (#745) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## ℹ️ Description - Link to the related issue(s): Issue #N/A - Describe the motivation and context for this change. Ensure update-check compares against release tags instead of moving branch tips and keep tests/translations in sync. ## 📋 Changes Summary - compare release commit via tag name first and fall back only when missing - update update-checker tests for commit-ish resolution and tag-based release data - refresh German translations for update-checker log strings ### ⚙️ Type of Change Select the type(s) of change(s) included in this pull request: - [x] 🐞 Bug fix (non-breaking change which fixes an issue) ## ✅ 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 * **Bug Fixes** * More reliable update checks by resolving commits from tags, branches or hashes and robustly comparing short vs full hashes. * Improved prerelease handling to avoid inappropriate preview updates and better handling of missing release data. * **Localization & UX** * Error and prerelease messages now use localized strings; commit dates shown consistently in UTC and short-hash form. * **Tests** * Updated tests to cover the new resolution flow, error cases, and logging behavior. ✏️ Tip: You can customize this high-level summary in your review settings. --- .../resources/translations.de.yaml | 8 +- src/kleinanzeigen_bot/update_checker.py | 126 ++++---- tests/unit/test_update_checker.py | 290 +++++++++++++----- 3 files changed, 274 insertions(+), 150 deletions(-) diff --git a/src/kleinanzeigen_bot/resources/translations.de.yaml b/src/kleinanzeigen_bot/resources/translations.de.yaml index 31d4d8b..7f86861 100644 --- a/src/kleinanzeigen_bot/resources/translations.de.yaml +++ b/src/kleinanzeigen_bot/resources/translations.de.yaml @@ -520,10 +520,8 @@ kleinanzeigen_bot/utils/web_scraping_mixin.py: ################################################# kleinanzeigen_bot/update_checker.py: ################################################# - _get_commit_date: - "Could not get commit date: %s": "Konnte Commit-Datum nicht ermitteln: %s" - _get_release_commit: - "Could not get release commit: %s": "Konnte Release-Commit nicht ermitteln: %s" + _resolve_commitish: + "Could not resolve commit '%s': %s": "Konnte Commit '%s' nicht aufloesen: %s" check_for_updates: "A new version is available: %s from %s UTC (current: %s from %s UTC, channel: %s)": "Eine neue Version ist verfügbar: %s vom %s UTC (aktuell: %s vom %s UTC, Kanal: %s)" "Could not determine commit dates for comparison.": "Konnte Commit-Daten für den Vergleich nicht ermitteln." @@ -531,8 +529,6 @@ kleinanzeigen_bot/update_checker.py: "Could not determine local version.": "Konnte lokale Version nicht ermitteln." "Could not determine release commit hash.": "Konnte Release-Commit-Hash nicht ermitteln." "Could not get releases: %s": "Konnte Releases nicht abrufen: %s" - "Failed to get commit dates: %s": "Fehler beim Abrufen der Commit-Daten: %s" - "Failed to get release commit: %s": "Fehler beim Abrufen des Release-Commits: %s" ? "Release notes:\n%s" : "Release-Notizen:\n%s" "You are on the latest version: %s (compared to %s in channel %s)": "Sie verwenden die neueste Version: %s (verglichen mit %s im Kanal %s)" diff --git a/src/kleinanzeigen_bot/update_checker.py b/src/kleinanzeigen_bot/update_checker.py index 35ccbb3..e7b7d8b 100644 --- a/src/kleinanzeigen_bot/update_checker.py +++ b/src/kleinanzeigen_bot/update_checker.py @@ -6,6 +6,7 @@ from __future__ import annotations import logging from datetime import datetime +from gettext import gettext as _ from pathlib import Path from typing import TYPE_CHECKING @@ -66,51 +67,33 @@ class UpdateChecker: return version.split("+")[1] return None - def _get_release_commit(self, tag_name:str) -> str | None: - """Get the commit hash for a release tag. + def _resolve_commitish(self, commitish:str) -> tuple[str | None, datetime | None]: + """Resolve a commit-ish to a full commit hash and date. Args: - tag_name: The release tag name (e.g. 'latest'). + commitish: The commit hash, tag, or branch. Returns: - The commit hash, or None if it cannot be determined. + Tuple of (full commit hash, commit date), or (None, None) if it cannot be determined. """ try: response = requests.get( - f"https://api.github.com/repos/Second-Hand-Friends/kleinanzeigen-bot/releases/tags/{tag_name}", + f"https://api.github.com/repos/Second-Hand-Friends/kleinanzeigen-bot/commits/{commitish}", timeout = self._request_timeout() ) response.raise_for_status() data = response.json() - if isinstance(data, dict) and "target_commitish" in data: - return str(data["target_commitish"]) - return None + if not isinstance(data, dict): + return None, None + commit_date = None + if "commit" in data and "author" in data["commit"] and "date" in data["commit"]["author"]: + commit_date = datetime.fromisoformat(data["commit"]["author"]["date"].replace("Z", "+00:00")) + sha = data.get("sha") + commit_hash = str(sha) if sha else None + return commit_hash, commit_date except Exception as e: - logger.warning("Could not get release commit: %s", e) - return None - - def _get_commit_date(self, commit:str) -> datetime | None: - """Get the commit date for a commit hash. - - Args: - commit: The commit hash. - - Returns: - The commit date, or None if it cannot be determined. - """ - try: - response = requests.get( - f"https://api.github.com/repos/Second-Hand-Friends/kleinanzeigen-bot/commits/{commit}", - timeout = self._request_timeout() - ) - response.raise_for_status() - data = response.json() - if isinstance(data, dict) and "commit" in data and "author" in data["commit"] and "date" in data["commit"]["author"]: - return datetime.fromisoformat(data["commit"]["author"]["date"].replace("Z", "+00:00")) - return None - except Exception as e: - logger.warning("Could not get commit date: %s", e) - return None + logger.warning(_("Could not resolve commit '%s': %s"), commitish, e) + return None, None def _get_short_commit_hash(self, commit:str) -> str: """Get the short version of a commit hash. @@ -123,6 +106,19 @@ class UpdateChecker: """ return commit[:7] + def _commits_match(self, local_commit:str, release_commit:str) -> bool: + """Determine whether two commits refer to the same hash. + + This accounts for short vs. full hashes (e.g. 7 chars vs. 40 chars). + """ + local_commit = local_commit.strip() + release_commit = release_commit.strip() + if local_commit == release_commit: + return True + if len(local_commit) < len(release_commit) and release_commit.startswith(local_commit): + return True + return len(release_commit) < len(local_commit) and local_commit.startswith(release_commit) + def check_for_updates(self, *, skip_interval_check:bool = False) -> None: """Check for updates to the bot. @@ -138,12 +134,12 @@ class UpdateChecker: local_version = self.get_local_version() if not local_version: - logger.warning("Could not determine local version.") + logger.warning(_("Could not determine local version.")) return - local_commit = self._get_commit_hash(local_version) - if not local_commit: - logger.warning("Could not determine local commit hash.") + local_commitish = self._get_commit_hash(local_version) + if not local_commitish: + logger.warning(_("Could not determine local commit hash.")) return # --- Fetch release info from GitHub using correct endpoint per channel --- @@ -158,7 +154,9 @@ class UpdateChecker: release = response.json() # Defensive: ensure it's not a prerelease if release.get("prerelease", False): - logger.warning("Latest release from GitHub is a prerelease, but 'latest' channel expects a stable release.") + logger.warning( + _("Latest release from GitHub is a prerelease, but 'latest' channel expects a stable release.") + ) return elif self.config.update_check.channel == "preview": # Use /releases endpoint and select the most recent prerelease @@ -169,51 +167,47 @@ class UpdateChecker: response.raise_for_status() releases = response.json() # Find the most recent prerelease - release = next((r for r in releases if r.get("prerelease", False)), None) + release = next((r for r in releases if r.get("prerelease", False) and not r.get("draft", False)), None) if not release: - logger.warning("No prerelease found for 'preview' channel.") + logger.warning(_("No prerelease found for 'preview' channel.")) return else: - logger.warning("Unknown update channel: %s", self.config.update_check.channel) + logger.warning(_("Unknown update channel: %s"), self.config.update_check.channel) return except Exception as e: - logger.warning("Could not get releases: %s", e) + logger.warning(_("Could not get releases: %s"), e) return - # Get release commit - try: - release_commit = self._get_release_commit(release["tag_name"]) - except Exception as e: - logger.warning("Failed to get release commit: %s", e) - return - if not release_commit: - logger.warning("Could not determine release commit hash.") + # Get release commit-ish (use tag name to avoid branch tip drift) + release_commitish = release.get("tag_name") + if not release_commitish: + release_commitish = release.get("target_commitish") + if not release_commitish: + logger.warning(_("Could not determine release commit hash.")) return - # Get commit dates - try: - local_commit_date = self._get_commit_date(local_commit) - release_commit_date = self._get_commit_date(release_commit) - except Exception as e: - logger.warning("Failed to get commit dates: %s", e) - return - if not local_commit_date or not release_commit_date: - logger.warning("Could not determine commit dates for comparison.") + # Resolve commit hashes and dates for comparison + local_commit, local_commit_date = self._resolve_commitish(local_commitish) + release_commit, release_commit_date = self._resolve_commitish(str(release_commitish)) + if not local_commit or not release_commit or not local_commit_date or not release_commit_date: + logger.warning(_("Could not determine commit dates for comparison.")) return - if local_commit == release_commit: + if self._commits_match(local_commit, release_commit): # If the commit hashes are identical, we are on the latest version. Do not proceed to other checks. logger.info( - "You are on the latest version: %s (compared to %s in channel %s)", + _("You are on the latest version: %s (compared to %s in channel %s)"), local_version, self._get_short_commit_hash(release_commit), self.config.update_check.channel ) + self.state.update_last_check() + self.state.save(self.state_file) return # All commit dates are in UTC; append ' UTC' to timestamps in logs for clarity. if local_commit_date < release_commit_date: logger.warning( - "A new version is available: %s from %s UTC (current: %s from %s UTC, channel: %s)", + _("A new version is available: %s from %s UTC (current: %s from %s UTC, channel: %s)"), self._get_short_commit_hash(release_commit), release_commit_date.strftime("%Y-%m-%d %H:%M:%S"), local_version, @@ -221,11 +215,13 @@ class UpdateChecker: self.config.update_check.channel ) if release.get("body"): - logger.info("Release notes:\n%s", release["body"]) + logger.info(_("Release notes:\n%s"), release["body"]) else: logger.info( - "You are on a different commit than the release for channel '%s' (tag: %s). This may mean you are ahead, behind, or on a different branch. " - "Local commit: %s (%s UTC), Release commit: %s (%s UTC)", + _( + "You are on a different commit than the release for channel '%s' (tag: %s). This may mean you are ahead, behind, or on a different branch. " + "Local commit: %s (%s UTC), Release commit: %s (%s UTC)" + ), self.config.update_check.channel, release.get("tag_name", "unknown"), self._get_short_commit_hash(local_commit), diff --git a/tests/unit/test_update_checker.py b/tests/unit/test_update_checker.py index e9fa9d1..7d9817e 100644 --- a/tests/unit/test_update_checker.py +++ b/tests/unit/test_update_checker.py @@ -1,4 +1,4 @@ -# SPDX-FileCopyrightText: © jens Bergmann and contributors +# SPDX-FileCopyrightText: © Jens Bergmann and contributors # SPDX-License-Identifier: AGPL-3.0-or-later # SPDX-ArtifactOfProjectHomePage: https://github.com/Second-Hand-Friends/kleinanzeigen-bot/ @@ -7,7 +7,7 @@ from __future__ import annotations import json import logging from datetime import datetime, timedelta, timezone, tzinfo -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any, cast from unittest.mock import MagicMock, patch if TYPE_CHECKING: @@ -90,29 +90,56 @@ class TestUpdateChecker: assert checker._get_commit_hash("2025+fb00f11") == "fb00f11" assert checker._get_commit_hash("2025") is None - def test_get_release_commit(self, config:Config) -> None: - """Test that the release commit hash is correctly retrieved from the GitHub API.""" + def test_resolve_commitish(self, config:Config) -> None: + """Test that a commit-ish is resolved to a full hash and date.""" checker = UpdateChecker(config) - with patch("requests.get", return_value = MagicMock(json = lambda: {"target_commitish": "e7a3d46"})): - assert checker._get_release_commit("latest") == "e7a3d46" + with patch( + "requests.get", + return_value = MagicMock(json = lambda: {"sha": "e7a3d46", "commit": {"author": {"date": "2025-05-18T00:00:00Z"}}}) + ): + commit_hash, commit_date = checker._resolve_commitish("latest") + assert commit_hash == "e7a3d46" + assert commit_date == datetime(2025, 5, 18, tzinfo = timezone.utc) def test_request_timeout_uses_config(self, config:Config, mocker:"MockerFixture") -> None: """Ensure HTTP calls honor the timeout configuration.""" config.timeouts.multiplier = 1.5 checker = UpdateChecker(config) - mock_response = MagicMock(json = lambda: {"target_commitish": "abc"}) + mock_response = MagicMock(json = lambda: {"sha": "abc", "commit": {"author": {"date": "2025-05-18T00:00:00Z"}}}) mock_get = mocker.patch("requests.get", return_value = mock_response) - checker._get_release_commit("latest") + checker._resolve_commitish("latest") expected_timeout = config.timeouts.effective("update_check") assert mock_get.call_args.kwargs["timeout"] == expected_timeout - def test_get_commit_date(self, config:Config) -> None: - """Test that the commit date is correctly retrieved from the GitHub API.""" + def test_resolve_commitish_no_commit(self, config:Config, mocker:"MockerFixture") -> None: + """Test resolving a commit-ish when the API returns no commit data.""" checker = UpdateChecker(config) - with patch("requests.get", return_value = MagicMock(json = lambda: {"commit": {"author": {"date": "2025-05-18T00:00:00Z"}}})): - assert checker._get_commit_date("e7a3d46") == datetime(2025, 5, 18, tzinfo = timezone.utc) + mocker.patch("requests.get", return_value = mocker.Mock(json = lambda: {"sha": "abc"})) + commit_hash, commit_date = checker._resolve_commitish("sha") + assert commit_hash == "abc" + assert commit_date is None + + def test_resolve_commitish_logs_warning_on_exception( + self, + config:Config, + caplog:pytest.LogCaptureFixture + ) -> None: + """Test resolving a commit-ish logs a warning when the request fails.""" + caplog.set_level("WARNING", logger = "kleinanzeigen_bot.update_checker") + checker = UpdateChecker(config) + with patch("requests.get", side_effect = Exception("boom")): + commit_hash, commit_date = checker._resolve_commitish("sha") + + assert commit_hash is None + assert commit_date is None + assert any("Could not resolve commit 'sha': boom" in r.getMessage() for r in caplog.records) + + def test_commits_match_short_hash(self, config:Config) -> None: + """Test that short commit hashes are treated as matching prefixes.""" + checker = UpdateChecker(config) + assert checker._commits_match("abc1234", "abc1234def5678") is True def test_check_for_updates_disabled(self, config:Config) -> None: """Test that the update checker does not check for updates if disabled.""" @@ -125,9 +152,24 @@ class TestUpdateChecker: def test_check_for_updates_no_local_version(self, config:Config) -> None: """Test that the update checker handles the case where the local version cannot be determined.""" checker = UpdateChecker(config) - with patch.object(UpdateChecker, "get_local_version", return_value = None): + with patch.object(UpdateCheckState, "should_check", return_value = True), \ + patch.object(UpdateChecker, "get_local_version", return_value = None): checker.check_for_updates() # Should not raise exception + def test_check_for_updates_logs_missing_local_version( + self, + config:Config, + caplog:pytest.LogCaptureFixture + ) -> None: + """Test that the update checker logs a warning when the local version is missing.""" + caplog.set_level("WARNING", logger = "kleinanzeigen_bot.update_checker") + checker = UpdateChecker(config) + with patch.object(UpdateCheckState, "should_check", return_value = True), \ + patch.object(UpdateChecker, "get_local_version", return_value = None): + checker.check_for_updates() + + assert any("Could not determine local version." in r.getMessage() for r in caplog.records) + def test_check_for_updates_no_commit_hash(self, config:Config) -> None: """Test that the update checker handles the case where the commit hash cannot be extracted.""" checker = UpdateChecker(config) @@ -146,24 +188,48 @@ class TestUpdateChecker: with patch("requests.get", side_effect = Exception("API Error")): checker.check_for_updates() # Should not raise exception + def test_check_for_updates_latest_prerelease_warning( + self, + config:Config, + mocker:"MockerFixture", + caplog:pytest.LogCaptureFixture + ) -> None: + """Test that the update checker warns when latest points to a prerelease.""" + caplog.set_level("WARNING", logger = "kleinanzeigen_bot.update_checker") + mocker.patch.object(UpdateCheckState, "should_check", return_value = True) + mocker.patch.object(UpdateChecker, "get_local_version", return_value = "2025+fb00f11") + mocker.patch.object(UpdateChecker, "_get_commit_hash", return_value = "fb00f11") + mocker.patch.object( + requests, + "get", + return_value = mocker.Mock(json = lambda: {"tag_name": "latest", "prerelease": True}) + ) + + checker = UpdateChecker(config) + checker.check_for_updates() + + expected = "Latest release from GitHub is a prerelease, but 'latest' channel expects a stable release." + assert any(expected in r.getMessage() for r in caplog.records) + def test_check_for_updates_ahead(self, config:Config, mocker:"MockerFixture", caplog:pytest.LogCaptureFixture) -> None: """Test that the update checker correctly identifies when the local version is ahead of the latest release.""" caplog.set_level("INFO", logger = "kleinanzeigen_bot.update_checker") mocker.patch.object(UpdateChecker, "get_local_version", return_value = "2025+fb00f11") mocker.patch.object(UpdateChecker, "_get_commit_hash", return_value = "fb00f11") - mocker.patch.object(UpdateChecker, "_get_release_commit", return_value = "e7a3d46") mocker.patch.object( UpdateChecker, - "_get_commit_date", + "_resolve_commitish", side_effect = [ - datetime(2025, 5, 18, tzinfo = timezone.utc), - datetime(2025, 5, 16, tzinfo = timezone.utc) + ("fb00f11", datetime(2025, 5, 18, tzinfo = timezone.utc)), + ("e7a3d46", datetime(2025, 5, 16, tzinfo = timezone.utc)) ] ) mocker.patch.object( requests, "get", - return_value = mocker.Mock(json = lambda: {"tag_name": "latest", "prerelease": False}) + return_value = mocker.Mock( + json = lambda: {"tag_name": "latest", "prerelease": False} + ) ) mocker.patch.object(UpdateCheckState, "should_check", return_value = True) @@ -186,19 +252,20 @@ class TestUpdateChecker: config.update_check.channel = "preview" mocker.patch.object(UpdateChecker, "get_local_version", return_value = "2025+fb00f11") mocker.patch.object(UpdateChecker, "_get_commit_hash", return_value = "fb00f11") - mocker.patch.object(UpdateChecker, "_get_release_commit", return_value = "e7a3d46") mocker.patch.object( UpdateChecker, - "_get_commit_date", + "_resolve_commitish", side_effect = [ - datetime(2025, 5, 18, tzinfo = timezone.utc), - datetime(2025, 5, 16, tzinfo = timezone.utc) + ("fb00f11", datetime(2025, 5, 18, tzinfo = timezone.utc)), + ("e7a3d46", datetime(2025, 5, 16, tzinfo = timezone.utc)) ] ) mocker.patch.object( requests, "get", - return_value = mocker.Mock(json = lambda: [{"tag_name": "preview", "prerelease": True}]) + return_value = mocker.Mock( + json = lambda: [{"tag_name": "preview", "prerelease": True, "draft": False}] + ) ) mocker.patch.object(UpdateCheckState, "should_check", return_value = True) @@ -216,24 +283,48 @@ class TestUpdateChecker: ) assert any(expected in r.getMessage() for r in caplog.records) + def test_check_for_updates_preview_missing_prerelease( + self, + config:Config, + mocker:"MockerFixture", + caplog:pytest.LogCaptureFixture + ) -> None: + """Test that the update checker warns when no preview prerelease is available.""" + caplog.set_level("WARNING", logger = "kleinanzeigen_bot.update_checker") + config.update_check.channel = "preview" + mocker.patch.object(UpdateCheckState, "should_check", return_value = True) + mocker.patch.object(UpdateChecker, "get_local_version", return_value = "2025+fb00f11") + mocker.patch.object(UpdateChecker, "_get_commit_hash", return_value = "fb00f11") + mocker.patch.object( + requests, + "get", + return_value = mocker.Mock(json = lambda: [{"tag_name": "v1", "prerelease": False, "draft": False}]) + ) + + checker = UpdateChecker(config) + checker.check_for_updates() + + assert any("No prerelease found for 'preview' channel." in r.getMessage() for r in caplog.records) + def test_check_for_updates_behind(self, config:Config, mocker:"MockerFixture", caplog:pytest.LogCaptureFixture) -> None: """Test that the update checker correctly identifies when the local version is behind the latest release.""" caplog.set_level("INFO", logger = "kleinanzeigen_bot.update_checker") mocker.patch.object(UpdateChecker, "get_local_version", return_value = "2025+fb00f11") mocker.patch.object(UpdateChecker, "_get_commit_hash", return_value = "fb00f11") - mocker.patch.object(UpdateChecker, "_get_release_commit", return_value = "e7a3d46") mocker.patch.object( UpdateChecker, - "_get_commit_date", + "_resolve_commitish", side_effect = [ - datetime(2025, 5, 16, tzinfo = timezone.utc), - datetime(2025, 5, 18, tzinfo = timezone.utc) + ("fb00f11", datetime(2025, 5, 16, tzinfo = timezone.utc)), + ("e7a3d46", datetime(2025, 5, 18, tzinfo = timezone.utc)) ] ) mocker.patch.object( requests, "get", - return_value = mocker.Mock(json = lambda: {"tag_name": "latest", "prerelease": False}) + return_value = mocker.Mock( + json = lambda: {"tag_name": "latest", "prerelease": False} + ) ) mocker.patch.object(UpdateCheckState, "should_check", return_value = True) @@ -247,21 +338,57 @@ class TestUpdateChecker: expected = "A new version is available: e7a3d46 from 2025-05-18 00:00:00 UTC (current: 2025+fb00f11 from 2025-05-16 00:00:00 UTC, channel: latest)" assert any(expected in r.getMessage() for r in caplog.records) + def test_check_for_updates_logs_release_notes( + self, + config:Config, + mocker:"MockerFixture", + caplog:pytest.LogCaptureFixture + ) -> None: + """Test that release notes are logged when present.""" + caplog.set_level("INFO", logger = "kleinanzeigen_bot.update_checker") + mocker.patch.object(UpdateChecker, "get_local_version", return_value = "2025+fb00f11") + mocker.patch.object(UpdateChecker, "_get_commit_hash", return_value = "fb00f11") + mocker.patch.object( + UpdateChecker, + "_resolve_commitish", + side_effect = [ + ("fb00f11", datetime(2025, 5, 16, tzinfo = timezone.utc)), + ("e7a3d46", datetime(2025, 5, 18, tzinfo = timezone.utc)) + ] + ) + mocker.patch.object(UpdateCheckState, "should_check", return_value = True) + mocker.patch.object( + requests, + "get", + return_value = mocker.Mock( + json = lambda: {"tag_name": "latest", "prerelease": False, "body": "Release notes here"} + ) + ) + + checker = UpdateChecker(config) + checker.check_for_updates() + + assert any("Release notes:\nRelease notes here" in r.getMessage() for r in caplog.records) + def test_check_for_updates_same(self, config:Config, mocker:"MockerFixture", caplog:pytest.LogCaptureFixture) -> None: """Test that the update checker correctly identifies when the local version is the same as the latest release.""" caplog.set_level("INFO", logger = "kleinanzeigen_bot.update_checker") mocker.patch.object(UpdateChecker, "get_local_version", return_value = "2025+fb00f11") mocker.patch.object(UpdateChecker, "_get_commit_hash", return_value = "fb00f11") - mocker.patch.object(UpdateChecker, "_get_release_commit", return_value = "fb00f11") mocker.patch.object( UpdateChecker, - "_get_commit_date", - return_value = datetime(2025, 5, 18, tzinfo = timezone.utc) + "_resolve_commitish", + side_effect = [ + ("fb00f11", datetime(2025, 5, 18, tzinfo = timezone.utc)), + ("fb00f11", datetime(2025, 5, 18, tzinfo = timezone.utc)) + ] ) mocker.patch.object( requests, "get", - return_value = mocker.Mock(json = lambda: {"tag_name": "latest", "prerelease": False}) + return_value = mocker.Mock( + json = lambda: {"tag_name": "latest", "prerelease": False} + ) ) mocker.patch.object(UpdateCheckState, "should_check", return_value = True) @@ -275,6 +402,26 @@ class TestUpdateChecker: expected = "You are on the latest version: 2025+fb00f11 (compared to fb00f11 in channel latest)" assert any(expected in r.getMessage() for r in caplog.records) + def test_check_for_updates_unknown_channel( + self, + config:Config, + mocker:"MockerFixture", + caplog:pytest.LogCaptureFixture + ) -> None: + """Test that the update checker warns on unknown update channels.""" + caplog.set_level("WARNING", logger = "kleinanzeigen_bot.update_checker") + cast(Any, config.update_check).channel = "unknown" + mocker.patch.object(UpdateCheckState, "should_check", return_value = True) + mocker.patch.object(UpdateChecker, "get_local_version", return_value = "2025+fb00f11") + mocker.patch.object(UpdateChecker, "_get_commit_hash", return_value = "fb00f11") + mock_get = mocker.patch("requests.get") + + checker = UpdateChecker(config) + checker.check_for_updates() + + mock_get.assert_not_called() + assert any("Unknown update channel: unknown" in r.getMessage() for r in caplog.records) + def test_check_for_updates_respects_interval_gate( self, config:Config, @@ -457,59 +604,40 @@ class TestUpdateChecker: # Should not raise state.save(state_file) - def test_get_release_commit_no_sha(self, config:Config, mocker:"MockerFixture") -> None: - """Test _get_release_commit with API returning no sha key.""" + def test_resolve_commitish_no_author(self, config:Config, mocker:"MockerFixture") -> None: + """Test resolving a commit-ish when the API returns no author key.""" checker = UpdateChecker(config) - mocker.patch("requests.get", return_value = mocker.Mock(json = dict)) - assert checker._get_release_commit("latest") is None + mocker.patch("requests.get", return_value = mocker.Mock(json = lambda: {"sha": "abc", "commit": {}})) + commit_hash, commit_date = checker._resolve_commitish("sha") + assert commit_hash == "abc" + assert commit_date is None - def test_get_release_commit_list_instead_of_dict(self, config:Config, mocker:"MockerFixture") -> None: - """Test _get_release_commit with API returning a list instead of dict.""" + def test_resolve_commitish_no_date(self, config:Config, mocker:"MockerFixture") -> None: + """Test resolving a commit-ish when the API returns no date key.""" + checker = UpdateChecker(config) + mocker.patch("requests.get", return_value = mocker.Mock(json = lambda: {"sha": "abc", "commit": {"author": {}}})) + commit_hash, commit_date = checker._resolve_commitish("sha") + assert commit_hash == "abc" + assert commit_date is None + + def test_resolve_commitish_list_instead_of_dict(self, config:Config, mocker:"MockerFixture") -> None: + """Test resolving a commit-ish when the API returns a list instead of dict.""" checker = UpdateChecker(config) mocker.patch("requests.get", return_value = mocker.Mock(json = list)) - assert checker._get_release_commit("latest") is None + commit_hash, commit_date = checker._resolve_commitish("sha") + assert commit_hash is None + assert commit_date is None - def test_get_commit_date_no_commit(self, config:Config, mocker:"MockerFixture") -> None: - """Test _get_commit_date with API returning no commit key.""" - checker = UpdateChecker(config) - mocker.patch("requests.get", return_value = mocker.Mock(json = dict)) - assert checker._get_commit_date("sha") is None - - def test_get_commit_date_no_author(self, config:Config, mocker:"MockerFixture") -> None: - """Test _get_commit_date with API returning no author key.""" - checker = UpdateChecker(config) - mocker.patch("requests.get", return_value = mocker.Mock(json = lambda: {"commit": {}})) - assert checker._get_commit_date("sha") is None - - def test_get_commit_date_no_date(self, config:Config, mocker:"MockerFixture") -> None: - """Test _get_commit_date with API returning no date key.""" - checker = UpdateChecker(config) - mocker.patch("requests.get", return_value = mocker.Mock(json = lambda: {"commit": {"author": {}}})) - assert checker._get_commit_date("sha") is None - - def test_get_commit_date_list_instead_of_dict(self, config:Config, mocker:"MockerFixture") -> None: - """Test _get_commit_date with API returning a list instead of dict.""" - checker = UpdateChecker(config) - mocker.patch("requests.get", return_value = mocker.Mock(json = list)) - assert checker._get_commit_date("sha") is None - - def test_check_for_updates_release_commit_exception(self, config:Config, mocker:"MockerFixture") -> None: - """Test check_for_updates handles exception in _get_release_commit.""" + def test_check_for_updates_missing_release_commitish(self, config:Config, mocker:"MockerFixture") -> None: + """Test check_for_updates handles missing release commit-ish.""" checker = UpdateChecker(config) mocker.patch.object(UpdateChecker, "get_local_version", return_value = "2025+fb00f11") mocker.patch.object(UpdateChecker, "_get_commit_hash", return_value = "fb00f11") - mocker.patch.object(UpdateChecker, "_get_release_commit", side_effect = Exception("fail")) - mocker.patch.object(UpdateCheckState, "should_check", return_value = True) - checker.check_for_updates() # Should not raise - - def test_check_for_updates_commit_date_exception(self, config:Config, mocker:"MockerFixture") -> None: - """Test check_for_updates handles exception in _get_commit_date.""" - checker = UpdateChecker(config) - mocker.patch.object(UpdateChecker, "get_local_version", return_value = "2025+fb00f11") - mocker.patch.object(UpdateChecker, "_get_commit_hash", return_value = "fb00f11") - mocker.patch.object(UpdateChecker, "_get_release_commit", return_value = "e7a3d46") - mocker.patch.object(UpdateChecker, "_get_commit_date", side_effect = Exception("fail")) mocker.patch.object(UpdateCheckState, "should_check", return_value = True) + mocker.patch( + "requests.get", + return_value = mocker.Mock(json = lambda: {"prerelease": False}) + ) checker.check_for_updates() # Should not raise def test_check_for_updates_no_releases_empty(self, config:Config, mocker:"MockerFixture") -> None: @@ -531,11 +659,15 @@ class TestUpdateChecker: caplog.set_level("WARNING", logger = "kleinanzeigen_bot.update_checker") mocker.patch.object(UpdateChecker, "get_local_version", return_value = "2025+fb00f11") mocker.patch.object(UpdateChecker, "_get_commit_hash", return_value = "fb00f11") - mocker.patch.object(UpdateChecker, "_get_release_commit", return_value = "e7a3d46") - mocker.patch.object(UpdateChecker, "_get_commit_date", return_value = None) + mocker.patch.object(UpdateChecker, "_resolve_commitish", return_value = (None, None)) mocker.patch.object(UpdateCheckState, "should_check", return_value = True) # Patch requests.get to avoid any real HTTP requests - mocker.patch("requests.get", return_value = mocker.Mock(json = lambda: {"tag_name": "latest", "prerelease": False})) + mocker.patch( + "requests.get", + return_value = mocker.Mock( + json = lambda: {"tag_name": "latest", "prerelease": False} + ) + ) checker = UpdateChecker(config) checker.check_for_updates() assert any("Could not determine commit dates for comparison." in r.getMessage() for r in caplog.records)