Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ensure mypy validates tests #78

Merged
merged 5 commits into from
Aug 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions tests/po_lib/__init__.py
Original file line number Diff line number Diff line change
@@ -1,27 +1,27 @@
"""
This package is just for overrides testing purposes.
"""
from typing import Any, Callable, Dict
from typing import Any, Dict, Type

from url_matcher import Patterns

from web_poet import handle_urls
from web_poet import ItemPage, handle_urls

# NOTE: this module contains a PO with @handle_rules
from .. import po_lib_sub # noqa: F401


class POBase:
expected_overrides: Callable
class POBase(ItemPage):
expected_overrides: Type[ItemPage]
expected_patterns: Patterns
expected_meta: Dict[str, Any]


class POTopLevelOverriden1:
class POTopLevelOverriden1(ItemPage):
...


class POTopLevelOverriden2:
class POTopLevelOverriden2(ItemPage):
...


Expand Down
4 changes: 2 additions & 2 deletions tests/po_lib/a_module.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
from url_matcher import Patterns

from tests.po_lib import POBase
from web_poet import handle_urls
from web_poet import ItemPage, handle_urls


class POModuleOverriden:
class POModuleOverriden(ItemPage):
...


Expand Down
4 changes: 2 additions & 2 deletions tests/po_lib/nested_package/__init__.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
from url_matcher import Patterns

from tests.po_lib import POBase
from web_poet import handle_urls
from web_poet import ItemPage, handle_urls


class PONestedPkgOverriden:
class PONestedPkgOverriden(ItemPage):
...


Expand Down
4 changes: 2 additions & 2 deletions tests/po_lib/nested_package/a_nested_module.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
from url_matcher import Patterns

from tests.po_lib import POBase
from web_poet import handle_urls
from web_poet import ItemPage, handle_urls


class PONestedModuleOverriden:
class PONestedModuleOverriden(ItemPage):
...


Expand Down
10 changes: 5 additions & 5 deletions tests/po_lib_sub/__init__.py
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
"""This package is being used by tests/po_lib to validate some behaviors on
external depedencies.
"""
from typing import Any, Callable, Dict
from typing import Any, Dict, Type

from url_matcher import Patterns

from web_poet import handle_urls
from web_poet import ItemPage, handle_urls


class POBase:
expected_overrides: Callable
class POBase(ItemPage):
expected_overrides: Type[ItemPage]
expected_patterns: Patterns
expected_meta: Dict[str, Any]


class POLibSubOverriden:
class POLibSubOverriden(ItemPage):
...


Expand Down
8 changes: 4 additions & 4 deletions tests/test_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
URL = "https://example.com"


def test_http_error_init():
def test_http_error_init() -> None:
exc = HttpError()
assert exc.request is None
assert exc.args
Expand All @@ -16,7 +16,7 @@ def test_http_error_init():
assert exc.request == request


def test_http_request_error_init():
def test_http_request_error_init() -> None:
exc = HttpRequestError()
assert exc.request is None
assert exc.args
Expand All @@ -27,10 +27,10 @@ def test_http_request_error_init():

response = HttpResponse(URL, b"")
with pytest.raises(TypeError):
HttpRequestError(request=request, response=response)
HttpRequestError(request=request, response=response) # type: ignore[call-arg]
Gallaecio marked this conversation as resolved.
Show resolved Hide resolved


def test_http_response_error_init():
def test_http_response_error_init() -> None:
exc = HttpResponseError()
assert exc.request is None
assert exc.response is None
Expand Down
31 changes: 16 additions & 15 deletions tests/test_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def unknown_attribute(self): # noqa: D102


@pytest.mark.asyncio
async def test_fields():
async def test_fields() -> None:
page = Page(response=EXAMPLE_RESPONSE)

assert page.name == "Hello!"
Expand All @@ -68,15 +68,15 @@ async def test_fields():


