From 3cc769437961028a37e7662c140877c08e4d3b79 Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Tue, 25 Jul 2023 15:48:43 +0200 Subject: [PATCH] Make completion proposal calculation test time-independent #906 The test cases for completion proposals in CompletionTest depend on specific timing behavior of a test proposal processor. On one hand, this is prone to errors if proper timing is not given on the test environment. On the other hand, the tests run unnecessarily long because of waiting long enough. This change replaces the fixed-time waiting of the LongRunningBarContentAssistProcessor with an explicit trigger that finalizes the content calculation as soon as some barrier is reached within the tests. In addition, the long-running processor is only enabled on demand and will be finalized whenever a test case was executed to avoid that the processor thread is leaking to the next test case. Contributes to https://github.com/eclipse-platform/eclipse.platform.ui/issues/906 --- .../genericeditor/tests/CompletionTest.java | 161 ++++++++++-------- .../genericeditor/tests/TestQuickAssist.java | 13 +- .../LongRunningBarContentAssistProcessor.java | 22 ++- 3 files changed, 119 insertions(+), 77 deletions(-) diff --git a/tests/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/CompletionTest.java b/tests/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/CompletionTest.java index 6c00a42bd265..0cbb5dc1cbb9 100644 --- a/tests/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/CompletionTest.java +++ b/tests/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/CompletionTest.java @@ -35,6 +35,7 @@ import java.util.List; import java.util.Queue; import java.util.Set; +import java.util.function.BooleanSupplier; import java.util.stream.Collectors; import org.junit.jupiter.api.AfterEach; @@ -87,8 +88,8 @@ public class CompletionTest extends AbstratGenericEditorTest { @DisabledOnOs(value = OS.MAC, disabledReason = "test fails on Mac, see https://github.com/eclipse-platform/eclipse.platform.ui/issues/906") public void testCompletion() throws Exception { editor.selectAndReveal(3, 0); - this.completionShell = openContentAssist(true); - final Table completionProposalList = findCompletionSelectionControl(completionShell); + Shell shell = openContentAssistWithLongRunningProposalComputation(); + final Table completionProposalList = findCompletionSelectionControl(shell); checkCompletionContent(completionProposalList); // TODO find a way to actually trigger completion and verify result against // Editor content @@ -102,7 +103,7 @@ public void testDefaultContentAssistBug570488() throws Exception { TestLogListener listener= new TestLogListener(); log.addLogListener(listener); createAndOpenFile("Bug570488.txt", "bar 'bar'"); - openContentAssist(false); + assertNull(openContentAssist(), "No shell is expected to open"); runEventLoop(Display.getCurrent(), 0); assertFalse(listener.messages.stream().anyMatch(s -> s.matches(IStatus.ERROR)), "There are errors in the log"); log.removeLogListener(listener); @@ -120,8 +121,8 @@ public void testCompletionService() throws Exception { new Hashtable<>(Collections.singletonMap("contentType", "org.eclipse.ui.genericeditor.tests.content-type"))); runEventLoop(Display.getCurrent(), 0); editor.selectAndReveal(3, 0); - this.completionShell= openContentAssist(true); - final Table completionProposalList= findCompletionSelectionControl(completionShell); + Shell shell = openContentAssistWithLongRunningProposalComputation(); + final Table completionProposalList = findCompletionSelectionControl(shell); assertTrue(service.called, "Service was not called!"); checkCompletionContent(completionProposalList); registration.unregister(); @@ -132,37 +133,57 @@ public void testCompletionService() throws Exception { public void testCompletionUsingViewerSelection() throws Exception { editor.getDocumentProvider().getDocument(editor.getEditorInput()).set("abc"); editor.selectAndReveal(0, 3); - this.completionShell= openContentAssist(true); - final Table completionProposalList = findCompletionSelectionControl(completionShell); - assertTrue(waitForCondition(completionProposalList.getDisplay(), 5000, () -> { + final Shell shell = openContentAssist(); + assertNotNull(shell, "Shell is expected to open for completion proposals"); + final Table completionProposalList = findCompletionSelectionControl(shell); + waitForProposalRelatedCondition("Proposal list did not contain expected item 'ABC'", completionProposalList, + () -> Arrays.stream(completionProposalList.getItems()).map(TableItem::getText).anyMatch("ABC"::equals), 5_000); + } + + private static void waitForProposalRelatedCondition(String expectedListContentDescription, + Table completionProposalList, BooleanSupplier condition, int timeoutInMsec) { + boolean result = waitForCondition(completionProposalList.getDisplay(), timeoutInMsec, () -> { assertFalse(completionProposalList.isDisposed(), "Completion proposal list was unexpectedly disposed"); - return Arrays.stream(completionProposalList.getItems()).map(TableItem::getText).anyMatch("ABC"::equals); - }), "Proposal list did not contain expected item: ABC"); + return condition.getAsBoolean(); + }); + assertTrue(result, expectedListContentDescription + " but contained: " + + Arrays.toString(completionProposalList.getItems())); } - + @Test public void testEnabledWhenCompletion() throws Exception { - // Confirm that when disabled, a completion shell is present + // Confirm that when disabled, a completion shell is not present EnabledPropertyTester.setEnabled(false); createAndOpenFile("enabledWhen.txt", "bar 'bar'"); editor.selectAndReveal(3, 0); - assertNull(openContentAssist(false), "A new shell was found"); + assertNull(openContentAssist(), "No shell is expected to open"); cleanFileAndEditor(); // Confirm that when enabled, a completion shell is present EnabledPropertyTester.setEnabled(true); createAndOpenFile("enabledWhen.txt", "bar 'bar'"); - editor.selectAndReveal(3, 0); - assertNotNull(openContentAssist(true)); + editor.selectAndReveal(3, 0); + assertNotNull(openContentAssist(), "Shell is expected to open for completion proposals"); + } + + private Shell openContentAssistWithLongRunningProposalComputation() { + LongRunningBarContentAssistProcessor.enable(); + Shell shell = openContentAssist(); + assertNotNull(shell, "Shell is expected to open for completion proposals"); + return shell; } - private Shell openContentAssist(boolean expectShell) { + private Shell openContentAssist() { ContentAssistAction action = (ContentAssistAction) editor.getAction(ITextEditorActionConstants.CONTENT_ASSIST); action.update(); - final Set beforeShells = Arrays.stream(editor.getSite().getShell().getDisplay().getShells()).filter(Shell::isVisible).collect(Collectors.toSet()); - action.run(); //opens shell - Shell shell= findNewShell(beforeShells, editor.getSite().getShell().getDisplay(),expectShell); - runEventLoop(PlatformUI.getWorkbench().getDisplay(), 100); // can dispose shell when focus lost during debugging + final Set beforeShells = Arrays.stream(editor.getSite().getShell().getDisplay().getShells()) + .filter(Shell::isVisible).collect(Collectors.toSet()); + action.run(); + Shell shell = findNewShell(beforeShells, editor.getSite().getShell().getDisplay()); + runEventLoop(PlatformUI.getWorkbench().getDisplay(), 100); + if (shell != null) { + this.completionShell = shell; + } return shell; } @@ -170,51 +191,49 @@ private Shell openContentAssist(boolean expectShell) { * Checks that completion behaves as expected: * 1. Computing is shown instantaneously * 2. 1st proposal shown instantaneously - * 3. 2s later, 2nd proposal is shown + * 3. Calculation finishes when the test explicitly releases it * @param completionProposalList the completion list */ private void checkCompletionContent(final Table completionProposalList) { - // should be instantaneous, but happens to go asynchronous on CI so let's allow a wait - assertTrue(waitForCondition(completionProposalList.getDisplay(), 200, () -> { - assertFalse(completionProposalList.isDisposed(), "Completion proposal list was unexpectedly disposed"); - return completionProposalList.getItemCount() == 2 && completionProposalList.getItem(1).getData() != null; - }), "Proposal list did not show two initial items"); - assertTrue(isComputingInfoEntry(completionProposalList.getItem(0)), "Missing computing info entry in proposal list"); + waitForProposalRelatedCondition("Proposal list should show two initial items", completionProposalList, + () -> completionProposalList.getItemCount() == 2 + && completionProposalList.getItem(1).getData() != null, + 200); + assertTrue(isComputingInfoEntry(completionProposalList.getItem(0)), "Missing computing info entry"); final TableItem initialProposalItem = completionProposalList.getItem(1); - final String initialProposalString = ((ICompletionProposal)initialProposalItem.getData()).getDisplayString(); - assertThat("Unexpected initial proposal item", + final String initialProposalString = ((ICompletionProposal) initialProposalItem.getData()).getDisplayString(); + assertThat("Unexpected initial proposal item", BAR_CONTENT_ASSIST_PROPOSAL, endsWith(initialProposalString)); completionProposalList.setSelection(initialProposalItem); - // asynchronous - assertTrue(waitForCondition(completionProposalList.getDisplay(), LongRunningBarContentAssistProcessor.DELAY * 2, - () -> { - assertFalse(completionProposalList.isDisposed(), - "Completion proposal list was unexpectedly disposed"); - return !isComputingInfoEntry(completionProposalList.getItem(0)) - && completionProposalList.getItemCount() == 2; - }), "Proposal list did not show two items after finishing computing"); + + LongRunningBarContentAssistProcessor.finish(); + waitForProposalRelatedCondition("Proposal list should contain two items", completionProposalList, + () -> !isComputingInfoEntry(completionProposalList.getItem(0)) + && completionProposalList.getItemCount() == 2, + 5_000); final TableItem firstCompletionProposalItem = completionProposalList.getItem(0); final TableItem secondCompletionProposalItem = completionProposalList.getItem(1); - String firstCompletionProposalText = ((ICompletionProposal)firstCompletionProposalItem.getData()).getDisplayString(); - String secondCompletionProposalText = ((ICompletionProposal)secondCompletionProposalItem.getData()).getDisplayString(); + String firstCompletionProposalText = ((ICompletionProposal) firstCompletionProposalItem.getData()).getDisplayString(); + String secondCompletionProposalText = ((ICompletionProposal) secondCompletionProposalItem.getData()).getDisplayString(); assertThat("Unexpected first proposal item", BAR_CONTENT_ASSIST_PROPOSAL, endsWith(firstCompletionProposalText)); - assertThat("Unexpected second proposal item", LONG_RUNNING_BAR_CONTENT_ASSIST_PROPOSAL, endsWith(secondCompletionProposalText)); - String selectedProposalString = ((ICompletionProposal)completionProposalList.getSelection()[0].getData()).getDisplayString(); - assertEquals(initialProposalString, selectedProposalString, "Addition of completion proposal should keep selection"); + assertThat("Unexpected second proposal item", LONG_RUNNING_BAR_CONTENT_ASSIST_PROPOSAL, + endsWith(secondCompletionProposalText)); + String selectedProposalString = ((ICompletionProposal) completionProposalList.getSelection()[0].getData()) + .getDisplayString(); + assertEquals(initialProposalString, selectedProposalString, + "Addition of completion proposal should keep selection"); } - + private static boolean isComputingInfoEntry(TableItem item) { return item.getText().contains("Computing"); } - public static Shell findNewShell(Set beforeShells, Display display, boolean expectShell) { + public static Shell findNewShell(Set beforeShells, Display display) { List afterShells = Arrays.stream(display.getShells()) .filter(Shell::isVisible) .filter(shell -> !beforeShells.contains(shell)) .toList(); - if (expectShell) { - assertEquals(1, afterShells.size(), "No new shell found"); - } + assertTrue(afterShells.size() <= 1, "More than one new shell was found"); return afterShells.isEmpty() ? null : afterShells.get(0); } @@ -222,37 +241,41 @@ public static Shell findNewShell(Set beforeShells, Display display, boole @DisabledOnOs(value = OS.MAC, disabledReason = "test fails on Mac, see https://github.com/eclipse-platform/eclipse.platform.ui/issues/906") public void testCompletionFreeze_bug521484() throws Exception { editor.selectAndReveal(3, 0); - this.completionShell=openContentAssist(true); - final Table completionProposalList = findCompletionSelectionControl(this.completionShell); - // should be instantaneous, but happens to go asynchronous on CI so let's allow a wait - assertTrue(waitForCondition(completionProposalList.getDisplay(), 200, () -> { - assertFalse(completionProposalList.isDisposed(), "Completion proposal list was unexpectedly disposed"); - return completionProposalList.getItemCount() == 2; - }), "Proposal list did not show two items"); + final Shell shell = openContentAssistWithLongRunningProposalComputation(); + assertNotNull(shell, "Shell is expected to open for completion proposals"); + final Table completionProposalList = findCompletionSelectionControl(shell); + waitForProposalRelatedCondition("Proposal list should show two items", completionProposalList, + () -> completionProposalList.getItemCount() == 2, 200); assertTrue(isComputingInfoEntry(completionProposalList.getItem(0)), "Missing computing info entry"); - // Some processors are long running, moving cursor can cause freeze (bug 521484) - // asynchronous long timestamp = System.currentTimeMillis(); emulatePressLeftArrowKey(); - sleep(editor.getSite().getShell().getDisplay(), 200); //give time to process events + sleep(editor.getSite().getShell().getDisplay(), 200); long processingDuration = System.currentTimeMillis() - timestamp; - assertTrue(processingDuration < LongRunningBarContentAssistProcessor.DELAY, "UI Thread frozen for " + processingDuration + "ms"); + assertTrue(processingDuration < LongRunningBarContentAssistProcessor.TIMEOUT_MSEC, + "UI Thread frozen for " + processingDuration + "ms"); } @Test @DisabledOnOs(value = OS.MAC, disabledReason = "test fails on Mac, see https://github.com/eclipse-platform/eclipse.platform.ui/issues/906") public void testMoveCaretBackUsesAllProcessors_bug522255() throws Exception { - testCompletion(); + editor.selectAndReveal(3, 0); + Shell shell = openContentAssistWithLongRunningProposalComputation(); + final Table completionProposalList = findCompletionSelectionControl(shell); + checkCompletionContent(completionProposalList); + LongRunningBarContentAssistProcessor.enable(); emulatePressLeftArrowKey(); - final Set beforeShells = Arrays.stream(editor.getSite().getShell().getDisplay().getShells()).filter(Shell::isVisible).collect(Collectors.toSet()); + final Set beforeShells = Arrays.stream(editor.getSite().getShell().getDisplay().getShells()) + .filter(Shell::isVisible).collect(Collectors.toSet()); sleep(editor.getSite().getShell().getDisplay(), 200); - this.completionShell= findNewShell(beforeShells, editor.getSite().getShell().getDisplay(), true); - final Table completionProposalList = findCompletionSelectionControl(this.completionShell); - checkCompletionContent(completionProposalList); + assertTrue(shell.isDisposed(), "Completion proposal shell should be disposed after moving the cursor"); + this.completionShell = findNewShell(beforeShells, editor.getSite().getShell().getDisplay()); + assertNotNull(completionShell, "Shell is expected to open for completion proposals"); + final Table newCompletionProposalList = findCompletionSelectionControl(completionShell); + checkCompletionContent(newCompletionProposalList); } private void emulatePressLeftArrowKey() { - editor.selectAndReveal(((ITextSelection)editor.getSelectionProvider().getSelection()).getOffset() - 1, 0); + editor.selectAndReveal(((ITextSelection) editor.getSelectionProvider().getSelection()).getOffset() - 1, 0); Control styledText = editor.getAdapter(Control.class); Event e = new Event(); e.type = ST.VerifyKey; @@ -284,6 +307,11 @@ public void closeShell() { } } + @AfterEach + public void stopLongRunningCompletionProposalProcessor() { + LongRunningBarContentAssistProcessor.finish(); + } + private static final class TestLogListener implements ILogListener { List messages= new ArrayList<>(); @@ -301,8 +329,8 @@ private static final class MockContentAssistProcessor implements IContentAssistP @Override public ICompletionProposal[] computeCompletionProposals(ITextViewer viewer, int offset) { - called= true; - return null; + this.called = true; + return new ICompletionProposal[0]; } @Override @@ -329,6 +357,5 @@ public String getErrorMessage() { public IContextInformationValidator getContextInformationValidator() { return null; } - } } diff --git a/tests/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/TestQuickAssist.java b/tests/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/TestQuickAssist.java index dfc33eba1bd9..88c6da99d33e 100644 --- a/tests/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/TestQuickAssist.java +++ b/tests/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/TestQuickAssist.java @@ -17,6 +17,7 @@ import static org.eclipse.ui.tests.harness.util.DisplayHelper.runEventLoop; import static org.eclipse.ui.tests.harness.util.DisplayHelper.waitForCondition; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.Arrays; @@ -102,21 +103,22 @@ private Shell openQuickAssist() { editor.selectAndReveal(3, 0); TextOperationAction action = (TextOperationAction) editor.getAction(ITextEditorActionConstants.QUICK_ASSIST); action.update(); - final Set beforeShells = Arrays.stream(editor.getSite().getShell().getDisplay().getShells()).filter(Shell::isVisible).collect(Collectors.toSet()); + final Set beforeShells = Arrays.stream(editor.getSite().getShell().getDisplay().getShells()) + .filter(Shell::isVisible).collect(Collectors.toSet()); action.run(); - Shell shell= CompletionTest.findNewShell(beforeShells, editor.getSite().getShell().getDisplay(), true); - runEventLoop(PlatformUI.getWorkbench().getDisplay(),100); + Shell shell= CompletionTest.findNewShell(beforeShells, editor.getSite().getShell().getDisplay()); + assertNotNull(shell, "Shell is expected to open for quick assist"); + runEventLoop(PlatformUI.getWorkbench().getDisplay(), 100); return shell; } /** * Checks that a mock quick assist proposal comes up - * + * * @param completionProposalList the quick assist proposal list * @param proposals expected proposals */ private void checkCompletionContent(final Table completionProposalList, String[] proposals) { - // should be instantaneous, but happens to go asynchronous on CI so let's allow a wait waitForCondition(completionProposalList.getDisplay(), 200, () -> completionProposalList.getItemCount() >= proposals.length); assertEquals(proposals.length, completionProposalList.getItemCount()); @@ -126,7 +128,6 @@ private void checkCompletionContent(final Table completionProposalList, String[] } } - @AfterEach public void closeShell() { if (this.completionShell != null && !completionShell.isDisposed()) { diff --git a/tests/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/contributions/LongRunningBarContentAssistProcessor.java b/tests/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/contributions/LongRunningBarContentAssistProcessor.java index 82ef6b783130..c401867ef4fb 100644 --- a/tests/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/contributions/LongRunningBarContentAssistProcessor.java +++ b/tests/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/contributions/LongRunningBarContentAssistProcessor.java @@ -13,13 +13,16 @@ *******************************************************************************/ package org.eclipse.ui.genericeditor.tests.contributions; +import java.util.concurrent.atomic.AtomicBoolean; + import org.eclipse.jface.text.ITextViewer; import org.eclipse.jface.text.contentassist.ICompletionProposal; public class LongRunningBarContentAssistProcessor extends BarContentAssistProcessor { public static final String LONG_RUNNING_BAR_CONTENT_ASSIST_PROPOSAL = "bars are also good for soft drink cocktails."; - public static final int DELAY = 2000; + public static final int TIMEOUT_MSEC = 10_000; + private static final AtomicBoolean running = new AtomicBoolean(); public LongRunningBarContentAssistProcessor() { super(LONG_RUNNING_BAR_CONTENT_ASSIST_PROPOSAL); @@ -28,11 +31,22 @@ public LongRunningBarContentAssistProcessor() { @Override public ICompletionProposal[] computeCompletionProposals(ITextViewer viewer, int offset) { try { - Thread.sleep(DELAY); + long startExecutionTime = System.currentTimeMillis(); + while (running.get() && (System.currentTimeMillis() - startExecutionTime) < TIMEOUT_MSEC) { + Thread.sleep(20); + } } catch (InterruptedException e) { - // TODO Auto-generated catch block - e.printStackTrace(); + // Just finish on unexpected interrupt } return super.computeCompletionProposals(viewer, offset); } + + public static void enable() { + running.set(true); + } + + public static void finish() { + running.set(false); + } + }