Fix scrapinghubGH-131

.flake8

@@ -45,6 +45,7 @@ per-file-ignores =
     web_poet/testing/__init__.py:F401,F403
     web_poet/testing/pytest.py:D102
     tests/po_lib_to_return/__init__.py:D102
+    tests/test_testing.py:D102
 
     # the suggestion makes the code worse
     tests/test_serialization.py:B028

docs/page-objects/testing.rst

@@ -107,20 +107,28 @@ The provided ``pytest`` plugin is automatically registered when ``web-poet`` is
 installed, and running ``python -m pytest`` in a directory containing fixtures
 will discover them and run tests for them.
 
-By default, the plugin generates a test per each output attribute of the item,
-and an additional test to check that there are no extra attributes in the output.
+By default, the plugin generates
+
+* a test which checks that ``to_item()`` doesn't raise an exception
+  (i.e. it can be executed),
+* a test per each output attribute of the item,
+* an additional test to check that there are no extra attributes in the output.
+
 For example, if your item has 5 attributes, and you created 2 fixtures, pytest
-will run (5+1)*2 = 12 tests. This allows to report failures for individual
+will run (5+1+1)*2 = 14 tests. This allows to report failures for individual
 fields separately.
 
-If you prefer less granular test running, you can use pytest with
+If ``to_item`` raises an error, there is no point in running other tests,
+so they're skipped in this case.
+
+If you prefer less granular test failure reporting, you can use pytest with
 the ``--web-poet-test-per-item`` option::
 
     python -m pytest --web-poet-test-per-item
 
-In this case there is going to be a test per fixture: if the result
+In this case there is going to be a single test per fixture: if the result
 is not fully correct, the test fails. So, following the previous example,
-it'd be 2 tests instead of 12.
+it'd be 2 tests instead of 14.
 
 .. _web-poet-testing-frozen_time:
 

tests/test_testing.py

@@ -11,14 +11,14 @@
 from itemadapter import ItemAdapter
 from zyte_common_items import Item, Metadata, Product
 
-from web_poet import HttpClient, HttpRequest, HttpResponse, WebPage
+from web_poet import HttpClient, HttpRequest, HttpResponse, WebPage, field
 from web_poet.exceptions import HttpResponseError
 from web_poet.page_inputs.client import _SavedResponseData
 from web_poet.testing import Fixture
 from web_poet.testing.fixture import INPUT_DIR_NAME, META_FILE_NAME, OUTPUT_FILE_NAME
 from web_poet.utils import get_fq_class_name
 
-N_TESTS = len(attrs.fields(Product)) + 1
+N_TESTS = len(attrs.fields(Product)) + 2
 
 
 def test_save_fixture(book_list_html_response, tmp_path) -> None:
@@ -77,7 +77,7 @@ def test_pytest_plugin_pass(pytester, book_list_html_response) -> None:
         expected={"foo": "bar"},
     )
     result = pytester.runpytest()
-    result.assert_outcomes(passed=2)
+    result.assert_outcomes(passed=3)
 
 
 def test_pytest_plugin_bad_field_value(pytester, book_list_html_response) -> None:
@@ -88,7 +88,7 @@ def test_pytest_plugin_bad_field_value(pytester, book_list_html_response) -> Non
         expected={"foo": "not bar"},
     )
     result = pytester.runpytest()
-    result.assert_outcomes(failed=1, passed=1)
+    result.assert_outcomes(failed=1, passed=2)
     result.stdout.fnmatch_lines("item.foo is not correct*")
 
 
@@ -100,7 +100,7 @@ def test_pytest_plugin_bad_field_value_None(pytester, book_list_html_response) -
         expected={"foo": "bar"},
     )
     result = pytester.runpytest()
-    result.assert_outcomes(failed=1, passed=1)
+    result.assert_outcomes(failed=1, passed=2)
     result.stdout.fnmatch_lines("item.foo is not correct*")
     result.stdout.fnmatch_lines("Expected: 'bar', got: None*")
 
@@ -113,7 +113,7 @@ def test_pytest_plugin_missing_field(pytester, book_list_html_response) -> None:
         expected={"foo": "bar", "foo2": "bar2"},
     )
     result = pytester.runpytest()
-    result.assert_outcomes(failed=1, passed=2)
+    result.assert_outcomes(failed=1, passed=3)
     result.stdout.fnmatch_lines("item.foo2 is missing*")
 
 
@@ -125,12 +125,34 @@ def test_pytest_plugin_extra_field(pytester, book_list_html_response) -> None:
         expected={"foo2": "bar2"},
     )
     result = pytester.runpytest()
-    result.assert_outcomes(failed=2, passed=0)
+    result.assert_outcomes(failed=2, passed=1)
     result.stdout.fnmatch_lines("item.foo2 is missing*")
     result.stdout.fnmatch_lines("*unexpected fields*")
     result.stdout.fnmatch_lines("*foo = 'bar'*")
 
 