@pytest.mark.asyncio
async def test_fields_invalid_page():
async def test_fields_invalid_page() -> None:
page = InvalidPage(response=EXAMPLE_RESPONSE)
with pytest.raises(
TypeError, match="unexpected keyword argument 'unknown_attribute'"
):
await page.to_item()


def test_item_from_fields_sync():
def test_item_from_fields_sync() -> None:
@attrs.define
class Page(ItemPage):
@field
Expand All @@ -90,12 +90,13 @@ def to_item(self): # noqa: D102
assert page.to_item() == dict(name="name")


def test_field_non_callable():
def test_field_non_callable() -> None:
with pytest.raises(TypeError):

@attrs.define
class Page(ItemPage):
@field
# https://github.com/python/mypy/issues/1362#issuecomment-438246775
@field # type: ignore
@property
def name(self): # noqa: D102
return "name"
Expand All @@ -104,7 +105,7 @@ def to_item(self): # noqa: D102
return item_from_fields_sync(self, dict)


def test_field_classmethod():
def test_field_classmethod() -> None:
with pytest.raises(TypeError):

@attrs.define
Expand All @@ -119,7 +120,7 @@ def to_item(self): # noqa: D102


@pytest.mark.asyncio
async def test_field_order():
async def test_field_order() -> None:
class DictItemPage(Page):
async def to_item(self):
return await item_from_fields(self)
Expand All @@ -130,7 +131,7 @@ async def to_item(self):
assert list(item.keys()) == ["name", "price"]


def test_field_decorator_no_arguments():
def test_field_decorator_no_arguments() -> None:
class Page(ItemPage):
@field()
def name(self):
Expand All @@ -143,7 +144,7 @@ def to_item(self):
assert page.to_item() == {"name": "Name"}


def test_field_cache_sync():
def test_field_cache_sync() -> None:
class Page:
_n_called_1 = 0
_n_called_2 = 0
Expand Down Expand Up @@ -171,7 +172,7 @@ def n_called_2(self):


@pytest.mark.asyncio
async def test_field_cache_async():
async def test_field_cache_async() -> None:
class Page:
_n_called_1 = 0
_n_called_2 = 0
Expand Down Expand Up @@ -199,7 +200,7 @@ async def n_called_2(self):


@pytest.mark.asyncio
async def test_field_cache_async_locked():
async def test_field_cache_async_locked() -> None:
class Page:
_n_called = 0

Expand All @@ -221,7 +222,7 @@ async def n_called(self):


@pytest.mark.asyncio
async def test_skip_nonitem_fields_async():
async def test_skip_nonitem_fields_async() -> None:
class ExtendedPage(Page):
@field
def new_attribute(self):
Expand All @@ -240,7 +241,7 @@ async def to_item(self) -> Item:
assert item == Item(name="Hello!", price="$123")


def test_skip_nonitem_fields():
def test_skip_nonitem_fields() -> None:
@attrs.define
class SyncPage(Injectable):
response: HttpResponse
Expand Down Expand Up @@ -274,7 +275,7 @@ def to_item(self) -> Item:
assert item == Item(name="Hello!", price="$123")


def test_field_meta():
def test_field_meta() -> None:
class MyPage(ItemPage):
@field(meta={"good": True})
def field1(self):
Expand All @@ -297,7 +298,7 @@ def to_item(self):
assert fields["field2"].meta is None


def test_field_subclassing():
def test_field_subclassing() -> None:
class Page(ItemPage):
@field
def field1(self):
Expand Down
14 changes: 7 additions & 7 deletions tests/test_mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,33 +15,33 @@ def my_page(book_list_html_response):
return MyPage(book_list_html_response)


def test_url(my_page):
def test_url(my_page) -> None:
assert my_page.url == "http://books.toscrape.com/index.html"


def test_html(my_page, book_list_html):
def test_html(my_page, book_list_html) -> None:
assert my_page.html == book_list_html


def test_xpath(my_page):
def test_xpath(my_page) -> None:
title = my_page.xpath(".//title/text()").get().strip()
assert title == "All products | Books to Scrape - Sandbox"


