Skip to content

Conversation

oroztocil
Copy link
Member

@oroztocil oroztocil commented Sep 8, 2025

This test is failing in CI and when run locally without debugger. The reason is that in the headless mode the window width is too small and the sidebar UI is hidden. This causes visibility-sensitive properties such as IWebElement.Text to return empty strings for hidden elements from the sidebar, causing assertions to fail.

The PR solves the issue by explicitly setting the window size for this particular test. We can discuss if the preferred solution is to set the window size globally when creating the test browser in the headless mode. Alternatively, the test could be fixed by using a visibility-independent API such as IWebElement.GetDomProperty("innerText") for reading the relevant values. However, using a realistic window size seems more appropriate for an E2E test.

The PR also adds the IWebDriver.SetWindowSize extension method. A few existing cases where window size is manipulated were refactored to use this method in order to make such cases more discoverable.

Fixes #54497

…ow size explictly, add SetWindowSize extension method
@Copilot Copilot AI review requested due to automatic review settings September 8, 2025 14:21
@oroztocil oroztocil requested a review from a team as a code owner September 8, 2025 14:21
Copy link
Contributor

@Copilot Copilot AI left a 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 fixes a failing test in headless mode by addressing viewport size issues that cause sidebar elements to be hidden, leading to empty text properties in assertions.

Key changes:

  • Adds a SetWindowSize extension method for IWebDriver to standardize window size manipulation
  • Sets explicit window size (1920x1080) in the failing NavMenuHighlightsCurrentLocation test
  • Refactors existing window size manipulations to use the new extension method

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/Shared/E2ETesting/WebDriverExtensions.cs Adds new SetWindowSize extension method for consistent window size management
src/Components/test/E2ETest/Tests/StandaloneAppTest.cs Sets explicit window size to prevent sidebar visibility issues in headless mode
src/Components/test/E2ETest/Tests/ComponentRenderingTestBase.cs Refactors three existing tests to use the new SetWindowSize extension method

@github-actions github-actions bot added the area-blazor Includes: Blazor, Razor Components label Sep 8, 2025
Copy link
Member

@ilonatommy ilonatommy left a comment

Choose a reason for hiding this comment

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

I do not see any tests that would manipulate with window size or would rely on a specific view, so setting it globally for all sounds like a good idea.

@oroztocil
Copy link
Member Author

oroztocil commented Sep 8, 2025

I do not see any tests that would manipulate with window size or would rely on a specific view, so setting it globally for all sounds like a good idea.

There are few in src/Components/test/E2ETest/Tests/ComponentRenderingTestBase.cs - the one's that I updated to use the new extension method. However, if I understand it correctly, there would be no problem to have a default window size explicitly set because these tests will override the size (just as they do now). So the question is if there are some tests that implicitly rely on no value being set by default.

// Without setting the window size explicitly, visibility-sensitive properties
// such as IWebElement.Text can return empty strings, causing assertions to fail.
// In particular, this happens in the headless mode (used when running without debugger).
Browser.SetWindowSize(1920, 1080);
Copy link
Member

Choose a reason for hiding this comment

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

I assume this doesn't fail if the physical screen size is lower than this right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Based on docs and empirical testing:

  • In GUI mode the size is clamped so that the window fits into the screen.
  • In headless mode the window size is set to whatever is specified and the rest of the test executes fine. (I tried 10000x10000 and had no issues.)

@oroztocil oroztocil force-pushed the oroztocil/54497-fix-NavMenuHighlightsCurrentLocation branch from 27c3f2f to 46df082 Compare September 9, 2025 15:41
@oroztocil
Copy link
Member Author

We also decided to add a mechanism for resetting the window size between individual tests to prevent unexpected dependencies between tests in the future.

@oroztocil
Copy link
Member Author

/backport to release/10.0

Copy link
Contributor

Started backporting to release/10.0: https://github.com/dotnet/aspnetcore/actions/runs/17613935455

@oroztocil oroztocil merged commit abbcda2 into main Sep 10, 2025
30 checks passed
@oroztocil oroztocil deleted the oroztocil/54497-fix-NavMenuHighlightsCurrentLocation branch September 10, 2025 20:34
@dotnet-policy-service dotnet-policy-service bot added this to the 11.0-preview1 milestone Sep 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-blazor Includes: Blazor, Razor Components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quarantine NavMenuHighlightsCurrentLocation
4 participants