[NO_MERGE] [JENKINS-16316] - Bug fix and additional tests

pom.xml

@@ -1,5 +1,5 @@
 <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+    xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
 
     <modelVersion>4.0.0</modelVersion>
 
@@ -43,13 +43,31 @@
         <ivy.plugin.version>1.21</ivy.plugin.version>
         <junit.version>4.9</junit.version>
         <mockito.version>1.8.5</mockito.version>
+        <powermock.version>1.4.9</powermock.version>
     </properties>
 
     <scm>
         <connection>scm:git:git://github.com/jenkinsci/envinject-plugin.git</connection>
         <developerConnection>scm:git:[email protected]:jenkinsci/envinject-plugin.git</developerConnection>
     </scm>
 
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-surefire-plugin</artifactId>
+                <version>2.14.1</version>
+                <dependencies>
+                    <dependency>
+                        <groupId>org.apache.maven.surefire</groupId>
+                        <artifactId>surefire-junit4</artifactId>
+                        <version>2.14.1</version>
+                    </dependency>
+                </dependencies>
+            </plugin>
+        </plugins>
+    </build>
+
     <dependencies>
         <dependency>
             <groupId>org.jenkins-ci.main</groupId>
@@ -82,6 +100,15 @@
             <version>${mockito.version}</version>
             <scope>test</scope>
         </dependency>
+
+        <dependency>
+            <groupId>org.powermock</groupId>
+            <artifactId>powermock-mockito-release-full</artifactId>
+            <version>${powermock.version}</version>
+            <type>pom</type>
+            <scope>test</scope>
+        </dependency>
+
     </dependencies>
 
     <repositories>
@@ -98,6 +125,6 @@
         </pluginRepository>
     </pluginRepositories>
 
-</project>  
-  
+</project>
+
 

src/main/java/org/jenkinsci/plugins/envinject/EnvInjectListener.java

@@ -246,7 +246,7 @@ public void buildEnvVars(Map<String, String> env) {
         };
     }
 
