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

@@ -82,8 +82,8 @@ public class CompletionTest extends AbstratGenericEditorTest {
 	@Test
 	public void testCompletion() throws Exception {
 		editor.selectAndReveal(3, 0);
-		this.completionShell= openConentAssist();
-		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
 		// Assert.assertEquals("Completion didn't complete", "bars are good for a beer.", ((StyledText)editor.getAdapter(Control.class)).getText());
@@ -95,7 +95,7 @@ public void testDefaultContentAssistBug570488() throws Exception {
 		TestLogListener listener= new TestLogListener();
 		log.addLogListener(listener);
 		createAndOpenFile("Bug570488.txt", "bar 'bar'");
-		openConentAssist(false);
+		assertNull("No shell is expected to open", openContentAssist());
 		DisplayHelper.driveEventQueue(Display.getCurrent());
 		assertFalse("There are errors in the log", listener.messages.stream().anyMatch(s -> s.matches(IStatus.ERROR)));
 		log.removeLogListener(listener);
@@ -112,8 +112,8 @@ public void testCompletionService() throws Exception {
 				new Hashtable<>(Collections.singletonMap("contentType", "org.eclipse.ui.genericeditor.tests.content-type")));
 		DisplayHelper.driveEventQueue(Display.getCurrent());
 		editor.selectAndReveal(3, 0);
-		this.completionShell= openConentAssist();
-		final Table completionProposalList= findCompletionSelectionControl(completionShell);
+		Shell shell = openContentAssistWithLongRunningProposalComputation();
+		final Table completionProposalList = findCompletionSelectionControl(shell);
 		checkCompletionContent(completionProposalList);
 		assertTrue("Service was not called!", service.called);
 		registration.unregister();
@@ -123,20 +123,22 @@ public void testCompletionService() throws Exception {
 	public void testCompletionUsingViewerSelection() throws Exception {
 		editor.getDocumentProvider().getDocument(editor.getEditorInput()).set("abc");
 		editor.selectAndReveal(0, 3);
-		this.completionShell= openConentAssist();
-		final Table completionProposalList = findCompletionSelectionControl(completionShell);
-		waitForProposalRelatedCondition("Proposal list did not contain expected item: ABC", completionProposalList,
+		final Shell shell = openContentAssist();
+		assertNotNull("Shell is expected to open for completion proposals", shell);
+		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 errorMessage, Table completionProposalList, BooleanSupplier condition, int timeoutInMsec) {
-		assertTrue(errorMessage, new DisplayHelper() {
+	private static void waitForProposalRelatedCondition(String expectedListContentDescription, Table completionProposalList, BooleanSupplier condition, int timeoutInMsec) {
+		boolean result = new DisplayHelper() {
 			@Override
 			protected boolean condition() {
 				assertFalse("Completion proposal list was unexpectedly disposed", completionProposalList.isDisposed());
 				return condition.getAsBoolean();
 			}
-		}.waitForCondition(completionProposalList.getDisplay(), timeoutInMsec));
+		}.waitForCondition(completionProposalList.getDisplay(), timeoutInMsec);
+		assertTrue(expectedListContentDescription + " but contained: " + Arrays.toString(completionProposalList.getItems()), result); 
 	}
 
 	@Test
@@ -145,26 +147,33 @@ public void testEnabledWhenCompletion() throws Exception {
 		EnabledPropertyTester.setEnabled(false);
 		createAndOpenFile("enabledWhen.txt", "bar 'bar'");
 		editor.selectAndReveal(3, 0);
-		assertNull("A new shell was found", openConentAssist(false));
+		assertNull("No shell is expected to open", openContentAssist());
 		cleanFileAndEditor();
 
 		// Confirm that when enabled, a completion shell is present
 		EnabledPropertyTester.setEnabled(true);
 		createAndOpenFile("enabledWhen.txt", "bar 'bar'");
-		editor.selectAndReveal(3, 0);		
-		assertNotNull(openConentAssist());
+		editor.selectAndReveal(3, 0);
+		assertNotNull("Shell is expected to open for completion proposals", openContentAssist());
 	}
 
-	private Shell openConentAssist() {
-		return openConentAssist(true);
+	private Shell openContentAssistWithLongRunningProposalComputation() {
+		LongRunningBarContentAssistProcessor.enable();
+		Shell shell = openContentAssist();
+		assertNotNull("Shell is expected to open for completion proposals", shell);
+		return shell;
 	}
-	private Shell openConentAssist(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);
+		Shell shell = findNewShell(beforeShells, editor.getSite().getShell().getDisplay());
 		waitAndDispatch(100); // can dispose shell when focus lost during debugging
+		if (shell != null) {
+			this.completionShell = shell;
+		}
 		return shell;
 	}
 
@@ -177,25 +186,25 @@ private Shell openConentAssist(boolean expectShell) {
 	 */
 	private void checkCompletionContent(final Table completionProposalList) {
 		// should be instantaneous, but happens to go asynchronous on CI so let's allow a wait
-		waitForProposalRelatedCondition("Proposal list did not show two initial items", completionProposalList, 
+		waitForProposalRelatedCondition("Proposal list should show two initial items", completionProposalList, 
 				() -> completionProposalList.getItemCount() == 2, 200);
 		assertTrue("Missing computing info entry", isComputingInfoEntry(completionProposalList.getItem(0)));
-		assertTrue("Missing computing info entry in proposal list", isComputingInfoEntry(completionProposalList.getItem(0)));
 		final TableItem initialProposalItem = completionProposalList.getItem(1);
 		final String initialProposalString = ((ICompletionProposal)initialProposalItem.getData()).getDisplayString();
 		assertThat("Unexpected initial proposal item", 
 				BarContentAssistProcessor.PROPOSAL, endsWith(initialProposalString));
 		completionProposalList.setSelection(initialProposalItem);
+
+		LongRunningBarContentAssistProcessor.finish();
 		// asynchronous
-		waitForProposalRelatedCondition("Proposal list did not show two items after finishing computing", completionProposalList, 
-				() -> !isComputingInfoEntry(completionProposalList.getItem(0)) && completionProposalList.getItemCount() == 2,
-				LongRunningBarContentAssistProcessor.DELAY + 200);
+		waitForProposalRelatedCondition("Proposal list should contain two items", completionProposalList, 
+				() -> !isComputingInfoEntry(completionProposalList.getItem(0)) && completionProposalList.getItemCount() == 2, 5000);
 		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 secondCompletionProposalText =  ((ICompletionProposal)secondCompletionProposalItem.getData()).getDisplayString();
 		assertThat("Unexpected first proposal item", BarContentAssistProcessor.PROPOSAL, endsWith(firstCompletionProposalText));
-		assertThat("Unexpected second proposal item", LongRunningBarContentAssistProcessor.PROPOSAL, endsWith(secondCOmpletionProposalText));
+		assertThat("Unexpected second proposal item", LongRunningBarContentAssistProcessor.PROPOSAL, endsWith(secondCompletionProposalText));
 		String selectedProposalString = ((ICompletionProposal)completionProposalList.getSelection()[0].getData()).getDisplayString();
 		assertEquals("Addition of completion proposal should keep selection", initialProposalString, selectedProposalString);
 	}
@@ -204,24 +213,23 @@ 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) {
 		Shell[] afterShells = Arrays.stream(display.getShells())
 				.filter(Shell::isVisible)
 				.filter(shell -> !beforeShells.contains(shell))
 				.toArray(Shell[]::new);
-		if (expectShell) {
-			assertEquals("No new shell found", 1, afterShells.length);
-		}
+		assertTrue("More than one new shell was found", afterShells.length <= 1);
 		return afterShells.length > 0 ? afterShells[0] : null;
 	}
 
 	@Test
 	public void testCompletionFreeze_bug521484() throws Exception {
 		editor.selectAndReveal(3, 0);
-		this.completionShell=openConentAssist();
-		final Table completionProposalList = findCompletionSelectionControl(this.completionShell);
+		final Shell shell = openContentAssistWithLongRunningProposalComputation();
+		assertNotNull("Shell is expected to open for completion proposals", shell);
+		final Table completionProposalList = findCompletionSelectionControl(shell);
 		// should be instantaneous, but happens to go asynchronous on CI so let's allow a wait
-		waitForProposalRelatedCondition("Proposal list did not show two items", completionProposalList, 
+		waitForProposalRelatedCondition("Proposal list should show two items", completionProposalList, 
 				() -> completionProposalList.getItemCount() == 2, 200);
 		assertTrue("Missing computing info entry", isComputingInfoEntry(completionProposalList.getItem(0)));
 		// Some processors are long running, moving cursor can cause freeze (bug 521484)
@@ -230,18 +238,24 @@ public void testCompletionFreeze_bug521484() throws Exception {
 		emulatePressLeftArrowKey();
 		DisplayHelper.sleep(editor.getSite().getShell().getDisplay(), 200); //give time to process events
 		long processingDuration = System.currentTimeMillis() - timestamp;
-		assertTrue("UI Thread frozen for " + processingDuration + "ms", processingDuration < LongRunningBarContentAssistProcessor.DELAY);
+		assertTrue("UI Thread frozen for " + processingDuration + "ms", processingDuration < LongRunningBarContentAssistProcessor.TIMEOUT_MSEC);
 	}
 
 	@Test
 	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());
 		DisplayHelper.sleep(editor.getSite().getShell().getDisplay(), 200);
-		this.completionShell= findNewShell(beforeShells, editor.getSite().getShell().getDisplay(), true);
-		final Table completionProposalList = findCompletionSelectionControl(this.completionShell);
-		checkCompletionContent(completionProposalList);
+		assertTrue("Completion proposal shell should be disposed after moving the cusor", shell.isDisposed());
+		this.completionShell = findNewShell(beforeShells, editor.getSite().getShell().getDisplay());
+		assertNotNull("Shell is expected to open for completion proposals", completionShell);
+		final Table newCompletionProposalList = findCompletionSelectionControl(completionShell);
+		checkCompletionContent(newCompletionProposalList);
 	}
 
 	private void emulatePressLeftArrowKey() {
@@ -276,6 +290,11 @@ public void closeShell() {
 			completionShell.close();
 		}
 	}
+	
+	@After
+	public void stopLongRunningCompletionProposalProcessor() {
+		LongRunningBarContentAssistProcessor.finish();
+	}
 
 	private static final class TestLogListener implements ILogListener {
 

tests/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/TestQuickAssist.java

@@ -15,6 +15,7 @@
 package org.eclipse.ui.genericeditor.tests;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
 import java.util.Arrays;
@@ -103,7 +104,8 @@ private Shell openQuickAssist() {
 		action.update();
 		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);
+		Shell shell= CompletionTest.findNewShell(beforeShells, editor.getSite().getShell().getDisplay());
+		assertNotNull("Shell is expected to open for quick assist", shell);
 		waitAndDispatch(100);
 		return shell;
 	}

tests/org.eclipse.ui.genericeditor.tests/src/org/eclipse/ui/genericeditor/tests/contributions/LongRunningBarContentAssistProcessor.java

@@ -19,20 +19,32 @@
 public class LongRunningBarContentAssistProcessor extends BarContentAssistProcessor {
 
 	public static final String 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 boolean running = false;
+	
 	public LongRunningBarContentAssistProcessor() {
 		super(PROPOSAL);
 	}
 
 	@Override
 	public ICompletionProposal[] computeCompletionProposals(ITextViewer viewer, int offset) {
 		try {
-			Thread.sleep(DELAY);
+			long startExecutionTime = System.currentTimeMillis();
+			while (running && (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 = true;
+	}
+	
+	public static void finish() {
+		running = false;
+	}
+	
 }