From 7d90e44ae270490c2852619f8d606735bfacdda8 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 3 Jan 2022 18:09:05 -0800 Subject: [PATCH 1/9] fix bug in element key identity old children by key were being copied for elements. as a result we were attempting to delete elements more than once. --- src/idom/core/_fixed_jsonpatch.py | 54 +++++++++++++++++++++++++++++++ src/idom/core/dispatcher.py | 2 +- src/idom/core/layout.py | 2 +- 3 files changed, 56 insertions(+), 2 deletions(-) create mode 100644 src/idom/core/_fixed_jsonpatch.py diff --git a/src/idom/core/_fixed_jsonpatch.py b/src/idom/core/_fixed_jsonpatch.py new file mode 100644 index 000000000..217ce1bc7 --- /dev/null +++ b/src/idom/core/_fixed_jsonpatch.py @@ -0,0 +1,54 @@ +"""A patched version of jsonpatch + +We need this because of: https://github.com/stefankoegl/python-json-patch/issues/138 + +The core of this patch is in `DiffBuilder._item_removed`. The rest is just boilerplate +that's been copied over with little to no changes. +""" + +from jsonpatch import _ST_REMOVE +from jsonpatch import DiffBuilder as _DiffBuilder +from jsonpatch import JsonPatch as _JsonPatch +from jsonpatch import JsonPointer, RemoveOperation, _path_join + + +def apply_patch(doc, patch, in_place=False, pointer_cls=JsonPointer): + if isinstance(patch, (str, bytes)): + patch = JsonPatch.from_string(patch, pointer_cls=pointer_cls) + else: + patch = JsonPatch(patch, pointer_cls=pointer_cls) + return patch.apply(doc, in_place) + + +def make_patch(src, dst, pointer_cls=JsonPointer): + return JsonPatch.from_diff(src, dst, pointer_cls=pointer_cls) + + +class JsonPatch(_JsonPatch): + @classmethod + def from_diff( + cls, + src, + dst, + optimization=True, + dumps=None, + pointer_cls=JsonPointer, + ): + json_dumper = dumps or cls.json_dumper + builder = DiffBuilder(src, dst, json_dumper, pointer_cls=pointer_cls) + builder._compare_values("", None, src, dst) + ops = list(builder.execute()) + return cls(ops, pointer_cls=pointer_cls) + + +class DiffBuilder(_DiffBuilder): + def _item_removed(self, path, key, item): + new_op = RemoveOperation( + { + "op": "remove", + "path": _path_join(path, key), + }, + pointer_cls=self.pointer_cls, + ) + new_index = self.insert(new_op) + self.store_index(item, new_index, _ST_REMOVE) diff --git a/src/idom/core/dispatcher.py b/src/idom/core/dispatcher.py index 5c643abf2..04fed20b4 100644 --- a/src/idom/core/dispatcher.py +++ b/src/idom/core/dispatcher.py @@ -19,10 +19,10 @@ from weakref import WeakSet from anyio import create_task_group -from jsonpatch import apply_patch, make_patch from idom.utils import Ref +from ._fixed_jsonpatch import apply_patch, make_patch from .layout import LayoutEvent, LayoutUpdate from .proto import LayoutType, VdomJson diff --git a/src/idom/core/layout.py b/src/idom/core/layout.py index 64f31ac6e..b92f8c87f 100644 --- a/src/idom/core/layout.py +++ b/src/idom/core/layout.py @@ -538,7 +538,7 @@ def _update_element_model_state( key=old_model_state.key, model=Ref(), # does not copy the model patch_path=old_model_state.patch_path, - children_by_key=old_model_state.children_by_key.copy(), + children_by_key={}, targets_by_event={}, ) From bf21951cde8a3ef162fc245738baef04a61c1c53 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 3 Jan 2022 19:55:38 -0800 Subject: [PATCH 2/9] test for logged errors in layout as a result of this additional check, we found what seemed to be an unnecessary assertion about old component state havint the same key and index. we change the assertion to only check for the same key since only the key is meaningful for identity --- src/idom/core/_fixed_jsonpatch.py | 29 ++--- src/idom/core/layout.py | 5 +- src/idom/log.py | 15 +-- src/idom/testing.py | 90 +++++++++++++++ tests/test_core/test_layout.py | 185 ++++++++++++++++++++---------- 5 files changed, 231 insertions(+), 93 deletions(-) diff --git a/src/idom/core/_fixed_jsonpatch.py b/src/idom/core/_fixed_jsonpatch.py index 217ce1bc7..f571ff128 100644 --- a/src/idom/core/_fixed_jsonpatch.py +++ b/src/idom/core/_fixed_jsonpatch.py @@ -9,36 +9,28 @@ from jsonpatch import _ST_REMOVE from jsonpatch import DiffBuilder as _DiffBuilder from jsonpatch import JsonPatch as _JsonPatch -from jsonpatch import JsonPointer, RemoveOperation, _path_join +from jsonpatch import RemoveOperation, _path_join -def apply_patch(doc, patch, in_place=False, pointer_cls=JsonPointer): +def apply_patch(doc, patch, in_place=False): if isinstance(patch, (str, bytes)): - patch = JsonPatch.from_string(patch, pointer_cls=pointer_cls) + patch = JsonPatch.from_string(patch) else: - patch = JsonPatch(patch, pointer_cls=pointer_cls) + patch = JsonPatch(patch) return patch.apply(doc, in_place) -def make_patch(src, dst, pointer_cls=JsonPointer): - return JsonPatch.from_diff(src, dst, pointer_cls=pointer_cls) +def make_patch(src, dst): + return JsonPatch.from_diff(src, dst) class JsonPatch(_JsonPatch): @classmethod - def from_diff( - cls, - src, - dst, - optimization=True, - dumps=None, - pointer_cls=JsonPointer, - ): - json_dumper = dumps or cls.json_dumper - builder = DiffBuilder(src, dst, json_dumper, pointer_cls=pointer_cls) + def from_diff(cls, src, dst, optimization=True): + builder = DiffBuilder() builder._compare_values("", None, src, dst) ops = list(builder.execute()) - return cls(ops, pointer_cls=pointer_cls) + return cls(ops) class DiffBuilder(_DiffBuilder): @@ -47,8 +39,7 @@ def _item_removed(self, path, key, item): { "op": "remove", "path": _path_join(path, key), - }, - pointer_cls=self.pointer_cls, + } ) new_index = self.insert(new_op) self.store_index(item, new_index, _ST_REMOVE) diff --git a/src/idom/core/layout.py b/src/idom/core/layout.py index b92f8c87f..0e228b83f 100644 --- a/src/idom/core/layout.py +++ b/src/idom/core/layout.py @@ -217,10 +217,9 @@ def _render_component( else: key, index = new_state.key, new_state.index if old_state is not None: - assert (key, index) == (old_state.key, old_state.index,), ( + assert key == old_state.key, ( "state mismatch during component update - " - f"key {key!r}!={old_state.key} " - f"or index {index}!={old_state.index}" + f"key {key!r}!={old_state.key!r} " ) parent.children_by_key[key] = new_state # need to do insertion in case where old_state is None and we're appending diff --git a/src/idom/log.py b/src/idom/log.py index a4c6d6d2b..856ee9ab2 100644 --- a/src/idom/log.py +++ b/src/idom/log.py @@ -1,17 +1,12 @@ import logging import sys from logging.config import dictConfig -from typing import Any from .config import IDOM_DEBUG_MODE -root_logger = logging.getLogger("idom") - - -def logging_config_defaults() -> Any: - """Get default logging configuration""" - return { +dictConfig( + { "version": 1, "disable_existing_loggers": False, "loggers": { @@ -35,10 +30,12 @@ def logging_config_defaults() -> Any: } }, } +) -dictConfig(logging_config_defaults()) +ROOT_LOGGER = logging.getLogger("idom") +"""IDOM's root logger instance""" if IDOM_DEBUG_MODE.current: - root_logger.debug("IDOM is in debug mode") + ROOT_LOGGER.debug("IDOM is in debug mode") diff --git a/src/idom/testing.py b/src/idom/testing.py index 36adc5284..a8e71da32 100644 --- a/src/idom/testing.py +++ b/src/idom/testing.py @@ -1,13 +1,18 @@ +from __future__ import annotations + import logging import re import shutil +from contextlib import contextmanager from functools import wraps +from traceback import format_exception from types import TracebackType from typing import ( Any, Callable, Dict, Generic, + Iterator, List, Optional, Tuple, @@ -29,6 +34,8 @@ from idom.server.proto import ServerFactory, ServerType from idom.server.utils import find_available_port +from .log import ROOT_LOGGER + __all__ = [ "find_available_port", @@ -166,6 +173,86 @@ def __exit__( return None +@contextmanager +def assert_idom_logged( + match_message: str = "", + error_type: type[Exception] | None = None, + match_error: str = "", + clear_matched_records: bool = False, +) -> Iterator[None]: + """Assert that IDOM produced a log matching the described message or error. + + Args: + match_message: Must match a logged message. + error_type: Checks the type of logged exceptions. + match_error: Must match an error message. + clear_matched_records: Whether to remove logged records that match. + """ + message_pattern = re.compile(match_message) + error_pattern = re.compile(match_error) + + try: + with capture_idom_logs() as handler: + yield None + finally: + found = False + for record in list(handler.records): + if ( + # record message matches + message_pattern.findall(record.getMessage()) + # error type matches + and ( + not error_type + or ( + record.exc_info is not None + and issubclass(record.exc_info[0], error_type) + ) + ) + # error message pattern matches + and ( + not match_error + or ( + record.exc_info is not None + and error_pattern.findall( + "".join(format_exception(*record.exc_info)) + ) + ) + ) + ): + found = True + if clear_matched_records: + handler.records.remove(record) + + if not found: + conditions = [] + if match_message: + conditions.append(f"log message pattern {match_message!r}") + if error_type: + conditions.append(f"exception type {error_type}") + if match_error: + conditions.append(f"error message pattern {match_error!r}") + raise AssertionError( + "Could not find a log record matching the given " + + " and ".join(conditions) + ) + + +@contextmanager +def capture_idom_logs() -> Iterator[_LogRecordCaptor]: + """Capture logs from IDOM""" + if _LOG_RECORD_CAPTOR_SINGLTON in ROOT_LOGGER.handlers: + # this is being handled by an outer capture context + yield _LOG_RECORD_CAPTOR_SINGLTON + return None + + ROOT_LOGGER.addHandler(_LOG_RECORD_CAPTOR_SINGLTON) + try: + yield _LOG_RECORD_CAPTOR_SINGLTON + finally: + ROOT_LOGGER.removeHandler(_LOG_RECORD_CAPTOR_SINGLTON) + _LOG_RECORD_CAPTOR_SINGLTON.records = [] + + class _LogRecordCaptor(logging.NullHandler): def __init__(self) -> None: self.records: List[logging.LogRecord] = [] @@ -176,6 +263,9 @@ def handle(self, record: logging.LogRecord) -> bool: return True +_LOG_RECORD_CAPTOR_SINGLTON = _LogRecordCaptor() + + class HookCatcher: """Utility for capturing a LifeCycleHook from a component diff --git a/tests/test_core/test_layout.py b/tests/test_core/test_layout.py index adf307bf6..46a52c731 100644 --- a/tests/test_core/test_layout.py +++ b/tests/test_core/test_layout.py @@ -10,10 +10,24 @@ from idom.config import IDOM_DEBUG_MODE from idom.core.dispatcher import render_json_patch from idom.core.layout import LayoutEvent -from idom.testing import HookCatcher, StaticEventHandler +from idom.testing import ( + HookCatcher, + StaticEventHandler, + assert_idom_logged, + capture_idom_logs, +) from tests.general_utils import assert_same_items +@pytest.fixture(autouse=True) +def no_logged_errors(): + with capture_idom_logs() as handler: + yield + for record in handler.records: + if record.exc_info: + raise record.exc_info[1] + + def test_layout_repr(): @idom.component def MyComponent(): @@ -164,25 +178,30 @@ def OkChild(): @idom.component def BadChild(): - raise ValueError("Something went wrong :(") - - with idom.Layout(Main()) as layout: - patch = await render_json_patch(layout) - assert_same_items( - patch.changes, - [ - { - "op": "add", - "path": "/children", - "value": [ - {"tagName": "div", "children": ["hello"]}, - {"tagName": "", "error": ""}, - {"tagName": "div", "children": ["hello"]}, - ], - }, - {"op": "add", "path": "/tagName", "value": "div"}, - ], - ) + raise ValueError("error from bad child") + + with assert_idom_logged( + match_error="error from bad child", + clear_matched_records=True, + ): + + with idom.Layout(Main()) as layout: + patch = await render_json_patch(layout) + assert_same_items( + patch.changes, + [ + { + "op": "add", + "path": "/children", + "value": [ + {"tagName": "div", "children": ["hello"]}, + {"tagName": "", "error": ""}, + {"tagName": "div", "children": ["hello"]}, + ], + }, + {"op": "add", "path": "/tagName", "value": "div"}, + ], + ) async def test_render_raw_vdom_dict_with_single_component_object_as_children(): @@ -622,11 +641,13 @@ def ComponentReturnsDuplicateKeys(): idom.html.div(key="duplicate"), idom.html.div(key="duplicate") ) - with idom.Layout(ComponentReturnsDuplicateKeys()) as layout: - await layout.render() - - with pytest.raises(ValueError, match=r"Duplicate keys \['duplicate'\] at '/'"): - raise next(iter(caplog.records)).exc_info[1] + with assert_idom_logged( + error_type=ValueError, + match_error=r"Duplicate keys \['duplicate'\] at '/'", + clear_matched_records=True, + ): + with idom.Layout(ComponentReturnsDuplicateKeys()) as layout: + await layout.render() async def test_keyed_components_preserve_hook_on_parent_update(): @@ -652,7 +673,7 @@ def Inner(): assert old_inner_hook is inner_hook.latest -async def test_log_error_on_bad_event_handler(caplog): +async def test_log_error_on_bad_event_handler(): bad_handler = StaticEventHandler() @idom.component @@ -663,14 +684,15 @@ def raise_error(): return idom.html.button({"onClick": raise_error}) - with idom.Layout(ComponentWithBadEventHandler()) as layout: - await layout.render() - event = LayoutEvent(bad_handler.target, []) - await layout.deliver(event) + with assert_idom_logged( + match_error="bad event handler", + clear_matched_records=True, + ): - assert next(iter(caplog.records)).message.startswith( - "Failed to execute event handler" - ) + with idom.Layout(ComponentWithBadEventHandler()) as layout: + await layout.render() + event = LayoutEvent(bad_handler.target, []) + await layout.deliver(event) async def test_schedule_render_from_unmounted_hook(caplog): @@ -689,31 +711,29 @@ def Child(state): idom.hooks.use_effect(lambda: lambda: print("unmount", state)) return idom.html.div(state) - with idom.Layout(Parent()) as layout: - await layout.render() - - old_hook = child_hook.latest + with assert_idom_logged( + r"Did not render component with model state ID .*? - component already unmounted", + ): + with idom.Layout(Parent()) as layout: + await layout.render() - # cause initial child to be unmounted - parent_set_state.current(2) - await layout.render() + old_hook = child_hook.latest - # trigger render for hook that's been unmounted - old_hook.schedule_render() + # cause initial child to be unmounted + parent_set_state.current(2) + await layout.render() - # schedule one more render just to make it so `layout.render()` doesn't hang - # when the scheduled render above gets skipped - parent_set_state.current(3) + # trigger render for hook that's been unmounted + old_hook.schedule_render() - await layout.render() + # schedule one more render just to make it so `layout.render()` doesn't hang + # when the scheduled render above gets skipped + parent_set_state.current(3) - assert re.match( - r"Did not render component with model state ID .*? - component already unmounted", - caplog.records[0].message, - ) + await layout.render() -async def test_layout_element_cannot_become_a_component(caplog): +async def test_layout_element_cannot_become_a_component(): set_child_type = idom.Ref() @idom.component @@ -730,18 +750,21 @@ def Child(): "component": Child(key="the-same-key"), } - with idom.Layout(Root()) as layout: - await layout.render() + with assert_idom_logged( + error_type=ValueError, + match_error="prior element with this key wasn't a component", + clear_matched_records=True, + ): - set_child_type.current("component") + with idom.Layout(Root()) as layout: + await layout.render() - await layout.render() + set_child_type.current("component") - error = caplog.records[0].exc_info[1] - assert "prior element with this key wasn't a component" in str(error) + await layout.render() -async def test_layout_component_cannot_become_an_element(caplog): +async def test_layout_component_cannot_become_an_element(): set_child_type = idom.Ref() @idom.component @@ -758,12 +781,50 @@ def Child(): "component": Child(key="the-same-key"), } - with idom.Layout(Root()) as layout: + with assert_idom_logged( + error_type=ValueError, + match_error="prior element with this key was a component", + clear_matched_records=True, + ): + + with idom.Layout(Root()) as layout: + await layout.render() + + set_child_type.current("element") + + await layout.render() + + +async def test_layout_does_not_copy_element_children_by_key(): + # this is a regression test for a subtle bug: + # https://github.com/idom-team/idom/issues/556 + + set_items = idom.Ref() + + @idom.component + def SomeComponent(): + items, set_items.current = idom.use_state([1, 2, 3]) + return idom.html.div( + [ + idom.html.div( + idom.html.input({"onChange": lambda event: None}), + key=str(i), + ) + for i in items + ] + ) + + with idom.Layout(SomeComponent()) as layout: + await layout.render() + + set_items.current([2, 3]) + await layout.render() - set_child_type.current("element") + set_items.current([3]) await layout.render() - error = caplog.records[0].exc_info[1] - assert "prior element with this key was a component" in str(error) + set_items.current([]) + + await layout.render() From 314dcff913ea6ec0c17a9417f93c513873efa090 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 3 Jan 2022 21:00:13 -0800 Subject: [PATCH 3/9] fail after 3 errors in CI --- .github/workflows/test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 17f0fc5f5..82c00bc93 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -27,7 +27,7 @@ jobs: run: pip install -r requirements/nox-deps.txt - name: Run Tests env: { "CI": "true" } - run: nox -s test_python_suite -- --headless + run: nox -s test_python_suite -- --headless --maxfail=3 test-python-environments: runs-on: ${{ matrix.os }} strategy: @@ -48,7 +48,7 @@ jobs: run: pip install -r requirements/nox-deps.txt - name: Run Tests env: { "CI": "true" } - run: nox -s test_python -- --headless --no-cov + run: nox -s test_python -- --headless --maxfail=3 --no-cov test-docs: runs-on: ubuntu-latest steps: From 48d2fd7a704a8c73122d4fee2e675c83f3b37a7f Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 3 Jan 2022 21:04:14 -0800 Subject: [PATCH 4/9] require jsonpatch 1.32 --- requirements/pkg-deps.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/pkg-deps.txt b/requirements/pkg-deps.txt index 1fc2e4ab9..f4d440e32 100644 --- a/requirements/pkg-deps.txt +++ b/requirements/pkg-deps.txt @@ -1,6 +1,6 @@ typing-extensions >=3.10 mypy-extensions >=0.4.3 anyio >=3.0 -jsonpatch >=1.26 +jsonpatch >=1.32 fastjsonschema >=2.14.5 requests >=2.0 From 85a2a466e623a9afc7074077d67b15cbbc39d4bc Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 3 Jan 2022 21:18:19 -0800 Subject: [PATCH 5/9] fix misc things about jsonpatch correction --- src/idom/core/_fixed_jsonpatch.py | 31 +++++++++++++++++++++---------- src/idom/core/dispatcher.py | 2 +- src/idom/testing.py | 3 ++- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/idom/core/_fixed_jsonpatch.py b/src/idom/core/_fixed_jsonpatch.py index f571ff128..702bc4dc6 100644 --- a/src/idom/core/_fixed_jsonpatch.py +++ b/src/idom/core/_fixed_jsonpatch.py @@ -1,3 +1,5 @@ +# type: ignore + """A patched version of jsonpatch We need this because of: https://github.com/stefankoegl/python-json-patch/issues/138 @@ -9,28 +11,37 @@ from jsonpatch import _ST_REMOVE from jsonpatch import DiffBuilder as _DiffBuilder from jsonpatch import JsonPatch as _JsonPatch -from jsonpatch import RemoveOperation, _path_join +from jsonpatch import RemoveOperation, _path_join, basestring +from jsonpointer import JsonPointer -def apply_patch(doc, patch, in_place=False): - if isinstance(patch, (str, bytes)): - patch = JsonPatch.from_string(patch) +def apply_patch(doc, patch, in_place=False, pointer_cls=JsonPointer): + if isinstance(patch, basestring): + patch = JsonPatch.from_string(patch, pointer_cls=pointer_cls) else: - patch = JsonPatch(patch) + patch = JsonPatch(patch, pointer_cls=pointer_cls) return patch.apply(doc, in_place) -def make_patch(src, dst): - return JsonPatch.from_diff(src, dst) +def make_patch(src, dst, pointer_cls=JsonPointer): + return JsonPatch.from_diff(src, dst, pointer_cls=pointer_cls) class JsonPatch(_JsonPatch): @classmethod - def from_diff(cls, src, dst, optimization=True): - builder = DiffBuilder() + def from_diff( + cls, + src, + dst, + optimization=True, + dumps=None, + pointer_cls=JsonPointer, + ): + json_dumper = dumps or cls.json_dumper + builder = DiffBuilder(src, dst, json_dumper, pointer_cls=pointer_cls) builder._compare_values("", None, src, dst) ops = list(builder.execute()) - return cls(ops) + return cls(ops, pointer_cls=pointer_cls) class DiffBuilder(_DiffBuilder): diff --git a/src/idom/core/dispatcher.py b/src/idom/core/dispatcher.py index 04fed20b4..2768879ad 100644 --- a/src/idom/core/dispatcher.py +++ b/src/idom/core/dispatcher.py @@ -22,7 +22,7 @@ from idom.utils import Ref -from ._fixed_jsonpatch import apply_patch, make_patch +from ._fixed_jsonpatch import apply_patch, make_patch # type: ignore from .layout import LayoutEvent, LayoutUpdate from .proto import LayoutType, VdomJson diff --git a/src/idom/testing.py b/src/idom/testing.py index a8e71da32..0b63ac250 100644 --- a/src/idom/testing.py +++ b/src/idom/testing.py @@ -202,9 +202,10 @@ def assert_idom_logged( message_pattern.findall(record.getMessage()) # error type matches and ( - not error_type + error_type is None or ( record.exc_info is not None + and record.exc_info[0] is not None and issubclass(record.exc_info[0], error_type) ) ) From b0aa08125e29aded9b4ff875ffbaf51d642cbdfc Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 3 Jan 2022 21:39:56 -0800 Subject: [PATCH 6/9] fix coverage --- setup.cfg | 1 + src/idom/testing.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 5d1bb1235..b1c314036 100644 --- a/setup.cfg +++ b/setup.cfg @@ -34,6 +34,7 @@ exclude_lines = raise NotImplementedError omit = src/idom/__main__.py + src/idom/core/_fixed_jsonpatch.py [build_sphinx] all-files = true diff --git a/src/idom/testing.py b/src/idom/testing.py index 0b63ac250..24592208e 100644 --- a/src/idom/testing.py +++ b/src/idom/testing.py @@ -224,7 +224,7 @@ def assert_idom_logged( if clear_matched_records: handler.records.remove(record) - if not found: + if not found: # pragma: no cover conditions = [] if match_message: conditions.append(f"log message pattern {match_message!r}") From 06ba6cb4bdf6f34c6773298146d3a669ec5982d1 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 3 Jan 2022 22:22:41 -0800 Subject: [PATCH 7/9] capture logged error --- tests/test_core/test_layout.py | 44 ++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/tests/test_core/test_layout.py b/tests/test_core/test_layout.py index 46a52c731..08df5e7b3 100644 --- a/tests/test_core/test_layout.py +++ b/tests/test_core/test_layout.py @@ -142,25 +142,33 @@ def OkChild(): @idom.component def BadChild(): - raise ValueError("Something went wrong :(") + raise ValueError("error from bad child") - with idom.Layout(Main()) as layout: - patch = await render_json_patch(layout) - assert_same_items( - patch.changes, - [ - { - "op": "add", - "path": "/children", - "value": [ - {"tagName": "div", "children": ["hello"]}, - {"tagName": "", "error": "ValueError: Something went wrong :("}, - {"tagName": "div", "children": ["hello"]}, - ], - }, - {"op": "add", "path": "/tagName", "value": "div"}, - ], - ) + with assert_idom_logged( + match_error="error from bad child", + clear_matched_records=True, + ): + + with idom.Layout(Main()) as layout: + patch = await render_json_patch(layout) + assert_same_items( + patch.changes, + [ + { + "op": "add", + "path": "/children", + "value": [ + {"tagName": "div", "children": ["hello"]}, + { + "tagName": "", + "error": "ValueError: error from bad child", + }, + {"tagName": "div", "children": ["hello"]}, + ], + }, + {"op": "add", "path": "/tagName", "value": "div"}, + ], + ) @pytest.mark.skipif( From ed21673a34aaede0445bff2f2e3ec561cafb19e0 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 3 Jan 2022 23:22:19 -0800 Subject: [PATCH 8/9] stop on first error in CI matrix --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 82c00bc93..657d7749a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -48,7 +48,7 @@ jobs: run: pip install -r requirements/nox-deps.txt - name: Run Tests env: { "CI": "true" } - run: nox -s test_python -- --headless --maxfail=3 --no-cov + run: nox -s test_python --stop-on-first-error -- --headless --maxfail=3 --no-cov test-docs: runs-on: ubuntu-latest steps: From 0c314fa90474f1237809ec22516ed02255be3a6f Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 3 Jan 2022 23:36:14 -0800 Subject: [PATCH 9/9] try to re-run failed tests --- noxfile.py | 2 ++ requirements/test-env.txt | 5 +++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/noxfile.py b/noxfile.py index c55a7515f..4e868cc90 100644 --- a/noxfile.py +++ b/noxfile.py @@ -181,6 +181,8 @@ def test_python_suite(session: Session) -> None: install_requirements_file(session, "test-env") posargs = session.posargs + posargs += ["--reruns", "3", "--reruns-delay", "1"] + if "--no-cov" in session.posargs: session.log("Coverage won't be checked") session.install(".[all]") diff --git a/requirements/test-env.txt b/requirements/test-env.txt index 6fd1686da..c277b335c 100644 --- a/requirements/test-env.txt +++ b/requirements/test-env.txt @@ -1,8 +1,9 @@ +ipython pytest pytest-asyncio pytest-cov pytest-mock +pytest-rerunfailures pytest-timeout -selenium -ipython responses +selenium