Skip to content

Commit

Permalink
Find/replace tests: remove reflective logic access
Browse files Browse the repository at this point in the history 
The IFindReplaceUIAccess currently provides access to the
IFindReplaceTarget, to which the UI is attached. The implementations for
the dialog and overlay employ reflection to access the target within the
UI implementation. It is, however, not necessary to access this
information via the find/replace UI since the tests set up the target
anyway and thus have access to it.

This change removes the according methods from the IFindReplaceUIAccess
and the reflective access from its implementations.
  • Loading branch information
@HeikoKlare
HeikoKlare committed Sep 10, 2024
1 parent 5c74619 commit adbbaa6
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 88 deletions.
13 changes: 8 additions & 5 deletions ...kbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/FindReplaceUITest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ public void testDisableWholeWordIfRegEx() {
dialog.assertEnabled(SearchOptions.REGEX);
dialog.assertEnabled(SearchOptions.WHOLE_WORD);

dialog.getFindReplaceLogic().updateTarget(fTextViewer.getFindReplaceTarget(), false);
dialog.select(SearchOptions.WHOLE_WORD);
dialog.select(SearchOptions.REGEX);
dialog.assertEnabled(SearchOptions.REGEX);
Expand Down Expand Up @@ -161,7 +160,7 @@ public void testShiftEnterReversesSearchDirection() {
dialog.select(SearchOptions.INCREMENTAL);
dialog.setFindText("line");
ensureHasFocusOnGTK();
IFindReplaceTarget target= dialog.getTarget();
IFindReplaceTarget target= getFindReplaceTarget();

assertEquals(0, (target.getSelection()).x);
assertEquals(4, (target.getSelection()).y);
Expand Down Expand Up @@ -190,7 +189,7 @@ public void testChangeInputForIncrementalSearch() {
dialog.select(SearchOptions.INCREMENTAL);

dialog.setFindText("lin");
IFindReplaceTarget target= dialog.getTarget();
IFindReplaceTarget target= getFindReplaceTarget();
assertEquals(0, (target.getSelection()).x);
assertEquals(dialog.getFindText().length(), (target.getSelection()).y);

Expand All @@ -206,7 +205,7 @@ public void testFindWithWholeWordEnabledWithMultipleWords() {
dialog.select(SearchOptions.WHOLE_WORD);
dialog.select(SearchOptions.WRAP);
dialog.setFindText("two");
IFindReplaceTarget target= dialog.getTarget();
IFindReplaceTarget target= getFindReplaceTarget();
assertEquals(0, (target.getSelection()).x);
assertEquals(3, (target.getSelection()).y);

Expand All @@ -227,7 +226,7 @@ public void testRegExSearch() {
dialog.select(SearchOptions.REGEX);
dialog.setFindText("(a|bc)");

IFindReplaceTarget target= dialog.getTarget();
IFindReplaceTarget target= getFindReplaceTarget();
dialog.simulateKeyboardInteractionInFindInputField(SWT.CR, false);
assertEquals(0, (target.getSelection()).x);
assertEquals(1, (target.getSelection()).y);
Expand Down Expand Up @@ -374,4 +373,8 @@ protected TextViewer getTextViewer() {
return fTextViewer;
}

protected final IFindReplaceTarget getFindReplaceTarget() {
return fTextViewer.getFindReplaceTarget();
}

}
6 changes: 0 additions & 6 deletions ...nch.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/IFindReplaceUIAccess.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,11 @@

import org.eclipse.swt.widgets.Widget;

import org.eclipse.jface.text.IFindReplaceTarget;

/**
* Wraps UI access for different find/replace UIs
*/
public interface IFindReplaceUIAccess {

IFindReplaceTarget getTarget();

void closeAndRestore();

void close();
Expand All @@ -47,8 +43,6 @@ public interface IFindReplaceUIAccess {

Widget getButtonForSearchOption(SearchOptions option);

IFindReplaceLogic getFindReplaceLogic();

void performReplaceAll();

void performReplace();
Expand Down
25 changes: 13 additions & 12 deletions ...itor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ 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(overlayAccessor);
return new OverlayAccess(getFindReplaceTarget(), overlayAccessor);
}

@Test
Expand All @@ -55,7 +55,7 @@ public void testDirectionalSearchButtons() {
OverlayAccess dialog= getDialog();

dialog.setFindText("line");
IFindReplaceTarget target= dialog.getTarget();
IFindReplaceTarget target= getFindReplaceTarget();

assertEquals(0, (target.getSelection()).x);
assertEquals(4, (target.getSelection()).y);
Expand Down Expand Up @@ -89,14 +89,14 @@ public void testDirectionalSearchButtons() {
public void testIncrementalSearchUpdatesAfterChangingOptions() {
initializeTextViewerWithFindReplaceUI("alinee\naLinee\nline\nline");
OverlayAccess dialog= getDialog();
IFindReplaceTarget target= dialog.getTarget();
IFindReplaceTarget target= getFindReplaceTarget();
dialog.setFindText("Line");
assertThat(dialog.getTarget().getSelectionText(), is("line"));
assertEquals(new Point(1,4), dialog.getTarget().getSelection());
assertThat(target.getSelectionText(), is("line"));
assertEquals(new Point(1, 4), target.getSelection());

dialog.select(SearchOptions.CASE_SENSITIVE);
assertThat(dialog.getTarget().getSelectionText(), is("Line"));
assertEquals(new Point(8,4), dialog.getTarget().getSelection());
assertThat(target.getSelectionText(), is("Line"));
assertEquals(new Point(8, 4), target.getSelection());

dialog.unselect(SearchOptions.CASE_SENSITIVE);
assertEquals(1, (target.getSelection()).x);
Expand All @@ -110,7 +110,7 @@ public void testIncrementalSearchUpdatesAfterChangingOptions() {
dialog.unselect(SearchOptions.WHOLE_WORD);
assertEquals(1, (target.getSelection()).x);
assertEquals(4, (target.getSelection()).y);
assertThat(dialog.getTarget().getSelectionText(), is("line"));
assertThat(target.getSelectionText(), is("line"));
}

@Test
Expand Down Expand Up @@ -153,18 +153,19 @@ public void testRememberReplaceExpandState() {
@Test
public void testSearchBackwardsWithRegEx() {
initializeTextViewerWithFindReplaceUI("text text text");
IFindReplaceTarget target= getFindReplaceTarget();

OverlayAccess dialog= getDialog();
dialog.select(SearchOptions.REGEX);
dialog.setFindText("text"); // with RegEx enabled, there is no incremental search!
dialog.pressSearch(true);
assertThat(dialog.getTarget().getSelection().y, is(4));
assertThat(target.getSelection().y, is(4));
dialog.pressSearch(true);
assertThat(dialog.getTarget().getSelection().x, is("text ".length()));
assertThat(target.getSelection().x, is("text ".length()));
dialog.pressSearch(true);
assertThat(dialog.getTarget().getSelection().x, is("text text ".length()));
assertThat(target.getSelection().x, is("text text ".length()));
dialog.pressSearch(false);
assertThat(dialog.getTarget().getSelection().x, is("text ".length()));
assertThat(target.getSelection().x, is("text ".length()));
}

@Test
Expand Down
56 changes: 21 additions & 35 deletions ...ch.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/OverlayAccess.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,45 +36,43 @@
import org.eclipse.jface.text.IFindReplaceTarget;
import org.eclipse.jface.text.IFindReplaceTargetExtension;

import org.eclipse.ui.internal.findandreplace.FindReplaceLogic;
import org.eclipse.ui.internal.findandreplace.IFindReplaceLogic;
import org.eclipse.ui.internal.findandreplace.IFindReplaceUIAccess;
import org.eclipse.ui.internal.findandreplace.SearchOptions;

class OverlayAccess implements IFindReplaceUIAccess {
FindReplaceLogic findReplaceLogic;
private final IFindReplaceTarget findReplaceTarget;

HistoryTextWrapper find;
private final HistoryTextWrapper find;

HistoryTextWrapper replace;
private final ToolItem inSelection;

ToolItem inSelection;
private final ToolItem caseSensitive;

ToolItem caseSensitive;
private final ToolItem wholeWord;

ToolItem wholeWord;
private final ToolItem regEx;

ToolItem regEx;
private final ToolItem searchForward;

ToolItem searchForward;
private final ToolItem searchBackward;

ToolItem searchBackward;
private final Button openReplaceDialog;

Button openReplaceDialog;
private HistoryTextWrapper replace;

ToolItem replaceButton;
private ToolItem replaceButton;

ToolItem replaceAllButton;
private ToolItem replaceAllButton;

private Runnable closeOperation;
private final Runnable closeOperation;

Accessor dialogAccessor;
private final Accessor dialogAccessor;

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

OverlayAccess(Accessor findReplaceOverlayAccessor) {
OverlayAccess(IFindReplaceTarget findReplaceTarget, Accessor findReplaceOverlayAccessor) {
this.findReplaceTarget= findReplaceTarget;
dialogAccessor= findReplaceOverlayAccessor;
findReplaceLogic= (FindReplaceLogic) findReplaceOverlayAccessor.get("findReplaceLogic");
find= (HistoryTextWrapper) findReplaceOverlayAccessor.get("searchBar");
replace= (HistoryTextWrapper) findReplaceOverlayAccessor.get("replaceBar");
caseSensitive= (ToolItem) findReplaceOverlayAccessor.get("caseSensitiveSearchButton");
Expand All @@ -90,11 +88,6 @@ class OverlayAccess implements IFindReplaceUIAccess {
shellRetriever= () -> ((Shell) findReplaceOverlayAccessor.invoke("getShell", null));
}

@Override
public IFindReplaceTarget getTarget() {
return findReplaceLogic.getTarget();
}

private void restoreInitialConfiguration() {
find.setText("");
select(SearchOptions.GLOBAL);
Expand Down Expand Up @@ -228,11 +221,6 @@ public void pressSearch(boolean forward) {
}
}

@Override
public IFindReplaceLogic getFindReplaceLogic() {
return findReplaceLogic;
}

@Override
public void performReplaceAll() {
openReplaceDialog();
Expand Down Expand Up @@ -277,25 +265,23 @@ public void assertInitialConfiguration() {
assertUnselected(SearchOptions.REGEX);
assertUnselected(SearchOptions.WHOLE_WORD);
assertUnselected(SearchOptions.CASE_SENSITIVE);
if (!doesTextViewerHaveMultiLineSelection(findReplaceLogic.getTarget())) {
if (!doesTextViewerHaveMultiLineSelection()) {
assertSelected(SearchOptions.GLOBAL);
assertTrue(findReplaceLogic.isActive(SearchOptions.GLOBAL));
} else {
assertUnselected(SearchOptions.GLOBAL);
assertFalse(findReplaceLogic.isActive(SearchOptions.GLOBAL));
}
assertEnabled(SearchOptions.GLOBAL);
assertEnabled(SearchOptions.REGEX);
assertEnabled(SearchOptions.CASE_SENSITIVE);
if (getFindText().equals("") || findReplaceLogic.isAvailable(SearchOptions.WHOLE_WORD)) {
if (!getFindText().contains(" ")) {
assertEnabled(SearchOptions.WHOLE_WORD);
} else {
assertDisabled(SearchOptions.WHOLE_WORD);
}
}

private boolean doesTextViewerHaveMultiLineSelection(IFindReplaceTarget target) {
if (target instanceof IFindReplaceTargetExtension scopeProvider) {
private boolean doesTextViewerHaveMultiLineSelection() {
if (findReplaceTarget instanceof IFindReplaceTargetExtension scopeProvider) {
return scopeProvider.getScope() != null; // null is returned for global scope
}
return false;
Expand Down
30 changes: 4 additions & 26 deletions ...orkbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/DialogAccess.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import static org.hamcrest.Matchers.hasItems;
import static org.hamcrest.Matchers.not;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

import java.util.Arrays;
import java.util.Set;
Expand All @@ -34,14 +33,12 @@
import org.eclipse.jface.text.IFindReplaceTarget;
import org.eclipse.jface.text.IFindReplaceTargetExtension;

import org.eclipse.ui.internal.findandreplace.FindReplaceLogic;
import org.eclipse.ui.internal.findandreplace.IFindReplaceLogic;
import org.eclipse.ui.internal.findandreplace.IFindReplaceUIAccess;
import org.eclipse.ui.internal.findandreplace.SearchOptions;

class DialogAccess implements IFindReplaceUIAccess {

FindReplaceLogic findReplaceLogic;
private final IFindReplaceTarget findReplaceTarget;

Combo findCombo;

Expand Down Expand Up @@ -77,9 +74,9 @@ class DialogAccess implements IFindReplaceUIAccess {

Accessor dialogAccessor;

DialogAccess(Accessor findReplaceDialogAccessor) {
DialogAccess(IFindReplaceTarget findReplaceTarget, Accessor findReplaceDialogAccessor) {
this.findReplaceTarget= findReplaceTarget;
dialogAccessor= findReplaceDialogAccessor;
findReplaceLogic= (FindReplaceLogic) findReplaceDialogAccessor.get("findReplaceLogic");
findCombo= (Combo) findReplaceDialogAccessor.get("fFindField");
replaceCombo= (Combo) findReplaceDialogAccessor.get("fReplaceField");
forwardRadioButton= (Button) findReplaceDialogAccessor.get("fForwardRadioButton");
Expand Down Expand Up @@ -199,11 +196,6 @@ public void setReplaceText(String text) {
replaceCombo.notifyListeners(SWT.Modify, null);
}

@Override
public IFindReplaceTarget getTarget() {
return findReplaceLogic.getTarget();
}

@Override
public String getFindText() {
return findCombo.getText();
Expand All @@ -219,11 +211,6 @@ public Combo getFindCombo() {
return findCombo;
}

@Override
public IFindReplaceLogic getFindReplaceLogic() {
return findReplaceLogic;
}

@Override
public void performReplaceAll() {
replaceAllButton.notifyListeners(SWT.Selection, null);
Expand All @@ -241,20 +228,11 @@ public void performReplaceAndFind() {

@Override
public void assertInitialConfiguration() {
assertTrue(findReplaceLogic.isActive(SearchOptions.FORWARD));
assertFalse(findReplaceLogic.isActive(SearchOptions.CASE_SENSITIVE));
assertTrue(findReplaceLogic.isActive(SearchOptions.WRAP));
assertFalse(findReplaceLogic.isActive(SearchOptions.INCREMENTAL));
assertFalse(findReplaceLogic.isActive(SearchOptions.REGEX));
assertFalse(findReplaceLogic.isActive(SearchOptions.WHOLE_WORD));

assertSelected(SearchOptions.FORWARD);
if (!doesTextViewerHaveMultiLineSelection(findReplaceLogic.getTarget())) {
if (!doesTextViewerHaveMultiLineSelection(findReplaceTarget)) {
assertSelected(SearchOptions.GLOBAL);
assertTrue(findReplaceLogic.isActive(SearchOptions.GLOBAL));
} else {
assertUnselected(SearchOptions.GLOBAL);
assertFalse(findReplaceLogic.isActive(SearchOptions.GLOBAL));
}
assertSelected(SearchOptions.WRAP);

Expand Down
8 changes: 4 additions & 4 deletions ...texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/FindReplaceDialogTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public DialogAccess openUIFromTextViewer(TextViewer viewer) {
Accessor fFindReplaceDialogStubAccessor= new Accessor(fFindReplaceDialogStub, "org.eclipse.ui.texteditor.FindReplaceAction$FindReplaceDialogStub", getClass().getClassLoader());

Accessor dialogAccessor= new Accessor(fFindReplaceDialogStubAccessor.invoke("getDialog", null), "org.eclipse.ui.texteditor.FindReplaceDialog", getClass().getClassLoader());
return new DialogAccess(dialogAccessor);
return new DialogAccess(getFindReplaceTarget(), dialogAccessor);
}

@Test
Expand Down Expand Up @@ -122,7 +122,7 @@ public void testShiftEnterReversesSearchDirectionDialogSpecific() {

dialog.setFindText("line");
ensureHasFocusOnGTK();
IFindReplaceTarget target= dialog.getTarget();
IFindReplaceTarget target= getFindReplaceTarget();

dialog.simulateKeyboardInteractionInFindInputField(SWT.CR, false);
assertEquals(0, (target.getSelection()).x);
Expand Down Expand Up @@ -152,7 +152,7 @@ public void testReplaceAndFindAfterInitializingFindWithSelectedString() {

assertThat(dialog.getFindText(), is("text"));

IFindReplaceTarget target= dialog.getTarget();
IFindReplaceTarget target= getFindReplaceTarget();
assertEquals(0, (target.getSelection()).x);
assertEquals(4, (target.getSelection()).y);

Expand Down Expand Up @@ -209,7 +209,7 @@ public void testFindWithWholeWordEnabledWithMultipleWordsNotIncremental() {
dialog.setFindText("two");
dialog.select(SearchOptions.WHOLE_WORD);
dialog.select(SearchOptions.WRAP);
IFindReplaceTarget target= dialog.getTarget();
IFindReplaceTarget target= getFindReplaceTarget();

dialog.simulateKeyboardInteractionInFindInputField(SWT.CR, false);
assertEquals(0, (target.getSelection()).x);
Expand Down

0 comments on commit adbbaa6

Please sign in to comment.