Skip to content

Commit

Permalink
Remove reflective access from find/replace tests #2060
Browse files Browse the repository at this point in the history
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
  • Loading branch information
HeikoKlare committed Sep 30, 2024
1 parent f35ef3a commit 8f010df
Show file tree
Hide file tree
Showing 7 changed files with 218 additions and 91 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ private final class KeyboardShortcuts {
KeyStroke.getInstance(SWT.MOD1, 'R'), KeyStroke.getInstance(SWT.MOD1, 'r'));
}

public static final String ID_DATA_KEY = "org.eclipse.ui.internal.findreplace.overlay.FindReplaceOverlay.id"; //$NON-NLS-1$

private static final String REPLACE_BAR_OPEN_DIALOG_SETTING = "replaceBarOpen"; //$NON-NLS-1$
private static final double WORST_CASE_RATIO_EDITOR_TO_OVERLAY = 0.95;
private static final double BIG_WIDTH_RATIO_EDITOR_TO_OVERLAY = 0.7;
Expand All @@ -130,9 +132,9 @@ private final class KeyboardShortcuts {
private ToolItem wholeWordSearchButton;
private ToolItem caseSensitiveSearchButton;
private ToolItem regexSearchButton;
private ToolItem searchUpButton;
private ToolItem searchDownButton;
private ToolItem searchAllButton;
private ToolItem searchBackwardButton;
private ToolItem searchForwardButton;
private ToolItem selectAllButton;
private AccessibleToolBar closeTools;
private ToolItem closeButton;

Expand Down Expand Up @@ -370,6 +372,7 @@ public int open() {
}
overlayOpen = true;
applyOverlayColors(backgroundToUse, true);
assignIDs();
updateFromTargetSelection();
searchBar.forceFocus();

Expand All @@ -391,6 +394,25 @@ private void restoreOverlaySettings() {
}
}

@SuppressWarnings("nls")
private void assignIDs() {
replaceToggle.setData(ID_DATA_KEY, "replaceToggle");
searchBar.setData(ID_DATA_KEY, "searchInput");
searchBackwardButton.setData(ID_DATA_KEY, "searchBackward");
searchForwardButton.setData(ID_DATA_KEY, "searchForward");
selectAllButton.setData(ID_DATA_KEY, "selectAll");
searchInSelectionButton.setData(ID_DATA_KEY, "searchInSelection");
wholeWordSearchButton.setData(ID_DATA_KEY, "wholeWordSearch");
regexSearchButton.setData(ID_DATA_KEY, "regExSearch");
caseSensitiveSearchButton.setData(ID_DATA_KEY, "caseSensitiveSearch");

if (replaceBarOpen) {
replaceBar.setData(ID_DATA_KEY, "replaceInput");
replaceButton.setData(ID_DATA_KEY, "replaceOne");
replaceAllButton.setData(ID_DATA_KEY, "replaceAll");
}
}

private void applyOverlayColors(Color color, boolean tryToColorReplaceBar) {
closeTools.setBackground(color);
closeButton.setBackground(color);
Expand All @@ -400,9 +422,9 @@ private void applyOverlayColors(Color color, boolean tryToColorReplaceBar) {
wholeWordSearchButton.setBackground(color);
regexSearchButton.setBackground(color);
caseSensitiveSearchButton.setBackground(color);
searchAllButton.setBackground(color);
searchUpButton.setBackground(color);
searchDownButton.setBackground(color);
selectAllButton.setBackground(color);
searchBackwardButton.setBackground(color);
searchForwardButton.setBackground(color);

searchBarContainer.setBackground(color);
searchBar.setBackground(color);
Expand Down Expand Up @@ -511,20 +533,20 @@ private void createSearchTools() {

searchTools.createToolItem(SWT.SEPARATOR);

searchUpButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.PUSH)
searchBackwardButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.PUSH)
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_FIND_PREV))
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_upSearchButton_toolTip)
.withOperation(() -> performSearch(false))
.withShortcuts(KeyboardShortcuts.SEARCH_BACKWARD).build();

searchDownButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.PUSH)
searchForwardButton = new AccessibleToolItemBuilder(searchTools).withStyleBits(SWT.PUSH)
.withImage(FindReplaceOverlayImages.get(FindReplaceOverlayImages.KEY_FIND_NEXT))
.withToolTipText(FindReplaceMessages.FindReplaceOverlay_downSearchButton_toolTip)
.withOperation(() -> performSearch(true))
.withShortcuts(KeyboardShortcuts.SEARCH_FORWARD).build();
searchDownButton.setSelection(true); // by default, search down
searchForwardButton.setSelection(true); // by default, search down

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

updatePlacementAndVisibility();
applyOverlayColors(backgroundToUse, true);
assignIDs();
replaceBar.forceFocus();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@
*/
class FindReplaceDialog extends Dialog {

public static final String ID_DATA_KEY = "org.eclipse.ui.texteditor.FindReplaceDialog.id"; //$NON-NLS-1$

private static final int CLOSE_BUTTON_ID = 101;
private IFindReplaceLogic findReplaceLogic;

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

updateButtonState();
assignIDs();
}

