[JENKINS-26128] Demonstrating that non-block-scoped env sets are shad…

…owed by block context. Originally-Committed-As: 932adc8fd4a0c3b73cce54a657da2b7e7f985db1
jenkinsci · Mar 27, 2015 · 711927d · 711927d
1 parent 2f22fcd
commit 711927d
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 7 deletions.
diff --git a/aggregator/src/test/java/org/jenkinsci/plugins/workflow/steps/CoreWrapperStepTest.java b/aggregator/src/test/java/org/jenkinsci/plugins/workflow/steps/CoreWrapperStepTest.java
@@ -82,7 +82,6 @@ public class CoreWrapperStepTest {
             }
         });
     }
-
     public static class MockWrapper extends SimpleBuildWrapper {
         @DataBoundConstructor public MockWrapper() {}
         @Override public void setUp(Context context, Run<?,?> build, FilePath workspace, Launcher launcher, TaskListener listener, EnvVars initialEnvironment) throws IOException, InterruptedException {
@@ -97,15 +96,61 @@ private static final class DisposerImpl extends Disposer {
                 listener.getLogger().println("ran DisposerImpl");
             }
         }
-        @TestExtension public static class DescriptorImpl extends BuildWrapperDescriptor {
+        @TestExtension("useWrapper") public static class DescriptorImpl extends BuildWrapperDescriptor {
             @Override public String getDisplayName() {
                 return "MockWrapper";
             }
             @Override public boolean isApplicable(AbstractProject<?,?> item) {
                 return true;
             }
         }
+    }
 
+    @Test public void envStickiness() throws Exception {
+        story.addStep(new Statement() {
+            @Override public void evaluate() throws Throwable {
+                WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p");
+                p.setDefinition(new CpsFlowDefinition(
+                    "def show(which) {\n" +
+                    "  echo \"groovy ${which} ${env.TESTVAR}\"\n" +
+                    "  sh \"echo shell ${which} \\$TESTVAR\"\n" +
+                    "}\n" +
+                    "env.TESTVAR = 'initial'\n" +
+                    "node {\n" +
+                    "  wrap([$class: 'OneVarWrapper']) {\n" +
+                    "    show 'before'\n" +
+                    "    env.TESTVAR = 'edited'\n" +
+                    "    show 'after'\n" +
+                    "  }\n" +
+                    "  show 'outside'\n" +
+                    "}"));
+                WorkflowRun b = story.j.assertBuildStatusSuccess(p.scheduleBuild2(0));
+                story.j.assertLogContains("received initial", b);
+                story.j.assertLogContains("groovy before wrapped", b);
+                story.j.assertLogContains("shell before wrapped", b);
+                // Any custom values set via EnvActionImpl.setProperty will be “frozen” for the duration of the CoreWrapperStep,
+                // because they are always overridden by contextual values.
+                story.j.assertLogContains("groovy after wrapped", b);
+                story.j.assertLogContains("shell after wrapped", b);
+                story.j.assertLogContains("groovy outside edited", b);
+                story.j.assertLogContains("shell outside edited", b);
+            }
+        });
+    }
+    public static class OneVarWrapper extends SimpleBuildWrapper {
+        @DataBoundConstructor public OneVarWrapper() {}
+        @Override public void setUp(Context context, Run<?,?> build, FilePath workspace, Launcher launcher, TaskListener listener, EnvVars initialEnvironment) throws IOException, InterruptedException {
+            listener.getLogger().println("received " + initialEnvironment.get("TESTVAR"));
+            context.env("TESTVAR", "wrapped");
+        }
+        @TestExtension("envStickiness") public static class DescriptorImpl extends BuildWrapperDescriptor {
+            @Override public String getDisplayName() {
+                return "OneVarWrapper";
+            }
+            @Override public boolean isApplicable(AbstractProject<?,?> item) {
+                return true;
+            }
+        }
     }
 
 }
diff --git a/basic-steps/src/main/java/org/jenkinsci/plugins/workflow/steps/CoreWrapperStep.java b/basic-steps/src/main/java/org/jenkinsci/plugins/workflow/steps/CoreWrapperStep.java
@@ -64,11 +64,6 @@ public static final class Execution extends AbstractStepExecutionImpl {
         @StepContextParameter private transient FilePath workspace;
         @StepContextParameter private transient Launcher launcher;
         @StepContextParameter private transient TaskListener listener;
-        // TODO ideally we would like to just create a fresh EnvVars with only our own bindings.
-        // But DefaultStepContext has no notion of merging multiple EnvVars instances across nested scopes.
-        // So we need to pick up the bindings created by ExecutorStepExecution and append to them.
-        // This has the unfortunate effect of “freezing” any custom values set via EnvActionImpl.setProperty for the duration of this step,
-        // because these will also be present in the inherited EnvVars.
         @StepContextParameter private transient EnvVars env;
 
         @Override public boolean start() throws Exception {