Improved use of `java.io.tmpdir` #838

jglick · 2024-09-18T19:15:00Z

Ensure that temporary files are written to target/tmp/ (where mvn clean can delete them) rather than the system temporary directory, including in RealJenkinsRule forked JVMs; note that in the test JVM it seems to be necessary to look up this system property explicitly, I think since Surefire sets the system property after startup rather than using -D, and something reads and caches the value soon after startup. Ensure that all temporary directories are deleted when the test exits. Use temporary directories instead of $JENKINS_HOME for anything added by the test harness rather than Jenkins itself. (Extracted from #827.)

jglick · 2024-09-18T19:18:07Z

src/main/java/org/jvnet/hudson/test/UnitTestSupportingPluginManager.java

@@ -63,7 +63,7 @@
 public class UnitTestSupportingPluginManager extends PluginManager {

    public UnitTestSupportingPluginManager(File rootDir) {
-        super((ServletContext) null, new File(rootDir, "plugins"));
+        super((ServletContext) null, rootDir);


jenkins-test-harness/src/main/java/org/jvnet/hudson/test/TestPluginManager.java

Line 38 in e89db46

Util.deleteRecursive(INSTANCE.rootDir);

was deleting plugins but not the parent dir jenkinsXXX created by

jenkins-test-harness/src/main/java/org/jvnet/hudson/test/TestPluginManager.java

Line 50 in e89db46

super(Util.createTempDir());

jglick · 2024-09-18T19:18:50Z

src/main/java/org/jvnet/hudson/test/recipes/WithPluginManager.java

@@ -90,7 +90,7 @@ public void setup(JenkinsRule jenkinsRule, WithPluginManager recipe) throws Exce
        public void decorateHome(JenkinsRule jenkinsRule, File home) throws Exception {
            Class<? extends PluginManager> c = recipe.value();
            Constructor<? extends PluginManager> ctr = c.getDeclaredConstructor(File.class);
-            jenkinsRule.setPluginManager(ctr.newInstance(home));
+            jenkinsRule.setPluginManager(ctr.newInstance(new File(home, "plugins")));


To restore behavior for those tests requesting UnitTestSupportingPluginManager.

jglick · 2024-09-18T19:20:26Z

src/main/java/org/jvnet/hudson/test/JavaNetReverseProxy2.java

@@ -90,7 +101,6 @@ private String getMimeType(String path) {
     */
    public static synchronized JavaNetReverseProxy2 getInstance() throws Exception {
        if (INSTANCE == null) {
-            // TODO: think of a better location --- ideally inside the target/ dir so that clean would wipe them out


Already worked. https://github.com/jenkinsci/plugin-pom/blob/e807519c3869c298fca47578e1f38c2bdc3cc735/pom.xml#L155-L156 https://github.com/jenkinsci/plugin-pom/blob/e807519c3869c298fca47578e1f38c2bdc3cc735/pom.xml#L806

jglick · 2024-09-18T19:21:46Z

src/main/java/org/jvnet/hudson/test/RealJenkinsRule.java

-                }
-                if (home.get() != null) {
+                try {
+                    jenkinsOptions("--webroot=" + createTempDirectory("webroot"), "--pluginroot=" + createTempDirectory("pluginroot"));


Various controller packagings set these options to temp dirs, so I think it is realistic to set them here automatically. Otherwise the fallback is inside $JENKINS_HOME which is undesirable.

jglick · 2024-09-18T19:24:53Z

src/main/java/org/jvnet/hudson/test/RealJenkinsRule.java

                    try {
-                        deprovision();
+                        tmp.dispose();


May be confusing depending on whether or not you have whitespace displayed: now just deleting all temp dirs when the test completes, regardless of #610 mode. (The other code in deprovision did not seem important since a rule instance is only expected to be used once.)

jglick · 2024-09-18T19:26:29Z

src/main/java/org/jvnet/hudson/test/RealJenkinsRule.java

+     * and will honor {@code java.io.tmpdir} set by Surefire
+     * after {@code StaticProperty.JAVA_IO_TMPDIR} has been initialized.
+     */
+    public Path createTempDirectory(String prefix) throws IOException {


Exposing this utility since it can be useful from certain tests, and it is otherwise tedious to arrange for dirs to be deleted at test exit.

src/main/java/org/jvnet/hudson/test/InboundAgentRule.java

…rness into java.io.tmpdir

basil · 2024-09-19T15:56:05Z

java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:87)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.junit.Assert.assertTrue(Assert.java:53)
	at hudson.PluginManagerTest.uberClassLoaderIsAvailableDuringStart(PluginManagerTest.java:213)

org.jvnet.hudson.test.RealJenkinsRule$StepException: 
Remote step threw an exception: java.lang.AssertionError: 
Expected: is "/home/jenkins/agent/workspace/Core_jenkins_PR-9756/test/target/j h7251048736527506496/agent.jar"
     but: was "/home/jenkins/agent/workspace/Core_jenkins_PR-9756/test/target/agent.jar"
	at org.jvnet.hudson.test.RealJenkinsRule.runRemotely(RealJenkinsRule.java:998)
	at org.jvnet.hudson.test.RealJenkinsRule.runRemotely(RealJenkinsRule.java:983)
	at hudson.slaves.JNLPLauncherRealTest.then(JNLPLauncherRealTest.java:81)
	at hudson.slaves.JNLPLauncherRealTest.webSocket(JNLPLauncherRealTest.java:70)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at org.jvnet.hudson.test.RealJenkinsRule$1.evaluate(RealJenkinsRule.java:527)
Caused by: java.lang.AssertionError: 
Expected: is "/home/jenkins/agent/workspace/Core_jenkins_PR-9756/test/target/j h7251048736527506496/agent.jar"
     but: was "/home/jenkins/agent/workspace/Core_jenkins_PR-9756/test/target/agent.jar"
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:6)
	at hudson.slaves.JNLPLauncherRealTest$RunJobStep.run(JNLPLauncherRealTest.java:107)
	at org.jvnet.hudson.test.RealJenkinsRule$StepsToStep2.run(RealJenkinsRule.java:1419)
	at org.jvnet.hudson.test.RealJenkinsRule$Endpoint.lambda$doStep$0(RealJenkinsRule.java:1332)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at java.base/java.lang.Thread.run(Thread.java:840)

jtnord · 2024-09-20T17:01:23Z

src/main/java/org/jvnet/hudson/test/InboundAgentRule.java

@@ -465,7 +481,7 @@ private static AgentArguments getAgentArguments(JenkinsRule r, String name) thro
        if (!launcher.getWorkDirSettings().isDisabled()) {
            commandLineArgs = launcher.getWorkDirSettings().toCommandLineArgs(c);
        }
-        File agentJar = new File(r.jenkins.getRootDir(), "agent.jar");
+        File agentJar = new File(System.getProperty("java.io.tmpdir"), "agent.jar");


this appears to completely break some things:

e.g. -DforkCount=<number_greater_than_one> as the file may not exist for 2 forks at the time of checking (which use the same surefire temp directory) , is completely written by one and the other and then used whilst the other is overwriting it.
Or in Windows locking exceptions for the second attempt to write the file.

the ability to quickly switch between version using -DJenkins.version=x (you now need to mvn clean as opposed to just using the incremental compilation which is faster)

Yeah a temp file would be better for this case. I can prepare a patch for it.

Improved use of java.io.tmpdir

e89db46

jglick added the enhancement label Sep 18, 2024

jglick requested a review from Vlatombe September 18, 2024 19:15

jglick commented Sep 18, 2024

View reviewed changes

jglick mentioned this pull request Sep 18, 2024

Generalize catch to SocketException #839

Merged

Merge branch 'master' into java.io.tmpdir

832b0e3

Vlatombe approved these changes Sep 19, 2024

View reviewed changes

src/main/java/org/jvnet/hudson/test/InboundAgentRule.java Outdated Show resolved Hide resolved

Merge branch 'master' of https://github.com/jenkinsci/jenkins-test-ha…

b116c96

…rness into java.io.tmpdir

jglick marked this pull request as draft September 19, 2024 12:22

InboundAgentRule.after should delete just those work dirs it created

f3f72c7

jglick marked this pull request as ready for review September 19, 2024 13:17

jglick requested a review from Vlatombe September 19, 2024 13:17

Vlatombe approved these changes Sep 19, 2024

View reviewed changes

jglick merged commit 9e8bb87 into jenkinsci:master Sep 19, 2024
14 checks passed

jglick deleted the java.io.tmpdir branch September 19, 2024 13:45

basil mentioned this pull request Sep 19, 2024

Update dependency org.jenkins-ci.main:jenkins-test-harness to v2286 - autoclosed jenkinsci/jenkins#9759

Closed

1 task

This was referenced Sep 19, 2024

Revert WithPluginManager constructor behavior #842

Merged

Test regressions after JTH bump jenkinsci/jenkins#9762

Merged

jtnord reviewed Sep 20, 2024

View reviewed changes

jglick mentioned this pull request Sep 20, 2024

InboundAgentRule should use a temp file for agent.jar #843

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improved use of `java.io.tmpdir` #838

Improved use of `java.io.tmpdir` #838

jglick commented Sep 18, 2024

jglick Sep 18, 2024

jglick Sep 18, 2024

jglick Sep 18, 2024

jglick Sep 18, 2024

jglick Sep 18, 2024

jglick Sep 18, 2024

basil commented Sep 19, 2024

jtnord Sep 20, 2024 •

edited

Loading

jglick Sep 20, 2024

Improved use of java.io.tmpdir #838

Improved use of java.io.tmpdir #838

Conversation

jglick commented Sep 18, 2024

jglick Sep 18, 2024

Choose a reason for hiding this comment

jglick Sep 18, 2024

Choose a reason for hiding this comment

jglick Sep 18, 2024

Choose a reason for hiding this comment

jglick Sep 18, 2024

Choose a reason for hiding this comment

jglick Sep 18, 2024

Choose a reason for hiding this comment

jglick Sep 18, 2024

Choose a reason for hiding this comment

basil commented Sep 19, 2024

jtnord Sep 20, 2024 • edited Loading

Choose a reason for hiding this comment

jglick Sep 20, 2024

Choose a reason for hiding this comment

Improved use of `java.io.tmpdir` #838

Improved use of `java.io.tmpdir` #838

jtnord Sep 20, 2024 •

edited

Loading