Add E2E tests for MPA V2 #8743
Add E2E tests for MPA V2 #8743
Conversation
| def test_titles_are_set_correctly(app: Page): | ||
| """Test that page titles work as expected""" | ||
| nav = app.get_by_test_id("stSidebarNav") | ||
| expect(nav.locator("a").nth(0)).to_contain_text("page 2") |
There was a problem hiding this comment.
nit: instead of doing nav.locator("a").nth(0) in every test, we have started to use a pattern like:
def _get_page2_link(app: Page) -> Locator:
return nav.locator("a").nth(0)
# etc.(see here)
and use these convenient functions in the tests. It makes them easier to read and if the app ever is modified, e.g. new elements are added, then you have to change only a single line.
Additionally, it might make sense to move the expects such as
expect(nav.locator("a").nth(0)).to_contain_text("page 2")
expect(nav.locator("a").nth(1)).to_contain_text("Different Title")
expect(nav.locator("a").nth(2)).to_contain_text("page 5")
expect(nav.locator("a").nth(3)).to_contain_text("page 9")into a convenient function and call that from every test that checks for it instead of duplicating it.
| def test_switch_page_by_path(app: Page): | ||
| """Test that we can switch between pages by triggering st.switch_page with a path.""" | ||
|
|
||
| app.get_by_test_id("stButton").nth(0).locator("button").first.click() |
There was a problem hiding this comment.
I would suggest to use a filter at places like this to get the correct button, filter(has_text="page 5"). This makes it much easier to read the test and understand what exactly is supposed to happen
|
Thanks for adding e2e tests! It could make sense to add also one that mixes strategy v1 and v2 to make sure that this works as expected too end-to-end. What do you think? |
| # Expand the nav | ||
| view_button.click(force=True) | ||
| # We apply a quick timeout here so that the UI has some time to | ||
| # adjust for the screenshot after the click | ||
| themed_app.wait_for_timeout(250) | ||
| assert_snapshot( | ||
| themed_app.get_by_test_id("stSidebarNav"), name="mpa-sidebar_nav_expanded" | ||
| ) | ||
|
|
||
| # Collapse the nav | ||
| view_button.click(force=True) | ||
| themed_app.wait_for_timeout(250) | ||
| assert_snapshot( | ||
| themed_app.get_by_test_id("stSidebarNav"), name="mpa-sidebar_nav_collapsed" | ||
| ) | ||
|
|
||
| # Expand the nav again | ||
| view_button.click(force=True) | ||
| themed_app.wait_for_timeout(250) | ||
| assert_snapshot( | ||
| themed_app.get_by_test_id("stSidebarNav"), name="mpa-sidebar_nav_expanded" | ||
| ) |
There was a problem hiding this comment.
nit: could we wait on some other thing such as expected pages appearing at the bottom of the list being present/absent so that we don't have to have a hard coded number here?
| def test_switch_page_removes_query_params(page: Page, app_port: int): | ||
| """Test that query params are removed when navigating via st.switch_page""" | ||
|
|
||
| # Start at main page with query params | ||
| page.goto(f"http://localhost:{app_port}/?foo=bar") | ||
| wait_for_app_loaded(page) | ||
|
|
||
| # Trigger st.switch_page | ||
| page.get_by_test_id("baseButton-secondary").filter(has_text="page 5").click() | ||
| wait_for_app_loaded(page) | ||
| # Check that query params don't persist | ||
| expect(page).to_have_url(f"http://localhost:{app_port}/page_5") | ||
|
|
||
|
|
||
| def test_removes_query_params_when_swapping_pages(page: Page, app_port: int): | ||
| """Test that query params are removed when swapping pages""" | ||
|
|
||
| page.goto(f"http://localhost:{app_port}/page_7?foo=bar") | ||
| wait_for_app_loaded(page) | ||
|
|
||
| get_page_link(page, "page 4").click() | ||
| wait_for_app_loaded(page) | ||
| expect(page).to_have_url(f"http://localhost:{app_port}/page_4") |
There was a problem hiding this comment.
nit: might be worth more loudly calling out the difference between these two tests as it took a second for me to realize how they're not duplicates
Describe your changes
E2E tests for MPA v2 in many of the positive use cases.
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.