From b13007a773848328eeb546ea6a472fc3c09f485e Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Fri, 12 Jul 2019 13:22:56 -0400 Subject: [PATCH 1/6] Remove override of StepDescriptor.newInstance --- .../support/steps/build/BuildTriggerStep.java | 33 ------------------- 1 file changed, 33 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep.java b/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep.java index 4be9495b..68867caf 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep.java @@ -86,39 +86,6 @@ public DescriptorImpl() { super(BuildTriggerStepExecution.class); } - @Override public Step newInstance(StaplerRequest req, JSONObject formData) throws FormException { - BuildTriggerStep step = (BuildTriggerStep) super.newInstance(req, formData); - // Cf. ParametersDefinitionProperty._doBuild: - Object parameter = formData.get("parameter"); - JSONArray params = parameter != null ? JSONArray.fromObject(parameter) : null; - if (params != null) { - Job context = StaplerReferer.findItemFromRequest(Job.class); - Job job = Jenkins.getActiveInstance().getItem(step.getJob(), context, Job.class); - if (job != null) { - ParametersDefinitionProperty pdp = job.getProperty(ParametersDefinitionProperty.class); - if (pdp != null) { - List values = new ArrayList(); - for (Object o : params) { - JSONObject jo = (JSONObject) o; - String name = jo.getString("name"); - ParameterDefinition d = pdp.getParameterDefinition(name); - if (d == null) { - throw new IllegalArgumentException("No such parameter definition: " + name); - } - ParameterValue parameterValue = d.createValue(req, jo); - if (parameterValue != null) { - values.add(parameterValue); - } else { - throw new IllegalArgumentException("Cannot retrieve the parameter value: " + name); - } - } - step.setParameters(values); - } - } - } - return step; - } - @Override public String getFunctionName() { return "build"; From 7830589d6c1b796596a4abaac5e16144a90eb1d7 Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Fri, 12 Jul 2019 14:14:43 -0400 Subject: [PATCH 2/6] Show that SnippetizerTester.assertRoundTrip passes without the override --- pom.xml | 10 +++++++++- .../steps/build/BuildTriggerStepTest.java | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index e196ce0d..9968959c 100644 --- a/pom.xml +++ b/pom.xml @@ -43,6 +43,7 @@ 2.2.7 2.18 3.1 + 2.62 @@ -88,7 +89,14 @@ org.jenkins-ci.plugins.workflow workflow-cps - 2.62 + ${workflow-cps-plugin.version} + test + + + org.jenkins-ci.plugins.workflow + workflow-cps + ${workflow-cps-plugin.version} + tests test diff --git a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java index b86d3fd7..871d86ea 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java @@ -6,6 +6,7 @@ import hudson.model.AbstractBuild; import hudson.model.Action; import hudson.model.BooleanParameterDefinition; +import hudson.model.BooleanParameterValue; import hudson.model.BuildListener; import hudson.model.Cause; import hudson.model.Computer; @@ -21,6 +22,7 @@ import hudson.model.Queue; import hudson.model.Result; import hudson.model.StringParameterDefinition; +import hudson.model.StringParameterValue; import hudson.model.TaskListener; import hudson.model.User; import hudson.model.queue.QueueTaskFuture; @@ -41,10 +43,12 @@ import jenkins.scm.impl.mock.MockSCMNavigator; import jenkins.security.QueueItemAuthenticatorConfiguration; import org.apache.commons.lang.StringUtils; +import org.jenkinsci.Symbol; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; import org.jenkinsci.plugins.workflow.cps.CpsFlowExecution; +import org.jenkinsci.plugins.workflow.cps.SnippetizerTester; import org.jenkinsci.plugins.workflow.flow.FlowExecution; import org.jenkinsci.plugins.workflow.graph.FlowNode; import org.jenkinsci.plugins.workflow.job.WorkflowJob; @@ -549,4 +553,16 @@ public void invalidChoiceParameterValue() throws Exception { j.assertLogContains("Value for choice parameter 'letter' is 'c', but valid choices are [a, b]", j.assertBuildStatus(Result.FAILURE, us.scheduleBuild2(0))); } + + @Test public void buildTriggerStep() throws Exception { + SnippetizerTester st = new SnippetizerTester(j); + BuildTriggerStep step = new BuildTriggerStep("downstream"); + st.assertRoundTrip(step, "build 'downstream'"); + step.setParameters(Arrays.asList(new StringParameterValue("branch", "default"), new BooleanParameterValue("correct", true))); + if (StringParameterDefinition.DescriptorImpl.class.isAnnotationPresent(Symbol.class)) { + st.assertRoundTrip(step, "build job: 'downstream', parameters: [string(name: 'branch', value: 'default'), booleanParam(name: 'correct', value: true)]"); + } else { // TODO 2.x delete + st.assertRoundTrip(step, "build job: 'downstream', parameters: [[$class: 'StringParameterValue', name: 'branch', value: 'default'], [$class: 'BooleanParameterValue', name: 'correct', value: true]]"); + } + } } From 71306850cdf0442fccb520acd0d613cf36419085 Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Fri, 12 Jul 2019 15:12:25 -0400 Subject: [PATCH 3/6] Revert DescriptorImpl.newInstance and add comment --- .../support/steps/build/BuildTriggerStep.java | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep.java b/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep.java index 68867caf..7bbfbdcc 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep.java @@ -86,6 +86,43 @@ public DescriptorImpl() { super(BuildTriggerStepExecution.class); } + // Note: This is necessary because the JSON format of the parameters produced by config.jelly when + // using the snippet generator does not match what would be neccessary for databinding to work automatically. + // For non-snippet generator use, this is unnecessary. Somewhat surprisingly, + // `SnippetizerTester.assertRoundTrip(BuildTriggerStep.class)` does not actually test this behavior. + @Override public Step newInstance(StaplerRequest req, JSONObject formData) throws FormException { + BuildTriggerStep step = (BuildTriggerStep) super.newInstance(req, formData); + // Cf. ParametersDefinitionProperty._doBuild: + Object parameter = formData.get("parameter"); + JSONArray params = parameter != null ? JSONArray.fromObject(parameter) : null; + if (params != null) { + Job context = StaplerReferer.findItemFromRequest(Job.class); + Job job = Jenkins.getActiveInstance().getItem(step.getJob(), context, Job.class); + if (job != null) { + ParametersDefinitionProperty pdp = job.getProperty(ParametersDefinitionProperty.class); + if (pdp != null) { + List values = new ArrayList(); + for (Object o : params) { + JSONObject jo = (JSONObject) o; + String name = jo.getString("name"); + ParameterDefinition d = pdp.getParameterDefinition(name); + if (d == null) { + throw new IllegalArgumentException("No such parameter definition: " + name); + } + ParameterValue parameterValue = d.createValue(req, jo); + if (parameterValue != null) { + values.add(parameterValue); + } else { + throw new IllegalArgumentException("Cannot retrieve the parameter value: " + name); + } + } + step.setParameters(values); + } + } + } + return step; + } + @Override public String getFunctionName() { return "build"; From 82fdfe72e7d7ea375bd513b05cffca6395ad1238 Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Fri, 12 Jul 2019 15:16:55 -0400 Subject: [PATCH 4/6] Update test for current Jenkins baseline --- .../support/steps/build/BuildTriggerStepTest.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java index 871d86ea..2a52017f 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java @@ -554,15 +554,12 @@ public void invalidChoiceParameterValue() throws Exception { j.assertBuildStatus(Result.FAILURE, us.scheduleBuild2(0))); } - @Test public void buildTriggerStep() throws Exception { + @Test public void snippetizer() throws Exception { SnippetizerTester st = new SnippetizerTester(j); BuildTriggerStep step = new BuildTriggerStep("downstream"); st.assertRoundTrip(step, "build 'downstream'"); step.setParameters(Arrays.asList(new StringParameterValue("branch", "default"), new BooleanParameterValue("correct", true))); - if (StringParameterDefinition.DescriptorImpl.class.isAnnotationPresent(Symbol.class)) { - st.assertRoundTrip(step, "build job: 'downstream', parameters: [string(name: 'branch', value: 'default'), booleanParam(name: 'correct', value: true)]"); - } else { // TODO 2.x delete - st.assertRoundTrip(step, "build job: 'downstream', parameters: [[$class: 'StringParameterValue', name: 'branch', value: 'default'], [$class: 'BooleanParameterValue', name: 'correct', value: true]]"); - } + // Note: This does not actually test the format of the JSON produced by the snippet generator for parameters. + st.assertRoundTrip(step, "build job: 'downstream', parameters: [string(name: 'branch', value: 'default'), booleanParam(name: 'correct', value: true)]"); } } From b0e398ebc1e5d45522c96c36b3d24cc6abc08a98 Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Fri, 12 Jul 2019 17:39:59 -0400 Subject: [PATCH 5/6] Migrate the rest of the snippet generator tests --- .../support/steps/build/BuildTriggerStep.java | 3 +- .../steps/build/BuildTriggerStepTest.java | 34 +++++++++++++++++-- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep.java b/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep.java index 7bbfbdcc..39fef579 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStep.java @@ -88,8 +88,7 @@ public DescriptorImpl() { // Note: This is necessary because the JSON format of the parameters produced by config.jelly when // using the snippet generator does not match what would be neccessary for databinding to work automatically. - // For non-snippet generator use, this is unnecessary. Somewhat surprisingly, - // `SnippetizerTester.assertRoundTrip(BuildTriggerStep.class)` does not actually test this behavior. + // For non-snippet generator use, this is unnecessary. @Override public Step newInstance(StaplerRequest req, JSONObject formData) throws FormException { BuildTriggerStep step = (BuildTriggerStep) super.newInstance(req, formData); // Cf. ParametersDefinitionProperty._doBuild: diff --git a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java index 2a52017f..9d7bc890 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java @@ -43,7 +43,6 @@ import jenkins.scm.impl.mock.MockSCMNavigator; import jenkins.security.QueueItemAuthenticatorConfiguration; import org.apache.commons.lang.StringUtils; -import org.jenkinsci.Symbol; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; @@ -554,12 +553,41 @@ public void invalidChoiceParameterValue() throws Exception { j.assertBuildStatus(Result.FAILURE, us.scheduleBuild2(0))); } - @Test public void snippetizer() throws Exception { + @Test public void snippetizerRoundTrip() throws Exception { SnippetizerTester st = new SnippetizerTester(j); BuildTriggerStep step = new BuildTriggerStep("downstream"); st.assertRoundTrip(step, "build 'downstream'"); step.setParameters(Arrays.asList(new StringParameterValue("branch", "default"), new BooleanParameterValue("correct", true))); - // Note: This does not actually test the format of the JSON produced by the snippet generator for parameters. + // Note: This does not actually test the format of the JSON produced by the snippet generator for parameters, see generateSnippet* for tests of that behavior. st.assertRoundTrip(step, "build job: 'downstream', parameters: [string(name: 'branch', value: 'default'), booleanParam(name: 'correct', value: true)]"); } + + @Issue("JENKINS-26093") + @Test public void generateSnippetForBuildTrigger() throws Exception { + SnippetizerTester st = new SnippetizerTester(j); + MockFolder d1 = j.createFolder("d1"); + FreeStyleProject ds = d1.createProject(FreeStyleProject.class, "ds"); + MockFolder d2 = j.createFolder("d2"); + WorkflowJob us = d2.createProject(WorkflowJob.class, "us"); + ds.addProperty(new ParametersDefinitionProperty(new StringParameterDefinition("key", ""), new BooleanParameterDefinition("flag", false, ""))); + String snippet = "build job: '../d1/ds', parameters: [string(name: 'key', value: 'stuff'), booleanParam(name: 'flag', value: true)]"; + st.assertGenerateSnippet("{'stapler-class':'" + BuildTriggerStep.class.getName() + "', 'job':'../d1/ds', 'parameter': [{'name':'key', 'value':'stuff'}, {'name':'flag', 'value':true}]}", snippet, us.getAbsoluteUrl() + "configure"); + } + + @Issue("JENKINS-29739") + @Test public void generateSnippetForBuildTriggerSingle() throws Exception { + SnippetizerTester st = new SnippetizerTester(j); + FreeStyleProject ds = j.jenkins.createProject(FreeStyleProject.class, "ds1"); + FreeStyleProject us = j.jenkins.createProject(FreeStyleProject.class, "us1"); + ds.addProperty(new ParametersDefinitionProperty(new StringParameterDefinition("key", ""))); + String snippet = "build job: 'ds1', parameters: [string(name: 'key', value: 'stuff')]"; + st.assertGenerateSnippet("{'stapler-class':'" + BuildTriggerStep.class.getName() + "', 'job':'ds1', 'parameter': {'name':'key', 'value':'stuff'}}", snippet, us.getAbsoluteUrl() + "configure"); + } + + @Test public void generateSnippetForBuildTriggerNone() throws Exception { + SnippetizerTester st = new SnippetizerTester(j); + FreeStyleProject ds = j.jenkins.createProject(FreeStyleProject.class, "ds0"); + FreeStyleProject us = j.jenkins.createProject(FreeStyleProject.class, "us0"); + st.assertGenerateSnippet("{'stapler-class':'" + BuildTriggerStep.class.getName() + "', 'job':'ds0'}", "build 'ds0'", us.getAbsoluteUrl() + "configure"); + } } From 3f705d2da90cc655ddea98d61dacd2fec9efb6be Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Fri, 12 Jul 2019 17:48:14 -0400 Subject: [PATCH 6/6] Migrate doc generation test as well --- .../workflow/support/steps/build/BuildTriggerStepTest.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java index 9d7bc890..26253f4b 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/BuildTriggerStepTest.java @@ -590,4 +590,9 @@ public void invalidChoiceParameterValue() throws Exception { FreeStyleProject us = j.jenkins.createProject(FreeStyleProject.class, "us0"); st.assertGenerateSnippet("{'stapler-class':'" + BuildTriggerStep.class.getName() + "', 'job':'ds0'}", "build 'ds0'", us.getAbsoluteUrl() + "configure"); } + + @Test + public void buildStepDocs() throws Exception { + SnippetizerTester.assertDocGeneration(BuildTriggerStep.class); + } }