+class FieldExceptionPage(WebPage):
+    @field
+    def foo(self):
+        return "foo"
+
+    @field
+    def bar(self):
+        raise Exception
+
+
+def test_pytest_plugin_field_exception(pytester, book_list_html_response) -> None:
+    _save_fixture(
+        pytester,
+        page_cls=FieldExceptionPage,
+        page_inputs=[book_list_html_response],
+        expected={"foo": "foo", "bar": "bar"},
+    )
+    result = pytester.runpytest()
+    result.assert_outcomes(failed=1, skipped=3)
+    result.stdout.fnmatch_lines("*TO_ITEM_DOESNT_RAISE - Exception*")
+
+
 def test_pytest_plugin_compare_item(pytester, book_list_html_response) -> None:
     _save_fixture(
         pytester,
@@ -286,7 +308,7 @@ def test_httpclient(pytester, book_list_html_response) -> None:
     assert (input_dir / "HttpClient-0-HttpResponse.body.html").read_bytes() == b"body1"
     assert (input_dir / "HttpClient-1-HttpResponse.body.html").read_bytes() == b"body2"
     result = pytester.runpytest()
-    result.assert_outcomes(passed=3)
+    result.assert_outcomes(passed=4)
 
 
 def test_httpclient_no_response(pytester, book_list_html_response) -> None:
@@ -309,7 +331,7 @@ def test_httpclient_no_response(pytester, book_list_html_response) -> None:
         expected=item,
     )
     result = pytester.runpytest()
-    result.assert_outcomes(failed=3)
+    result.assert_outcomes(failed=1, skipped=3)
 
 
 @attrs.define
@@ -345,4 +367,4 @@ def test_httpclient_exception(pytester, book_list_html_response) -> None:
         expected=item,
     )
     result = pytester.runpytest()
-    result.assert_outcomes(passed=3)
+    result.assert_outcomes(passed=4)

web_poet/testing/fixture.py

@@ -53,6 +53,7 @@ class Fixture:
 
     def __init__(self, path: Path) -> None:
         self.path = path
+        self._output_error: Optional[Exception] = None
 
     @property
     def type_name(self) -> str:
@@ -113,14 +114,18 @@ def get_output(self) -> dict:
         Return the output from the recreated Page Object,
         taking frozen time in account.
         """
-        meta = self.get_meta()
-        frozen_time: Optional[str] = meta.get("frozen_time")
-        if frozen_time:
-            frozen_time_parsed = self._parse_frozen_time(frozen_time)
-            with time_machine.travel(frozen_time_parsed):
+        try:
+            meta = self.get_meta()
+            frozen_time: Optional[str] = meta.get("frozen_time")
+            if frozen_time:
+                frozen_time_parsed = self._parse_frozen_time(frozen_time)
+                with time_machine.travel(frozen_time_parsed):
+                    return self._get_output()
+            else:
                 return self._get_output()
-        else:
-            return self._get_output()
+        except Exception as e:
+            self._output_error = e
+            raise
 
     @memoizemethod_noargs
     def get_expected_output(self) -> dict:
@@ -189,6 +194,16 @@ def assert_no_extra_fields(self):
         if extra_fields:
             raise FieldsUnexpected(extra_fields)
 
+    def to_item_raised(self) -> bool:
+        """Return True if to_item raised an error.
+        Note that if to_item hasn't been called yet, this method returns False.
+        """
+        return self._output_error is not None
+
+    def assert_no_toitem_exceptions(self):
+        """Assert that to_item() can be run (doesn't raise an error)"""
+        self.get_output()
+
     @classmethod
     def save(
         cls: Type[FixtureT],

web_poet/testing/pytest.py

@@ -62,18 +62,21 @@ def collect(self) -> Iterable[Union[pytest.Item, pytest.Collector]]:
                 WebPoetItem.from_parent(parent=self, name="item", fixture=self.fixture)
             ]
         else:
+            overall_tests = [
+                WebPoetNoToItemException.from_parent(
+                    parent=self, name="TO_ITEM_DOESNT_RAISE", fixture=self.fixture
+                ),
+                WebPoetNoExtraFieldsItem.from_parent(
+                    parent=self, name="NO_EXTRA_FIELDS", fixture=self.fixture
+                ),
+            ]
             field_tests = [
                 WebPoetFieldItem.from_parent(
                     parent=self, name=field, fixture=self.fixture, field_name=field
                 )
                 for field in self.fixture.get_expected_output_fields()
             ]
-            no_extra_fields_tests = [
-                WebPoetNoExtraFieldsItem.from_parent(
-                    parent=self, name="NO_EXTRA_FIELDS", fixture=self.fixture
-                )
-            ]
-            return field_tests + no_extra_fields_tests
+            return overall_tests + field_tests
 
 
 class _WebPoetItem(pytest.Item, _PathCompatMixin):
@@ -105,6 +108,11 @@ def repr_failure(self, excinfo, style=None):
 
 class WebPoetNoExtraFieldsItem(_WebPoetItem):
     def runtest(self) -> None:
+        if self.fixture.to_item_raised():
+            raise pytest.skip(
+                "Skipping a test for unexpected item fields "
+                "because to_item raised an exception."
+            )
         self.fixture.assert_no_extra_fields()
 
     def reportinfo(self):
@@ -124,12 +132,29 @@ def _format_extra_fields(self, extra_fields):
         return "\n".join(lines)
 
 
+class WebPoetNoToItemException(_WebPoetItem):
+    def runtest(self) -> None:
+        self.fixture.assert_no_toitem_exceptions()
+
+    def reportinfo(self):
+        return (
+            self._path,
+            0,
+            f"{self.fixture.short_name}: to_item doesn't raise an error",
+        )
+
+
 class WebPoetFieldItem(_WebPoetItem):
     def __init__(self, *, field_name: str, **kwargs) -> None:
         super().__init__(**kwargs)
         self.field_name = field_name
 
     def runtest(self) -> None:
+        if self.fixture.to_item_raised():
+            raise pytest.skip(
+                f"Skipping a test for item.{self.field_name} "
+                f"because to_item raised an exception"
+            )
         self.fixture.assert_field_correct(self.field_name)
 
     def reportinfo(self):