Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unit tests should not store data in the .lemminx folder under the user's home directory. #1265

Closed
fbricon opened this issue Jul 29, 2022 · 2 comments · Fixed by #1286
Closed
Assignees
Labels
build debt This issue or enhancement is related to technical debt
Milestone

Comments

@fbricon
Copy link
Contributor

fbricon commented Jul 29, 2022

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 fbricon added debt This issue or enhancement is related to technical debt build labels Jul 29, 2022
@angelozerr
Copy link
Contributor

+1

@rgrunber
Copy link
Contributor

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.

@datho7561 datho7561 added this to the 0.22.0 milestone Aug 31, 2022
@datho7561 datho7561 self-assigned this Aug 31, 2022
datho7561 added a commit to datho7561/lemminx that referenced this issue Aug 31, 2022
datho7561 added a commit to datho7561/lemminx that referenced this issue Sep 2, 2022
@angelozerr angelozerr changed the title Unit tests should not store data in ~/.lemminx Unit tests should not store data in the .lemminx folder under the user's home directory. Oct 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build debt This issue or enhancement is related to technical debt
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants