From cba6993247f4727a53303128acc83a076ac4c842 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Brand=C3=A9ho?= Date: Thu, 24 Oct 2024 14:53:42 -0700 Subject: [PATCH 1/5] test for stateless apps (#3816) * test for stateless apps * add playwright to dev dependencies * fix docstring * fix install of playwright in CI * fix install again * add allowed license * add retry on running integrations step * another attempt to fix licensing issue * update timeout duration for retry * fix timeout workflows * remove dep changes * remove outdated diff * fix scope in new test and workflow for appharness * run playwright tests last --- .github/workflows/integration_app_harness.yml | 1 + .../tests_playwright/test_stateless_app.py | 59 +++++++++++++++++++ 2 files changed, 60 insertions(+) create mode 100644 tests/integration/tests_playwright/test_stateless_app.py diff --git a/.github/workflows/integration_app_harness.yml b/.github/workflows/integration_app_harness.yml index 9644e0a19..c86893556 100644 --- a/.github/workflows/integration_app_harness.yml +++ b/.github/workflows/integration_app_harness.yml @@ -51,6 +51,7 @@ jobs: SCREENSHOT_DIR: /tmp/screenshots REDIS_URL: ${{ matrix.state_manager == 'redis' && 'redis://localhost:6379' || '' }} run: | + poetry run playwright install --with-deps poetry run pytest tests/integration - uses: actions/upload-artifact@v4 name: Upload failed test screenshots diff --git a/tests/integration/tests_playwright/test_stateless_app.py b/tests/integration/tests_playwright/test_stateless_app.py new file mode 100644 index 000000000..129b69dc7 --- /dev/null +++ b/tests/integration/tests_playwright/test_stateless_app.py @@ -0,0 +1,59 @@ +"""Integration tests for a stateless app.""" + +from typing import Generator + +import httpx +import pytest +from playwright.sync_api import Page, expect + +import reflex as rx +from reflex.testing import AppHarness + + +def StatelessApp(): + """A stateless app that renders a heading.""" + import reflex as rx + + def index(): + return rx.heading("This is a stateless app") + + app = rx.App() + app.add_page(index) + + +@pytest.fixture(scope="module") +def stateless_app(tmp_path_factory) -> Generator[AppHarness, None, None]: + """Create a stateless app AppHarness. + + Args: + tmp_path_factory: pytest fixture for creating temporary directories. + + Yields: + AppHarness: A harness for testing the stateless app. + """ + with AppHarness.create( + root=tmp_path_factory.mktemp("stateless_app"), + app_source=StatelessApp, # type: ignore + ) as harness: + yield harness + + +def test_statelessness(stateless_app: AppHarness, page: Page): + """Test that the stateless app renders a heading but backend/_event is not mounted. + + Args: + stateless_app: A harness for testing the stateless app. + page: A Playwright page. + """ + assert stateless_app.frontend_url is not None + assert stateless_app.backend is not None + assert stateless_app.backend.started + + res = httpx.get(rx.config.get_config().api_url + "/_event") + assert res.status_code == 404 + + res2 = httpx.get(rx.config.get_config().api_url + "/ping") + assert res2.status_code == 200 + + page.goto(stateless_app.frontend_url) + expect(page.get_by_role("heading")).to_have_text("This is a stateless app") From d85236b9b09e40f1c24fe510fbe90019eca04f9e Mon Sep 17 00:00:00 2001 From: Masen Furer Date: Thu, 24 Oct 2024 16:19:32 -0700 Subject: [PATCH 2/5] [ENG-3970] When normal pickle fails, try dill (#4239) * [ENG-3970] When normal pickle fails, try dill If dill is not installed, suggest that the user `pip install` it. Fix #4147 * re-lock depenedencies * Include original pickle error message for better debugging When the pickling throws a warning and dill is not installed, include the original pickle error. Add a test case for an object that even dill cannot pickle to ensure error path is hit as expected. * py3.9 compatibility --- poetry.lock | 21 ++++++++++++++++++--- pyproject.toml | 1 + reflex/state.py | 20 ++++++++++++++++---- tests/units/test_state.py | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 67 insertions(+), 7 deletions(-) diff --git a/poetry.lock b/poetry.lock index ff9323ad4..71be30d76 100644 --- a/poetry.lock +++ b/poetry.lock @@ -521,6 +521,21 @@ files = [ {file = "darglint-1.8.1.tar.gz", hash = "sha256:080d5106df149b199822e7ee7deb9c012b49891538f14a11be681044f0bb20da"}, ] +[[package]] +name = "dill" +version = "0.3.9" +description = "serialize all of Python" +optional = false +python-versions = ">=3.8" +files = [ + {file = "dill-0.3.9-py3-none-any.whl", hash = "sha256:468dff3b89520b474c0397703366b7b95eebe6303f108adf9b19da1f702be87a"}, + {file = "dill-0.3.9.tar.gz", hash = "sha256:81aa267dddf68cbfe8029c42ca9ec6a4ab3b22371d1c450abc54422577b4512c"}, +] + +[package.extras] +graph = ["objgraph (>=1.7.2)"] +profile = ["gprof2dot (>=2022.7.29)"] + [[package]] name = "distlib" version = "0.3.9" @@ -1333,8 +1348,8 @@ files = [ [package.dependencies] numpy = [ - {version = ">=1.26.0", markers = "python_version >= \"3.12\""}, {version = ">=1.23.2", markers = "python_version == \"3.11\""}, + {version = ">=1.26.0", markers = "python_version >= \"3.12\""}, {version = ">=1.22.4", markers = "python_version < \"3.11\""}, ] python-dateutil = ">=2.8.2" @@ -1652,8 +1667,8 @@ files = [ annotated-types = ">=0.6.0" pydantic-core = "2.23.4" typing-extensions = [ - {version = ">=4.12.2", markers = "python_version >= \"3.13\""}, {version = ">=4.6.1", markers = "python_version < \"3.13\""}, + {version = ">=4.12.2", markers = "python_version >= \"3.13\""}, ] [package.extras] @@ -3033,4 +3048,4 @@ type = ["pytest-mypy"] [metadata] lock-version = "2.0" python-versions = "^3.9" -content-hash = "8090ccaeca173bd8612e17a0b8d157d7492618e49450abd1c8373e2976349db0" +content-hash = "e03374b85bf10f0a7bb857969b2d6714f25affa63e14a48a88be9fa154b24326" diff --git a/pyproject.toml b/pyproject.toml index 93f3c5d50..2635e1156 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -65,6 +65,7 @@ pytest = ">=7.1.2,<9.0" pytest-mock = ">=3.10.0,<4.0" pyright = ">=1.1.229,<1.1.335" darglint = ">=1.8.1,<2.0" +dill = ">=0.3.8" toml = ">=0.10.2,<1.0" pytest-asyncio = ">=0.24.0" pytest-cov = ">=4.0.0,<6.0" diff --git a/reflex/state.py b/reflex/state.py index e3e189b22..6e229b97d 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -2063,12 +2063,24 @@ class BaseState(Base, ABC, extra=pydantic.Extra.allow): """ try: return pickle.dumps((self._to_schema(), self)) - except pickle.PicklingError: - console.warn( + except (pickle.PicklingError, AttributeError) as og_pickle_error: + error = ( f"Failed to serialize state {self.get_full_name()} due to unpicklable object. " - "This state will not be persisted." + "This state will not be persisted. " ) - return b"" + try: + import dill + + return dill.dumps((self._to_schema(), self)) + except ImportError: + error += ( + f"Pickle error: {og_pickle_error}. " + "Consider `pip install 'dill>=0.3.8'` for more exotic serialization support." + ) + except (pickle.PicklingError, TypeError, ValueError) as ex: + error += f"Dill was also unable to pickle the state: {ex}" + console.warn(error) + return b"" @classmethod def _deserialize( diff --git a/tests/units/test_state.py b/tests/units/test_state.py index 544ddc606..89dd1fd3d 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -3364,3 +3364,35 @@ async def test_deserialize_gc_state_disk(token): assert s.num == 43 c = await root.get_state(Child) assert c.foo == "bar" + + +class Obj(Base): + """A object containing a callable for testing fallback pickle.""" + + _f: Callable + + +def test_fallback_pickle(): + """Test that state serialization will fall back to dill.""" + + class DillState(BaseState): + _o: Optional[Obj] = None + _f: Optional[Callable] = None + _g: Any = None + + state = DillState(_reflex_internal_init=True) # type: ignore + state._o = Obj(_f=lambda: 42) + state._f = lambda: 420 + + pk = state._serialize() + + unpickled_state = BaseState._deserialize(pk) + assert unpickled_state._f() == 420 + assert unpickled_state._o._f() == 42 + + # Some object, like generator, are still unpicklable with dill. + state._g = (i for i in range(10)) + pk = state._serialize() + assert len(pk) == 0 + with pytest.raises(EOFError): + BaseState._deserialize(pk) From 3fba4101e74f90c8a367579fcece79bf09c1eb29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Brand=C3=A9ho?= Date: Fri, 25 Oct 2024 09:43:24 -0700 Subject: [PATCH 3/5] convert test_table to use playwright (#4241) * convert test_table to use playwright * clean up test --- .../{ => tests_playwright}/test_table.py | 75 +++++++------------ 1 file changed, 29 insertions(+), 46 deletions(-) rename tests/integration/{ => tests_playwright}/test_table.py (57%) diff --git a/tests/integration/test_table.py b/tests/integration/tests_playwright/test_table.py similarity index 57% rename from tests/integration/test_table.py rename to tests/integration/tests_playwright/test_table.py index 09004a874..917247b89 100644 --- a/tests/integration/test_table.py +++ b/tests/integration/tests_playwright/test_table.py @@ -3,10 +3,18 @@ from typing import Generator import pytest -from selenium.webdriver.common.by import By +from playwright.sync_api import Page from reflex.testing import AppHarness +expected_col_headers = ["Name", "Age", "Location"] +expected_row_headers = ["John", "Jane", "Joe"] +expected_cells_data = [ + ["30", "New York"], + ["31", "San Fransisco"], + ["32", "Los Angeles"], +] + def Table(): """App using table component.""" @@ -17,11 +25,6 @@ def Table(): @app.add_page def index(): return rx.center( - rx.input( - id="token", - value=rx.State.router.session.client_token, - is_read_only=True, - ), rx.table.root( rx.table.header( rx.table.row( @@ -53,7 +56,7 @@ def Table(): @pytest.fixture() -def table(tmp_path_factory) -> Generator[AppHarness, None, None]: +def table_app(tmp_path_factory) -> Generator[AppHarness, None, None]: """Start Table app at tmp_path via AppHarness. Args: @@ -71,47 +74,27 @@ def table(tmp_path_factory) -> Generator[AppHarness, None, None]: yield harness -@pytest.fixture -def driver(table: AppHarness): - """GEt an instance of the browser open to the table app. - - Args: - table: harness for Table app - - Yields: - WebDriver instance. - """ - driver = table.frontend() - try: - token_input = driver.find_element(By.ID, "token") - assert token_input - # wait for the backend connection to send the token - token = table.poll_for_value(token_input) - assert token is not None - - yield driver - finally: - driver.quit() - - -def test_table(driver, table: AppHarness): +def test_table(page: Page, table_app: AppHarness): """Test that a table component is rendered properly. Args: - driver: Selenium WebDriver open to the app - table: Harness for Table app + table_app: Harness for Table app + page: Playwright page instance """ - assert table.app_instance is not None, "app is not running" + assert table_app.frontend_url is not None, "frontend url is not available" - thead = driver.find_element(By.TAG_NAME, "thead") - # poll till page is fully loaded. - table.poll_for_content(element=thead) - # check headers - assert thead.find_element(By.TAG_NAME, "tr").text == "Name Age Location" - # check first row value - assert ( - driver.find_element(By.TAG_NAME, "tbody") - .find_elements(By.TAG_NAME, "tr")[0] - .text - == "John 30 New York" - ) + page.goto(table_app.frontend_url) + table = page.get_by_role("table") + + # Check column headers + headers = table.get_by_role("columnheader").all_inner_texts() + assert headers == expected_col_headers + + # Check rows headers + rows = table.get_by_role("rowheader").all_inner_texts() + assert rows == expected_row_headers + + # Check cells + rows = table.get_by_role("cell").all_inner_texts() + for i, expected_row in enumerate(expected_cells_data): + assert [rows[idx := i * 2], rows[idx + 1]] == expected_row From f2bcb47986d3bffe8e796858e588a72588d515fe Mon Sep 17 00:00:00 2001 From: Masen Furer Date: Fri, 25 Oct 2024 11:10:30 -0700 Subject: [PATCH 4/5] Support `locale` prop for `rx.moment` (#4229) --- reflex/components/moment/moment.py | 34 +++++++++++++---------------- reflex/components/moment/moment.pyi | 8 ++++--- 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/reflex/components/moment/moment.py b/reflex/components/moment/moment.py index 51b09d55d..4ac835b35 100644 --- a/reflex/components/moment/moment.py +++ b/reflex/components/moment/moment.py @@ -3,10 +3,10 @@ import dataclasses from typing import List, Optional -from reflex.components.component import Component, NoSSRComponent +from reflex.components.component import NoSSRComponent from reflex.event import EventHandler, identity_event from reflex.utils.imports import ImportDict -from reflex.vars.base import Var +from reflex.vars.base import LiteralVar, Var @dataclasses.dataclass(frozen=True) @@ -92,6 +92,9 @@ class Moment(NoSSRComponent): # Display the date in the given timezone. tz: Var[str] + # The locale to use when rendering. + locale: Var[str] + # Fires when the date changes. on_change: EventHandler[identity_event(str)] @@ -101,22 +104,15 @@ class Moment(NoSSRComponent): Returns: The import dict for the component. """ + imports = {} + + if isinstance(self.locale, LiteralVar): + imports[""] = f"moment/locale/{self.locale._var_value}" + elif self.locale is not None: + # If the user is using a variable for the locale, we can't know the + # value at compile time so import all locales available. + imports[""] = "moment/min/locales" if self.tz is not None: - return {"moment-timezone": ""} - return {} + imports["moment-timezone"] = "" - @classmethod - def create(cls, *children, **props) -> Component: - """Create a Moment component. - - Args: - *children: The children of the component. - **props: The properties of the component. - - Returns: - The Moment Component. - """ - comp = super().create(*children, **props) - if "tz" in props: - comp.lib_dependencies.append("moment-timezone") - return comp + return imports diff --git a/reflex/components/moment/moment.pyi b/reflex/components/moment/moment.pyi index ccffbb8d1..415d0f049 100644 --- a/reflex/components/moment/moment.pyi +++ b/reflex/components/moment/moment.pyi @@ -51,6 +51,7 @@ class Moment(NoSSRComponent): unix: Optional[Union[Var[bool], bool]] = None, local: Optional[Union[Var[bool], bool]] = None, tz: Optional[Union[Var[str], str]] = None, + locale: Optional[Union[Var[str], str]] = None, style: Optional[Style] = None, key: Optional[Any] = None, id: Optional[Any] = None, @@ -75,7 +76,7 @@ class Moment(NoSSRComponent): on_unmount: Optional[EventType[[]]] = None, **props, ) -> "Moment": - """Create a Moment component. + """Create the component. Args: *children: The children of the component. @@ -99,15 +100,16 @@ class Moment(NoSSRComponent): unix: Tells Moment to parse the given date value as a unix timestamp. local: Outputs the result in local time. tz: Display the date in the given timezone. + locale: The locale to use when rendering. style: The style of the component. key: A unique key for the component. id: The id for the component. class_name: The class name for the component. autofocus: Whether the component should take the focus once the page is loaded custom_attrs: custom attribute - **props: The properties of the component. + **props: The props of the component. Returns: - The Moment Component. + The component. """ ... From 6341846cea0edf9d48687cf1f4afe47bb8436632 Mon Sep 17 00:00:00 2001 From: Masen Furer Date: Fri, 25 Oct 2024 11:43:55 -0700 Subject: [PATCH 5/5] Include value._get_all_var_data when ClientStateVar.set_value is used (#4161) If `set_value` is called with a State var as the argument, ensure that its context is brought into scope. --- reflex/experimental/client_state.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/reflex/experimental/client_state.py b/reflex/experimental/client_state.py index 698829d33..a8566eafb 100644 --- a/reflex/experimental/client_state.py +++ b/reflex/experimental/client_state.py @@ -178,9 +178,12 @@ class ClientStateVar(Var): if self._global_ref else self._setter_name ) + _var_data = VarData(imports=_refs_import if self._global_ref else {}) if value is not NoValue: # This is a hack to make it work like an EventSpec taking an arg - value_str = str(LiteralVar.create(value)) + value_var = LiteralVar.create(value) + _var_data = _var_data.merge(value_var._get_all_var_data()) + value_str = str(value_var) if value_str.startswith("_"): # remove patterns of ["*"] from the value_str using regex @@ -190,7 +193,7 @@ class ClientStateVar(Var): setter = f"(() => {setter}({value_str}))" return Var( _js_expr=setter, - _var_data=VarData(imports=_refs_import if self._global_ref else {}), + _var_data=_var_data, ).to(FunctionVar, EventChain) @property