Merge pull request #319 from marcusholl/pr/stepReturnVoid

Pr/step return void
SAP · Oct 18, 2018 · 63dd8f4 · 63dd8f4
2 parents 43acbcf + 6653b68
commit 63dd8f4
Show file tree

Hide file tree

Showing 28 changed files with 70 additions and 38 deletions.
diff --git a/test/groovy/CheckChangeInDevelopmentTest.groovy b/test/groovy/CheckChangeInDevelopmentTest.groovy
@@ -40,17 +40,19 @@ class CheckChangeInDevelopmentTest extends BasePiperTest {
     public void changeIsInStatusDevelopmentTest() {
 
         ChangeManagement cm = getChangeManagementUtils(true)
-        boolean inDevelopment = jsr.step.checkChangeInDevelopment(
-                                    cmUtils: cm,
-                                    changeManagement: [endpoint: 'https://example.org/cm'])
+        jsr.step.checkChangeInDevelopment(
+                     cmUtils: cm,
+                     changeManagement: [endpoint: 'https://example.org/cm'],
+                     failIfStatusIsNotInDevelopment: true)
 
-        assert inDevelopment
         assert cmUtilReceivedParams == [
             changeId: '001',
             endpoint: 'https://example.org/cm',
             credentialsId: 'CM',
             cmclientOpts: ''
         ]
+
+        // no exception in thrown, so the change is in status 'in development'.
     }
 
     @Test

diff --git a/test/groovy/CommonStepsTest.groovy b/test/groovy/CommonStepsTest.groovy
@@ -2,8 +2,10 @@ import static java.util.stream.Collectors.toList
 import static org.hamcrest.Matchers.empty
 import static org.hamcrest.Matchers.is
 import static org.junit.Assert.assertThat
+import static org.junit.Assert.fail
 
 import java.lang.reflect.Field
+import java.io.File;
 
 import org.junit.Assert
 import org.junit.Rule
@@ -12,6 +14,7 @@ import org.junit.rules.RuleChain
 
 import groovy.io.FileType
 import util.BasePiperTest
+
 import util.Rules
 
 /*
@@ -64,6 +67,35 @@ public class CommonStepsTest extends BasePiperTest{
         assertThat("Steps with wrong step name or without STEP_NAME field.: ${stepsWithWrongStepName}",
             stepsWithWrongStepName, is(empty()))
     }
+    /*
+     * With that test we ensure that all return types of the call methods of all the steps
+     * are void. Return types other than void are not possible when running inside declarative
+     * pipelines. Parameters shared between several steps needs to be shared via the commonPipelineEnvironment.
+     */
+    @Test
+    public void returnTypeForCallMethodsIsVoidTest() {
+
+        def stepsWithCallMethodsOtherThanVoid = []
+
+        def whitelist = [
+            'transportRequestCreate',
+            'durationMeasure',
+            ]
+
+        for(def step in getSteps()) {
+            def methods = loadScript("${step}.groovy").getClass().getDeclaredMethods() as List
+            Collection callMethodsWithReturnTypeOtherThanVoid =
+                methods.stream()
+                       .filter { ! whitelist.contains(step) }
+                       .filter { it.getName() == 'call' &&
+                                 it.getReturnType() != Void.TYPE }
+                       .collect(toList())
+            if(!callMethodsWithReturnTypeOtherThanVoid.isEmpty()) stepsWithCallMethodsOtherThanVoid << step
+        }
+
+        assertThat("Steps with call methods with return types other than void: ${stepsWithCallMethodsOtherThanVoid}",
+            stepsWithCallMethodsOtherThanVoid, is(empty()))
+    }
 
     private static getSteps() {
         List steps = []

diff --git a/test/groovy/TransportRequestCreateTest.groovy b/test/groovy/TransportRequestCreateTest.groovy
@@ -118,7 +118,6 @@ public class TransportRequestCreateTest extends BasePiperTest {
                 result.developmentSystemId = developmentSystemId
                 result.cmEndpoint = cmEndpoint
                 result.credentialId = credentialId
-
                 result.clientOpts = clientOpts
                 return '001'
             }

diff --git a/vars/artifactSetVersion.groovy b/vars/artifactSetVersion.groovy
@@ -25,7 +25,7 @@ import groovy.text.SimpleTemplateEngine
 ]
 @Field Set PARAMETER_KEYS = STEP_CONFIG_KEYS.plus('gitCommitId')
 
-def call(Map parameters = [:], Closure body = null) {
+void call(Map parameters = [:], Closure body = null) {
 
     handlePipelineStepErrors (stepName: STEP_NAME, stepParameters: parameters) {
 

diff --git a/vars/batsExecuteTests.groovy b/vars/batsExecuteTests.groovy
@@ -21,7 +21,7 @@ import groovy.transform.Field
 ]
 @Field Set PARAMETER_KEYS = STEP_CONFIG_KEYS
 
-def call(Map parameters = [:]) {
+void call(Map parameters = [:]) {
     handlePipelineStepErrors (stepName: STEP_NAME, stepParameters: parameters) {
 
         def utils = parameters.juStabUtils ?: new Utils()

diff --git a/vars/checkChangeInDevelopment.groovy b/vars/checkChangeInDevelopment.groovy
@@ -19,7 +19,7 @@ import com.sap.piper.cm.ChangeManagementException
 
 @Field Set generalConfigurationKeys = stepConfigurationKeys
 
-def call(parameters = [:]) {
+void call(parameters = [:]) {
 
     handlePipelineStepErrors (stepName: STEP_NAME, stepParameters: parameters) {
 
@@ -103,14 +103,12 @@ def call(parameters = [:]) {
 
         if(isInDevelopment) {
             echo "[INFO] Change '${changeId}' is in status 'in development'."
-            return true
         } else {
             if(configuration.failIfStatusIsNotInDevelopment.toBoolean()) {
                 throw new AbortException("Change '${changeId}' is not in status 'in development'.")
 
             } else {
                 echo "[WARNING] Change '${changeId}' is not in status 'in development'. Failing the pipeline has been explicitly disabled."
-                return false
             }
         }
     }

diff --git a/vars/checksPublishResults.groovy b/vars/checksPublishResults.groovy
@@ -21,7 +21,7 @@ import groovy.transform.Field
  *
  * @param others document all parameters
  */
-def call(Map parameters = [:]) {
+void call(Map parameters = [:]) {
     handlePipelineStepErrors (stepName: STEP_NAME, stepParameters: parameters) {
         def script = parameters.script
         if (script == null)

diff --git a/vars/cloudFoundryDeploy.groovy b/vars/cloudFoundryDeploy.groovy
@@ -21,7 +21,7 @@ import groovy.transform.Field
 @Field Map CONFIG_KEY_COMPATIBILITY = [cloudFoundry: [apiEndpoint: 'cfApiEndpoint', appName:'cfAppName', credentialsId: 'cfCredentialsId', manifest: 'cfManifest', org: 'cfOrg', space: 'cfSpace']]
 @Field Set PARAMETER_KEYS = STEP_CONFIG_KEYS
 
-def call(Map parameters = [:]) {
+void call(Map parameters = [:]) {
 
     handlePipelineStepErrors (stepName: STEP_NAME, stepParameters: parameters) {
 

diff --git a/vars/handlePipelineStepErrors.groovy b/vars/handlePipelineStepErrors.groovy
@@ -3,7 +3,7 @@ import groovy.transform.Field
 @Field STEP_NAME = 'handlePipelineStepErrors'
 
 
-def call(Map parameters = [:], body) {
+void call(Map parameters = [:], body) {
 
     def stepParameters = parameters.stepParameters //mandatory
     def stepName = parameters.stepName //mandatory
@@ -40,7 +40,7 @@ FURTHER INFORMATION:
 * Source code of library step ${stepName}: https://github.com/SAP/jenkins-library/blob/master/vars/${stepName}.groovy
 * Library documentation: https://sap.github.io/jenkins-library/
 * Library repository: https://github.com/SAP/jenkins-library
- 
+
 ----------------------------------------------------------"""
         throw err
     } finally {

diff --git a/vars/influxWriteData.groovy b/vars/influxWriteData.groovy
@@ -17,7 +17,7 @@ import groovy.transform.Field
     'artifactVersion'
 ])
 
-def call(Map parameters = [:]) {
+void call(Map parameters = [:]) {
     handlePipelineStepErrors (stepName: STEP_NAME, stepParameters: parameters, allowBuildFailure: true) {
         def script = parameters.script
         if (script == null)

diff --git a/vars/mavenExecute.groovy b/vars/mavenExecute.groovy
@@ -21,7 +21,7 @@ import groovy.transform.Field
     'logSuccessfulMavenTransfers'
 ])
 
-def call(Map parameters = [:]) {
+void call(Map parameters = [:]) {
     handlePipelineStepErrors(stepName: STEP_NAME, stepParameters: parameters) {
         final script = parameters.script
 

diff --git a/vars/mtaBuild.groovy b/vars/mtaBuild.groovy
@@ -20,7 +20,7 @@ import groovy.transform.Field
     'dockerOptions'
 ])
 
-def call(Map parameters = [:]) {
+void call(Map parameters = [:]) {
     handlePipelineStepErrors(stepName: STEP_NAME, stepParameters: parameters) {
         final script = parameters?.script ?: [commonPipelineEnvironment: commonPipelineEnvironment]
 

diff --git a/vars/neoDeploy.groovy b/vars/neoDeploy.groovy
@@ -29,7 +29,7 @@ import groovy.transform.Field
     'warAction'
 ])
 
-def call(parameters = [:]) {
+void call(parameters = [:]) {
     handlePipelineStepErrors (stepName: STEP_NAME, stepParameters: parameters) {
 
         def script = parameters?.script ?: [commonPipelineEnvironment: commonPipelineEnvironment]
@@ -70,7 +70,7 @@ def call(parameters = [:]) {
             parameters.put('neoCredentialsId', credId)
         }
         // Backward compatibility end
-        
+
         // load default & individual configuration
         Map configuration = ConfigurationHelper.newInstance(this)
             .loadStepDefaults()
@@ -81,9 +81,9 @@ def call(parameters = [:]) {
             .addIfEmpty('archivePath', script.commonPipelineEnvironment.getMtarFilePath())
             .mixin(parameters, PARAMETER_KEYS)
             .use()
-        
+
         utils.pushToSWA([
-            step: STEP_NAME, 
+            step: STEP_NAME,
             stepParam1: configuration.deployMode == 'mta'?'mta':'war', // ['mta', 'warParams', 'warPropertiesFile']
             stepParam2: configuration.warAction == 'rolling-update'?'blue-green':'standard' // ['deploy', 'deploy-mta', 'rolling-update']
         ], configuration)

diff --git a/vars/newmanExecute.groovy b/vars/newmanExecute.groovy
@@ -19,7 +19,7 @@ import groovy.transform.Field
 ]
 @Field Set PARAMETER_KEYS = STEP_CONFIG_KEYS
 
-def call(Map parameters = [:]) {
+void call(Map parameters = [:]) {
     handlePipelineStepErrors(stepName: STEP_NAME, stepParameters: parameters) {
         def script = parameters?.script ?: [commonPipelineEnvironment: commonPipelineEnvironment]
         def utils = parameters?.juStabUtils ?: new Utils()

diff --git a/vars/pipelineExecute.groovy b/vars/pipelineExecute.groovy
@@ -11,7 +11,7 @@ import groovy.transform.Field
  * Load and executes a pipeline from another git repository.
  *
  */
-def call(Map parameters = [:]) {
+void call(Map parameters = [:]) {
 
     node() {
 

diff --git a/vars/pipelineRestartSteps.groovy b/vars/pipelineRestartSteps.groovy
@@ -9,7 +9,7 @@ import groovy.transform.Field
 ]
 @Field Set PARAMETER_KEYS = STEP_CONFIG_KEYS
 
-def call(Map parameters = [:], body) {
+void call(Map parameters = [:], body) {
     handlePipelineStepErrors (stepName: STEP_NAME, stepParameters: parameters) {
         def script = parameters.script ?: [commonPipelineEnvironment: commonPipelineEnvironment]
         def jenkinsUtils = parameters.jenkinsUtilsStub ?: new JenkinsUtils()

diff --git a/vars/pipelineStashFiles.groovy b/vars/pipelineStashFiles.groovy
@@ -2,7 +2,7 @@ import groovy.transform.Field
 
 @Field STEP_NAME = 'pipelineStashFiles'
 
-def call(Map parameters = [:], body) {
+void call(Map parameters = [:], body) {
     handlePipelineStepErrors (stepName: 'pipelineStashFiles', stepParameters: parameters) {
         pipelineStashFilesBeforeBuild(parameters)
         body() //execute build

diff --git a/vars/pipelineStashFilesAfterBuild.groovy b/vars/pipelineStashFilesAfterBuild.groovy
@@ -6,7 +6,7 @@ import groovy.transform.Field
 @Field Set STEP_CONFIG_KEYS = ['runCheckmarx', 'stashIncludes', 'stashExcludes']
 @Field Set PARAMETER_KEYS = STEP_CONFIG_KEYS
 
-def call(Map parameters = [:]) {
+void call(Map parameters = [:]) {
 
     handlePipelineStepErrors (stepName: STEP_NAME, stepParameters: parameters, stepNameDoc: 'stashFiles') {
         def utils = parameters.juStabUtils

diff --git a/vars/pipelineStashFilesBeforeBuild.groovy b/vars/pipelineStashFilesBeforeBuild.groovy
@@ -6,7 +6,7 @@ import groovy.transform.Field
 @Field Set STEP_CONFIG_KEYS = ['runOpaTests', 'stashIncludes', 'stashExcludes']
 @Field Set PARAMETER_KEYS = STEP_CONFIG_KEYS
 
-def call(Map parameters = [:]) {
+void call(Map parameters = [:]) {
 
     handlePipelineStepErrors (stepName: STEP_NAME, stepParameters: parameters, stepNameDoc: 'stashFiles') {
 

diff --git a/vars/prepareDefaultValues.groovy b/vars/prepareDefaultValues.groovy
@@ -5,7 +5,7 @@ import groovy.transform.Field
 
 @Field STEP_NAME = 'prepareDefaultValues'
 
-def call(Map parameters = [:]) {
+void call(Map parameters = [:]) {
     handlePipelineStepErrors (stepName: 'prepareDefaultValues', stepParameters: parameters) {
         if(!DefaultValueCache.getInstance() || parameters.customDefaults) {
             def defaultValues = [:]

diff --git a/vars/seleniumExecuteTests.groovy b/vars/seleniumExecuteTests.groovy
@@ -25,7 +25,7 @@ import groovy.text.SimpleTemplateEngine
 ]
 @Field Set PARAMETER_KEYS = STEP_CONFIG_KEYS
 
-def call(Map parameters = [:], Closure body) {
+void call(Map parameters = [:], Closure body) {
     handlePipelineStepErrors(stepName: STEP_NAME, stepParameters: parameters) {
         def script = parameters?.script ?: [commonPipelineEnvironment: commonPipelineEnvironment]
         def utils = parameters?.juStabUtils ?: new Utils()

diff --git a/vars/setupCommonPipelineEnvironment.groovy b/vars/setupCommonPipelineEnvironment.groovy
@@ -5,7 +5,7 @@ import groovy.transform.Field
 @Field String STEP_NAME = 'setupCommonPipelineEnvironment'
 @Field Set GENERAL_CONFIG_KEYS = ['collectTelemetryData']
 
-def call(Map parameters = [:]) {
+void call(Map parameters = [:]) {
 
     handlePipelineStepErrors (stepName: STEP_NAME, stepParameters: parameters) {
 

diff --git a/vars/snykExecute.groovy b/vars/snykExecute.groovy
@@ -19,7 +19,7 @@ import groovy.transform.Field
 ])
 @Field Set PARAMETER_KEYS = STEP_CONFIG_KEYS
 
-def call(Map parameters = [:]) {
+void call(Map parameters = [:]) {
     handlePipelineStepErrors(stepName: STEP_NAME, stepParameters: parameters) {
         def utils = parameters.juStabUtils ?: new Utils()
         def script = parameters.script ?: [commonPipelineEnvironment: commonPipelineEnvironment]

diff --git a/vars/testsPublishResults.groovy b/vars/testsPublishResults.groovy
@@ -19,7 +19,7 @@ import groovy.transform.Field
  * @param script global script environment of the Jenkinsfile run
  * @param others document all parameters
  */
-def call(Map parameters = [:]) {
+void call(Map parameters = [:]) {
     handlePipelineStepErrors (stepName: STEP_NAME, stepParameters: parameters) {
         def script = parameters.script
         if (script == null)

diff --git a/vars/toolValidate.groovy b/vars/toolValidate.groovy
@@ -9,7 +9,7 @@ import hudson.AbortException
 
 @Field STEP_NAME = 'toolValidate'
 
-def call(Map parameters = [:]) {
+void call(Map parameters = [:]) {
 
     handlePipelineStepErrors (stepName: 'toolValidate', stepParameters: parameters) {
 

diff --git a/vars/transportRequestCreate.groovy b/vars/transportRequestCreate.groovy
@@ -21,6 +21,8 @@ import hudson.AbortException
 
 def call(parameters = [:]) {
 
+    def transportRequestId
+
     handlePipelineStepErrors (stepName: STEP_NAME, stepParameters: parameters) {
 
         def script = parameters?.script ?: [commonPipelineEnvironment: commonPipelineEnvironment]
@@ -76,8 +78,6 @@ def call(parameters = [:]) {
                                         "Change document id not provided (parameter: \'changeDocumentId\' or via commit history).")
                                     .use()
 
-        def transportRequestId
-
         echo "[INFO] Creating transport request for change document '${configuration.changeDocumentId}' and development system '${configuration.developmentSystemId}'."
 
             try {
@@ -92,6 +92,7 @@ def call(parameters = [:]) {
 
 
         echo "[INFO] Transport Request '$transportRequestId' has been successfully created."
-        return transportRequestId
     }
+
+    return transportRequestId
 }
diff --git a/vars/transportRequestRelease.groovy b/vars/transportRequestRelease.groovy
@@ -21,7 +21,7 @@ import hudson.AbortException
 
 @Field Set generalConfigurationKeys = stepConfigurationKeys
 
-def call(parameters = [:]) {
+void call(parameters = [:]) {
 
     handlePipelineStepErrors (stepName: STEP_NAME, stepParameters: parameters) {
 

diff --git a/vars/transportRequestUploadFile.groovy b/vars/transportRequestUploadFile.groovy
@@ -23,7 +23,7 @@ import hudson.AbortException
     'filePath',
     'transportRequestId'])
 
-def call(parameters = [:]) {
+void call(parameters = [:]) {
 
     handlePipelineStepErrors (stepName: STEP_NAME, stepParameters: parameters) {