JENKINS-46482 Migrate implementation from Parameter concept to Enviro…

…nment variables. Using Environment because theoretically what we are trying to do here is not parameters. Update acceptance test to assert environment instead of parameter. Update JenkinsBlockingQueue to return scheduled jobs instead of queue item. This is necessary as the 'environment' variable is only available in a build object.
jenkinsci · Aug 30, 2017 · 267e946 · 267e946
1 parent b571995
commit 267e946
Show file tree

Hide file tree

Showing 6 changed files with 90 additions and 40 deletions.
diff --git a/...tionTest/groovy/com/ceilfors/jenkins/plugins/jiratrigger/JiraTriggerAcceptanceTest.groovy b/...tionTest/groovy/com/ceilfors/jenkins/plugins/jiratrigger/JiraTriggerAcceptanceTest.groovy
@@ -71,6 +71,7 @@ class JiraTriggerAcceptanceTest extends Specification {
 
     def "Should reply back to JIRA when a build is scheduled"() {
         given:
+        jenkins.quietPeriod = 100
         String issueKey = jira.createIssue()
         def project = jenkins.createJiraCommentTriggeredProject('job')
 

diff --git a/...t/groovy/com/ceilfors/jenkins/plugins/jiratrigger/integration/JenkinsBlockingQueue.groovy b/...t/groovy/com/ceilfors/jenkins/plugins/jiratrigger/integration/JenkinsBlockingQueue.groovy
@@ -4,7 +4,6 @@ import com.atlassian.jira.rest.client.api.domain.Issue
 import com.ceilfors.jenkins.plugins.jiratrigger.JiraTriggerExecutor
 import com.ceilfors.jenkins.plugins.jiratrigger.JiraTriggerListener
 import hudson.model.AbstractProject
-import hudson.model.Queue
 import jenkins.model.Jenkins
 
 import java.util.concurrent.ArrayBlockingQueue
@@ -17,27 +16,23 @@ import java.util.concurrent.TimeUnit
  */
 class JenkinsBlockingQueue {
 
-    private final BlockingQueue scheduledItemBlockingQueue = new ArrayBlockingQueue<>(1)
+    private final BlockingQueue<Collection> scheduledJobsQueue = new ArrayBlockingQueue<>(1)
     private final CountDownLatch countDownLatch = new CountDownLatch(1)
     private long timeout = 5
 
-    /** Can't extend from Queue.Item due to package access methods. */
-    private class NullItem {
-    }
-
     JenkinsBlockingQueue(Jenkins jenkins) {
         def jiraTriggerExecutor = jenkins.injector.getInstance(JiraTriggerExecutor)
         jiraTriggerExecutor.addJiraTriggerListener(new JiraTriggerListener() {
 
             @Override
-            void buildScheduled(Issue issue, Collection<? extends AbstractProject> projects) {
-                scheduledItemBlockingQueue.offer(jenkins.queue.items.last(), timeout, TimeUnit.SECONDS)
+            void buildScheduled(Issue issue, Collection<? extends AbstractProject> jobs) {
+                scheduledJobsQueue.offer(jobs, timeout, TimeUnit.SECONDS)
                 countDownLatch.countDown()
             }
 
             @Override
             void buildNotScheduled(Issue issue) {
-                scheduledItemBlockingQueue.offer(new NullItem(), timeout, TimeUnit.SECONDS)
+                scheduledJobsQueue.offer([], timeout, TimeUnit.SECONDS)
                 countDownLatch.countDown()
             }
         })
@@ -47,14 +42,13 @@ class JenkinsBlockingQueue {
         this.timeout = timeout
     }
 
-    Queue.Item getScheduledItem() {
-        def scheduledItem = scheduledItemBlockingQueue.poll(timeout, TimeUnit.SECONDS)
-        scheduledItem instanceof NullItem ? null : scheduledItem as Queue.Item
+    Collection<? extends AbstractProject> getScheduledJobs() {
+        scheduledJobsQueue.poll(timeout, TimeUnit.SECONDS)
     }
 
-    boolean isItemScheduled() {
+    boolean isAnyJobScheduled() {
         countDownLatch.await(timeout, TimeUnit.SECONDS)
-        def scheduledItem = scheduledItemBlockingQueue.peek()
-        scheduledItem instanceof NullItem ? false : scheduledItem != null
+        Collection scheduledJobs = scheduledJobsQueue.peek()
+        scheduledJobs != null && !scheduledJobs.isEmpty()
     }
 }
diff --git a/...tionTest/groovy/com/ceilfors/jenkins/plugins/jiratrigger/integration/JenkinsRunner.groovy b/...tionTest/groovy/com/ceilfors/jenkins/plugins/jiratrigger/integration/JenkinsRunner.groovy
@@ -7,19 +7,20 @@ import com.ceilfors.jenkins.plugins.jiratrigger.JiraTriggerExecutor
 import com.ceilfors.jenkins.plugins.jiratrigger.JiraTriggerGlobalConfiguration
 import com.ceilfors.jenkins.plugins.jiratrigger.jira.JiraClient
 import com.ceilfors.jenkins.plugins.jiratrigger.webhook.JiraWebhook
+import hudson.model.AbstractBuild
+import hudson.model.AbstractProject
 import hudson.model.FreeStyleProject
-import hudson.model.ParametersAction
-import hudson.model.Project
 import hudson.model.Queue
-import hudson.model.StringParameterValue
 import jenkins.model.GlobalConfiguration
 import org.jvnet.hudson.test.JenkinsRule
 
-import static org.hamcrest.Matchers.containsInAnyOrder
+import static org.hamcrest.Matchers.empty
+import static org.hamcrest.Matchers.equalTo
+import static org.hamcrest.Matchers.hasEntry
 import static org.hamcrest.Matchers.is
 import static org.hamcrest.Matchers.not
-import static org.hamcrest.Matchers.nullValue
 import static org.junit.Assert.assertThat
+
 /**
  * @author ceilfors
  */
@@ -30,29 +31,41 @@ class JenkinsRunner extends JenkinsRule {
     @Override
     void before() throws Throwable {
         super.before()
-        jenkins.quietPeriod = 100
+        jenkins.quietPeriod = 0
         jenkinsQueue = new JenkinsBlockingQueue(instance)
 
         JulLogLevelRule.configureLog() // Needed when @IgnoreRest is used in acceptance tests
     }
 
+    private static AbstractBuild getScheduledBuild(AbstractProject project) {
+        Queue.Item item = project.queueItem
+        if (item == null) {
+            project.getBuildByNumber(1)
+        } else {
+            item.future.get()
+            (item.future.startCondition.get() as AbstractBuild)
+        }
+    }
+
     void buildShouldBeScheduled(String jobName, Map<String, String> parameterMap = [:]) {
-        Queue.Item scheduledItem = jenkinsQueue.scheduledItem
-        assertThat('Build is not scheduled!', scheduledItem, is(not(nullValue())))
-        assertThat('Last build scheduled does not match the job name asserted',
-                (scheduledItem.task as Project).fullName, is(jobName))
+        Collection<AbstractProject> jobs  = jenkinsQueue.scheduledJobs
+        assertThat('Build is not scheduled!', jobs, is(not(empty())))
+        assertThat('Only one project is scheduled', jobs.size(), is(equalTo(1)))
+
+        AbstractProject job = jobs.first()
+        assertThat('Last build scheduled does not match the job name asserted', job.fullName, is(jobName))
         if (parameterMap) {
-            def parametersAction = scheduledItem.getAction(ParametersAction)
-            assertThat(parametersAction.parameters,
-                    containsInAnyOrder(*parameterMap.collect { key, value -> new StringParameterValue(key, value) }))
+            AbstractBuild build = getScheduledBuild(job)
+            parameterMap.each { key, value ->
+                assertThat(build.environment, hasEntry(key, value))
+            }
         }
     }
 
     void noBuildShouldBeScheduled() {
-        if (jenkinsQueue.isItemScheduled()) {
-            Queue.Item scheduledItem = jenkinsQueue.scheduledItem
-            assertThat("Build is scheduled: ${(scheduledItem.task as Project).fullName}",
-                    scheduledItem, is(nullValue()))
+        if (jenkinsQueue.isAnyJobScheduled()) {
+            Collection<AbstractProject> jobs  = jenkinsQueue.scheduledJobs
+            assertThat("Build is scheduled: ${jobs*.fullName}", jobs, is(empty()))
         }
     }
 
@@ -99,4 +112,8 @@ class JenkinsRunner extends JenkinsRule {
         jenkins.injector.getInstance(JiraTriggerExecutor).jiraTriggerListeners
                 .grep(JiraCommentReplier)[0].jiraClient = jiraClient
     }
+
+    void setQuietPeriod(int quietPeriod) {
+        this.jenkins.quietPeriod = quietPeriod
+    }
 }
diff --git a/...ionTest/groovy/com/ceilfors/jenkins/plugins/jiratrigger/integration/RealJiraRunner.groovy b/...ionTest/groovy/com/ceilfors/jenkins/plugins/jiratrigger/integration/RealJiraRunner.groovy
@@ -78,7 +78,7 @@ class RealJiraRunner extends JrjcJiraClient implements JiraRunner {
 
     @Override
     void shouldBeNotifiedWithComment(String issueKey, String jobName) {
-        Queue.Item scheduledItem = jenkinsQueue.scheduledItem
+        Queue.Item scheduledItem = jenkinsQueue.scheduledJobs
         assertThat('Build is not scheduled!', scheduledItem, is(not(nullValue())))
 
         def issue = jiraRestClient.issueClient.getIssue(issueKey).claim()

diff --git a/src/main/groovy/com/ceilfors/jenkins/plugins/jiratrigger/JiraTrigger.groovy b/src/main/groovy/com/ceilfors/jenkins/plugins/jiratrigger/JiraTrigger.groovy
@@ -10,8 +10,6 @@ import hudson.model.Cause
 import hudson.model.CauseAction
 import hudson.model.Item
 import hudson.model.Job
-import hudson.model.ParameterValue
-import hudson.model.ParametersAction
 import hudson.triggers.Trigger
 import hudson.triggers.TriggerDescriptor
 import jenkins.model.Jenkins
@@ -50,7 +48,7 @@ abstract class JiraTrigger<T> extends Trigger<Job> {
 
         List<Action> actions = []
         if (parameterMappings) {
-            actions << new ParametersAction(collectParameterValues(issue))
+            actions << new ParameterMappingAction(issue, parameterMappings)
         }
         actions << new JiraIssueEnvironmentContributingAction(issue)
         actions << new CauseAction(getCause(issue, t))
@@ -77,10 +75,6 @@ abstract class JiraTrigger<T> extends Trigger<Job> {
 
     abstract boolean filter(Issue issue, T t)
 
-    protected List<ParameterValue> collectParameterValues(Issue issue) {
-        parameterMappings.collect { it.parameterResolver.resolve(issue) }
-    }
-
     private String getId(T t) {
         t instanceof AddressableEntity ? (t as AddressableEntity).self : t.toString()
     }

diff --git a/src/main/groovy/com/ceilfors/jenkins/plugins/jiratrigger/ParameterMappingAction.groovy b/src/main/groovy/com/ceilfors/jenkins/plugins/jiratrigger/ParameterMappingAction.groovy
@@ -0,0 +1,44 @@
+package com.ceilfors.jenkins.plugins.jiratrigger
+
+import com.atlassian.jira.rest.client.api.domain.Issue
+import com.ceilfors.jenkins.plugins.jiratrigger.parameter.ParameterMapping
+import hudson.EnvVars
+import hudson.model.AbstractBuild
+import hudson.model.EnvironmentContributingAction
+
+/**
+ * @author ceilfors
+ */
+class ParameterMappingAction implements EnvironmentContributingAction {
+
+    private final Issue issue
+    private final List<ParameterMapping> parameterMappings
+
+    ParameterMappingAction(Issue issue, List<ParameterMapping> parameterMappings) {
+        this.issue = issue
+        this.parameterMappings = parameterMappings
+    }
+
+    @Override
+    void buildEnvVars(AbstractBuild<?, ?> build, EnvVars env) {
+        parameterMappings.each { parameterMapping ->
+            env.put(parameterMapping.jenkinsParameter,
+                    parameterMapping.parameterResolver.resolve(issue).value as String)
+        }
+    }
+
+    @Override
+    String getIconFileName() {
+        null
+    }
+
+    @Override
+    String getDisplayName() {
+        null
+    }
+
+    @Override
+    String getUrlName() {
+        null
+    }
+}