lemminx icon indicating copy to clipboard operation
lemminx copied to clipboard
verified publisher icon eclipse

Unit tests should not store data in ~/.lemminx

Open fbricon opened this issue 3 years ago • 2 comments

Unit tests currently write data in ~/.lemminx, this is wrong:

[INFO] Running XML Validation Command Test 2022-07-29 10:22:14.995:INFO::main: Logging initialized @19707ms to org.eclipse.jetty.util.log.StdErrLog 2022-07-29 10:22:15.094:INFO:oejs.Server:main: jetty-9.4.41.v20210516; built: 2021-05-16T23:56:28.993Z; git: 98607f93c7833e7dc59489b13f3cb0a114fb9f4c; jvm 17.0.1+12 2022-07-29 10:22:15.146:INFO:oejs.AbstractConnector:main: Started ServerConnector@7c40ffef{HTTP/1.1, (http/1.1)}{0.0.0.0:55113} 2022-07-29 10:22:15.146:INFO:oejs.Server:main: Started @19858ms Jul 29, 2022 10:22:15 AM org.eclipse.lemminx.uriresolver.FileServer start INFO: http server started on port 55113 Jul 29, 2022 10:22:15 AM org.eclipse.lemminx.uriresolver.FileServer getUri INFO: remote uri : http://localhost:55113/tag.xsd Jul 29, 2022 10:22:15 AM org.eclipse.lemminx.uriresolver.FileServer getUri INFO: remote uri : http://localhost:55113/tag.dtd Jul 29, 2022 10:22:15 AM org.eclipse.lemminx.uriresolver.CacheResourcesManager lambda$downloadResource$0 INFO: Downloading http://localhost:55113/tag.xsd to /Users/fbricon/.lemminx/cache/http/localhost/55113/tag.xsd... Jul 29, 2022 10:22:15 AM org.eclipse.lemminx.uriresolver.CacheResourcesManager lambda$downloadResource$0 INFO: Downloaded http://localhost:55113/tag.xsd to /Users/fbricon/.lemminx/cache/http/localhost/55113/tag.xsd in 111ms Jul 29, 2022 10:22:17 AM org.eclipse.lemminx.uriresolver.CacheResourcesManager lambda$downloadResource$0 INFO: Downloading http://localhost:55113/tag.dtd to /Users/fbricon/.lemminx/cache/http/localhost/55113/tag.dtd... Jul 29, 2022 10:22:17 AM org.eclipse.lemminx.uriresolver.CacheResourcesManager lambda$downloadResource$0 INFO: Downloaded http://localhost:55113/tag.dtd to /Users/fbricon/.lemminx/cache/http/localhost/55113/tag.dtd in 4ms Jul 29, 2022 10:22:21 AM org.eclipse.lemminx.uriresolver.CacheResourcesManager lambda$downloadResource$0 INFO: Downloading http://localhost:55113/tag.dtd to /Users/fbricon/.lemminx/cache/http/localhost/55113/tag.dtd... Jul 29, 2022 10:22:21 AM org.eclipse.lemminx.uriresolver.CacheResourcesManager lambda$downloadResource$0 INFO: Downloading http://localhost:55113/tag.xsd to /Users/fbricon/.lemminx/cache/http/localhost/55113/tag.xsd... Jul 29, 2022 10:22:21 AM org.eclipse.lemminx.uriresolver.CacheResourcesManager lambda$downloadResource$0 INFO: Downloaded http://localhost:55113/tag.dtd to /Users/fbricon/.lemminx/cache/http/localhost/55113/tag.dtd in 3ms Jul 29, 2022 10:22:21 AM org.eclipse.lemminx.uriresolver.CacheResourcesManager lambda$downloadResource$0 INFO: Downloaded http://localhost:55113/tag.xsd to /Users/fbricon/.lemminx/cache/http/localhost/55113/tag.xsd in 3ms 2022-07-29 10:22:22.373:INFO:oejs.AbstractConnector:main: Stopped ServerConnector@7c40ffef{HTTP/1.1, (http/1.1)}{0.0.0.0:0} 2022-07-29 10:22:22.375:INFO:oejs.Server:main: jetty-9.4.41.v20210516; built: 2021-05-16T23:56:28.993Z; git: 98607f93c7833e7dc59489b13f3cb0a114fb9f4c; jvm 17.0.1+12 2022-07-29 10:22:22.377:INFO:oejs.AbstractConnector:main: Started ServerConnector@5d05f453{HTTP/1.1, (http/1.1)}{0.0.0.0:55126} 2022-07-29 10:22:22.377:INFO:oejs.Server:main: Started @27089ms Jul 29, 2022 10:22:22 AM org.eclipse.lemminx.uriresolver.FileServer start

Screenshot 2022-07-29 at 10 24 56

Tests should use a clean cache directory, that would not pollute the user cache and also ensure idempotency

fbricon avatar Jul 29 '22 08:07 fbricon

+1

angelozerr avatar Jul 29 '22 08:07 angelozerr

It's difficult to know which tests may trigger the downloading behaviour, so the only way to be safe is to ensure all test suites use a different cache.

Easiest way is probably something like :

diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/AbstractCacheBasedTest.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/AbstractCacheBasedTest.java
index 51a4cf22..97fdd6c3 100644
--- a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/AbstractCacheBasedTest.java
+++ b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/AbstractCacheBasedTest.java
@@ -21,6 +21,7 @@ import com.google.common.io.RecursiveDeleteOption;
 import org.eclipse.lemminx.utils.FilesUtils;
 import org.eclipse.lemminx.utils.ProjectUtils;
 import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 
 /**
@@ -33,7 +34,6 @@ public abstract class AbstractCacheBasedTest {
 	@BeforeEach
 	public final void setupCache() throws Exception {
 		clearCache();
-		System.setProperty(FilesUtils.LEMMINX_WORKDIR_KEY, TEST_WORK_DIRECTORY.toAbsolutePath().toString());
 	}
 
 	@AfterEach
@@ -41,7 +41,6 @@ public abstract class AbstractCacheBasedTest {
 		if (Files.exists(TEST_WORK_DIRECTORY)) {
 			MoreFiles.deleteDirectoryContents(TEST_WORK_DIRECTORY,RecursiveDeleteOption.ALLOW_INSECURE);
 		}
-		System.clearProperty(FilesUtils.LEMMINX_WORKDIR_KEY);
 		FilesUtils.resetDeployPath();
 	}
 }
\ No newline at end of file
diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/utils/FilesUtilsTest.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/utils/FilesUtilsTest.java
index 86dcb797..12154179 100644
--- a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/utils/FilesUtilsTest.java
+++ b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/utils/FilesUtilsTest.java
@@ -31,6 +31,7 @@ public class FilesUtilsTest {
 
 	@Test
 	public void testFilesCachePathPreference() throws Exception {
+		String origWorkDir = System.getProperty(FilesUtils.LEMMINX_WORKDIR_KEY);
 		System.clearProperty(FilesUtils.LEMMINX_WORKDIR_KEY);
 		String newBasePathString = System.getProperty("user.home");
 		String newSubPathString = Paths.get("New", "Sub", "Path").toString();
@@ -38,6 +39,7 @@ public class FilesUtilsTest {
 		FilesUtils.setCachePathSetting(newBasePathString);
 		Path finalPath = FilesUtils.getDeployedPath(newSubPath);
 		assertEquals(Paths.get(newBasePathString, newSubPathString).toString(), finalPath.toString());
+		System.setProperty(FilesUtils.LEMMINX_WORKDIR_KEY, origWorkDir);
 	}
 
 	@Test
diff --git a/pom.xml b/pom.xml
index 2410dd21..72c7eabb 100644
--- a/pom.xml
+++ b/pom.xml
@@ -133,6 +133,7 @@
 					<version>3.0.0-M4</version>
 					<configuration>
 						<runOrder>random</runOrder>
+						<argLine>-Dlemminx.workdir=${project.build.directory}/test-cache/</argLine>
 						<statelessTestsetReporter implementation="org.apache.maven.plugin.surefire.extensions.junit5.JUnit5Xml30StatelessReporter">
 							<disable>false</disable>
 							<usePhrasedFileName>false</usePhrasedFileName>

The problem is those running the tests through an IDE (eg. Eclipse) would also need to make sure that property is set in the test launch configuration.

rgrunber avatar Aug 10 '22 21:08 rgrunber