From 91f4cb1e7cc6251cf59ddfac4625dde6675fcfdc Mon Sep 17 00:00:00 2001 From: Ahmed Anwar Date: Sat, 11 Oct 2025 06:46:02 +0300 Subject: [PATCH 1/5] Refactor AbstractIssueSelector and DefaultIssueSelector for clarity and reuse --- .../jira/selector/AbstractIssueSelector.java | 82 +++++++++++++++++++ .../jira/selector/DefaultIssueSelector.java | 75 ----------------- 2 files changed, 82 insertions(+), 75 deletions(-) diff --git a/src/main/java/hudson/plugins/jira/selector/AbstractIssueSelector.java b/src/main/java/hudson/plugins/jira/selector/AbstractIssueSelector.java index 6f48655ef..2c2e20ab6 100644 --- a/src/main/java/hudson/plugins/jira/selector/AbstractIssueSelector.java +++ b/src/main/java/hudson/plugins/jira/selector/AbstractIssueSelector.java @@ -3,10 +3,22 @@ import edu.umd.cs.findbugs.annotations.NonNull; import hudson.ExtensionPoint; import hudson.model.AbstractDescribableImpl; +import hudson.model.ParameterValue; +import hudson.model.ParametersAction; import hudson.model.Run; import hudson.model.TaskListener; import hudson.plugins.jira.JiraSite; +import hudson.plugins.jira.RunScmChangeExtractor; +import hudson.plugins.jira.listissuesparameter.JiraIssueParameterValue; +import hudson.scm.ChangeLogSet; +import hudson.scm.ChangeLogSet.Entry; + import java.util.Set; +import java.util.logging.Logger; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.apache.commons.lang.StringUtils; /** * Strategy of finding issues which should be updated after completed run. @@ -16,6 +28,8 @@ public abstract class AbstractIssueSelector extends AbstractDescribableImpl implements ExtensionPoint { + private static final Logger BASE_LOGGER = Logger.getLogger(AbstractIssueSelector.class.getName()); + /** * Finds the strings that match Jira issue ID patterns. * @@ -29,4 +43,72 @@ public abstract class AbstractIssueSelector extends AbstractDescribableImpl findIssueIds( @NonNull Run run, @NonNull JiraSite site, @NonNull TaskListener listener); + + /** + * Adds issues to issueIds from parameters + */ + protected void addIssuesFromParameters( + Run build, JiraSite site, TaskListener listener, Set issueIds) { + // Now look for any JiraIssueParameterValue's set in the build + // Implements JENKINS-12312 + ParametersAction parameters = build.getAction(ParametersAction.class); + + if (parameters != null) { + for (ParameterValue val : parameters.getParameters()) { + if (val instanceof JiraIssueParameterValue) { + String issueId = ((JiraIssueParameterValue) val).getValue().toString(); + if (issueIds.add(issueId)) { + BASE_LOGGER.finer("Added perforce issue " + issueId + " from build " + build); + } + } + } + } + } + + /** + * Calls {@link #findIssues(Run, Set, Pattern, TaskListener)} with + * {@link JiraSite#getIssuePattern()} as pattern + */ + protected void addIssuesFromChangeLog(Run build, JiraSite site, TaskListener listener, Set issueIds) { + Pattern pattern = site.getIssuePattern(); + findIssues(build, issueIds, pattern, listener); + } + + /** + * Adds issues to issueIds from the current build. Issues from parameters + * are added as well as issues matching pattern + * {@link #addIssuesFromChangeLog(Run, JiraSite, TaskListener, Set)} + * {@link #addIssuesFromParameters(Run, JiraSite, TaskListener, Set)} + */ + protected void addIssuesFromCurrentBuild( + Run build, JiraSite site, TaskListener listener, Set issueIds) { + addIssuesFromChangeLog(build, site, listener, issueIds); + addIssuesFromParameters(build, site, listener, issueIds); + } + + /** + * Finds the strings that match Jira issue ID patterns. This method returns + * all likely candidates and doesn't check if such ID actually exists or + * not. We don't want to use {@link JiraSite#existsIssue(String)} here so + * that new projects in Jira can be detected. + * + */ + protected static void findIssues(Run build, Set issueIds, Pattern pattern, TaskListener listener) { + for (ChangeLogSet set : RunScmChangeExtractor.getChanges(build)) { + for (Entry change : set) { + BASE_LOGGER.fine("Looking for Jira ID in " + change.getMsg()); + Matcher m = pattern.matcher(change.getMsg()); + + while (m.find()) { + if (m.groupCount() >= 1) { + String content = StringUtils.upperCase(m.group(1)); + issueIds.add(content); + } else { + listener.getLogger() + .println("Warning: The Jira pattern " + pattern + " doesn't define a capturing group!"); + } + } + } + } + } } diff --git a/src/main/java/hudson/plugins/jira/selector/DefaultIssueSelector.java b/src/main/java/hudson/plugins/jira/selector/DefaultIssueSelector.java index 9126b7919..2ef0298a7 100644 --- a/src/main/java/hudson/plugins/jira/selector/DefaultIssueSelector.java +++ b/src/main/java/hudson/plugins/jira/selector/DefaultIssueSelector.java @@ -5,26 +5,19 @@ import hudson.model.AbstractBuild; import hudson.model.AbstractBuild.DependencyChange; import hudson.model.Descriptor; -import hudson.model.ParameterValue; -import hudson.model.ParametersAction; import hudson.model.Run; import hudson.model.TaskListener; import hudson.plugins.jira.JiraCarryOverAction; import hudson.plugins.jira.JiraSite; import hudson.plugins.jira.Messages; import hudson.plugins.jira.RunScmChangeExtractor; -import hudson.plugins.jira.listissuesparameter.JiraIssueParameterValue; -import hudson.scm.ChangeLogSet; -import hudson.scm.ChangeLogSet.Entry; import java.util.Collection; import java.util.HashSet; import java.util.LinkedHashSet; import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.regex.Matcher; import java.util.regex.Pattern; -import org.apache.commons.lang.StringUtils; import org.kohsuke.stapler.DataBoundConstructor; public class DefaultIssueSelector extends AbstractIssueSelector { @@ -58,41 +51,6 @@ protected Logger getLogger() { return LOGGER; } - /** - * Finds the strings that match Jira issue ID patterns. This method returns - * all likely candidates and doesn't check if such ID actually exists or - * not. We don't want to use {@link JiraSite#existsIssue(String)} here so - * that new projects in Jira can be detected. - * - */ - protected static void findIssues(Run build, Set issueIds, Pattern pattern, TaskListener listener) { - for (ChangeLogSet set : RunScmChangeExtractor.getChanges(build)) { - for (Entry change : set) { - LOGGER.fine("Looking for Jira ID in " + change.getMsg()); - Matcher m = pattern.matcher(change.getMsg()); - - while (m.find()) { - if (m.groupCount() >= 1) { - String content = StringUtils.upperCase(m.group(1)); - issueIds.add(content); - } else { - listener.getLogger() - .println("Warning: The Jira pattern " + pattern + " doesn't define a capturing group!"); - } - } - } - } - } - - /** - * Calls {@link #findIssues(Run, Set, Pattern, TaskListener)} with - * {@link JiraSite#getIssuePattern()} as pattern - */ - protected void addIssuesFromChangeLog(Run build, JiraSite site, TaskListener listener, Set issueIds) { - Pattern pattern = site.getIssuePattern(); - findIssues(build, issueIds, pattern, listener); - } - /** * Adds issues to issueIds. Adds issues carried over from previous build, * issues from current build and from dependent builds @@ -106,18 +64,6 @@ protected void addIssuesRecursive(Run build, JiraSite site, TaskListener l addIssuesFromDependentBuilds(build, site, listener, issuesIds); } - /** - * Adds issues to issueIds from the current build. Issues from parameters - * are added as well as issues matching pattern - * {@link #addIssuesFromChangeLog(Run, JiraSite, TaskListener, Set)} - * {@link #addIssuesFromParameters(Run, JiraSite, TaskListener, Set)} - */ - protected void addIssuesFromCurrentBuild( - Run build, JiraSite site, TaskListener listener, Set issueIds) { - addIssuesFromChangeLog(build, site, listener, issueIds); - addIssuesFromParameters(build, site, listener, issueIds); - } - /** * Adds issues to issueIds by examining dependency changes from last build. * For each dependency change @@ -139,27 +85,6 @@ protected void addIssuesFromDependentBuilds( } } - /** - * Adds issues to issueIds from parameters - */ - protected void addIssuesFromParameters( - Run build, JiraSite site, TaskListener listener, Set issueIds) { - // Now look for any JiraIssueParameterValue's set in the build - // Implements JENKINS-12312 - ParametersAction parameters = build.getAction(ParametersAction.class); - - if (parameters != null) { - for (ParameterValue val : parameters.getParameters()) { - if (val instanceof JiraIssueParameterValue) { - String issueId = ((JiraIssueParameterValue) val).getValue().toString(); - if (issueIds.add(issueId)) { - getLogger().finer("Added perforce issue " + issueId + " from build " + build); - } - } - } - } - } - /** * Adds issues that were carried over from previous build to issueIds */ From f5817303dcc76059aa1e3b0338b9375565ffbba2 Mon Sep 17 00:00:00 2001 From: Ahmed Anwar Date: Sat, 11 Oct 2025 06:49:02 +0300 Subject: [PATCH 2/5] Add selector to collect issues since last successful build and corresponding tests --- ...SinceLastSuccessfulBuildIssueSelector.java | 67 +++++ .../hudson/plugins/jira/Messages.properties | 1 + ...eLastSuccessfulBuildIssueSelectorTest.java | 243 ++++++++++++++++++ 3 files changed, 311 insertions(+) create mode 100644 src/main/java/hudson/plugins/jira/selector/SinceLastSuccessfulBuildIssueSelector.java create mode 100644 src/test/java/hudson/plugins/jira/selector/SinceLastSuccessfulBuildIssueSelectorTest.java diff --git a/src/main/java/hudson/plugins/jira/selector/SinceLastSuccessfulBuildIssueSelector.java b/src/main/java/hudson/plugins/jira/selector/SinceLastSuccessfulBuildIssueSelector.java new file mode 100644 index 000000000..e03ccd4df --- /dev/null +++ b/src/main/java/hudson/plugins/jira/selector/SinceLastSuccessfulBuildIssueSelector.java @@ -0,0 +1,67 @@ +package hudson.plugins.jira.selector; + +import java.util.LinkedHashSet; +import java.util.Set; + +import org.kohsuke.stapler.DataBoundConstructor; + +import edu.umd.cs.findbugs.annotations.NonNull; +import hudson.Extension; +import hudson.model.Descriptor; +import hudson.model.Run; +import hudson.model.TaskListener; +import hudson.plugins.jira.JiraSite; +import hudson.plugins.jira.Messages; + +/** + * Selects JIRA issues from the current build and all builds + * since the last successful one. + */ +public class SinceLastSuccessfulBuildIssueSelector extends AbstractIssueSelector { + + @DataBoundConstructor + public SinceLastSuccessfulBuildIssueSelector() {} + + @Override + public Set findIssueIds( + @NonNull final Run run, @NonNull final JiraSite site, @NonNull final TaskListener listener) { + Set issueIds = new LinkedHashSet<>(); + + Run lastSuccessfulBuild = run.getPreviousSuccessfulBuild(); + + if (lastSuccessfulBuild == null) { + listener.getLogger().println("No previous successful build found. Searching only in current build."); + addIssuesFromCurrentBuild(run, site, listener, issueIds); + return issueIds; + } + + listener.getLogger().println( + "Collecting JIRA issues since last successful build #" + + lastSuccessfulBuild.getNumber()); + + Run build = run; + int buildsProcessed = 0; + + while (build != null && build != lastSuccessfulBuild) { + addIssuesFromCurrentBuild(build, site, listener, issueIds); + + buildsProcessed++; + build = build.getPreviousBuild(); + } + + listener.getLogger().println( + "Found " + issueIds.size() + " JIRA issue(s) across " + + buildsProcessed + " build(s)"); + + return issueIds; + } + + @Extension + public static final class DescriptorImpl extends Descriptor { + + @Override + public String getDisplayName() { + return Messages.SinceLastSuccessfulBuildIssueSelector_DisplayName(); + } + } +} diff --git a/src/main/resources/hudson/plugins/jira/Messages.properties b/src/main/resources/hudson/plugins/jira/Messages.properties index 4b71b021c..3b053ef1d 100644 --- a/src/main/resources/hudson/plugins/jira/Messages.properties +++ b/src/main/resources/hudson/plugins/jira/Messages.properties @@ -47,3 +47,4 @@ ErrorCommentingIssues=[Jira] Could not comment on some issues: {0} JiraSite.threadExecutorMinimunSize = Thread Executor Size must be at least {0} (higher values are recommended) JiraSite.timeoutMinimunValue = Connection timeout must be at least {0} JiraSite.readTimeoutMinimunValue = Read timeout must be at least {0} +SinceLastSuccessfulBuildIssueSelector.DisplayName = Since last successful build selector diff --git a/src/test/java/hudson/plugins/jira/selector/SinceLastSuccessfulBuildIssueSelectorTest.java b/src/test/java/hudson/plugins/jira/selector/SinceLastSuccessfulBuildIssueSelectorTest.java new file mode 100644 index 000000000..d7ec96280 --- /dev/null +++ b/src/test/java/hudson/plugins/jira/selector/SinceLastSuccessfulBuildIssueSelectorTest.java @@ -0,0 +1,243 @@ +package hudson.plugins.jira.selector; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.collection.IsCollectionWithSize.hasSize; +import static org.hamcrest.collection.IsEmptyCollection.empty; +import static org.hamcrest.core.IsEqual.equalTo; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import hudson.model.FreeStyleBuild; +import hudson.model.TaskListener; +import hudson.plugins.jira.JiraSite; +import hudson.scm.ChangeLogSet; +import hudson.scm.ChangeLogSet.Entry; +import java.io.PrintStream; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class SinceLastSuccessfulBuildIssueSelectorTest { + + private static class MockEntry extends Entry { + private final String msg; + + public MockEntry(String msg) { + this.msg = msg; + } + + @Override + public java.util.Collection getAffectedPaths() { + return null; + } + + @Override + public hudson.model.User getAuthor() { + return null; + } + + @Override + public String getMsg() { + return this.msg; + } + } + + @Mock(strictness = Mock.Strictness.LENIENT) + private FreeStyleBuild currentRun; + + @Mock(strictness = Mock.Strictness.LENIENT) + private FreeStyleBuild previousSuccessfulRun; + + @Mock(strictness = Mock.Strictness.LENIENT) + private FreeStyleBuild intermediateRun1; + + @Mock(strictness = Mock.Strictness.LENIENT) + private FreeStyleBuild intermediateRun2; + + @Mock(strictness = Mock.Strictness.LENIENT) + private JiraSite site; + + @Mock(strictness = Mock.Strictness.LENIENT) + private TaskListener listener; + + @Mock(strictness = Mock.Strictness.LENIENT) + private PrintStream logger; + + @Mock(strictness = Mock.Strictness.LENIENT) + private ChangeLogSet changeLogSet; + + private SinceLastSuccessfulBuildIssueSelector selector; + + @BeforeEach + void setUp() { + selector = new SinceLastSuccessfulBuildIssueSelector(); + when(listener.getLogger()).thenReturn(logger); + + // Mock JiraSite with default issue pattern + when(site.getIssuePattern()).thenReturn(JiraSite.DEFAULT_ISSUE_PATTERN); + + // Mock empty change sets by default (can be overridden in individual tests) + when(changeLogSet.iterator()).thenReturn(Collections.emptyIterator()); + when(changeLogSet.isEmptySet()).thenReturn(true); + + // Mock getChangeSet() for AbstractBuild compatibility + when(currentRun.getChangeSet()).thenReturn(changeLogSet); + when(previousSuccessfulRun.getChangeSet()).thenReturn(changeLogSet); + when(intermediateRun1.getChangeSet()).thenReturn(changeLogSet); + when(intermediateRun2.getChangeSet()).thenReturn(changeLogSet); + + // Mock getChangeSets() for reflection-based extraction + when(currentRun.getChangeSets()).thenReturn(Collections.singletonList(changeLogSet)); + when(previousSuccessfulRun.getChangeSets()).thenReturn(Collections.singletonList(changeLogSet)); + when(intermediateRun1.getChangeSets()).thenReturn(Collections.singletonList(changeLogSet)); + when(intermediateRun2.getChangeSets()).thenReturn(Collections.singletonList(changeLogSet)); + } + + @Test + void findIssueIdsWhenNoPreviousSuccessfulBuild() { + when(currentRun.getPreviousSuccessfulBuild()).thenReturn(null); + + Set result = selector.findIssueIds(currentRun, site, listener); + + assertThat(result, empty()); + verify(logger).println("No previous successful build found. Searching only in current build."); + } + + @Test + void findIssueIdsWhenPreviousSuccessfulBuildExists() { + when(currentRun.getPreviousSuccessfulBuild()).thenReturn(previousSuccessfulRun); + when(previousSuccessfulRun.getNumber()).thenReturn(5); + when(currentRun.getPreviousBuild()).thenReturn(intermediateRun1); + when(intermediateRun1.getPreviousBuild()).thenReturn(intermediateRun2); + when(intermediateRun2.getPreviousBuild()).thenReturn(previousSuccessfulRun); + + selector.findIssueIds(currentRun, site, listener); + + verify(logger).println("Collecting JIRA issues since last successful build #5"); + verify(logger).println("Found 0 JIRA issue(s) across 3 build(s)"); + } + + @Test + void findIssueIdsWithSingleBuildSinceSuccess() { + // Only one build since last successful build + when(currentRun.getPreviousSuccessfulBuild()).thenReturn(previousSuccessfulRun); + when(previousSuccessfulRun.getNumber()).thenReturn(10); + when(currentRun.getPreviousBuild()).thenReturn(previousSuccessfulRun); + + selector.findIssueIds(currentRun, site, listener); + + verify(logger).println("Collecting JIRA issues since last successful build #10"); + verify(logger).println("Found 0 JIRA issue(s) across 1 build(s)"); + } + + @Test + void findIssueIdsWithMultipleBuildsSinceSuccess() { + when(currentRun.getPreviousSuccessfulBuild()).thenReturn(previousSuccessfulRun); + when(previousSuccessfulRun.getNumber()).thenReturn(3); + when(currentRun.getPreviousBuild()).thenReturn(intermediateRun1); + when(intermediateRun1.getPreviousBuild()).thenReturn(intermediateRun2); + when(intermediateRun2.getPreviousBuild()).thenReturn(previousSuccessfulRun); + + selector.findIssueIds(currentRun, site, listener); + + verify(logger).println("Collecting JIRA issues since last successful build #3"); + verify(logger).println("Found 0 JIRA issue(s) across 3 build(s)"); + } + + @Test + void findIssueIdsStopsAtSuccessfulBuild() { + // Build chain that includes the successful build + when(currentRun.getPreviousSuccessfulBuild()).thenReturn(previousSuccessfulRun); + when(previousSuccessfulRun.getNumber()).thenReturn(7); + when(currentRun.getPreviousBuild()).thenReturn(intermediateRun1); + when(intermediateRun1.getPreviousBuild()).thenReturn(previousSuccessfulRun); + + selector.findIssueIds(currentRun, site, listener); + + // Should stop processing at the successful build + verify(logger).println("Collecting JIRA issues since last successful build #7"); + verify(logger).println("Found 0 JIRA issue(s) across 2 build(s)"); + } + + @Test + void findIssueIdsHandlesNullBuildChain() { + // Build chain with null intermediate builds + when(currentRun.getPreviousSuccessfulBuild()).thenReturn(previousSuccessfulRun); + when(previousSuccessfulRun.getNumber()).thenReturn(1); + when(currentRun.getPreviousBuild()).thenReturn(null); + + selector.findIssueIds(currentRun, site, listener); + + verify(logger).println("Collecting JIRA issues since last successful build #1"); + verify(logger).println("Found 0 JIRA issue(s) across 1 build(s)"); + } + + @Test + void findIssueIdsExtractsIssuesFromChangelogs() { + when(currentRun.getPreviousSuccessfulBuild()).thenReturn(previousSuccessfulRun); + when(previousSuccessfulRun.getNumber()).thenReturn(5); + when(currentRun.getPreviousBuild()).thenReturn(intermediateRun1); + when(intermediateRun1.getPreviousBuild()).thenReturn(previousSuccessfulRun); + + Set currentRunEntries = new HashSet<>(Arrays.asList( + new MockEntry("Fixed JIRA-123"), + new MockEntry("Updated ABC-456") + )); + Set intermediateRunEntries = new HashSet<>(Arrays.asList( + new MockEntry("Fixed DEF-789") + )); + + // Create separate ChangeLogSet mocks for each build + ChangeLogSet currentRunChangeLogSet = mock(ChangeLogSet.class); + ChangeLogSet intermediateRunChangeLogSet = mock(ChangeLogSet.class); + + when(currentRunChangeLogSet.iterator()).thenReturn(currentRunEntries.iterator()); + when(currentRunChangeLogSet.isEmptySet()).thenReturn(false); + when(intermediateRunChangeLogSet.iterator()).thenReturn(intermediateRunEntries.iterator()); + when(intermediateRunChangeLogSet.isEmptySet()).thenReturn(false); + + when(currentRun.getChangeSet()).thenReturn(currentRunChangeLogSet); + when(currentRun.getChangeSets()).thenReturn(Collections.singletonList(currentRunChangeLogSet)); + when(intermediateRun1.getChangeSet()).thenReturn(intermediateRunChangeLogSet); + when(intermediateRun1.getChangeSets()).thenReturn(Collections.singletonList(intermediateRunChangeLogSet)); + + Set result = selector.findIssueIds(currentRun, site, listener); + + assertThat(result, hasSize(3)); + assertThat(result, equalTo(new HashSet<>(Arrays.asList("JIRA-123", "ABC-456", "DEF-789")))); + verify(logger).println("Collecting JIRA issues since last successful build #5"); + verify(logger).println("Found 3 JIRA issue(s) across 2 build(s)"); + } + + @Test + void findIssueIdsWithNoPreviousSuccessfulBuildExtractsFromCurrentBuild() { + // No previous successful build, but current build has issues + when(currentRun.getPreviousSuccessfulBuild()).thenReturn(null); + + Set currentRunEntries = new HashSet<>(Arrays.asList( + new MockEntry("Fixed JIRA-999"), + new MockEntry("Updated XYZ-111") + )); + + ChangeLogSet currentRunChangeLogSet = mock(ChangeLogSet.class); + when(currentRunChangeLogSet.iterator()).thenReturn(currentRunEntries.iterator()); + when(currentRunChangeLogSet.isEmptySet()).thenReturn(false); + + when(currentRun.getChangeSet()).thenReturn(currentRunChangeLogSet); + when(currentRun.getChangeSets()).thenReturn(Collections.singletonList(currentRunChangeLogSet)); + + Set result = selector.findIssueIds(currentRun, site, listener); + + assertThat(result, hasSize(2)); + assertThat(result, equalTo(new HashSet<>(Arrays.asList("JIRA-999", "XYZ-111")))); + verify(logger).println("No previous successful build found. Searching only in current build."); + } +} From 665b738c96e01012c73925681ad9a1ed1c12ce14 Mon Sep 17 00:00:00 2001 From: Ahmed Anwar Date: Sat, 11 Oct 2025 07:18:25 +0300 Subject: [PATCH 3/5] Apply Spotless formatting to fix style issues --- .../jira/selector/AbstractIssueSelector.java | 2 -- ...SinceLastSuccessfulBuildIssueSelector.java | 18 ++++++-------- ...eLastSuccessfulBuildIssueSelectorTest.java | 24 +++++++------------ 3 files changed, 16 insertions(+), 28 deletions(-) diff --git a/src/main/java/hudson/plugins/jira/selector/AbstractIssueSelector.java b/src/main/java/hudson/plugins/jira/selector/AbstractIssueSelector.java index 2c2e20ab6..63c6f9915 100644 --- a/src/main/java/hudson/plugins/jira/selector/AbstractIssueSelector.java +++ b/src/main/java/hudson/plugins/jira/selector/AbstractIssueSelector.java @@ -12,12 +12,10 @@ import hudson.plugins.jira.listissuesparameter.JiraIssueParameterValue; import hudson.scm.ChangeLogSet; import hudson.scm.ChangeLogSet.Entry; - import java.util.Set; import java.util.logging.Logger; import java.util.regex.Matcher; import java.util.regex.Pattern; - import org.apache.commons.lang.StringUtils; /** diff --git a/src/main/java/hudson/plugins/jira/selector/SinceLastSuccessfulBuildIssueSelector.java b/src/main/java/hudson/plugins/jira/selector/SinceLastSuccessfulBuildIssueSelector.java index e03ccd4df..b06899acc 100644 --- a/src/main/java/hudson/plugins/jira/selector/SinceLastSuccessfulBuildIssueSelector.java +++ b/src/main/java/hudson/plugins/jira/selector/SinceLastSuccessfulBuildIssueSelector.java @@ -1,10 +1,5 @@ package hudson.plugins.jira.selector; -import java.util.LinkedHashSet; -import java.util.Set; - -import org.kohsuke.stapler.DataBoundConstructor; - import edu.umd.cs.findbugs.annotations.NonNull; import hudson.Extension; import hudson.model.Descriptor; @@ -12,6 +7,9 @@ import hudson.model.TaskListener; import hudson.plugins.jira.JiraSite; import hudson.plugins.jira.Messages; +import java.util.LinkedHashSet; +import java.util.Set; +import org.kohsuke.stapler.DataBoundConstructor; /** * Selects JIRA issues from the current build and all builds @@ -35,9 +33,8 @@ public Set findIssueIds( return issueIds; } - listener.getLogger().println( - "Collecting JIRA issues since last successful build #" + - lastSuccessfulBuild.getNumber()); + listener.getLogger() + .println("Collecting JIRA issues since last successful build #" + lastSuccessfulBuild.getNumber()); Run build = run; int buildsProcessed = 0; @@ -49,9 +46,8 @@ public Set findIssueIds( build = build.getPreviousBuild(); } - listener.getLogger().println( - "Found " + issueIds.size() + " JIRA issue(s) across " + - buildsProcessed + " build(s)"); + listener.getLogger() + .println("Found " + issueIds.size() + " JIRA issue(s) across " + buildsProcessed + " build(s)"); return issueIds; } diff --git a/src/test/java/hudson/plugins/jira/selector/SinceLastSuccessfulBuildIssueSelectorTest.java b/src/test/java/hudson/plugins/jira/selector/SinceLastSuccessfulBuildIssueSelectorTest.java index d7ec96280..95e8df730 100644 --- a/src/test/java/hudson/plugins/jira/selector/SinceLastSuccessfulBuildIssueSelectorTest.java +++ b/src/test/java/hudson/plugins/jira/selector/SinceLastSuccessfulBuildIssueSelectorTest.java @@ -80,20 +80,20 @@ public String getMsg() { void setUp() { selector = new SinceLastSuccessfulBuildIssueSelector(); when(listener.getLogger()).thenReturn(logger); - + // Mock JiraSite with default issue pattern when(site.getIssuePattern()).thenReturn(JiraSite.DEFAULT_ISSUE_PATTERN); - + // Mock empty change sets by default (can be overridden in individual tests) when(changeLogSet.iterator()).thenReturn(Collections.emptyIterator()); when(changeLogSet.isEmptySet()).thenReturn(true); - + // Mock getChangeSet() for AbstractBuild compatibility when(currentRun.getChangeSet()).thenReturn(changeLogSet); when(previousSuccessfulRun.getChangeSet()).thenReturn(changeLogSet); when(intermediateRun1.getChangeSet()).thenReturn(changeLogSet); when(intermediateRun2.getChangeSet()).thenReturn(changeLogSet); - + // Mock getChangeSets() for reflection-based extraction when(currentRun.getChangeSets()).thenReturn(Collections.singletonList(changeLogSet)); when(previousSuccessfulRun.getChangeSets()).thenReturn(Collections.singletonList(changeLogSet)); @@ -187,13 +187,9 @@ void findIssueIdsExtractsIssuesFromChangelogs() { when(currentRun.getPreviousBuild()).thenReturn(intermediateRun1); when(intermediateRun1.getPreviousBuild()).thenReturn(previousSuccessfulRun); - Set currentRunEntries = new HashSet<>(Arrays.asList( - new MockEntry("Fixed JIRA-123"), - new MockEntry("Updated ABC-456") - )); - Set intermediateRunEntries = new HashSet<>(Arrays.asList( - new MockEntry("Fixed DEF-789") - )); + Set currentRunEntries = + new HashSet<>(Arrays.asList(new MockEntry("Fixed JIRA-123"), new MockEntry("Updated ABC-456"))); + Set intermediateRunEntries = new HashSet<>(Arrays.asList(new MockEntry("Fixed DEF-789"))); // Create separate ChangeLogSet mocks for each build ChangeLogSet currentRunChangeLogSet = mock(ChangeLogSet.class); @@ -222,10 +218,8 @@ void findIssueIdsWithNoPreviousSuccessfulBuildExtractsFromCurrentBuild() { // No previous successful build, but current build has issues when(currentRun.getPreviousSuccessfulBuild()).thenReturn(null); - Set currentRunEntries = new HashSet<>(Arrays.asList( - new MockEntry("Fixed JIRA-999"), - new MockEntry("Updated XYZ-111") - )); + Set currentRunEntries = + new HashSet<>(Arrays.asList(new MockEntry("Fixed JIRA-999"), new MockEntry("Updated XYZ-111"))); ChangeLogSet currentRunChangeLogSet = mock(ChangeLogSet.class); when(currentRunChangeLogSet.iterator()).thenReturn(currentRunEntries.iterator()); From 43fcd5b65dd04c471851ac14ca1d3c95c3124206 Mon Sep 17 00:00:00 2001 From: Ahmed Anwar Date: Sun, 19 Oct 2025 21:50:07 +0300 Subject: [PATCH 4/5] Update Messages.properties --- src/main/resources/hudson/plugins/jira/Messages.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/resources/hudson/plugins/jira/Messages.properties b/src/main/resources/hudson/plugins/jira/Messages.properties index 3b053ef1d..5b0c2b0d1 100644 --- a/src/main/resources/hudson/plugins/jira/Messages.properties +++ b/src/main/resources/hudson/plugins/jira/Messages.properties @@ -47,4 +47,4 @@ ErrorCommentingIssues=[Jira] Could not comment on some issues: {0} JiraSite.threadExecutorMinimunSize = Thread Executor Size must be at least {0} (higher values are recommended) JiraSite.timeoutMinimunValue = Connection timeout must be at least {0} JiraSite.readTimeoutMinimunValue = Read timeout must be at least {0} -SinceLastSuccessfulBuildIssueSelector.DisplayName = Since last successful build selector +SinceLastSuccessfulBuildIssueSelector.DisplayName = Select issues since the last successful build From 53f4247798827b0115e759f3bb94e70294a60a01 Mon Sep 17 00:00:00 2001 From: Ahmed Anwar Date: Sun, 19 Oct 2025 21:59:19 +0300 Subject: [PATCH 5/5] Improve coverage for shared issue selection logic --- .../selector/DefaultIssueSelectorTest.java | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/src/test/java/hudson/plugins/jira/selector/DefaultIssueSelectorTest.java b/src/test/java/hudson/plugins/jira/selector/DefaultIssueSelectorTest.java index 70fd5b8f1..4072fccd4 100644 --- a/src/test/java/hudson/plugins/jira/selector/DefaultIssueSelectorTest.java +++ b/src/test/java/hudson/plugins/jira/selector/DefaultIssueSelectorTest.java @@ -3,6 +3,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import hudson.model.BuildListener; @@ -233,4 +234,91 @@ void defaultPatternNotToMatchMavenRelease() { DefaultIssueSelector.findIssues(build, ids, JiraSite.DEFAULT_ISSUE_PATTERN, null); assertEquals(0, ids.size()); } + + /** + * Tests that non-JiraIssueParameterValue parameters are ignored. + * when ParameterValue is NOT instanceof JiraIssueParameterValue + */ + @Test + void testNonJiraIssueParameterValueIgnored() { + FreeStyleBuild build = mock(FreeStyleBuild.class); + ChangeLogSet changeLogSet = mock(ChangeLogSet.class); + BuildListener listener = mock(BuildListener.class); + JiraSite site = mock(JiraSite.class); + + when(site.getIssuePattern()).thenReturn(JiraSite.DEFAULT_ISSUE_PATTERN); + when(changeLogSet.iterator()).thenReturn(Collections.EMPTY_LIST.iterator()); + when(build.getChangeSet()).thenReturn(changeLogSet); + + // Create ParametersAction with non-JiraIssueParameterValue + ParametersAction action = mock(ParametersAction.class); + List parameters = new ArrayList<>(); + ParameterValue nonJiraParam = mock(ParameterValue.class); + parameters.add(nonJiraParam); + + when(build.getAction(ParametersAction.class)).thenReturn(action); + when(action.getParameters()).thenReturn(parameters); + + Set ids = new DefaultIssueSelector().findIssueIds(build, site, listener); + assertTrue(ids.isEmpty()); + } + + @Test + void testDuplicateIssueIdsNotAddedTwice() { + FreeStyleBuild build = mock(FreeStyleBuild.class); + ChangeLogSet changeLogSet = mock(ChangeLogSet.class); + BuildListener listener = mock(BuildListener.class); + JiraSite site = mock(JiraSite.class); + + when(site.getIssuePattern()).thenReturn(JiraSite.DEFAULT_ISSUE_PATTERN); + when(changeLogSet.iterator()).thenReturn(Collections.EMPTY_LIST.iterator()); + when(build.getChangeSet()).thenReturn(changeLogSet); + + // Create ParametersAction with duplicate JiraIssueParameterValue + ParametersAction action = mock(ParametersAction.class); + List parameters = new ArrayList<>(); + JiraIssueParameterValue parameter1 = mock(JiraIssueParameterValue.class); + JiraIssueParameterValue parameter2 = mock(JiraIssueParameterValue.class); + + when(parameter1.getValue()).thenReturn("JIRA-123"); + when(parameter2.getValue()).thenReturn("JIRA-123"); // Same issue ID + + parameters.add(parameter1); + parameters.add(parameter2); + + when(build.getAction(ParametersAction.class)).thenReturn(action); + when(action.getParameters()).thenReturn(parameters); + + Set ids = new DefaultIssueSelector().findIssueIds(build, site, listener); + assertEquals(1, ids.size()); + assertEquals("JIRA-123", ids.iterator().next()); + } + + @Test + void testPatternWithoutCapturingGroupTriggersWarning() { + FreeStyleBuild build = mock(FreeStyleBuild.class); + ChangeLogSet changeLogSet = mock(ChangeLogSet.class); + BuildListener listener = mock(BuildListener.class); + java.io.PrintStream printStream = mock(java.io.PrintStream.class); + + when(build.getChangeSet()).thenReturn(changeLogSet); + when(listener.getLogger()).thenReturn(printStream); + + // Create entry with text that matches pattern but pattern has no capturing group + Set entries = new HashSet(Arrays.asList(new MockEntry("Fixed ABC-123"))); + when(changeLogSet.iterator()).thenReturn(entries.iterator()); + + List> changeSets = new ArrayList<>(); + changeSets.add(changeLogSet); + when(build.getChangeSets()).thenReturn(changeSets); + + Set ids = new LinkedHashSet<>(); + // Pattern without capturing group - just matches but doesn't capture + Pattern patternWithoutGroup = Pattern.compile("ABC-\\d+"); + DefaultIssueSelector.findIssues(build, ids, patternWithoutGroup, listener); + + assertEquals(0, ids.size()); + verify(printStream) + .println("Warning: The Jira pattern " + patternWithoutGroup + " doesn't define a capturing group!"); + } }