JENKINS-43642 Move the fix from Trigger to Descriptor.

Updated unit test to cover the new scenario.
jenkinsci · Aug 9, 2017 · 6025058 · 6025058
1 parent 8948655
commit 6025058
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 17 deletions.
diff --git a/...ionTest/groovy/com/ceilfors/jenkins/plugins/jiratrigger/JiraTriggerIntegrationTest.groovy b/...ionTest/groovy/com/ceilfors/jenkins/plugins/jiratrigger/JiraTriggerIntegrationTest.groovy
@@ -85,10 +85,7 @@ class JiraTriggerIntegrationTest extends Specification {
         given:
         WorkflowJob p = jenkins.jenkins.createProject(WorkflowJob, 'pipeline')
         JiraChangelogTrigger before = new JiraChangelogTrigger()
-        before.changelogMatchers = [
-                new JiraFieldChangelogMatcher('status', 'new value', 'old value'),
-                new CustomFieldChangelogMatcher('custom field', 'new value', 'old value'),
-        ]
+        before.changelogMatchers = [ new JiraFieldChangelogMatcher('status', 'new value', 'old value') ]
         p.addTrigger(before)
         p.save()
 

diff --git a/src/main/groovy/com/ceilfors/jenkins/plugins/jiratrigger/JiraTrigger.groovy b/src/main/groovy/com/ceilfors/jenkins/plugins/jiratrigger/JiraTrigger.groovy
@@ -71,12 +71,7 @@ abstract class JiraTrigger<T> extends Trigger<Job> {
     @Override
     void stop() {
         super.stop()
-        // Avoid null as this method might be called more than once as per Trigger#stop documentation:
-        // "Under some circumstances, this may be invoked more than once for a given Trigger,
-        // so be prepared for that."
-        if (job) {
-            jiraTriggerDescriptor.removeTrigger(this)
-        }
+        jiraTriggerDescriptor.removeTrigger(this)
     }
 
     Job getJob() {
@@ -147,10 +142,15 @@ abstract class JiraTrigger<T> extends Trigger<Job> {
             if (result) {
                 log.finest("Removed [${jiraTrigger.job.fullName}]:[${jiraTrigger.class.simpleName}] from triggers list")
             } else {
-                log.warning(
-                        "Bug! Failed to remove [${jiraTrigger.job.fullName}]:[${jiraTrigger.class.simpleName}] " +
-                                'from triggers list. ' +
-                                'The job might accidentally be triggered by JIRA. Restart Jenkins to recover.')
+                if (jiraTrigger.job) {
+                    log.warning(
+                            "Bug! Failed to remove [${jiraTrigger.job.fullName}]:[${jiraTrigger.class.simpleName}] " +
+                                    'from triggers list. ' +
+                                    'The job might accidentally be triggered by JIRA. Restart Jenkins to recover.')
+                } else {
+                    log.finest("Failed to remove trigger as it might not be started yet." +
+                            "This is normal for pipeline job.")
+                }
             }
         }
 

diff --git a/src/test/groovy/com/ceilfors/jenkins/plugins/jiratrigger/JiraTriggerTest.groovy b/src/test/groovy/com/ceilfors/jenkins/plugins/jiratrigger/JiraTriggerTest.groovy
@@ -4,7 +4,6 @@ import hudson.model.FreeStyleProject
 import org.junit.Rule
 import org.jvnet.hudson.test.Issue
 import org.jvnet.hudson.test.JenkinsRule
-import spock.lang.Ignore
 import spock.lang.Specification
 
 import static com.ceilfors.jenkins.plugins.jiratrigger.JiraCommentTrigger.JiraCommentTriggerDescriptor
@@ -54,16 +53,17 @@ class JiraTriggerTest extends Specification {
     }
 
     @Issue('JENKINS-43642')
-    @Ignore
     def 'Should be able to stop JiraTrigger when the trigger is not started yet'() {
         setup:
         JiraCommentTriggerDescriptor descriptor = jenkinsRule.instance.getDescriptor(JiraCommentTrigger)
 
         when:
+        FreeStyleProject job = jenkinsRule.createFreeStyleProject('job')
         def trigger = new JiraCommentTrigger()
         trigger.stop()
+        trigger.start(job, true)
 
         then:
-        descriptor.allTriggers().size() == 0
+        descriptor.allTriggers().size() == 1
     }
 }