mirror of
https://github.com/Second-Hand-Friends/kleinanzeigen-bot.git
synced 2026-03-12 02:31:45 +01:00
feat: upgrade nodriver from 0.39 to 0.47 (#635)
## ℹ️ Description Upgrade nodriver dependency from pinned version 0.39.0 to latest 0.47.0 to resolve browser startup issues and JavaScript evaluation problems that affected versions 0.40-0.44. - Link to the related issue(s): Resolves nodriver compatibility issues - This upgrade addresses browser startup problems and window.BelenConf evaluation failures that were blocking the use of newer nodriver versions. ## 📋 Changes Summary - Updated nodriver dependency from pinned 0.39.0 to >=0.47.0 in pyproject.toml - Fixed RemoteObject handling in web_execute method for nodriver 0.47 compatibility - Added comprehensive BelenConf test fixture with real production data structure - Added integration test to validate window.BelenConf evaluation works correctly - Added German translation for new error message - Replaced real user data with privacy-safe dummy data in test fixtures ### 🔧 Type Safety Improvements **Added explicit `str()` conversions to resolve type inference issues:** The comprehensive BelenConf test fixture contains deeply nested data structures that caused pyright's type checker to infer complex dictionary types throughout the codebase. To ensure type safety and prevent runtime errors, I added explicit `str()` conversions in key locations: - **CSRF tokens**: `str(csrf_token)` - Ensures CSRF tokens are treated as strings - **Special attributes**: `str(special_attribute_value)` - Converts special attribute values to strings - **DOM attributes**: `str(special_attr_elem.attrs.id)` - Ensures element IDs are strings - **URL handling**: `str(current_img_url)` and `str(href_attributes)` - Converts URLs and href attributes to strings - **Price values**: `str(ad_cfg.price)` - Ensures price values are strings These conversions are defensive programming measures that ensure backward compatibility and prevent type-related runtime errors, even if the underlying data structures change in the future. ### ⚙️ Type of Change - [x] ✨ New feature (adds new functionality without breaking existing usage) - [ ] 🐞 Bug fix (non-breaking change which fixes an issue) - [ ] 💥 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 commit is contained in:
@@ -587,6 +587,31 @@ class TestAdExtractorCategory:
|
||||
mock_web_find.assert_any_call(By.CSS_SELECTOR, "a:nth-of-type(2)", parent = category_line)
|
||||
mock_web_find.assert_any_call(By.CSS_SELECTOR, "a:nth-of-type(3)", parent = category_line)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
# pylint: disable=protected-access
|
||||
async def test_extract_category_with_non_string_href(self, extractor:AdExtractor) -> None:
|
||||
"""Test category extraction with non-string href attributes to cover str() conversion."""
|
||||
category_line = MagicMock()
|
||||
first_part = MagicMock()
|
||||
# Use non-string href to test str() conversion
|
||||
first_part.attrs = {"href": 12345} # This will need str() conversion
|
||||
second_part = MagicMock()
|
||||
second_part.attrs = {"href": 67890} # This will need str() conversion
|
||||
|
||||
with patch.object(extractor, "web_find", new_callable = AsyncMock) as mock_web_find:
|
||||
mock_web_find.side_effect = [
|
||||
category_line,
|
||||
first_part,
|
||||
second_part
|
||||
]
|
||||
|
||||
result = await extractor._extract_category_from_ad_page()
|
||||
assert result == "2345/7890" # After str() conversion and slicing
|
||||
|
||||
mock_web_find.assert_any_call(By.ID, "vap-brdcrmb")
|
||||
mock_web_find.assert_any_call(By.CSS_SELECTOR, "a:nth-of-type(2)", parent = category_line)
|
||||
mock_web_find.assert_any_call(By.CSS_SELECTOR, "a:nth-of-type(3)", parent = category_line)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
# pylint: disable=protected-access
|
||||
async def test_extract_special_attributes_empty(self, extractor:AdExtractor) -> None:
|
||||
|
||||
@@ -6,7 +6,7 @@ from collections.abc import Generator
|
||||
from contextlib import redirect_stdout
|
||||
from datetime import timedelta
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
from typing import Any, cast
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
@@ -895,7 +895,7 @@ class TestKleinanzeigenBotAdDeletion:
|
||||
|
||||
# Create config with ID for deletion by ID
|
||||
ad_cfg = Ad.model_validate(minimal_ad_config | {
|
||||
id: "12345"
|
||||
"id": "12345" # Fixed: use proper dict key syntax
|
||||
})
|
||||
|
||||
published_ads = [
|
||||
@@ -911,6 +911,38 @@ class TestKleinanzeigenBotAdDeletion:
|
||||
result = await test_bot.delete_ad(ad_cfg, published_ads, delete_old_ads_by_title = False)
|
||||
assert result is True
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delete_ad_by_id_with_non_string_csrf_token(self, test_bot:KleinanzeigenBot, minimal_ad_config:dict[str, Any]) -> None:
|
||||
"""Test deleting an ad by ID with non-string CSRF token to cover str() conversion."""
|
||||
test_bot.page = MagicMock()
|
||||
test_bot.page.evaluate = AsyncMock(return_value = {"statusCode": 200, "content": "{}"})
|
||||
test_bot.page.sleep = AsyncMock()
|
||||
|
||||
# Create config with ID for deletion by ID
|
||||
ad_cfg = Ad.model_validate(minimal_ad_config | {
|
||||
"id": "12345"
|
||||
})
|
||||
|
||||
published_ads = [
|
||||
{"title": "Different Title", "id": "12345"},
|
||||
{"title": "Other Title", "id": "11111"}
|
||||
]
|
||||
|
||||
with patch.object(test_bot, "web_open", new_callable = AsyncMock), \
|
||||
patch.object(test_bot, "web_find", new_callable = AsyncMock) as mock_find, \
|
||||
patch.object(test_bot, "web_click", new_callable = AsyncMock), \
|
||||
patch.object(test_bot, "web_check", new_callable = AsyncMock, return_value = True), \
|
||||
patch.object(test_bot, "web_request", new_callable = AsyncMock) as mock_request:
|
||||
# Mock non-string CSRF token to test str() conversion
|
||||
mock_find.return_value.attrs = {"content": 12345} # Non-string token
|
||||
result = await test_bot.delete_ad(ad_cfg, published_ads, delete_old_ads_by_title = False)
|
||||
assert result is True
|
||||
|
||||
# Verify that str() was called on the CSRF token
|
||||
mock_request.assert_called_once()
|
||||
call_args = mock_request.call_args
|
||||
assert call_args[1]["headers"]["x-csrf-token"] == "12345" # Should be converted to string
|
||||
|
||||
|
||||
class TestKleinanzeigenBotAdRepublication:
|
||||
"""Tests for ad republication functionality."""
|
||||
@@ -1067,6 +1099,79 @@ class TestKleinanzeigenBotShippingOptions:
|
||||
# Verify the file was created in the temporary directory
|
||||
assert ad_file.exists()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_special_attributes_with_non_string_values(self, test_bot:KleinanzeigenBot, base_ad_config:dict[str, Any]) -> None:
|
||||
"""Test that special attributes with non-string values are converted to strings."""
|
||||
# Create ad config with string special attributes first (to pass validation)
|
||||
ad_cfg = Ad.model_validate(base_ad_config | {
|
||||
"special_attributes": {
|
||||
"art_s": "12345", # String value initially
|
||||
"condition_s": "67890", # String value initially
|
||||
"color_s": "red" # String value
|
||||
},
|
||||
"updated_on": "2024-01-01T00:00:00",
|
||||
"created_on": "2024-01-01T00:00:00"
|
||||
})
|
||||
|
||||
# Now modify the special attributes to non-string values to test str() conversion
|
||||
# This simulates the scenario where the values come from external sources as non-strings
|
||||
# We need to cast to Any to bypass type checking for this test
|
||||
special_attrs = cast(Any, ad_cfg.special_attributes)
|
||||
special_attrs["art_s"] = 12345 # Non-string value
|
||||
special_attrs["condition_s"] = 67890 # Non-string value
|
||||
|
||||
# Mock special attribute elements
|
||||
art_s_elem = MagicMock()
|
||||
art_s_attrs = MagicMock()
|
||||
art_s_attrs.id = "art_s"
|
||||
art_s_attrs.name = "art_s"
|
||||
art_s_elem.attrs = art_s_attrs
|
||||
art_s_elem.local_name = "select"
|
||||
|
||||
condition_s_elem = MagicMock()
|
||||
condition_s_attrs = MagicMock()
|
||||
condition_s_attrs.id = "condition_s"
|
||||
condition_s_attrs.name = "condition_s"
|
||||
condition_s_elem.attrs = condition_s_attrs
|
||||
condition_s_elem.local_name = "select"
|
||||
|
||||
color_s_elem = MagicMock()
|
||||
color_s_attrs = MagicMock()
|
||||
color_s_attrs.id = "color_s"
|
||||
color_s_attrs.name = "color_s"
|
||||
color_s_elem.attrs = color_s_attrs
|
||||
color_s_elem.local_name = "select"
|
||||
|
||||
# Mock the necessary web interaction methods
|
||||
with patch.object(test_bot, "web_find", new_callable = AsyncMock) as mock_find, \
|
||||
patch.object(test_bot, "web_select", new_callable = AsyncMock) as mock_select, \
|
||||
patch.object(test_bot, "web_check", new_callable = AsyncMock, return_value = True), \
|
||||
patch.object(test_bot, "_KleinanzeigenBot__set_condition", new_callable = AsyncMock) as mock_set_condition:
|
||||
|
||||
# Mock web_find to simulate element detection
|
||||
async def mock_find_side_effect(selector_type:By, selector_value:str, **_:Any) -> Element | None:
|
||||
# Handle XPath queries for special attributes
|
||||
if selector_type == By.XPATH and "contains(@name" in selector_value:
|
||||
if "art_s" in selector_value:
|
||||
return art_s_elem
|
||||
if "condition_s" in selector_value:
|
||||
return condition_s_elem
|
||||
if "color_s" in selector_value:
|
||||
return color_s_elem
|
||||
return None
|
||||
|
||||
mock_find.side_effect = mock_find_side_effect
|
||||
|
||||
# Test the __set_special_attributes method directly
|
||||
await getattr(test_bot, "_KleinanzeigenBot__set_special_attributes")(ad_cfg)
|
||||
|
||||
# Verify that web_select was called with string values (str() conversion)
|
||||
mock_select.assert_any_call(By.ID, "art_s", "12345") # Converted to string
|
||||
mock_select.assert_any_call(By.ID, "color_s", "red") # Already string
|
||||
|
||||
# Verify that __set_condition was called with string value
|
||||
mock_set_condition.assert_called_once_with("67890") # Converted to string
|
||||
|
||||
|
||||
class TestKleinanzeigenBotUrlConstruction:
|
||||
"""Tests for URL construction functionality."""
|
||||
|
||||
134
tests/unit/test_web_scraping_mixin_remoteobject.py
Normal file
134
tests/unit/test_web_scraping_mixin_remoteobject.py
Normal file
@@ -0,0 +1,134 @@
|
||||
# SPDX-FileCopyrightText: © Jens Bergmann and contributors
|
||||
# SPDX-License-Identifier: AGPL-3.0-or-later
|
||||
# SPDX-ArtifactOfProjectHomePage: https://github.com/Second-Hand-Friends/kleinanzeigen-bot/
|
||||
"""Unit tests for web_scraping_mixin.py RemoteObject handling.
|
||||
|
||||
Copyright (c) 2024, kleinanzeigen-bot contributors.
|
||||
All rights reserved.
|
||||
"""
|
||||
|
||||
from unittest.mock import AsyncMock, Mock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from kleinanzeigen_bot.utils.web_scraping_mixin import WebScrapingMixin
|
||||
|
||||
|
||||
class TestWebExecuteRemoteObjectHandling:
|
||||
"""Test web_execute method with nodriver 0.47+ RemoteObject behavior."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_web_execute_remoteobject_with_deep_serialized_value(self) -> None:
|
||||
"""Test web_execute with RemoteObject that has deep_serialized_value."""
|
||||
mixin = WebScrapingMixin()
|
||||
|
||||
# Mock RemoteObject with deep_serialized_value
|
||||
mock_remote_object = Mock()
|
||||
mock_remote_object.deep_serialized_value = Mock()
|
||||
mock_remote_object.deep_serialized_value.value = [
|
||||
["key1", "value1"],
|
||||
["key2", "value2"]
|
||||
]
|
||||
|
||||
# Mock the page evaluation to return our RemoteObject
|
||||
with patch.object(mixin, "page") as mock_page:
|
||||
mock_page.evaluate = AsyncMock(return_value = mock_remote_object)
|
||||
|
||||
result = await mixin.web_execute("window.test")
|
||||
|
||||
# Should convert the RemoteObject to a dict
|
||||
assert result == {"key1": "value1", "key2": "value2"}
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_web_execute_remoteobject_with_none_deep_serialized_value(self) -> None:
|
||||
"""Test web_execute with RemoteObject that has None deep_serialized_value."""
|
||||
mixin = WebScrapingMixin()
|
||||
|
||||
# Mock RemoteObject with None deep_serialized_value
|
||||
mock_remote_object = Mock()
|
||||
mock_remote_object.deep_serialized_value = None
|
||||
|
||||
with patch.object(mixin, "page") as mock_page:
|
||||
mock_page.evaluate = AsyncMock(return_value = mock_remote_object)
|
||||
|
||||
result = await mixin.web_execute("window.test")
|
||||
|
||||
# Should return the original RemoteObject
|
||||
assert result is mock_remote_object
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_web_execute_remoteobject_with_non_list_serialized_data(self) -> None:
|
||||
"""Test web_execute with RemoteObject that has non-list serialized data."""
|
||||
mixin = WebScrapingMixin()
|
||||
|
||||
# Mock RemoteObject with non-list serialized data
|
||||
mock_remote_object = Mock()
|
||||
mock_remote_object.deep_serialized_value = Mock()
|
||||
mock_remote_object.deep_serialized_value.value = {"direct": "dict"}
|
||||
|
||||
with patch.object(mixin, "page") as mock_page:
|
||||
mock_page.evaluate = AsyncMock(return_value = mock_remote_object)
|
||||
|
||||
result = await mixin.web_execute("window.test")
|
||||
|
||||
# Should return the serialized data directly
|
||||
assert result == {"direct": "dict"}
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_web_execute_remoteobject_with_none_serialized_data(self) -> None:
|
||||
"""Test web_execute with RemoteObject that has None serialized data."""
|
||||
mixin = WebScrapingMixin()
|
||||
|
||||
# Mock RemoteObject with None serialized data
|
||||
mock_remote_object = Mock()
|
||||
mock_remote_object.deep_serialized_value = Mock()
|
||||
mock_remote_object.deep_serialized_value.value = None
|
||||
|
||||
with patch.object(mixin, "page") as mock_page:
|
||||
mock_page.evaluate = AsyncMock(return_value = mock_remote_object)
|
||||
|
||||
result = await mixin.web_execute("window.test")
|
||||
|
||||
# Should return the original RemoteObject
|
||||
assert result is mock_remote_object
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_web_execute_regular_result(self) -> None:
|
||||
"""Test web_execute with regular result (no RemoteObject)."""
|
||||
mixin = WebScrapingMixin()
|
||||
|
||||
# Mock regular result (no deep_serialized_value attribute)
|
||||
mock_result = {"regular": "dict"}
|
||||
|
||||
with patch.object(mixin, "page") as mock_page:
|
||||
mock_page.evaluate = AsyncMock(return_value = mock_result)
|
||||
|
||||
result = await mixin.web_execute("window.test")
|
||||
|
||||
# Should return the result unchanged
|
||||
assert result == mock_result
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_web_execute_remoteobject_conversion_exception(self) -> None:
|
||||
"""Test web_execute with RemoteObject that raises exception during conversion."""
|
||||
mixin = WebScrapingMixin()
|
||||
|
||||
# Mock RemoteObject that will raise an exception when trying to convert to dict
|
||||
mock_remote_object = Mock()
|
||||
mock_deep_serialized = Mock()
|
||||
|
||||
# Create a list-like object that raises an exception when dict() is called on it
|
||||
class ExceptionRaisingList(list[str]):
|
||||
def __iter__(self) -> None: # type: ignore[override]
|
||||
raise ValueError("Simulated conversion error")
|
||||
|
||||
mock_deep_serialized.value = ExceptionRaisingList([["key", "value"]]) # type: ignore[list-item]
|
||||
mock_remote_object.deep_serialized_value = mock_deep_serialized
|
||||
|
||||
with patch.object(mixin, "page") as mock_page:
|
||||
mock_page.evaluate = AsyncMock(return_value = mock_remote_object)
|
||||
|
||||
result = await mixin.web_execute("window.test")
|
||||
|
||||
# Should return the original RemoteObject when conversion fails
|
||||
assert result is mock_remote_object
|
||||
Reference in New Issue
Block a user