From 8ed21c558a141d21f5ccbe25a553b0ccc699ea5c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 08:24:11 +0000 Subject: [PATCH 1/3] Fix stale element flake in test_on_load_navigate_non_dynamic `poll_for_navigation` returns as soon as the URL changes, but the client side router swaps the route component after that, replacing the DOM nodes. Since index and /static/x render the same component, a link located right after navigating back to index could go stale before the click was dispatched, raising StaleElementReferenceException. Add a `click_element` helper that re-locates the element on every attempt and use it for the link clicks in the dynamic route navigation tests. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01W7hxgQtxcFzZC795xyftVS --- tests/integration/test_dynamic_routes.py | 18 ++++------- tests/integration/utils.py | 39 +++++++++++++++++++++++- 2 files changed, 44 insertions(+), 13 deletions(-) diff --git a/tests/integration/test_dynamic_routes.py b/tests/integration/test_dynamic_routes.py index 7c3e7641f86..4e02a75a382 100644 --- a/tests/integration/test_dynamic_routes.py +++ b/tests/integration/test_dynamic_routes.py @@ -12,7 +12,7 @@ from reflex.testing import AppHarness, WebDriver -from .utils import poll_assert_event_order, poll_for_navigation +from .utils import click_element, poll_assert_event_order, poll_for_navigation def DynamicRoute(): @@ -300,9 +300,8 @@ def test_on_load_navigate( # next/link to a 404 and ensure we still hydrate exp_order += ["/404-no page id"] - link = driver.find_element(By.ID, "link_missing") with poll_for_navigation(driver): - link.click() + click_element(driver, By.ID, "link_missing") # hit a page that redirects back to dynamic page exp_order += ["on_load_redir-{'foo': 'bar', 'page_id': '0'}", "/page/[page_id]-0"] @@ -330,29 +329,24 @@ def test_on_load_navigate_non_dynamic( driver: WebDriver instance. """ assert dynamic_route.app_instance is not None - link = driver.find_element(By.ID, "link_page_x") - assert link with poll_for_navigation(driver): - link.click() + click_element(driver, By.ID, "link_page_x") assert urlsplit(driver.current_url).path.removesuffix("/") == "/static/x" poll_assert_event_order(driver, ["/static/x-no page id"]) # go back to the index and navigate back to the static route - link = driver.find_element(By.ID, "link_index") with poll_for_navigation(driver): - link.click() + click_element(driver, By.ID, "link_index") assert urlsplit(driver.current_url).path.removesuffix("/") == "" - link = driver.find_element(By.ID, "link_page_x") with poll_for_navigation(driver): - link.click() + click_element(driver, By.ID, "link_page_x") assert urlsplit(driver.current_url).path.removesuffix("/") == "/static/x" poll_assert_event_order(driver, ["/static/x-no page id", "/static/x-no page id"]) for _ in range(3): - link = driver.find_element(By.ID, "link_page_x") - link.click() + click_element(driver, By.ID, "link_page_x") assert urlsplit(driver.current_url).path.removesuffix("/") == "/static/x" poll_assert_event_order(driver, ["/static/x-no page id"] * 5) diff --git a/tests/integration/utils.py b/tests/integration/utils.py index 9b8931ee604..55cb7b41c74 100644 --- a/tests/integration/utils.py +++ b/tests/integration/utils.py @@ -10,7 +10,7 @@ from selenium.webdriver.common.by import By from selenium.webdriver.remote.webdriver import WebDriver -from reflex.testing import AppHarness +from reflex.testing import AppHarness, TimeoutType def request_raw( @@ -64,6 +64,43 @@ def poll_for_navigation( AppHarness.expect(lambda: prev_url != driver.current_url, timeout=timeout) +def click_element( + driver: WebDriver, + by: str, + value: str, + timeout: TimeoutType = None, +) -> None: + """Locate an element and click it, retrying until the click lands. + + Client-side navigation swaps the DOM after the URL changes, so an element + located right after navigating can go stale before the click is dispatched. + Re-locating on each attempt clicks whichever node is currently rendered. + + Args: + driver: WebDriver instance. + by: Locator strategy, one of the `By` constants. + value: Locator value. + timeout: Time to wait for the click to succeed. + + Raises: + TimeoutError: if the element could not be clicked within the timeout. + """ + last_exc: Exception | None = None + + def _click() -> bool: + nonlocal last_exc + try: + driver.find_element(by, value).click() + except Exception as exc: + last_exc = exc + raise + return True + + if not AppHarness._poll_for(_click, timeout=timeout): + msg = f"Could not click element {by}={value!r} while polling: {last_exc}" + raise TimeoutError(msg) + + def n_expected_events(exp_event_order: Sequence[str | set[str]]) -> int: """Calculate the number of expected events, accounting for sets in the expected order. From 46d814f9ba59b5524ac9c72f04b725e3619bb418 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 02:10:30 +0000 Subject: [PATCH 2/3] Narrow click_element retries to stale references Review feedback: retrying on any exception could re-dispatch a click that had already landed (the exact-count event assertions would then flake) and made a bad locator wait out the full timeout. Retry only on StaleElementReferenceException, which chromedriver raises before the click reaches the browser, and let every other error propagate immediately. Retrying NoSuchElementException in particular would be costly here: this module sets a 30s implicit wait, so a missing element already blocks that long per lookup. Poll in a plain loop instead of AppHarness._poll_for, which suppresses all exceptions and required carrying the last one out via nonlocal. Also convert the remaining find-then-click-across-navigation sites in test_dynamic_routes.py, which have the same stale-reference shape. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01W7hxgQtxcFzZC795xyftVS --- tests/integration/test_dynamic_routes.py | 19 +++--------- tests/integration/utils.py | 38 +++++++++++++----------- 2 files changed, 24 insertions(+), 33 deletions(-) diff --git a/tests/integration/test_dynamic_routes.py b/tests/integration/test_dynamic_routes.py index 4e02a75a382..e2630345bb3 100644 --- a/tests/integration/test_dynamic_routes.py +++ b/tests/integration/test_dynamic_routes.py @@ -235,24 +235,18 @@ def test_on_load_navigate( token: The token visible in the driver browser. """ assert dynamic_route.app_instance is not None - link = driver.find_element(By.ID, "link_page_next") - assert link exp_order = [f"/page/[page_id]-{ix}" for ix in range(10)] # click the link a few times for ix in range(10): # wait for navigation, then assert on url with poll_for_navigation(driver): - link.click() + click_element(driver, By.ID, "link_page_next") assert urlsplit(driver.current_url).path == f"/page/{ix}" - link = AppHarness.poll_for_or_raise_timeout( - lambda: driver.find_element(By.ID, "link_page_next") - ) page_id_input = driver.find_element(By.ID, "page_id") raw_path_input = driver.find_element(By.ID, "raw_path") - assert link assert page_id_input assert dynamic_route.poll_for_value( @@ -273,9 +267,8 @@ def test_on_load_navigate( # make sure internal nav still hydrates after redirect exp_order += ["/page/[page_id]-11"] - link = driver.find_element(By.ID, "link_page_next") with poll_for_navigation(driver): - link.click() + click_element(driver, By.ID, "link_page_next") poll_assert_event_order(driver, exp_order) # load same page with a query param and make sure it passes through @@ -393,19 +386,15 @@ def assert_content(expected: str, expect_not: str): ) assert_content("0", "") - next_page_link = driver.find_element(By.ID, "next-page") - assert next_page_link with poll_for_navigation(driver): - next_page_link.click() + click_element(driver, By.ID, "next-page") assert ( driver.current_url.removesuffix("/") == f"{frontend_url.removesuffix('/')}/arg/1" ) assert_content("1", "0") - next_page_link = driver.find_element(By.ID, "next-page") - assert next_page_link with poll_for_navigation(driver): - next_page_link.click() + click_element(driver, By.ID, "next-page") assert ( driver.current_url.removesuffix("/") == f"{frontend_url.removesuffix('/')}/arg/2" diff --git a/tests/integration/utils.py b/tests/integration/utils.py index 55cb7b41c74..3aa52e23362 100644 --- a/tests/integration/utils.py +++ b/tests/integration/utils.py @@ -2,15 +2,17 @@ from __future__ import annotations +import time from collections.abc import Generator, Iterator, Sequence from contextlib import contextmanager from http.client import HTTPConnection from urllib.parse import urlsplit +from selenium.common.exceptions import StaleElementReferenceException from selenium.webdriver.common.by import By from selenium.webdriver.remote.webdriver import WebDriver -from reflex.testing import AppHarness, TimeoutType +from reflex.testing import DEFAULT_TIMEOUT, POLL_INTERVAL, AppHarness, TimeoutType def request_raw( @@ -70,35 +72,35 @@ def click_element( value: str, timeout: TimeoutType = None, ) -> None: - """Locate an element and click it, retrying until the click lands. + """Locate an element and click it, re-locating it if it goes stale. Client-side navigation swaps the DOM after the URL changes, so an element - located right after navigating can go stale before the click is dispatched. - Re-locating on each attempt clicks whichever node is currently rendered. + located right after navigating can be unmounted before the click is + dispatched. Only a stale reference is retried: it is raised before the + click reaches the browser, so the click is never dispatched twice. Args: driver: WebDriver instance. by: Locator strategy, one of the `By` constants. value: Locator value. - timeout: Time to wait for the click to succeed. + timeout: How long to keep re-locating a stale element. Raises: - TimeoutError: if the element could not be clicked within the timeout. + TimeoutError: if the element remained stale for the whole timeout. """ - last_exc: Exception | None = None - - def _click() -> bool: - nonlocal last_exc + deadline = time.monotonic() + ( + DEFAULT_TIMEOUT if timeout is None else float(timeout) + ) + while True: try: driver.find_element(by, value).click() - except Exception as exc: - last_exc = exc - raise - return True - - if not AppHarness._poll_for(_click, timeout=timeout): - msg = f"Could not click element {by}={value!r} while polling: {last_exc}" - raise TimeoutError(msg) + except StaleElementReferenceException as exc: + if time.monotonic() >= deadline: + msg = f"Element {by}={value!r} remained stale while polling." + raise TimeoutError(msg) from exc + time.sleep(POLL_INTERVAL) + else: + return def n_expected_events(exp_event_order: Sequence[str | set[str]]) -> int: From a4be554d48b200a9b0ba3ef2283d9e3db2112853 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 02:21:29 +0000 Subject: [PATCH 3/3] Use click_element for the other stale-prone clicks in Selenium tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same failure shape as the dynamic route test: an element located right after a navigation, clicked once the route component has been swapped. - test_navigation: `external`/`external2`, located immediately after `driver.back()` and clicked later, with a window switch in between. - test_login_flow: `doit`, located as soon as the URL became /login; `login` and `logout` for consistency in the same flow. - test_event_chain: `unmount`, located right after `driver.get()` and clicked after `assert_token()` waits out hydration. These modules set no implicit wait and relied on a presence poll before the click, so click_element now retries NoSuchElementException as well as StaleElementReferenceException — both are raised while locating or validating the reference, never after the click is dispatched, so the click still cannot fire twice. InvalidSelectorException derives from WebDriverException rather than NoSuchElementException, so a bad locator still fails immediately. Left alone: single-expression `find_element(...).click()` calls (no window between locating and clicking) and clicks that never cross a navigation. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01W7hxgQtxcFzZC795xyftVS --- tests/integration/test_event_chain.py | 6 ++---- tests/integration/test_login_flow.py | 8 +++----- tests/integration/test_navigation.py | 15 ++++----------- tests/integration/utils.py | 23 ++++++++++++++--------- 4 files changed, 23 insertions(+), 29 deletions(-) diff --git a/tests/integration/test_event_chain.py b/tests/integration/test_event_chain.py index 3e19e20895e..2059095bf6d 100644 --- a/tests/integration/test_event_chain.py +++ b/tests/integration/test_event_chain.py @@ -10,6 +10,7 @@ from reflex.testing import AppHarness, WebDriver from tests.integration.utils import ( + click_element, poll_assert_event_order, poll_assert_relative_event_order, ) @@ -622,11 +623,8 @@ def test_event_chain_on_mount( assert event_chain.frontend_url is not None driver.get(event_chain.frontend_url.removesuffix("/") + uri) - unmount_button = AppHarness.poll_for_or_raise_timeout( - lambda: driver.find_element(By.ID, "unmount") - ) assert_token(event_chain, driver) - unmount_button.click() + click_element(driver, By.ID, "unmount") poll_assert_relative_event_order(driver, expected_counts, ordering_rules) diff --git a/tests/integration/test_login_flow.py b/tests/integration/test_login_flow.py index 17681370d62..306ba0b28b2 100644 --- a/tests/integration/test_login_flow.py +++ b/tests/integration/test_login_flow.py @@ -126,12 +126,11 @@ def test_login_flow( login_sample.poll_for_content(login_button) with utils.poll_for_navigation(driver): - login_button.click() + utils.click_element(driver, By.ID, "login") assert driver.current_url.endswith("/login") - do_it_button = driver.find_element(By.ID, "doit") with utils.poll_for_navigation(driver): - do_it_button.click() + utils.click_element(driver, By.ID, "doit") assert driver.current_url == login_sample.frontend_url def check_auth_token_header(): @@ -143,8 +142,7 @@ def check_auth_token_header(): assert AppHarness.poll_for_or_raise_timeout(check_auth_token_header) == "12345" - logout_button = driver.find_element(By.ID, "logout") - logout_button.click() + utils.click_element(driver, By.ID, "logout") state_name = login_sample.get_full_state_name(["_state"]) AppHarness.expect( diff --git a/tests/integration/test_navigation.py b/tests/integration/test_navigation.py index 8b78711a54f..56ed5ce00cf 100644 --- a/tests/integration/test_navigation.py +++ b/tests/integration/test_navigation.py @@ -8,7 +8,7 @@ from reflex.testing import AppHarness -from .utils import SessionStorage, poll_for_navigation +from .utils import SessionStorage, click_element, poll_for_navigation def NavigationApp(): @@ -69,26 +69,19 @@ def test_navigation_app(navigation_app: AppHarness): ss = SessionStorage(driver) assert AppHarness._poll_for(lambda: ss.get("token") is not None), "token not found" - internal_link = driver.find_element(By.ID, "internal") - with poll_for_navigation(driver): - internal_link.click() + click_element(driver, By.ID, "internal") assert urlsplit(driver.current_url).path == "/internal" with poll_for_navigation(driver): driver.back() - external_link = AppHarness.poll_for_or_raise_timeout( - lambda: driver.find_element(By.ID, "external") - ) - external2_link = driver.find_element(By.ID, "external2") - - external_link.click() + click_element(driver, By.ID, "external") # Expect a new tab to open AppHarness.expect(lambda: len(driver.window_handles) == 2) # Switch back to the main tab driver.switch_to.window(driver.window_handles[0]) - external2_link.click() + click_element(driver, By.ID, "external2") # Expect another new tab to open AppHarness.expect(lambda: len(driver.window_handles) == 3) diff --git a/tests/integration/utils.py b/tests/integration/utils.py index 3aa52e23362..ce9d4c4deb5 100644 --- a/tests/integration/utils.py +++ b/tests/integration/utils.py @@ -8,7 +8,10 @@ from http.client import HTTPConnection from urllib.parse import urlsplit -from selenium.common.exceptions import StaleElementReferenceException +from selenium.common.exceptions import ( + NoSuchElementException, + StaleElementReferenceException, +) from selenium.webdriver.common.by import By from selenium.webdriver.remote.webdriver import WebDriver @@ -72,21 +75,23 @@ def click_element( value: str, timeout: TimeoutType = None, ) -> None: - """Locate an element and click it, re-locating it if it goes stale. + """Locate an element and click it, tolerating the churn of a navigation. Client-side navigation swaps the DOM after the URL changes, so an element - located right after navigating can be unmounted before the click is - dispatched. Only a stale reference is retried: it is raised before the - click reaches the browser, so the click is never dispatched twice. + located right after navigating may not be rendered yet, or may be unmounted + before the click is dispatched. Both are retried until `timeout`, since both + are raised while locating or validating the element reference, never after + the click reaches the browser. Every other error, including an invalid + selector or an intercepted click, propagates on the first attempt. Args: driver: WebDriver instance. by: Locator strategy, one of the `By` constants. value: Locator value. - timeout: How long to keep re-locating a stale element. + timeout: How long to keep re-locating the element. Raises: - TimeoutError: if the element remained stale for the whole timeout. + TimeoutError: if the element could not be clicked within the timeout. """ deadline = time.monotonic() + ( DEFAULT_TIMEOUT if timeout is None else float(timeout) @@ -94,9 +99,9 @@ def click_element( while True: try: driver.find_element(by, value).click() - except StaleElementReferenceException as exc: + except (NoSuchElementException, StaleElementReferenceException) as exc: if time.monotonic() >= deadline: - msg = f"Element {by}={value!r} remained stale while polling." + msg = f"Could not click element {by}={value!r} while polling." raise TimeoutError(msg) from exc time.sleep(POLL_INTERVAL) else: