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
#906

Author: HeikoKlare

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,127 +133,149 @@ 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<Shell> 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<Shell> 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;
 	}
 
 	/**
 	 * 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<Shell> beforeShells, Display display, boolean expectShell) {
+	public static Shell findNewShell(Set<Shell> beforeShells, Display display) {
 		List<Shell> 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);
 	}
 
 	@Test
 	@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<Shell> beforeShells = Arrays.stream(editor.getSite().getShell().getDisplay().getShells()).filter(Shell::isVisible).collect(Collectors.toSet());
+		final Set<Shell> 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<IStatus> 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;
 		}
-
 	}
 }

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<Shell> beforeShells = Arrays.stream(editor.getSite().getShell().getDisplay().getShells()).filter(Shell::isVisible).collect(Collectors.toSet());
+		final Set<Shell> 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()) {

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);
+	}
+
 }