Skip to content

Commit

Permalink
Merge pull request #155 from abayer/jenkins-24141
Browse files Browse the repository at this point in the history 
[JENKINS-24141] Switch to using RunWithSCM for getCulprits logic
  • Loading branch information
@abayer
abayer committed Oct 23, 2017
2 parents f0cc12d + b5c0d76 commit dfb6bbb
Show file tree
Hide file tree
Showing 10 changed files with 201 additions and 74 deletions.
78 changes: 67 additions & 11 deletions pom.xml
View file
Expand Up @@ -4,7 +4,7 @@
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>2.23</version>
<version>2.27</version>
<relativePath />
</parent>

Expand All @@ -30,8 +30,16 @@
<properties>
<powermock.version>1.6.4</powermock.version>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<workflow.version>1.10</workflow.version>
<workflow-step-api.version>2.9</workflow-step-api.version>
<workflow-aggregator.version>2.5</workflow-aggregator.version>
<workflow-job.version>2.11</workflow-job.version>
<jenkins.version>2.0</jenkins.version>
<workflow-api.version>2.11</workflow-api.version>
<workflow-cps.version>2.29</workflow-cps.version>
<workflow-durable-task-step.version>2.11</workflow-durable-task-step.version>
<workflow-basic-steps.version>2.4</workflow-basic-steps.version>
<workflow-support.version>2.13</workflow-support.version>
<scm-api.version>2.0.7</scm-api.version>
<java.level>7</java.level>
<concurrency>2</concurrency>
<argLine />
Expand Down Expand Up @@ -125,7 +133,7 @@
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>mailer</artifactId>
<version>1.5</version>
<version>1.13</version>
</dependency>
<dependency>
<groupId>org.jvnet.hudson.plugins</groupId>
Expand All @@ -150,31 +158,61 @@
<artifactId>token-macro</artifactId>
<version>2.0</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>structs</artifactId>
<version>1.6</version>
</dependency>

