From 7d3f201609b80c4791199204e7b16c398ebf32ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= Date: Mon, 18 May 2020 10:59:02 +0200 Subject: [PATCH] Ignore invalid entries in custom defaults (#1558) Extend tests, ignore invalid custom defaults entries --- .../SetupCommonPipelineEnvironmentTest.groovy | 43 ++++++++++++++++++- test/groovy/com/sap/piper/UtilsTest.groovy | 39 +++++++++++++---- vars/setupCommonPipelineEnvironment.groovy | 4 ++ 3 files changed, 76 insertions(+), 10 deletions(-) diff --git a/test/groovy/SetupCommonPipelineEnvironmentTest.groovy b/test/groovy/SetupCommonPipelineEnvironmentTest.groovy index 3324432662..c734e8ea72 100644 --- 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 index cf98f9156d..5a0bb17948 100644 --- 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 index c9d46c64a2..dbdda679c3 100644 --- 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"