fix: make ContextMenuTester open and close the menu like a real user - #249
Merged
Merged
Conversation
ContextMenuTester.open() and close() wrote the menu's opened element property with a plain server-side setProperty, so the resulting OpenedChangeEvent reported isFromClient() as false. An application that branches on isFromClient() saw the tester's open as a programmatic one. The opened property is @Synchronize-d on ContextMenuBase#isOpened(), so the update now goes through setPropertyAsUser, the same helper Details and Accordion use for that property. The usability check in open() moves ahead of the property update: a client originated update is refused on a component that is not usable, so running the check first keeps the tester's own error message and leaves the menu closed when it throws.
The usability check in ContextMenuTester.open() now runs before the property update, so a menu that is not usable stays closed when open() throws instead of being left half-opened. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
open() fires the before-open DOM event, which attaches the menu content to the UI, before it can check usability: the menu is not attached until that event, so ensureComponentIsUsable() cannot run any earlier. A menu that turned out not to be usable was therefore left attached with opened=false, so a closed menu stayed reachable through a top level find(). The attach is now undone when the check refuses the open, making open() leave no trace when it throws. The test also pins the message of the refusal, so that moving the usability check back after the property update is caught: that would throw the generic "unable to simulate a client side update" instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Test Results1 536 tests 1 536 ✅ 44s ⏱️ Results for commit af86286. ♻️ This comment has been updated with latest results. |
GridContextMenuTester.open(int, String) pushed opened=true through the client path before checking that the menu is usable. A menu that is not usable was therefore left opened and attached, and a GridContextMenuOpenedEvent was fired for an open that then threw. The usability check now runs before the property update, and the attach done by the before-open event is undone when the check refuses the open, so a refused open leaves no trace. This mirrors ContextMenuTester.open(). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ters ContextMenuTester.open() and GridContextMenuTester.open(int, String) had the same try/catch undoing the attach done by the before-open event when the usability check refuses the open. The invariant now lives once, as ComponentTester#ensureComponentIsUsableOrDetach(), next to the check it wraps, so the two menu testers cannot drift apart. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The getItemTexts expectation was written against the item list as it was before a visible "Duplicated Hidden" item was appended to ContextMenuView, so it asserted 9 items where the menu now shows 10. Both changes pass on their own, which is why neither pull request caught it; together the expectation is stale. The tester is right: the appended item is visible, so it belongs in the texts a user sees. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mcollovati
approved these changes
Sep 18, 2026
mcollovati
pushed a commit
that referenced
this pull request
Sep 18, 2026
…249) (CP: 25.3) (#254) This PR cherry-picks changes from the original PR #249 to branch 25.3. --- #### Original PR description > ## Summary > `ContextMenuTester.open()` and `close()` changed the menu's `opened` property from the server, so the resulting `OpenedChangeEvent` said it did not come from the client. The tester now pushes that property through the client path, and a refused open no longer leaves the menu half-opened. > > Fixes #233 > > ## What changed > > **Behavior change (affects tests that open or close a `ContextMenu` or `GridContextMenu` through the testers):** > > - `OpenedChangeEvent` from `ContextMenuTester.open()` and `close()` now reports `isFromClient() == true`. Application code that branches on `isFromClient()` now takes the "user action" path, as it does in a real browser. Tests asserting `isFromClient() == false` for tester-driven opens will need updating. > - An `open()` that is refused because the menu is not usable (for example hidden) now leaves the menu closed and detached from the UI. Before, the menu was left with its content attached, so a top level `find(...)` could still reach items of a closed menu. > - `GridContextMenuTester.open(int, String)` now checks usability *before* it marks the menu as opened. A refused open no longer fires a `GridContextMenuOpenedEvent`, and it fails with the tester's own "is not usable" message instead of the generic "unable to simulate a client side update". > > Internal cleanup: the shared "undo the attach when the usability check refuses the open" logic now lives once in `ComponentTester#ensureComponentIsUsableOrDetach()`, used by both menu testers. This is additive for subclasses. > > Also fixes a stale test expectation: `getItemTexts()` now expects the visible "Duplicated Hidden" item that was appended to the test view, so the list is 10 items instead of 9. > > ## API Changes > > ### com.vaadin.browserless.ComponentTester > > ```java > // Added > protected void ensureComponentIsUsableOrDetach() // like ensureComponentIsUsable(), but detaches the component from the UI before rethrowing > ``` > > ## Test summary > - Opening and closing a context menu through the tester reports both opened-change events as user actions. > - Opening a context menu that is not usable fails, and leaves the menu closed, detached, and its content unreachable through a top level `find(...)`. > - The failure message for a refused open is the tester's "is not usable" message, not a generic client-update error. > - Opening a grid context menu on a row when the menu is not usable fails the same way and leaves no open menu behind. > - The list of visible item texts includes every item the user can see, in browser order. Co-authored-by: totally-not-ai[bot] <290682512+totally-not-ai[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ContextMenuTester.open()andclose()changed the menu'sopenedproperty from the server, so the resultingOpenedChangeEventsaid it did not come from the client. The tester now pushes that property through the client path, and a refused open no longer leaves the menu half-opened.Fixes #233
What changed
Behavior change (affects tests that open or close a
ContextMenuorGridContextMenuthrough the testers):OpenedChangeEventfromContextMenuTester.open()andclose()now reportsisFromClient() == true. Application code that branches onisFromClient()now takes the "user action" path, as it does in a real browser. Tests assertingisFromClient() == falsefor tester-driven opens will need updating.open()that is refused because the menu is not usable (for example hidden) now leaves the menu closed and detached from the UI. Before, the menu was left with its content attached, so a top levelfind(...)could still reach items of a closed menu.GridContextMenuTester.open(int, String)now checks usability before it marks the menu as opened. A refused open no longer fires aGridContextMenuOpenedEvent, and it fails with the tester's own "is not usable" message instead of the generic "unable to simulate a client side update".Internal cleanup: the shared "undo the attach when the usability check refuses the open" logic now lives once in
ComponentTester#ensureComponentIsUsableOrDetach(), used by both menu testers. This is additive for subclasses.Also fixes a stale test expectation:
getItemTexts()now expects the visible "Duplicated Hidden" item that was appended to the test view, so the list is 10 items instead of 9.API Changes
com.vaadin.browserless.ComponentTester
Test summary
find(...).