Skip to content

Conversation

tartansandal
Copy link
Contributor

@tartansandal tartansandal commented Aug 14, 2025

Introduces an integration test for #5674.
This xfails on main but passes on #5698

Edit: also fixed an apparent bug in test_link_hover.py. There may be a deeper issue here.
Edit: removed xfail after merging main which now includes #5698.
Edit: added a test for 404s.

Copy link

codspeed-hq bot commented Aug 14, 2025

CodSpeed Performance Report

Merging #5712 will not alter performance

Comparing tartansandal:kjh/integration-tests-for-frontend-path (bbe8847) with main (5f3bf56)

Summary

✅ 8 untouched

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Summary

This PR introduces integration testing for frontend path functionality, specifically addressing issue #5674 where on_load and on_mount handlers fail to redirect properly when Reflex applications are served from non-root frontend paths (e.g., '/prefix').

The changes include two main components:

  1. Environment Variable Addition: A new REFLEX_FRONTEND_PATH environment variable is added to the EnvironmentVariables class in reflex/environment.py. This string variable defaults to an empty string and provides the configuration mechanism for specifying the base path under which the frontend is served.

  2. Comprehensive Integration Test: A new test file tests/integration/tests_playwright/test_frontend_path.py creates a complete end-to-end test using Playwright. The test creates a Reflex app with redirect pages and validates that redirects work correctly both in normal conditions and when served from a non-root frontend path.

The integration test uses two fixtures - one for production mode and another that sets the REFLEX_FRONTEND_PATH to '/prefix'. It employs parametrized testing to verify both scenarios, with the prefix case marked as xfail since it demonstrates the known bug that should be resolved by the related PR #5698.

This change fits into the broader Reflex ecosystem by enabling proper deployment behind reverse proxies, a common pattern where multiple applications are served from different paths on the same domain. The environment variable provides a clean configuration interface that other parts of the codebase can reference to adjust their behavior for non-root deployments.

Confidence score: 4/5

  • This PR introduces well-structured integration testing with minimal risk to existing functionality
  • Score reflects solid test design and environment variable implementation, though the xfail test indicates known issues
  • Pay close attention to the test file to ensure proper cleanup of environment variables and test isolation

2 files reviewed, 2 comments

Edit Code Review Bot Settings | Greptile

@tartansandal
Copy link
Contributor Author

Note I'm using simple environment module tweaks to simulate changing the environment. An alternative would be to modify AppHarness to take a config object. That would be a much larger change, but might be useful testing some configuration scenarios.

@tartansandal tartansandal changed the title Add an integration tests for frontend path Add integration tests for frontend path Aug 17, 2025
@tartansandal tartansandal force-pushed the kjh/integration-tests-for-frontend-path branch from 619abbb to e5d9f9b Compare August 26, 2025 02:44
@tartansandal tartansandal force-pushed the kjh/integration-tests-for-frontend-path branch from e5d9f9b to 26c3be7 Compare September 30, 2025 05:26
@tartansandal tartansandal force-pushed the kjh/integration-tests-for-frontend-path branch from a8e571d to 26c3be7 Compare October 2, 2025 03:02
@tartansandal tartansandal force-pushed the kjh/integration-tests-for-frontend-path branch from 26c3be7 to 02650f3 Compare October 2, 2025 03:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant