Renable DocumentLinkReconcilingTest.testClippedDocumentLinkReconciling

[rubenporras]

The test has been disabled in #1469 because it was failing consitently.

[FlorianKroiss]

Test failure might be related to eclipse-platform/eclipse.platform.ui#3585
We should check if the test passes again, once this is merged

[danthe1st]

As the person who contributed (at least some of) the relevant changes leading to that issue, I can provide the following information:

Test failure might be related to eclipse-platform/eclipse.platform.ui#3585 We should check if the test passes again, once this is merged

I can confirm that the exception no longer occurs with that change. However, the test still fails with a different exception if eclipse-platform/eclipse.platform.ui#3585 is applied:

org.opentest4j.AssertionFailedError: expected: <8> but was: <9>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:145)
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:531)
	at org.eclipse.lsp4e.test.documentLink.DocumentLinkReconcilingTest.testClippedDocumentLinkReconciling(DocumentLinkReconcilingTest.java:174)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)

This is because folding/projection regions can only fold full lines (at least at the end) and not partial lines (the linked PR makes sure that the last line is fully included vs fully excluded).

Before the changes in eclipse-platform/eclipse.platform.ui#3074, it was not possible to use projections and visible regions at all (it was generally disabled) but now (since it was merged in October) that works with the limitation that visible regions are limited to full lines if projections are enabled. The test fails because it expects more text to be shown.

Content the visible region is set to (substring of setVisibleRegion) - this would be what you get without calling enableProjections():

#LINK2_END 3rd-line #LINK3 3rd-line
#LINK4
5th-line #LINK5_START

Content that is actually made visible now (causing the exception you are facing):

#LINK2_END 3rd-line #LINK3 3rd-line
#LINK4

Content that is made visible with eclipse-platform/eclipse.platform.ui#3585:

#LINK2_END 3rd-line #LINK3 3rd-line
#LINK4
5th-line #LINK5_START

If you really want the old behavior, you should be able to call disableProjection() before setVisibleRegion() but I guess you might want to benefit from being able to use both folding/projections and visible regions simultaneously.

If you want to change the test to the behavior you'd get with eclipse-platform/eclipse.platform.ui#3585 merged, you'd get something like the following but I can't tell you whether that's what you want:

diff --git a/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/documentLink/DocumentLinkReconcilingTest.java b/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/documentLink/DocumentLinkReconcilingTest.java
index c4a71959..99fdd121 100644
--- a/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/documentLink/DocumentLinkReconcilingTest.java
+++ b/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/documentLink/DocumentLinkReconcilingTest.java
@@ -36,7 +36,6 @@
 import org.eclipse.lsp4j.Range;
 import org.eclipse.swt.custom.StyleRange;
 import org.eclipse.swt.graphics.Color;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
 public class DocumentLinkReconcilingTest extends AbstractTestWithProject {
@@ -138,7 +137,7 @@ public void testFullDocumentLinkReconciling() throws Exception {
 	}
 
 	@Test
-	@Disabled("Test faling because of SWT Exception due to invalid ranges")
+//	@Disabled("Test faling because of SWT Exception due to invalid ranges")
 	public void testClippedDocumentLinkReconciling() throws Exception {
 		MockLanguageServer.INSTANCE.setDocumentLinks(CONTENT_LINKS);
 
@@ -172,7 +171,7 @@ public void testClippedDocumentLinkReconciling() throws Exception {
 
 		var styles = viewer.getTextWidget().getStyleRanges();
 
-		assertEquals(8, styles.length);
+		assertEquals(9, styles.length);
 
 		pos = 0;
 		len = 0;
@@ -185,7 +184,8 @@ public void testClippedDocumentLinkReconciling() throws Exception {
 		assertEquals(textStyle(pos += len, len = 1, COLOR_4TH_LINE), styles[5]); // new line char
 
 		assertEquals(textStyle(pos += len, len = 9, COLOR_5TH_LINE), styles[6]);
-		assertEquals(linkStyle(pos += len, 12, COLOR_5TH_LINE), styles[7]);
+		assertEquals(linkStyle(pos += len, len = 12, COLOR_5TH_LINE), styles[7]);
+		assertEquals(linkStyle(pos += len, len = 11, null), styles[8]);
 	}

I hope this helps.

[rubenporras]

Hi @danthe1st , thanks for the help.

@RedeemerSK, do you have an opinion on how to adapt the test for the new behaviour in the platform? Otherwise I would just go along with the suggestion and merge the proposal (I created PR #1472 for that).

[danthe1st]

Otherwise I would just go along with the suggestion and merge the proposal

Note that it still depends on eclipse-platform/eclipse.platform.ui#3585 to be reviewed so merging it now might not be the best idea.