diff --git a/.flake8 b/.flake8 index d72d7daa..a2abfde7 100644 --- a/.flake8 +++ b/.flake8 @@ -43,6 +43,7 @@ per-file-ignores = web_poet/page_inputs/__init__.py:F401,F403 web_poet/serialization/__init__.py:F401,F403 web_poet/testing/__init__.py:F401,F403 + web_poet/testing/pytest.py:D102 tests/po_lib_to_return/__init__.py:D102 # the suggestion makes the code worse diff --git a/.github/workflows/tests-ubuntu.yml b/.github/workflows/tests-ubuntu.yml index a61e1642..0b0cb917 100644 --- a/.github/workflows/tests-ubuntu.yml +++ b/.github/workflows/tests-ubuntu.yml @@ -42,7 +42,7 @@ jobs: uses: codecov/codecov-action@v3 if: ${{ success() }} with: - fail_ci_if_error: true + fail_ci_if_error: false check: runs-on: ubuntu-latest diff --git a/.github/workflows/tests-windows.yml b/.github/workflows/tests-windows.yml index cde8fabb..f67ce84d 100644 --- a/.github/workflows/tests-windows.yml +++ b/.github/workflows/tests-windows.yml @@ -42,4 +42,4 @@ jobs: uses: codecov/codecov-action@v3 if: ${{ success() }} with: - fail_ci_if_error: true + fail_ci_if_error: false diff --git a/.gitignore b/.gitignore index 2a581ab6..966d749c 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,4 @@ htmlcov/ docs/_build *.egg-info/ __pycache__/ +build/ \ No newline at end of file diff --git a/docs/page-objects/testing.rst b/docs/page-objects/testing.rst index f58665a2..d55fc832 100644 --- a/docs/page-objects/testing.rst +++ b/docs/page-objects/testing.rst @@ -100,6 +100,21 @@ 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. +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 +fields separately. + +If you prefer less granular test running, 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 +is not fully correct, the test fails. So, following the previous example, +it'd be 2 tests instead of 12. + .. _web-poet-testing-frozen_time: Handling time fields diff --git a/tests/test_testing.py b/tests/test_testing.py index ef9d397a..deaca5ca 100644 --- a/tests/test_testing.py +++ b/tests/test_testing.py @@ -16,6 +16,8 @@ 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 + def test_save_fixture(book_list_html_response, tmp_path) -> None: base_dir = tmp_path / "fixtures" / "some.po" @@ -55,20 +57,101 @@ async def to_item(self) -> dict: # noqa: D102 return {"foo": "bar"} +class MyItemPage2(WebPage): + async def to_item(self) -> dict: # noqa: D102 + return {"foo": None} + + +def _save_fixture(pytester, page_cls, page_inputs, expected): + base_dir = pytester.path / "fixtures" / get_fq_class_name(page_cls) + Fixture.save(base_dir, inputs=page_inputs, item=expected) + + def test_pytest_plugin_pass(pytester, book_list_html_response) -> None: - item = {"foo": "bar"} - base_dir = pytester.path / "fixtures" / get_fq_class_name(MyItemPage) - Fixture.save(base_dir, inputs=[book_list_html_response], item=item) + _save_fixture( + pytester, + page_cls=MyItemPage, + page_inputs=[book_list_html_response], + expected={"foo": "bar"}, + ) result = pytester.runpytest() - result.assert_outcomes(passed=1) + result.assert_outcomes(passed=2) -def test_pytest_plugin_fail(pytester, book_list_html_response) -> None: - item = {"foo": "wrong"} - base_dir = pytester.path / "fixtures" / get_fq_class_name(MyItemPage) - Fixture.save(base_dir, inputs=[book_list_html_response], item=item) +def test_pytest_plugin_bad_field_value(pytester, book_list_html_response) -> None: + _save_fixture( + pytester, + page_cls=MyItemPage, + page_inputs=[book_list_html_response], + expected={"foo": "not bar"}, + ) + result = pytester.runpytest() + result.assert_outcomes(failed=1, passed=1) + result.stdout.fnmatch_lines("item.foo is not correct*") + + +def test_pytest_plugin_bad_field_value_None(pytester, book_list_html_response) -> None: + _save_fixture( + pytester, + page_cls=MyItemPage2, + page_inputs=[book_list_html_response], + expected={"foo": "bar"}, + ) + result = pytester.runpytest() + result.assert_outcomes(failed=1, passed=1) + result.stdout.fnmatch_lines("item.foo is not correct*") + result.stdout.fnmatch_lines("Expected: 'bar', got: None*") + + +def test_pytest_plugin_missing_field(pytester, book_list_html_response) -> None: + _save_fixture( + pytester, + page_cls=MyItemPage, + page_inputs=[book_list_html_response], + expected={"foo": "bar", "foo2": "bar2"}, + ) result = pytester.runpytest() - result.assert_outcomes(failed=1) + result.assert_outcomes(failed=1, passed=2) + result.stdout.fnmatch_lines("item.foo2 is missing*") + + +def test_pytest_plugin_extra_field(pytester, book_list_html_response) -> None: + _save_fixture( + pytester, + page_cls=MyItemPage, + page_inputs=[book_list_html_response], + expected={"foo2": "bar2"}, + ) + result = pytester.runpytest() + result.assert_outcomes(failed=2, passed=0) + result.stdout.fnmatch_lines("item.foo2 is missing*") + result.stdout.fnmatch_lines("*unexpected fields*") + result.stdout.fnmatch_lines("*foo = 'bar'*") + + +def test_pytest_plugin_compare_item(pytester, book_list_html_response) -> None: + _save_fixture( + pytester, + page_cls=MyItemPage, + page_inputs=[book_list_html_response], + expected={"foo": "bar"}, + ) + result = pytester.runpytest("--web-poet-test-per-item") + result.assert_outcomes(passed=1) + + +def test_pytest_plugin_compare_item_fail(pytester, book_list_html_response) -> None: + _save_fixture( + pytester, + page_cls=MyItemPage, + page_inputs=[book_list_html_response], + expected={"foo": "not bar"}, + ) + result = pytester.runpytest("--web-poet-test-per-item") + result.assert_outcomes(passed=0, failed=1) + + result.stdout.fnmatch_lines("*{'foo': 'bar'} != {'foo': 'not bar'}*") + result.stdout.fnmatch_lines("*The output doesn't match*") @attrs.define(kw_only=True) @@ -114,7 +197,7 @@ def _assert_frozen_item( # the result should contain frozen_time in the datetime fields result = pytester.runpytest() if outcomes is None: - outcomes = {"passed": 1} + outcomes = {"passed": N_TESTS} result.assert_outcomes(**outcomes) @@ -145,10 +228,13 @@ def test_pytest_frozen_time_tz(pytester, book_list_html_response, offset) -> Non @pytest.mark.skipif(time_machine.HAVE_TZSET, reason="Tests Windows-specific code") def test_pytest_frozen_time_tz_windows_fail(pytester, book_list_html_response) -> None: frozen_time = datetime.datetime( - 2022, 3, 4, 20, 21, 22, tzinfo=dateutil.tz.tzoffset(None, -7.5) + 2022, 3, 4, 20, 21, 22, tzinfo=dateutil.tz.tzoffset(None, -7.5 * 3600) ) _assert_frozen_item( - frozen_time, pytester, book_list_html_response, outcomes={"failed": 1} + frozen_time, + pytester, + book_list_html_response, + outcomes={"failed": 1, "passed": N_TESTS - 1}, ) diff --git a/web_poet/testing/exceptions.py b/web_poet/testing/exceptions.py new file mode 100644 index 00000000..1208d683 --- /dev/null +++ b/web_poet/testing/exceptions.py @@ -0,0 +1,14 @@ +class FieldMissing(AssertionError): + pass + + +class FieldValueIncorrect(AssertionError): + pass + + +class FieldsUnexpected(AssertionError): + pass + + +class ItemValueIncorrect(AssertionError): + pass diff --git a/web_poet/testing/fixture.py b/web_poet/testing/fixture.py index 57a31f0d..a7aee455 100644 --- a/web_poet/testing/fixture.py +++ b/web_poet/testing/fixture.py @@ -19,7 +19,14 @@ load_class, serialize, ) -from web_poet.utils import ensure_awaitable +from web_poet.utils import ensure_awaitable, memoizemethod_noargs + +from .exceptions import ( + FieldMissing, + FieldsUnexpected, + FieldValueIncorrect, + ItemValueIncorrect, +) logger = logging.getLogger(__name__) @@ -52,6 +59,16 @@ def type_name(self) -> str: """The name of the type being tested.""" return self.path.parent.name + @property + def test_name(self) -> str: + """The name of the test.""" + return self.path.name + + @property + def short_name(self) -> str: + """The name of this fixture""" + return f"{self.type_name}/{self.test_name}" + @property def input_path(self) -> Path: """The inputs subdirectory path.""" @@ -85,12 +102,27 @@ def get_meta(self) -> dict: return {} return json.loads(self.meta_path.read_bytes()) - def get_output(self) -> dict: - """Return the output from the recreated Page Object.""" - po = self.get_page() - item = asyncio.run(ensure_awaitable(po.to_item())) + def _get_output(self) -> dict: + page = self.get_page() + item = asyncio.run(ensure_awaitable(page.to_item())) return ItemAdapter(item).asdict() + @memoizemethod_noargs + 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): + return self._get_output() + else: + return self._get_output() + + @memoizemethod_noargs def get_expected_output(self) -> dict: """Return the saved output.""" return json.loads(self.output_path.read_bytes()) @@ -127,18 +159,35 @@ def _parse_frozen_time(meta_value: str) -> datetime.datetime: tzinfo = ZoneInfo(f"Etc/GMT{-offset_hours:+d}") return parsed_value.replace(tzinfo=tzinfo) - def assert_output(self): + def get_expected_output_fields(self): + """Return a list of the expected output field names.""" + output = self.get_expected_output() + return list(output.keys()) + + def assert_full_item_correct(self): """Get the output and assert that it matches the expected output.""" - 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): - output = self.get_output() - else: - output = self.get_output() + output = self.get_output() + expected_output = self.get_expected_output() + if output != expected_output: + raise ItemValueIncorrect(output, expected_output) + + def assert_field_correct(self, name: str): + """Assert that a certain field in the output matches the expected value""" + expected_output = self.get_expected_output()[name] + if name not in self.get_output(): + raise FieldMissing(name) + output = self.get_output()[name] + if output != expected_output: + raise FieldValueIncorrect(output, expected_output) + + def assert_no_extra_fields(self): + """Assert that there are no extra fields in the output""" + output = self.get_output() expected_output = self.get_expected_output() - assert output == expected_output + extra_field_keys = output.keys() - expected_output.keys() + extra_fields = {key: output[key] for key in extra_field_keys} + if extra_fields: + raise FieldsUnexpected(extra_fields) @classmethod def save( diff --git a/web_poet/testing/pytest.py b/web_poet/testing/pytest.py index fef26eac..752db3f9 100644 --- a/web_poet/testing/pytest.py +++ b/web_poet/testing/pytest.py @@ -1,30 +1,43 @@ import operator from pathlib import Path -from typing import Any, Iterable, List, Optional, Set, Union +from typing import Iterable, List, Optional, Set, Union import pytest +from web_poet.testing.exceptions import ( + FieldMissing, + FieldsUnexpected, + FieldValueIncorrect, + ItemValueIncorrect, +) from web_poet.testing.fixture import OUTPUT_FILE_NAME, Fixture +from web_poet.testing.utils import comparison_error_message # https://github.com/pytest-dev/pytest/discussions/10261 _version_tuple = getattr(pytest, "version_tuple", None) _new_pytest = _version_tuple and _version_tuple[0] >= 7 -class WebPoetFile(pytest.File): +class _PathCompatMixin: + @property + def _path(self): + return self.path if _new_pytest else Path(self.fspath) + + +class WebPoetFile(pytest.File, _PathCompatMixin): """Represents a directory containing test subdirectories for one Page Object.""" @staticmethod - def sorted(items: List["WebPoetItem"]) -> List["WebPoetItem"]: + def sorted(items: List["WebPoetCollector"]) -> List["WebPoetCollector"]: """Sort the test list by the test name.""" return sorted(items, key=operator.attrgetter("name")) def collect(self) -> Iterable[Union[pytest.Item, pytest.Collector]]: # noqa: D102 - result: List[WebPoetItem] = [] - path = self.path if _new_pytest else Path(self.fspath) + result: List[WebPoetCollector] = [] + path = self._path for entry in path.iterdir(): if entry.is_dir(): - item: WebPoetItem = _get_item( + item: WebPoetCollector = _get_collector( self, name=entry.name, path=entry, @@ -34,38 +47,109 @@ def collect(self) -> Iterable[Union[pytest.Item, pytest.Collector]]: # noqa: D1 return self.sorted(result) -class WebPoetItem(pytest.Item): +class WebPoetCollector(pytest.Collector, _PathCompatMixin): """Represents a directory containing one test.""" - if _new_pytest: - - def __init__( - self, - name, - parent=None, - config: Optional["pytest.Config"] = None, - session: Optional[pytest.Session] = None, - nodeid: Optional[str] = None, - **kw, - ) -> None: - super().__init__(name, parent, config, session, nodeid, **kw) - self.fixture = Fixture(self.path) - - else: - - def __init__( # type: ignore[misc] - self, - name, - parent=None, - config: Optional[Any] = None, - session: Optional[pytest.Session] = None, - nodeid: Optional[str] = None, - ) -> None: - super().__init__(name, parent, config, session, nodeid) - self.fixture = Fixture(Path(self.fspath, self.name)) - - def runtest(self) -> None: # noqa: D102 - self.fixture.assert_output() + def __init__(self, name: str, parent=None, **kwargs) -> None: + super(WebPoetCollector, self).__init__(name, parent, **kwargs) + self.fixture = Fixture(self._path) + + def collect(self) -> Iterable[Union[pytest.Item, pytest.Collector]]: + """Return a list of children (items and collectors) for this + collection node.""" + if self.config.getoption("WEB_POET_TEST_PER_ITEM", default=False): + return [ + WebPoetItem.from_parent(parent=self, name="item", fixture=self.fixture) + ] + else: + 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 + + +class _WebPoetItem(pytest.Item, _PathCompatMixin): + def __init__(self, *, fixture: Fixture, **kwargs) -> None: + super().__init__(**kwargs) + self.fixture = fixture + + +class WebPoetItem(_WebPoetItem): + def runtest(self) -> None: + self.fixture.assert_full_item_correct() + + def reportinfo(self): + return self._path, 0, f"{self.fixture.short_name}" + + def repr_failure(self, excinfo, style=None): + if isinstance(excinfo.value, ItemValueIncorrect): + got, expected = excinfo.value.args + return comparison_error_message( + config=self.config, + op="==", + expected=expected, + got=got, + prefix="The output doesn't match.", + ) + else: + return super().repr_failure(excinfo, style) + + +class WebPoetNoExtraFieldsItem(_WebPoetItem): + def runtest(self) -> None: + self.fixture.assert_no_extra_fields() + + def reportinfo(self): + return self._path, 0, f"{self.fixture.short_name}: extra fields" + + def repr_failure(self, excinfo, style=None): + if isinstance(excinfo.value, FieldsUnexpected): + fields = excinfo.value.args[0] + return f"The item contains unexpected fields: \n{self._format_extra_fields(fields)}" + else: + return super().repr_failure(excinfo, style) + + def _format_extra_fields(self, extra_fields): + lines = [] + for field, value in extra_fields.items(): + lines.append(f" * {field} = {value!r}") + return "\n".join(lines) + + +class WebPoetFieldItem(_WebPoetItem): + def __init__(self, *, field_name: str, **kwargs) -> None: + super().__init__(**kwargs) + self.field_name = field_name + + def runtest(self) -> None: + self.fixture.assert_field_correct(self.field_name) + + def reportinfo(self): + return self._path, 0, f"{self.fixture.short_name} @ {self.field_name}" + + def repr_failure(self, excinfo, style=None): + if isinstance(excinfo.value, FieldValueIncorrect): + got, expected = excinfo.value.args + return comparison_error_message( + config=self.config, + op="==", + expected=expected, + got=got, + prefix=f"item.{self.field_name} is not correct.", + ) + elif isinstance(excinfo.value, FieldMissing): + field_name = excinfo.value.args[0] + return f"item.{field_name} is missing." + else: + return super().repr_failure(excinfo, style) _found_type_dirs: Set[Path] = set() @@ -91,6 +175,17 @@ def collect_file_hook( return None +def pytest_addoption( + parser: "pytest.Parser", pluginmanager: "pytest.PytestPluginManager" +): + parser.addoption( + "--web-poet-test-per-item", + dest="WEB_POET_TEST_PER_ITEM", + action="store_true", + help="web-poet: use a single test per item, not a test per field", + ) + + if _new_pytest: def _get_item(parent: pytest.Collector, *, name: str, path: Path) -> WebPoetItem: @@ -100,6 +195,15 @@ def _get_item(parent: pytest.Collector, *, name: str, path: Path) -> WebPoetItem path=path, ) + def _get_collector( + parent: pytest.Collector, *, name: str, path: Path + ) -> WebPoetCollector: + return WebPoetCollector.from_parent( + parent, + name=name, + path=path, + ) + def _get_file(parent: pytest.Collector, *, path: Path) -> WebPoetFile: return WebPoetFile.from_parent( parent, @@ -120,6 +224,15 @@ def _get_item(parent: pytest.Collector, *, name: str, path: Path) -> WebPoetItem name=name, ) + def _get_collector( + parent: pytest.Collector, *, name: str, path: Path + ) -> WebPoetCollector: + return WebPoetCollector.from_parent( + parent, + name=name, + fspath=py.path.local(path), + ) + def _get_file(parent: pytest.Collector, *, path: Path) -> WebPoetFile: return WebPoetFile.from_parent( parent, diff --git a/web_poet/testing/utils.py b/web_poet/testing/utils.py new file mode 100644 index 00000000..ad195c00 --- /dev/null +++ b/web_poet/testing/utils.py @@ -0,0 +1,19 @@ +import pytest +from _pytest.assertion.util import assertrepr_compare + + +def comparison_error_message( + config: "pytest.Config", op: str, expected, got, prefix: str = "" +) -> str: + """Generate an error message""" + lines = [prefix] if prefix else [] + + explanation_lines = assertrepr_compare( + config=config, op=op, left=got, right=expected + ) + if explanation_lines: + lines.extend(explanation_lines) + else: + lines.append(f"Expected: {expected!r}, got: {got!r}") + + return "\n".join(lines)