def test_css(my_page):
def test_css(my_page) -> None:
title = my_page.css("title::text").get().strip()
assert title == "All products | Books to Scrape - Sandbox"


def test_baseurl(my_page):
def test_baseurl(my_page) -> None:
assert my_page.base_url == "http://books.toscrape.com/index.html"


def test_urljoin(my_page):
def test_urljoin(my_page) -> None:
assert my_page.urljoin("foo") == "http://books.toscrape.com/foo"


def test_custom_baseurl():
def test_custom_baseurl() -> None:
body = b"""
<html>
<head>
Expand Down
20 changes: 11 additions & 9 deletions tests/test_overrides.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
POS = {POTopLevel1, POTopLevel2, POModule, PONestedPkg, PONestedModule}


def test_override_rule_uniqueness():
def test_override_rule_uniqueness() -> None:
"""The same instance of an OverrideRule with the same attribute values should
have the same hash identity.
"""
Expand All @@ -34,7 +34,7 @@ def test_override_rule_uniqueness():
assert hash(rule1) == hash(rule2)


def test_list_page_objects_all():
def test_list_page_objects_all() -> None:
rules = default_registry.get_overrides()
page_objects = {po.use for po in rules}

Expand All @@ -53,17 +53,19 @@ def test_list_page_objects_all():
# registry's @handle_urls annotation was used.
assert page_objects == POS.union({POLibSub})
for rule in rules:
assert rule.instead_of == rule.use.expected_overrides, rule.use
assert rule.for_patterns == rule.use.expected_patterns, rule.use
assert rule.meta == rule.use.expected_meta, rule.use
# We're ignoring the types below since mypy expects ``Type[ItemPage]``
# which doesn't contain the ``expected_*`` fields in our tests.
assert rule.instead_of == rule.use.expected_overrides, rule.use # type: ignore[attr-defined]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it failing, is a missing type annotation somewhere? I.e. why isn't mypy aware that rules contains Rule instances?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The exact error here is "Callable[..., Any]" has no attribute "expected_overrides" [attr-defined] which is mainly due to the test definitions in https://github.com/scrapinghub/web-poet/blob/master/tests/po_lib/__init__.py#L15-L17.

Speaking of this error, I realized that we're labelling POs as Callable. I've updated the code to make it Type[ItemPage] instead. ceab08e This ensures that we're promoting the overrides to be a subclass of ItemPage. I'm also not quite sure why we were labelling POs as Callable before. 🤔 What do you think of this change?

The type: ignore is still needed here since it's expecting Type[ItemPage] which doesn't contain class attributes like expected_overrides which are only used in the tests.

assert rule.for_patterns == rule.use.expected_patterns, rule.use # type: ignore[attr-defined]
assert rule.meta == rule.use.expected_meta, rule.use # type: ignore[attr-defined]


def test_consume_module_not_existing():
def test_consume_module_not_existing() -> None:
with pytest.raises(ImportError):
consume_modules("this_does_not_exist")


def test_list_page_objects_all_consume():
def test_list_page_objects_all_consume() -> None:
"""A test similar to the one above but calls ``consume_modules()`` to properly
load the @handle_urls annotations from other modules/packages.
"""
Expand All @@ -73,7 +75,7 @@ def test_list_page_objects_all_consume():
assert any(["po_lib_sub_not_imported" in po.__module__ for po in page_objects])


def test_registry_search_overrides():
def test_registry_search_overrides() -> None:
rules = default_registry.search_overrides(use=POTopLevel2)
assert len(rules) == 1
assert rules[0].use == POTopLevel2
Expand All @@ -87,7 +89,7 @@ def test_registry_search_overrides():
assert len(rules) == 0


def test_from_override_rules():
def test_from_override_rules() -> None:
rules = [
OverrideRule(
for_patterns=Patterns(include=["sample.com"]),
Expand Down
Loading