-
Notifications
You must be signed in to change notification settings - Fork 4k
[feat] Add external URL runner in e2e playwright #13678
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
[feat] Add external URL runner in e2e playwright #13678
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
✅ PR preview is ready!
|
9e9dfa9 to
24b4488
Compare
📉 Frontend coverage change detectedThe frontend unit test (vitest) coverage has decreased by 0.0000%
✅ Coverage change is within normal range. |
24b4488 to
07b02c8
Compare
SummaryThis PR adds the ability to run E2E Playwright tests against an externally hosted Streamlit app URL instead of spinning up a local server. It introduces:
Code QualityThe implementation is clean and follows existing patterns in the codebase. Key observations: Positives:
Issues:
Test Coverage
Backwards CompatibilityThis change is fully backwards compatible:
Security & Risk
Recommendations
# In conftest.py
config.addinivalue_line(
"markers",
"iframe_test: mark test as compatible with external app execution mode",
)
# In skip fixture
if external_app_url and request.node.get_closest_marker("iframe_test") is None:
VerdictCHANGES REQUESTED: The implementation is solid and the feature is useful, but the marker naming should be changed from This is an automated AI review using |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds support for running e2e playwright tests against externally hosted Streamlit apps instead of requiring a local server. This enables testing against production-like environments or remote deployments.
Changes:
- Added CLI options
--external-app-urland--browser-state-path(with corresponding environment variables) to configure external app testing - Created
app_base_urlfixture that returns either the external URL or local URL based on configuration - Modified
app_serverfixture to skip server startup when external mode is active - Added auto-skip logic to restrict external mode to tests marked with
@pytest.mark.iframeTest - Updated
test_no_console_errorsto useapp_base_urland marked it as external-compatible
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| e2e_playwright/conftest.py | Added fixtures and CLI options for external app URL and browser state, modified app_server to skip startup in external mode, added iframeTest marker and auto-skip logic |
| e2e_playwright/mega_tester_app_test.py | Updated test_no_console_errors to use app_base_url fixture and marked as iframeTest compatible |
Comments suppressed due to low confidence (1)
e2e_playwright/conftest.py:408
- The
static_appfixture at line 393 hardcodeshttp://localhost:{app_port}and doesn't support external app mode. Similar to theappfixture, this should either be updated to support external mode or there should be documentation explaining why it's excluded. Consider whether this fixture should useapp_base_urlor if static apps inherently can't work with external mode.
@pytest.fixture
def static_app(
page: Page,
app_port: int,
request: pytest.FixtureRequest,
) -> Page:
"""Fixture that opens the app."""
query_param = request.node.get_closest_marker("query_param")
query_string = query_param.args[0] if query_param else ""
# Indicate this is a StaticPage
page.__class__ = StaticPage
page.goto(f"http://localhost:{app_port}/{query_string}")
start_capture_traces(page)
wait_for_app_loaded(page)
return page
07b02c8 to
212927b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (1)
e2e_playwright/conftest.py:408
- The
static_appfixture uses hardcodedhttp://localhost:{app_port}instead of the newapp_base_urlfixture. This will prevent tests using this fixture from working with external apps. Consider updating this fixture to accept and useapp_base_urlparameter.
@pytest.fixture
def static_app(
page: Page,
app_port: int,
request: pytest.FixtureRequest,
) -> Page:
"""Fixture that opens the app."""
query_param = request.node.get_closest_marker("query_param")
query_string = query_param.args[0] if query_param else ""
# Indicate this is a StaticPage
page.__class__ = StaticPage
page.goto(f"http://localhost:{app_port}/{query_string}")
start_capture_traces(page)
wait_for_app_loaded(page)
return page
212927b to
6ca20fe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
e2e_playwright/conftest.py:454
- The
static_appfixture still usesapp_portdirectly to construct URLs. For consistency and to support external app testing, consider updating it to useapp_base_urlinstead, similar to how theappfixture was updated. This would allowstatic_appto work with externally hosted apps.
@pytest.fixture
def static_app(
page: Page,
app_port: int,
request: pytest.FixtureRequest,
) -> Page:
"""Fixture that opens the app."""
query_param = request.node.get_closest_marker("query_param")
query_string = query_param.args[0] if query_param else ""
# Indicate this is a StaticPage
page.__class__ = StaticPage
page.goto(f"http://localhost:{app_port}/{query_string}")
start_capture_traces(page)
wait_for_app_loaded(page)
return page
6ca20fe to
852ba18
Compare
852ba18 to
7a22365
Compare

Describe your changes
Added support for running E2E tests against externally hosted Streamlit apps by introducing two new command-line options:
--external-app-url: Allows tests to run against an external app URL instead of localhost--browser-state-path: Enables using a Playwright storage state JSON file for authenticationAdded a new
iframeTestmarker to identify tests that are compatible with external app execution mode, and implemented a fixture to skip non-compatible tests when running in external mode.Created helper functions to retrieve configuration from either command-line options or environment variables, and added fixtures to manage the app base URL and browser state.
Testing Plan
Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.