Migrate integration tests from Selenium to Playwright#6378
Migrate integration tests from Selenium to Playwright#6378
Conversation
Every AppHarness-based integration test now uses pytest-playwright's `page` fixture in place of Selenium's WebDriver. The Selenium frontend/polling helpers are removed from `reflex.testing.AppHarness`, the `selenium` dev dependency is dropped, and `APP_HARNESS_HEADLESS`/`APP_HARNESS_DRIVER(_ARGS)` env vars are deleted. `tests_playwright/` is flattened back into `tests/integration/`, and the CI workflow collapses its selenium + playwright jobs into a single `uv run pytest tests/integration` invocation. `tests/integration/utils.py` is rewritten for Playwright (`LocalStorage`, `SessionStorage`, `poll_for_token`, `poll_for_navigation`, `poll_assert_event_order`, `poll_assert_relative_event_order`) with the same API surface as before so call sites only need a `page` instead of a `driver`. AGENTS.md and CONTRIBUTING.md now document Playwright as the sole integration framework and require `scope="module"` on every `AppHarness`-returning fixture. https://claude.ai/code/session_01B2zzr5B8FE4R5ePeQaetaZ
…aywright-migration-7qluV # Conflicts: # tests/integration/test_linked_state.py
Greptile SummaryThis PR completes a full migration of the integration test suite from Selenium WebDriver to Playwright's sync API, removing
Confidence Score: 4/5Mostly safe migration but the asyncio/playwright event-loop compatibility in the merged CI job should be confirmed before merging. The Selenium-to-Playwright migration itself is thorough and correct; the P1 concern is specifically about whether pytest-asyncio and pytest-playwright can share an event loop in the same session, which the old code explicitly avoided via separate jobs. If the author confirms CI passes end-to-end, the score would move to 5. .github/workflows/integration_app_harness.yml (removed job isolation), and any test file that is both async def and uses the page fixture (e.g. test_client_storage.py, test_upload.py, test_connection_banner.py). Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph OLD["Old CI Structure"]
J1["integration-app-harness\n(Selenium tests)\npytest --ignore=tests_playwright"]
J2["integration-app-harness-playwright\n(Playwright tests)\npytest tests_playwright/"]
end
subgraph NEW["New CI Structure"]
J3["integration-app-harness\n(All tests: Playwright only)\npytest tests/integration --splits 2"]
J3a["split group 1"]
J3b["split group 2"]
J3 --> J3a
J3 --> J3b
end
subgraph CONCERN["Compatibility Concern"]
PW["pytest-playwright\n(session-scoped event loop)"]
PA["pytest-asyncio\nasyncio_mode=auto"]
PW -. "potential conflict" .-> PA
end
OLD --> NEW
J3 --> CONCERN
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| - name: Run app harness tests | ||
| env: | ||
| REFLEX_REDIS_URL: ${{ matrix.state_manager == 'redis' && 'redis://localhost:6379' || '' }} | ||
| run: uv run pytest tests/integration/tests_playwright --reruns 3 -v --maxfail=5 | ||
| run: uv run pytest tests/integration --reruns 3 -v --maxfail=5 --splits 2 --group ${{matrix.split_index}} |
There was a problem hiding this comment.
The old workflow had an explicit comment and a separate CI job for Playwright tests because:
"The pytest-playwright plugin keeps an asyncio event loop running on the main thread for the entire session, which is incompatible with pytest-asyncio tests."
Several tests (e.g., test_client_storage.py::test_client_side_state, test_upload.py::test_upload_file_multiple, test_connection_banner.py::test_connection_banner) are both async and request Playwright's page fixture. With asyncio_mode = "auto" and asyncio_default_fixture_loop_scope = "function", pytest-asyncio creates a per-test event loop; if pytest-playwright's sync wrapper also drives an event loop on the main thread during the same session, you may still hit the original conflict. Please confirm these tests pass end-to-end in CI before merging.
| suffix = "_secondary" if secondary else "" | ||
|
|
||
| upload_box = driver.find_elements(By.XPATH, "//input[@type='file']")[ | ||
| 1 if secondary else 0 | ||
| ] | ||
| assert upload_box | ||
| upload_button = driver.find_element(By.ID, f"upload_button{suffix}") | ||
| assert upload_button | ||
| upload_box = page.locator("input[type='file']").nth(1 if secondary else 0) |
There was a problem hiding this comment.
The single-file case no longer normalizes the filename before asserting, while the multi-file tests still apply Path(name).name. The old assertion was Path(selected_files.text).name == Path(exp_name).name, which strips any OS path prefix. For browsers the file.name property is path-free so this is unlikely to fail in practice, but the inconsistency between the two cases is fragile.
| suffix = "_secondary" if secondary else "" | |
| upload_box = driver.find_elements(By.XPATH, "//input[@type='file']")[ | |
| 1 if secondary else 0 | |
| ] | |
| assert upload_box | |
| upload_button = driver.find_element(By.ID, f"upload_button{suffix}") | |
| assert upload_button | |
| upload_box = page.locator("input[type='file']").nth(1 if secondary else 0) | |
| selected_files = page.locator(f"#selected_files{suffix}") | |
| assert Path(selected_files.text_content() or "").name == Path(exp_name).name |
Merging this PR will degrade performance by 6.23%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | test_compile_stateful[_complicated_page] |
595.6 µs | 635.2 µs | -6.23% |
Comparing claude/selenium-to-playwright-migration-7qluV (16ff9dd) with main (1a64caa)
Unit tests import `attrs` but it was not declared in the dev dependency group. https://claude.ai/code/session_01B2zzr5B8FE4R5ePeQaetaZ
pytest-asyncio's event-loop runner and pytest-playwright's sync driver can't share the main thread — mixing `@pytest.mark.asyncio` with the sync `page` fixture trips teardown with "Cannot run the event loop while another loop is running" (warned now, error in future pytest-asyncio releases). All tests that touch the Playwright `page` fixture are now plain `def` tests: `asyncio.sleep` becomes `time.sleep`, and the one test that needs an async Redis client (`test_connection_banner`) wraps its `await redis.get(...)` calls in a small `asyncio.run` helper and switches its fixture to a sync `@pytest.fixture` that also runs `redis.aclose()` via `asyncio.run`. The app factory functions (which legitimately define `async def` event handlers) are untouched. https://claude.ai/code/session_01B2zzr5B8FE4R5ePeQaetaZ
Drop the `asyncio.run` wrappers around `redis.get`/`redis.aclose` and switch the `redis` fixture to `get_redis_sync()` with `redis.Redis.close()` teardown. https://claude.ai/code/session_01B2zzr5B8FE4R5ePeQaetaZ
pytest-playwright's sync driver owns the main-thread event loop for the whole session; pytest-asyncio's scoped runner tries to initialise its own and races with it at teardown even when no `async def test_` is collected. None of the integration tests need pytest-asyncio after the Playwright migration, so disable the plugin for this invocation. https://claude.ai/code/session_01B2zzr5B8FE4R5ePeQaetaZ
pre-commit uses ruff 0.15.8, which treats `async def` with only a `yield` (i.e. an async generator used by @asynccontextmanager) as not using async features; newer ruff 0.15.11 got smarter and no longer flags them. Add `# noqa: RUF029` on the five affected functions so the hook passes on whichever ruff version CI picks up. https://claude.ai/code/session_01B2zzr5B8FE4R5ePeQaetaZ
- `-p no:asyncio` alone may still trip on `asyncio_mode = "auto"` in pyproject.toml; add `-o asyncio_mode=strict` to override it too. - `playwright install chromium --only-shell` doesn't fetch the OS libraries chromium needs; add `--with-deps` so a fresh ubuntu-22.04 runner has them. https://claude.ai/code/session_01B2zzr5B8FE4R5ePeQaetaZ
- main's ruff does not flag RUF029 on @asynccontextmanager+yield functions, so adding `# noqa: RUF029` just creates RUF100 "unused noqa" errors on CI's ruff. Match main exactly for the 5 affected functions. - Drop `--with-deps` from the playwright install step (main's split- Playwright job never needed it). Drop the redundant `-o asyncio_mode=strict`; `-p no:asyncio` alone is sufficient. https://claude.ai/code/session_01B2zzr5B8FE4R5ePeQaetaZ
- `page.context.cookies()` returns Playwright's TypedDict ``Cookie``
where `name` is not required; switch to ``cookie_info.get("name", "")``.
- In `test_linked_state.tab_factory`, the inner ``factory`` closure
lost the ``linked_state.frontend_url is not None`` narrowing; capture
the URL in a local variable before defining the closure so pyright
keeps the non-None type.
https://claude.ai/code/session_01B2zzr5B8FE4R5ePeQaetaZ
Print pytest/playwright versions and the Playwright browser cache contents before running tests so we can see what's actually failing when the 16-job matrix all exits with code 1 and no log is readable through the unauthenticated Actions UI. https://claude.ai/code/session_01B2zzr5B8FE4R5ePeQaetaZ
`playwright.__version__` isn't an attribute on the module, so the previous diagnostics step bombed before the test step ran. Drop that, use `uv pip list` / `uv run pytest --version` / `uv run playwright --version` instead, and mark the step continue-on-error so it can't swallow the actual pytest run. https://claude.ai/code/session_01B2zzr5B8FE4R5ePeQaetaZ
|
Thanks for the review. In the future, you don't need a separate issue tracking a change unless the code has merged to main. Just a comment on the PR/follow on PR targeting the branch is fine. 👍 |
3 participants
Type of change
Description
This PR migrates the integration test suite from Selenium WebDriver to Playwright, modernizing the browser automation framework used for testing. The changes include:
Key Changes:
sync_api(Page, Locator, expect)AppHarnessto remove Selenium-specific methods (frontend(),poll_for_value()) and WebDriver managementpagefixture instead of custom WebDriver fixturesfind_element(By.ID, "id")→page.locator("#id")element.send_keys()→locator.fill()element.click()→locator.click()driver.execute_script()→page.evaluate()driver.get_cookies()→page.context.cookies()driver.delete_all_cookies()→page.context.clear_cookies()LocalStorage,SessionStorage) to work with Playwright pagespoll_for_token()utility function to replace Selenium-based token pollingpyproject.tomldependenciesAPP_HARNESS_HEADLESSenvironment variable from CI workflows (Playwright handles headless mode differently)Files Modified:
reflex/testing.py: Removed Selenium WebDriver support and related infrastructuretests/integration/utils.py: Updated storage helpers and added Playwright-compatible utilitiestests/integration/test_*.py: Updated all integration tests to use Playwright.github/workflows/: Removed Selenium-specific environment variablespyproject.toml: Removed Selenium dependencyCONTRIBUTING.md: Added integration test documentationTesting
All integration tests have been updated and should pass with Playwright. The test suite now uses Playwright's native fixtures and APIs, which provide better performance and reliability compared to Selenium.
Migration Notes
Tests now rely on Playwright's built-in
pagefixture (provided bypytest-playwright). The fixture automatically manages browser lifecycle and context. Tests that previously created multiple WebDriver instances now usepage.context.new_page()to create additional pages within the same browser context.https://claude.ai/code/session_01B2zzr5B8FE4R5ePeQaetaZ