Stabilize two test that seem to often fail on the CI

- Use a timeout instead of static wait times - retry diagnostics - use a copy of the current state Signed-off-by: Christoph Läubrich <[email protected]>
eclipse-lemminx · Nov 10, 2024 · c1f7c79 · c1f7c79
1 parent 0aadd16
commit c1f7c79
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 13 deletions.
diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/MockXMLLanguageClient.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/MockXMLLanguageClient.java
@@ -14,6 +14,7 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CopyOnWriteArrayList;
 
 import org.eclipse.lemminx.customservice.ActionableNotification;
 import org.eclipse.lemminx.customservice.XMLLanguageClientAPI;
@@ -43,7 +44,7 @@ public class MockXMLLanguageClient implements XMLLanguageClientAPI {
 	private final List<MessageParams> logMessages;
 
 	public MockXMLLanguageClient() {
-		publishDiagnostics = new ArrayList<>();
+		publishDiagnostics = new CopyOnWriteArrayList<>();
 		showMessages = new ArrayList<>();
 		logMessages = new ArrayList<>();
 		actionableNotifications = new ArrayList<>();

diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/XMLAssert.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/XMLAssert.java
@@ -634,7 +634,7 @@ public static void testCompletionItemResolveFor(XMLLanguageService xmlLanguageSe
 		CompletionList resolved = new CompletionList(
 				list.getItems().stream() //
 						.map((item) -> {
-							return (CompletionItem) xmlLanguageService.resolveCompletionItem(item, htmlDoc,
+							return xmlLanguageService.resolveCompletionItem(item, htmlDoc,
 									sharedSettings,
 									() -> {
 									});
@@ -860,6 +860,24 @@ public static void testPublishDiagnosticsFor(String xml, String fileURI, XMLLang
 		testPublishDiagnosticsFor(xml, fileURI, null, xmlLanguageService, expected);
 	}
 
+	public static void testPublishDiagnosticsFor(long timeout, String xml, String fileURI,
+			XMLValidationRootSettings validationSettings, XMLLanguageService xmlLanguageService,
+			PublishDiagnosticsParams... expected) {
+		long deadline = System.currentTimeMillis() + timeout;
+		while (true) {
+			try {
+				testPublishDiagnosticsFor(xml, fileURI, validationSettings, xmlLanguageService, expected);
+				return;
+			} catch (AssertionError e) {
+				if (System.currentTimeMillis() < deadline) {
+					Thread.yield();
+					continue;
+				}
+				throw e;
+			}
+		}
+	}
+
 	public static void testPublishDiagnosticsFor(String xml, String fileURI,
 			XMLValidationRootSettings validationSettings,
 			XMLLanguageService xmlLanguageService, PublishDiagnosticsParams... expected) {

diff --git a/...se.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/XMLExternalTest.java b/...se.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/XMLExternalTest.java
@@ -13,11 +13,13 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.fail;
 
 import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.TimeUnit;
 
 import org.eclipse.lemminx.MockXMLLanguageServer;
 import org.eclipse.lemminx.XMLLanguageServer;
@@ -81,7 +83,7 @@ public void externalDTDTest() throws InterruptedException, IOException {
 
 		Thread.sleep(threadSleepMs);
 
-		List<PublishDiagnosticsParams> actualDiagnostics = languageServer.getPublishDiagnostics();
+		List<PublishDiagnosticsParams> actualDiagnostics = getDiagnostic(1);
 		assertEquals(1, actualDiagnostics.size());
 		assertEquals(0, actualDiagnostics.get(0).getDiagnostics().size());
 
@@ -134,21 +136,30 @@ public void externalXSDTest() throws InterruptedException, IOException {
 
 		clientOpenFile(languageServer, xmlTextDocument);
 
-		Thread.sleep(threadSleepMs);
-
-		List<PublishDiagnosticsParams> actualDiagnostics = languageServer.getPublishDiagnostics();
+		List<PublishDiagnosticsParams> actualDiagnostics = getDiagnostic(1);
 		assertEquals(1, actualDiagnostics.size());
 		assertEquals(0, actualDiagnostics.get(0).getDiagnostics().size());
 
 		editFile(testXsd, 12, "            maxOccurs=\"2\"/>");
 		didChangedWatchedFiles(languageServer, testXsd);
-
-		Thread.sleep(threadSleepMs);
-
+		actualDiagnostics = getDiagnostic(2);
 		assertEquals(2, actualDiagnostics.size());
 		assertEquals("cvc-complex-type.2.4.f", actualDiagnostics.get(1).getDiagnostics().get(0).getCode().getLeft());
 	}
 
+	private List<PublishDiagnosticsParams> getDiagnostic(int wanted) {
+		long deadline = System.currentTimeMillis() + TimeUnit.SECONDS.toMillis(5);
+		while (System.currentTimeMillis() < deadline) {
+			List<PublishDiagnosticsParams> list = new ArrayList<>(languageServer.getPublishDiagnostics());
+			if (list.size() >= wanted) {
+				return list;
+			}
+			Thread.yield();
+		}
+		fail("Did not recived at laest " + wanted + " diagnostics withing time frame!");
+		return new ArrayList<>();
+	}
+
 	private TextDocumentItem getXMLTextDocumentItem(String filename, String xmlContents) {
 		String languageId = "xml";
 		int version = 1;

diff --git a/...eclipse/lemminx/extensions/contentmodel/XMLValidationExternalResourcesBasedOnDTDTest.java b/...eclipse/lemminx/extensions/contentmodel/XMLValidationExternalResourcesBasedOnDTDTest.java
@@ -167,7 +167,7 @@ public void entityRefDownloadProblem() throws Exception {
 		String dtdCachePath = CacheResourcesManager.getResourceCachePath("http://localhost:8080/sample.dtd").toString();
 		String fileURI = "test.xml";
 		// Downloading...
-		XMLAssert.testPublishDiagnosticsFor(xml, fileURI, validation, ls,
+		XMLAssert.testPublishDiagnosticsFor(TimeUnit.SECONDS.toMillis(5), xml, fileURI, validation, ls,
 				pd(fileURI,
 						new Diagnostic(r(2, 32, 2, 64),
 								"The resource 'http://localhost:8080/sample.dtd' is downloading in the cache path '"
@@ -177,10 +177,8 @@ public void entityRefDownloadProblem() throws Exception {
 						new Diagnostic(r(6, 1, 6, 7), "The entity \"abcd\" was referenced, but not declared.",
 								DiagnosticSeverity.Error, "xml", DTDErrorCode.EntityNotDeclared.getCode())));
 
-		TimeUnit.SECONDS.sleep(5); // HACK: to make the timing work on slow machines
-
 		// Downloaded error
-		XMLAssert.testPublishDiagnosticsFor(xml, fileURI, validation, ls,
+		XMLAssert.testPublishDiagnosticsFor(TimeUnit.SECONDS.toMillis(5), xml, fileURI, validation, ls,
 				pd(fileURI,
 						new Diagnostic(r(2, 32, 2, 64),
 								"Error while downloading 'http://localhost:8080/sample.dtd' to '" + dtdCachePath