<!-- workflow stuff -->
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<version>${workflow.version}</version>
<version>${workflow-step-api.version}</version>
<artifactId>workflow-step-api</artifactId>
<optional>true</optional>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-job</artifactId>
<version>${workflow.version}</version>
<version>${workflow-job.version}</version>
<optional>true</optional>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-aggregator</artifactId>
<version>${workflow.version}</version>
<artifactId>workflow-api</artifactId>
<version>${workflow-api.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-support</artifactId>
<version>${workflow-support.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-support</artifactId>
<version>${workflow-support.version}</version>
<classifier>tests</classifier>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>scm-api</artifactId>
<version>${scm-api.version}</version>
<scope>test</scope>
</dependency>
<dependency> <!-- StepConfigTester -->
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-step-api</artifactId>
<classifier>tests</classifier>
<version>${workflow.version}</version>
<version>${workflow-step-api.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-cps</artifactId>
<version>${workflow-cps.version}</version>
<scope>test</scope>
</dependency>
<dependency>
Expand Down Expand Up @@ -218,6 +256,12 @@
<artifactId>powermock-module-junit4</artifactId>
<version>${powermock.version}</version>
<scope>test</scope>
<exclusions>
<exclusion>
<groupId>org.javassist</groupId>
<artifactId>javassist</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.powermock</groupId>
Expand All @@ -228,20 +272,32 @@
<dependency>
<groupId>org.javassist</groupId>
<artifactId>javassist</artifactId>
<version>3.18.1-GA</version>
<version>3.20.0-GA</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jenkins-ci.main</groupId>
<artifactId>jenkins-war</artifactId>
<type>war</type>
<version>${jenkins.version}</version>
<version>${jenkins-war.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>credentials</artifactId>
<version>2.1.4</version>
<version>2.1.5</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-durable-task-step</artifactId>
<version>${workflow-durable-task-step.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-basic-steps</artifactId>
<version>${workflow-basic-steps.version}</version>
<scope>test</scope>
</dependency>

Expand Down
7 changes: 5 additions & 2 deletions src/main/java/hudson/plugins/emailext/ExtendedEmailPublisherDescriptor.java
View file
Expand Up @@ -543,12 +543,15 @@ public List<String> getDefaultTriggerIds() {
for(EmailTriggerDescriptor t : this.defaultTriggers) {
// we have to do the below because a bunch of stuff is not serialized for the Descriptor
EmailTriggerDescriptor d = Jenkins.getActiveInstance().getDescriptorByType(t.getClass());
if(!defaultTriggerIds.contains(d.getId())) {
if(d != null && !defaultTriggerIds.contains(d.getId())) {
defaultTriggerIds.add(d.getId());
}
}
} else {
defaultTriggerIds.add(Jenkins.getActiveInstance().getDescriptor(FailureTrigger.class).getId());
FailureTrigger.DescriptorImpl f = Jenkins.getActiveInstance().getDescriptorByType(FailureTrigger.DescriptorImpl.class);
if (f != null) {
defaultTriggerIds.add(f.getId());
}
}
save();
}
Expand Down
29 changes: 24 additions & 5 deletions src/main/java/hudson/plugins/emailext/plugins/recipients/CulpritsRecipientProvider.java
View file
Expand Up @@ -6,9 +6,10 @@

package hudson.plugins.emailext.plugins.recipients;

import com.google.common.base.Predicates;
import com.google.common.collect.Iterables;
import hudson.EnvVars;
import hudson.Extension;
import hudson.model.AbstractBuild;
import hudson.model.Result;
import hudson.model.Run;
import hudson.model.User;
Expand All @@ -17,10 +18,13 @@
import hudson.plugins.emailext.plugins.RecipientProvider;
import hudson.plugins.emailext.plugins.RecipientProviderDescriptor;
import jenkins.model.Jenkins;
import org.jenkinsci.plugins.workflow.job.WorkflowRun;
import org.kohsuke.stapler.DataBoundConstructor;

import javax.mail.internet.InternetAddress;
import java.io.PrintStream;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
Expand Down Expand Up @@ -51,10 +55,25 @@ public void send(final String format, final Object... args) {
}
final Debug debug = new Debug();
Run<?,?> run = context.getRun();
if (run instanceof AbstractBuild) {
Set<User> users = ((AbstractBuild<?,?>)run).getCulprits();
RecipientProviderUtilities.addUsers(users, context, env, to, cc, bcc, debug);
} else {

boolean runHasGetCulprits = false;

// TODO: core 2.60+, workflow-job 2.12+: Switch to checking if run is RunWithSCM and make catch an else block
try {
Method getCulprits = run.getClass().getMethod("getCulprits");
runHasGetCulprits = true;
if (Set.class.isAssignableFrom(getCulprits.getReturnType())) {
@SuppressWarnings("unchecked")
Set<User> users = (Set<User>) getCulprits.invoke(run);
if (Iterables.all(users, Predicates.instanceOf(User.class))) {
RecipientProviderUtilities.addUsers(users, context, env, to, cc, bcc, debug);
}
}
} catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException e) {
// Only log a debug message if the exception is not due to an older WorkflowRun without getCulprits...
if (!(run instanceof WorkflowRun && !runHasGetCulprits)) {
debug.send("Exception getting culprits for %s: %s", run, e);
}
List<Run<?, ?>> builds = new ArrayList<>();
Run<?, ?> build = run;
builds.add(build);
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/hudson/plugins/emailext/plugins/recipients/RecipientProviderUtilities.java
View file
Expand Up @@ -67,6 +67,7 @@ public static Set<User> getChangeSetAuthors(final Collection<Run<?, ?>> runs, fi
final Set<User> users = new HashSet<>();
for (final Run<?, ?> run : runs) {
debug.send(" build: %d", run.getNumber());
// TODO: core 2.60+, workflow-job 2.12+: Switch to checking if run is an instance of RunWithSCM and call getChangeSets directly.
if (run instanceof AbstractBuild<?,?>) {
final ChangeLogSet<?> changeLogSet = ((AbstractBuild<?,?>)run).getChangeSet();
if (changeLogSet == null) {
Expand All @@ -75,6 +76,7 @@ public static Set<User> getChangeSetAuthors(final Collection<Run<?, ?>> runs, fi
addChangeSetUsers(changeLogSet, users, debug);
}
} else {
// TODO: core 2.60+, workflow-job 2.12+: Decide whether to remove this logic since it won't be needed for Pipelines any more.
try {
Method getChangeSets = run.getClass().getMethod("getChangeSets");
if (List.class.isAssignableFrom(getChangeSets.getReturnType())) {
Expand Down
6 changes: 5 additions & 1 deletion src/main/java/hudson/plugins/emailext/plugins/trigger/FailureTrigger.java
View file
Expand Up @@ -33,7 +33,11 @@ public FailureTrigger(boolean sendToList, boolean sendToDevs, boolean sendToRequ
@Deprecated
public static FailureTrigger createDefault() {
DescriptorImpl descriptor = (DescriptorImpl) Jenkins.getActiveInstance().getDescriptor(FailureTrigger.class);
return (FailureTrigger) descriptor.createDefault();
if (descriptor != null) {
return (FailureTrigger) descriptor.createDefault();
} else {
return null;
}
}

@Override
Expand Down
30 changes: 19 additions & 11 deletions src/test/java/hudson/plugins/emailext/plugins/recipients/CulpritsRecipientProviderTest.java
View file
@@ -1,11 +1,13 @@
package hudson.plugins.emailext.plugins.recipients;

import hudson.model.FreeStyleBuild;
import hudson.model.Job;
import hudson.model.Result;
import hudson.model.User;
import hudson.plugins.emailext.ExtendedEmailPublisherDescriptor;
import hudson.tasks.Mailer;
import jenkins.model.Jenkins;
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
import org.jenkinsci.plugins.workflow.job.WorkflowRun;
import org.junit.Before;
import org.junit.Test;
Expand All @@ -22,7 +24,9 @@
Mailer.class,
Mailer.DescriptorImpl.class,
User.class,
WorkflowRun.class
WorkflowRun.class,
WorkflowJob.class,
Job.class
})
public class CulpritsRecipientProviderTest {

Expand All @@ -46,38 +50,42 @@ public void before() throws Exception {

@Test
public void testAddRecipients1() throws Exception {
final WorkflowRun build1 = PowerMockito.mock(WorkflowRun.class);
final WorkflowJob j = PowerMockito.mock(WorkflowJob.class);
final WorkflowRun build1 = PowerMockito.spy(new WorkflowRun(j));
PowerMockito.when(build1.getResult()).thenReturn(Result.UNSTABLE);
MockUtilities.addChangeSet(build1, "X", "V");
PowerMockito.doReturn(null).when(build1).getPreviousBuild();

final WorkflowRun build2 = PowerMockito.mock(WorkflowRun.class);
final WorkflowRun build2 = PowerMockito.spy(new WorkflowRun(j));
PowerMockito.when(build2.getResult()).thenReturn(Result.SUCCESS);
MockUtilities.addChangeSet(build2, "Z", "V");
PowerMockito.when(build2.getPreviousCompletedBuild()).thenReturn(build1);
PowerMockito.doReturn(build1).when(build2).getPreviousCompletedBuild();

final WorkflowRun build3 = PowerMockito.mock(WorkflowRun.class);
final WorkflowRun build3 = PowerMockito.spy(new WorkflowRun(j));
PowerMockito.when(build3.getResult()).thenReturn(Result.UNSTABLE);
MockUtilities.addChangeSet(build3, "A");
PowerMockito.when(build3.getPreviousCompletedBuild()).thenReturn(build2);
PowerMockito.doReturn(build2).when(build3).getPreviousCompletedBuild();

final WorkflowRun build4 = PowerMockito.mock(WorkflowRun.class);
final WorkflowRun build4 = PowerMockito.spy(new WorkflowRun(j));
PowerMockito.when(build4.getResult()).thenReturn(Result.UNSTABLE);
MockUtilities.addChangeSet(build4, "B");
PowerMockito.when(build4.getPreviousCompletedBuild()).thenReturn(build3);
PowerMockito.doReturn(build3).when(build4).getPreviousCompletedBuild();

TestUtilities.checkRecipients(build4, new CulpritsRecipientProvider(), "A", "B");
}

@Test
public void testAddRecipients2() throws Exception {
final WorkflowRun build1 = PowerMockito.mock(WorkflowRun.class);
final WorkflowJob j = PowerMockito.mock(WorkflowJob.class);
final WorkflowRun build1 = PowerMockito.spy(new WorkflowRun(j));
PowerMockito.when(build1.getResult()).thenReturn(Result.UNSTABLE);
MockUtilities.addChangeSet(build1, "X", "V");
PowerMockito.doReturn(null).when(build1).getPreviousBuild();

final WorkflowRun build2 = PowerMockito.mock(WorkflowRun.class);
final WorkflowRun build2 = PowerMockito.spy(new WorkflowRun(j));
PowerMockito.when(build2.getResult()).thenReturn(Result.SUCCESS);
MockUtilities.addChangeSet(build2, "Z", "V");
PowerMockito.when(build2.getPreviousCompletedBuild()).thenReturn(build1);
PowerMockito.doReturn(build1).when(build2).getPreviousCompletedBuild();

TestUtilities.checkRecipients(build2, new CulpritsRecipientProvider(), "X", "V", "Z");
}
Expand Down
16 changes: 11 additions & 5 deletions ...test/java/hudson/plugins/emailext/plugins/recipients/DevelopersRecipientProviderTest.java
View file
@@ -1,11 +1,13 @@
package hudson.plugins.emailext.plugins.recipients;

import hudson.model.FreeStyleBuild;
import hudson.model.FreeStyleProject;
import hudson.model.Result;
import hudson.model.User;
import hudson.plugins.emailext.ExtendedEmailPublisherDescriptor;
import hudson.tasks.Mailer;
import jenkins.model.Jenkins;
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
import org.jenkinsci.plugins.workflow.job.WorkflowRun;
import org.junit.Before;
import org.junit.Test;
Expand All @@ -22,7 +24,9 @@
Mailer.class,
Mailer.DescriptorImpl.class,
User.class,
WorkflowRun.class
WorkflowRun.class,
WorkflowJob.class,
FreeStyleProject.class
})
public class DevelopersRecipientProviderTest {

Expand All @@ -46,14 +50,16 @@ public void before() throws Exception {

@Test
public void testAddRecipients() throws Exception {
final FreeStyleBuild build1 = PowerMockito.mock(FreeStyleBuild.class);
PowerMockito.when(build1.getResult()).thenReturn(Result.UNSTABLE);
final FreeStyleProject p = PowerMockito.mock(FreeStyleProject.class);
final FreeStyleBuild build1 = PowerMockito.spy(new FreeStyleBuild(p));
PowerMockito.doReturn(Result.UNSTABLE).when(build1).getResult();
MockUtilities.addRequestor(build1, "A");
MockUtilities.addChangeSet(build1, "X", "V");
TestUtilities.checkRecipients(build1, new DevelopersRecipientProvider(), "X", "V");

final WorkflowRun build2 = PowerMockito.mock(WorkflowRun.class);
PowerMockito.when(build2.getResult()).thenReturn(Result.UNSTABLE);
final WorkflowJob j = PowerMockito.mock(WorkflowJob.class);
final WorkflowRun build2 = PowerMockito.spy(new WorkflowRun(j));
PowerMockito.doReturn(Result.UNSTABLE).when(build2).getResult();
MockUtilities.addChangeSet(build2, "X", "V");
TestUtilities.checkRecipients(build2, new DevelopersRecipientProvider(), "X", "V");
}
Expand Down

0 comments on commit dfb6bbb

Please sign in to comment.