Generate build targets in comment id order

Author: robertoffmoura

src/main/java/stashpullrequestbuilder/stashpullrequestbuilder/StashRepository.java

@@ -159,7 +159,7 @@ private boolean isStatusMessage(String content) {
     return false;
   }
 
-  public Collection<StashPullRequestBuildTarget> getBuildTargets(
+  public List<StashPullRequestBuildTarget> getBuildTargets(
       StashPullRequestResponseValue pullRequest) {
     if (shouldSkip(pullRequest)) {
       return new ArrayList<>();
@@ -186,19 +186,18 @@ public Collection<StashPullRequestBuildTarget> getBuildTargets(
     if (!isOnlyBuildOnComment) {
       return getBuildTargetsWithoutOnlyBuildOnCommentLogic(pullRequest, comments);
     }
-    Collection<StashPullRequestBuildTarget> buildTargets =
+    List<StashPullRequestBuildTarget> buildTargets =
         getBuildTargetsWithOnlyBuildOnCommentLogic(pullRequest, comments);
 
     if (!trigger.getCancelOutdatedJobsEnabled()) {
       return buildTargets;
     }
-    List<StashPullRequestBuildTarget> buildTargetsList = new ArrayList<>(buildTargets);
-    buildTargetsList.sort(
+    buildTargets.sort(
         Comparator.comparing(StashPullRequestBuildTarget::getBuildCommandCommentId).reversed());
-    Collection<StashPullRequestBuildTarget> buildTargetsToKeep =
-        buildTargetsList.stream().limit(1).collect(Collectors.toList());
-    Collection<StashPullRequestBuildTarget> buildTargetsToCancel =
-        buildTargetsList.stream().skip(1).collect(Collectors.toList());
+    List<StashPullRequestBuildTarget> buildTargetsToKeep =
+        buildTargets.stream().limit(1).collect(Collectors.toList());
+    List<StashPullRequestBuildTarget> buildTargetsToCancel =
+        buildTargets.stream().skip(1).collect(Collectors.toList());
     try {
       for (StashPullRequestBuildTarget buildTarget : buildTargetsToCancel) {
         postBuildCancelComment(pullRequest, buildTarget.getBuildCommandCommentId());
@@ -219,10 +218,13 @@ public Collection<StashPullRequestBuildTarget> getBuildTargets(
     return buildTargetsToKeep;
   }
 
-  private Collection<StashPullRequestBuildTarget> getBuildTargetsWithOnlyBuildOnCommentLogic(
+  private List<StashPullRequestBuildTarget> getBuildTargetsWithOnlyBuildOnCommentLogic(
       StashPullRequestResponseValue pullRequest, List<StashPullRequestComment> comments) {
     List<StashPullRequestBuildTarget> buildTargets = new ArrayList<>();
 
+    // Start with least recent comments
+    comments.sort(Comparator.naturalOrder());
+
     for (StashPullRequestComment comment : comments) {
       String content = comment.getText();
       if (StringUtils.isEmpty(content)) {
@@ -244,7 +246,7 @@ private Collection<StashPullRequestBuildTarget> getBuildTargetsWithOnlyBuildOnCo
     return buildTargets;
   }
 
-  private Collection<StashPullRequestBuildTarget> getBuildTargetsWithoutOnlyBuildOnCommentLogic(
+  private List<StashPullRequestBuildTarget> getBuildTargetsWithoutOnlyBuildOnCommentLogic(
       StashPullRequestResponseValue pullRequest, List<StashPullRequestComment> comments) {
     String sourceCommit = pullRequest.getFromRef().getLatestCommit();
     String destinationCommit = pullRequest.getToRef().getLatestCommit();

src/test/java/stashpullrequestbuilder/stashpullrequestbuilder/StashRepositoryTest.java

@@ -6,7 +6,6 @@
 import static org.hamcrest.Matchers.anEmptyMap;
 import static org.hamcrest.Matchers.arrayWithSize;
 import static org.hamcrest.Matchers.contains;
-import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.emptyArray;
@@ -453,20 +452,40 @@ public void getBuildTargets_onlyBuildOnComment_multiple_comments_generate_multip
     when(stashApiClient.getPullRequestComments(any(), any(), any())).thenReturn(comments);
     when(trigger.getCiBuildPhrases()).thenReturn("DO TEST");
     when(trigger.getOnlyBuildOnComment()).thenReturn(true);
+    when(trigger.getCancelOutdatedJobsEnabled()).thenReturn(false);
 
-    Collection<StashPullRequestBuildTarget> buildTargets =
-        stashRepository.getBuildTargets(pullRequest);
+    List<StashPullRequestBuildTarget> buildTargets = stashRepository.getBuildTargets(pullRequest);
 
     assertThat(buildTargets, hasSize(2));
     assertThat(
-        buildTargets,
-        containsInAnyOrder(
-            allOf(
-                hasProperty("additionalParameters", aMapWithSize(2)),
-                hasProperty(
-                    "additionalParameters",
-                    allOf(hasEntry("key1", "value1"), hasEntry("key2", "value2")))),
-            hasProperty("additionalParameters", anEmptyMap())));
+        buildTargets.get(0),
+        allOf(
+            hasProperty("additionalParameters", aMapWithSize(2)),
+            hasProperty(
+                "additionalParameters",
+                allOf(hasEntry("key1", "value1"), hasEntry("key2", "value2")))));
+    assertThat(buildTargets.get(1), hasProperty("additionalParameters", anEmptyMap()));
+  }
+
+  @Test
+  public void
+      getBuildTargets_onlyBuildOnComment_cancelOutdatedJobsDisabled_generates_targets_in_comment_id_order()
+          throws Exception {
+    StashPullRequestComment comment1 = new StashPullRequestComment(1, "DO TEST\np:key1=value1");
+    StashPullRequestComment comment2 = new StashPullRequestComment(2, "DO TEST\np:key2=value2");
+    List<StashPullRequestComment> comments = Arrays.asList(comment2, comment1);
+    when(stashApiClient.getPullRequestComments(any(), any(), any())).thenReturn(comments);
+    when(trigger.getCiBuildPhrases()).thenReturn("DO TEST");
+    when(trigger.getOnlyBuildOnComment()).thenReturn(true);
+    when(trigger.getCancelOutdatedJobsEnabled()).thenReturn(false);
+
+    List<StashPullRequestBuildTarget> buildTargets = stashRepository.getBuildTargets(pullRequest);
+
+    assertThat(buildTargets, hasSize(2));
+    assertThat(
+        buildTargets.get(0), hasProperty("additionalParameters", hasEntry("key1", "value1")));
+    assertThat(
+        buildTargets.get(1), hasProperty("additionalParameters", hasEntry("key2", "value2")));
   }
 
   @Test