Fix #499 by prioritizing trigger phrase over retest phrase

jenkinsci · Oct 18, 2019 · 6ae51f9 · 6ae51f9
1 parent f4eb4bc
commit 6ae51f9
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 9 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequest.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequest.java
@@ -607,16 +607,11 @@ private void checkComment(GHIssueComment comment) throws IOException {
         } else if (helper.isOktotestPhrase(body) && helper.isAdmin(sender)) { // ok to test
             LOGGER.log(Level.FINEST, "Admin {0} gave OK to test", sender);
             setAccepted(true);
-        } else if (helper.isRetestPhrase(body)) { // test this please
-            LOGGER.log(Level.FINEST, "Retest phrase");
-            if (helper.isAdmin(sender)) {
-                LOGGER.log(Level.FINEST, "Admin {0} gave retest phrase", sender);
-                shouldRun = true;
-            } else if (accepted && helper.isWhitelisted(sender)) {
-                LOGGER.log(Level.FINEST, "Retest accepted and user {0} is whitelisted", sender);
-                shouldRun = true;
-            }
         } else if (helper.isTriggerPhrase(body)) { // trigger phrase
+            // The trigger phrase is prioritized over retest phrase because it
+            // encomposes the logic of retestPhrase (shouldRun = true), but also
+            // sets the triggered flag, which is important when the job is
+            // configured to "Only use trigger phrase for build triggering"
             LOGGER.log(Level.FINEST, "Trigger phrase");
             if (helper.isAdmin(sender)) {
                 LOGGER.log(Level.FINEST, "Admin {0} ran trigger phrase", sender);
@@ -627,6 +622,15 @@ private void checkComment(GHIssueComment comment) throws IOException {
                 shouldRun = true;
                 triggered = true;
             }
+        } else if (helper.isRetestPhrase(body)) { // test this please
+            LOGGER.log(Level.FINEST, "Retest phrase");
+            if (helper.isAdmin(sender)) {
+                LOGGER.log(Level.FINEST, "Admin {0} gave retest phrase", sender);
+                shouldRun = true;
+            } else if (accepted && helper.isWhitelisted(sender)) {
+                LOGGER.log(Level.FINEST, "Retest accepted and user {0} is whitelisted", sender);
+                shouldRun = true;
+            }
         }
 
         if (shouldRun) {

diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java
@@ -694,6 +694,7 @@ public void testCheckMethodWhenPrWasUpdatedWithRetestPhrase() throws Exception {
 
         verify(helper).isWhitelistPhrase(eq("test this please"));
         verify(helper).isOktotestPhrase(eq("test this please"));
+        verify(helper).isTriggerPhrase(eq("test this please"));
         verify(helper).isRetestPhrase(eq("test this please"));
         verify(helper).isAdmin(eq(ghUser));
         verify(helper, times(4)).isProjectDisabled();