From d31e0e73cb5a0d22f33bb2ecde65e2e86368a9fe Mon Sep 17 00:00:00 2001 From: Marcus Holl Date: Mon, 10 Sep 2018 16:50:53 +0200 Subject: [PATCH 1/6] Remove not (really) needed declarations of prepareObjectInterceptors - In ArtifactSetVersionTest the method was declared, but not used at all. - In MtaUtilsTest there was a particular Object created for the sole purpose of serving as script. For that we have the nullScript. - In DockerArtifactVersioningTest 'this' was configured to serve as script. There is basically no reason why the instance of the JUnitTest should serve as script. Instead we have the nullScript for that purpose. --- test/groovy/ArtifactSetVersionTest.groovy | 7 ------- test/groovy/com/sap/piper/MtaUtilsTest.groovy | 5 +---- .../versioning/DockerArtifactVersioningTest.groovy | 13 ++----------- 3 files changed, 3 insertions(+), 22 deletions(-) diff --git a/test/groovy/ArtifactSetVersionTest.groovy b/test/groovy/ArtifactSetVersionTest.groovy index 7dd6d59ac..3cc498447 100644 --- a/test/groovy/ArtifactSetVersionTest.groovy +++ b/test/groovy/ArtifactSetVersionTest.groovy @@ -156,11 +156,4 @@ class ArtifactSetVersionTest extends BasePiperTest { ) assertThat(sshAgentList, hasItem('testCredentials')) } - - void prepareObjectInterceptors(object) { - object.metaClass.invokeMethod = helper.getMethodInterceptor() - object.metaClass.static.invokeMethod = helper.getMethodInterceptor() - object.metaClass.methodMissing = helper.getMethodMissingInterceptor() - } - } diff --git a/test/groovy/com/sap/piper/MtaUtilsTest.groovy b/test/groovy/com/sap/piper/MtaUtilsTest.groovy index 9f72c89a7..a1da44d66 100644 --- a/test/groovy/com/sap/piper/MtaUtilsTest.groovy +++ b/test/groovy/com/sap/piper/MtaUtilsTest.groovy @@ -39,10 +39,7 @@ class MtaUtilsTest extends BasePiperTest { @Before void init() { targetMtaDescriptor = "${tmp.getRoot()}/generated_mta.yml" - def script = new Object() - mtaUtils = new MtaUtils(script) - - prepareObjectInterceptors(script) + mtaUtils = new MtaUtils(nullScript) this.helper.registerAllowedMethod('readJSON', [Map], { Map parameters -> return new JsonSlurper().parse(new File(parameters.file)) diff --git a/test/groovy/com/sap/piper/versioning/DockerArtifactVersioningTest.groovy b/test/groovy/com/sap/piper/versioning/DockerArtifactVersioningTest.groovy index 99a8c716b..d61245256 100644 --- a/test/groovy/com/sap/piper/versioning/DockerArtifactVersioningTest.groovy +++ b/test/groovy/com/sap/piper/versioning/DockerArtifactVersioningTest.groovy @@ -40,13 +40,11 @@ class DockerArtifactVersioningTest extends BasePiperTest{ passedDir = s return closure() }) - - prepareObjectInterceptors(this) } @Test void testVersioningFrom() { - av = new DockerArtifactVersioning(this, [filePath: 'Dockerfile', dockerVersionSource: 'FROM']) + av = new DockerArtifactVersioning(nullScript, [filePath: 'Dockerfile', dockerVersionSource: 'FROM']) assertEquals('1.2.3', av.getVersion()) av.setVersion('1.2.3-20180101') assertEquals('1.2.3-20180101', jwfr.files['VERSION']) @@ -55,15 +53,8 @@ class DockerArtifactVersioningTest extends BasePiperTest{ @Test void testVersioningEnv() { - av = new DockerArtifactVersioning(this, [filePath: 'Dockerfile', dockerVersionSource: 'TEST']) + av = new DockerArtifactVersioning(nullScript, [filePath: 'Dockerfile', dockerVersionSource: 'TEST']) assertEquals('2.3.4', av.getVersion()) assertTrue(jlr.log.contains('[DockerArtifactVersioning] Version from Docker environment variable TEST: 2.3.4')) } - - - void prepareObjectInterceptors(object) { - object.metaClass.invokeMethod = helper.getMethodInterceptor() - object.metaClass.static.invokeMethod = helper.getMethodInterceptor() - object.metaClass.methodMissing = helper.getMethodMissingInterceptor() - } } From 2d295558111fa07a09665a136ac67ea8e1966336 Mon Sep 17 00:00:00 2001 From: Marcus Holl Date: Thu, 13 Sep 2018 11:08:40 +0200 Subject: [PATCH 2/6] Fix not working link to contribution guidelines --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 243ba5782..aec9b07bf 100644 --- a/README.md +++ b/README.md @@ -117,7 +117,7 @@ otherwise in the [LICENSE file][piper-library-license] [piper-library-pages-plugins]: https://sap.github.io/jenkins-library/jenkins/requiredPlugins [piper-library-issues]: https://github.com/SAP/jenkins-library/issues [piper-library-license]: ./LICENSE -[piper-library-contribution]: ./CONTRIBUTING.md +[piper-library-contribution]: .github/CONTRIBUTING.md [jenkins-doc-pipelines]: https://jenkins.io/solutions/pipeline [jenkins-doc-libraries]: https://jenkins.io/doc/book/pipeline/shared-libraries [jenkins-doc-steps]: https://jenkins.io/doc/pipeline/steps From dc9ce9a732f4f9aece7f86454b0de09091a46aac Mon Sep 17 00:00:00 2001 From: Marcus Holl Date: Thu, 13 Sep 2018 16:01:35 +0200 Subject: [PATCH 3/6] Remove generic gitUtils from BasePiperTest there is only one test class making use of it left. This test class is the GitUtilsTest itself. Hence moving this member downwards in the test class hierarchy into GitUtilsTest. I doubt it makes sense to make use of a generic git utils mock somewhere else since this basically means to test the GitUtils class in the conxtext of other examinees. For that there is no need. The GitUtils class should be tested by the corresponding test class, but not in a transive manner by other tests. For all other tests a specific git utils mock should be provided mocking the corresponding method class. This is already the case today, otherwise we would have more test classes making use of the generic git utils mock. --- test/groovy/com/sap/piper/GitUtilsTest.groovy | 5 +++++ test/groovy/util/BasePiperTest.groovy | 3 --- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/test/groovy/com/sap/piper/GitUtilsTest.groovy b/test/groovy/com/sap/piper/GitUtilsTest.groovy index cacd053fe..121cc2c76 100644 --- a/test/groovy/com/sap/piper/GitUtilsTest.groovy +++ b/test/groovy/com/sap/piper/GitUtilsTest.groovy @@ -19,8 +19,13 @@ import static org.junit.Assert.assertNotNull import static org.junit.Assert.assertNull import static org.junit.Assert.assertThat +import org.springframework.beans.factory.annotation.Autowired + class GitUtilsTest extends BasePiperTest { + @Autowired + GitUtils gitUtils + JenkinsShellCallRule jscr = new JenkinsShellCallRule(this) ExpectedException thrown = ExpectedException.none() diff --git a/test/groovy/util/BasePiperTest.groovy b/test/groovy/util/BasePiperTest.groovy index 4c343b8ef..230a111a9 100644 --- a/test/groovy/util/BasePiperTest.groovy +++ b/test/groovy/util/BasePiperTest.groovy @@ -20,9 +20,6 @@ abstract class BasePiperTest extends BasePipelineTest { @Autowired Script nullScript - @Autowired - GitUtils gitUtils - @Autowired Utils utils From c1900f6ecd0a74d31ed110f416c41e84cfb0fad2 Mon Sep 17 00:00:00 2001 From: Marcus Holl Date: Fri, 14 Sep 2018 09:08:49 +0200 Subject: [PATCH 4/6] Remove output to stdout from tests In fact nobody reads it, but on some IDEs it is shown when tests are executed. --- test/groovy/util/BasePiperTestContext.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/groovy/util/BasePiperTestContext.groovy b/test/groovy/util/BasePiperTestContext.groovy index 4687e6748..77010a1b0 100644 --- a/test/groovy/util/BasePiperTestContext.groovy +++ b/test/groovy/util/BasePiperTestContext.groovy @@ -30,8 +30,8 @@ class BasePiperTestContext { Utils mockUtils() { def mockUtils = new Utils() mockUtils.steps = [ - stash : { m -> println "stash name = ${m.name}" }, - unstash: { println "unstash called ..." } + stash : { }, + unstash: { } ] LibraryLoadingTestExecutionListener.prepareObjectInterceptors(mockUtils) return mockUtils From d22af0f9d465153ee472cd48b3d4f95852d35eda Mon Sep 17 00:00:00 2001 From: Marcus Holl Date: Mon, 17 Sep 2018 16:51:13 +0200 Subject: [PATCH 5/6] Remove explict getters and setters from common pipeline environment We get getters and setters generated automatically. --- vars/commonPipelineEnvironment.groovy | 47 ++------------------------- 1 file changed, 3 insertions(+), 44 deletions(-) diff --git a/vars/commonPipelineEnvironment.groovy b/vars/commonPipelineEnvironment.groovy index eade1b078..18aaa98bb 100644 --- a/vars/commonPipelineEnvironment.groovy +++ b/vars/commonPipelineEnvironment.groovy @@ -5,8 +5,8 @@ class commonPipelineEnvironment implements Serializable { def artifactVersion //stores the gitCommitId as well as additional git information for the build during pipeline run - private String gitCommitId - private String gitSshUrl + String gitCommitId + String gitSshUrl //stores properties for a pipeline which build an artifact and then bundles it into a container private Map appContainerProperties = [:] @@ -19,7 +19,7 @@ class commonPipelineEnvironment implements Serializable { //influxCustomData represents measurement jenkins_custom_data in Influx. Metrics can be written into this map private Map influxCustomData = [:] - private String mtarFilePath + String mtarFilePath def reset() { appContainerProperties = [:] @@ -45,22 +45,6 @@ class commonPipelineEnvironment implements Serializable { return appContainerProperties[property] } - def setArtifactVersion(version) { - artifactVersion = version - } - - def getArtifactVersion() { - return artifactVersion - } - - def setConfigProperties(map) { - configProperties = map - } - - def getConfigProperties() { - return configProperties - } - def setConfigProperty(property, value) { configProperties[property] = value } @@ -72,22 +56,6 @@ class commonPipelineEnvironment implements Serializable { return configProperties[property] } - def setGitCommitId(commitId) { - gitCommitId = commitId - } - - def getGitCommitId() { - return gitCommitId - } - - def setGitSshUrl(url) { - gitSshUrl = url - } - - def getGitSshUrl() { - return gitSshUrl - } - def getInfluxCustomData() { return influxCustomData } @@ -103,15 +71,6 @@ class commonPipelineEnvironment implements Serializable { return influxCustomDataMap.step_data[dataKey] } - - def getMtarFilePath() { - return mtarFilePath - } - - void setMtarFilePath(mtarFilePath) { - this.mtarFilePath = mtarFilePath - } - def setPipelineMeasurement (measurementName, value) { influxCustomDataMap.pipeline_data[measurementName] = value } From 341cb97c25b8511a246e7e6f04a735d3bb8676f8 Mon Sep 17 00:00:00 2001 From: Marcus Holl Date: Fri, 21 Sep 2018 09:24:47 +0200 Subject: [PATCH 6/6] Revert "Remove explict getters and setters from common pipeline environment" This reverts commit d22af0f9d465153ee472cd48b3d4f95852d35eda. --- vars/commonPipelineEnvironment.groovy | 47 +++++++++++++++++++++++++-- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/vars/commonPipelineEnvironment.groovy b/vars/commonPipelineEnvironment.groovy index 18aaa98bb..eade1b078 100644 --- a/vars/commonPipelineEnvironment.groovy +++ b/vars/commonPipelineEnvironment.groovy @@ -5,8 +5,8 @@ class commonPipelineEnvironment implements Serializable { def artifactVersion //stores the gitCommitId as well as additional git information for the build during pipeline run - String gitCommitId - String gitSshUrl + private String gitCommitId + private String gitSshUrl //stores properties for a pipeline which build an artifact and then bundles it into a container private Map appContainerProperties = [:] @@ -19,7 +19,7 @@ class commonPipelineEnvironment implements Serializable { //influxCustomData represents measurement jenkins_custom_data in Influx. Metrics can be written into this map private Map influxCustomData = [:] - String mtarFilePath + private String mtarFilePath def reset() { appContainerProperties = [:] @@ -45,6 +45,22 @@ class commonPipelineEnvironment implements Serializable { return appContainerProperties[property] } + def setArtifactVersion(version) { + artifactVersion = version + } + + def getArtifactVersion() { + return artifactVersion + } + + def setConfigProperties(map) { + configProperties = map + } + + def getConfigProperties() { + return configProperties + } + def setConfigProperty(property, value) { configProperties[property] = value } @@ -56,6 +72,22 @@ class commonPipelineEnvironment implements Serializable { return configProperties[property] } + def setGitCommitId(commitId) { + gitCommitId = commitId + } + + def getGitCommitId() { + return gitCommitId + } + + def setGitSshUrl(url) { + gitSshUrl = url + } + + def getGitSshUrl() { + return gitSshUrl + } + def getInfluxCustomData() { return influxCustomData } @@ -71,6 +103,15 @@ class commonPipelineEnvironment implements Serializable { return influxCustomDataMap.step_data[dataKey] } + + def getMtarFilePath() { + return mtarFilePath + } + + void setMtarFilePath(mtarFilePath) { + this.mtarFilePath = mtarFilePath + } + def setPipelineMeasurement (measurementName, value) { influxCustomDataMap.pipeline_data[measurementName] = value }