[FIXED JENKINS-41490, JENKINS-41491] Tree steps and validation!

So this started with fixing JENKINS-41490, so that the editor can actually do nested tree steps (d'oh), but in the process, I discovered a strange decision from waaaaaay back in the day to force validation in certain cases to treat the step parameter type as a String, even when it wasn't one. That...was bad. So, fixing both those things.
jenkinsci · Jan 26, 2017 · e4aae36 · e4aae36
1 parent f52546e
commit e4aae36
Show file tree

Hide file tree

Showing 6 changed files with 103 additions and 5 deletions.
diff --git a/pipeline-model-api/src/main/resources/ast-schema.json b/pipeline-model-api/src/main/resources/ast-schema.json
@@ -228,7 +228,14 @@
           "type": "array",
           "minItems": 1,
           "items": {
-            "$ref": "#/definitions/step"
+            "anyOf": [
+              {
+                "$ref": "#/definitions/step"
+              },
+              {
+                "$ref": "#/definitions/treeStep"
+              }
+            ]
           }
         }
       },

diff --git a/...groovy/org/jenkinsci/plugins/pipeline/modeldefinition/validator/ModelValidatorImpl.groovy b/...groovy/org/jenkinsci/plugins/pipeline/modeldefinition/validator/ModelValidatorImpl.groovy
@@ -260,9 +260,6 @@ class ModelValidatorImpl implements ModelValidator {
                 valid = false
             } else {
                 Class erasedType = p?.erasedType
-                if (lookup.stepTakesClosure(desc)) {
-                    erasedType = String.class
-                }
                 def v = arg.value;
 
                 if (!validateParameterType(v, erasedType)) {

diff --git a/...on/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/AbstractModelDefTest.java b/...on/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/AbstractModelDefTest.java
@@ -138,7 +138,8 @@ public void setUp() throws Exception {
             "whenBranchFalse",
             "whenEnvFalse",
             "parallelPipelineWithSpaceInBranch",
-            "parallelPipelineQuoteEscaping"
+            "parallelPipelineQuoteEscaping",
+            "nestedTreeSteps"
     );
 
     public static Iterable<Object[]> configsWithErrors() {

diff --git a/...ition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/BasicModelDefTest.java b/...ition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/BasicModelDefTest.java
@@ -142,6 +142,13 @@ public void validStepParameters() throws Exception {
                 .go();
     }
 
+    @Test
+    public void nestedTreeSteps() throws Exception {
+        expect("nestedTreeSteps")
+                .logContains("[Pipeline] { (foo)", "[Pipeline] timeout", "[Pipeline] retry", "hello")
+                .go();
+    }
+
     @Test
     public void metaStepSyntax() throws Exception {
         env(s).set();

diff --git a/pipeline-model-definition/src/test/resources/json/nestedTreeSteps.json b/pipeline-model-definition/src/test/resources/json/nestedTreeSteps.json
@@ -0,0 +1,45 @@
+{"pipeline": {
+  "stages": [  {
+    "name": "foo",
+    "branches": [    {
+      "name": "default",
+      "steps": [      {
+        "name": "timeout",
+        "arguments":         [
+          {
+            "key": "time",
+            "value":             {
+              "isLiteral": true,
+              "value": 5
+            }
+          },
+          {
+            "key": "unit",
+            "value":             {
+              "isLiteral": true,
+              "value": "SECONDS"
+            }
+          }
+        ],
+        "children": [        {
+          "name": "retry",
+          "arguments":           {
+            "isLiteral": true,
+            "value": 4
+          },
+          "children": [          {
+            "name": "echo",
+            "arguments": [            {
+              "key": "message",
+              "value":               {
+                "isLiteral": true,
+                "value": "hello"
+              }
+            }]
+          }]
+        }]
+      }]
+    }]
+  }],
+  "agent": {"type": "none"}
+}}
diff --git a/pipeline-model-definition/src/test/resources/nestedTreeSteps.groovy b/pipeline-model-definition/src/test/resources/nestedTreeSteps.groovy
@@ -0,0 +1,41 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2017, CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+pipeline {
+    agent none
+    stages {
+        stage("foo") {
+            steps {
+                timeout(time: 5, unit: "SECONDS") {
+                    retry(4) {
+                        echo "hello"
+                    }
+                }
+            }
+        }
+    }
+}
+
+
+