-    private Environment setUpEnvironmentWithoutJobPropertyObject(AbstractBuild build, Launcher launcher, BuildListener listener) throws IOException, InterruptedException, EnvInjectException {
+    protected Environment setUpEnvironmentWithoutJobPropertyObject(AbstractBuild build, Launcher launcher, BuildListener listener) throws IOException, InterruptedException, EnvInjectException {
 
         final Map<String, String> resultVariables = new HashMap<String, String>();
 

src/main/java/org/jenkinsci/plugins/envinject/service/EnvInjectVariableGetter.java

@@ -1,10 +1,9 @@
 package org.jenkinsci.plugins.envinject.service;
 
-import hudson.EnvVars;
 import hudson.Util;
 import hudson.matrix.MatrixRun;
 import hudson.model.*;
-import hudson.util.LogTaskListener;
+import hudson.slaves.EnvironmentVariablesNodeProperty;
 import org.jenkinsci.lib.envinject.EnvInjectException;
 import org.jenkinsci.lib.envinject.EnvInjectLogger;
 import org.jenkinsci.plugins.envinject.EnvInjectJobProperty;
@@ -16,7 +15,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.TreeMap;
-import java.util.logging.Level;
 import java.util.logging.Logger;
 
 /**
@@ -54,6 +52,12 @@ public Map<String, String> getJenkinsSystemVariables(boolean forceOnMaster) thro
         result.put("JENKINS_HOME", Hudson.getInstance().getRootDir().getPath());
         result.put("HUDSON_HOME", Hudson.getInstance().getRootDir().getPath());   // legacy compatibility
 
+        List<EnvironmentVariablesNodeProperty> globalNodeProperties = Hudson.getInstance().getGlobalNodeProperties()
+            .getAll(EnvironmentVariablesNodeProperty.class);
+        for (EnvironmentVariablesNodeProperty environmentVariablesNodeProperty : globalNodeProperties) {
+            result.putAll(environmentVariablesNodeProperty.getEnvVars());
+        }
+
         return result;
     }
 
@@ -66,11 +70,12 @@ public Map<String, String> getBuildVariables(AbstractBuild build, EnvInjectLogge
         result.putAll(build.getCharacteristicEnvVars());
 
         try {
-            EnvVars envVars = new EnvVars();
-            for (EnvironmentContributor ec : EnvironmentContributor.all()) {
-                ec.buildEnvironmentFor(build, envVars, new LogTaskListener(LOG, Level.ALL));
-                result.putAll(envVars);
-            }
+//            EnvVars envVars = new EnvVars();
+//            for (EnvironmentContributor ec : EnvironmentContributor.all()) {
+//                ec.buildEnvironmentFor(build, envVars, new LogTaskListener(LOG, Level.ALL));
+//                result.putAll(envVars);
+//            }
+
 
             JDK jdk = build.getProject().getJDK();
             if (jdk != null) {

src/test/java/org/jenkinsci/plugins/envinject/GlobalPropertiesTest.java

@@ -1,17 +1,27 @@
 package org.jenkinsci.plugins.envinject;
 
 import hudson.model.FreeStyleBuild;
+import hudson.model.Result;
+import hudson.model.Computer;
 import hudson.model.FreeStyleProject;
 import hudson.model.Hudson;
-import hudson.model.Result;
-import hudson.slaves.EnvironmentVariablesNodeProperty;
+import hudson.model.Node;
 import hudson.slaves.NodeProperty;
 import hudson.slaves.NodePropertyDescriptor;
+import hudson.slaves.EnvironmentVariablesNodeProperty;
+import hudson.slaves.SlaveComputer;
+import hudson.tasks.Shell;
 import hudson.util.DescribableList;
-import junit.framework.Assert;
-import org.jvnet.hudson.test.HudsonTestCase;
 
 import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Set;
+
+import junit.framework.Assert;
+
+import org.apache.commons.lang.StringUtils;
+import org.junit.Test;
+import org.jvnet.hudson.test.HudsonTestCase;
 
 /**
  * @author Gregory Boissinot
@@ -27,7 +37,7 @@ public void setUp() throws Exception {
         project = createFreeStyleProject();
     }
 
-
+    @Test
     public void testGlobalPropertiesWithWORKSPACE() throws Exception {
 
         final String testWorkspaceVariableName = "TEST_WORKSPACE";
@@ -59,6 +69,7 @@ public void testGlobalPropertiesWithWORKSPACE() throws Exception {
     }
 
     //Specific use case: We set a global workspace at job level
+    @Test
     public void testGlobalPropertiesSetWORKSPACE() throws Exception {
 
         final String testGlobalVariableName = "WORKSPACE";
@@ -90,4 +101,57 @@ public void testGlobalPropertiesSetWORKSPACE() throws Exception {
 
         assertEquals(result_testGlobalVariableName, result_testJobVariableName);
     }
+
+    @Test
+    public void testChangeOfGlobalPropertyGetsRecognizedWhenWithoutJobPropertyAndRunOnSlaves() throws Exception {
+
+        final String testVariableName = "TESTVAR";
+        final String testVariableValue = "value1";
+        final String testVariableValueAfterChange = "value2";
+
+        //A global node property TESTVAR
+        DescribableList<NodeProperty<?>, NodePropertyDescriptor> globalNodeProperties = Hudson.getInstance().getGlobalNodeProperties();
+        EnvironmentVariablesNodeProperty.Entry testVarEntry = new EnvironmentVariablesNodeProperty.Entry(testVariableName, testVariableValue);
+        EnvironmentVariablesNodeProperty testVarNodePropertyItem = new EnvironmentVariablesNodeProperty(testVarEntry);
+        globalNodeProperties.add(testVarNodePropertyItem);
+
+        Node slaveNode = createOnlineSlave();
+        project.setAssignedNode(slaveNode);
+        project.getBuildersList().add(new Shell("echo \"TESTVAR=$TESTVAR\""));
+
+        // we do NOT add a jobProperty - we want to test "default" behaviour of jenkins with installed envinject plugin
+        // so we run the build right away
+        FreeStyleBuild build = project.scheduleBuild2(0).get();
+        Assert.assertEquals(Result.SUCCESS, build.getResult());
+        assertEquals(slaveNode.getNodeName(), build.getBuiltOn().getNodeName());
+
+        // assert correct injection of testVariable #1
+        org.jenkinsci.lib.envinject.EnvInjectAction action = build.getAction(org.jenkinsci.lib.envinject.EnvInjectAction.class);
+        Map<String, String> envVars = action.getEnvMap();
+        String actualTestVariableValueInBuild = envVars.get(testVariableName);
+        assertNotNull("actual testVariableValue is null", actualTestVariableValueInBuild);
+        assertEquals(testVariableValue, actualTestVariableValueInBuild);
+
+        Set<Entry<String, String>> beforeChange = hudson.getComputer(slaveNode.getNodeName()).getEnvironment().entrySet();
+
+        // now we change the global property variable value...
+        testVarEntry = new EnvironmentVariablesNodeProperty.Entry(testVariableName, testVariableValueAfterChange);
+        Hudson.getInstance().getGlobalNodeProperties().add(new EnvironmentVariablesNodeProperty(testVarEntry));
+
+        Set<Entry<String, String>> afterChange = hudson.getComputer(slaveNode.getNodeName()).getEnvironment().entrySet();
+        // environment of the slave does not change without restarting it. assert it to make test fail if there will be
+        // some kind of auto-restart to reload config.
+        Assert.assertEquals(beforeChange, afterChange);
+        Assert.assertEquals(beforeChange.toString(), afterChange.toString());
+
+        //...run the job again...
+        FreeStyleBuild secondBuild = project.scheduleBuild2(0).get();
+        Assert.assertEquals(Result.SUCCESS, secondBuild.getResult());
+        assertEquals(slaveNode.getNodeName(), secondBuild.getBuiltOn().getNodeName());
+
+        //...and expect the testvariable to have the changed value
+        org.jenkinsci.lib.envinject.EnvInjectAction action2 = secondBuild.getAction(org.jenkinsci.lib.envinject.EnvInjectAction.class);
+        assertNotNull("actual testVariableValue is null", action2.getEnvMap().get(testVariableName));
+        assertEquals(testVariableValueAfterChange, action2.getEnvMap().get(testVariableName));
+    }
 }

src/test/java/org/jenkinsci/plugins/envinject/sevice/EnvInjectVariableGetterTest.java

@@ -1,23 +1,41 @@
 package org.jenkinsci.plugins.envinject.sevice;
 
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import hudson.EnvVars;
 import hudson.matrix.MatrixRun;
 import hudson.model.AbstractBuild;
+import hudson.model.Computer;
+import hudson.model.Hudson;
+import hudson.model.Node;
+import hudson.model.labels.LabelAtom;
+import hudson.slaves.EnvironmentVariablesNodeProperty;
+import hudson.slaves.NodeProperty;
+import hudson.slaves.NodePropertyDescriptor;
+import hudson.util.DescribableList;
+
+import java.io.File;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+
+import junit.framework.Assert;
+
 import org.jenkinsci.lib.envinject.EnvInjectLogger;
 import org.jenkinsci.plugins.envinject.EnvInjectPluginAction;
 import org.jenkinsci.plugins.envinject.service.EnvInjectVariableGetter;
 import org.junit.Before;
 import org.junit.Test;
-
-import java.util.HashMap;
-import java.util.Map;
-
-import static org.junit.Assert.assertTrue;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
+import org.junit.runner.RunWith;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
 
 /**
  * @author Gregory Boissinot
  */
+@RunWith(PowerMockRunner.class)
 public class EnvInjectVariableGetterTest {
 
     private EnvInjectVariableGetter variableGetter;
@@ -65,6 +83,40 @@ public void getEnvVarsPreviousStepsWithEnvInjectActionMatrixRun() throws Excepti
         assertTrue(sameMap(expectedEnvVars, resultEnvVars));
     }
 
+    @Test
+    @PrepareForTest({ Computer.class, Hudson.class })
+    public void testGetJenkinsSystemVariablesForceFetchesGlobalNodesPropertiesFromMaster() throws Exception {
+
+        PowerMockito.mockStatic(Computer.class);
+        PowerMockito.mockStatic(Hudson.class);
+        Computer computer = mock(Computer.class);
+        Node node = mock(Node.class);
+        EnvVars envVars = new EnvVars();
+        final String PROPERTY_KEY = "PATH";
+        final String VALUE_FROM_SLAVE_COMPUTER = "VALUE_FROM_SLAVE_COMPUTER";
+        final String VALUE_FROM_GNP_MASTER = "VALUE_FROM_GNP_MASTER";
+        envVars.put(PROPERTY_KEY, VALUE_FROM_SLAVE_COMPUTER);
+        Hudson hudson = mock(Hudson.class);
+
+        when(Computer.currentComputer()).thenReturn(computer);
+        when(computer.getNode()).thenReturn(node);
+        when(computer.getEnvironment()).thenReturn(envVars);
+        when(node.getAssignedLabels()).thenReturn(new HashSet<LabelAtom>());
+        when(computer.getName()).thenReturn("slave0");
+        when(Hudson.getInstance()).thenReturn(hudson);
+        when(hudson.getRootDir()).thenReturn(new File(""));
+
+        DescribableList<NodeProperty<?>, NodePropertyDescriptor> globalNodeProperties = new DescribableList<NodeProperty<?>, NodePropertyDescriptor>(
+            hudson);
+        EnvironmentVariablesNodeProperty property = new EnvironmentVariablesNodeProperty(
+            new EnvironmentVariablesNodeProperty.Entry(PROPERTY_KEY, VALUE_FROM_GNP_MASTER));
+        globalNodeProperties.add(property);
+        when(Hudson.getInstance().getGlobalNodeProperties()).thenReturn(globalNodeProperties);
+
+        Map<String, String> jenkinsSystemVariables = variableGetter.getJenkinsSystemVariables(false);
+        Assert.assertNotNull(jenkinsSystemVariables);
+        Assert.assertEquals(VALUE_FROM_GNP_MASTER, jenkinsSystemVariables.get(PROPERTY_KEY));
+    }
 
     private boolean sameMap(Map<String, String> expectedMap, Map<String, String> actualMap) {