Find/Replace overlay: add tests for its interplay with the host editor - #4292
Open
HeikoKlare wants to merge 1 commit into
Open
Find/Replace overlay: add tests for its interplay with the host editor#4292HeikoKlare wants to merge 1 commit into
HeikoKlare wants to merge 1 commit into
Conversation
Contributor
The overlay's existing tests drive it on a bare text viewer, which has no workbench part behind it. Everything about how the overlay and its host editor compete for a keystroke depends on there being a part, so none of it is currently covered: the overlay could start letting the editor act on keys typed into its input fields without any test noticing. Adds an end-to-end test that opens a real editor and reaches the overlay through FindReplaceAction, the way a user does. It observes only where a command ends up taking effect, never how that is arranged, so that it keeps describing what the overlay owes its users regardless of how the arbitration is implemented: keys typed into an input field do not act on the document, Select All applies to the focused field, Enter finds in the search field and replaces in the replace one, window commands such as Save still reach the editor, the editor stays the active part, and it acts on its keys again once the overlay loses focus. Key strokes are delivered by notifying the widget rather than by posting native events, so the display filter the key binding dispatcher installs is exercised without depending on the operating system to deliver anything. Because that path is easy to get wrong in a way that would make the test vacuous, the test also asserts that a key the overlay does bind takes effect. A real editor is needed but the workspace and the IDE are not, so the test contributes a minimal editor of its own rather than opening a file. Extensions from this bundle were previously ignored because it was not marked as a singleton. Assisted-by: Claude Opus 5 <noreply@anthropic.com>
HeikoKlare
force-pushed
the
findreplace-overlay-editor-tests
branch
from
August 28, 2026 16:29
91f86cc to
f40e654
Compare
There was a problem hiding this comment.
Pull request overview
Adds an end-to-end UI regression test to ensure the Find/Replace overlay correctly arbitrates key bindings and command routing when hosted inside a real editor part (i.e., the editor remains the active workbench part while overlay text fields have focus).
Changes:
- Introduces
FindReplaceOverlayInEditorTest, which opens a realStatusTextEditor, opens the overlay viaFindReplaceAction, and asserts where commands/keystrokes take effect (overlay vs. editor vs. window). - Adds a minimal in-memory
StatusTextEditor+IEditorInputand registers it viaplugin.xmlso the test can run without workspace/IDE dependencies. - Wires the new test into the existing JUnit suite and updates test bundle metadata/resources to include and load the new extension.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/WorkbenchTextEditorTestSuite.java | Registers the new end-to-end overlay-in-editor test in the suite. |
| tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/TestTextEditorInput.java | Adds an in-memory editor input used by the minimal test editor. |
| tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/TestTextEditor.java | Adds a minimal StatusTextEditor backed by an in-memory document provider for UI testing. |
| tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayInEditorTest.java | New end-to-end test covering command/keybinding arbitration between overlay fields and host editor. |
| tests/org.eclipse.ui.workbench.texteditor.tests/plugin.xml | Registers the minimal test editor via the org.eclipse.ui.editors extension point. |
| tests/org.eclipse.ui.workbench.texteditor.tests/META-INF/MANIFEST.MF | Marks the test bundle as a singleton (to ensure extension metadata is read as intended). |
| tests/org.eclipse.ui.workbench.texteditor.tests/build.properties | Ensures plugin.xml is included in the built test bundle. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
HeikoKlare
marked this pull request as ready for review
August 28, 2026 17:09
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
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.
Why
The Find/Replace overlay is drawn inside its host editor's widget tree, so focusing one of its input fields does not change the active workbench part. The editor stays active, and its key bindings and command handlers therefore compete for every keystroke with the field the user is typing into. Getting that wrong is very visible: Ctrl+Z undoing in the document instead of in the search field, Ctrl+A selecting the whole file, Delete editing the document.
This has gone wrong repeatedly since the overlay was introduced, and every time it was noticed by someone trying it out rather than by a test, because nothing covers it:
FindReplaceOverlayTestdrives the overlay on a bareTextViewer, which has no workbench part behind it, so the situation the whole mechanism exists for never arises there.What this adds
FindReplaceOverlayInEditorTest, an end-to-end test that opens a real editor and reaches the overlay throughFindReplaceAction, the way a user does. It asserts only where a command ends up taking effect, never how that is arranged, so it stays valid across changes to the implementation.It passes against unmodified production code: this is a regression guard, not a specification of new behaviour, and there is no production change in this PR.
Why now
The overlay is currently restricted to
StatusTextEditor:FindReplaceAction#shouldUseOverlay()only uses it for editors of that type, andFindReplaceOverlayitself branches on it in two places. Subsequent changes are intended to remove that restriction so the overlay can serve other editors, for instance the compare editor, which still opens the modal Find/Replace dialog (#2498).Lifting that restriction means changing how the overlay and its host editor arbitrate commands, which is exactly the behaviour described above. Adding the guard first means those changes, and any later ones in this area, can be judged against behaviour that is already pinned down rather than re-verified by hand.
Notes on the test setup
A real editor part is needed but the workspace and the IDE are not, so the test contributes a minimal editor of its own; marking the test bundle as a singleton is what makes its
plugin.xmlread at all. Key strokes are notified to the widget rather than posted as native events, which still runs the display filter the key binding dispatcher installs but does not depend on the operating system, so the test also works headless.🤖 Generated with Claude Code