Skip to content

Commit

Permalink
Fix #65: Do not add common DSL closure multiple times in PipelineBuilder
Browse files Browse the repository at this point in the history
Pass the common DSL closure to the build method of the job builder instead
of adding it to the list of dsl closures. This prevents it from being added
multiple times when PipelineBuilder.build() is called multiple times.

Add byte-buddy to the test dependencies to enable Spock to mock
PipelineJobBuilder which is not an interface [1][2].

[1] http://spockframework.org/spock/docs/1.1/all_in_one.html#_mocking_classes
[2] http://spockframework.org/spock/docs/1.1/all_in_one.html#_1_1_rc_3_released_2016_10_17
  • Loading branch information
mnonnenmacher committed Feb 7, 2018
1 parent 55d7e87 commit e19af3d
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 22 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
- Upgrade job-dsl-core to 1.67.
- Process Job DSL scripts in specific order. Scripts from the same folder are processed in alphabetical order and before
any scripts from subfolders. Subfolders are also processed in alphabetical order.
- Fix #65 - PipelineBuilder: Add common DSL Closure only once when PipelineBuilder.build() is called multiple times.

## 3.0.0 (2018-01-10)

Expand Down
2 changes: 2 additions & 0 deletions plugin/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ dependencies {
exclude(module: 'groovy-all')
}

testCompile 'net.bytebuddy:byte-buddy:1.7.9'

functionalTestCompile('org.spockframework:spock-core:1.1-groovy-2.4') {
exclude(module: 'groovy-all')
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,24 +31,26 @@ class JobBuilder {
/**
* Create the job object using the configured DSL closures and job class.
*
* @param dslClosure An optional Job DSL Closure that will be applied to the job builder.
*
* @return The created {@link Job} object.
*/
final Job build() {
final Job build(Closure dslClosure = { }) {
checkNameIsValid()
def dslClosure = concatenateDslClosures()
def combinedClosure = concatenateDslClosures(dslClosure)
switch (jobClass) {
case BuildFlowJob:
return dslFactory.buildFlowJob(fullJobName(), dslClosure)
return dslFactory.buildFlowJob(fullJobName(), combinedClosure)
case FreeStyleJob:
return dslFactory.freeStyleJob(fullJobName(), dslClosure)
return dslFactory.freeStyleJob(fullJobName(), combinedClosure)
case MatrixJob:
return dslFactory.matrixJob(fullJobName(), dslClosure)
return dslFactory.matrixJob(fullJobName(), combinedClosure)
case MavenJob:
return dslFactory.mavenJob(fullJobName(), dslClosure)
return dslFactory.mavenJob(fullJobName(), combinedClosure)
case MultiJob:
return dslFactory.multiJob(fullJobName(), dslClosure)
return dslFactory.multiJob(fullJobName(), combinedClosure)
case WorkflowJob:
return dslFactory.pipelineJob(fullJobName(), dslClosure)
return dslFactory.pipelineJob(fullJobName(), combinedClosure)
default:
throw new GradleJobDslPluginException("Job type ${jobClass} is not supported.")
}
Expand Down Expand Up @@ -201,8 +203,16 @@ class JobBuilder {
}
}

Closure concatenateDslClosures() {
return dslClosures.inject({ }) { acc, val -> acc >> val }
/**
* Concatenate all {@link #dslClosures} to a single Closure.
*
* @param dslClosure An optional DSL Closure that will be appended to the result.
*
* @return The concatenated closure.
*/
Closure concatenateDslClosures(Closure dslClosure = { }) {
def allClosures = [*dslClosures, dslClosure]
return allClosures.inject({ }) { acc, val -> acc >> val }
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,7 @@ class PipelineBuilder {
void build() {
jobBuilders.each { jobBuilder ->
applyDefaultConfiguration(jobBuilder)
applyCommonDsl(jobBuilder)
jobBuilder.build()
jobBuilder.build(commonDsl)
}
}

Expand All @@ -65,8 +64,4 @@ class PipelineBuilder {
}
}

private applyCommonDsl(PipelineJobBuilder jobBuilder) {
jobBuilder.addDsl(commonDsl)
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.here.gradle.plugins.jobdsl.util

import com.here.gradle.plugins.jobdsl.GradleJobDslPluginException
import javaposse.jobdsl.dsl.DslFactory
import org.codehaus.groovy.runtime.ComposedClosure
import spock.lang.Specification
Expand Down Expand Up @@ -61,11 +62,7 @@ class PipelineBuilderSpec extends Specification {

def 'common DSL is applied'() {
given:
def dslFactory = Mock(DslFactory)

def job = new PipelineJobBuilder(dslFactory)
job.name = 'job'
job.freeStyleJob { }
def job = Mock(PipelineJobBuilder)

def pipelineBuilder = new PipelineBuilder()

Expand All @@ -77,7 +74,11 @@ class PipelineBuilderSpec extends Specification {
pipelineBuilder.build()

then:
job.dslClosures.contains(commonDsl)
1 * job.invokeMethod('concatenateDslClosures', [commonDsl])

// There is no easy way to set the jobClass property of the mocked job, so expect the job building to fail.
def ex = thrown GradleJobDslPluginException
ex.message == 'Job type null is not supported.'
}

def 'base folders are applied'() {
Expand Down

0 comments on commit e19af3d

Please sign in to comment.