From 98ab0ade5f9f8f64933b48163ee3249b3ad108df Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Thu, 3 Oct 2024 09:17:03 +0200 Subject: [PATCH] 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 --- .../findandreplace/FindReplaceTestUtil.java | 24 +++++++++++++++++++ .../findandreplace/FindReplaceUITest.java | 23 +++++------------- .../overlay/FindReplaceOverlayTest.java | 5 +++- .../findandreplace/overlay/OverlayAccess.java | 2 +- .../tests/FindReplaceDialogTest.java | 11 ++++----- 5 files changed, 40 insertions(+), 25 deletions(-) diff --git a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/FindReplaceTestUtil.java b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/FindReplaceTestUtil.java index 77c515c1c0f..945d2ccfb3a 100644 --- a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/FindReplaceTestUtil.java +++ b/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 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/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/FindReplaceUITest.java b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/FindReplaceUITest.java index 6b3a0a4127a..f943211d545 100644 --- a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/FindReplaceUITest.java +++ b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/FindReplaceUITest.java @@ -13,23 +13,20 @@ *******************************************************************************/ 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; @@ -37,8 +34,6 @@ import org.eclipse.ui.PlatformUI; -import org.eclipse.ui.workbench.texteditor.tests.ScreenshotTest; - import org.eclipse.ui.texteditor.FindReplaceAction; public abstract class FindReplaceUITest { @@ -51,6 +46,11 @@ public abstract class FindReplaceUITest 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/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayTest.java b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayTest.java index f9dda34774d..c19210ac315 100644 --- a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayTest.java +++ b/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 diff --git a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/OverlayAccess.java b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/OverlayAccess.java index 51450754661..342087b7cb3 100644 --- a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/OverlayAccess.java +++ b/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 diff --git a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/FindReplaceDialogTest.java b/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/FindReplaceDialogTest.java index 2fa0fb63b03..5e846f760de 100644 --- a/tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/FindReplaceDialogTest.java +++ b/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);