Ignore invalid entries in custom defaults (#1558)

Extend tests, ignore invalid custom defaults entries
SAP · May 18, 2020 · 7d3f201 · 7d3f201
1 parent 5f48d4d
commit 7d3f201
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 10 deletions.
diff --git a/test/groovy/SetupCommonPipelineEnvironmentTest.groovy b/test/groovy/SetupCommonPipelineEnvironmentTest.groovy
@@ -6,6 +6,7 @@ import org.junit.rules.ExpectedException
 import org.junit.rules.RuleChain
 import org.yaml.snakeyaml.Yaml
 import util.BasePiperTest
+import util.JenkinsLoggingRule
 import util.JenkinsReadFileRule
 import util.JenkinsShellCallRule
 import util.JenkinsStepRule
@@ -24,6 +25,7 @@ class SetupCommonPipelineEnvironmentTest extends BasePiperTest {
     private ExpectedException thrown = ExpectedException.none()
     private JenkinsShellCallRule shellRule = new JenkinsShellCallRule(this)
     private JenkinsReadFileRule readFileRule = new JenkinsReadFileRule(this, "./")
+    private JenkinsLoggingRule loggingRule = new JenkinsLoggingRule(this)
 
     @Rule
     public RuleChain rules = Rules
@@ -33,6 +35,7 @@ class SetupCommonPipelineEnvironmentTest extends BasePiperTest {
         .around(thrown)
         .around(shellRule)
         .around(readFileRule)
+        .around(loggingRule)
 
 
     @Before
@@ -95,13 +98,14 @@ class SetupCommonPipelineEnvironmentTest extends BasePiperTest {
     }
 
     @Test
-    public void testAttemptToLoadNonExistingConfigFile() {
+    void testAttemptToLoadNonExistingConfigFile() {
 
         helper.registerAllowedMethod("fileExists", [String], { String path ->
             switch(path) {
                 case 'default_pipeline_environment.yml': return false
                 case 'custom.yml': return false
                 case 'notFound.yml': return false
+                case '': throw new RuntimeException('cannot call fileExists with empty path')
                 default: return true
             }
         })
@@ -120,11 +124,48 @@ class SetupCommonPipelineEnvironmentTest extends BasePiperTest {
         )
     }
 
+    @Test
+    void testInvalidEntriesInCustomDefaults() {
+
+        helper.registerAllowedMethod("fileExists", [String], { String path ->
+            switch(path) {
+                case 'default_pipeline_environment.yml': return false
+                case '': throw new RuntimeException('cannot call fileExists with empty path')
+                default: return true
+            }
+        })
+
+        helper.registerAllowedMethod("handlePipelineStepErrors", [Map,Closure], { Map map, Closure closure ->
+            closure()
+        })
+
+        helper.registerAllowedMethod("readYaml", [Map], { Map parameters ->
+            Yaml yamlParser = new Yaml()
+            if (parameters.text) {
+                return yamlParser.load(parameters.text)
+            } else if (parameters.file) {
+                if (parameters.file == '.pipeline/config-with-custom-defaults.yml') {
+                    return [customDefaults: ['', true]]
+                }
+            }
+            throw new IllegalArgumentException("Unexpected invocation of readYaml step")
+        })
+
+        stepRule.step.setupCommonPipelineEnvironment(
+            script: nullScript,
+            configFile: '.pipeline/config-with-custom-defaults.yml'
+        )
+
+        assertEquals('WARNING: Ignoring invalid entry in custom defaults from files: \'\' \n' +
+            'WARNING: Ignoring invalid entry in custom defaults from files: \'true\' \n', loggingRule.getLog())
+    }
+
     @Test
     void testAttemptToLoadFileFromURL() {
         helper.registerAllowedMethod("fileExists", [String], {String path ->
             switch (path) {
                 case 'default_pipeline_environment.yml': return false
+                case '': throw new RuntimeException('cannot call fileExists with empty path')
                 default: return true
             }
         })

diff --git a/test/groovy/com/sap/piper/UtilsTest.groovy b/test/groovy/com/sap/piper/UtilsTest.groovy
@@ -2,24 +2,20 @@ package com.sap.piper
 
 import org.junit.Rule
 import org.junit.Before
-import org.junit.Ignore
 import org.junit.Test
+import static org.junit.Assert.assertEquals
 import static org.junit.Assert.assertThat
+import static org.junit.Assert.assertTrue
 import org.junit.rules.ExpectedException
 import org.junit.rules.RuleChain
 
-import static org.hamcrest.Matchers.containsString
-import static org.hamcrest.Matchers.hasItem
 import static org.hamcrest.Matchers.is
-import static org.hamcrest.Matchers.not
 
 import util.JenkinsLoggingRule
 import util.JenkinsShellCallRule
 import util.BasePiperTest
 import util.Rules
 
-import com.sap.piper.Utils
-
 class UtilsTest extends BasePiperTest {
     private ExpectedException thrown = ExpectedException.none()
     private JenkinsLoggingRule loggingRule = new JenkinsLoggingRule(this)
@@ -36,9 +32,7 @@ class UtilsTest extends BasePiperTest {
 
     @Before
     void setup() {
-
         parameters = [:]
-
     }
 
     @Test
@@ -51,8 +45,35 @@ class UtilsTest extends BasePiperTest {
 
     @Test
     void testUnstashAllSkipNull() {
-
         def stashResult = utils.unstashAll(['a', null, 'b'])
         assert stashResult == ['a', 'b']
     }
+
+    @Test
+    void testAppendNonExistingParameterToStringList() {
+        Map parameters = [:]
+        List result = Utils.appendParameterToStringList([], parameters, 'non-existing')
+        assertTrue(result.isEmpty())
+    }
+
+    @Test
+    void testAppendStringParameterToStringList() {
+        Map parameters = ['param': 'string']
+        List result = Utils.appendParameterToStringList([], parameters, 'param')
+        assertEquals(1, result.size())
+    }
+
+    @Test
+    void testAppendListParameterToStringList() {
+        Map parameters = ['param': ['string2', 'string3']]
+        List result = Utils.appendParameterToStringList(['string1'], parameters, 'param')
+        assertEquals(['string1', 'string2', 'string3'], result)
+    }
+
+    @Test
+    void testAppendEmptyListParameterToStringList() {
+        Map parameters = ['param': []]
+        List result = Utils.appendParameterToStringList(['string'], parameters, 'param')
+        assertEquals(['string'], result)
+    }
 }
diff --git a/vars/setupCommonPipelineEnvironment.groovy b/vars/setupCommonPipelineEnvironment.groovy
@@ -117,6 +117,10 @@ private static List copyOrDownloadCustomDefaultsIntoPipelineEnv(script, List cus
     int urlCount = 0
     for (int i = 0; i < customDefaults.size(); i++) {
         // copy retrieved file to .pipeline/ to make sure they are in the pipelineConfigAndTests stash
+        if (!(customDefaults[i] in CharSequence) || customDefaults[i] == '') {
+            script.echo "WARNING: Ignoring invalid entry in custom defaults from files: '${customDefaults[i]}'"
+            continue
+        }
         String fileName
         if (customDefaults[i].startsWith('http://') || customDefaults[i].startsWith('https://')) {
             fileName = "custom_default_from_url_${urlCount}.yml"