Skip to content

Commit 865f7d0

Browse files
fbriconclaude
authored and
angelozerr
committed
Replace Thread.sleep() with proper synchronization in tests
Tests were using Thread.sleep() (up to 5 seconds each) to wait for async operations like resource downloads and validation triggers, making the test suite unnecessarily slow (~71s test time). Key changes: - BaseFileTempTest: replace sleep(1050) with Files.setLastModifiedTime() to ensure filesystem change detection without waiting - XMLLanguageService: return download futures from publishDiagnostics() so tests can await them directly - XMLAssert: add awaitDownloads() helper, remove retrigger callback - MockXMLLanguageClient: add waitForDiagnosticCount() polling helper - Replace all 5-second HACK sleeps with awaitDownloads() - Replace async validation sleeps with waitForDiagnosticCount() - DocumentLifecycleParticipantTest: poll for lifecycle callback completion - CacheResourcesManagerTest: reduce cache TTL from 1s to 100ms Build time reduced from ~1m17s to ~15s (5x faster). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent c525ca6 commit 865f7d0

14 files changed

Lines changed: 100 additions & 73 deletions

‎org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/XMLLanguageService.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
*/
1313
package org.eclipse.lemminx.services;
1414

15-
import java.nio.file.Path;
1615
import java.util.List;
1716
import java.util.Map;
1817
import java.util.concurrent.CompletableFuture;
@@ -190,7 +189,7 @@ public List<Diagnostic> doDiagnostics(DOMDocument xmlDocument, XMLValidationSett
190189
return diagnostics.doDiagnostics(xmlDocument, validationSettings, validationArgs, cancelChecker);
191190
}
192191

193-
public CompletableFuture<Path> publishDiagnostics(DOMDocument xmlDocument,
192+
public CompletableFuture<Void> publishDiagnostics(DOMDocument xmlDocument,
194193
Consumer<PublishDiagnosticsParams> publishDiagnostics, Consumer<TextDocument> triggerValidation,
195194
XMLValidationRootSettings validationSettings, Map<String, Object> validationArgs,
196195
CancelChecker cancelChecker) {
@@ -218,6 +217,7 @@ public CompletableFuture<Path> publishDiagnostics(DOMDocument xmlDocument,
218217
triggerValidation.accept(document);
219218
return null;
220219
});
220+
return allFutures;
221221
}
222222
return null;
223223
}

‎org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/MockXMLLanguageClient.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import java.util.ArrayList;
1515
import java.util.List;
1616
import java.util.concurrent.CompletableFuture;
17+
import java.util.concurrent.TimeUnit;
1718

1819
import org.eclipse.lemminx.customservice.ActionableNotification;
1920
import org.eclipse.lemminx.customservice.XMLLanguageClientAPI;
@@ -98,6 +99,25 @@ public List<PublishDiagnosticsParams> getPublishDiagnostics() {
9899
return publishDiagnostics;
99100
}
100101

102+
/**
103+
* Wait until the total number of published diagnostics reaches the expected
104+
* count, or the timeout expires.
105+
*/
106+
public void waitForDiagnosticCount(int expectedCount, long timeoutMs) {
107+
long deadline = System.currentTimeMillis() + timeoutMs;
108+
while (publishDiagnostics.size() < expectedCount) {
109+
if (System.currentTimeMillis() >= deadline) {
110+
break;
111+
}
112+
try {
113+
Thread.sleep(20);
114+
} catch (InterruptedException e) {
115+
Thread.currentThread().interrupt();
116+
break;
117+
}
118+
}
119+
}
120+
101121
public List<MessageParams> getLogMessages() {
102122
return logMessages;
103123
}

‎org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/MockXMLLanguageServer.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ public List<PublishDiagnosticsParams> getPublishDiagnostics() {
4646
return getLanguageClient().getPublishDiagnostics();
4747
}
4848

