[FIX JENKINS-42232] Fix node executable not initialised on slave node…

… where Computer.currentComputer was null. In case current computer is null platform is calculated on the current system properties, that it is also the case of MasterToSlave callable.
jenkinsci · Feb 22, 2017 · dc4e294 · dc4e294
1 parent fa872a2
commit dc4e294
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 10 deletions.
diff --git a/src/main/java/jenkins/plugins/nodejs/tools/NodeJSInstallation.java b/src/main/java/jenkins/plugins/nodejs/tools/NodeJSInstallation.java
@@ -64,7 +64,7 @@
 public class NodeJSInstallation extends ToolInstallation implements EnvironmentSpecific<NodeJSInstallation>, NodeSpecific<NodeJSInstallation> {
 
     @SuppressFBWarnings(value = "SE_TRANSIENT_FIELD_NOT_RESTORED", justification = "calculate at runtime, its value depends on the OS where it run")
-    private final transient Platform platform;
+    private transient Platform platform;
 
     @DataBoundConstructor
     public NodeJSInstallation(@Nonnull String name, @Nonnull String home, List<? extends ToolProperty<?>> properties) {
@@ -173,17 +173,19 @@ private Platform getPlatform() throws DetectionFailedException {
         // missed call method forNode
         if (currentPlatform == null) {
             Computer computer = Computer.currentComputer();
-            if (computer == null) {
-                // pipeline use case
-                throw new DetectionFailedException(Messages.NodeJSBuilders_nodeOffline());
-            }
+            if (computer != null) {
+                Node node = computer.getNode();
+                if (node == null) {
+                    throw new DetectionFailedException(Messages.NodeJSBuilders_nodeOffline());
+                }
 
-            Node node = computer.getNode();
-            if (node == null) {
-                throw new DetectionFailedException(Messages.NodeJSBuilders_nodeOffline());
+                currentPlatform = Platform.of(node);
+            } else {
+                // pipeline or MasterToSlave use case
+                currentPlatform = Platform.current();
             }
 
-            currentPlatform = Platform.of(node);
+            platform = currentPlatform;
         }
 
         return currentPlatform;

diff --git a/src/test/java/jenkins/plugins/nodejs/tools/NodeJSInstallationTest.java b/src/test/java/jenkins/plugins/nodejs/tools/NodeJSInstallationTest.java
@@ -10,12 +10,14 @@
 import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
 import org.jvnet.hudson.test.recipes.LocalData;
+import org.powermock.reflect.Whitebox;
 import org.xml.sax.SAXException;
 
 import com.gargoylesoftware.htmlunit.html.HtmlButton;
 import com.gargoylesoftware.htmlunit.html.HtmlForm;
 import com.gargoylesoftware.htmlunit.html.HtmlPage;
 
+import hudson.model.Computer;
 import hudson.tools.InstallSourceProperty;
 import hudson.tools.ToolProperty;
 import hudson.tools.ToolPropertyDescriptor;
@@ -28,9 +30,22 @@ public class NodeJSInstallationTest {
     @Rule
     public JenkinsRule r = new JenkinsRule();
 
+    /**
+     * Verify node executable is begin initialised correctly on a slave
+     * node where {@link Computer#currentComputer()} is {@code null}.
+     */
+    @Issue("JENKINS-42232")
+    @Test
+    public void test_executable_resolved_on_slave_node() throws Exception {
+        assertNull(Computer.currentComputer());
+        NodeJSInstallation installation = new NodeJSInstallation("test_executable_resolved_on_slave_node", "/home/nodejs", null);
+        Platform platform = Whitebox.invokeMethod(installation, "getPlatform");
+        assertEquals(Platform.current(), platform);
+    }
+
     /**
      * Verify that the singleton installation descriptor load data from disk
-     * when on its constructor
+     * when its constructor is called.
      */
     @LocalData
     @Test