/**
Expand Down Expand Up @@ -1353,4 +1356,23 @@ private String getCurrentSelection() {
return null;
return target.getSelectionText();
}

@SuppressWarnings("nls")
private void assignIDs() {
fFindField.setData(ID_DATA_KEY, "searchInput");
fReplaceField.setData(ID_DATA_KEY, "replaceInput");
fForwardRadioButton.setData(ID_DATA_KEY, "searchForward");
fGlobalRadioButton.setData(ID_DATA_KEY, "globalSearch");
fSelectedRangeRadioButton.setData(ID_DATA_KEY, "searchInSelection");
fCaseCheckBox.setData(ID_DATA_KEY, "caseSensitiveSearch");
fWrapCheckBox.setData(ID_DATA_KEY, "wrappedSearch");
fWholeWordCheckBox.setData(ID_DATA_KEY, "wholeWordSearch");
fIncrementalCheckBox.setData(ID_DATA_KEY, "incrementalSearch");
fIsRegExCheckBox.setData(ID_DATA_KEY, "regExSearch");

fReplaceSelectionButton.setData(ID_DATA_KEY, "replaceOne");
fReplaceFindButton.setData(ID_DATA_KEY, "replaceFindOne");
fReplaceAllButton.setData(ID_DATA_KEY, "replaceAll");
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*******************************************************************************
* Copyright (c) 2024 Vector Informatik GmbH and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Vector Informatik GmbH - initial API and implementation
*******************************************************************************/
package org.eclipse.ui.internal.findandreplace;

import static org.junit.Assert.assertFalse;

import java.util.ArrayList;
import java.util.List;

import org.eclipse.swt.widgets.Button;
import org.eclipse.swt.widgets.Combo;
import org.eclipse.swt.widgets.Composite;
import org.eclipse.swt.widgets.ToolBar;
import org.eclipse.swt.widgets.ToolItem;
import org.eclipse.swt.widgets.Widget;

import org.eclipse.ui.internal.findandreplace.overlay.HistoryTextWrapper;

