Skip to content

Commit 5a1635b

Browse files
HeikoKlareHeikoKlare
committed
Remove reflective access from find/replace tests #2060
The tests for the FindReplaceDialog and FindReplaceOverlay currently use reflection to access specific UI elements. This ties the test implementations to implementation details of the production classes (i.e., specific hidden field to be present) and particularly requires the production code to contain (hidden) fields even if they would not be required just to provide according tests. This change replaces the reflective access with widget extraction functionality based on explicit IDs assigned to the UI elements of interest. Fixes #2060
1 parent f9b743f commit 5a1635b

File tree

7 files changed

+218
-91
lines changed

7 files changed

+218
-91
lines changed

‎bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java‎

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,8 @@ private final class KeyboardShortcuts {
104104
KeyStroke.getInstance(SWT.MOD1, 'R'), KeyStroke.getInstance(SWT.MOD1, 'r'));
105105
}
106106

107+
public static final String ID_DATA_KEY = "org.eclipse.ui.internal.findreplace.overlay.FindReplaceOverlay.id"; //$NON-NLS-1$
108+
107109
private static final String REPLACE_BAR_OPEN_DIALOG_SETTING = "replaceBarOpen"; //$NON-NLS-1$
108110
private static final double WORST_CASE_RATIO_EDITOR_TO_OVERLAY = 0.95;
109111
private static final double BIG_WIDTH_RATIO_EDITOR_TO_OVERLAY = 0.7;
@@ -130,9 +132,9 @@ private final class KeyboardShortcuts {
130132
private ToolItem wholeWordSearchButton;
131133
private ToolItem caseSensitiveSearchButton;
132134
private ToolItem regexSearchButton;
133-
private ToolItem searchUpButton;
134-
private ToolItem searchDownButton;
135-
private ToolItem searchAllButton;
135+
private ToolItem searchBackwardButton;
136+
private ToolItem searchForwardButton;
137+
private ToolItem selectAllButton;
136138
private AccessibleToolBar closeTools;
137139
private ToolItem closeButton;
138140

@@ -370,6 +372,7 @@ public int open() {
370372
}
371373
overlayOpen = true;
372374
applyOverlayColors(backgroundToUse, true);
375+
assignIDs();
373376
updateFromTargetSelection();
374377
searchBar.forceFocus();
375378

@@ -391,6 +394,25 @@ private void restoreOverlaySettings() {
391394
}
392395
}
393396

397+
@SuppressWarnings("nls")
398+
private void assignIDs() {
399+
replaceToggle.setData(ID_DATA_KEY, "replaceToggle");
400+
searchBar.setData(ID_DATA_KEY, "searchInput");
401+
searchBackwardButton.setData(ID_DATA_KEY, "searchBackward");
402+
searchForwardButton.setData(ID_DATA_KEY, "searchForward");
403+
selectAllButton.setData(ID_DATA_KEY, "selectAll");
404+
searchInSelectionButton.setData(ID_DATA_KEY, "searchInSelection");
405+
wholeWordSearchButton.setData(ID_DATA_KEY, "wholeWordSearch");
406+
regexSearchButton.setData(ID_DATA_KEY, "regExSearch");
407+
caseSensitiveSearchButton.setData(ID_DATA_KEY, "caseSensitiveSearch");
408+
409+
if (replaceBarOpen) {
410+
replaceBar.setData(ID_DATA_KEY, "replaceInput");
411+
replaceButton.setData(ID_DATA_KEY, "replaceOne");
412+
replaceAllButton.setData(ID_DATA_KEY, "replaceAll");
413+
}
414+
}
415+
394416
private void applyOverlayColors(Color color, boolean tryToColorReplaceBar) {
395417
closeTools.setBackground(color);
396418
closeButton.setBackground(color);
@@ -400,9 +422,9 @@ private void applyOverlayColors(Color color, boolean tryToColorReplaceBar) {
400422
wholeWordSearchButton.setBackground(color);
401423
regexSearchButton.setBackground(color);
402424
caseSensitiveSearchButton.setBackground(color);
403-
searchAllButton.setBackground(color);
404-
searchUpButton.setBackground(color);
405-
searchDownButton.setBackground(color);
425+
selectAllButton.setBackground(color);
426+
searchBackwardButton.setBackground(color);
427+
searchForwardButton.setBackground(color);
406428

407429
searchBarContainer.setBackground(color);
408430
searchBar.setBackground(color);
@@ -511,20 +533,20 @@ private void createSearchTools() {
511533

512534
searchTools.createToolItem(SWT.SEPARATOR);
513535

514-
searchUpButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.PUSH)
536+
searchBackwardButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.PUSH)
515537
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_FIND_PREV))
516538
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_upSearchButton_toolTip)
517539
.withOperation(() -> performSearch(false))
518540
.withShortcuts(KeyboardShortcuts.SEARCH_BACKWARD).build();
519541

