Find/replace UI tests: unify focus validation

The find/replace UI tests currently validate for proper focus only in
specific situations, usually restricted to when running on GTK. On the
one hand, this makes debugging more difficult in case the focus was not
properly set in the beginning, e.g., because the workbench window was
not active when the test started. On the other hand, it makes the test
execution more prone to be indeterministic and platform-specific, as
GTK-specific code is involved.

This change provides three improvements to mitigate these issues:
- It ensures that the workbench window is active when test execution
starts
- It validates that the find/replace UI (overlay/dialog) has focus every
time it is opened during test execution
- It gets rid of GTK-specific focus validation

Author: HeikoKlare

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

@@ -13,10 +13,16 @@
  *******************************************************************************/
 package org.eclipse.ui.internal.findandreplace;
 
+import static org.junit.Assert.fail;
+
+import java.util.function.Supplier;
+
 import org.eclipse.swt.widgets.Display;
 
 import org.eclipse.ui.PlatformUI;
 
+import org.eclipse.ui.workbench.texteditor.tests.ScreenshotTest;
+
 public final class FindReplaceTestUtil {
 
 	private FindReplaceTestUtil() {
@@ -36,4 +42,22 @@ public static void runEventQueue() {
 		}
 	}
 
+	public static void waitForFocus(Supplier<Boolean> hasFocusValidator, String testName) {
+		int focusAttempts= 0;
+		while (!hasFocusValidator.get() && focusAttempts < 10) {
+			focusAttempts++;
+			PlatformUI.getWorkbench().getDisplay().readAndDispatch();
+			if (!hasFocusValidator.get()) {
+				try {
+					Thread.sleep(50);
+				} catch (InterruptedException e) {
+				}
+			}
+		}
+		if (!hasFocusValidator.get()) {
+			String screenshotPath= ScreenshotTest.takeScreenshot(FindReplaceUITest.class, testName, System.out);
+			fail("The find/replace UI did not receive focus. Screenshot: " + screenshotPath);
+		}
+	}
+
 }

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

@@ -13,32 +13,27 @@
  *******************************************************************************/
 package org.eclipse.ui.internal.findandreplace;
 
-import static org.eclipse.ui.internal.findandreplace.FindReplaceTestUtil.runEventQueue;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.fail;
 
 import java.util.ResourceBundle;
 
 import org.junit.After;
+import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TestName;
 
 import org.eclipse.swt.SWT;
 
-import org.eclipse.jface.util.Util;
-
 import org.eclipse.jface.text.Document;
 import org.eclipse.jface.text.IFindReplaceTarget;
 import org.eclipse.jface.text.TextSelection;
 import org.eclipse.jface.text.TextViewer;
 
 import org.eclipse.ui.PlatformUI;
 