public final class WidgetExtractor {

private final Composite rootContainer;

private final String idDataKey;

public WidgetExtractor(String idDataKey, Composite container) {
this.idDataKey= idDataKey;
this.rootContainer= container;
}

public HistoryTextWrapper findHistoryTextWrapper(String id) {
return findWidget(rootContainer, HistoryTextWrapper.class, id);
}

public Combo findCombo(String id) {
return findWidget(rootContainer, Combo.class, id);
}

public Button findButton(String id) {
return findWidget(rootContainer, Button.class, id);
}

public ToolItem findToolItem(String id) {
return findWidget(rootContainer, ToolItem.class, id);
}

private <T extends Widget> T findWidget(Composite container, Class<T> type, String id) {
List<T> widgets= findWidgets(container, type, id);
assertFalse("more than one matching widget found for id '" + id + "':" + widgets, widgets.size() > 1);
return widgets.isEmpty() ? null : widgets.get(0);
}

private <T extends Widget> List<T> findWidgets(Composite container, Class<T> type, String id) {
List<Widget> children= new ArrayList<>();
children.addAll(List.of(container.getChildren()));
if (container instanceof ToolBar toolbar) {
children.addAll(List.of(toolbar.getItems()));
}
List<T> result= new ArrayList<>();
for (Widget child : children) {
if (type.isInstance(child)) {
if (id.equals(child.getData(idDataKey))) {
result.add(type.cast(child));
}
}
if (child instanceof Composite compositeChild) {
result.addAll(findWidgets(compositeChild, type, id));
}
}
return result;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ public class FindReplaceOverlayTest extends FindReplaceUITest<OverlayAccess> {
public OverlayAccess openUIFromTextViewer(TextViewer viewer) {
Accessor actionAccessor= new Accessor(getFindReplaceAction(), FindReplaceAction.class);
actionAccessor.invoke("showOverlayInEditor", null);
Accessor overlayAccessor= new Accessor(actionAccessor.get("overlay"), "org.eclipse.ui.internal.findandreplace.overlay.FindReplaceOverlay", getClass().getClassLoader());
return new OverlayAccess(getFindReplaceTarget(), overlayAccessor);
FindReplaceOverlay overlay= (FindReplaceOverlay) actionAccessor.get("overlay");
return new OverlayAccess(getFindReplaceTarget(), overlay);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import java.util.Objects;
import java.util.Set;
import java.util.function.Predicate;
import java.util.function.Supplier;
import java.util.stream.Collectors;

import org.eclipse.swt.SWT;
Expand All @@ -31,13 +30,12 @@
import org.eclipse.swt.widgets.Shell;
import org.eclipse.swt.widgets.ToolItem;

import org.eclipse.text.tests.Accessor;

import org.eclipse.jface.text.IFindReplaceTarget;
import org.eclipse.jface.text.IFindReplaceTargetExtension;

import org.eclipse.ui.internal.findandreplace.IFindReplaceUIAccess;
import org.eclipse.ui.internal.findandreplace.SearchOptions;
import org.eclipse.ui.internal.findandreplace.WidgetExtractor;

class OverlayAccess implements IFindReplaceUIAccess {
private final IFindReplaceTarget findReplaceTarget;
Expand All @@ -64,28 +62,33 @@ class OverlayAccess implements IFindReplaceUIAccess {

private ToolItem replaceAllButton;

private final Runnable closeOperation;

private final Accessor dialogAccessor;
private final FindReplaceOverlay overlay;

private final Supplier<Shell> shellRetriever;
private final Shell shell;

OverlayAccess(IFindReplaceTarget findReplaceTarget, Accessor findReplaceOverlayAccessor) {
OverlayAccess(IFindReplaceTarget findReplaceTarget, FindReplaceOverlay findReplaceOverlay) {
this.findReplaceTarget= findReplaceTarget;
dialogAccessor= findReplaceOverlayAccessor;
find= (HistoryTextWrapper) findReplaceOverlayAccessor.get("searchBar");
replace= (HistoryTextWrapper) findReplaceOverlayAccessor.get("replaceBar");
caseSensitive= (ToolItem) findReplaceOverlayAccessor.get("caseSensitiveSearchButton");
wholeWord= (ToolItem) findReplaceOverlayAccessor.get("wholeWordSearchButton");
regEx= (ToolItem) findReplaceOverlayAccessor.get("regexSearchButton");
searchForward= (ToolItem) findReplaceOverlayAccessor.get("searchDownButton");
searchBackward= (ToolItem) findReplaceOverlayAccessor.get("searchUpButton");
closeOperation= () -> findReplaceOverlayAccessor.invoke("close", null);
openReplaceDialog= (Button) findReplaceOverlayAccessor.get("replaceToggle");
replaceButton= (ToolItem) findReplaceOverlayAccessor.get("replaceButton");
replaceAllButton= (ToolItem) findReplaceOverlayAccessor.get("replaceAllButton");
inSelection= (ToolItem) findReplaceOverlayAccessor.get("searchInSelectionButton");
shellRetriever= () -> ((Shell) findReplaceOverlayAccessor.invoke("getShell", null));
overlay= findReplaceOverlay;
shell= overlay.getShell();
WidgetExtractor widgetExtractor= new WidgetExtractor(FindReplaceOverlay.ID_DATA_KEY, shell);
find= widgetExtractor.findHistoryTextWrapper("searchInput");
caseSensitive= widgetExtractor.findToolItem("caseSensitiveSearch");
wholeWord= widgetExtractor.findToolItem("wholeWordSearch");
regEx= widgetExtractor.findToolItem("regExSearch");
inSelection= widgetExtractor.findToolItem("searchInSelection");
searchForward= widgetExtractor.findToolItem("searchForward");
searchBackward= widgetExtractor.findToolItem("searchBackward");
openReplaceDialog= widgetExtractor.findButton("replaceToggle");
extractReplaceWidgets();
}

private void extractReplaceWidgets() {
if (!isReplaceDialogOpen() && Objects.nonNull(openReplaceDialog)) {
WidgetExtractor widgetExtractor= new WidgetExtractor(FindReplaceOverlay.ID_DATA_KEY, shell);
replace= widgetExtractor.findHistoryTextWrapper("replaceInput");
replaceButton= widgetExtractor.findToolItem("replaceOne");
replaceAllButton= widgetExtractor.findToolItem("replaceAll");
}
}

private void restoreInitialConfiguration() {
Expand All @@ -100,12 +103,12 @@ private void restoreInitialConfiguration() {
public void closeAndRestore() {
restoreInitialConfiguration();
assertInitialConfiguration();
closeOperation.run();
overlay.close();
}

@Override
public void close() {
closeOperation.run();
overlay.close();
}

@Override
Expand Down Expand Up @@ -234,15 +237,13 @@ public void performReplace() {
}

public boolean isReplaceDialogOpen() {
return dialogAccessor.getBoolean("replaceBarOpen");
return replace != null;
}

public void openReplaceDialog() {
if (!isReplaceDialogOpen() && Objects.nonNull(openReplaceDialog)) {
openReplaceDialog.notifyListeners(SWT.Selection, null);
replace= (HistoryTextWrapper) dialogAccessor.get("replaceBar");
replaceButton= (ToolItem) dialogAccessor.get("replaceButton");
replaceAllButton= (ToolItem) dialogAccessor.get("replaceAllButton");
extractReplaceWidgets();
}
}

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

@Override
public boolean isShown() {
return shellRetriever.get() != null && shellRetriever.get().isVisible();
return !shell.isDisposed() && shell.isVisible();
}

@Override
public boolean hasFocus() {
Shell overlayShell= shellRetriever.get();
Control focusControl= overlayShell.getDisplay().getFocusControl();
Control focusControl= shell.getDisplay().getFocusControl();
Shell focusControlShell= focusControl != null ? focusControl.getShell() : null;
return focusControlShell == overlayShell;
return focusControlShell == shell;
}

}
Loading

0 comments on commit 8f010df

Please sign in to comment.