520-
searchDownButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.PUSH)
542+
searchForwardButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.PUSH)
521543
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_FIND_NEXT))
522544
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_downSearchButton_toolTip)
523545
.withOperation(() -> performSearch(true))
524546
.withShortcuts(KeyboardShortcuts.SEARCH_FORWARD).build();
525-
searchDownButton.setSelection(true); // by default, search down
547+
searchForwardButton.setSelection(true); // by default, search down
526548

527-
searchAllButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.PUSH)
549+
selectAllButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.PUSH)
528550
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_SEARCH_ALL))
529551
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_searchAllButton_toolTip)
530552
.withOperation(this::performSelectAll).withShortcuts(KeyboardShortcuts.SEARCH_ALL).build();
@@ -759,6 +781,7 @@ private void createReplaceDialog() {
759781

760782
updatePlacementAndVisibility();
761783
applyOverlayColors(backgroundToUse, true);
784+
assignIDs();
762785
replaceBar.forceFocus();
763786
}
764787

‎bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/FindReplaceDialog.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@
7979
*/
8080
class FindReplaceDialog extends Dialog {
8181

82+
public static final String ID_DATA_KEY = "org.eclipse.ui.texteditor.FindReplaceDialog.id"; //$NON-NLS-1$
83+
8284
private static final int CLOSE_BUTTON_ID = 101;
8385
private IFindReplaceLogic findReplaceLogic;
8486

@@ -275,6 +277,7 @@ public void create() {
275277
shell.setText(FindReplaceMessages.FindReplace_Dialog_Title);
276278

277279
updateButtonState();
280+
assignIDs();
278281
}
279282

280283
/**
@@ -1353,4 +1356,23 @@ private String getCurrentSelection() {
13531356
return null;
13541357
return target.getSelectionText();
13551358
}
1359+
1360+
@SuppressWarnings("nls")
1361+
private void assignIDs() {
1362+
fFindField.setData(ID_DATA_KEY, "searchInput");
1363+
fReplaceField.setData(ID_DATA_KEY, "replaceInput");
1364+
fForwardRadioButton.setData(ID_DATA_KEY, "searchForward");
1365+
fGlobalRadioButton.setData(ID_DATA_KEY, "globalSearch");
1366+
fSelectedRangeRadioButton.setData(ID_DATA_KEY, "searchInSelection");
1367+
fCaseCheckBox.setData(ID_DATA_KEY, "caseSensitiveSearch");
1368+
fWrapCheckBox.setData(ID_DATA_KEY, "wrappedSearch");
1369+
fWholeWordCheckBox.setData(ID_DATA_KEY, "wholeWordSearch");
1370+
fIncrementalCheckBox.setData(ID_DATA_KEY, "incrementalSearch");
1371+
fIsRegExCheckBox.setData(ID_DATA_KEY, "regExSearch");
1372+
1373+
fReplaceSelectionButton.setData(ID_DATA_KEY, "replaceOne");
1374+
fReplaceFindButton.setData(ID_DATA_KEY, "replaceFindOne");
1375+
fReplaceAllButton.setData(ID_DATA_KEY, "replaceAll");
1376+
}
1377+
13561378
}

‎tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/WidgetExtractor.java‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2024 Vector Informatik GmbH and others.
3+
*
4+
* This program and the accompanying materials
5+
* are made available under the terms of the Eclipse Public License 2.0
6+
* which accompanies this distribution, and is available at
7+
* https://www.eclipse.org/legal/epl-2.0/
8+
*
9+
* SPDX-License-Identifier: EPL-2.0
10+
*
11+
* Contributors:
12+
* Vector Informatik GmbH - initial API and implementation
13+
*******************************************************************************/
14+
package org.eclipse.ui.internal.findandreplace;
15+
16+
import static org.junit.Assert.assertFalse;
17+
18+
import java.util.ArrayList;
19+
import java.util.List;
20+
21+
import org.eclipse.swt.widgets.Button;
22+
import org.eclipse.swt.widgets.Combo;
23+
import org.eclipse.swt.widgets.Composite;
24+
import org.eclipse.swt.widgets.ToolBar;
25+
import org.eclipse.swt.widgets.ToolItem;
26+
import org.eclipse.swt.widgets.Widget;
27+
28+
import org.eclipse.ui.internal.findandreplace.overlay.HistoryTextWrapper;
29+
30+
public final class WidgetExtractor {
31+
32+
private final Composite rootContainer;
33+
34+
private final String idDataKey;
35+
36+
public WidgetExtractor(String idDataKey, Composite container) {
37+
this.idDataKey= idDataKey;
38+
this.rootContainer= container;
39+
}
40+
41+
public HistoryTextWrapper findHistoryTextWrapper(String id) {
42+
return findWidget(rootContainer, HistoryTextWrapper.class, id);
43+
}
44+
45+
public Combo findCombo(String id) {
46+
return findWidget(rootContainer, Combo.class, id);
47+
}
48+
49+
public Button findButton(String id) {
50+
return findWidget(rootContainer, Button.class, id);
51+
}
52+
53+
public ToolItem findToolItem(String id) {
54+
return findWidget(rootContainer, ToolItem.class, id);
55+
}
56+
57+
private <T extends Widget> T findWidget(Composite container, Class<T> type, String id) {
58+
List<T> widgets= findWidgets(container, type, id);
59+
assertFalse("more than one matching widget found for id '" + id + "':" + widgets, widgets.size() > 1);
60+
return widgets.isEmpty() ? null : widgets.get(0);
61+
}
62+
63+
private <T extends Widget> List<T> findWidgets(Composite container, Class<T> type, String id) {
64+
List<Widget> children= new ArrayList<>();
65+
children.addAll(List.of(container.getChildren()));
66+
if (container instanceof ToolBar toolbar) {
67+
children.addAll(List.of(toolbar.getItems()));
68+
}
69+
List<T> result= new ArrayList<>();
70+
for (Widget child : children) {
71+
if (type.isInstance(child)) {
72+
if (id.equals(child.getData(idDataKey))) {
73+
result.add(type.cast(child));
74+
}
75+
}
76+
if (child instanceof Composite compositeChild) {
77+
result.addAll(findWidgets(compositeChild, type, id));
78+
}
79+
}
80+
return result;
81+
}
82+
83+
}

‎tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ public class FindReplaceOverlayTest extends FindReplaceUITest<OverlayAccess> {
4545
public OverlayAccess openUIFromTextViewer(TextViewer viewer) {
4646
Accessor actionAccessor= new Accessor(getFindReplaceAction(), FindReplaceAction.class);
4747
actionAccessor.invoke("showOverlayInEditor", null);
48-
Accessor overlayAccessor= new Accessor(actionAccessor.get("overlay"), "org.eclipse.ui.internal.findandreplace.overlay.FindReplaceOverlay", getClass().getClassLoader());
49-
return new OverlayAccess(getFindReplaceTarget(), overlayAccessor);
48+
FindReplaceOverlay overlay= (FindReplaceOverlay) actionAccessor.get("overlay");
49+
return new OverlayAccess(getFindReplaceTarget(), overlay);
5050
}
5151

5252
@Test

‎tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/OverlayAccess.java‎

Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import java.util.Objects;
2222
import java.util.Set;
2323
import java.util.function.Predicate;
24-
import java.util.function.Supplier;
2524
import java.util.stream.Collectors;
2625

2726
import org.eclipse.swt.SWT;
@@ -31,13 +30,12 @@
3130
import org.eclipse.swt.widgets.Shell;
3231
import org.eclipse.swt.widgets.ToolItem;
3332

34-
import org.eclipse.text.tests.Accessor;
35-
3633
import org.eclipse.jface.text.IFindReplaceTarget;
3734
import org.eclipse.jface.text.IFindReplaceTargetExtension;
3835

3936
import org.eclipse.ui.internal.findandreplace.IFindReplaceUIAccess;
4037
import org.eclipse.ui.internal.findandreplace.SearchOptions;
38+
import org.eclipse.ui.internal.findandreplace.WidgetExtractor;
4139

4240
class OverlayAccess implements IFindReplaceUIAccess {
4341
private final IFindReplaceTarget findReplaceTarget;
@@ -64,28 +62,33 @@ class OverlayAccess implements IFindReplaceUIAccess {
6462

6563
private ToolItem replaceAllButton;
6664

67-
private final Runnable closeOperation;
68-
69-
private final Accessor dialogAccessor;
65+
private final FindReplaceOverlay overlay;
7066

71-
private final Supplier<Shell> shellRetriever;
67+
private final Shell shell;
7268

73-
OverlayAccess(IFindReplaceTarget findReplaceTarget, Accessor findReplaceOverlayAccessor) {
69+
OverlayAccess(IFindReplaceTarget findReplaceTarget, FindReplaceOverlay findReplaceOverlay) {
7470
this.findReplaceTarget= findReplaceTarget;
75-
dialogAccessor= findReplaceOverlayAccessor;
76-
find= (HistoryTextWrapper) findReplaceOverlayAccessor.get("searchBar");
77-
replace= (HistoryTextWrapper) findReplaceOverlayAccessor.get("replaceBar");
78-
caseSensitive= (ToolItem) findReplaceOverlayAccessor.get("caseSensitiveSearchButton");
79-
wholeWord= (ToolItem) findReplaceOverlayAccessor.get("wholeWordSearchButton");
80-
regEx= (ToolItem) findReplaceOverlayAccessor.get("regexSearchButton");
81-
searchForward= (ToolItem) findReplaceOverlayAccessor.get("searchDownButton");
82-
searchBackward= (ToolItem) findReplaceOverlayAccessor.get("searchUpButton");
83-
closeOperation= () -> findReplaceOverlayAccessor.invoke("close", null);
84-
openReplaceDialog= (Button) findReplaceOverlayAccessor.get("replaceToggle");
85-
replaceButton= (ToolItem) findReplaceOverlayAccessor.get("replaceButton");
86-
replaceAllButton= (ToolItem) findReplaceOverlayAccessor.get("replaceAllButton");
87-
inSelection= (ToolItem) findReplaceOverlayAccessor.get("searchInSelectionButton");
88-
shellRetriever= () -> ((Shell) findReplaceOverlayAccessor.invoke("getShell", null));
71+
overlay= findReplaceOverlay;
72+
shell= overlay.getShell();
73+
WidgetExtractor widgetExtractor= new WidgetExtractor(FindReplaceOverlay.ID_DATA_KEY, shell);
74+
find= widgetExtractor.findHistoryTextWrapper("searchInput");
75+
caseSensitive= widgetExtractor.findToolItem("caseSensitiveSearch");
76+
wholeWord= widgetExtractor.findToolItem("wholeWordSearch");
77+
regEx= widgetExtractor.findToolItem("regExSearch");
78+
inSelection= widgetExtractor.findToolItem("searchInSelection");
79+
searchForward= widgetExtractor.findToolItem("searchForward");
80+
searchBackward= widgetExtractor.findToolItem("searchBackward");
81+
openReplaceDialog= widgetExtractor.findButton("replaceToggle");
82+
extractReplaceWidgets();
83+
}
84+
85+
private void extractReplaceWidgets() {
86+
if (!isReplaceDialogOpen() && Objects.nonNull(openReplaceDialog)) {
87+
WidgetExtractor widgetExtractor= new WidgetExtractor(FindReplaceOverlay.ID_DATA_KEY, shell);
88+
replace= widgetExtractor.findHistoryTextWrapper("replaceInput");
89+
replaceButton= widgetExtractor.findToolItem("replaceOne");
90+
replaceAllButton= widgetExtractor.findToolItem("replaceAll");
91+
}
8992
}
9093

9194
private void restoreInitialConfiguration() {
@@ -100,12 +103,12 @@ private void restoreInitialConfiguration() {
100103
public void closeAndRestore() {
101104
restoreInitialConfiguration();
102105
assertInitialConfiguration();
103-
closeOperation.run();
106+
overlay.close();
104107
}
105108

106109
@Override
107110
public void close() {
108-
closeOperation.run();
111+
overlay.close();
109112
}
110113

111114
@Override
@@ -234,15 +237,13 @@ public void performReplace() {
234237
}
235238

236239
public boolean isReplaceDialogOpen() {
237-
return dialogAccessor.getBoolean("replaceBarOpen");
240+
return replace != null;
238241
}
239242

240243
public void openReplaceDialog() {
241244
if (!isReplaceDialogOpen() && Objects.nonNull(openReplaceDialog)) {
242245
openReplaceDialog.notifyListeners(SWT.Selection, null);
243-
replace= (HistoryTextWrapper) dialogAccessor.get("replaceBar");
244-
replaceButton= (ToolItem) dialogAccessor.get("replaceButton");
245-
replaceAllButton= (ToolItem) dialogAccessor.get("replaceAllButton");
246+
extractReplaceWidgets();
246247
}
247248
}
248249

@@ -309,15 +310,14 @@ public void assertEnabled(SearchOptions option) {
309310

310311
@Override
311312
public boolean isShown() {
312-
return shellRetriever.get() != null && shellRetriever.get().isVisible();
313+
return !shell.isDisposed() && shell.isVisible();
313314
}
314315

315316
@Override
316317
public boolean hasFocus() {
317-
Shell overlayShell= shellRetriever.get();
318-
Control focusControl= overlayShell.getDisplay().getFocusControl();
318+
Control focusControl= shell.getDisplay().getFocusControl();
319319
Shell focusControlShell= focusControl != null ? focusControl.getShell() : null;
320-
return focusControlShell == overlayShell;
320+
return focusControlShell == shell;
321321
}
322322

323323
}

0 commit comments

Comments
 (0)