From 6cce465feef63af825fb4098483c3584c3251dfa Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Thu, 4 May 2017 13:24:02 -0400 Subject: [PATCH 1/8] [JENKINS-24141] Switch to using RunWithSCM for getCulprits logic --- pom.xml | 29 +++++++++++------- .../recipients/CulpritsRecipientProvider.java | 6 ++-- .../RecipientProviderUtilities.java | 9 ++---- .../CulpritsRecipientProviderTest.java | 30 ++++++++++++------- 4 files changed, 44 insertions(+), 30 deletions(-) diff --git a/pom.xml b/pom.xml index aefd03d52..102703857 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ org.jenkins-ci.plugins plugin - 2.23 + 2.27 @@ -30,9 +30,12 @@ 1.6.4 UTF-8 - 1.10 - 2.0 - 7 + 2.9 + 2.5 + 2.11-20170504.162002-1 + 2.58-20170502.192524-8 + 2.58-20170502.192544-8 + 8 2 @@ -154,27 +157,27 @@ org.jenkins-ci.plugins.workflow - ${workflow.version} + ${workflow-step-api.version} workflow-step-api true org.jenkins-ci.plugins.workflow workflow-job - ${workflow.version} + ${workflow-job.version} true org.jenkins-ci.plugins.workflow workflow-aggregator - ${workflow.version} + ${workflow-aggregator.version} test org.jenkins-ci.plugins.workflow workflow-step-api tests - ${workflow.version} + ${workflow-step-api.version} test @@ -218,6 +221,12 @@ powermock-module-junit4 ${powermock.version} test + + + org.javassist + javassist + + org.powermock @@ -228,14 +237,14 @@ org.javassist javassist - 3.18.1-GA + 3.20.0-GA test org.jenkins-ci.main jenkins-war war - ${jenkins.version} + ${jenkins-war.version} test diff --git a/src/main/java/hudson/plugins/emailext/plugins/recipients/CulpritsRecipientProvider.java b/src/main/java/hudson/plugins/emailext/plugins/recipients/CulpritsRecipientProvider.java index 62a24d42b..98999bb15 100644 --- a/src/main/java/hudson/plugins/emailext/plugins/recipients/CulpritsRecipientProvider.java +++ b/src/main/java/hudson/plugins/emailext/plugins/recipients/CulpritsRecipientProvider.java @@ -8,7 +8,6 @@ import hudson.EnvVars; import hudson.Extension; -import hudson.model.AbstractBuild; import hudson.model.Result; import hudson.model.Run; import hudson.model.User; @@ -17,6 +16,7 @@ import hudson.plugins.emailext.plugins.RecipientProvider; import hudson.plugins.emailext.plugins.RecipientProviderDescriptor; import jenkins.model.Jenkins; +import jenkins.scm.RunWithSCM; import org.kohsuke.stapler.DataBoundConstructor; import javax.mail.internet.InternetAddress; @@ -51,8 +51,8 @@ public void send(final String format, final Object... args) { } final Debug debug = new Debug(); Run run = context.getRun(); - if (run instanceof AbstractBuild) { - Set users = ((AbstractBuild)run).getCulprits(); + if (run instanceof RunWithSCM) { + Set users = ((RunWithSCM)run).getCulprits(); RecipientProviderUtilities.addUsers(users, context, env, to, cc, bcc, debug); } else { List> builds = new ArrayList<>(); diff --git a/src/main/java/hudson/plugins/emailext/plugins/recipients/RecipientProviderUtilities.java b/src/main/java/hudson/plugins/emailext/plugins/recipients/RecipientProviderUtilities.java index 54a099d6e..12f561b33 100644 --- a/src/main/java/hudson/plugins/emailext/plugins/recipients/RecipientProviderUtilities.java +++ b/src/main/java/hudson/plugins/emailext/plugins/recipients/RecipientProviderUtilities.java @@ -26,7 +26,6 @@ import com.google.common.base.Predicates; import com.google.common.collect.Iterables; import hudson.EnvVars; -import hudson.model.AbstractBuild; import hudson.model.Cause; import hudson.model.Item; import hudson.model.Run; @@ -48,6 +47,7 @@ import java.util.logging.Logger; import javax.annotation.CheckForNull; import jenkins.model.Jenkins; +import jenkins.scm.RunWithSCM; import org.acegisecurity.Authentication; import org.acegisecurity.userdetails.UsernameNotFoundException; @@ -66,11 +66,8 @@ public static Set getChangeSetAuthors(final Collection> runs, fi final Set users = new HashSet<>(); for (final Run run : runs) { debug.send(" build: %d", run.getNumber()); - if (run instanceof AbstractBuild) { - final ChangeLogSet changeLogSet = ((AbstractBuild)run).getChangeSet(); - if (changeLogSet == null) { - debug.send(" changeLogSet was null"); - } else { + if (run instanceof RunWithSCM) { + for (ChangeLogSet changeLogSet : ((RunWithSCM)run).getChangeSets()) { addChangeSetUsers(changeLogSet, users, debug); } } else { diff --git a/src/test/java/hudson/plugins/emailext/plugins/recipients/CulpritsRecipientProviderTest.java b/src/test/java/hudson/plugins/emailext/plugins/recipients/CulpritsRecipientProviderTest.java index 1e9139c8f..f3b204007 100644 --- a/src/test/java/hudson/plugins/emailext/plugins/recipients/CulpritsRecipientProviderTest.java +++ b/src/test/java/hudson/plugins/emailext/plugins/recipients/CulpritsRecipientProviderTest.java @@ -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; @@ -22,7 +24,9 @@ Mailer.class, Mailer.DescriptorImpl.class, User.class, - WorkflowRun.class + WorkflowRun.class, + WorkflowJob.class, + Job.class }) public class CulpritsRecipientProviderTest { @@ -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"); } From 2e38d1d2f6d197e7b4bfe4e33d06846d20427850 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Thu, 4 May 2017 14:18:45 -0400 Subject: [PATCH 2/8] Update dependencies and tests, fix FindBugs. --- pom.xml | 40 ++++++++++++++++-- .../ExtendedEmailPublisherDescriptor.java | 7 +++- .../DevelopersRecipientProvider.java | 1 + .../plugins/trigger/FailureTrigger.java | 6 ++- .../DevelopersRecipientProviderTest.java | 16 +++++--- ...lingTestSuspectsRecipientProviderTest.java | 41 +++++++++++-------- ...ingBuildSuspectsRecipientProviderTest.java | 40 ++++++++++-------- .../plugins/recipients/MockUtilities.java | 6 +-- 8 files changed, 107 insertions(+), 50 deletions(-) diff --git a/pom.xml b/pom.xml index 102703857..fc9b01079 100644 --- a/pom.xml +++ b/pom.xml @@ -35,6 +35,11 @@ 2.11-20170504.162002-1 2.58-20170502.192524-8 2.58-20170502.192544-8 + 2.11 + 2.29 + 2.11 + 2.4 + 2.0.7 8 2 @@ -153,6 +158,11 @@ token-macro 2.0 + + org.jenkins-ci.plugins + structs + 1.6 + @@ -169,8 +179,14 @@ org.jenkins-ci.plugins.workflow - workflow-aggregator - ${workflow-aggregator.version} + workflow-api + ${workflow-api.version} + test + + + org.jenkins-ci.plugins + scm-api + ${scm-api.version} test @@ -180,6 +196,12 @@ ${workflow-step-api.version} test + + org.jenkins-ci.plugins.workflow + workflow-cps + ${workflow-cps.version} + test + org.jenkins-ci.plugins script-security @@ -250,7 +272,19 @@ org.jenkins-ci.plugins credentials - 2.1.4 + 2.1.5 + test + + + org.jenkins-ci.plugins.workflow + workflow-durable-task-step + ${workflow-durable-task-step.version} + test + + + org.jenkins-ci.plugins.workflow + workflow-basic-steps + ${workflow-basic-steps.version} test diff --git a/src/main/java/hudson/plugins/emailext/ExtendedEmailPublisherDescriptor.java b/src/main/java/hudson/plugins/emailext/ExtendedEmailPublisherDescriptor.java index c14228707..4fe88d950 100644 --- a/src/main/java/hudson/plugins/emailext/ExtendedEmailPublisherDescriptor.java +++ b/src/main/java/hudson/plugins/emailext/ExtendedEmailPublisherDescriptor.java @@ -543,12 +543,15 @@ public List 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(); } diff --git a/src/main/java/hudson/plugins/emailext/plugins/recipients/DevelopersRecipientProvider.java b/src/main/java/hudson/plugins/emailext/plugins/recipients/DevelopersRecipientProvider.java index 0ec682d0c..b0f639843 100644 --- a/src/main/java/hudson/plugins/emailext/plugins/recipients/DevelopersRecipientProvider.java +++ b/src/main/java/hudson/plugins/emailext/plugins/recipients/DevelopersRecipientProvider.java @@ -39,6 +39,7 @@ public void send(final String format, final Object... args) { } } final Debug debug = new Debug(); + Set users = RecipientProviderUtilities.getChangeSetAuthors(Collections.>singleton(context.getRun()), debug); RecipientProviderUtilities.addUsers(users, context, env, to, cc, bcc, debug); } diff --git a/src/main/java/hudson/plugins/emailext/plugins/trigger/FailureTrigger.java b/src/main/java/hudson/plugins/emailext/plugins/trigger/FailureTrigger.java index 2f26fb3f1..f8e1c721a 100644 --- a/src/main/java/hudson/plugins/emailext/plugins/trigger/FailureTrigger.java +++ b/src/main/java/hudson/plugins/emailext/plugins/trigger/FailureTrigger.java @@ -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 diff --git a/src/test/java/hudson/plugins/emailext/plugins/recipients/DevelopersRecipientProviderTest.java b/src/test/java/hudson/plugins/emailext/plugins/recipients/DevelopersRecipientProviderTest.java index 0e7e05370..70e8f6d84 100644 --- a/src/test/java/hudson/plugins/emailext/plugins/recipients/DevelopersRecipientProviderTest.java +++ b/src/test/java/hudson/plugins/emailext/plugins/recipients/DevelopersRecipientProviderTest.java @@ -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; @@ -22,7 +24,9 @@ Mailer.class, Mailer.DescriptorImpl.class, User.class, - WorkflowRun.class + WorkflowRun.class, + WorkflowJob.class, + FreeStyleProject.class }) public class DevelopersRecipientProviderTest { @@ -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"); } diff --git a/src/test/java/hudson/plugins/emailext/plugins/recipients/FailingTestSuspectsRecipientProviderTest.java b/src/test/java/hudson/plugins/emailext/plugins/recipients/FailingTestSuspectsRecipientProviderTest.java index d2f97eb7b..606b28fa6 100644 --- a/src/test/java/hudson/plugins/emailext/plugins/recipients/FailingTestSuspectsRecipientProviderTest.java +++ b/src/test/java/hudson/plugins/emailext/plugins/recipients/FailingTestSuspectsRecipientProviderTest.java @@ -24,7 +24,9 @@ package hudson.plugins.emailext.plugins.recipients; import hudson.model.FreeStyleBuild; +import hudson.model.FreeStyleProject; import hudson.model.Result; +import hudson.model.Run; import hudson.model.User; import hudson.plugins.emailext.ExtendedEmailPublisherDescriptor; import hudson.tasks.Mailer; @@ -43,7 +45,8 @@ Jenkins.class, Mailer.class, Mailer.DescriptorImpl.class, - User.class + User.class, + FreeStyleProject.class }) public class FailingTestSuspectsRecipientProviderTest { @@ -73,8 +76,10 @@ public void testAddRecipients() throws Exception { * No committers. * Tests {a,b} fail. */ - 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(); + PowerMockito.doReturn(null).when(build1).getPreviousCompletedBuild(); MockUtilities.addRequestor(build1, "A"); MockUtilities.addTestResultAction(build1, build1, build1); TestUtilities.checkRecipients(build1, new FailingTestSuspectsRecipientProvider(), "A"); @@ -84,9 +89,9 @@ public void testAddRecipients() throws Exception { * Committers {U,V}. * Tests {a,b,c} fail. */ - final FreeStyleBuild build2 = PowerMockito.mock(FreeStyleBuild.class); - PowerMockito.when(build2.getPreviousCompletedBuild()).thenReturn(build1); - PowerMockito.when(build2.getResult()).thenReturn(Result.UNSTABLE); + final FreeStyleBuild build2 = PowerMockito.spy(new FreeStyleBuild(p)); + PowerMockito.doReturn(Result.UNSTABLE).when(build2).getResult(); + PowerMockito.doReturn(build1).when(build2).getPreviousCompletedBuild(); MockUtilities.addChangeSet(build2, "U", "V"); MockUtilities.addTestResultAction(build2, build1, build1, build2); TestUtilities.checkRecipients(build2, new FailingTestSuspectsRecipientProvider(), "A", "U", "V"); @@ -96,9 +101,9 @@ public void testAddRecipients() throws Exception { * Committers {X,V}. * Tests {c,d} fail. */ - final FreeStyleBuild build3 = PowerMockito.mock(FreeStyleBuild.class); - PowerMockito.when(build3.getPreviousCompletedBuild()).thenReturn(build2); - PowerMockito.when(build3.getResult()).thenReturn(Result.UNSTABLE); + final FreeStyleBuild build3 = PowerMockito.spy(new FreeStyleBuild(p)); + PowerMockito.doReturn(Result.UNSTABLE).when(build3).getResult(); + PowerMockito.doReturn(build2).when(build3).getPreviousCompletedBuild(); MockUtilities.addChangeSet(build3, "X", "V"); MockUtilities.addTestResultAction(build3, build2, build3); TestUtilities.checkRecipients(build3, new FailingTestSuspectsRecipientProvider(), "U", "V", "X"); @@ -108,9 +113,9 @@ public void testAddRecipients() throws Exception { * Committers {K} * No tests were performed. The build failed. */ - final FreeStyleBuild build4 = PowerMockito.mock(FreeStyleBuild.class); - PowerMockito.when(build4.getPreviousCompletedBuild()).thenReturn(build3); - PowerMockito.when(build4.getResult()).thenReturn(Result.FAILURE); + final FreeStyleBuild build4 = PowerMockito.spy(new FreeStyleBuild(p)); + PowerMockito.doReturn(Result.FAILURE).when(build4).getResult(); + PowerMockito.doReturn(build3).when(build4).getPreviousCompletedBuild(); MockUtilities.addChangeSet(build4, "K"); TestUtilities.checkRecipients(build4, new FailingTestSuspectsRecipientProvider()); @@ -119,9 +124,9 @@ public void testAddRecipients() throws Exception { * Committers {X,U,V}. * No tests were performed. The build failed. */ - final FreeStyleBuild build5 = PowerMockito.mock(FreeStyleBuild.class); - PowerMockito.when(build5.getPreviousCompletedBuild()).thenReturn(build4); - PowerMockito.when(build5.getResult()).thenReturn(Result.FAILURE); + final FreeStyleBuild build5 = PowerMockito.spy(new FreeStyleBuild(p)); + PowerMockito.doReturn(Result.FAILURE).when(build5).getResult(); + PowerMockito.doReturn(build4).when(build5).getPreviousCompletedBuild(); MockUtilities.addChangeSet(build5, "U", "W"); TestUtilities.checkRecipients(build5, new FailingTestSuspectsRecipientProvider()); @@ -130,9 +135,9 @@ public void testAddRecipients() throws Exception { * Committers {W}. * Tests {a,e (new test)} fail. */ - final FreeStyleBuild build6 = PowerMockito.mock(FreeStyleBuild.class); - PowerMockito.when(build6.getPreviousCompletedBuild()).thenReturn(build5); - PowerMockito.when(build6.getResult()).thenReturn(Result.UNSTABLE); + final FreeStyleBuild build6 = PowerMockito.spy(new FreeStyleBuild(p)); + PowerMockito.doReturn(Result.UNSTABLE).when(build6).getResult(); + PowerMockito.doReturn(build5).when(build6).getPreviousCompletedBuild(); MockUtilities.addRequestor(build6, "A"); MockUtilities.addChangeSet(build6, "W"); MockUtilities.addTestResultAction(build6, build6, build6); diff --git a/src/test/java/hudson/plugins/emailext/plugins/recipients/FirstFailingBuildSuspectsRecipientProviderTest.java b/src/test/java/hudson/plugins/emailext/plugins/recipients/FirstFailingBuildSuspectsRecipientProviderTest.java index b054771c1..56f479967 100644 --- a/src/test/java/hudson/plugins/emailext/plugins/recipients/FirstFailingBuildSuspectsRecipientProviderTest.java +++ b/src/test/java/hudson/plugins/emailext/plugins/recipients/FirstFailingBuildSuspectsRecipientProviderTest.java @@ -24,6 +24,7 @@ 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; @@ -43,7 +44,8 @@ Jenkins.class, Mailer.class, Mailer.DescriptorImpl.class, - User.class + User.class, + FreeStyleProject.class }) public class FirstFailingBuildSuspectsRecipientProviderTest { @@ -72,8 +74,10 @@ public void testAddRecipients() throws Exception { * No committers. * Failed. */ - final FreeStyleBuild build1 = PowerMockito.mock(FreeStyleBuild.class); - PowerMockito.when(build1.getResult()).thenReturn(Result.FAILURE); + final FreeStyleProject p = PowerMockito.mock(FreeStyleProject.class); + final FreeStyleBuild build1 = PowerMockito.spy(new FreeStyleBuild(p)); + PowerMockito.doReturn(Result.FAILURE).when(build1).getResult(); + PowerMockito.doReturn(null).when(build1).getPreviousCompletedBuild(); MockUtilities.addRequestor(build1, "A"); TestUtilities.checkRecipients(build1, new FirstFailingBuildSuspectsRecipientProvider(), "A"); @@ -82,9 +86,9 @@ public void testAddRecipients() throws Exception { * No committers. * Unstable. */ - final FreeStyleBuild build2 = PowerMockito.mock(FreeStyleBuild.class); - PowerMockito.when(build2.getPreviousCompletedBuild()).thenReturn(build1); - PowerMockito.when(build2.getResult()).thenReturn(Result.UNSTABLE); + final FreeStyleBuild build2 = PowerMockito.spy(new FreeStyleBuild(p)); + PowerMockito.doReturn(Result.UNSTABLE).when(build2).getResult(); + PowerMockito.doReturn(build1).when(build2).getPreviousCompletedBuild(); MockUtilities.addRequestor(build2, "A"); TestUtilities.checkRecipients(build2, new FirstFailingBuildSuspectsRecipientProvider()); @@ -93,9 +97,9 @@ public void testAddRecipients() throws Exception { * Committers {X,V}. * Failed. */ - final FreeStyleBuild build3 = PowerMockito.mock(FreeStyleBuild.class); - PowerMockito.when(build3.getPreviousCompletedBuild()).thenReturn(build2); - PowerMockito.when(build3.getResult()).thenReturn(Result.FAILURE); + final FreeStyleBuild build3 = PowerMockito.spy(new FreeStyleBuild(p)); + PowerMockito.doReturn(Result.FAILURE).when(build3).getResult(); + PowerMockito.doReturn(build2).when(build3).getPreviousCompletedBuild(); MockUtilities.addRequestor(build3, "A"); MockUtilities.addChangeSet(build3, "X", "V"); TestUtilities.checkRecipients(build3, new FirstFailingBuildSuspectsRecipientProvider(), "X", "V", "A"); @@ -105,9 +109,9 @@ public void testAddRecipients() throws Exception { * Committers {X}. * Aborted. */ - final FreeStyleBuild build4 = PowerMockito.mock(FreeStyleBuild.class); - PowerMockito.when(build4.getPreviousCompletedBuild()).thenReturn(build3); - PowerMockito.when(build4.getResult()).thenReturn(Result.ABORTED); + final FreeStyleBuild build4 = PowerMockito.spy(new FreeStyleBuild(p)); + PowerMockito.doReturn(Result.ABORTED).when(build4).getResult(); + PowerMockito.doReturn(build3).when(build4).getPreviousCompletedBuild(); MockUtilities.addRequestor(build4, "B"); MockUtilities.addChangeSet(build4, "X"); TestUtilities.checkRecipients(build4, new FirstFailingBuildSuspectsRecipientProvider()); @@ -117,9 +121,9 @@ public void testAddRecipients() throws Exception { * Committers {U,V}. * Failed. */ - final FreeStyleBuild build5 = PowerMockito.mock(FreeStyleBuild.class); - PowerMockito.when(build5.getPreviousCompletedBuild()).thenReturn(build4); - PowerMockito.when(build5.getResult()).thenReturn(Result.FAILURE); + final FreeStyleBuild build5 = PowerMockito.spy(new FreeStyleBuild(p)); + PowerMockito.doReturn(Result.FAILURE).when(build5).getResult(); + PowerMockito.doReturn(build4).when(build5).getPreviousCompletedBuild(); MockUtilities.addRequestor(build5, "B"); MockUtilities.addChangeSet(build5, "U", "V"); TestUtilities.checkRecipients(build5, new FirstFailingBuildSuspectsRecipientProvider(), "X", "V", "A"); @@ -129,9 +133,9 @@ public void testAddRecipients() throws Exception { * Committers {W}. * Success. */ - final FreeStyleBuild build6 = PowerMockito.mock(FreeStyleBuild.class); - PowerMockito.when(build6.getPreviousCompletedBuild()).thenReturn(build5); - PowerMockito.when(build6.getResult()).thenReturn(Result.UNSTABLE); + final FreeStyleBuild build6 = PowerMockito.spy(new FreeStyleBuild(p)); + PowerMockito.doReturn(Result.UNSTABLE).when(build6).getResult(); + PowerMockito.doReturn(build5).when(build6).getPreviousCompletedBuild(); MockUtilities.addRequestor(build6, "A"); MockUtilities.addChangeSet(build6, "W"); TestUtilities.checkRecipients(build6, new FirstFailingBuildSuspectsRecipientProvider()); diff --git a/src/test/java/hudson/plugins/emailext/plugins/recipients/MockUtilities.java b/src/test/java/hudson/plugins/emailext/plugins/recipients/MockUtilities.java index e81bf1af8..7247f35ff 100644 --- a/src/test/java/hudson/plugins/emailext/plugins/recipients/MockUtilities.java +++ b/src/test/java/hudson/plugins/emailext/plugins/recipients/MockUtilities.java @@ -123,7 +123,7 @@ public static void addChangeSet(final WorkflowRun build, final String... inAutho public static void addChangeSet(final AbstractBuild build, final String... inAuthors) { ChangeLogSet changeSet = makeChangeSet(build, inAuthors); - PowerMockito.when(build.getChangeSet()).thenReturn((ChangeLogSet)changeSet); + PowerMockito.doReturn(changeSet).when(build).getChangeSet(); } public static void addRequestor(final AbstractBuild build, final String requestor) throws Exception { @@ -137,7 +137,7 @@ public User answer(InvocationOnMock invocation) throws Throwable { }).when(User.class, "get", Mockito.anyString(), Mockito.anyBoolean(), Mockito.anyMap()); final Cause.UserIdCause cause = PowerMockito.mock(Cause.UserIdCause.class); PowerMockito.when(cause.getUserId()).thenReturn(requestor); - PowerMockito.when(build.getCause(Cause.UserIdCause.class)).thenReturn(cause); + PowerMockito.doReturn(cause).when(build).getCause(Cause.UserIdCause.class); } public static void addTestResultAction(final AbstractBuild build, final AbstractBuild... failedSinces) { @@ -150,7 +150,7 @@ public static void addTestResultAction(final AbstractBuild build, final Ab final TestResultAction testResultAction = PowerMockito.mock(TestResultAction.class); PowerMockito.when(testResultAction.getFailedTests()).thenReturn(failedTests); PowerMockito.when(testResultAction.getFailCount()).thenReturn(failedTests.size()); - PowerMockito.when(build.getAction(AbstractTestResultAction.class)).thenReturn(testResultAction); + PowerMockito.doReturn(testResultAction).when(build).getAction(AbstractTestResultAction.class); } } From 57a94b4194ad46a84cd08e9f6c0fca5f8300d87e Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Mon, 8 May 2017 09:35:40 -0400 Subject: [PATCH 3/8] Revert pesky newline, bump mailer version to fix InjectedTest --- pom.xml | 2 +- .../plugins/recipients/DevelopersRecipientProvider.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index fc9b01079..c1899fe36 100644 --- a/pom.xml +++ b/pom.xml @@ -133,7 +133,7 @@ org.jenkins-ci.plugins mailer - 1.5 + 1.13 org.jvnet.hudson.plugins diff --git a/src/main/java/hudson/plugins/emailext/plugins/recipients/DevelopersRecipientProvider.java b/src/main/java/hudson/plugins/emailext/plugins/recipients/DevelopersRecipientProvider.java index b0f639843..0ec682d0c 100644 --- a/src/main/java/hudson/plugins/emailext/plugins/recipients/DevelopersRecipientProvider.java +++ b/src/main/java/hudson/plugins/emailext/plugins/recipients/DevelopersRecipientProvider.java @@ -39,7 +39,6 @@ public void send(final String format, final Object... args) { } } final Debug debug = new Debug(); - Set users = RecipientProviderUtilities.getChangeSetAuthors(Collections.>singleton(context.getRun()), debug); RecipientProviderUtilities.addUsers(users, context, env, to, cc, bcc, debug); } From cd6c1f6b9aeaf6aef6df1ad33e42e111e9eab916 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Thu, 18 May 2017 12:22:38 -0400 Subject: [PATCH 4/8] Use Jenkins 2.60 and workflow-job 2.12. --- pom.xml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pom.xml b/pom.xml index c1899fe36..bd47cd639 100644 --- a/pom.xml +++ b/pom.xml @@ -32,9 +32,8 @@ UTF-8 2.9 2.5 - 2.11-20170504.162002-1 - 2.58-20170502.192524-8 - 2.58-20170502.192544-8 + 2.12 + 2.60 2.11 2.29 2.11 From da47c132df43db65d505fdaade2a0099ace5fb21 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Fri, 19 May 2017 12:11:17 -0400 Subject: [PATCH 5/8] Switch to reflection to keep a lower core. --- pom.xml | 6 +++--- .../recipients/CulpritsRecipientProvider.java | 21 ++++++++++++++----- .../RecipientProviderUtilities.java | 12 ++++++++--- 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/pom.xml b/pom.xml index bd47cd639..2a1319964 100644 --- a/pom.xml +++ b/pom.xml @@ -32,14 +32,14 @@ UTF-8 2.9 2.5 - 2.12 - 2.60 + 2.11 + 2.0 2.11 2.29 2.11 2.4 2.0.7 - 8 + 7 2 diff --git a/src/main/java/hudson/plugins/emailext/plugins/recipients/CulpritsRecipientProvider.java b/src/main/java/hudson/plugins/emailext/plugins/recipients/CulpritsRecipientProvider.java index 98999bb15..b6fb7d779 100644 --- a/src/main/java/hudson/plugins/emailext/plugins/recipients/CulpritsRecipientProvider.java +++ b/src/main/java/hudson/plugins/emailext/plugins/recipients/CulpritsRecipientProvider.java @@ -6,6 +6,8 @@ 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.Result; @@ -16,11 +18,12 @@ import hudson.plugins.emailext.plugins.RecipientProvider; import hudson.plugins.emailext.plugins.RecipientProviderDescriptor; import jenkins.model.Jenkins; -import jenkins.scm.RunWithSCM; 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; @@ -51,10 +54,18 @@ public void send(final String format, final Object... args) { } final Debug debug = new Debug(); Run run = context.getRun(); - if (run instanceof RunWithSCM) { - Set users = ((RunWithSCM)run).getCulprits(); - RecipientProviderUtilities.addUsers(users, context, env, to, cc, bcc, debug); - } else { + // 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"); + if (Set.class.isAssignableFrom(getCulprits.getReturnType())) { + @SuppressWarnings("unchecked") + Set users = (Set) 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) { + debug.send("Exception getting culprits for %s: %s", run, e); List> builds = new ArrayList<>(); Run build = run; builds.add(build); diff --git a/src/main/java/hudson/plugins/emailext/plugins/recipients/RecipientProviderUtilities.java b/src/main/java/hudson/plugins/emailext/plugins/recipients/RecipientProviderUtilities.java index 12f561b33..aa63a51d0 100644 --- a/src/main/java/hudson/plugins/emailext/plugins/recipients/RecipientProviderUtilities.java +++ b/src/main/java/hudson/plugins/emailext/plugins/recipients/RecipientProviderUtilities.java @@ -26,6 +26,7 @@ import com.google.common.base.Predicates; import com.google.common.collect.Iterables; import hudson.EnvVars; +import hudson.model.AbstractBuild; import hudson.model.Cause; import hudson.model.Item; import hudson.model.Run; @@ -36,6 +37,7 @@ import hudson.scm.ChangeLogSet; import hudson.tasks.MailSender; +import javax.mail.MethodNotSupportedException; import javax.mail.internet.InternetAddress; import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; @@ -47,7 +49,6 @@ import java.util.logging.Logger; import javax.annotation.CheckForNull; import jenkins.model.Jenkins; -import jenkins.scm.RunWithSCM; import org.acegisecurity.Authentication; import org.acegisecurity.userdetails.UsernameNotFoundException; @@ -66,11 +67,16 @@ public static Set getChangeSetAuthors(final Collection> runs, fi final Set users = new HashSet<>(); for (final Run run : runs) { debug.send(" build: %d", run.getNumber()); - if (run instanceof RunWithSCM) { - for (ChangeLogSet changeLogSet : ((RunWithSCM)run).getChangeSets()) { + // 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) { + debug.send(" changeLogSet was null"); + } else { 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())) { From 1ca8b491ed3630d3fb61607b6796f5ae5fdaaec7 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Sat, 20 May 2017 11:34:33 -0400 Subject: [PATCH 6/8] Minor code review responses --- .../plugins/recipients/CulpritsRecipientProvider.java | 10 +++++++++- .../plugins/recipients/RecipientProviderUtilities.java | 1 - 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/main/java/hudson/plugins/emailext/plugins/recipients/CulpritsRecipientProvider.java b/src/main/java/hudson/plugins/emailext/plugins/recipients/CulpritsRecipientProvider.java index b6fb7d779..add492723 100644 --- a/src/main/java/hudson/plugins/emailext/plugins/recipients/CulpritsRecipientProvider.java +++ b/src/main/java/hudson/plugins/emailext/plugins/recipients/CulpritsRecipientProvider.java @@ -18,6 +18,7 @@ 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; @@ -54,9 +55,13 @@ public void send(final String format, final Object... args) { } final Debug debug = new Debug(); Run run = context.getRun(); + + 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 users = (Set) getCulprits.invoke(run); @@ -65,7 +70,10 @@ public void send(final String format, final Object... args) { } } } catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException e) { - debug.send("Exception getting culprits for %s: %s", run, 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> builds = new ArrayList<>(); Run build = run; builds.add(build); diff --git a/src/main/java/hudson/plugins/emailext/plugins/recipients/RecipientProviderUtilities.java b/src/main/java/hudson/plugins/emailext/plugins/recipients/RecipientProviderUtilities.java index aa63a51d0..629eb7a5d 100644 --- a/src/main/java/hudson/plugins/emailext/plugins/recipients/RecipientProviderUtilities.java +++ b/src/main/java/hudson/plugins/emailext/plugins/recipients/RecipientProviderUtilities.java @@ -37,7 +37,6 @@ import hudson.scm.ChangeLogSet; import hudson.tasks.MailSender; -import javax.mail.MethodNotSupportedException; import javax.mail.internet.InternetAddress; import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; From cd7d5691e52b52d0b3293a21f66527b68ac6798d Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Fri, 16 Jun 2017 11:49:11 -0400 Subject: [PATCH 7/8] Explicit workflow-support dependency. --- pom.xml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/pom.xml b/pom.xml index 2a1319964..c694c83d9 100644 --- a/pom.xml +++ b/pom.xml @@ -38,6 +38,7 @@ 2.29 2.11 2.4 + 2.13 2.0.7 7 2 @@ -182,6 +183,19 @@ ${workflow-api.version} test + + org.jenkins-ci.plugins.workflow + workflow-support + ${workflow-support.version} + test + + + org.jenkins-ci.plugins.workflow + workflow-support + ${workflow-support.version} + tests + test + org.jenkins-ci.plugins scm-api From b5c0d76d9482406c230608f20d08ee69da403921 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Tue, 20 Jun 2017 12:54:50 -0400 Subject: [PATCH 8/8] Fix broken tests. Sigh, mocking. --- .../FailingTestSuspectsRecipientProviderTest.java | 12 +++++++++++- ...rstFailingBuildSuspectsRecipientProviderTest.java | 12 +++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/test/java/hudson/plugins/emailext/plugins/recipients/FailingTestSuspectsRecipientProviderTest.java b/src/test/java/hudson/plugins/emailext/plugins/recipients/FailingTestSuspectsRecipientProviderTest.java index 606b28fa6..01df8ccc5 100644 --- a/src/test/java/hudson/plugins/emailext/plugins/recipients/FailingTestSuspectsRecipientProviderTest.java +++ b/src/test/java/hudson/plugins/emailext/plugins/recipients/FailingTestSuspectsRecipientProviderTest.java @@ -23,6 +23,7 @@ */ package hudson.plugins.emailext.plugins.recipients; +import hudson.PluginManager; import hudson.model.FreeStyleBuild; import hudson.model.FreeStyleProject; import hudson.model.Result; @@ -37,6 +38,7 @@ import org.powermock.api.mockito.PowerMockito; import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.modules.junit4.PowerMockRunner; +import org.powermock.reflect.Whitebox; @RunWith(PowerMockRunner.class) @PrepareForTest({ @@ -46,7 +48,8 @@ Mailer.class, Mailer.DescriptorImpl.class, User.class, - FreeStyleProject.class + FreeStyleProject.class, + PluginManager.class }) public class FailingTestSuspectsRecipientProviderTest { @@ -59,8 +62,15 @@ public void before() throws Exception { PowerMockito.when(extendedEmailPublisherDescriptor.getExcludedCommitters()).thenReturn(""); PowerMockito.when(jenkins.getDescriptorByType(ExtendedEmailPublisherDescriptor.class)).thenReturn(extendedEmailPublisherDescriptor); + + final PluginManager pluginManager = PowerMockito.mock(PluginManager.class); + Whitebox.setInternalState(pluginManager, "uberClassLoader", this.getClass().getClassLoader()); + + PowerMockito.when(jenkins.getPluginManager()).thenReturn(pluginManager); + PowerMockito.mockStatic(Jenkins.class); PowerMockito.doReturn(jenkins).when(Jenkins.class, "getActiveInstance"); + PowerMockito.doReturn(jenkins).when(Jenkins.class, "getInstance"); final Mailer.DescriptorImpl descriptor = PowerMockito.mock(Mailer.DescriptorImpl.class); PowerMockito.when(descriptor.getDefaultSuffix()).thenReturn("DOMAIN"); diff --git a/src/test/java/hudson/plugins/emailext/plugins/recipients/FirstFailingBuildSuspectsRecipientProviderTest.java b/src/test/java/hudson/plugins/emailext/plugins/recipients/FirstFailingBuildSuspectsRecipientProviderTest.java index 56f479967..819578a68 100644 --- a/src/test/java/hudson/plugins/emailext/plugins/recipients/FirstFailingBuildSuspectsRecipientProviderTest.java +++ b/src/test/java/hudson/plugins/emailext/plugins/recipients/FirstFailingBuildSuspectsRecipientProviderTest.java @@ -23,6 +23,7 @@ */ package hudson.plugins.emailext.plugins.recipients; +import hudson.PluginManager; import hudson.model.FreeStyleBuild; import hudson.model.FreeStyleProject; import hudson.model.Result; @@ -36,6 +37,7 @@ import org.powermock.api.mockito.PowerMockito; import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.modules.junit4.PowerMockRunner; +import org.powermock.reflect.Whitebox; @RunWith(PowerMockRunner.class) @PrepareForTest({ @@ -45,7 +47,8 @@ Mailer.class, Mailer.DescriptorImpl.class, User.class, - FreeStyleProject.class + FreeStyleProject.class, + PluginManager.class }) public class FirstFailingBuildSuspectsRecipientProviderTest { @@ -57,8 +60,15 @@ public void before() throws Exception { extendedEmailPublisherDescriptor.setDebugMode(true); PowerMockito.when(jenkins.getDescriptorByType(ExtendedEmailPublisherDescriptor.class)).thenReturn(extendedEmailPublisherDescriptor); + + final PluginManager pluginManager = PowerMockito.mock(PluginManager.class); + Whitebox.setInternalState(pluginManager, "uberClassLoader", this.getClass().getClassLoader()); + + PowerMockito.when(jenkins.getPluginManager()).thenReturn(pluginManager); + PowerMockito.mockStatic(Jenkins.class); PowerMockito.doReturn(jenkins).when(Jenkins.class, "getActiveInstance"); + PowerMockito.doReturn(jenkins).when(Jenkins.class, "getInstance"); final Mailer.DescriptorImpl descriptor = PowerMockito.mock(Mailer.DescriptorImpl.class); PowerMockito.when(descriptor.getDefaultSuffix()).thenReturn("DOMAIN");