From 21ab92d4437daa4d615e7d17b519b83ebe456f3b Mon Sep 17 00:00:00 2001 From: Joseph Petersen Date: Mon, 27 Jan 2020 21:26:50 +0100 Subject: [PATCH 01/11] support map with EnvStep --- .../plugins/workflow/steps/EnvStep.java | 24 +++- .../plugins/workflow/steps/EnvStepTest.java | 105 ++++++++++++++++++ 2 files changed, 128 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java b/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java index 57a29744..fe11033f 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java @@ -24,6 +24,7 @@ package org.jenkinsci.plugins.workflow.steps; +import edu.umd.cs.findbugs.annotations.NonNull; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.EnvVars; import hudson.Extension; @@ -34,7 +35,9 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; import net.sf.json.JSONObject; +import org.jenkinsci.plugins.structs.describable.CustomDescribableModel; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.StaplerRequest; @@ -103,7 +106,7 @@ private static final class ExpanderImpl extends EnvironmentExpander { } } - @Extension public static class DescriptorImpl extends StepDescriptor { + @Extension public static class DescriptorImpl extends StepDescriptor implements CustomDescribableModel { @Override public String getFunctionName() { return "withEnv"; @@ -155,6 +158,25 @@ private static final class ExpanderImpl extends EnvironmentExpander { } } + @NonNull + @Override + public Map customInstantiate(@NonNull Map arguments) { + Map r = new HashMap<>(arguments); + Object overrides = r.get("overrides"); + if (overrides instanceof Map) { + r.put("overrides", toKeyValueList((Map) overrides)); + } + return r; + } + + private static List toKeyValueList(Map map) { + return map.entrySet().stream() + .filter(e -> e.getKey() instanceof String) + .collect(Collectors.toMap(e -> (String)e.getKey(), e -> e.getValue() == null ? "" : (String) e.getValue())) + .entrySet().stream() + .map(m -> m.getKey() + "=" + m.getValue()) + .collect(Collectors.toList()); + } } } diff --git a/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java index 2922b2dd..d48cca73 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java @@ -83,6 +83,36 @@ public class EnvStepTest { }); } + @Test public void mapOverriding() { + story.addStep(new Statement() { + @Override public void evaluate() throws Throwable { + WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p"); + p.setDefinition(new CpsFlowDefinition( + "env.CUSTOM = 'initial'\n" + + "env.FOOPATH = node {isUnix() ? '/opt/foos' : 'C:\\\\foos'}\n" + + "env.NULLED = 'outside'\n" + + "node {\n" + + " withEnv([CUSTOM: 'override', NOVEL: 'val', BUILD_TAG: 'custom', NULLED: null, 'FOOPATH+BALL': isUnix() ? '/opt/ball' : 'C:\\\\ball']) {\n" + + " isUnix() ? sh('echo inside CUSTOM=$CUSTOM NOVEL=$NOVEL BUILD_TAG=$BUILD_TAG NULLED=$NULLED FOOPATH=$FOOPATH:') : bat('echo inside CUSTOM=%CUSTOM% NOVEL=%NOVEL% BUILD_TAG=%BUILD_TAG% NULLED=%NULLED% FOOPATH=%FOOPATH%;')\n" + + " echo \"groovy NULLED=${env.NULLED}\"\n" + + " }\n" + + " isUnix() ? sh('echo outside CUSTOM=$CUSTOM NOVEL=$NOVEL NULLED=outside') : bat('echo outside CUSTOM=%CUSTOM% NOVEL=%NOVEL% NULLED=outside')\n" + + "}", true)); + WorkflowRun b = story.j.assertBuildStatusSuccess(p.scheduleBuild2(0)); + story.j.assertLogContains(Functions.isWindows() ? "inside CUSTOM=override NOVEL=val BUILD_TAG=custom NULLED= FOOPATH=C:\\ball;C:\\foos;" : "inside CUSTOM=override NOVEL=val BUILD_TAG=custom NULLED= FOOPATH=/opt/ball:/opt/foos:", b); + story.j.assertLogContains("groovy NULLED=null", b); + story.j.assertLogContains("outside CUSTOM=initial NOVEL= NULLED=outside", b); + List coreStepNodes = new DepthFirstScanner().filteredNodes( + b.getExecution(), + Predicates.and( + new NodeStepTypePredicate("withEnv"), + n -> n instanceof StepStartNode && !((StepStartNode) n).isBody())); + assertThat(coreStepNodes, Matchers.hasSize(1)); + assertEquals("CUSTOM, NOVEL, BUILD_TAG, NULLED, FOOPATH+BALL", ArgumentsAction.getStepArgumentsAsString(coreStepNodes.get(0))); + } + }); + } + @Test public void parallel() { story.addStep(new Statement() { @Override public void evaluate() throws Throwable { @@ -105,6 +135,28 @@ public class EnvStepTest { }); } + @Test public void mapParallel() { + story.addStep(new Statement() { + @Override public void evaluate() throws Throwable { + WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p"); + p.setDefinition(new CpsFlowDefinition( + "parallel a: {\n" + + " node {withEnv([TOOL: 'aloc']) {semaphore 'a'; isUnix() ? sh('echo a TOOL=$TOOL') : bat('echo a TOOL=%TOOL%')}}\n" + + "}, b: {\n" + + " node {withEnv([TOOL: 'bloc']) {semaphore 'b'; isUnix() ? sh('echo b TOOL=$TOOL') : bat('echo b TOOL=%TOOL%')}}\n" + + "}", true)); + WorkflowRun b = p.scheduleBuild2(0).getStartCondition().get(); + SemaphoreStep.waitForStart("a/1", b); + SemaphoreStep.waitForStart("b/1", b); + SemaphoreStep.success("a/1", null); + SemaphoreStep.success("b/1", null); + story.j.assertBuildStatusSuccess(story.j.waitForCompletion(b)); + story.j.assertLogContains("a TOOL=aloc", b); + story.j.assertLogContains("b TOOL=bloc", b); + } + }); + } + @Test public void restarting() { story.addStep(new Statement() { @Override public void evaluate() throws Throwable { @@ -140,6 +192,41 @@ public class EnvStepTest { }); } + @Test public void mapRestarting() { + 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" + + " isUnix() ? sh(\"echo shell ${which} \\$TESTVAR:\") : bat(\"echo shell ${which} %TESTVAR%:\")\n" + + "}\n" + + "node {\n" + + " withEnv([TESTVAR: 'val']) {\n" + + " show 'before'\n" + + " semaphore 'restarting'\n" + + " show 'after'\n" + + " }\n" + + " show 'outside'\n" + + "}", true)); + WorkflowRun b = p.scheduleBuild2(0).getStartCondition().get(); + SemaphoreStep.waitForStart("restarting/1", b); + } + }); + story.addStep(new Statement() { + @Override public void evaluate() throws Throwable { + SemaphoreStep.success("restarting/1", null); + WorkflowRun b = story.j.assertBuildStatusSuccess(story.j.waitForCompletion(story.j.jenkins.getItemByFullName("p", WorkflowJob.class).getLastBuild())); + story.j.assertLogContains("groovy before val:", b); + story.j.assertLogContains("shell before val:", b); + story.j.assertLogContains("groovy after val:", b); + story.j.assertLogContains("shell after val:", b); + story.j.assertLogContains("groovy outside null:", b); + story.j.assertLogContains("shell outside :", b); + } + }); + } + @Test public void nested() { story.addStep(new Statement() { @Override public void evaluate() throws Throwable { @@ -158,6 +245,24 @@ public class EnvStepTest { }); } + @Test public void mapNested() { + story.addStep(new Statement() { + @Override public void evaluate() throws Throwable { + WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p"); + p.setDefinition(new CpsFlowDefinition( + "node {\n" + + " withEnv([A: 'one']) {\n" + + " withEnv([B: 'two']) {\n" + + " isUnix() ? sh('echo A=$A B=$B') : bat('echo A=%A% B=%B%')\n" + + " }\n" + + " }\n" + + "}", true)); + WorkflowRun b = story.j.assertBuildStatusSuccess(p.scheduleBuild2(0)); + story.j.assertLogContains("A=one B=two", b); + } + }); + } + @Test public void configRoundTrip() throws Exception { story.addStep(new Statement() { @Override public void evaluate() throws Throwable { From 898b05eeddf664bfb74c2aeb9d99593ac8e254de Mon Sep 17 00:00:00 2001 From: Joseph Petersen Date: Mon, 27 Jan 2020 22:03:02 +0100 Subject: [PATCH 02/11] map arguments to string --- .../java/org/jenkinsci/plugins/workflow/steps/EnvStep.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java b/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java index fe11033f..261c1f67 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java @@ -153,6 +153,12 @@ private static final class ExpanderImpl extends EnvironmentExpander { } } return b.toString(); + } else if (overrides instanceof Map) { + return ((Map) overrides).keySet() + .stream() + .filter(e -> e instanceof String) + .map(Object::toString) + .collect(Collectors.joining(", ")); } else { return null; } From b89ccbcd3a43904b89dd2bb29b550fa9b6fe649f Mon Sep 17 00:00:00 2001 From: Joseph Petersen Date: Mon, 27 Jan 2020 22:41:23 +0100 Subject: [PATCH 03/11] call toString on value --- .../plugins/workflow/steps/EnvStep.java | 2 +- .../plugins/workflow/steps/EnvStepTest.java | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java b/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java index 261c1f67..02208041 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java @@ -178,7 +178,7 @@ public Map customInstantiate(@NonNull Map argume private static List toKeyValueList(Map map) { return map.entrySet().stream() .filter(e -> e.getKey() instanceof String) - .collect(Collectors.toMap(e -> (String)e.getKey(), e -> e.getValue() == null ? "" : (String) e.getValue())) + .collect(Collectors.toMap(e -> (String)e.getKey(), e -> e.getValue() == null ? "" : e.getValue().toString())) .entrySet().stream() .map(m -> m.getKey() + "=" + m.getValue()) .collect(Collectors.toList()); diff --git a/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java index d48cca73..e1e12417 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java @@ -245,6 +245,26 @@ public class EnvStepTest { }); } + @Test public void mapNumbersNested() { + story.addStep(new Statement() { + @Override public void evaluate() throws Throwable { + WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p"); + p.setDefinition(new CpsFlowDefinition( + "node {\n" + + " withEnv([A: 1]) {\n" + + " withEnv([B: 2]) {\n" + + " withEnv([C: true]) {\n" + + " isUnix() ? sh('echo A=$A B=$B C=$C') : bat('echo A=%A% B=%B% C=%C%')\n" + + " }\n" + + " }\n" + + " }\n" + + "}", true)); + WorkflowRun b = story.j.assertBuildStatusSuccess(p.scheduleBuild2(0)); + story.j.assertLogContains("A=1 B=2 C=true", b); + } + }); + } + @Test public void mapNested() { story.addStep(new Statement() { @Override public void evaluate() throws Throwable { From 34178ae89e4d71c7c28594eb7bf1c90db0f46be5 Mon Sep 17 00:00:00 2001 From: Joseph Petersen Date: Thu, 30 Jan 2020 05:49:58 +0100 Subject: [PATCH 04/11] DRY --- .../java/org/jenkinsci/plugins/workflow/steps/EnvStep.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java b/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java index 02208041..1379483e 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java @@ -168,9 +168,10 @@ private static final class ExpanderImpl extends EnvironmentExpander { @Override public Map customInstantiate(@NonNull Map arguments) { Map r = new HashMap<>(arguments); - Object overrides = r.get("overrides"); + final String key = "overrides"; + Object overrides = r.get(key); if (overrides instanceof Map) { - r.put("overrides", toKeyValueList((Map) overrides)); + r.put(key, toKeyValueList((Map) overrides)); } return r; } From 2b151a0f822130a47d8407af790e1f7b52e2d89e Mon Sep 17 00:00:00 2001 From: Joseph Petersen Date: Mon, 3 Feb 2020 23:10:43 +0100 Subject: [PATCH 05/11] simplify map tests and conversion based on review --- .../plugins/workflow/steps/EnvStep.java | 5 +- .../plugins/workflow/steps/EnvStepTest.java | 123 ++++-------------- 2 files changed, 23 insertions(+), 105 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java b/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java index 1379483e..64738318 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java @@ -178,10 +178,7 @@ public Map customInstantiate(@NonNull Map argume private static List toKeyValueList(Map map) { return map.entrySet().stream() - .filter(e -> e.getKey() instanceof String) - .collect(Collectors.toMap(e -> (String)e.getKey(), e -> e.getValue() == null ? "" : e.getValue().toString())) - .entrySet().stream() - .map(m -> m.getKey() + "=" + m.getValue()) + .map(m -> m.getKey() + "=" + (m.getValue() == null ? "" : m.getValue().toString())) .collect(Collectors.toList()); } } diff --git a/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java index e1e12417..c77dd134 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java @@ -83,36 +83,6 @@ public class EnvStepTest { }); } - @Test public void mapOverriding() { - story.addStep(new Statement() { - @Override public void evaluate() throws Throwable { - WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p"); - p.setDefinition(new CpsFlowDefinition( - "env.CUSTOM = 'initial'\n" + - "env.FOOPATH = node {isUnix() ? '/opt/foos' : 'C:\\\\foos'}\n" + - "env.NULLED = 'outside'\n" + - "node {\n" + - " withEnv([CUSTOM: 'override', NOVEL: 'val', BUILD_TAG: 'custom', NULLED: null, 'FOOPATH+BALL': isUnix() ? '/opt/ball' : 'C:\\\\ball']) {\n" + - " isUnix() ? sh('echo inside CUSTOM=$CUSTOM NOVEL=$NOVEL BUILD_TAG=$BUILD_TAG NULLED=$NULLED FOOPATH=$FOOPATH:') : bat('echo inside CUSTOM=%CUSTOM% NOVEL=%NOVEL% BUILD_TAG=%BUILD_TAG% NULLED=%NULLED% FOOPATH=%FOOPATH%;')\n" + - " echo \"groovy NULLED=${env.NULLED}\"\n" + - " }\n" + - " isUnix() ? sh('echo outside CUSTOM=$CUSTOM NOVEL=$NOVEL NULLED=outside') : bat('echo outside CUSTOM=%CUSTOM% NOVEL=%NOVEL% NULLED=outside')\n" + - "}", true)); - WorkflowRun b = story.j.assertBuildStatusSuccess(p.scheduleBuild2(0)); - story.j.assertLogContains(Functions.isWindows() ? "inside CUSTOM=override NOVEL=val BUILD_TAG=custom NULLED= FOOPATH=C:\\ball;C:\\foos;" : "inside CUSTOM=override NOVEL=val BUILD_TAG=custom NULLED= FOOPATH=/opt/ball:/opt/foos:", b); - story.j.assertLogContains("groovy NULLED=null", b); - story.j.assertLogContains("outside CUSTOM=initial NOVEL= NULLED=outside", b); - List coreStepNodes = new DepthFirstScanner().filteredNodes( - b.getExecution(), - Predicates.and( - new NodeStepTypePredicate("withEnv"), - n -> n instanceof StepStartNode && !((StepStartNode) n).isBody())); - assertThat(coreStepNodes, Matchers.hasSize(1)); - assertEquals("CUSTOM, NOVEL, BUILD_TAG, NULLED, FOOPATH+BALL", ArgumentsAction.getStepArgumentsAsString(coreStepNodes.get(0))); - } - }); - } - @Test public void parallel() { story.addStep(new Statement() { @Override public void evaluate() throws Throwable { @@ -135,28 +105,6 @@ public class EnvStepTest { }); } - @Test public void mapParallel() { - story.addStep(new Statement() { - @Override public void evaluate() throws Throwable { - WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p"); - p.setDefinition(new CpsFlowDefinition( - "parallel a: {\n" + - " node {withEnv([TOOL: 'aloc']) {semaphore 'a'; isUnix() ? sh('echo a TOOL=$TOOL') : bat('echo a TOOL=%TOOL%')}}\n" + - "}, b: {\n" + - " node {withEnv([TOOL: 'bloc']) {semaphore 'b'; isUnix() ? sh('echo b TOOL=$TOOL') : bat('echo b TOOL=%TOOL%')}}\n" + - "}", true)); - WorkflowRun b = p.scheduleBuild2(0).getStartCondition().get(); - SemaphoreStep.waitForStart("a/1", b); - SemaphoreStep.waitForStart("b/1", b); - SemaphoreStep.success("a/1", null); - SemaphoreStep.success("b/1", null); - story.j.assertBuildStatusSuccess(story.j.waitForCompletion(b)); - story.j.assertLogContains("a TOOL=aloc", b); - story.j.assertLogContains("b TOOL=bloc", b); - } - }); - } - @Test public void restarting() { story.addStep(new Statement() { @Override public void evaluate() throws Throwable { @@ -192,41 +140,6 @@ public class EnvStepTest { }); } - @Test public void mapRestarting() { - 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" + - " isUnix() ? sh(\"echo shell ${which} \\$TESTVAR:\") : bat(\"echo shell ${which} %TESTVAR%:\")\n" + - "}\n" + - "node {\n" + - " withEnv([TESTVAR: 'val']) {\n" + - " show 'before'\n" + - " semaphore 'restarting'\n" + - " show 'after'\n" + - " }\n" + - " show 'outside'\n" + - "}", true)); - WorkflowRun b = p.scheduleBuild2(0).getStartCondition().get(); - SemaphoreStep.waitForStart("restarting/1", b); - } - }); - story.addStep(new Statement() { - @Override public void evaluate() throws Throwable { - SemaphoreStep.success("restarting/1", null); - WorkflowRun b = story.j.assertBuildStatusSuccess(story.j.waitForCompletion(story.j.jenkins.getItemByFullName("p", WorkflowJob.class).getLastBuild())); - story.j.assertLogContains("groovy before val:", b); - story.j.assertLogContains("shell before val:", b); - story.j.assertLogContains("groovy after val:", b); - story.j.assertLogContains("shell after val:", b); - story.j.assertLogContains("groovy outside null:", b); - story.j.assertLogContains("shell outside :", b); - } - }); - } - @Test public void nested() { story.addStep(new Statement() { @Override public void evaluate() throws Throwable { @@ -245,40 +158,48 @@ public class EnvStepTest { }); } - @Test public void mapNumbersNested() { + @Test public void mapArguments() { story.addStep(new Statement() { @Override public void evaluate() throws Throwable { WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p"); p.setDefinition(new CpsFlowDefinition( "node {\n" + - " withEnv([A: 1]) {\n" + - " withEnv([B: 2]) {\n" + - " withEnv([C: true]) {\n" + - " isUnix() ? sh('echo A=$A B=$B C=$C') : bat('echo A=%A% B=%B% C=%C%')\n" + - " }\n" + - " }\n" + + " withEnv(a: 1, b: 2, c: 'hello world', d: true) {\n" + + " echo \"a=$a b=$b c=$c d=$d\"" + " }\n" + "}", true)); WorkflowRun b = story.j.assertBuildStatusSuccess(p.scheduleBuild2(0)); - story.j.assertLogContains("A=1 B=2 C=true", b); + story.j.assertLogContains("a=1 b=2 c=hello world", b); + List coreStepNodes = new DepthFirstScanner().filteredNodes( + b.getExecution(), + Predicates.and( + new NodeStepTypePredicate("withEnv"), + n -> n instanceof StepStartNode && !((StepStartNode) n).isBody())); + assertThat(coreStepNodes, Matchers.hasSize(1)); + assertEquals("a, b, c, d", ArgumentsAction.getStepArgumentsAsString(coreStepNodes.get(0))); } }); } - @Test public void mapNested() { + @Test public void mapArgumentsAsMap() { story.addStep(new Statement() { @Override public void evaluate() throws Throwable { WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p"); p.setDefinition(new CpsFlowDefinition( "node {\n" + - " withEnv([A: 'one']) {\n" + - " withEnv([B: 'two']) {\n" + - " isUnix() ? sh('echo A=$A B=$B') : bat('echo A=%A% B=%B%')\n" + - " }\n" + + " withEnv([A: 1, B: 2, C: 'hello world', D: true]) {\n" + + " echo \"A=$A B=$B C=$C D=$D\"\n" + " }\n" + "}", true)); WorkflowRun b = story.j.assertBuildStatusSuccess(p.scheduleBuild2(0)); - story.j.assertLogContains("A=one B=two", b); + story.j.assertLogContains("A=1 B=2 C=hello world D=true", b); + List coreStepNodes = new DepthFirstScanner().filteredNodes( + b.getExecution(), + Predicates.and( + new NodeStepTypePredicate("withEnv"), + n -> n instanceof StepStartNode && !((StepStartNode) n).isBody())); + assertThat(coreStepNodes, Matchers.hasSize(1)); + assertEquals("A, B, C, D", ArgumentsAction.getStepArgumentsAsString(coreStepNodes.get(0))); } }); } From 1aa3c3f2ba133e0aa2e95e9ebd07ee0a613ef0e7 Mon Sep 17 00:00:00 2001 From: Joseph Petersen Date: Mon, 3 Feb 2020 23:13:03 +0100 Subject: [PATCH 06/11] add missing env from assertion --- .../java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java index c77dd134..e73b1ceb 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java @@ -169,7 +169,7 @@ public class EnvStepTest { " }\n" + "}", true)); WorkflowRun b = story.j.assertBuildStatusSuccess(p.scheduleBuild2(0)); - story.j.assertLogContains("a=1 b=2 c=hello world", b); + story.j.assertLogContains("a=1 b=2 c=hello world d=true", b); List coreStepNodes = new DepthFirstScanner().filteredNodes( b.getExecution(), Predicates.and( From 20d8cf70d5bcff02244d21574010d1a11652bf1e Mon Sep 17 00:00:00 2001 From: Joseph Petersen Date: Wed, 19 Feb 2020 06:05:56 +0100 Subject: [PATCH 07/11] check null values --- .../plugins/workflow/steps/EnvStepTest.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java index e73b1ceb..7d13f9f0 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java @@ -164,19 +164,19 @@ public class EnvStepTest { WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p"); p.setDefinition(new CpsFlowDefinition( "node {\n" + - " withEnv(a: 1, b: 2, c: 'hello world', d: true) {\n" + - " echo \"a=$a b=$b c=$c d=$d\"" + + " withEnv(a: 1, b: 2, c: 'hello world', d: true, e: null) {\n" + + " echo(/a=$a b=$b c=$c d=$d, e=${env.e}/)" + " }\n" + "}", true)); WorkflowRun b = story.j.assertBuildStatusSuccess(p.scheduleBuild2(0)); - story.j.assertLogContains("a=1 b=2 c=hello world d=true", b); + story.j.assertLogContains("a=1 b=2 c=hello world d=true e=null", b); List coreStepNodes = new DepthFirstScanner().filteredNodes( b.getExecution(), Predicates.and( new NodeStepTypePredicate("withEnv"), n -> n instanceof StepStartNode && !((StepStartNode) n).isBody())); assertThat(coreStepNodes, Matchers.hasSize(1)); - assertEquals("a, b, c, d", ArgumentsAction.getStepArgumentsAsString(coreStepNodes.get(0))); + assertEquals("a, b, c, d, e", ArgumentsAction.getStepArgumentsAsString(coreStepNodes.get(0))); } }); } @@ -187,19 +187,19 @@ public class EnvStepTest { WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p"); p.setDefinition(new CpsFlowDefinition( "node {\n" + - " withEnv([A: 1, B: 2, C: 'hello world', D: true]) {\n" + - " echo \"A=$A B=$B C=$C D=$D\"\n" + + " withEnv([A: 1, B: 2, C: 'hello world', D: true, E: null]) {\n" + + " echo(/A=$A B=$B C=$C D=$D E=${env.E}/)\n" + " }\n" + "}", true)); WorkflowRun b = story.j.assertBuildStatusSuccess(p.scheduleBuild2(0)); - story.j.assertLogContains("A=1 B=2 C=hello world D=true", b); + story.j.assertLogContains("A=1 B=2 C=hello world D=true E=null", b); List coreStepNodes = new DepthFirstScanner().filteredNodes( b.getExecution(), Predicates.and( new NodeStepTypePredicate("withEnv"), n -> n instanceof StepStartNode && !((StepStartNode) n).isBody())); assertThat(coreStepNodes, Matchers.hasSize(1)); - assertEquals("A, B, C, D", ArgumentsAction.getStepArgumentsAsString(coreStepNodes.get(0))); + assertEquals("A, B, C, D, E", ArgumentsAction.getStepArgumentsAsString(coreStepNodes.get(0))); } }); } From 4f52a0fc37d8b21cfe52f7c47f6257e6f8822051 Mon Sep 17 00:00:00 2001 From: Joseph Petersen Date: Wed, 19 Feb 2020 06:29:23 +0100 Subject: [PATCH 08/11] a comma too much --- .../java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java index 7d13f9f0..acb71ea2 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java @@ -165,7 +165,7 @@ public class EnvStepTest { p.setDefinition(new CpsFlowDefinition( "node {\n" + " withEnv(a: 1, b: 2, c: 'hello world', d: true, e: null) {\n" + - " echo(/a=$a b=$b c=$c d=$d, e=${env.e}/)" + + " echo(/a=$a b=$b c=$c d=$d e=${env.e}/)" + " }\n" + "}", true)); WorkflowRun b = story.j.assertBuildStatusSuccess(p.scheduleBuild2(0)); From 8d385a2e554f38b9e201024f99b0819eeef59b97 Mon Sep 17 00:00:00 2001 From: Joseph Petersen Date: Wed, 19 Feb 2020 23:30:24 +0100 Subject: [PATCH 09/11] Cast key to String --- src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java b/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java index 64738318..05c93ce0 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/steps/EnvStep.java @@ -178,7 +178,7 @@ public Map customInstantiate(@NonNull Map argume private static List toKeyValueList(Map map) { return map.entrySet().stream() - .map(m -> m.getKey() + "=" + (m.getValue() == null ? "" : m.getValue().toString())) + .map(m -> (String) m.getKey() + "=" + (m.getValue() == null ? "" : m.getValue().toString())) .collect(Collectors.toList()); } } From ae58a66f33ca9273e628935f7ff1e7be2e523752 Mon Sep 17 00:00:00 2001 From: Stuart Rowe Date: Sat, 14 Jun 2025 20:28:34 -0700 Subject: [PATCH 10/11] Fix EnvStepTest compile errors after merge --- .../plugins/workflow/steps/EnvStepTest.java | 76 +++++++++---------- 1 file changed, 36 insertions(+), 40 deletions(-) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java index 4353418b..73f3855b 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/steps/EnvStepTest.java @@ -149,49 +149,45 @@ public class EnvStepTest { }); } - @Test public void mapArguments() { - story.addStep(new Statement() { - @Override public void evaluate() throws Throwable { - WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p"); - p.setDefinition(new CpsFlowDefinition( - "node {\n" + - " withEnv(a: 1, b: 2, c: 'hello world', d: true, e: null) {\n" + - " echo(/a=$a b=$b c=$c d=$d e=${env.e}/)" + - " }\n" + - "}", true)); - WorkflowRun b = story.j.assertBuildStatusSuccess(p.scheduleBuild2(0)); - story.j.assertLogContains("a=1 b=2 c=hello world d=true e=null", b); - List coreStepNodes = new DepthFirstScanner().filteredNodes( - b.getExecution(), - Predicates.and( - new NodeStepTypePredicate("withEnv"), - n -> n instanceof StepStartNode && !((StepStartNode) n).isBody())); - assertThat(coreStepNodes, Matchers.hasSize(1)); - assertEquals("a, b, c, d, e", ArgumentsAction.getStepArgumentsAsString(coreStepNodes.get(0))); - } + @Test public void mapArguments() throws Throwable { + sessions.then(j -> { + WorkflowJob p = j.createProject(WorkflowJob.class, "p"); + p.setDefinition(new CpsFlowDefinition( + "node {\n" + + " withEnv(a: 1, b: 2, c: 'hello world', d: true, e: null) {\n" + + " echo(/a=$a b=$b c=$c d=$d e=${env.e}/)" + + " }\n" + + "}", true)); + WorkflowRun b = j.assertBuildStatusSuccess(p.scheduleBuild2(0)); + j.assertLogContains("a=1 b=2 c=hello world d=true e=null", b); + List coreStepNodes = new DepthFirstScanner().filteredNodes( + b.getExecution(), + Predicates.and( + new NodeStepTypePredicate("withEnv"), + n -> n instanceof StepStartNode && !((StepStartNode) n).isBody())); + assertThat(coreStepNodes, Matchers.hasSize(1)); + assertEquals("a, b, c, d, e", ArgumentsAction.getStepArgumentsAsString(coreStepNodes.get(0))); }); } - @Test public void mapArgumentsAsMap() { - story.addStep(new Statement() { - @Override public void evaluate() throws Throwable { - WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p"); - p.setDefinition(new CpsFlowDefinition( - "node {\n" + - " withEnv([A: 1, B: 2, C: 'hello world', D: true, E: null]) {\n" + - " echo(/A=$A B=$B C=$C D=$D E=${env.E}/)\n" + - " }\n" + - "}", true)); - WorkflowRun b = story.j.assertBuildStatusSuccess(p.scheduleBuild2(0)); - story.j.assertLogContains("A=1 B=2 C=hello world D=true E=null", b); - List coreStepNodes = new DepthFirstScanner().filteredNodes( - b.getExecution(), - Predicates.and( - new NodeStepTypePredicate("withEnv"), - n -> n instanceof StepStartNode && !((StepStartNode) n).isBody())); - assertThat(coreStepNodes, Matchers.hasSize(1)); - assertEquals("A, B, C, D, E", ArgumentsAction.getStepArgumentsAsString(coreStepNodes.get(0))); - } + @Test public void mapArgumentsAsMap() throws Throwable { + sessions.then(j -> { + WorkflowJob p = j.createProject(WorkflowJob.class, "p"); + p.setDefinition(new CpsFlowDefinition( + "node {\n" + + " withEnv([A: 1, B: 2, C: 'hello world', D: true, E: null]) {\n" + + " echo(/A=$A B=$B C=$C D=$D E=${env.E}/)\n" + + " }\n" + + "}", true)); + WorkflowRun b = j.assertBuildStatusSuccess(p.scheduleBuild2(0)); + j.assertLogContains("A=1 B=2 C=hello world D=true E=null", b); + List coreStepNodes = new DepthFirstScanner().filteredNodes( + b.getExecution(), + Predicates.and( + new NodeStepTypePredicate("withEnv"), + n -> n instanceof StepStartNode && !((StepStartNode) n).isBody())); + assertThat(coreStepNodes, Matchers.hasSize(1)); + assertEquals("A, B, C, D, E", ArgumentsAction.getStepArgumentsAsString(coreStepNodes.get(0))); }); } From 211ede5890018faad6c2c20720848b6dcc34c8d0 Mon Sep 17 00:00:00 2001 From: Stuart Rowe Date: Sat, 14 Jun 2025 20:44:14 -0700 Subject: [PATCH 11/11] Update EnvStep help and help-overrides to mention the newly supported map syntaxt --- .../plugins/workflow/steps/EnvStep/help-overrides.html | 5 +++++ .../jenkinsci/plugins/workflow/steps/EnvStep/help.html | 9 +++++++++ 2 files changed, 14 insertions(+) diff --git a/src/main/resources/org/jenkinsci/plugins/workflow/steps/EnvStep/help-overrides.html b/src/main/resources/org/jenkinsci/plugins/workflow/steps/EnvStep/help-overrides.html index 6a4a7625..dfd68b9e 100644 --- a/src/main/resources/org/jenkinsci/plugins/workflow/steps/EnvStep/help-overrides.html +++ b/src/main/resources/org/jenkinsci/plugins/workflow/steps/EnvStep/help-overrides.html @@ -1,6 +1,11 @@
+

A list of environment variables to set, each in the form VARIABLE=value or VARIABLE= to unset variables otherwise defined. You may also use the syntax PATH+WHATEVER=/something to prepend /something to $PATH. +

+

+ Environment variables to set can also be defined in a map, each entry of the form [VARIABLE: 'value']. +

diff --git a/src/main/resources/org/jenkinsci/plugins/workflow/steps/EnvStep/help.html b/src/main/resources/org/jenkinsci/plugins/workflow/steps/EnvStep/help.html index 980ca27e..8ab96e87 100644 --- a/src/main/resources/org/jenkinsci/plugins/workflow/steps/EnvStep/help.html +++ b/src/main/resources/org/jenkinsci/plugins/workflow/steps/EnvStep/help.html @@ -11,5 +11,14 @@ }

(Note that here we are using single quotes in Groovy, so the variable expansion is being done by the Bourne shell, not Jenkins.) +

Environment variables to be set may also be defined in a map. For example: +

+node {
+  withEnv([MYTOOL_HOME: '/usr/local/mytool']) {
+    sh '$MYTOOL_HOME/bin/start'
+  }
+}
+
+

See the documentation for the env singleton for more information on environment variables.