More test fixups related to JTH change (#9767)

jenkinsci · Sep 21, 2024 · f5ccd65 · f5ccd65
1 parent 79d1f7a
commit f5ccd65
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 12 deletions.
diff --git a/test/src/test/java/hudson/slaves/JNLPLauncherRealTest.java b/test/src/test/java/hudson/slaves/JNLPLauncherRealTest.java
@@ -25,7 +25,7 @@
 package hudson.slaves;
 
 import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.endsWith;
+import static org.hamcrest.Matchers.hasKey;
 import static org.hamcrest.Matchers.is;
 
 import hudson.ExtensionList;
@@ -104,7 +104,7 @@ public void run(JenkinsRule r) throws Throwable {
             p.setAssignedNode(agent);
             FreeStyleBuild b = r.buildAndAssertSuccess(p);
             if (webSocket) {
-                assertThat(agent.toComputer().getSystemProperties().get("java.class.path").toString(), endsWith("agent.jar"));
+                assertThat(agent.toComputer().getSystemProperties(), hasKey("os.name"));
             }
             System.err.println(JenkinsRule.getLog(b));
         }

diff --git a/test/src/test/java/jenkins/security/Security3430Test.java b/test/src/test/java/jenkins/security/Security3430Test.java
@@ -38,9 +38,9 @@
 import org.hamcrest.Description;
 import org.hamcrest.Matcher;
 import org.hamcrest.TypeSafeMatcher;
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
 import org.jvnet.hudson.test.InboundAgentRule;
 import org.jvnet.hudson.test.JenkinsRule;
 import org.jvnet.hudson.test.RealJenkinsRule;
@@ -54,6 +54,9 @@ public class Security3430Test {
     @Rule
     public InboundAgentRule agents = new InboundAgentRule();
 
+    @Rule
+    public TemporaryFolder tmp = new TemporaryFolder();
+
     @Test
     public void runWithOldestSupportedAgentJar() throws Throwable {
         runWithRemoting(RemotingVersionInfo.getMinimumSupportedVersion().toString(), "/old-remoting/remoting-minimum-supported.jar", true);
@@ -64,36 +67,48 @@ public void runWithPreviousAgentJar() throws Throwable {
         runWithRemoting("3256.v88a_f6e922152", "/old-remoting/remoting-before-SECURITY-3430-fix.jar", true);
     }
 
-    @Ignore("TODO Expected: is an empty collection; but: <[Allowing URL: file:/…/test/target/webroot…/WEB-INF/lib/stapler-1903.v994a_db_314d58.jar, Determined to be core jar: file:/…/test/target/webroot…/WEB-INF/lib/stapler-1903.v994a_db_314d58.jar]>")
     @Test
     public void runWithCurrentAgentJar() throws Throwable {
-        runWithRemoting(null, null, false);
+        runWithRemoting(Launcher.VERSION, null, false);
     }
 
     private void runWithRemoting(String expectedRemotingVersion, String remotingResourcePath, boolean requestingJarFromAgent) throws Throwable {
-        if (expectedRemotingVersion != null) {
-            // TODO brittle; rather call InboundAgentRule.start(AgentArguments, Options) with a known agentJar
-            FileUtils.copyURLToFile(Security3430Test.class.getResource(remotingResourcePath), new File(System.getProperty("java.io.tmpdir"), "agent.jar"));
-        }
-
         jj.startJenkins();
         final String agentName = "agent1";
         try {
-            agents.createAgent(jj, InboundAgentRule.Options.newBuilder().name(agentName).build());
+            createAgent(agentName, remotingResourcePath);
             jj.runRemotely(Security3430Test::_run, agentName, expectedRemotingVersion, requestingJarFromAgent, true);
         } finally {
             agents.stop(jj, agentName);
         }
         jj.runRemotely(Security3430Test::disableJarURLValidatorImpl);
         final String agentName2 = "agent2";
         try {
-            agents.createAgent(jj, InboundAgentRule.Options.newBuilder().name(agentName2).build());
+            createAgent(agentName2, remotingResourcePath);
             jj.runRemotely(Security3430Test::_run, agentName2, expectedRemotingVersion, requestingJarFromAgent, false);
         } finally {
             agents.stop(jj, agentName2);
         }
     }
 
+    private void createAgent(String name, String remotingResourcePath) throws Throwable {
+        if (remotingResourcePath != null) {
+            var jar = tmp.newFile(name + ".jar");
+            FileUtils.copyURLToFile(Security3430Test.class.getResource(remotingResourcePath), jar);
+            // TODO awkward, especially as InboundAgentRule.getAgentArguments is private;
+            // would be helpful to have an option for a specific agent JAR:
+            var opts = InboundAgentRule.Options.newBuilder().name(name).skipStart().build();
+            agents.createAgent(jj, opts);
+            agents.start(new InboundAgentRule.AgentArguments(jj.getUrl() + "computer/" + name + "/slave-agent.jnlp", jar, jj.runRemotely(Security3430Test::getJnlpMac, name), 1, List.of()), opts);
+        } else {
+            agents.createAgent(jj, InboundAgentRule.Options.newBuilder().name(name).build());
+        }
+    }
+
+    private static String getJnlpMac(JenkinsRule r, String name) throws Throwable {
+        return ((SlaveComputer) r.jenkins.getComputer(name)).getJnlpMac();
+    }
+
     // This is quite artificial, but demonstrating that without JarURLValidatorImpl we do not allow any calls from the agent:
     private static void disableJarURLValidatorImpl(JenkinsRule j) {
         assertTrue(ExtensionList.lookup(ChannelConfigurator.class).remove(ExtensionList.lookupSingleton(JarURLValidatorImpl.class)));
@@ -114,6 +129,7 @@ private static void _run(JenkinsRule j, String agentName, String expectedRemotin
         final Computer computer = j.jenkins.getComputer(agentName);
         assertThat(computer, instanceOf(SlaveComputer.class));
         SlaveComputer agent = (SlaveComputer) computer;
+        j.waitOnline(agent.getNode());
         final Channel channel = agent.getChannel();
         if (expectedRemotingVersion != null) {
             final String result = channel.call(new AgentVersionCallable());