fix(handlePipelineStepError): correct handling of failed steps (#2498)

* WIP: debug incorrect error message

* Update piperExecuteBin.groovy

* Update piperExecuteBin.groovy

* Update handlePipelineStepErrors.groovy

* Update piperExecuteBin.groovy

* Update handlePipelineStepErrors.groovy

* Update piperExecuteBin.groovy

* Update piperExecuteBin.groovy

* Update piperExecuteBin.groovy

* Update piperExecuteBin.groovy

* Update handlePipelineStepErrors.groovy

* Update handlePipelineStepErrors.groovy

* Update commonPipelineEnvironment.groovy

* Update piperExecuteBin.groovy

* Update handlePipelineStepErrors.groovy

* Update handlePipelineStepErrors.groovy

* Update handlePipelineStepErrors.groovy

* Update handlePipelineStepErrors.groovy

* Update handlePipelineStepErrors.groovy

* Update piperExecuteBin.groovy

* Update piperExecuteBin.groovy

* Update piperExecuteBin.groovy

* Update commonPipelineEnvironment.groovy

* Update handlePipelineStepErrors.groovy

* Update handlePipelineStepErrors.groovy

* Update piperExecuteBin.groovy

* Update vars/piperExecuteBin.groovy

* Update vars/handlePipelineStepErrors.groovy

* Update handlePipelineStepErrors.groovy

* Apply suggestions from code review

* correct test case

* change test case

Co-authored-by: Christopher Fenner <26137398+CCFenner@users.noreply.github.com>
Co-authored-by: Christopher Fenner <Christopher.Fenner@sap.com>

test/groovy/ChecksPublishResultsTest.groovy

@@ -182,9 +182,16 @@ class ChecksPublishResultsTest extends BasePiperTest {
     void testPublishWithChangedStepDefaultSettings() throws Exception {
         // init
         // pmd has been set to active: true in step configuration
-        def cpe = [configuration: [steps: [checksPublishResults: [pmd: [active: true]]]]]
+        nullScript.commonPipelineEnvironment.configuration =
+        [
+            steps: [
+                checksPublishResults: [
+                    pmd: [active: true]
+                ]
+            ]
+        ]
         // test
-        stepRule.step.checksPublishResults(script: [commonPipelineEnvironment: cpe])
+        stepRule.step.checksPublishResults(script: nullScript)
         // assert
         assertThat(publisherStepOptions, hasKey('PmdPublisher'))
         // ensure nothing else is published

test/groovy/XsDeployTest.groovy

@@ -1,3 +1,4 @@
+import com.sap.piper.DefaultValueCache
 import static org.hamcrest.Matchers.allOf
 import static org.hamcrest.Matchers.contains
 import static org.hamcrest.Matchers.containsInAnyOrder
@@ -378,7 +379,7 @@ class XsDeployTest extends BasePiperTest {
 
         shellRule.setReturnValue(JenkinsShellCallRule.Type.REGEX, '.*xsDeploy .*', '{"operationId": "1234"}')
 
-        nullScript.commonPipelineEnvironment = ['reset': {}, 'getCustomDefaults': {['a.yml', 'b.yml']}]
+        DefaultValueCache.createInstance([:], ['a.yml', 'b.yml'])
 
         goUtils = new PiperGoUtils(null) {
             void unstashPiperBin() {

vars/handlePipelineStepErrors.groovy

@@ -64,6 +64,9 @@ void call(Map parameters = [:], body) {
         .addIfEmpty('stepNameDoc' , parameters.stepName)
         .use()
 
+    // Load 'unstableSteps' before 'body' execution.
+    // If piperExecuteBin is used in the 'body', the transformation of CPE to filesystem and back will turn the ArrayList into an JSONArray.
+    List unstableSteps = cpe?.getValue('unstableSteps') ?: []
     def message = ''
     try {
         if (config.echoDetails)
@@ -100,13 +103,10 @@ void call(Map parameters = [:], body) {
             echo failureMessage
         }
 
-        List unstableSteps = cpe?.getValue('unstableSteps') ?: []
-
         // add information about unstable steps to pipeline environment
         // this helps to bring this information to users in a consolidated manner inside a pipeline
         unstableSteps.add(config.stepName)
         cpe?.setValue('unstableSteps', unstableSteps)
-
     } catch (Throwable error) {
         if (config.echoDetails)
             message += formatErrorMessage(config, error)

vars/piperExecuteBin.groovy

@@ -13,13 +13,12 @@ import static com.sap.piper.Prerequisites.checkScript
 
 void call(Map parameters = [:], String stepName, String metadataFile, List credentialInfo, boolean failOnMissingReports = false, boolean failOnMissingLinks = false, boolean failOnError = false) {
 
-    def handlePipelineStepErrorsParameters = [stepName: stepName, stepParameters: parameters]
+    Map handlePipelineStepErrorsParameters = [stepName: stepName, stepParameters: parameters]
     if (failOnError) {
         handlePipelineStepErrorsParameters.failOnError = true
     }
 
     handlePipelineStepErrors(handlePipelineStepErrorsParameters) {
-
         Script script = checkScript(this, parameters) ?: this
         def jenkinsUtils = parameters.jenkinsUtilsStub ?: new JenkinsUtils()
         def utils = parameters.juStabUtils ?: new Utils()