49+
public void waitForDiagnosticCount(int expectedCount, long timeoutMs) {
50+
getLanguageClient().waitForDiagnosticCount(expectedCount, timeoutMs);
51+
}
52+
4953
@Override
5054
public MockXMLLanguageClient getLanguageClient() {
5155
return (MockXMLLanguageClient) super.getLanguageClient();

‎org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/XMLAssert.java‎

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import static org.junit.jupiter.api.Assertions.assertTrue;
2323
import static org.junit.jupiter.api.Assertions.fail;
2424

25-
import java.nio.file.Path;
2625
import java.nio.file.Paths;
2726
import java.util.ArrayList;
2827
import java.util.Arrays;
@@ -912,21 +911,41 @@ public static void publishDiagnostics(DOMDocument xmlDocument, List<PublishDiagn
912911

913912
public static void publishDiagnostics(DOMDocument xmlDocument, XMLValidationRootSettings validationSettings,
914913
List<PublishDiagnosticsParams> actual, XMLLanguageService languageService) {
915-
CompletableFuture<Path> error = languageService.publishDiagnostics(xmlDocument, params -> {
914+
languageService.publishDiagnostics(xmlDocument, params -> {
916915
actual.add(params);
917916
}, (doc) -> {
918-
// Retrigger validation
919-
publishDiagnostics(xmlDocument, actual, languageService);
920917
}, validationSettings, Collections.emptyMap(), () -> {
921918
});
919+
}
920+
921+
/**
922+
* Await completion of any in-progress resource downloads (XSD, DTD) for the
923+
* given XML content. This replaces Thread.sleep() calls in tests that need to
924+
* wait for async downloads to complete before re-validating.
925+
*/
926+
public static void awaitDownloads(XMLLanguageService ls, String xml, String fileURI) {
927+
awaitDownloads(ls, xml, fileURI, null);
928+
}
922929

923-
if (error != null) {
930+
/**
931+
* Await completion of any in-progress resource downloads (XSD, DTD) for the
932+
* given XML content. This replaces Thread.sleep() calls in tests that need to
933+
* wait for async downloads to complete before re-validating.
934+
*/
935+
public static void awaitDownloads(XMLLanguageService ls, String xml, String fileURI,
936+
XMLValidationRootSettings validationSettings) {
937+
DOMDocument xmlDocument = DOMParser.getInstance().parse(xml, fileURI, ls.getResolverExtensionManager());
938+
ls.setDocumentProvider((uri) -> xmlDocument);
939+
CompletableFuture<Void> downloadsFuture = ls.publishDiagnostics(xmlDocument,
940+
params -> {
941+
}, (doc) -> {
942+
}, validationSettings, Collections.emptyMap(), () -> {
943+
});
944+
if (downloadsFuture != null) {
924945
try {
925-
error.join();
926-
// Wait for 500 ms to collect the last params
927-
Thread.sleep(200);
946+
downloadsFuture.get(10, java.util.concurrent.TimeUnit.SECONDS);
928947
} catch (Exception e) {
929-
e.printStackTrace();
948+
// Expected - downloads may fail
930949
}
931950
}
932951
}

‎org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/BaseFileTempTest.java‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
import java.nio.file.Path;
1919
import java.nio.file.Paths;
2020
import java.nio.file.StandardOpenOption;
21+
import java.nio.file.attribute.FileTime;
22+
import java.time.Instant;
2123

2224
import org.eclipse.lemminx.AbstractCacheBasedTest;
2325

@@ -39,16 +41,12 @@ protected static void updateFile(String fileName, String contents) throws IOExce
3941
updateFile(fileURI, contents);
4042
}
4143
protected static void updateFile(URI fileURI, String contents) throws IOException {
42-
// For Mac OS, Linux OS, the call of Files.getLastModifiedTime is working for 1
43-
// second.
44-
// Here we wait for > 1s to be sure that call of Files.getLastModifiedTime will
45-
// work.
46-
try {
47-
Thread.sleep(1050);
48-
} catch (InterruptedException e) {
49-
Thread.currentThread().interrupt();
50-
}
5144
createFile(fileURI, contents);
45+
// Set the last modified time 2 seconds into the future to ensure
46+
// Files.getLastModifiedTime detects the change (filesystem timestamp
47+
// resolution on macOS/Linux is 1 second).
48+
Path path = Paths.get(fileURI);
49+
Files.setLastModifiedTime(path, FileTime.from(Instant.now().plusSeconds(2)));
5250
}
5351

5452
protected Path getTempDirPath() {

‎org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/XMLExternalTest.java‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
*/
4242
public class XMLExternalTest extends BaseFileTempTest {
4343

44-
private int threadSleepMs = 600;
4544
private MockXMLLanguageServer languageServer;
4645

4746
@BeforeEach
@@ -79,7 +78,7 @@ public void externalDTDTest() throws InterruptedException, IOException {
7978

8079
clientOpenFile(languageServer, xmlTextDocument);
8180

82-
Thread.sleep(threadSleepMs);
81+
languageServer.waitForDiagnosticCount(1, 5000);
8382

8483
List<PublishDiagnosticsParams> actualDiagnostics = languageServer.getPublishDiagnostics();
8584
assertEquals(1, actualDiagnostics.size());
@@ -88,7 +87,7 @@ public void externalDTDTest() throws InterruptedException, IOException {
8887
editFile(testDtd, 2, "");
8988
didChangedWatchedFiles(languageServer, testDtd);
9089

91-
Thread.sleep(threadSleepMs);
90+
languageServer.waitForDiagnosticCount(2, 5000);
9291

9392
assertEquals(2, actualDiagnostics.size());
9493
assertFalse(actualDiagnostics.get(1).getDiagnostics().isEmpty());
@@ -134,7 +133,7 @@ public void externalXSDTest() throws InterruptedException, IOException {
134133

135134
clientOpenFile(languageServer, xmlTextDocument);
136135

137-
Thread.sleep(threadSleepMs);
136+
languageServer.waitForDiagnosticCount(1, 5000);
138137

139138
List<PublishDiagnosticsParams> actualDiagnostics = languageServer.getPublishDiagnostics();
140139
assertEquals(1, actualDiagnostics.size());
@@ -143,7 +142,7 @@ public void externalXSDTest() throws InterruptedException, IOException {
143142
editFile(testXsd, 12, " maxOccurs=\"2\"/>");
144143
didChangedWatchedFiles(languageServer, testXsd);
145144

146-
Thread.sleep(threadSleepMs);
145+
languageServer.waitForDiagnosticCount(2, 5000);
147146

148147
assertEquals(2, actualDiagnostics.size());
149148
assertEquals("cvc-complex-type.2.4.f", actualDiagnostics.get(1).getDiagnostics().get(0).getCode().getLeft());

‎org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/XMLSchemaPublishDiagnosticsTest.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import static org.eclipse.lemminx.XMLAssert.r;
1717
import static org.eclipse.lemminx.XMLAssert.d;
1818

19-
import java.util.concurrent.TimeUnit;
2019
import java.util.function.Consumer;
2120

2221
import org.eclipse.lemminx.AbstractCacheBasedTest;
@@ -146,8 +145,6 @@ public void schemaWithUrlWithCache() throws Exception {
146145
"</invoice> \r\n" + //
147146
"";
148147

149-
TimeUnit.SECONDS.sleep(2); // HACK: to make the timing work on slow machines
150-
151148
// Downloading...
152149
XMLAssert.testPublishDiagnosticsFor(xml, fileURI, ls, pd(fileURI,
153150
new Diagnostic(r(2, 32, 2, 50),
@@ -156,7 +153,7 @@ public void schemaWithUrlWithCache() throws Exception {
156153
new Diagnostic(r(1, 1, 1, 8), "cvc-elt.1.a: Cannot find the declaration of element 'invoice'.",
157154
DiagnosticSeverity.Error, "xml", XMLSchemaErrorCode.cvc_elt_1_a.getCode())));
158155

159-
TimeUnit.SECONDS.sleep(5); // HACK: to make the timing work on slow machines
156+
XMLAssert.awaitDownloads(ls, xml, fileURI);
160157

161158
// Downloaded error
162159
XMLAssert.testPublishDiagnosticsFor(xml, fileURI, ls, pd(fileURI,

‎org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/XMLValidationExternalResourcesBasedOnDTDTest.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@
1717
import static org.eclipse.lemminx.XMLAssert.r;
1818
import static org.eclipse.lemminx.XMLAssert.testCodeActionsFor;
1919

20-
import java.util.concurrent.TimeUnit;
21-
2220
import org.eclipse.lemminx.AbstractCacheBasedTest;
2321
import org.eclipse.lemminx.XMLAssert;
2422
import org.eclipse.lemminx.extensions.contentmodel.model.ContentModelManager;
@@ -96,7 +94,7 @@ public void docTypeDownloadProblem() throws Exception {
9694
new Diagnostic(r(1, 1, 1, 13), "Element type \"root-element\" must be declared.",
9795
DiagnosticSeverity.Error, "xml", DTDErrorCode.MSG_ELEMENT_NOT_DECLARED.getCode())));
9896

99-
TimeUnit.SECONDS.sleep(5); // HACK: to make the timing work on slow machines
97+
XMLAssert.awaitDownloads(ls, xml, fileURI, validation);
10098

10199
// Downloaded error
102100
XMLAssert.testPublishDiagnosticsFor(xml, fileURI, validation, ls,
@@ -177,7 +175,7 @@ public void entityRefDownloadProblem() throws Exception {
177175
new Diagnostic(r(6, 1, 6, 7), "The entity \"abcd\" was referenced, but not declared.",
178176
DiagnosticSeverity.Error, "xml", DTDErrorCode.EntityNotDeclared.getCode())));
179177

180-
TimeUnit.SECONDS.sleep(5); // HACK: to make the timing work on slow machines
178+
XMLAssert.awaitDownloads(ls, xml, fileURI, validation);
181179

182180
// Downloaded error
183181
XMLAssert.testPublishDiagnosticsFor(xml, fileURI, validation, ls,

‎org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/XMLValidationExternalResourcesBasedOnXSDTest.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@
1717
import static org.eclipse.lemminx.XMLAssert.r;
1818
import static org.eclipse.lemminx.XMLAssert.testCodeActionsFor;
1919

20-
import java.util.concurrent.TimeUnit;
21-
2220
import org.eclipse.lemminx.AbstractCacheBasedTest;
2321
import org.eclipse.lemminx.XMLAssert;
2422
import org.eclipse.lemminx.extensions.contentmodel.model.ContentModelManager;
@@ -99,7 +97,7 @@ public void noNamespaceSchemaLocationDownloadProblem() throws Exception {
9997
new Diagnostic(r(0, 1, 0, 13), "cvc-elt.1.a: Cannot find the declaration of element 'root-element'.",
10098
DiagnosticSeverity.Error, "xml", XMLSchemaErrorCode.cvc_elt_1_a.getCode())));
10199

102-
TimeUnit.SECONDS.sleep(5); // HACK: to make the timing work on slow machines
100+
XMLAssert.awaitDownloads(ls, xml, fileURI, validation);
103101

104102
// Downloaded error
105103
XMLAssert.testPublishDiagnosticsFor(xml, fileURI, validation, ls, pd(fileURI,
@@ -173,7 +171,7 @@ public void schemaLocationDownloadProblem() throws Exception {
173171
new Diagnostic(r(0, 1, 0, 13), "cvc-elt.1.a: Cannot find the declaration of element 'root-element'.",
174172
DiagnosticSeverity.Error, "xml", XMLSchemaErrorCode.cvc_elt_1_a.getCode())));
175173

176-
TimeUnit.SECONDS.sleep(5); // HACK: to make the timing work on slow machines
174+
XMLAssert.awaitDownloads(ls, xml, fileURI, validation);
177175

178176
// Downloaded error
179177
XMLAssert.testPublishDiagnosticsFor(xml, fileURI, validation, ls, pd(fileURI,

‎org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/commands/XMLValidationCommandTest.java‎

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -86,10 +86,8 @@ public void validationFileCommand() throws Exception {
8686
// Open the XML document, the validation is triggered asynchronously
8787
TextDocumentIdentifier xmlIdentifier = languageServer.didOpen("test.xml", xml);
8888

89-
// Wait for:
90-
// - downloading of XSD from the HTTP server to lemminx cache
91-
// - validation triggers
92-
Thread.sleep(1000);
89+
// Wait for downloading of XSD and validation to trigger
90+
languageServer.waitForDiagnosticCount(2, 5000);
9391

9492
// 1.1 Validation test
9593

@@ -141,10 +139,8 @@ public void validationFileCommand() throws Exception {
141139
// Execute command cache
142140
languageServer.executeCommand(XMLValidationFileCommand.COMMAND_ID, xmlIdentifier).get();
143141

144-
// Wait for:
145-
// - downloading of XSD from the HTTP server to lemminx cache
146-
// - validation triggers
147-
Thread.sleep(1000);
142+
// Wait for downloading and validation to trigger
143+
languageServer.waitForDiagnosticCount(2, 5000);
148144

149145
// 3.1 Validation test
150146

@@ -224,9 +220,8 @@ public void validationAllFilesCommand() throws Exception {
224220
// Open the XML document, the validation is triggered asynchronously
225221
TextDocumentIdentifier xml1Identifier = languageServer.didOpen("test1.xml", xml1);
226222

227-
// Wait for to collect diagnostics in the proper order (XSD diagnostics followed
228-
// by DTD diagnostics)
229-
Thread.sleep(2000);
223+
// Wait for XSD diagnostics before opening second file
224+
languageServer.waitForDiagnosticCount(2, 5000);
230225

231226
// Open the second XML file bound to the DTD
232227
String xml2 = "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>\r\n" + //
@@ -238,10 +233,8 @@ public void validationAllFilesCommand() throws Exception {
238233
// Open the XML document, the validation is triggered asynchronously
239234
TextDocumentIdentifier xml2Identifier = languageServer.didOpen("test2.xml", xml2);
240235

241-
// Wait for:
242-
// - downloading of XSD from the HTTP server to lemminx cache
243-
// - validation triggers
244-
Thread.sleep(1000);
236+
// Wait for all 4 diagnostics (2 for each XML file)
237+
languageServer.waitForDiagnosticCount(4, 5000);
245238

246239
// 1.1 Validation test
247240

@@ -316,13 +309,10 @@ public void validationAllFilesCommand() throws Exception {
316309
XMLAssert.assertCompletion(list2, c("tag", "<tag />"), 5 /* region, endregion, cdata, comment, tag */);
317310

318311
// Execute command cache
319-
Thread.sleep(1000);
320312
languageServer.executeCommand(XMLValidationAllFilesCommand.COMMAND_ID).get();
321313

322-
// Wait for:
323-
// - downloading of XSD from the HTTP server to lemminx cache
324-
// - validation triggers
325-
Thread.sleep(1000);
314+
// Wait for downloading and validation to trigger (4 diagnostics: 2 per file)
315+
languageServer.waitForDiagnosticCount(4, 5000);
326316

327317
// 3.1 Validation test
328318

0 commit comments

Comments
 (0)