-import org.eclipse.ui.workbench.texteditor.tests.ScreenshotTest;
-
 import org.eclipse.ui.texteditor.FindReplaceAction;
 
 public abstract class FindReplaceUITest<AccessType extends IFindReplaceUIAccess> {
@@ -51,6 +46,11 @@ public abstract class FindReplaceUITest<AccessType extends IFindReplaceUIAccess>
 
 	private AccessType dialog;
 
+	@Before
+	public final void ensureWorkbenchWindowIsActive() {
+		PlatformUI.getWorkbench().getWorkbenchWindows()[0].getShell().forceActive();
+	}
+
 	protected FindReplaceAction getFindReplaceAction() {
 		return findReplaceAction;
 	}
@@ -78,16 +78,6 @@ protected void reopenFindReplaceUIForTextViewer() {
 		dialog= openUIFromTextViewer(fTextViewer);
 	}
 
-	protected final void ensureHasFocusOnGTK() {
-		if (Util.isGtk()) {
-			runEventQueue();
-			if (!dialog.hasFocus()) {
-				String screenshotPath= ScreenshotTest.takeScreenshot(FindReplaceUITest.class, testName.getMethodName(), System.out);
-				fail("this test does not work on GTK unless the runtime workbench has focus. Screenshot: " + screenshotPath);
-			}
-		}
-	}
-
 	protected abstract AccessType openUIFromTextViewer(TextViewer viewer);
 
 	@After
@@ -159,7 +149,6 @@ public void testShiftEnterReversesSearchDirection() {
 
 		dialog.select(SearchOptions.INCREMENTAL);
 		dialog.setFindText("line");
-		ensureHasFocusOnGTK();
 		IFindReplaceTarget target= getFindReplaceTarget();
 
 		assertEquals(0, (target.getSelection()).x);

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

@@ -13,6 +13,7 @@
  *******************************************************************************/
 package org.eclipse.ui.internal.findandreplace.overlay;
 
+import static org.eclipse.ui.internal.findandreplace.FindReplaceTestUtil.waitForFocus;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertEquals;
@@ -46,7 +47,9 @@ public OverlayAccess openUIFromTextViewer(TextViewer viewer) {
 		Accessor actionAccessor= new Accessor(getFindReplaceAction(), FindReplaceAction.class);
 		actionAccessor.invoke("showOverlayInEditor", null);
 		FindReplaceOverlay overlay= (FindReplaceOverlay) actionAccessor.get("overlay");
-		return new OverlayAccess(getFindReplaceTarget(), overlay);
+		OverlayAccess uiAccess= new OverlayAccess(getFindReplaceTarget(), overlay);
+		waitForFocus(uiAccess::hasFocus, testName.getMethodName());
+		return uiAccess;
 	}
 
 	@Test

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

@@ -102,7 +102,7 @@ private void restoreInitialConfiguration() {
 	public void closeAndRestore() {
 		restoreInitialConfiguration();
 		assertInitialConfiguration();
-		overlay.close();
+		close();
 	}
 
 	@Override

tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/FindReplaceDialogTest.java

@@ -14,6 +14,7 @@
 package org.eclipse.ui.workbench.texteditor.tests;
 
 import static org.eclipse.ui.internal.findandreplace.FindReplaceTestUtil.runEventQueue;
+import static org.eclipse.ui.internal.findandreplace.FindReplaceTestUtil.waitForFocus;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertEquals;
@@ -52,7 +53,9 @@ public DialogAccess openUIFromTextViewer(TextViewer viewer) {
 		Accessor fFindReplaceDialogStubAccessor= new Accessor(fFindReplaceDialogStub, "org.eclipse.ui.texteditor.FindReplaceAction$FindReplaceDialogStub", getClass().getClassLoader());
 
 		Dialog dialog= (Dialog) fFindReplaceDialogStubAccessor.invoke("getDialog", null);
-		return new DialogAccess(getFindReplaceTarget(), dialog);
+		DialogAccess uiAccess= new DialogAccess(getFindReplaceTarget(), dialog);
+		waitForFocus(uiAccess::hasFocus, testName.getMethodName());
+		return uiAccess;
 	}
 
 	@Test
@@ -65,8 +68,6 @@ public void testFocusNotChangedWhenEnterPressed() {
 		dialog.getFindCombo().setFocus();
 		dialog.setFindText("line");
 		dialog.simulateKeyboardInteractionInFindInputField(SWT.CR, false);
-		ensureHasFocusOnGTK();
-
 		assertTrue(dialog.getFindCombo().isFocusControl());
 
 		Button wrapCheckBox= dialog.getButtonForSearchOption(SearchOptions.WRAP);
@@ -86,9 +87,8 @@ public void testFocusNotChangedWhenButtonMnemonicPressed() {
 
 		initializeTextViewerWithFindReplaceUI("");
 		DialogAccess dialog= getDialog();
-
 		dialog.setFindText("line");
-		ensureHasFocusOnGTK();
+		runEventQueue();
 
 		Button wrapCheckBox= dialog.getButtonForSearchOption(SearchOptions.WRAP);
 		wrapCheckBox.setFocus();
@@ -122,7 +122,6 @@ public void testShiftEnterReversesSearchDirectionDialogSpecific() {
 		DialogAccess dialog= getDialog();
 
 		dialog.setFindText("line");
-		ensureHasFocusOnGTK();
 		IFindReplaceTarget target= getFindReplaceTarget();
 
 		dialog.simulateKeyboardInteractionInFindInputField(SWT.CR, false);