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
eclipse-platform · Oct 3, 2024 · c777660 · c777660
1 parent e37086d
commit c777660
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 25 deletions.
diff --git a/...ench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/FindReplaceTestUtil.java b/...ench.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);
+		}
+	}
+
 }
diff --git a/...kbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/FindReplaceUITest.java b/...kbench.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);

diff --git a/...itor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayTest.java b/...itor.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

diff --git a/...ch.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/OverlayAccess.java b/...ch.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

diff --git a/...texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/FindReplaceDialogTest.java b/...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);