From c4f2f783e2c20f85dc2c59dd7253ec6b86d91da1 Mon Sep 17 00:00:00 2001 From: Jose Molina Date: Wed, 11 Mar 2015 13:56:52 +0100 Subject: [PATCH 01/14] Checking "X-Hub-Signature" on GitHub hook POST reception. Added test cases --- .../plugins/ghprb/GhprbRootAction.java | 31 ++++++++++++++++++- .../jenkinsci/plugins/ghprb/GhprbTrigger.java | 13 ++++++++ .../plugins/ghprb/GhprbTrigger/config.jelly | 3 ++ .../org/jenkinsci/plugins/ghprb/GhprbIT.java | 6 ++-- .../ghprb/GhprbPullRequestMergeTest.java | 2 +- .../plugins/ghprb/GhprbRootActionTest.java | 1 + .../plugins/ghprb/GhprbTestUtil.java | 19 ++++++++++++ .../impl/GhprbDefaultBuildManagerTest.java | 2 +- .../BuildFlowBuildManagerTest.java | 2 +- 9 files changed, 72 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRootAction.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRootAction.java index db0ec3b3b..e65cdada7 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRootAction.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRootAction.java @@ -9,12 +9,15 @@ import org.acegisecurity.Authentication; import org.acegisecurity.context.SecurityContextHolder; import org.apache.commons.io.IOUtils; +import org.apache.mina.util.Base64; import org.kohsuke.github.GHEventPayload; import org.kohsuke.github.GHIssueState; import org.kohsuke.github.GHRepository; import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.StaplerResponse; +import javax.crypto.Mac; +import javax.crypto.spec.SecretKeySpec; import java.io.BufferedReader; import java.io.IOException; import java.io.StringReader; @@ -45,6 +48,7 @@ public String getUrlName() { public void doIndex(StaplerRequest req, StaplerResponse resp) { String event = req.getHeader("X-GitHub-Event"); + String signature = req.getHeader("X-GitHub-Signature"); String type = req.getContentType(); String payload = null; @@ -67,11 +71,36 @@ public void doIndex(StaplerRequest req, StaplerResponse resp) { } } + String secret = GhprbTrigger.getDscp().getSecret(); + if (payload == null) { logger.log(Level.SEVERE, "Payload is null, maybe content type '{0}' is not supported by this plugin. Please use 'application/json' or 'application/x-www-form-urlencoded'", new Object[] {type}); return; + } else if (secret != null && ! secret.isEmpty()) { + if (signature != null) { + SecretKeySpec keySpec = new SecretKeySpec(secret.getBytes(), "HmacSHA1"); + try { + Mac mac = Mac.getInstance("HmacSHA1"); + mac.init(keySpec); + byte[] clientSignatureBytes = mac.doFinal(payload.getBytes()); + String clientSignature = "sha1=" + new String(Base64.encodeBase64(clientSignatureBytes)); + if (! clientSignature.equals(signature)){ + logger.log(Level.SEVERE, "Local signature {0} doesn't match with external signature {1}.", + new Object[] {signature, clientSignature}); + return; + } else { + logger.log(Level.INFO, "Signatures checking OK"); + } + } catch (Exception e) { + logger.log(Level.SEVERE, "Couldn't match both signatures"); + return; + } + } else { + logger.log(Level.SEVERE, "Request doesn't contain a signature. Check that github has a secret that should be attached to the hook"); + return; + } } - + GhprbGitHub gh = GhprbTrigger.getDscp().getGitHub(); logger.log(Level.INFO, "Got payload event: {0}", event); diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java index aa5f2fa8e..cd52fe0d0 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java @@ -58,6 +58,7 @@ public class GhprbTrigger extends Trigger> { private String commitStatusContext; private transient Ghprb helper; private String project; + private String secret; @DataBoundConstructor public GhprbTrigger(String adminlist, @@ -75,6 +76,7 @@ public GhprbTrigger(String adminlist, Boolean allowMembersOfWhitelistedOrgsAsAdmin, String msgSuccess, String msgFailure, + String secret, String commitStatusContext) throws ANTLRException { super(cron); this.adminlist = adminlist; @@ -93,6 +95,7 @@ public GhprbTrigger(String adminlist, this.allowMembersOfWhitelistedOrgsAsAdmin = allowMembersOfWhitelistedOrgsAsAdmin; this.msgSuccess = msgSuccess; this.msgFailure = msgFailure; + this.secret = secret; } public static GhprbTrigger extractTrigger(AbstractProject p) { @@ -283,6 +286,10 @@ public String getMsgFailure() { return msgFailure; } + public String getSecret() { + return secret; + } + public String getTriggerPhrase() { if (triggerPhrase == null) { return ""; @@ -384,6 +391,7 @@ public static final class DescriptorImpl extends TriggerDescriptor { private String commitStatusContext = ""; private Boolean autoCloseFailedPullRequests = false; private Boolean displayBuildErrorsOnDownstreamBuilds = false; + private String secret = ""; @@ -437,6 +445,7 @@ public boolean configure(StaplerRequest req, JSONObject formData) throws FormExc msgSuccess = formData.getString("msgSuccess"); msgFailure = formData.getString("msgFailure"); commitStatusContext = formData.getString("commitStatusContext"); + secret = formData.getString("secret"); save(); gh = new GhprbGitHub(); @@ -516,6 +525,10 @@ public int getlogExcerptLines() { return logExcerptLines; } + public String getSecret() { + return secret; + } + public Boolean getAutoCloseFailedPullRequests() { return autoCloseFailedPullRequests; } diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbTrigger/config.jelly b/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbTrigger/config.jelly index 05bcc236d..9167af95b 100644 --- a/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbTrigger/config.jelly +++ b/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbTrigger/config.jelly @@ -6,6 +6,9 @@ + + + diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbIT.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbIT.java index c4843038d..1433688ff 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbIT.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbIT.java @@ -40,7 +40,7 @@ public void shouldBuildTriggersOnNewPR() throws Exception { // GIVEN FreeStyleProject project = jenkinsRule.createFreeStyleProject("PRJ"); GhprbTrigger trigger = new GhprbTrigger( - "user", "user", "", "*/1 * * * *", "retest this please", false, false, false, false, false, null, null, false, null, null, null + "user", "user", "", "*/1 * * * *", "retest this please", false, false, false, false, false, null, null, false, null, null, null, null ); given(commitPointer.getSha()).willReturn("sha"); JSONObject jsonObject = GhprbTestUtil.provideConfiguration(); @@ -74,7 +74,7 @@ public void shouldBuildTriggersOnUpdatingNewCommitsPR() throws Exception { // GIVEN FreeStyleProject project = jenkinsRule.createFreeStyleProject("PRJ"); GhprbTrigger trigger = new GhprbTrigger( - "user", "user", "", "*/1 * * * *", "retest this please", false, false, false, false, false, null, null, false, null, null, null + "user", "user", "", "*/1 * * * *", "retest this please", false, false, false, false, false, null, null, false, null, null, null, null ); given(commitPointer.getSha()).willReturn("sha").willReturn("sha").willReturn("newOne").willReturn("newOne"); given(ghPullRequest.getComments()).willReturn(Lists.newArrayList()); @@ -102,7 +102,7 @@ public void shouldBuildTriggersOnUpdatingRetestMessagePR() throws Exception { // GIVEN FreeStyleProject project = jenkinsRule.createFreeStyleProject("PRJ"); GhprbTrigger trigger = new GhprbTrigger( - "user", "user", "", "*/1 * * * *", "retest this please", false, false, false, false, false, null, null, false, null, null, null + "user", "user", "", "*/1 * * * *", "retest this please", false, false, false, false, false, null, null, false, null, null, null, null ); given(commitPointer.getSha()).willReturn("sha"); diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestMergeTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestMergeTest.java index a6dc4a626..e6c9ad0a4 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestMergeTest.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestMergeTest.java @@ -92,7 +92,7 @@ public class GhprbPullRequestMergeTest { @Before public void beforeTest() throws Exception { - GhprbTrigger trigger = spy(new GhprbTrigger(adminList, "user", "", "*/1 * * * *", triggerPhrase, false, false, false, false, false, null, null, false, null, null, null)); + GhprbTrigger trigger = spy(new GhprbTrigger(adminList, "user", "", "*/1 * * * *", triggerPhrase, false, false, false, false, false, null, null, false, null, null, null, null)); ConcurrentMap pulls = new ConcurrentHashMap(1); pulls.put(pullId, pullRequest); diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRootActionTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRootActionTest.java index 06ca95c98..79e8e67fa 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRootActionTest.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRootActionTest.java @@ -228,6 +228,7 @@ public void testUrlEncoded() { given(req.getContentType()).willReturn("application/x-www-form-urlencoded"); given(req.getParameter("payload")).willReturn(payload); given(req.getHeader("X-GitHub-Event")).willReturn("issue_comment"); + given(req.getHeader("X-GitHub-Signature")).willReturn(GhprbTestUtil.createSignature(payload)); GhprbRootAction ra = new GhprbRootAction(); ra.doIndex(req, null); diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTestUtil.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTestUtil.java index 3939f6d83..360353a71 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTestUtil.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTestUtil.java @@ -20,6 +20,7 @@ import java.net.MalformedURLException; import java.net.URL; +import org.apache.mina.util.Base64; import org.joda.time.DateTime; import org.kohsuke.github.GHCommitPointer; import org.kohsuke.github.GHPullRequest; @@ -35,10 +36,14 @@ import hudson.plugins.git.UserRemoteConfig; import net.sf.json.JSONObject; +import javax.crypto.Mac; +import javax.crypto.spec.SecretKeySpec; + public class GhprbTestUtil { public static final int INITIAL_RATE_LIMIT = 5000; public static final String GHPRB_PLUGIN_NAME = "ghprb"; + public static final String GITHUB_SECRET = "61c6f58144702262d65d1015445e5f2635ccb6a6"; // TODO: When anyone has time to investigate mocking the github request. // public static void mockGithubUserPage() { @@ -105,6 +110,7 @@ public static JSONObject provideConfiguration() { jsonObject.put("msgSuccess", "Success"); jsonObject.put("msgFailure", "Failure"); jsonObject.put("commitStatusContext", "Status Context"); + jsonObject.put("secret", GITHUB_SECRET); return jsonObject; } @@ -128,4 +134,17 @@ public static GitSCM provideGitSCM() { private GhprbTestUtil() { } + public static String createSignature(String payload) { + SecretKeySpec keySpec = new SecretKeySpec(GITHUB_SECRET.getBytes(), "HmacSHA1"); + try { + Mac mac = Mac.getInstance("HmacSHA1"); + mac.init(keySpec); + byte[] signatureBytes = mac.doFinal(payload.getBytes()); + String signature = new String(Base64.encodeBase64(signatureBytes)); + return "sha1=" + signature; + } catch (Exception e){ + return ""; + } + } + } \ No newline at end of file diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/manager/impl/GhprbDefaultBuildManagerTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/manager/impl/GhprbDefaultBuildManagerTest.java index e479f09e7..47959cbf8 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/manager/impl/GhprbDefaultBuildManagerTest.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/manager/impl/GhprbDefaultBuildManagerTest.java @@ -66,7 +66,7 @@ private MatrixProject givenThatGhprbHasBeenTriggeredForAMatrixProject() throws E GhprbTrigger trigger = new GhprbTrigger("user", "user", "", "*/1 * * * *", "retest this please", false, false, false, false, - false, null, null, false, null, null, null); + false, null, null, false, null, null, null, null); given(commitPointer.getSha()).willReturn("sha"); diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/manager/impl/downstreambuilds/BuildFlowBuildManagerTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/manager/impl/downstreambuilds/BuildFlowBuildManagerTest.java index 0116bee08..a4ab8bb5f 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/manager/impl/downstreambuilds/BuildFlowBuildManagerTest.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/manager/impl/downstreambuilds/BuildFlowBuildManagerTest.java @@ -109,7 +109,7 @@ private BuildFlow givenThatGhprbHasBeenTriggeredForABuildFlowProject() GhprbTrigger trigger = new GhprbTrigger("user", "user", "", "*/1 * * * *", "retest this please", false, false, false, false, - false, null, null, false, null, null, null); + false, null, null, false, null, null, null, null); given(commitPointer.getSha()).willReturn("sha"); JSONObject jsonObject = GhprbTestUtil.provideConfiguration(); From dd33f21c9991e872e96c755f2e3a4e17cc66bf71 Mon Sep 17 00:00:00 2001 From: Jose Molina Date: Wed, 18 Mar 2015 11:49:51 +0100 Subject: [PATCH 02/14] FIX: Moved signature checking to GhprbRepository. Added tests --- .../plugins/ghprb/GhprbRepository.java | 35 +++ .../plugins/ghprb/GhprbRootAction.java | 73 +++--- .../jenkinsci/plugins/ghprb/GhprbTrigger.java | 6 - .../plugins/ghprb/GhprbRepositoryTest.java | 34 +++ .../plugins/ghprb/GhprbRootActionTest.java | 199 +--------------- .../plugins/ghprb/GhprbTestUtil.java | 212 ++++++++++++++++-- 6 files changed, 301 insertions(+), 258 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRepository.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRepository.java index 53e23ec36..878160574 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRepository.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRepository.java @@ -6,10 +6,14 @@ import hudson.model.TaskListener; import jenkins.model.Jenkins; +import org.apache.commons.codec.binary.Hex; import org.kohsuke.github.*; import org.kohsuke.github.GHEventPayload.IssueComment; import org.kohsuke.github.GHEventPayload.PullRequest; +import javax.crypto.Mac; +import javax.crypto.spec.SecretKeySpec; + import java.io.FileNotFoundException; import java.io.IOException; import java.io.PrintStream; @@ -290,6 +294,37 @@ void onPullRequestHook(PullRequest pr) { GhprbTrigger.getDscp().save(); } + boolean checkSignature(String body, String signature) { + String secret = helper.getTrigger().getSecret(); + if (secret != null && ! secret.isEmpty()) { + if (signature != null && signature.startsWith("sha1=")) { + String expected = signature.substring(5); + String algorithm = "HmacSHA1"; + try { + SecretKeySpec keySpec = new SecretKeySpec(secret.getBytes(), algorithm); + Mac mac = Mac.getInstance(algorithm); + mac.init(keySpec); + byte[] localSignatureBytes = mac.doFinal(body.getBytes("UTF-8")); + String localSignature = new String(Hex.encodeHexString(localSignatureBytes)); + if (! localSignature.equals(expected)) { + logger.log(Level.SEVERE, "Local signature {0} does not match external signature {1}", + new Object[] {localSignature, expected}); + return false; + } + } catch (Exception e) { + logger.log(Level.SEVERE, "Couldn't match both signatures"); + return false; + } + } else { + logger.log(Level.SEVERE, "Request doesn't contain a signature. Check that github has a secret that should be attached to the hook"); + return false; + } + } + + logger.log(Level.INFO, "Signatures checking OK"); + return true; + } + @VisibleForTesting void setHelper(Ghprb helper) { this.helper = helper; diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRootAction.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRootAction.java index e65cdada7..8a527fc11 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRootAction.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRootAction.java @@ -9,18 +9,17 @@ import org.acegisecurity.Authentication; import org.acegisecurity.context.SecurityContextHolder; import org.apache.commons.io.IOUtils; -import org.apache.mina.util.Base64; import org.kohsuke.github.GHEventPayload; import org.kohsuke.github.GHIssueState; import org.kohsuke.github.GHRepository; import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.StaplerResponse; -import javax.crypto.Mac; -import javax.crypto.spec.SecretKeySpec; import java.io.BufferedReader; import java.io.IOException; import java.io.StringReader; +import java.io.UnsupportedEncodingException; +import java.net.URLDecoder; import java.util.HashSet; import java.util.Set; import java.util.logging.Level; @@ -48,57 +47,35 @@ public String getUrlName() { public void doIndex(StaplerRequest req, StaplerResponse resp) { String event = req.getHeader("X-GitHub-Event"); - String signature = req.getHeader("X-GitHub-Signature"); + String signature = req.getHeader("X-Hub-Signature"); String type = req.getContentType(); String payload = null; + String body = null; if ("application/json".equals(type)) { - BufferedReader br = null; - try { - br = req.getReader(); - payload = IOUtils.toString(req.getReader()); - } catch (IOException e) { + body = extractRequestBody(req); + if (body == null) { logger.log(Level.SEVERE, "Can't get request body for application/json."); return; - } finally { - IOUtils.closeQuietly(br); } + payload = body; } else if ("application/x-www-form-urlencoded".equals(type)) { - payload = req.getParameter("payload"); - if (payload == null) { + body = extractRequestBody(req); + if (body == null || body.length() <= 8) { logger.log(Level.SEVERE, "Request doesn't contain payload. You're sending url encoded request, so you should pass github payload through 'payload' request parameter"); return; } + try { + payload = URLDecoder.decode(body.substring(8), req.getCharacterEncoding()); + } catch (UnsupportedEncodingException e) { + logger.log(Level.SEVERE, "Error while trying to encode the payload"); + return; + } } - String secret = GhprbTrigger.getDscp().getSecret(); - if (payload == null) { logger.log(Level.SEVERE, "Payload is null, maybe content type '{0}' is not supported by this plugin. Please use 'application/json' or 'application/x-www-form-urlencoded'", new Object[] {type}); return; - } else if (secret != null && ! secret.isEmpty()) { - if (signature != null) { - SecretKeySpec keySpec = new SecretKeySpec(secret.getBytes(), "HmacSHA1"); - try { - Mac mac = Mac.getInstance("HmacSHA1"); - mac.init(keySpec); - byte[] clientSignatureBytes = mac.doFinal(payload.getBytes()); - String clientSignature = "sha1=" + new String(Base64.encodeBase64(clientSignatureBytes)); - if (! clientSignature.equals(signature)){ - logger.log(Level.SEVERE, "Local signature {0} doesn't match with external signature {1}.", - new Object[] {signature, clientSignature}); - return; - } else { - logger.log(Level.INFO, "Signatures checking OK"); - } - } catch (Exception e) { - logger.log(Level.SEVERE, "Couldn't match both signatures"); - return; - } - } else { - logger.log(Level.SEVERE, "Request doesn't contain a signature. Check that github has a secret that should be attached to the hook"); - return; - } } GhprbGitHub gh = GhprbTrigger.getDscp().getGitHub(); @@ -115,13 +92,15 @@ public void doIndex(StaplerRequest req, StaplerResponse resp) { for (GhprbRepository repo : getRepos(issueComment.getRepository())) { logger.log(Level.INFO, "Checking issue comment ''{0}'' for repo {1}", new Object[] {issueComment.getComment(), repo.getName()}); - repo.onIssueCommentHook(issueComment); + if(repo.checkSignature(body, signature)) + repo.onIssueCommentHook(issueComment); } } else if ("pull_request".equals(event)) { GHEventPayload.PullRequest pr = gh.get().parseEventPayload(new StringReader(payload), GHEventPayload.PullRequest.class); for (GhprbRepository repo : getRepos(pr.getPullRequest().getRepository())) { logger.log(Level.INFO, "Checking PR #{1} for {0}", new Object[] { repo.getName(), pr.getNumber()}); - repo.onPullRequestHook(pr); + if(repo.checkSignature(body, signature)) + repo.onPullRequestHook(pr); } } else { logger.log(Level.WARNING, "Request not known"); @@ -131,6 +110,20 @@ public void doIndex(StaplerRequest req, StaplerResponse resp) { } } + private String extractRequestBody(StaplerRequest req) { + String body = null; + BufferedReader br = null; + try { + br = req.getReader(); + body = IOUtils.toString(br); + } catch (IOException e) { + body = null; + } finally { + IOUtils.closeQuietly(br); + } + return body; + } + private Set getRepos(GHRepository repo) { return getRepos(repo.getFullName()); } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java index cd52fe0d0..0ec2a3eed 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java @@ -391,7 +391,6 @@ public static final class DescriptorImpl extends TriggerDescriptor { private String commitStatusContext = ""; private Boolean autoCloseFailedPullRequests = false; private Boolean displayBuildErrorsOnDownstreamBuilds = false; - private String secret = ""; @@ -445,7 +444,6 @@ public boolean configure(StaplerRequest req, JSONObject formData) throws FormExc msgSuccess = formData.getString("msgSuccess"); msgFailure = formData.getString("msgFailure"); commitStatusContext = formData.getString("commitStatusContext"); - secret = formData.getString("secret"); save(); gh = new GhprbGitHub(); @@ -525,10 +523,6 @@ public int getlogExcerptLines() { return logExcerptLines; } - public String getSecret() { - return secret; - } - public Boolean getAutoCloseFailedPullRequests() { return autoCloseFailedPullRequests; } diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java index 18a157019..11e01d689 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java @@ -1,6 +1,8 @@ package org.jenkinsci.plugins.ghprb; +import org.apache.commons.codec.binary.Hex; import org.joda.time.DateTime; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -19,9 +21,15 @@ import org.mockito.Mockito; import org.mockito.runners.MockitoJUnitRunner; +import javax.crypto.Mac; +import javax.crypto.spec.SecretKeySpec; import java.io.FileNotFoundException; import java.io.IOException; +import java.io.UnsupportedEncodingException; import java.net.URL; +import java.net.URLEncoder; +import java.security.InvalidKeyException; +import java.security.NoSuchAlgorithmException; import java.util.ArrayList; import java.util.Date; import java.util.List; @@ -455,6 +463,32 @@ public void testExceedRateLimit() throws IOException { verifyZeroInteractions(gt); } + @Test + public void testSignature() throws IOException, InvalidKeyException, NoSuchAlgorithmException { + String body = URLEncoder.encode("payload=" + GhprbTestUtil.PAYLOAD, "UTF-8"); + String actualSecret = "123"; + String actualSignature = createSHA1Signature(actualSecret, body); + String fakeSignature = createSHA1Signature("abc", body); + + given(helper.getTrigger()).willReturn(trigger); + given(trigger.getSecret()).willReturn(actualSecret); + + Assert.assertFalse(actualSignature.equals(fakeSignature)); + Assert.assertTrue(ghprbRepository.checkSignature(body, actualSignature)); + Assert.assertFalse(ghprbRepository.checkSignature(body, fakeSignature)); + } + + private String createSHA1Signature(String secret, String body) throws UnsupportedEncodingException, NoSuchAlgorithmException, InvalidKeyException { + String algorithm = "HmacSHA1"; + SecretKeySpec keySpec = new SecretKeySpec(secret.getBytes(), algorithm); + Mac mac = Mac.getInstance(algorithm); + mac.init(keySpec); + + byte[] signatureBytes = mac.doFinal(body.getBytes("UTF-8")); + String signature = new String(Hex.encodeHexString(signatureBytes)); + return "sha1=" + signature; + } + private void initGHPRWithTestData() throws IOException { /** Mock PR data */ given(ghPullRequest.getUser()).willReturn(ghUser); diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRootActionTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRootActionTest.java index 79e8e67fa..1f094602c 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRootActionTest.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRootActionTest.java @@ -6,6 +6,9 @@ import java.io.ByteArrayInputStream; import java.io.InputStream; import java.io.InputStreamReader; +import java.io.StringReader; +import java.io.IOException; +import java.net.URLEncoder; import org.junit.Before; import org.junit.Rule; @@ -25,191 +28,6 @@ public class GhprbRootActionTest { - private final String payload = "{" + - " \"action\": \"created\"," + - " \"issue\": {" + - " \"url\": \"https://api.github.com/repos/user/dropwizard/issues/1\"," + - " \"labels_url\": \"https://api.github.com/repos/user/dropwizard/issues/1/labels{/name}\"," + - " \"comments_url\": \"https://api.github.com/repos/user/dropwizard/issues/1/comments\"," + - " \"events_url\": \"https://api.github.com/repos/user/dropwizard/issues/1/events\"," + - " \"html_url\": \"https://github.com/user/dropwizard/pull/1\"," + - " \"id\": 44444444," + - " \"number\": 12," + - " \"title\": \"Adding version command\"," + - " \"user\": {" + - " \"login\": \"user\"," + - " \"id\": 444444," + - " \"avatar_url\": \"https://avatars.githubusercontent.com/u/444444?v=3\"," + - " \"gravatar_id\": \"\"," + - " \"url\": \"https://api.github.com/users/user\"," + - " \"html_url\": \"https://github.com/user\"," + - " \"followers_url\": \"https://api.github.com/users/user/followers\"," + - " \"following_url\": \"https://api.github.com/users/user/following{/other_user}\"," + - " \"gists_url\": \"https://api.github.com/users/user/gists{/gist_id}\"," + - " \"starred_url\": \"https://api.github.com/users/user/starred{/owner}{/repo}\"," + - " \"subscriptions_url\": \"https://api.github.com/users/user/subscriptions\"," + - " \"organizations_url\": \"https://api.github.com/users/user/orgs\"," + - " \"repos_url\": \"https://api.github.com/users/user/repos\"," + - " \"events_url\": \"https://api.github.com/users/user/events{/privacy}\"," + - " \"received_events_url\": \"https://api.github.com/users/user/received_events\"," + - " \"type\": \"User\"," + - " \"site_admin\": false" + - " }," + - " \"labels\": [" + - "" + - " ]," + - " \"state\": \"open\"," + - " \"locked\": false," + - " \"assignee\": null," + - " \"milestone\": null," + - " \"comments\": 2," + - " \"created_at\": \"2014-09-22T20:05:14Z\"," + - " \"updated_at\": \"2015-01-14T14:50:53Z\"," + - " \"closed_at\": null," + - " \"pull_request\": {" + - " \"url\": \"https://api.github.com/repos/user/dropwizard/pulls/1\"," + - " \"html_url\": \"https://github.com/user/dropwizard/pull/1\"," + - " \"diff_url\": \"https://github.com/user/dropwizard/pull/1.diff\"," + - " \"patch_url\": \"https://github.com/user/dropwizard/pull/1.patch\"" + - " }," + - " \"body\": \"\"" + - " }," + - " \"comment\": {" + - " \"url\": \"https://api.github.com/repos/user/dropwizard/issues/comments/44444444\"," + - " \"html_url\": \"https://github.com/user/dropwizard/pull/1#issuecomment-44444444\"," + - " \"issue_url\": \"https://api.github.com/repos/user/dropwizard/issues/1\"," + - " \"id\": 44444444," + - " \"user\": {" + - " \"login\": \"user\"," + - " \"id\": 444444," + - " \"avatar_url\": \"https://avatars.githubusercontent.com/u/444444?v=3\"," + - " \"gravatar_id\": \"\"," + - " \"url\": \"https://api.github.com/users/user\"," + - " \"html_url\": \"https://github.com/user\"," + - " \"followers_url\": \"https://api.github.com/users/user/followers\"," + - " \"following_url\": \"https://api.github.com/users/user/following{/other_user}\"," + - " \"gists_url\": \"https://api.github.com/users/user/gists{/gist_id}\"," + - " \"starred_url\": \"https://api.github.com/users/user/starred{/owner}{/repo}\"," + - " \"subscriptions_url\": \"https://api.github.com/users/user/subscriptions\"," + - " \"organizations_url\": \"https://api.github.com/users/user/orgs\"," + - " \"repos_url\": \"https://api.github.com/users/user/repos\"," + - " \"events_url\": \"https://api.github.com/users/user/events{/privacy}\"," + - " \"received_events_url\": \"https://api.github.com/users/user/received_events\"," + - " \"type\": \"User\"," + - " \"site_admin\": false" + - " }," + - " \"created_at\": \"2015-01-14T14:50:53Z\"," + - " \"updated_at\": \"2015-01-14T14:50:53Z\"," + - " \"body\": \"retest this please\"" + - " }," + - " \"repository\": {" + - " \"id\": 44444444," + - " \"name\": \"Testing\"," + - " \"full_name\": \"user/dropwizard\"," + - " \"owner\": {" + - " \"login\": \"user\"," + - " \"id\": 444444," + - " \"avatar_url\": \"https://avatars.githubusercontent.com/u/444444?v=3\"," + - " \"gravatar_id\": \"\"," + - " \"url\": \"https://api.github.com/users/user\"," + - " \"html_url\": \"https://github.com/user\"," + - " \"followers_url\": \"https://api.github.com/users/user/followers\"," + - " \"following_url\": \"https://api.github.com/users/user/following{/other_user}\"," + - " \"gists_url\": \"https://api.github.com/users/user/gists{/gist_id}\"," + - " \"starred_url\": \"https://api.github.com/users/user/starred{/owner}{/repo}\"," + - " \"subscriptions_url\": \"https://api.github.com/users/user/subscriptions\"," + - " \"organizations_url\": \"https://api.github.com/users/user/orgs\"," + - " \"repos_url\": \"https://api.github.com/users/user/repos\"," + - " \"events_url\": \"https://api.github.com/users/user/events{/privacy}\"," + - " \"received_events_url\": \"https://api.github.com/users/user/received_events\"," + - " \"type\": \"User\"," + - " \"site_admin\": false" + - " }," + - " \"private\": false," + - " \"html_url\": \"https://github.com/user/dropwizard\"," + - " \"description\": \"\"," + - " \"fork\": false," + - " \"url\": \"https://api.github.com/repos/user/dropwizard\"," + - " \"forks_url\": \"https://api.github.com/repos/user/dropwizard/forks\"," + - " \"keys_url\": \"https://api.github.com/repos/user/dropwizard/keys{/key_id}\"," + - " \"collaborators_url\": \"https://api.github.com/repos/user/dropwizard/collaborators{/collaborator}\"," + - " \"teams_url\": \"https://api.github.com/repos/user/dropwizard/teams\"," + - " \"hooks_url\": \"https://api.github.com/repos/user/dropwizard/hooks\"," + - " \"issue_events_url\": \"https://api.github.com/repos/user/dropwizard/issues/events{/number}\"," + - " \"events_url\": \"https://api.github.com/repos/user/dropwizard/events\"," + - " \"assignees_url\": \"https://api.github.com/repos/user/dropwizard/assignees{/user}\"," + - " \"branches_url\": \"https://api.github.com/repos/user/dropwizard/branches{/branch}\"," + - " \"tags_url\": \"https://api.github.com/repos/user/dropwizard/tags\"," + - " \"blobs_url\": \"https://api.github.com/repos/user/dropwizard/git/blobs{/sha}\"," + - " \"git_tags_url\": \"https://api.github.com/repos/user/dropwizard/git/tags{/sha}\"," + - " \"git_refs_url\": \"https://api.github.com/repos/user/dropwizard/git/refs{/sha}\"," + - " \"trees_url\": \"https://api.github.com/repos/user/dropwizard/git/trees{/sha}\"," + - " \"statuses_url\": \"https://api.github.com/repos/user/dropwizard/statuses/{sha}\"," + - " \"languages_url\": \"https://api.github.com/repos/user/dropwizard/languages\"," + - " \"stargazers_url\": \"https://api.github.com/repos/user/dropwizard/stargazers\"," + - " \"contributors_url\": \"https://api.github.com/repos/user/dropwizard/contributors\"," + - " \"subscribers_url\": \"https://api.github.com/repos/user/dropwizard/subscribers\"," + - " \"subscription_url\": \"https://api.github.com/repos/user/dropwizard/subscription\"," + - " \"commits_url\": \"https://api.github.com/repos/user/dropwizard/commits{/sha}\"," + - " \"git_commits_url\": \"https://api.github.com/repos/user/dropwizard/git/commits{/sha}\"," + - " \"comments_url\": \"https://api.github.com/repos/user/dropwizard/comments{/number}\"," + - " \"issue_comment_url\": \"https://api.github.com/repos/user/dropwizard/issues/comments/{number}\"," + - " \"contents_url\": \"https://api.github.com/repos/user/dropwizard/contents/{+path}\"," + - " \"compare_url\": \"https://api.github.com/repos/user/dropwizard/compare/{base}...{head}\"," + - " \"merges_url\": \"https://api.github.com/repos/user/dropwizard/merges\"," + - " \"archive_url\": \"https://api.github.com/repos/user/dropwizard/{archive_format}{/ref}\"," + - " \"downloads_url\": \"https://api.github.com/repos/user/dropwizard/downloads\"," + - " \"issues_url\": \"https://api.github.com/repos/user/dropwizard/issues{/number}\"," + - " \"pulls_url\": \"https://api.github.com/repos/user/dropwizard/pulls{/number}\"," + - " \"milestones_url\": \"https://api.github.com/repos/user/dropwizard/milestones{/number}\"," + - " \"notifications_url\": \"https://api.github.com/repos/user/dropwizard/notifications{?since,all,participating}\"," + - " \"labels_url\": \"https://api.github.com/repos/user/dropwizard/labels{/name}\"," + - " \"releases_url\": \"https://api.github.com/repos/user/dropwizard/releases{/id}\"," + - " \"created_at\": \"2014-07-23T15:52:14Z\"," + - " \"updated_at\": \"2014-09-04T21:10:34Z\"," + - " \"pushed_at\": \"2015-01-14T14:13:58Z\"," + - " \"git_url\": \"git://github.com/user/dropwizard.git\"," + - " \"ssh_url\": \"git@github.com:user/dropwizard.git\"," + - " \"clone_url\": \"https://github.com/user/dropwizard.git\"," + - " \"svn_url\": \"https://github.com/user/dropwizard\"," + - " \"homepage\": null," + - " \"size\": 20028," + - " \"stargazers_count\": 0," + - " \"watchers_count\": 0," + - " \"language\": \"JavaScript\"," + - " \"has_issues\": true," + - " \"has_downloads\": true," + - " \"has_wiki\": true," + - " \"has_pages\": false," + - " \"forks_count\": 0," + - " \"mirror_url\": null," + - " \"open_issues_count\": 1," + - " \"forks\": 0," + - " \"open_issues\": 1," + - " \"watchers\": 0," + - " \"default_branch\": \"master\"" + - " }," + - " \"sender\": {" + - " \"login\": \"user\"," + - " \"id\": 444444," + - " \"avatar_url\": \"https://avatars.githubusercontent.com/u/444444?v=3\"," + - " \"gravatar_id\": \"\"," + - " \"url\": \"https://api.github.com/users/user\"," + - " \"html_url\": \"https://github.com/user\"," + - " \"followers_url\": \"https://api.github.com/users/user/followers\"," + - " \"following_url\": \"https://api.github.com/users/user/following{/other_user}\"," + - " \"gists_url\": \"https://api.github.com/users/user/gists{/gist_id}\"," + - " \"starred_url\": \"https://api.github.com/users/user/starred{/owner}{/repo}\"," + - " \"subscriptions_url\": \"https://api.github.com/users/user/subscriptions\"," + - " \"organizations_url\": \"https://api.github.com/users/user/orgs\"," + - " \"repos_url\": \"https://api.github.com/users/user/repos\"," + - " \"events_url\": \"https://api.github.com/users/user/events{/privacy}\"," + - " \"received_events_url\": \"https://api.github.com/users/user/received_events\"," + - " \"type\": \"User\"," + - " \"site_admin\": false" + - " }" + - "}"; - @Rule public JenkinsRule jenkinsRule = new JenkinsRule(); @@ -224,11 +42,14 @@ public void setup() throws Exception { } @Test - public void testUrlEncoded() { + public void testUrlEncoded() throws IOException { + BufferedReader br = new BufferedReader(new StringReader( + "payload=" + URLEncoder.encode(GhprbTestUtil.PAYLOAD, "UTF-8"))); + given(req.getContentType()).willReturn("application/x-www-form-urlencoded"); - given(req.getParameter("payload")).willReturn(payload); + given(req.getReader()).willReturn(br); + given(req.getCharacterEncoding()).willReturn("UTF-8"); given(req.getHeader("X-GitHub-Event")).willReturn("issue_comment"); - given(req.getHeader("X-GitHub-Signature")).willReturn(GhprbTestUtil.createSignature(payload)); GhprbRootAction ra = new GhprbRootAction(); ra.doIndex(req, null); @@ -240,7 +61,7 @@ public void testJson() throws Exception { given(req.getHeader("X-GitHub-Event")).willReturn("issue_comment"); // convert String into InputStream - InputStream is = new ByteArrayInputStream(payload.getBytes()); + InputStream is = new ByteArrayInputStream(GhprbTestUtil.PAYLOAD.getBytes()); // read it with BufferedReader br = spy(new BufferedReader(new InputStreamReader(is))); diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTestUtil.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTestUtil.java index 360353a71..0b7359d30 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTestUtil.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTestUtil.java @@ -20,7 +20,6 @@ import java.net.MalformedURLException; import java.net.URL; -import org.apache.mina.util.Base64; import org.joda.time.DateTime; import org.kohsuke.github.GHCommitPointer; import org.kohsuke.github.GHPullRequest; @@ -36,21 +35,201 @@ import hudson.plugins.git.UserRemoteConfig; import net.sf.json.JSONObject; -import javax.crypto.Mac; -import javax.crypto.spec.SecretKeySpec; - public class GhprbTestUtil { public static final int INITIAL_RATE_LIMIT = 5000; public static final String GHPRB_PLUGIN_NAME = "ghprb"; - public static final String GITHUB_SECRET = "61c6f58144702262d65d1015445e5f2635ccb6a6"; - + public static final String PAYLOAD = "{" + + " \"action\": \"created\"," + + " \"issue\": {" + + " \"url\": \"https://api.github.com/repos/user/dropwizard/issues/1\"," + + " \"labels_url\": \"https://api.github.com/repos/user/dropwizard/issues/1/labels{/name}\"," + + " \"comments_url\": \"https://api.github.com/repos/user/dropwizard/issues/1/comments\"," + + " \"events_url\": \"https://api.github.com/repos/user/dropwizard/issues/1/events\"," + + " \"html_url\": \"https://github.com/user/dropwizard/pull/1\"," + + " \"id\": 44444444," + + " \"number\": 12," + + " \"title\": \"Adding version command\"," + + " \"user\": {" + + " \"login\": \"user\"," + + " \"id\": 444444," + + " \"avatar_url\": \"https://avatars.githubusercontent.com/u/444444?v=3\"," + + " \"gravatar_id\": \"\"," + + " \"url\": \"https://api.github.com/users/user\"," + + " \"html_url\": \"https://github.com/user\"," + + " \"followers_url\": \"https://api.github.com/users/user/followers\"," + + " \"following_url\": \"https://api.github.com/users/user/following{/other_user}\"," + + " \"gists_url\": \"https://api.github.com/users/user/gists{/gist_id}\"," + + " \"starred_url\": \"https://api.github.com/users/user/starred{/owner}{/repo}\"," + + " \"subscriptions_url\": \"https://api.github.com/users/user/subscriptions\"," + + " \"organizations_url\": \"https://api.github.com/users/user/orgs\"," + + " \"repos_url\": \"https://api.github.com/users/user/repos\"," + + " \"events_url\": \"https://api.github.com/users/user/events{/privacy}\"," + + " \"received_events_url\": \"https://api.github.com/users/user/received_events\"," + + " \"type\": \"User\"," + + " \"site_admin\": false" + + " }," + + " \"labels\": [" + + "" + + " ]," + + " \"state\": \"open\"," + + " \"locked\": false," + + " \"assignee\": null," + + " \"milestone\": null," + + " \"comments\": 2," + + " \"created_at\": \"2014-09-22T20:05:14Z\"," + + " \"updated_at\": \"2015-01-14T14:50:53Z\"," + + " \"closed_at\": null," + + " \"pull_request\": {" + + " \"url\": \"https://api.github.com/repos/user/dropwizard/pulls/1\"," + + " \"html_url\": \"https://github.com/user/dropwizard/pull/1\"," + + " \"diff_url\": \"https://github.com/user/dropwizard/pull/1.diff\"," + + " \"patch_url\": \"https://github.com/user/dropwizard/pull/1.patch\"" + + " }," + + " \"body\": \"\"" + + " }," + + " \"comment\": {" + + " \"url\": \"https://api.github.com/repos/user/dropwizard/issues/comments/44444444\"," + + " \"html_url\": \"https://github.com/user/dropwizard/pull/1#issuecomment-44444444\"," + + " \"issue_url\": \"https://api.github.com/repos/user/dropwizard/issues/1\"," + + " \"id\": 44444444," + + " \"user\": {" + + " \"login\": \"user\"," + + " \"id\": 444444," + + " \"avatar_url\": \"https://avatars.githubusercontent.com/u/444444?v=3\"," + + " \"gravatar_id\": \"\"," + + " \"url\": \"https://api.github.com/users/user\"," + + " \"html_url\": \"https://github.com/user\"," + + " \"followers_url\": \"https://api.github.com/users/user/followers\"," + + " \"following_url\": \"https://api.github.com/users/user/following{/other_user}\"," + + " \"gists_url\": \"https://api.github.com/users/user/gists{/gist_id}\"," + + " \"starred_url\": \"https://api.github.com/users/user/starred{/owner}{/repo}\"," + + " \"subscriptions_url\": \"https://api.github.com/users/user/subscriptions\"," + + " \"organizations_url\": \"https://api.github.com/users/user/orgs\"," + + " \"repos_url\": \"https://api.github.com/users/user/repos\"," + + " \"events_url\": \"https://api.github.com/users/user/events{/privacy}\"," + + " \"received_events_url\": \"https://api.github.com/users/user/received_events\"," + + " \"type\": \"User\"," + + " \"site_admin\": false" + + " }," + + " \"created_at\": \"2015-01-14T14:50:53Z\"," + + " \"updated_at\": \"2015-01-14T14:50:53Z\"," + + " \"body\": \"retest this please\"" + + " }," + + " \"repository\": {" + + " \"id\": 44444444," + + " \"name\": \"Testing\"," + + " \"full_name\": \"user/dropwizard\"," + + " \"owner\": {" + + " \"login\": \"user\"," + + " \"id\": 444444," + + " \"avatar_url\": \"https://avatars.githubusercontent.com/u/444444?v=3\"," + + " \"gravatar_id\": \"\"," + + " \"url\": \"https://api.github.com/users/user\"," + + " \"html_url\": \"https://github.com/user\"," + + " \"followers_url\": \"https://api.github.com/users/user/followers\"," + + " \"following_url\": \"https://api.github.com/users/user/following{/other_user}\"," + + " \"gists_url\": \"https://api.github.com/users/user/gists{/gist_id}\"," + + " \"starred_url\": \"https://api.github.com/users/user/starred{/owner}{/repo}\"," + + " \"subscriptions_url\": \"https://api.github.com/users/user/subscriptions\"," + + " \"organizations_url\": \"https://api.github.com/users/user/orgs\"," + + " \"repos_url\": \"https://api.github.com/users/user/repos\"," + + " \"events_url\": \"https://api.github.com/users/user/events{/privacy}\"," + + " \"received_events_url\": \"https://api.github.com/users/user/received_events\"," + + " \"type\": \"User\"," + + " \"site_admin\": false" + + " }," + + " \"private\": false," + + " \"html_url\": \"https://github.com/user/dropwizard\"," + + " \"description\": \"\"," + + " \"fork\": false," + + " \"url\": \"https://api.github.com/repos/user/dropwizard\"," + + " \"forks_url\": \"https://api.github.com/repos/user/dropwizard/forks\"," + + " \"keys_url\": \"https://api.github.com/repos/user/dropwizard/keys{/key_id}\"," + + " \"collaborators_url\": \"https://api.github.com/repos/user/dropwizard/collaborators{/collaborator}\"," + + " \"teams_url\": \"https://api.github.com/repos/user/dropwizard/teams\"," + + " \"hooks_url\": \"https://api.github.com/repos/user/dropwizard/hooks\"," + + " \"issue_events_url\": \"https://api.github.com/repos/user/dropwizard/issues/events{/number}\"," + + " \"events_url\": \"https://api.github.com/repos/user/dropwizard/events\"," + + " \"assignees_url\": \"https://api.github.com/repos/user/dropwizard/assignees{/user}\"," + + " \"branches_url\": \"https://api.github.com/repos/user/dropwizard/branches{/branch}\"," + + " \"tags_url\": \"https://api.github.com/repos/user/dropwizard/tags\"," + + " \"blobs_url\": \"https://api.github.com/repos/user/dropwizard/git/blobs{/sha}\"," + + " \"git_tags_url\": \"https://api.github.com/repos/user/dropwizard/git/tags{/sha}\"," + + " \"git_refs_url\": \"https://api.github.com/repos/user/dropwizard/git/refs{/sha}\"," + + " \"trees_url\": \"https://api.github.com/repos/user/dropwizard/git/trees{/sha}\"," + + " \"statuses_url\": \"https://api.github.com/repos/user/dropwizard/statuses/{sha}\"," + + " \"languages_url\": \"https://api.github.com/repos/user/dropwizard/languages\"," + + " \"stargazers_url\": \"https://api.github.com/repos/user/dropwizard/stargazers\"," + + " \"contributors_url\": \"https://api.github.com/repos/user/dropwizard/contributors\"," + + " \"subscribers_url\": \"https://api.github.com/repos/user/dropwizard/subscribers\"," + + " \"subscription_url\": \"https://api.github.com/repos/user/dropwizard/subscription\"," + + " \"commits_url\": \"https://api.github.com/repos/user/dropwizard/commits{/sha}\"," + + " \"git_commits_url\": \"https://api.github.com/repos/user/dropwizard/git/commits{/sha}\"," + + " \"comments_url\": \"https://api.github.com/repos/user/dropwizard/comments{/number}\"," + + " \"issue_comment_url\": \"https://api.github.com/repos/user/dropwizard/issues/comments/{number}\"," + + " \"contents_url\": \"https://api.github.com/repos/user/dropwizard/contents/{+path}\"," + + " \"compare_url\": \"https://api.github.com/repos/user/dropwizard/compare/{base}...{head}\"," + + " \"merges_url\": \"https://api.github.com/repos/user/dropwizard/merges\"," + + " \"archive_url\": \"https://api.github.com/repos/user/dropwizard/{archive_format}{/ref}\"," + + " \"downloads_url\": \"https://api.github.com/repos/user/dropwizard/downloads\"," + + " \"issues_url\": \"https://api.github.com/repos/user/dropwizard/issues{/number}\"," + + " \"pulls_url\": \"https://api.github.com/repos/user/dropwizard/pulls{/number}\"," + + " \"milestones_url\": \"https://api.github.com/repos/user/dropwizard/milestones{/number}\"," + + " \"notifications_url\": \"https://api.github.com/repos/user/dropwizard/notifications{?since,all,participating}\"," + + " \"labels_url\": \"https://api.github.com/repos/user/dropwizard/labels{/name}\"," + + " \"releases_url\": \"https://api.github.com/repos/user/dropwizard/releases{/id}\"," + + " \"created_at\": \"2014-07-23T15:52:14Z\"," + + " \"updated_at\": \"2014-09-04T21:10:34Z\"," + + " \"pushed_at\": \"2015-01-14T14:13:58Z\"," + + " \"git_url\": \"git://github.com/user/dropwizard.git\"," + + " \"ssh_url\": \"git@github.com:user/dropwizard.git\"," + + " \"clone_url\": \"https://github.com/user/dropwizard.git\"," + + " \"svn_url\": \"https://github.com/user/dropwizard\"," + + " \"homepage\": null," + + " \"size\": 20028," + + " \"stargazers_count\": 0," + + " \"watchers_count\": 0," + + " \"language\": \"JavaScript\"," + + " \"has_issues\": true," + + " \"has_downloads\": true," + + " \"has_wiki\": true," + + " \"has_pages\": false," + + " \"forks_count\": 0," + + " \"mirror_url\": null," + + " \"open_issues_count\": 1," + + " \"forks\": 0," + + " \"open_issues\": 1," + + " \"watchers\": 0," + + " \"default_branch\": \"master\"" + + " }," + + " \"sender\": {" + + " \"login\": \"user\"," + + " \"id\": 444444," + + " \"avatar_url\": \"https://avatars.githubusercontent.com/u/444444?v=3\"," + + " \"gravatar_id\": \"\"," + + " \"url\": \"https://api.github.com/users/user\"," + + " \"html_url\": \"https://github.com/user\"," + + " \"followers_url\": \"https://api.github.com/users/user/followers\"," + + " \"following_url\": \"https://api.github.com/users/user/following{/other_user}\"," + + " \"gists_url\": \"https://api.github.com/users/user/gists{/gist_id}\"," + + " \"starred_url\": \"https://api.github.com/users/user/starred{/owner}{/repo}\"," + + " \"subscriptions_url\": \"https://api.github.com/users/user/subscriptions\"," + + " \"organizations_url\": \"https://api.github.com/users/user/orgs\"," + + " \"repos_url\": \"https://api.github.com/users/user/repos\"," + + " \"events_url\": \"https://api.github.com/users/user/events{/privacy}\"," + + " \"received_events_url\": \"https://api.github.com/users/user/received_events\"," + + " \"type\": \"User\"," + + " \"site_admin\": false" + + " }" + + "}"; + // TODO: When anyone has time to investigate mocking the github request. // public static void mockGithubUserPage() { // new MockServerClient("https://api.github.com", 80) // .when(new HttpRequest().withMethod("GET").withPath("/user")) // .respond(new HttpResponse().withStatusCode(200)); -// +// // } public static void mockCommitList(GHPullRequest ghPullRequest) { @@ -110,11 +289,11 @@ public static JSONObject provideConfiguration() { jsonObject.put("msgSuccess", "Success"); jsonObject.put("msgFailure", "Failure"); jsonObject.put("commitStatusContext", "Status Context"); - jsonObject.put("secret", GITHUB_SECRET); + jsonObject.put("secret", "111"); return jsonObject; } - + public static GitSCM provideGitSCM() { return new GitSCM( newArrayList( @@ -134,17 +313,4 @@ public static GitSCM provideGitSCM() { private GhprbTestUtil() { } - public static String createSignature(String payload) { - SecretKeySpec keySpec = new SecretKeySpec(GITHUB_SECRET.getBytes(), "HmacSHA1"); - try { - Mac mac = Mac.getInstance("HmacSHA1"); - mac.init(keySpec); - byte[] signatureBytes = mac.doFinal(payload.getBytes()); - String signature = new String(Base64.encodeBase64(signatureBytes)); - return "sha1=" + signature; - } catch (Exception e){ - return ""; - } - } - -} \ No newline at end of file +} From e36d52cbd33adb2b5a78718804a5d14b08b94bbd Mon Sep 17 00:00:00 2001 From: Jose Molina Date: Wed, 1 Apr 2015 17:42:14 +0200 Subject: [PATCH 03/14] FIX: added UTF-8 encoding to project reporting in pom.xml --- pom.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/pom.xml b/pom.xml index e99ac5963..d4bdb3d92 100644 --- a/pom.xml +++ b/pom.xml @@ -46,6 +46,7 @@ UTF-8 + UTF-8 From cc899f3b696452adeb1c93d7329ca7ac1ce3f5c3 Mon Sep 17 00:00:00 2001 From: Jose Molina Date: Wed, 20 May 2015 09:31:02 +0200 Subject: [PATCH 04/14] FIX: pulled latests changes from master --- README.md | 25 +- pom.xml | 15 +- .../org/jenkinsci/plugins/ghprb/Ghprb.java | 11 +- .../jenkinsci/plugins/ghprb/GhprbBranch.java | 6 +- .../jenkinsci/plugins/ghprb/GhprbBuilds.java | 210 ++++---- .../jenkinsci/plugins/ghprb/GhprbCause.java | 160 ++++--- .../jenkinsci/plugins/ghprb/GhprbGitHub.java | 112 ++--- .../plugins/ghprb/GhprbPullRequest.java | 172 ++++--- .../plugins/ghprb/GhprbPullRequestMerge.java | 394 ++++++++------- .../plugins/ghprb/GhprbRepository.java | 60 ++- .../plugins/ghprb/GhprbRootAction.java | 47 +- .../plugins/ghprb/GhprbTokenMacro.java | 10 +- .../jenkinsci/plugins/ghprb/GhprbTrigger.java | 140 +++--- .../ghprb/HttpConnectorWithJenkinsProxy.java | 4 +- .../ghprb/manager/GhprbBuildManager.java | 36 +- .../configuration/JobConfiguration.java | 52 +- .../factory/GhprbBuildManagerFactoryUtil.java | 54 +-- .../manager/impl/GhprbBaseBuildManager.java | 312 ++++++------ .../impl/GhprbDefaultBuildManager.java | 6 +- .../BuildFlowBuildManager.java | 166 ++++--- .../plugins/ghprb/GhprbTrigger/global.jelly | 2 +- .../org/jenkinsci/plugins/ghprb/GhprbIT.java | 80 +++- .../plugins/ghprb/GhprbITBaseTestCase.java | 111 ++--- .../ghprb/GhprbPullRequestMergeTest.java | 448 +++++++++--------- .../plugins/ghprb/GhprbPullRequestTest.java | 2 - .../plugins/ghprb/GhprbRepositoryTest.java | 84 ++-- .../plugins/ghprb/GhprbRootActionTest.java | 147 +++++- .../plugins/ghprb/GhprbTestUtil.java | 224 +++++---- .../plugins/ghprb/GhprbTriggerTest.java | 74 ++- .../configuration/JobConfigurationTest.java | 32 +- .../GhprbBuildManagerFactoryUtilTest.java | 52 +- .../impl/GhprbDefaultBuildManagerTest.java | 91 ++-- .../BuildFlowBuildManagerTest.java | 139 +++--- .../ghprb/rules/JenkinsRuleWithBuildFlow.java | 13 +- 34 files changed, 1892 insertions(+), 1599 deletions(-) diff --git a/README.md b/README.md index 6a48fa828..c38a7e4ff 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,7 @@ A new build can also be started with a comment: ``retest this please``. You can extend the standard build comment message on github creating a comment file from shell console or any other jenkins plugin. Contents of that file will be added to the comment on GitHub. -This is usefull for posting some build dependent urls for users without +This is useful for posting some build dependent urls for users without access to the jenkins UI console. Jobs can be configured to only build if a matching comment is added to a pull request. For instance, if you have two job you want to run against a pull request, @@ -68,6 +68,7 @@ For more details, see https://wiki.jenkins-ci.org/display/JENKINS/GitHub+pull+re * If you **just** want to build PRs, set ``refspec`` to ``+refs/pull/*:refs/remotes/origin/pr/*`` * If you want to build PRs **and** branches, set ``refspec`` to ``+refs/heads/*:refs/remotes/origin/* +refs/pull/*:refs/remotes/origin/pr/*`` (see note below about [parameterized builds](#parameterized-builds)) * In ``Branch Specifier``, enter ``${sha1}`` instead of the default ``*/master``. +* If you want to use the actual commit in the pull request, use ``${ghprbActualCommit}`` instead of ``${sha1}`` * Under ``Build Triggers``, check ``GitHub pull requests builder``. * Add admins for this specific job. * If you want to use GitHub hooks for automatic testing, read the help for ``Use github hooks for build triggering`` in job configuration. Then you can check the checkbox. @@ -85,6 +86,28 @@ If you want to manually build the job, in the job setting check ``This build is ### Updates +#### -> 1.20.1 +* Null Pointer fix for trigger. +* Added clarity to error message when access is forbidden. + +#### -> 1.20 +* PullRequestMerger now notifies the taskListener of failures. +* AutoCloseFailedPullRequest has been extracted from the published URL check. + +#### -> 1.19 +* More work for disabled builds. +* Unified tabs to spaces. +* Updates to the tests, and added some tests. + +#### -> 1.18 +* Add support for folder projects. +* Correcting issue with default credentials. +* Correcting issue with ghRepository being null when it shouldn't be. +* Ignoring case when matching repo to url. +* Changing the wording for pull requests that are mergeable. +* Change requestForTesting phrase to a textArea. +* Check if project is disabled, if it is then don't do anything. + #### -> 1.14 * A comment file can be created during the build and added to any comment made to the pull request. podarok#33 * Added a ``[skip ci]`` setting, that can be changed. Adding the skip statement to the pull request body will cause the job not to run. sathiya-mit#29 diff --git a/pom.xml b/pom.xml index d4bdb3d92..5232423c8 100644 --- a/pom.xml +++ b/pom.xml @@ -10,7 +10,7 @@ ghprb GitHub Pull Request Builder - 1.16-9-SNAPSHOT + 1.21-SNAPSHOT hpi https://wiki.jenkins-ci.org/display/JENKINS/GitHub+pull+request+builder+plugin @@ -25,11 +25,6 @@ valdisrigdon Valdis Rigdon - - DavidTanner - David Tanner - david.joel.tanner@gmail.com - @@ -58,7 +53,7 @@ org.jenkins-ci.plugins github-api - 1.59 + 1.66 org.jenkins-ci.plugins @@ -100,12 +95,6 @@ 0.12 true - - org.mock-server - mockserver-netty - 3.9.1 - test - diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/Ghprb.java b/src/main/java/org/jenkinsci/plugins/ghprb/Ghprb.java index 05b88c3a9..e70f873ca 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/Ghprb.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/Ghprb.java @@ -16,7 +16,6 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; - /** * @author janinko */ @@ -80,6 +79,10 @@ public void addWhitelist(String author) { whitelisted.add(author); trigger.addWhitelist(author); } + + public boolean isProjectDisabled() { + return project.isDisabled(); + } public GhprbBuilds getBuilds() { return builds; @@ -136,7 +139,7 @@ public boolean isWhitelisted(GHUser user) { public boolean isAdmin(GHUser user) { return admins.contains(user.getLogin()) || (trigger.getAllowMembersOfWhitelistedOrgsAsAdmin() - && isInWhitelistedOrganisation(user)); + && isInWhitelistedOrganisation(user)); } public boolean isBotUser(GHUser user) { @@ -155,9 +158,9 @@ private boolean isInWhitelistedOrganisation(GHUser user) { List getWhiteListTargetBranches() { return trigger.getWhiteListTargetBranches(); } - + public static String replaceMacros(AbstractBuild build, String inputString) { - String returnString = inputString; + String returnString = inputString; if (build != null && inputString != null) { try { Map messageEnvVars = new HashMap(); diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbBranch.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbBranch.java index 2cdf00e10..714bf824d 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbBranch.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbBranch.java @@ -15,8 +15,8 @@ public class GhprbBranch extends AbstractDescribableImpl { public String getBranch() { return branch; } - - public boolean matches(String s){ + + public boolean matches(String s) { return s.matches(branch); } @@ -24,7 +24,7 @@ public boolean matches(String s){ public GhprbBranch(String branch) { this.branch = branch.trim(); } - + @Extension public static class DescriptorImpl extends Descriptor { @Override diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbBuilds.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbBuilds.java index 5cac8cd6f..7057ba84a 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbBuilds.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbBuilds.java @@ -44,16 +44,26 @@ public String build(GhprbPullRequest pr, GHUser triggerSender, String commentBod sb.append("Previous build stopped."); } + sb.append(" Build triggered."); + if (pr.isMergeable()) { - sb.append(" Merge build triggered."); + sb.append(" sha1 is merged."); } else { - sb.append(" Build triggered."); + sb.append(" sha1 is original commit."); } - GhprbCause cause = new GhprbCause(pr.getHead(), pr.getId(), - pr.isMergeable(), pr.getTarget(), pr.getSource(), - pr.getAuthorEmail(), pr.getTitle(), pr.getUrl(), - triggerSender, commentBody, pr.getCommitAuthor()); + GhprbCause cause = new GhprbCause( + pr.getHead(), + pr.getId(), + pr.isMergeable(), + pr.getTarget(), + pr.getSource(), + pr.getAuthorEmail(), + pr.getTitle(), + pr.getUrl(), + triggerSender, + commentBody, + pr.getCommitAuthor()); QueueTaskFuture build = trigger.startJob(cause, repo); if (build == null) { @@ -66,28 +76,33 @@ private boolean cancelBuild(int id) { return false; } - private GhprbCause getCause(AbstractBuild build) { + private GhprbCause getCause(AbstractBuild build) { Cause cause = build.getCause(GhprbCause.class); - if (cause == null || (!(cause instanceof GhprbCause))) return null; + if (cause == null || (!(cause instanceof GhprbCause))) { + return null; + } return (GhprbCause) cause; } - public void onStarted(AbstractBuild build, PrintStream logger) { + public void onStarted(AbstractBuild build, PrintStream logger) { GhprbCause c = getCause(build); if (c == null) { return; } - repo.createCommitStatus(build, GHCommitState.PENDING, (c.isMerged() ? "Merged build started." : "Build started."), c.getPullID(), trigger.getCommitStatusContext(), logger); + repo.createCommitStatus(build, GHCommitState.PENDING, + (c.isMerged() ? "Build started, sha1 is merged" : "Build started, sha1 is original commit."), c.getPullID(), + trigger.getCommitStatusContext(), logger); try { - build.setDescription("PR #" + c.getPullID() + ": " + c.getAbbreviatedTitle()); + build.setDescription("PR #" + c.getPullID() + ": " + c.getAbbreviatedTitle()); } catch (IOException ex) { logger.println("Can't update build description"); ex.printStackTrace(logger); } } - public void onCompleted(AbstractBuild build, TaskListener listener) { + public void onCompleted(AbstractBuild build, TaskListener listener) { GhprbCause c = getCause(build); if (c == null) { return; @@ -115,112 +130,115 @@ public void onCompleted(AbstractBuild build, TaskListener listener) { } else { state = GHCommitState.FAILURE; } - repo.createCommitStatus(build, state, (c.isMerged() ? "Merged build finished." : "Build finished."), c.getPullID(), trigger.getCommitStatusContext(), listener.getLogger()); + repo.createCommitStatus(build, state, "Build finished.", c.getPullID(), trigger.getCommitStatusContext(), listener.getLogger()); - StringBuilder msg = new StringBuilder(); String publishedURL = GhprbTrigger.getDscp().getPublishedURL(); if (publishedURL != null && !publishedURL.isEmpty()) { - String commentFilePath = trigger.getCommentFilePath(); - - if (commentFilePath != null && !commentFilePath.isEmpty()) { - try { - String scriptFilePathResolved = Ghprb.replaceMacros(build, commentFilePath); - - String content = FileUtils.readFileToString(new File(scriptFilePathResolved)); - msg.append("Build comment file: \n--------------\n"); - msg.append(content); - msg.append("\n--------------\n"); - } catch (IOException e) { - msg.append("\n!!! Couldn't read commit file !!!\n"); - listener.getLogger().println("Couldn't read comment file"); - e.printStackTrace(listener.getLogger()); - } - } - - msg.append("\nRefer to this link for build results (access rights to CI server needed): \n"); - msg.append(generateCustomizedMessage(build)); - - int numLines = GhprbTrigger.getDscp().getlogExcerptLines(); - if (state != GHCommitState.SUCCESS && numLines > 0) { - // on failure, append an excerpt of the build log - try { - // wrap log in "code" markdown - msg.append("\n\n**Build Log**\n*last ").append(numLines).append(" lines*\n"); - msg.append("\n ```\n"); - List log = build.getLog(numLines); - for (String line : log) { - msg.append(line).append('\n'); - } - msg.append("```\n"); - } catch (IOException ex) { - listener.getLogger().println("Can't add log excerpt to commit comments"); - ex.printStackTrace(listener.getLogger()); - } + buildResultMessage(build, listener, state, c); + } + // close failed pull request automatically + if (state == GHCommitState.FAILURE && trigger.isAutoCloseFailedPullRequests()) { + closeFailedRequest(listener, c); + } + } + + private void closeFailedRequest(TaskListener listener, GhprbCause c) { + try { + GHPullRequest pr = repo.getPullRequest(c.getPullID()); + + if (pr.getState().equals(GHIssueState.OPEN)) { + repo.closePullRequest(c.getPullID()); } - - - String buildMessage = null; - if (state == GHCommitState.SUCCESS) { - if (trigger.getMsgSuccess() != null && !trigger.getMsgSuccess().isEmpty()) { - buildMessage = trigger.getMsgSuccess(); - } else if (GhprbTrigger.getDscp().getMsgSuccess(build) != null && !GhprbTrigger.getDscp().getMsgSuccess(build).isEmpty()) { - buildMessage = GhprbTrigger.getDscp().getMsgSuccess(build); - } - } else if (state == GHCommitState.FAILURE) { - if (trigger.getMsgFailure() != null && !trigger.getMsgFailure().isEmpty()) { - buildMessage = trigger.getMsgFailure(); - } else if (GhprbTrigger.getDscp().getMsgFailure(build) != null && !GhprbTrigger.getDscp().getMsgFailure(build).isEmpty()) { - buildMessage = GhprbTrigger.getDscp().getMsgFailure(build); - } + } catch (IOException ex) { + listener.getLogger().println("Can't close pull request"); + ex.printStackTrace(listener.getLogger()); + } + } + + private void buildResultMessage(AbstractBuild build, TaskListener listener, GHCommitState state, GhprbCause c) { + StringBuilder msg = new StringBuilder(); + String commentFilePath = trigger.getCommentFilePath(); + + if (commentFilePath != null && !commentFilePath.isEmpty()) { + try { + String scriptFilePathResolved = Ghprb.replaceMacros(build, commentFilePath); + + String content = FileUtils.readFileToString(new File(scriptFilePathResolved)); + msg.append("Build comment file: \n--------------\n"); + msg.append(content); + msg.append("\n--------------\n"); + } catch (IOException e) { + msg.append("\n!!! Couldn't read commit file !!!\n"); + listener.getLogger().println("Couldn't read comment file"); + e.printStackTrace(listener.getLogger()); } - // Only Append the build's custom message if it has been set. - if (buildMessage != null && !buildMessage.isEmpty()) { - // When the msg is not empty, append a newline first, to seperate it from the rest of the String - if (!"".equals(msg.toString())) { - msg.append("\n"); + } + + msg.append("\nRefer to this link for build results (access rights to CI server needed): \n"); + msg.append(generateCustomizedMessage(build)); + + int numLines = GhprbTrigger.getDscp().getlogExcerptLines(); + if (state != GHCommitState.SUCCESS && numLines > 0) { + // on failure, append an excerpt of the build log + try { + // wrap log in "code" markdown + msg.append("\n\n**Build Log**\n*last ").append(numLines).append(" lines*\n"); + msg.append("\n ```\n"); + List log = build.getLog(numLines); + for (String line : log) { + msg.append(line).append('\n'); } - msg.append(buildMessage); + msg.append("```\n"); + } catch (IOException ex) { + listener.getLogger().println("Can't add log excerpt to commit comments"); + ex.printStackTrace(listener.getLogger()); } + } - if (msg.length() > 0) { - listener.getLogger().println(msg); - repo.addComment(c.getPullID(), msg.toString(), build, listener); + String buildMessage = null; + if (state == GHCommitState.SUCCESS) { + if (trigger.getMsgSuccess() != null && !trigger.getMsgSuccess().isEmpty()) { + buildMessage = trigger.getMsgSuccess(); + } else if (GhprbTrigger.getDscp().getMsgSuccess(build) != null + && !GhprbTrigger.getDscp().getMsgSuccess(build).isEmpty()) { + buildMessage = GhprbTrigger.getDscp().getMsgSuccess(build); } - - // close failed pull request automatically - if (state == GHCommitState.FAILURE && trigger.isAutoCloseFailedPullRequests()) { - - try { - GHPullRequest pr = repo.getPullRequest(c.getPullID()); - - if (pr.getState().equals(GHIssueState.OPEN)) { - repo.closePullRequest(c.getPullID()); - } - } catch (IOException ex) { - listener.getLogger().println("Can't close pull request"); - ex.printStackTrace(listener.getLogger()); - } + } else if (state == GHCommitState.FAILURE) { + if (trigger.getMsgFailure() != null && !trigger.getMsgFailure().isEmpty()) { + buildMessage = trigger.getMsgFailure(); + } else if (GhprbTrigger.getDscp().getMsgFailure(build) != null + && !GhprbTrigger.getDscp().getMsgFailure(build).isEmpty()) { + buildMessage = GhprbTrigger.getDscp().getMsgFailure(build); } } + // Only Append the build's custom message if it has been set. + if (buildMessage != null && !buildMessage.isEmpty()) { + // When the msg is not empty, append a newline first, to seperate it from the rest of the String + if (!"".equals(msg.toString())) { + msg.append("\n"); + } + msg.append(buildMessage); + } + + if (msg.length() > 0) { + listener.getLogger().println(msg); + repo.addComment(c.getPullID(), msg.toString(), build, listener); + } } private String generateCustomizedMessage(AbstractBuild build) { - JobConfiguration jobConfiguration = - JobConfiguration.builder() - .printStackTrace(trigger.isDisplayBuildErrorsOnDownstreamBuilds()) - .build(); + JobConfiguration jobConfiguration = JobConfiguration.builder() + .printStackTrace(trigger.isDisplayBuildErrorsOnDownstreamBuilds()).build(); - GhprbBuildManager buildManager = - GhprbBuildManagerFactoryUtil.getBuildManager(build, jobConfiguration); + GhprbBuildManager buildManager = GhprbBuildManagerFactoryUtil.getBuildManager(build, jobConfiguration); StringBuilder sb = new StringBuilder(); sb.append(buildManager.calculateBuildUrl()); if (build.getResult() != Result.SUCCESS) { - sb.append( - buildManager.getTestResults()); + sb.append(buildManager.getTestResults()); } return sb.toString(); diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbCause.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbCause.java index 546d725c2..ba8cce14c 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbCause.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbCause.java @@ -11,98 +11,106 @@ /** * @author Honza Brázdil */ -public class GhprbCause extends Cause{ - private final String commit; - private final int pullID; - private final boolean merged; - private final String targetBranch; - private final String sourceBranch; - private final String authorEmail; - private final String title; +public class GhprbCause extends Cause { + private final String commit; + private final int pullID; + private final boolean merged; + private final String targetBranch; + private final String sourceBranch; + private final String authorEmail; + private final String title; private final URL url; private final GHUser triggerSender; private final String commentBody; private final GitUser commitAuthor; - public GhprbCause(String commit, int pullID, boolean merged, - String targetBranch, String sourceBranch, String authorEmail, - String title, URL url, GHUser triggerSender, String commentBody, - GitUser commitAuthor){ - - this.commit = commit; - this.pullID = pullID; - this.merged = merged; - this.targetBranch = targetBranch; - this.sourceBranch = sourceBranch; - this.authorEmail = authorEmail; - this.title = title; + public GhprbCause(String commit, + int pullID, + boolean merged, + String targetBranch, + String sourceBranch, + String authorEmail, + String title, + URL url, + GHUser triggerSender, + String commentBody, + GitUser commitAuthor) { + + this.commit = commit; + this.pullID = pullID; + this.merged = merged; + this.targetBranch = targetBranch; + this.sourceBranch = sourceBranch; + this.authorEmail = authorEmail; + this.title = title; this.url = url; - + this.triggerSender = triggerSender; this.commentBody = commentBody; this.commitAuthor = commitAuthor; - } - - @Override - public String getShortDescription() { - return "GitHub pull request #" + pullID + " of commit " + commit + (merged? " automatically merged." : "."); - } - - public String getCommit() { - return commit; - } - - public boolean isMerged() { - return merged; - } - - public int getPullID(){ - return pullID; - } - - public String getTargetBranch() { - return targetBranch; - } - - public String getSourceBranch() { - return sourceBranch; - } - - public String getAuthorEmail() { - return authorEmail; - } + } + + @Override + public String getShortDescription() { + return "GitHub pull request #" + pullID + " of commit " + commit + (merged ? ", no merge conflicts." : ", has merge conflicts."); + } + + public String getCommit() { + return commit; + } + + public boolean isMerged() { + return merged; + } + + public int getPullID() { + return pullID; + } + + public String getTargetBranch() { + return targetBranch; + } + + public String getSourceBranch() { + return sourceBranch; + } + + public String getAuthorEmail() { + return authorEmail; + } public URL getUrl() { return url; } - + public GHUser getTriggerSender() { - return triggerSender; + return triggerSender; } - + public String getCommentBody() { - return commentBody; + return commentBody; + } + + /** + * Returns the title of the cause, not null. + * + * @return + */ + public String getTitle() { + return title != null ? title : ""; + } + + /** + * Returns at most the first 30 characters of the title, or + * + * @return + */ + public String getAbbreviatedTitle() { + return StringUtils.abbreviate(getTitle(), 30); + } + + public GitUser getCommitAuthor() { + return commitAuthor; } - /** - * Returns the title of the cause, not null. - * @return - */ - public String getTitle() { - return title != null ? title : ""; - } - - /** - * Returns at most the first 30 characters of the title, or - * @return - */ - public String getAbbreviatedTitle() { - return StringUtils.abbreviate(getTitle(), 30); - } - - public GitUser getCommitAuthor() { - return commitAuthor; - } - - } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHub.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHub.java index 5382377ce..aba765179 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHub.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHub.java @@ -3,72 +3,76 @@ import java.io.IOException; import java.util.logging.Level; import java.util.logging.Logger; + import org.kohsuke.github.GHOrganization; import org.kohsuke.github.GHUser; import org.kohsuke.github.GitHub; import org.kohsuke.github.GitHubBuilder; +import com.google.common.annotations.VisibleForTesting; + /** * @author janinko */ public class GhprbGitHub { - private static final Logger logger = Logger.getLogger(GhprbGitHub.class.getName()); - private GitHub gh; + private static final Logger logger = Logger.getLogger(GhprbGitHub.class.getName()); + private GitHub gh; - private void connect() throws IOException{ - String accessToken = GhprbTrigger.getDscp().getAccessToken(); - String serverAPIUrl = GhprbTrigger.getDscp().getServerAPIUrl(); - if(accessToken != null && !accessToken.isEmpty()) { - try { - gh = new GitHubBuilder() - .withEndpoint(serverAPIUrl) - .withOAuthToken(accessToken) - .withConnector(new HttpConnectorWithJenkinsProxy()) - .build(); - } catch(IOException e) { - logger.log(Level.SEVERE, "Can''t connect to {0} using oauth", serverAPIUrl); - throw e; - } - } else { - if (serverAPIUrl.contains("api/v3")) { - gh = GitHub.connectToEnterprise(serverAPIUrl, GhprbTrigger.getDscp().getUsername(), GhprbTrigger.getDscp().getPassword()); - } else { - gh = new GitHubBuilder() - .withPassword(GhprbTrigger.getDscp().getUsername(), GhprbTrigger.getDscp().getPassword()) - .withConnector(new HttpConnectorWithJenkinsProxy()) - .build(); - } - } - } + private void connect() throws IOException { + String accessToken = GhprbTrigger.getDscp().getAccessToken(); + String serverAPIUrl = GhprbTrigger.getDscp().getServerAPIUrl(); + if (accessToken != null && !accessToken.isEmpty()) { + try { + gh = new GitHubBuilder().withEndpoint(serverAPIUrl) + .withOAuthToken(accessToken).withConnector(new HttpConnectorWithJenkinsProxy()).build(); + } catch (IOException e) { + logger.log(Level.SEVERE, "Can''t connect to {0} using oauth", serverAPIUrl); + throw e; + } + } else { + if (serverAPIUrl.contains("api/v3")) { + gh = GitHub.connectToEnterprise(serverAPIUrl, + GhprbTrigger.getDscp().getUsername(), GhprbTrigger.getDscp().getPassword()); + } else { + gh = new GitHubBuilder().withPassword(GhprbTrigger.getDscp().getUsername(), + GhprbTrigger.getDscp().getPassword()).withConnector(new HttpConnectorWithJenkinsProxy()).build(); + } + } + } - public GitHub get() throws IOException{ - if(gh == null){ - connect(); - } - return gh; - } + public GitHub get() throws IOException { + if (gh == null) { + connect(); + } + return gh; + } + + @VisibleForTesting + void setGitHub(GitHub gh) { + this.gh = gh; + } - public boolean isUserMemberOfOrganization(String organisation, GHUser member){ - boolean orgHasMember = false; - try { - GHOrganization org = get().getOrganization(organisation); - orgHasMember = org.hasMember(member); - logger.log(Level.FINE, "org.hasMember(member)? user:{0} org: {1} == {2}", - new Object[]{member.getLogin(), organisation, orgHasMember ? "yes" : "no"}); + public boolean isUserMemberOfOrganization(String organisation, GHUser member) { + boolean orgHasMember = false; + try { + GHOrganization org = get().getOrganization(organisation); + orgHasMember = org.hasMember(member); + logger.log(Level.FINE, "org.hasMember(member)? user:{0} org: {1} == {2}", + new Object[] { member.getLogin(), organisation, orgHasMember ? "yes" : "no" }); - } catch (IOException ex) { - logger.log(Level.SEVERE, null, ex); - return false; - } - return orgHasMember; - } + } catch (IOException ex) { + logger.log(Level.SEVERE, null, ex); + return false; + } + return orgHasMember; + } - public String getBotUserLogin() { - try { - return get().getMyself().getLogin(); - } catch (IOException ex) { - logger.log(Level.SEVERE, null, ex); - return null; - } - } + public String getBotUserLogin() { + try { + return get().getMyself().getLogin(); + } catch (IOException ex) { + logger.log(Level.SEVERE, null, ex); + return null; + } + } } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequest.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequest.java index 134c32a0c..793debb7b 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequest.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequest.java @@ -23,8 +23,7 @@ import java.util.regex.Pattern; /** - * Maintains state about a Pull Request for a particular Jenkins job. This is what understands the current state - * of a PR for a particular job. + * Maintains state about a Pull Request for a particular Jenkins job. This is what understands the current state of a PR for a particular job. * * @author Honza Brázdil */ @@ -44,7 +43,7 @@ public class GhprbPullRequest { private String source; private String authorEmail; private URL url; - + private GHUser triggerSender; private GitUser commitAuthor; @@ -55,18 +54,23 @@ public class GhprbPullRequest { private transient Ghprb helper; private transient GhprbRepository repo; - private String commentBody; + private String commentBody; GhprbPullRequest(GHPullRequest pr, Ghprb helper, GhprbRepository repo) { id = pr.getNumber(); - updated = pr.getUpdatedAt(); + try { + updated = pr.getUpdatedAt(); + } catch (IOException e) { + e.printStackTrace(); + updated = new Date(); + } head = pr.getHead().getSha(); title = pr.getTitle(); author = pr.getUser(); reponame = repo.getName(); target = pr.getBase().getRef(); source = pr.getHead().getRef(); - url = pr.getUrl(); + url = pr.getHtmlUrl(); this.pr = pr; obtainAuthorEmail(pr); @@ -77,11 +81,13 @@ public class GhprbPullRequest { accepted = true; shouldRun = true; } else { - logger.log(Level.INFO, "Author of #{0} {1} on {2} not in whitelist!", new Object[]{id, author.getLogin(), reponame}); + logger.log(Level.INFO, "Author of #{0} {1} on {2} not in whitelist!", new Object[] { id, author.getLogin(), reponame }); repo.addComment(id, GhprbTrigger.getDscp().getRequestForTestingPhrase()); } - logger.log(Level.INFO, "Created Pull Request #{0} on {1} by {2} ({3}) updated at: {4} SHA: {5}", new Object[]{id, reponame, author.getLogin(), authorEmail, updated, head}); + logger.log(Level.INFO, "Created Pull Request #{0} on {1} by {2} ({3}) updated at: {4} SHA: {5}", + new Object[] { id, reponame, author.getLogin(), authorEmail, updated, head } + ); } public void init(Ghprb helper, GhprbRepository repo) { @@ -91,46 +97,51 @@ public void init(Ghprb helper, GhprbRepository repo) { reponame = repo.getName(); // If this instance was created before v1.8, it can be null. } } - + /** - * Returns skip build phrases from Jenkins global configuration + * Returns skip build phrases from Jenkins global configuration + * * @return */ public Set getSkipBuildPhrases() { - return new HashSet(Arrays.asList(GhprbTrigger.getDscp().getSkipBuildPhrase().split("[\\r\\n]+"))); - } - + return new HashSet(Arrays.asList(GhprbTrigger.getDscp().getSkipBuildPhrase().split("[\\r\\n]+"))); + } + /** * Checks for skip build phrase in pull request comment. If present it updates shouldRun as false. + * * @param issue */ - private void checkSkipBuild(GHIssue issue) { - // check for skip build phrase. - String pullRequestBody = issue.getBody(); - if(StringUtils.isNotBlank(pullRequestBody)) { - pullRequestBody = pullRequestBody.trim(); - Set skipBuildPhrases = getSkipBuildPhrases(); - skipBuildPhrases.remove(""); - - for (String skipBuildPhrase : skipBuildPhrases) { - skipBuildPhrase = skipBuildPhrase.trim(); - Pattern skipBuildPhrasePattern = Pattern.compile(skipBuildPhrase, Pattern.CASE_INSENSITIVE); - if(skipBuildPhrasePattern.matcher(pullRequestBody).matches()) { - logger.log(Level.INFO, "Pull request commented with {0} skipBuildPhrase. Hence skipping the build.", skipBuildPhrase); - shouldRun = false; - break; - } - } - } - } + private void checkSkipBuild(GHIssue issue) { + // check for skip build phrase. + String pullRequestBody = issue.getBody(); + if (StringUtils.isNotBlank(pullRequestBody)) { + pullRequestBody = pullRequestBody.trim(); + Set skipBuildPhrases = getSkipBuildPhrases(); + skipBuildPhrases.remove(""); + + for (String skipBuildPhrase : skipBuildPhrases) { + skipBuildPhrase = skipBuildPhrase.trim(); + Pattern skipBuildPhrasePattern = Pattern.compile(skipBuildPhrase, Pattern.CASE_INSENSITIVE); + if (skipBuildPhrasePattern.matcher(pullRequestBody).matches()) { + logger.log(Level.INFO, "Pull request commented with {0} skipBuildPhrase. Hence skipping the build.", skipBuildPhrase); + shouldRun = false; + break; + } + } + } + } /** - * Checks this Pull Request representation against a GitHub version of the Pull Request, and triggers - * a build if necessary. + * Checks this Pull Request representation against a GitHub version of the Pull Request, and triggers a build if necessary. * * @param pr */ public void check(GHPullRequest pr) { + if (helper.isProjectDisabled()) { + logger.log(Level.FINE, "Project is disabled, ignoring pull request"); + return; + } if (target == null) { target = pr.getBase().getRef(); // If this instance was created before target was introduced (before v1.8), it can be null. } @@ -139,7 +150,7 @@ public void check(GHPullRequest pr) { } if (isUpdated(pr)) { - logger.log(Level.INFO, "Pull request #{0} was updated on {1} at {2} by {3}", new Object[]{id, reponame, updated, author}); + logger.log(Level.INFO, "Pull request #{0} was updated on {1} at {2} by {3}", new Object[] { id, reponame, updated, author }); // the title could have been updated since the original PR was opened title = pr.getTitle(); @@ -147,15 +158,27 @@ public void check(GHPullRequest pr) { boolean newCommit = checkCommit(pr.getHead().getSha()); if (!newCommit && commentsChecked == 0) { - logger.log(Level.INFO, "Pull request #{0} was updated on repo {1} but there aren''t any new comments nor commits; that may mean that commit status was updated.", new Object[] {id, reponame}); + logger.log(Level.INFO, "Pull request #{0} was updated on repo {1} but there aren''t any new comments nor commits; " + + "that may mean that commit status was updated.", + new Object[] { id, reponame } + ); + } + try { + updated = pr.getUpdatedAt(); + } catch (IOException e) { + e.printStackTrace(); + updated = new Date(); } - updated = pr.getUpdatedAt(); } checkSkipBuild(pr); tryBuild(pr); } public void check(GHIssueComment comment) { + if (helper.isProjectDisabled()) { + logger.log(Level.FINE, "Project is disabled, ignoring comment"); + return; + } try { checkComment(comment); updated = comment.getUpdatedAt(); @@ -186,17 +209,30 @@ public boolean isWhiteListedTargetBranch() { return true; } } - logger.log(Level.FINEST, "PR #{0} target branch: {1} isn''t in our whitelist of target branches: {2}", new Object[]{id, target, Joiner.on(',').skipNulls().join(branches)}); + logger.log(Level.FINEST, "PR #{0} target branch: {1} isn''t in our whitelist of target branches: {2}", + new Object[] { id, target, Joiner.on(',').skipNulls().join(branches) } + ); return false; } private boolean isUpdated(GHPullRequest pr) { - boolean ret = updated.compareTo(pr.getUpdatedAt()) < 0; + Date lastUpdated = new Date(); + try { + lastUpdated = pr.getUpdatedAt(); + } catch (IOException e) { + e.printStackTrace(); + } + boolean ret = updated.compareTo(lastUpdated) < 0; ret = ret || !pr.getHead().getSha().equals(head); return ret; } private void tryBuild(GHPullRequest pr) { + if (helper.isProjectDisabled()) { + logger.log(Level.FINEST, "Project is disabled, not trying to build"); + shouldRun = false; + triggered = false; + } if (helper.ifOnlyTriggerPhrase() && !triggered) { logger.log(Level.FINEST, "Trigger only phrase but we are not triggered"); shouldRun = false; @@ -217,16 +253,16 @@ private void tryBuild(GHPullRequest pr) { logger.log(Level.FINEST, "PR is not null, checking if mergable"); checkMergeable(pr); try { - for (GHPullRequestCommitDetail commitDetails : pr.listCommits()) { - if (commitDetails.getSha().equals(getHead())) { - commitAuthor = commitDetails.getCommit().getCommitter(); - break; - } - } + for (GHPullRequestCommitDetail commitDetails : pr.listCommits()) { + if (commitDetails.getSha().equals(getHead())) { + commitAuthor = commitDetails.getCommit().getCommitter(); + break; + } + } } catch (Exception ex) { - logger.log(Level.INFO, "Unable to get PR commits: ", ex); + logger.log(Level.INFO, "Unable to get PR commits: ", ex); } - + } logger.log(Level.FINEST, "Running build..."); @@ -237,10 +273,10 @@ private void tryBuild(GHPullRequest pr) { } } - private void build() { + private void build() { String message = helper.getBuilds().build(this, triggerSender, commentBody); String context = helper.getTrigger().getCommitStatusContext(); - repo.createCommitStatus(head, GHCommitState.PENDING, null, message,id,context); + repo.createCommitStatus(head, GHCommitState.PENDING, null, message, id, context); logger.log(Level.INFO, message); } @@ -249,7 +285,7 @@ private boolean checkCommit(String sha) { if (head.equals(sha)) { return false; } - logger.log(Level.FINE, "New commit. Sha: {0} => {1}", new Object[]{head, sha}); + logger.log(Level.FINE, "New commit. Sha: {0} => {1}", new Object[] { head, sha }); head = sha; if (accepted) { shouldRun = true; @@ -265,23 +301,23 @@ private void checkComment(GHIssueComment comment) throws IOException { // ignore comments from bot user, this fixes an issue where the bot would auto-whitelist // a user or trigger a build when the 'request for testing' phrase contains the // whitelist/trigger phrase and the bot is a member of a whitelisted organisation -// if (helper.isBotUser(sender)) { -// logger.log(Level.INFO, "Comment from bot user {0} ignored.", sender); -// return; -// } + // if (helper.isBotUser(sender)) { + // logger.log(Level.INFO, "Comment from bot user {0} ignored.", sender); + // return; + // } - if (helper.isWhitelistPhrase(body) && helper.isAdmin(sender)) { // add to whitelist + if (helper.isWhitelistPhrase(body) && helper.isAdmin(sender)) { // add to whitelist if (!helper.isWhitelisted(author)) { logger.log(Level.FINEST, "Author {0} not whitelisted, adding to whitelist.", author); helper.addWhitelist(author.getLogin()); } accepted = true; shouldRun = true; - } else if (helper.isOktotestPhrase(body) && helper.isAdmin(sender)) { // ok to test + } else if (helper.isOktotestPhrase(body) && helper.isAdmin(sender)) { // ok to test logger.log(Level.FINEST, "Admin {0} gave OK to test", sender); accepted = true; shouldRun = true; - } else if (helper.isRetestPhrase(body)) { // test this please + } 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); @@ -291,7 +327,7 @@ private void checkComment(GHIssueComment comment) throws IOException { shouldRun = true; triggered = true; } - } else if (helper.isTriggerPhrase(body)) { // trigger phrase + } else if (helper.isTriggerPhrase(body)) { // trigger phrase logger.log(Level.FINEST, "Trigger phrase"); if (helper.isAdmin(sender)) { logger.log(Level.FINEST, "Admin {0} ran trigger phrase", sender); @@ -303,10 +339,10 @@ private void checkComment(GHIssueComment comment) throws IOException { triggered = true; } } - + if (shouldRun) { - triggerSender = sender; - commentBody = body; + triggerSender = sender; + commentBody = body; } } @@ -410,11 +446,11 @@ public URL getUrl() { return url; } - public GitUser getCommitAuthor() { - return commitAuthor; - } - - public GHPullRequest getPullRequest() { - return pr; - } + public GitUser getCommitAuthor() { + return commitAuthor; + } + + public GHPullRequest getPullRequest() { + return pr; + } } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestMerge.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestMerge.java index d9974f571..8e45d717a 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestMerge.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestMerge.java @@ -5,7 +5,6 @@ import java.util.concurrent.ConcurrentMap; import org.kohsuke.github.GHBranch; -//import org.kohsuke.github.GHBranch; import org.kohsuke.github.GHPullRequestCommitDetail.Commit; import org.kohsuke.github.GHPullRequest; import org.kohsuke.github.GHPullRequestCommitDetail; @@ -31,210 +30,203 @@ import hudson.util.FormValidation; public class GhprbPullRequestMerge extends Recorder { - + private PrintStream logger; - - - private final boolean onlyAdminsMerge; - private final boolean disallowOwnCode; - private boolean onlyTriggerPhrase; - private String mergeComment; - - @DataBoundConstructor - public GhprbPullRequestMerge(String mergeComment, boolean onlyTriggerPhrase, - boolean onlyAdminsMerge, boolean disallowOwnCode) { - - this.mergeComment = mergeComment; - this.onlyTriggerPhrase = onlyTriggerPhrase; - this.onlyAdminsMerge = onlyAdminsMerge; - this.disallowOwnCode = disallowOwnCode; - } - - public String getMergeComment() { - return mergeComment; - } - - public boolean isOnlyTriggerPhrase() { - return onlyTriggerPhrase; - } - - public boolean isOnlyAdminsMerge() { - return onlyAdminsMerge; - } - - public boolean isDisallowOwnCode() { - return disallowOwnCode; - } - - public BuildStepMonitor getRequiredMonitorService() { - return BuildStepMonitor.BUILD; - } - - private GhprbTrigger trigger; - private Ghprb helper; - private GhprbCause cause; - private GHPullRequest pr; - - @VisibleForTesting - void setHelper(Ghprb helper) { - this.helper = helper; - } - - @Override - public boolean perform(AbstractBuild build, Launcher launcher, final BuildListener listener) throws InterruptedException, IOException { - logger = listener.getLogger(); - AbstractProject project = build.getProject(); - if (build.getResult().isWorseThan(Result.SUCCESS)) { - logger.println("Build did not succeed, merge will not be run"); - return true; - } - - trigger = GhprbTrigger.extractTrigger(project); - if (trigger == null) return false; - - cause = getCause(build); - if (cause == null) { - return true; - } - - ConcurrentMap pulls = trigger.getDescriptor().getPullRequests(project.getName()); - - - pr = pulls.get(cause.getPullID()).getPullRequest(); - - if (pr == null) { - logger.println("Pull request is null for ID: " + cause.getPullID()); - logger.println("" + pulls.toString()); - return false; - } - - Boolean isMergeable = cause.isMerged(); - - if (helper == null) { - helper = new Ghprb(project, trigger, pulls); - helper.init(); - } - - if (isMergeable == null || !isMergeable) { - logger.println("Pull request cannot be automerged."); - commentOnRequest("Pull request is not mergeable."); - return false; - } - - - GHUser triggerSender = cause.getTriggerSender(); - - // ignore comments from bot user, this fixes an issue where the bot would auto-merge - // a PR when the 'request for testing' phrase contains the PR merge trigger phrase and - // the bot is a member of a whitelisted organisation - if (helper.isBotUser(triggerSender)) { - logger.println("Comment from bot user " + triggerSender.getLogin() + " ignored."); - return false; - } - - boolean merge = true; - - - if (isOnlyAdminsMerge() && !helper.isAdmin(triggerSender)){ - merge = false; - logger.println("Only admins can merge this pull request, " + triggerSender.getLogin() + " is not an admin."); - commentOnRequest( - String.format("Code not merged because %s is not in the Admin list.", - triggerSender.getName())); - } - - if (isOnlyTriggerPhrase() && !helper.isTriggerPhrase(cause.getCommentBody())) { - merge = false; - logger.println("The comment does not contain the required trigger phrase."); - - commentOnRequest( - String.format("Please comment with '%s' to automerge this request", - trigger.getTriggerPhrase())); - } - - if (isDisallowOwnCode() && isOwnCode(pr, triggerSender)) { - merge = false; - logger.println("The commentor is also one of the contributors."); - commentOnRequest( - String.format("Code not merged because %s has committed code in the request.", - triggerSender.getName())); - } - - if (merge) { - logger.println("Merging the pull request"); - - pr.merge(getMergeComment()); - logger.println("Pull request successfully merged"); -// deleteBranch(); //TODO: Update so it also deletes the branch being pulled from. probably make it an option. - } - - return merge; - } - - private void deleteBranch() { - String branchName = pr.getHead().getRef(); - try { - GHBranch branch = pr.getRepository().getBranches().get(branchName); - - } catch (IOException e) { - // TODO Auto-generated catch block - e.printStackTrace(); - } - } - - private void commentOnRequest(String comment) { - try { - helper.getRepository().addComment(pr.getNumber(), comment); - } catch (Exception e) { - logger.println("Failed to add comment"); - e.printStackTrace(logger); - } - } - - - private boolean isOwnCode(GHPullRequest pr, GHUser committer) { - try { - String commentorName = committer.getName(); - for (GHPullRequestCommitDetail detail : pr.listCommits()) { - Commit commit = detail.getCommit(); - String committerName = commit.getCommitter().getName(); - - if (committerName.equalsIgnoreCase(commentorName)) { - return true; - } - } - } catch (IOException e) { - logger.println("Unable to get committer name"); - e.printStackTrace(logger); - } - return false; - } - + + private final boolean onlyAdminsMerge; + private final boolean disallowOwnCode; + private boolean onlyTriggerPhrase; + private String mergeComment; + + @DataBoundConstructor + public GhprbPullRequestMerge(String mergeComment, boolean onlyTriggerPhrase, boolean onlyAdminsMerge, boolean disallowOwnCode) { + + this.mergeComment = mergeComment; + this.onlyTriggerPhrase = onlyTriggerPhrase; + this.onlyAdminsMerge = onlyAdminsMerge; + this.disallowOwnCode = disallowOwnCode; + } + + public String getMergeComment() { + return mergeComment; + } + + public boolean isOnlyTriggerPhrase() { + return onlyTriggerPhrase; + } + + public boolean isOnlyAdminsMerge() { + return onlyAdminsMerge; + } + + public boolean isDisallowOwnCode() { + return disallowOwnCode; + } + + public BuildStepMonitor getRequiredMonitorService() { + return BuildStepMonitor.BUILD; + } + + private GhprbTrigger trigger; + private Ghprb helper; + private GhprbCause cause; + private GHPullRequest pr; + + @VisibleForTesting + void setHelper(Ghprb helper) { + this.helper = helper; + } + + @Override + public boolean perform(AbstractBuild build, Launcher launcher, final BuildListener listener) throws InterruptedException, IOException { + logger = listener.getLogger(); + AbstractProject project = build.getProject(); + if (build.getResult().isWorseThan(Result.SUCCESS)) { + logger.println("Build did not succeed, merge will not be run"); + return true; + } + + trigger = GhprbTrigger.extractTrigger(project); + if (trigger == null) + return false; + + cause = getCause(build); + if (cause == null) { + return true; + } + + ConcurrentMap pulls = trigger.getDescriptor().getPullRequests(project.getFullName()); + + pr = pulls.get(cause.getPullID()).getPullRequest(); + + if (pr == null) { + logger.println("Pull request is null for ID: " + cause.getPullID()); + logger.println("" + pulls.toString()); + return false; + } + + Boolean isMergeable = cause.isMerged(); + + if (helper == null) { + helper = new Ghprb(project, trigger, pulls); + helper.init(); + } + + if (isMergeable == null || !isMergeable) { + logger.println("Pull request cannot be automerged."); + commentOnRequest("Pull request is not mergeable."); + listener.finished(Result.FAILURE); + return false; + } + + GHUser triggerSender = cause.getTriggerSender(); + + // ignore comments from bot user, this fixes an issue where the bot would auto-merge + // a PR when the 'request for testing' phrase contains the PR merge trigger phrase and + // the bot is a member of a whitelisted organisation + if (helper.isBotUser(triggerSender)) { + logger.println("Comment from bot user " + triggerSender.getLogin() + " ignored."); + return false; + } + + boolean merge = true; + + if (isOnlyAdminsMerge() && !helper.isAdmin(triggerSender)) { + merge = false; + logger.println("Only admins can merge this pull request, " + triggerSender.getLogin() + " is not an admin."); + commentOnRequest(String.format("Code not merged because %s is not in the Admin list.", triggerSender.getName())); + } + + if (isOnlyTriggerPhrase() && !helper.isTriggerPhrase(cause.getCommentBody())) { + merge = false; + logger.println("The comment does not contain the required trigger phrase."); + + commentOnRequest(String.format("Please comment with '%s' to automerge this request", trigger.getTriggerPhrase())); + } + + if (isDisallowOwnCode() && isOwnCode(pr, triggerSender)) { + merge = false; + logger.println("The commentor is also one of the contributors."); + commentOnRequest(String.format("Code not merged because %s has committed code in the request.", triggerSender.getName())); + } + + if (merge) { + logger.println("Merging the pull request"); + + pr.merge(getMergeComment()); + logger.println("Pull request successfully merged"); + // deleteBranch(); //TODO: Update so it also deletes the branch being pulled from. probably make it an option. + } + + if (merge) { + listener.finished(Result.SUCCESS); + } else { + listener.finished(Result.FAILURE); + } + return merge; + } + + private void deleteBranch() { + String branchName = pr.getHead().getRef(); + try { + GHBranch branch = pr.getRepository().getBranches().get(branchName); + + } catch (IOException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } + } + + private void commentOnRequest(String comment) { + try { + helper.getRepository().addComment(pr.getNumber(), comment); + } catch (Exception e) { + logger.println("Failed to add comment"); + e.printStackTrace(logger); + } + } + + private boolean isOwnCode(GHPullRequest pr, GHUser committer) { + try { + String commentorName = committer.getName(); + for (GHPullRequestCommitDetail detail : pr.listCommits()) { + Commit commit = detail.getCommit(); + String committerName = commit.getCommitter().getName(); + + if (committerName.equalsIgnoreCase(commentorName)) { + return true; + } + } + } catch (IOException e) { + logger.println("Unable to get committer name"); + e.printStackTrace(logger); + } + return false; + } + private GhprbCause getCause(AbstractBuild build) { Cause cause = build.getCause(GhprbCause.class); - if (cause == null || (!(cause instanceof GhprbCause))) return null; + if (cause == null || (!(cause instanceof GhprbCause))) + return null; return (GhprbCause) cause; } - - - @Extension(ordinal=-1) - public static class DescriptorImpl extends BuildStepDescriptor { - @Override - public String getDisplayName() { - return "Github Pull Request Merger"; - } - - @Override - public boolean isApplicable(Class jobType) { - return true; - } - - public FormValidation doCheck(@AncestorInPath AbstractProject project, @QueryParameter String value) - throws IOException { - return FilePath.validateFileMask(project.getSomeWorkspace(),value); - } - - - } + + @Extension(ordinal = -1) + public static class DescriptorImpl extends BuildStepDescriptor { + @Override + public String getDisplayName() { + return "Github Pull Request Merger"; + } + + @Override + public boolean isApplicable(Class jobType) { + return true; + } + + public FormValidation doCheck(@AncestorInPath AbstractProject project, @QueryParameter String value) throws IOException { + return FilePath.validateFileMask(project.getSomeWorkspace(), value); + } + + } } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRepository.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRepository.java index 878160574..7cb306ccc 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRepository.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRepository.java @@ -81,6 +81,11 @@ public void check() { return; } + if (helper.isProjectDisabled()) { + logger.log(Level.FINE, "Project is disabled, not checking github state"); + return; + } + List openPulls; try { openPulls = ghRepository.getPullRequests(GHIssueState.OPEN); @@ -127,9 +132,9 @@ public void createCommitStatus(AbstractBuild build, GHCommitState state, S } public void createCommitStatus(String sha1, GHCommitState state, String url, String message, int id, String context) { - createCommitStatus(null, sha1, state, url, message, id, context, null); + createCommitStatus(null, sha1, state, url, message, id, context, null); } - + public void createCommitStatus(AbstractBuild build, String sha1, GHCommitState state, String url, String message, int id, String context, PrintStream stream) { String newMessage = String.format("Setting status of %s to %s with url %s and message: %s", sha1, state, url, message); if (stream != null) { @@ -143,16 +148,12 @@ public void createCommitStatus(AbstractBuild build, String sha1, GHCommitS } else { ghRepository.createCommitStatus(sha1, state, url, message); } - } catch (FileNotFoundException ex) { - newMessage = "FileNotFoundException means that the credentials Jenkins is using is probably wrong. Or that something is really wrong with github."; - if (stream != null) { - stream.println(newMessage); - ex.printStackTrace(stream); + } catch (IOException ex) { + if (ex instanceof FileNotFoundException) { + newMessage = "FileNotFoundException means that the credentials Jenkins is using is probably wrong. Or the user account does not have write access to the repo."; } else { - logger.log(Level.INFO, newMessage, ex); + newMessage = "Could not update commit status of the Pull Request on GitHub."; } - } catch (IOException ex) { - newMessage = "Could not update commit status of the Pull Request on GitHub."; if (stream != null) { stream.println(newMessage); ex.printStackTrace(stream); @@ -160,15 +161,15 @@ public void createCommitStatus(AbstractBuild build, String sha1, GHCommitS logger.log(Level.INFO, newMessage, ex); } if (GhprbTrigger.getDscp().getUseComments()) { - + if (state == GHCommitState.SUCCESS) { message = message + " " + GhprbTrigger.getDscp().getMsgSuccess(build); } else if (state == GHCommitState.FAILURE) { message = message + " " + GhprbTrigger.getDscp().getMsgFailure(build); } if (GhprbTrigger.getDscp().getUseDetailedComments() || (state == GHCommitState.SUCCESS || state == GHCommitState.FAILURE)) { - logger.log(Level.INFO, "Trying to send comment.", ex); - addComment(id, message); + logger.log(Level.INFO, "Trying to send comment.", ex); + addComment(id, message); } } else { logger.log(Level.SEVERE, "Could not update commit status of the Pull Request on GitHub."); @@ -181,7 +182,7 @@ public String getName() { } public void addComment(int id, String comment) { - addComment(id, comment, null, null); + addComment(id, comment, null, null); } public void addComment(int id, String comment, AbstractBuild build, TaskListener listener) { @@ -189,11 +190,11 @@ public void addComment(int id, String comment, AbstractBuild build, TaskLi return; if (build != null && listener != null) { - try { - comment = build.getEnvironment(listener).expand(comment); - } catch (Exception e) { - logger.log(Level.SEVERE, "Error", e); - } + try { + comment = build.getEnvironment(listener).expand(comment); + } catch (Exception e) { + logger.log(Level.SEVERE, "Error", e); + } } try { @@ -253,11 +254,20 @@ public GHPullRequest getPullRequest(int id) throws IOException { } void onIssueCommentHook(IssueComment issueComment) throws IOException { + if (helper.isProjectDisabled()) { + logger.log(Level.FINE, "Not checking comments since build is disabled"); + return; + } int id = issueComment.getIssue().getNumber(); - logger.log(Level.FINER, "Comment on issue #{0} from {1}: {2}", new Object[]{id, issueComment.getComment().getUser(), issueComment.getComment().getBody()}); + logger.log(Level.FINER, "Comment on issue #{0} from {1}: {2}", new Object[] { id, issueComment.getComment().getUser(), issueComment.getComment().getBody() }); if (!"created".equals(issueComment.getAction())) { return; } + + if (ghRepository == null) { + init(); + } + GhprbPullRequest pull = pulls.get(id); if (pull == null) { pull = new GhprbPullRequest(ghRepository.getPullRequest(id), helper, this); @@ -268,7 +278,11 @@ void onIssueCommentHook(IssueComment issueComment) throws IOException { } void onPullRequestHook(PullRequest pr) { - if ("opened".equals(pr.getAction()) || "reopened".equals(pr.getAction())) { + if ("closed".equals(pr.getAction())) { + pulls.remove(pr.getNumber()); + } else if (helper.isProjectDisabled()) { + logger.log(Level.FINE, "Not processing Pull request since the build is disabled"); + } else if ("opened".equals(pr.getAction()) || "reopened".equals(pr.getAction())) { GhprbPullRequest pull = pulls.get(pr.getNumber()); if (pull == null) { pulls.putIfAbsent(pr.getNumber(), new GhprbPullRequest(pr.getPullRequest(), helper, this)); @@ -286,8 +300,6 @@ void onPullRequestHook(PullRequest pr) { return; } pull.check(pr.getPullRequest()); - } else if ("closed".equals(pr.getAction())) { - pulls.remove(pr.getNumber()); } else { logger.log(Level.WARNING, "Unknown Pull Request hook action: {0}", pr.getAction()); } @@ -305,7 +317,7 @@ boolean checkSignature(String body, String signature) { Mac mac = Mac.getInstance(algorithm); mac.init(keySpec); byte[] localSignatureBytes = mac.doFinal(body.getBytes("UTF-8")); - String localSignature = new String(Hex.encodeHexString(localSignatureBytes)); + String localSignature = Hex.encodeHexString(localSignatureBytes); if (! localSignature.equals(expected)) { logger.log(Level.SEVERE, "Local signature {0} does not match external signature {1}", new Object[] {localSignature, expected}); diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRootAction.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRootAction.java index 8a527fc11..7a4268fe7 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRootAction.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRootAction.java @@ -4,6 +4,7 @@ import hudson.model.AbstractProject; import hudson.model.UnprotectedRootAction; import hudson.security.ACL; +import hudson.security.csrf.CrumbExclusion; import jenkins.model.Jenkins; import org.acegisecurity.Authentication; @@ -25,6 +26,11 @@ import java.util.logging.Level; import java.util.logging.Logger; +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpServletRequest; + /** * @author Honza Brázdil */ @@ -62,7 +68,8 @@ public void doIndex(StaplerRequest req, StaplerResponse resp) { } else if ("application/x-www-form-urlencoded".equals(type)) { body = extractRequestBody(req); if (body == null || body.length() <= 8) { - logger.log(Level.SEVERE, "Request doesn't contain payload. You're sending url encoded request, so you should pass github payload through 'payload' request parameter"); + logger.log(Level.SEVERE, "Request doesn't contain payload. " + + "You're sending url encoded request, so you should pass github payload through 'payload' request parameter"); return; } try { @@ -74,7 +81,9 @@ public void doIndex(StaplerRequest req, StaplerResponse resp) { } if (payload == null) { - logger.log(Level.SEVERE, "Payload is null, maybe content type '{0}' is not supported by this plugin. Please use 'application/json' or 'application/x-www-form-urlencoded'", new Object[] {type}); + logger.log(Level.SEVERE, "Payload is null, maybe content type '{0}' is not supported by this plugin. " + + "Please use 'application/json' or 'application/x-www-form-urlencoded'", + new Object[] { type }); return; } @@ -83,7 +92,8 @@ public void doIndex(StaplerRequest req, StaplerResponse resp) { logger.log(Level.INFO, "Got payload event: {0}", event); try { if ("issue_comment".equals(event)) { - GHEventPayload.IssueComment issueComment = gh.get().parseEventPayload(new StringReader(payload), GHEventPayload.IssueComment.class); + GHEventPayload.IssueComment issueComment = gh.get() + .parseEventPayload(new StringReader(payload), GHEventPayload.IssueComment.class); GHIssueState state = issueComment.getIssue().getState(); if (state == GHIssueState.CLOSED) { logger.log(Level.INFO, "Skip comment on closed PR"); @@ -91,14 +101,16 @@ public void doIndex(StaplerRequest req, StaplerResponse resp) { } for (GhprbRepository repo : getRepos(issueComment.getRepository())) { - logger.log(Level.INFO, "Checking issue comment ''{0}'' for repo {1}", new Object[] {issueComment.getComment(), repo.getName()}); + logger.log(Level.INFO, "Checking issue comment ''{0}'' for repo {1}", + new Object[] { issueComment.getComment(), repo.getName() } + ); if(repo.checkSignature(body, signature)) repo.onIssueCommentHook(issueComment); } } else if ("pull_request".equals(event)) { GHEventPayload.PullRequest pr = gh.get().parseEventPayload(new StringReader(payload), GHEventPayload.PullRequest.class); for (GhprbRepository repo : getRepos(pr.getPullRequest().getRepository())) { - logger.log(Level.INFO, "Checking PR #{1} for {0}", new Object[] { repo.getName(), pr.getNumber()}); + logger.log(Level.INFO, "Checking PR #{1} for {0}", new Object[] { repo.getName(), pr.getNumber() }); if(repo.checkSignature(body, signature)) repo.onPullRequestHook(pr); } @@ -142,13 +154,36 @@ private Set getRepos(String repo) { continue; } GhprbRepository r = trigger.getRepository(); - if (repo.equals(r.getName())) { + if (repo.equalsIgnoreCase(r.getName())) { ret.add(r); } } } finally { SecurityContextHolder.getContext().setAuthentication(old); } + + if (ret.size() == 0) { + logger.log(Level.WARNING, "No repos with plugin trigger found for GitHub repo named {0}", repo); + } + return ret; } + + @Extension + public static class GhprbRootActionCrumbExclusion extends CrumbExclusion { + + @Override + public boolean process(HttpServletRequest req, HttpServletResponse resp, FilterChain chain) throws IOException, ServletException { + String pathInfo = req.getPathInfo(); + if (pathInfo != null && pathInfo.equals(getExclusionPath())) { + chain.doFilter(req, resp); + return true; + } + return false; + } + + public String getExclusionPath() { + return "/" + URL + "/"; + } + } } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTokenMacro.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTokenMacro.java index 122685d38..d0db0bd96 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTokenMacro.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTokenMacro.java @@ -10,12 +10,11 @@ import java.io.IOException; /** - * {@code PR_Name} token that expands to the PR Name. - * {@code PR_User} token that expands to the PR Opener's email. + * {@code PR_Name} token that expands to the PR Name. {@code PR_User} token that expands to the PR Opener's email. * * @author Josh Caldwell */ -@Extension(optional=true) +@Extension(optional = true) public class GhprbTokenMacro extends DataBoundTokenMacro { @Override public boolean acceptsMacroName(String macroName) { @@ -23,7 +22,8 @@ public boolean acceptsMacroName(String macroName) { } @Override - public String evaluate(AbstractBuild context, TaskListener listener, String macroName) throws MacroEvaluationException, IOException, InterruptedException { + public String evaluate(AbstractBuild context, TaskListener listener, String macroName) + throws MacroEvaluationException, IOException, InterruptedException { GhprbCause cause = (GhprbCause) context.getCause(GhprbCause.class); if (cause == null) { return ""; @@ -34,7 +34,7 @@ public String evaluate(AbstractBuild context, TaskListener listener, Strin } else if (macroName.equals("PR_Email")) { return cause.getAuthorEmail(); } else { - return ""; + return ""; } } } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java index 0ec2a3eed..db7a658f3 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java @@ -7,7 +7,7 @@ import hudson.Extension; import hudson.model.*; -import hudson.model.StringParameterValue; +import hudson.model.AbstractProject; import hudson.model.queue.QueueTaskFuture; import hudson.plugins.git.util.BuildData; import hudson.triggers.Trigger; @@ -61,23 +61,24 @@ public class GhprbTrigger extends Trigger> { private String secret; @DataBoundConstructor + public GhprbTrigger(String adminlist, - String whitelist, - String orgslist, - String cron, - String triggerPhrase, - Boolean onlyTriggerPhrase, - Boolean useGitHubHooks, - Boolean permitAll, - Boolean autoCloseFailedPullRequests, - Boolean displayBuildErrorsOnDownstreamBuilds, - String commentFilePath, - List whiteListTargetBranches, - Boolean allowMembersOfWhitelistedOrgsAsAdmin, - String msgSuccess, - String msgFailure, - String secret, - String commitStatusContext) throws ANTLRException { + String whitelist, + String orgslist, + String cron, + String triggerPhrase, + Boolean onlyTriggerPhrase, + Boolean useGitHubHooks, + Boolean permitAll, + Boolean autoCloseFailedPullRequests, + Boolean displayBuildErrorsOnDownstreamBuilds, + String commentFilePath, + List whiteListTargetBranches, + Boolean allowMembersOfWhitelistedOrgsAsAdmin, + String msgSuccess, + String msgFailure, + String secret, + String commitStatusContext) throws ANTLRException { super(cron); this.adminlist = adminlist; this.whitelist = whitelist; @@ -112,7 +113,11 @@ public static DescriptorImpl getDscp() { @Override public void start(AbstractProject project, boolean newInstance) { - this.project = project.getName(); + if (project.isDisabled()) { + logger.log(Level.FINE, "Project is disabled, not starting trigger"); + return; + } + this.project = project.getFullName(); if (project.getProperty(GithubProjectProperty.class) == null) { logger.log(Level.INFO, "GitHub project not set up, cannot start ghprb trigger for job " + this.project); return; @@ -124,13 +129,14 @@ public void start(AbstractProject project, boolean newInstance) { return; } - logger.log(Level.INFO, "Starting the ghprb trigger for the {0} job; newInstance is {1}", new String[]{this.project, String.valueOf(newInstance)}); + logger.log(Level.INFO, "Starting the ghprb trigger for the {0} job; newInstance is {1}", + new String[] { this.project, String.valueOf(newInstance) }); super.start(project, newInstance); helper.init(); } Ghprb createGhprb(AbstractProject project) { - return new Ghprb(project, this, getDescriptor().getPullRequests(project.getName())); + return new Ghprb(project, this, getDescriptor().getPullRequests(project.getFullName())); } @Override @@ -149,6 +155,17 @@ public void run() { if (getUseGitHubHooks()) { return; } + + if (helper == null) { + logger.log(Level.SEVERE, "Helper is null, unable to run trigger"); + return; + } + + if (helper.isProjectDisabled()) { + logger.log(Level.FINE, "Project is disabled, ignoring trigger run call"); + return; + } + helper.run(); getDescriptor().save(); } @@ -160,12 +177,16 @@ public QueueTaskFuture startJob(GhprbCause cause, GhprbRepository repo) { values.add(new StringParameterValue("ghprbActualCommit", cause.getCommit())); String triggerAuthor = ""; String triggerAuthorEmail = ""; - - try {triggerAuthor = getString(cause.getTriggerSender().getName(), "");} catch (Exception e) {} - try {triggerAuthorEmail = getString(cause.getTriggerSender().getEmail(), "");} catch (Exception e) {} - + + try { + triggerAuthor = getString(cause.getTriggerSender().getName(), ""); + } catch (Exception e) {} + try { + triggerAuthorEmail = getString(cause.getTriggerSender().getEmail(), ""); + } catch (Exception e) {} + setCommitAuthor(cause, values); - + values.add(new StringParameterValue("ghprbTriggerAuthor", triggerAuthor)); values.add(new StringParameterValue("ghprbTriggerAuthorEmail", triggerAuthorEmail)); final StringParameterValue pullIdPv = new StringParameterValue("ghprbPullId", String.valueOf(cause.getPullID())); @@ -182,24 +203,23 @@ public QueueTaskFuture startJob(GhprbCause cause, GhprbRepository repo) { // add the previous pr BuildData as an action so that the correct change log is generated by the GitSCM plugin // note that this will be removed from the Actions list after the job is completed so that the old (and incorrect) // one isn't there - return this.job.scheduleBuild2(job.getQuietPeriod(), cause, - new ParametersAction(values), findPreviousBuildForPullId(pullIdPv)); + return this.job.scheduleBuild2(job.getQuietPeriod(), cause, new ParametersAction(values), findPreviousBuildForPullId(pullIdPv)); } - + private void setCommitAuthor(GhprbCause cause, ArrayList values) { - String authorName = ""; - String authorEmail = ""; - if (cause.getCommitAuthor() != null) { - authorName = getString(cause.getCommitAuthor().getName(), ""); - authorEmail = getString(cause.getCommitAuthor().getEmail(), ""); - } - + String authorName = ""; + String authorEmail = ""; + if (cause.getCommitAuthor() != null) { + authorName = getString(cause.getCommitAuthor().getName(), ""); + authorEmail = getString(cause.getCommitAuthor().getEmail(), ""); + } + values.add(new StringParameterValue("ghprbActualCommitAuthor", authorName)); values.add(new StringParameterValue("ghprbActualCommitAuthorEmail", authorEmail)); } - + private String getString(String actual, String d) { - return actual == null ? d : actual; + return actual == null ? d : actual; } /** @@ -261,10 +281,9 @@ public String getWhitelist() { } return whitelist; } - public String getCommentFilePath() { - return commentFilePath; + return commentFilePath; } public String getOrgslist() { @@ -341,14 +360,13 @@ public DescriptorImpl getDescriptor() { return DESCRIPTOR; } - @VisibleForTesting void setHelper(Ghprb helper) { this.helper = helper; } public GhprbBuilds getBuilds() { - if(helper == null) { + if (helper == null) { logger.log(Level.SEVERE, "The ghprb trigger for {0} wasn''t properly started - helper is null", this.project); return null; } @@ -356,7 +374,7 @@ public GhprbBuilds getBuilds() { } public GhprbRepository getRepository() { - if(helper == null) { + if (helper == null) { logger.log(Level.SEVERE, "The ghprb trigger for {0} wasn''t properly started - helper is null", this.project); return null; } @@ -366,14 +384,13 @@ public GhprbRepository getRepository() { public static final class DescriptorImpl extends TriggerDescriptor { // GitHub username may only contain alphanumeric characters or dashes and cannot begin with a dash private static final Pattern adminlistPattern = Pattern.compile("((\\p{Alnum}[\\p{Alnum}-]*)|\\s)*"); - - + /** - * These settings only really affect testing. When Jenkins calls configure() then - * the formdata will be used to replace all of these fields. - * Leaving them here is useful for testing, but must not be confused with a - * default. They also should not be used as the default value in the global.jelly - * file as this value is dynamic and will not be retained once configure() is called. + * These settings only really affect testing. When Jenkins calls configure() then the formdata will + * be used to replace all of these fields. Leaving them here is useful for + * testing, but must not be confused with a default. They also should not be used as the default + * value in the global.jelly file as this value is dynamic and will not be + * retained once configure() is called. */ private String serverAPIUrl = "https://api.github.com"; private String whitelistPhrase = ".*add\\W+to\\W+whitelist.*"; @@ -391,9 +408,7 @@ public static final class DescriptorImpl extends TriggerDescriptor { private String commitStatusContext = ""; private Boolean autoCloseFailedPullRequests = false; private Boolean displayBuildErrorsOnDownstreamBuilds = false; - - - + private String username; private String password; private String accessToken; @@ -444,7 +459,7 @@ public boolean configure(StaplerRequest req, JSONObject formData) throws FormExc msgSuccess = formData.getString("msgSuccess"); msgFailure = formData.getString("msgFailure"); commitStatusContext = formData.getString("commitStatusContext"); - + save(); gh = new GhprbGitHub(); return super.configure(req, formData); @@ -452,7 +467,8 @@ public boolean configure(StaplerRequest req, JSONObject formData) throws FormExc public FormValidation doCheckAdminlist(@QueryParameter String value) throws ServletException { if (!adminlistPattern.matcher(value).matches()) { - return FormValidation.error("GitHub username may only contain alphanumeric characters or dashes and cannot begin with a dash. Separate them with whitespaces."); + return FormValidation.error("GitHub username may only contain alphanumeric characters or dashes " + + "and cannot begin with a dash. Separate them with whitespaces."); } return FormValidation.ok(); } @@ -502,7 +518,7 @@ public String getOkToTestPhrase() { public String getRetestPhrase() { return retestPhrase; } - + public String getSkipBuildPhrase() { return skipBuildPhrase; } @@ -558,7 +574,7 @@ public boolean isUseComments() { public boolean isUseDetailedComments() { return (useDetailedComments != null && useDetailedComments); } - + public String getCommitStatusContext() { return commitStatusContext; } @@ -570,6 +586,12 @@ public GhprbGitHub getGitHub() { return gh; } + + @VisibleForTesting + void setGitHub(GhprbGitHub gh) { + this.gh = gh; + } + public ConcurrentMap getPullRequests(String projectName) { ConcurrentMap ret; if (jobs.containsKey(projectName)) { @@ -586,10 +608,12 @@ public ConcurrentMap getPullRequests(String projectNa return ret; } - public FormValidation doCreateApiToken(@QueryParameter("username") final String username, @QueryParameter("password") final String password) { + public FormValidation doCreateApiToken(@QueryParameter("username") final String username, + @QueryParameter("password") final String password) { try { GitHub gh = GitHub.connectToEnterprise(this.serverAPIUrl, username, password); - GHAuthorization token = gh.createToken(Arrays.asList(GHAuthorization.REPO_STATUS, GHAuthorization.REPO), "Jenkins GitHub Pull Request Builder", null); + GHAuthorization token = gh.createToken(Arrays.asList(GHAuthorization.REPO_STATUS, + GHAuthorization.REPO), "Jenkins GitHub Pull Request Builder", null); return FormValidation.ok("Access token created: " + token.getToken()); } catch (IOException ex) { return FormValidation.error("GitHub API token couldn't be created: " + ex.getMessage()); diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/HttpConnectorWithJenkinsProxy.java b/src/main/java/org/jenkinsci/plugins/ghprb/HttpConnectorWithJenkinsProxy.java index bbfb3fe95..f44dacf67 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/HttpConnectorWithJenkinsProxy.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/HttpConnectorWithJenkinsProxy.java @@ -7,8 +7,8 @@ import java.net.HttpURLConnection; import java.net.URL; -public class HttpConnectorWithJenkinsProxy implements HttpConnector{ +public class HttpConnectorWithJenkinsProxy implements HttpConnector { public HttpURLConnection connect(URL url) throws IOException { - return (HttpURLConnection)ProxyConfiguration.open(url); + return (HttpURLConnection) ProxyConfiguration.open(url); } } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/manager/GhprbBuildManager.java b/src/main/java/org/jenkinsci/plugins/ghprb/manager/GhprbBuildManager.java index 1c5bf4145..b8109ccbf 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/manager/GhprbBuildManager.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/manager/GhprbBuildManager.java @@ -7,25 +7,25 @@ */ public interface GhprbBuildManager { - /** - * Calculate the build URL of a build - * - * @return the build URL - */ - String calculateBuildUrl(); + /** + * Calculate the build URL of a build + * + * @return the build URL + */ + String calculateBuildUrl(); - /** - * Returns downstream builds as an iterator - * - * @return the iterator - */ - Iterator downstreamProjects(); + /** + * Returns downstream builds as an iterator + * + * @return the iterator + */ + Iterator downstreamProjects(); - /** - * Print tests result of a build - * - * @return the tests result - */ - String getTestResults(); + /** + * Print tests result of a build + * + * @return the tests result + */ + String getTestResults(); } \ No newline at end of file diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/manager/configuration/JobConfiguration.java b/src/main/java/org/jenkinsci/plugins/ghprb/manager/configuration/JobConfiguration.java index e938c3193..db45d1762 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/manager/configuration/JobConfiguration.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/manager/configuration/JobConfiguration.java @@ -5,42 +5,38 @@ */ public class JobConfiguration { - public static PrintStackTrace builder() { - return new JobConfigurationBuilder(); - } + public static PrintStackTrace builder() { + return new JobConfigurationBuilder(); + } - private boolean printStacktrace; + private boolean printStacktrace; - private JobConfiguration() { - } + private JobConfiguration() {} - public boolean printStackTrace() { - return this.printStacktrace; - } + public boolean printStackTrace() { + return this.printStacktrace; + } - public static interface PrintStackTrace { - Build printStackTrace(boolean print); - } + public static interface PrintStackTrace { + Build printStackTrace(boolean print); + } - public static interface Build { - JobConfiguration build(); - } + public static interface Build { + JobConfiguration build(); + } + public static final class JobConfigurationBuilder implements Build, PrintStackTrace { - public static final class JobConfigurationBuilder implements Build, PrintStackTrace { + private JobConfiguration jobConfiguration = new JobConfiguration(); - private JobConfiguration jobConfiguration = new JobConfiguration(); + public JobConfiguration build() { + return jobConfiguration; + } - public JobConfiguration build() { - return jobConfiguration; - } - - public Build printStackTrace(boolean print) { - jobConfiguration.printStacktrace = print; - return this; - } - } + public Build printStackTrace(boolean print) { + jobConfiguration.printStacktrace = print; + return this; + } + } } - - diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/manager/factory/GhprbBuildManagerFactoryUtil.java b/src/main/java/org/jenkinsci/plugins/ghprb/manager/factory/GhprbBuildManagerFactoryUtil.java index 0f0a27d65..ce4f4a5b8 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/manager/factory/GhprbBuildManagerFactoryUtil.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/manager/factory/GhprbBuildManagerFactoryUtil.java @@ -1,6 +1,5 @@ package org.jenkinsci.plugins.ghprb.manager.factory; - import com.cloudbees.plugins.flow.FlowRun; import hudson.model.AbstractBuild; @@ -14,35 +13,28 @@ */ public class GhprbBuildManagerFactoryUtil { - /** - * Gets an instance of a library that is able to calculate build urls - * depending of build type. - * - * If the class representing the build type is not present on the classloader - * then default implementation is returned. - * - * @param build - * @return - */ - public static GhprbBuildManager getBuildManager(AbstractBuild build) { - JobConfiguration jobConfiguration = - JobConfiguration.builder() - .printStackTrace(false) - .build(); - - return getBuildManager(build, jobConfiguration); - } - - public static GhprbBuildManager getBuildManager(AbstractBuild build, JobConfiguration jobConfiguration) { - try { - if (build instanceof FlowRun) { - return new BuildFlowBuildManager(build, jobConfiguration); - } - } - catch (NoClassDefFoundError ncdfe) { - } - - return new GhprbDefaultBuildManager(build); - } + /** + * Gets an instance of a library that is able to calculate build urls depending of build type. + * + * If the class representing the build type is not present on the classloader then default implementation is returned. + * + * @param build + * @return + */ + public static GhprbBuildManager getBuildManager(AbstractBuild build) { + JobConfiguration jobConfiguration = JobConfiguration.builder().printStackTrace(false).build(); + + return getBuildManager(build, jobConfiguration); + } + + public static GhprbBuildManager getBuildManager(AbstractBuild build, JobConfiguration jobConfiguration) { + try { + if (build instanceof FlowRun) { + return new BuildFlowBuildManager(build, jobConfiguration); + } + } catch (NoClassDefFoundError ncdfe) {} + + return new GhprbDefaultBuildManager(build); + } } \ No newline at end of file diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/manager/impl/GhprbBaseBuildManager.java b/src/main/java/org/jenkinsci/plugins/ghprb/manager/impl/GhprbBaseBuildManager.java index 45dd5c237..448e67515 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/manager/impl/GhprbBaseBuildManager.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/manager/impl/GhprbBaseBuildManager.java @@ -1,6 +1,5 @@ package org.jenkinsci.plugins.ghprb.manager.impl; - import java.io.IOException; import java.util.ArrayList; import java.util.Iterator; @@ -27,166 +26,155 @@ */ public abstract class GhprbBaseBuildManager implements GhprbBuildManager { - public GhprbBaseBuildManager(AbstractBuild build) { - this.build = build; - this.jobConfiguration = buildDefaultConfiguration(); - } - - public GhprbBaseBuildManager(AbstractBuild build, JobConfiguration jobConfiguration) { - this.build = build; - this.jobConfiguration = jobConfiguration; - } - - private JobConfiguration buildDefaultConfiguration() { - return JobConfiguration.builder() - .printStackTrace(false) - .build(); - } - - /** - * Calculate the build URL of a build of default type. This will be overriden - * by specific build types. - * - * @return the build URL of a build of default type - */ - public String calculateBuildUrl() { - String publishedURL = GhprbTrigger.getDscp().getPublishedURL(); - - return publishedURL + "/" + build.getUrl(); - } - - /** - * Return a downstream iterator of a build of default type. This will be overriden - * by specific build types. - * - * If the receiver of the call has no child projects, it will return an - * iterator over itself - * - * @return the downstream builds as an iterator - */ - public Iterator downstreamProjects() { - List downstreamList = new ArrayList(); - - downstreamList.add(build); - - return downstreamList.iterator(); - } - - public JobConfiguration getJobConfiguration() { - return this.jobConfiguration; - } - - /** - * Return the tests results of a build of default type. This will be overriden - * by specific build types. - * - * @return the tests result of a build of default type - */ - public String getTestResults() { - return getAggregatedTestResults(build); - } - - protected String getAggregatedTestResults(AbstractBuild build) { - AggregatedTestResultAction testResultAction = - build.getAction(AggregatedTestResultAction.class); - - if (testResultAction == null) { - return ""; - } - - StringBuilder sb = new StringBuilder(); - - if (build.getResult() != Result.UNSTABLE) { - sb.append("

Build result: "); - sb.append(build.getResult().toString()); - sb.append("

"); - - try { - List buildLog = build.getLog(_MAX_LINES_COUNT); - - for (String buildLogLine : buildLog) { - sb.append(buildLogLine); - } - } - catch(IOException ioe) { - LOGGER.log(Level.WARNING, ioe.getMessage()); - } - - return sb.toString(); - } - - sb.append("

Failed Tests: "); - sb.append(""); - sb.append(testResultAction.getFailCount()); - sb.append("

"); - - List childReports = testResultAction.getChildReports(); - - for (ChildReport report : childReports) { - TestResult result = (TestResult)report.result; - - if (result.getFailCount() < 1) { - continue; - } - - AbstractProject project = - (AbstractProject)report.child.getProject(); - - String baseUrl = Jenkins.getInstance().getRootUrl() + build.getUrl() + - project.getShortUrl() + "testReport"; - - sb.append("

"); - sb.append(""); - sb.append(""); - sb.append(project.getName()); - sb.append(""); - sb.append(": "); - sb.append(""); - sb.append(result.getFailCount()); - sb.append("

"); - - sb.append(""); - } - - return sb.toString(); - } - - protected static final Logger LOGGER = Logger.getLogger( - GhprbBaseBuildManager.class.getName()); - - protected AbstractBuild build; - - private static final int _MAX_LINES_COUNT = 25; - - private JobConfiguration jobConfiguration; + public GhprbBaseBuildManager(AbstractBuild build) { + this.build = build; + this.jobConfiguration = buildDefaultConfiguration(); + } + + public GhprbBaseBuildManager(AbstractBuild build, JobConfiguration jobConfiguration) { + this.build = build; + this.jobConfiguration = jobConfiguration; + } + + private JobConfiguration buildDefaultConfiguration() { + return JobConfiguration.builder().printStackTrace(false).build(); + } + + /** + * Calculate the build URL of a build of default type. This will be overriden by specific build types. + * + * @return the build URL of a build of default type + */ + public String calculateBuildUrl() { + String publishedURL = GhprbTrigger.getDscp().getPublishedURL(); + + return publishedURL + "/" + build.getUrl(); + } + + /** + * Return a downstream iterator of a build of default type. This will be overriden by specific build types. + * + * If the receiver of the call has no child projects, it will return an iterator over itself + * + * @return the downstream builds as an iterator + */ + public Iterator downstreamProjects() { + List downstreamList = new ArrayList(); + + downstreamList.add(build); + + return downstreamList.iterator(); + } + + public JobConfiguration getJobConfiguration() { + return this.jobConfiguration; + } + + /** + * Return the tests results of a build of default type. This will be overriden by specific build types. + * + * @return the tests result of a build of default type + */ + public String getTestResults() { + return getAggregatedTestResults(build); + } + + protected String getAggregatedTestResults(AbstractBuild build) { + AggregatedTestResultAction testResultAction = build.getAction(AggregatedTestResultAction.class); + + if (testResultAction == null) { + return ""; + } + + StringBuilder sb = new StringBuilder(); + + if (build.getResult() != Result.UNSTABLE) { + sb.append("

Build result: "); + sb.append(build.getResult().toString()); + sb.append("

"); + + try { + List buildLog = build.getLog(_MAX_LINES_COUNT); + + for (String buildLogLine : buildLog) { + sb.append(buildLogLine); + } + } catch (IOException ioe) { + LOGGER.log(Level.WARNING, ioe.getMessage()); + } + + return sb.toString(); + } + + sb.append("

Failed Tests: "); + sb.append(""); + sb.append(testResultAction.getFailCount()); + sb.append("

"); + + List childReports = testResultAction.getChildReports(); + + for (ChildReport report : childReports) { + TestResult result = (TestResult) report.result; + + if (result.getFailCount() < 1) { + continue; + } + + AbstractProject project = (AbstractProject) report.child.getProject(); + + String baseUrl = Jenkins.getInstance().getRootUrl() + build.getUrl() + project.getShortUrl() + "testReport"; + + sb.append("

"); + sb.append(""); + sb.append(""); + sb.append(project.getFullName()); + sb.append(""); + sb.append(": "); + sb.append(""); + sb.append(result.getFailCount()); + sb.append("

"); + + sb.append(""); + } + + return sb.toString(); + } + + protected static final Logger LOGGER = Logger.getLogger(GhprbBaseBuildManager.class.getName()); + + protected AbstractBuild build; + + private static final int _MAX_LINES_COUNT = 25; + + private JobConfiguration jobConfiguration; } \ No newline at end of file diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/manager/impl/GhprbDefaultBuildManager.java b/src/main/java/org/jenkinsci/plugins/ghprb/manager/impl/GhprbDefaultBuildManager.java index c23cd6508..6c823d80f 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/manager/impl/GhprbDefaultBuildManager.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/manager/impl/GhprbDefaultBuildManager.java @@ -7,8 +7,8 @@ */ public class GhprbDefaultBuildManager extends GhprbBaseBuildManager { - public GhprbDefaultBuildManager(AbstractBuild build) { - super(build); - } + public GhprbDefaultBuildManager(AbstractBuild build) { + super(build); + } } \ No newline at end of file diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/manager/impl/downstreambuilds/BuildFlowBuildManager.java b/src/main/java/org/jenkinsci/plugins/ghprb/manager/impl/downstreambuilds/BuildFlowBuildManager.java index 682c3bfe5..c48df1178 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/manager/impl/downstreambuilds/BuildFlowBuildManager.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/manager/impl/downstreambuilds/BuildFlowBuildManager.java @@ -20,90 +20,86 @@ */ public class BuildFlowBuildManager extends GhprbBaseBuildManager { - private static final Logger logger = Logger.getLogger(BuildFlowBuildManager.class.getName()); - - public BuildFlowBuildManager(AbstractBuild build) { - super(build); - } - - public BuildFlowBuildManager(AbstractBuild build, JobConfiguration jobConfiguration) { - super(build, jobConfiguration); - } - - /** - * Calculate the build URL of a build of BuildFlow type, traversing its - * downstream builds graph - * - * @return the build URL of a BuildFlow build, with all its downstream builds - */ - @Override - public String calculateBuildUrl() { - Iterator iterator = downstreamProjects(); - - StringBuilder sb = new StringBuilder(); - - while (iterator.hasNext()) { - JobInvocation jobInvocation = iterator.next(); - - sb.append("\n"); - sb.append(""); - sb.append(jobInvocation.getBuildUrl()); - sb.append(""); - } - - return sb.toString(); - } - - /** - * Return a downstream iterator of a build of default type. This will be overriden - * by specific build types. - * - * @return the downstream builds as an iterator - */ - @Override - public Iterator downstreamProjects() { - FlowRun flowRun = (FlowRun) build; - - DirectedGraph directedGraph = flowRun.getJobsGraph(); - - return directedGraph.vertexSet().iterator(); - } - - /** - * Return the tests results of a build of default type. This will be overriden - * by specific build types. - * - * @return the tests result of a build of default type - */ - @Override - public String getTestResults() { - Iterator iterator = downstreamProjects(); - - StringBuilder sb = new StringBuilder(); - - while (iterator.hasNext()) { - JobInvocation jobInvocation = iterator.next(); - - try { - AbstractBuild build = (AbstractBuild)jobInvocation.getBuild(); - - AggregatedTestResultAction testResultAction = - build.getAction(AggregatedTestResultAction.class); - - if (testResultAction != null) { - sb.append("\n"); - sb.append(jobInvocation.getBuildUrl()); - sb.append("\n"); - sb.append(getAggregatedTestResults(build)); - } - } catch (Exception e) { - logger.log(Level.SEVERE, "Job execution has failed", e); - } - } - - return sb.toString(); - } + private static final Logger logger = Logger.getLogger(BuildFlowBuildManager.class.getName()); + + public BuildFlowBuildManager(AbstractBuild build) { + super(build); + } + + public BuildFlowBuildManager(AbstractBuild build, JobConfiguration jobConfiguration) { + super(build, jobConfiguration); + } + + /** + * Calculate the build URL of a build of BuildFlow type, traversing its downstream builds graph + * + * @return the build URL of a BuildFlow build, with all its downstream builds + */ + @Override + public String calculateBuildUrl() { + Iterator iterator = downstreamProjects(); + + StringBuilder sb = new StringBuilder(); + + while (iterator.hasNext()) { + JobInvocation jobInvocation = iterator.next(); + + sb.append("\n"); + sb.append(""); + sb.append(jobInvocation.getBuildUrl()); + sb.append(""); + } + + return sb.toString(); + } + + /** + * Return a downstream iterator of a build of default type. This will be overriden by specific build types. + * + * @return the downstream builds as an iterator + */ + @Override + public Iterator downstreamProjects() { + FlowRun flowRun = (FlowRun) build; + + DirectedGraph directedGraph = flowRun.getJobsGraph(); + + return directedGraph.vertexSet().iterator(); + } + + /** + * Return the tests results of a build of default type. This will be overriden by specific build types. + * + * @return the tests result of a build of default type + */ + @Override + public String getTestResults() { + Iterator iterator = downstreamProjects(); + + StringBuilder sb = new StringBuilder(); + + while (iterator.hasNext()) { + JobInvocation jobInvocation = iterator.next(); + + try { + AbstractBuild build = (AbstractBuild) jobInvocation.getBuild(); + + AggregatedTestResultAction testResultAction = build.getAction(AggregatedTestResultAction.class); + + if (testResultAction != null) { + sb.append("\n"); + sb.append(jobInvocation.getBuildUrl()); + sb.append("\n"); + sb.append(getAggregatedTestResults(build)); + } + } catch (Exception e) { + logger.log(Level.SEVERE, "Job execution has failed", e); + } + } + + return sb.toString(); + } } diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbTrigger/global.jelly b/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbTrigger/global.jelly index 3690dc649..2d3b14f42 100644 --- a/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbTrigger/global.jelly +++ b/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbTrigger/global.jelly @@ -39,7 +39,7 @@ - + diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbIT.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbIT.java index 1433688ff..e82bcab84 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbIT.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbIT.java @@ -13,6 +13,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.jvnet.hudson.test.JenkinsRule; +import org.kohsuke.github.GHCommitState; import org.kohsuke.github.GHIssueComment; import org.mockito.runners.MockitoJUnitRunner; @@ -22,6 +23,9 @@ import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Matchers.any; @RunWith(MockitoJUnitRunner.class) public class GhprbIT extends GhprbITBaseTestCase { @@ -29,24 +33,22 @@ public class GhprbIT extends GhprbITBaseTestCase { @Rule public JenkinsRule jenkinsRule = new JenkinsRule(); - @Before - public void setUp() throws Exception { -// GhprbTestUtil.mockGithubUserPage(); - super.beforeTest(); - } + @Before + public void setUp() throws Exception { + // GhprbTestUtil.mockGithubUserPage(); + super.beforeTest(); + } @Test public void shouldBuildTriggersOnNewPR() throws Exception { // GIVEN FreeStyleProject project = jenkinsRule.createFreeStyleProject("PRJ"); - GhprbTrigger trigger = new GhprbTrigger( - "user", "user", "", "*/1 * * * *", "retest this please", false, false, false, false, false, null, null, false, null, null, null, null - ); + GhprbTrigger trigger = GhprbTestUtil.getTrigger(null); given(commitPointer.getSha()).willReturn("sha"); JSONObject jsonObject = GhprbTestUtil.provideConfiguration(); GhprbTrigger.getDscp().configure(null, jsonObject); project.addProperty(new GithubProjectProperty("https://github.com/user/dropwizard")); - given(ghPullRequest.getNumber()).willReturn(1); + given(ghPullRequest.getNumber()).willReturn(1); // Creating spy on ghprb, configuring repo Ghprb ghprb = spy(trigger.createGhprb(project)); @@ -64,8 +66,8 @@ public void shouldBuildTriggersOnNewPR() throws Exception { trigger.start(project, true); trigger.setHelper(ghprb); - // THEN - Thread.sleep(65000); + GhprbTestUtil.triggerRunAndWait(10, trigger, project); + assertThat(project.getBuilds().toArray().length).isEqualTo(1); } @@ -73,11 +75,9 @@ public void shouldBuildTriggersOnNewPR() throws Exception { public void shouldBuildTriggersOnUpdatingNewCommitsPR() throws Exception { // GIVEN FreeStyleProject project = jenkinsRule.createFreeStyleProject("PRJ"); - GhprbTrigger trigger = new GhprbTrigger( - "user", "user", "", "*/1 * * * *", "retest this please", false, false, false, false, false, null, null, false, null, null, null, null - ); + GhprbTrigger trigger = GhprbTestUtil.getTrigger(null); given(commitPointer.getSha()).willReturn("sha").willReturn("sha").willReturn("newOne").willReturn("newOne"); - given(ghPullRequest.getComments()).willReturn(Lists.newArrayList()); + given(ghPullRequest.getComments()).willReturn(Lists. newArrayList()); JSONObject jsonObject = GhprbTestUtil.provideConfiguration(); GhprbTrigger.getDscp().configure(null, jsonObject); project.addProperty(new GithubProjectProperty("https://github.com/user/dropwizard")); @@ -92,8 +92,8 @@ public void shouldBuildTriggersOnUpdatingNewCommitsPR() throws Exception { GitSCM scm = GhprbTestUtil.provideGitSCM(); project.setScm(scm); - // THEN - Thread.sleep(130000); + GhprbTestUtil.triggerRunAndWait(10, trigger, project); + assertThat(project.getBuilds().toArray().length).isEqualTo(2); } @@ -101,9 +101,7 @@ public void shouldBuildTriggersOnUpdatingNewCommitsPR() throws Exception { public void shouldBuildTriggersOnUpdatingRetestMessagePR() throws Exception { // GIVEN FreeStyleProject project = jenkinsRule.createFreeStyleProject("PRJ"); - GhprbTrigger trigger = new GhprbTrigger( - "user", "user", "", "*/1 * * * *", "retest this please", false, false, false, false, false, null, null, false, null, null, null, null - ); + GhprbTrigger trigger = GhprbTestUtil.getTrigger(null); given(commitPointer.getSha()).willReturn("sha"); @@ -112,7 +110,7 @@ public void shouldBuildTriggersOnUpdatingRetestMessagePR() throws Exception { given(comment.getUpdatedAt()).willReturn(new DateTime().plusDays(1).toDate()); given(comment.getUser()).willReturn(ghUser); given(ghPullRequest.getComments()).willReturn(newArrayList(comment)); - given(ghPullRequest.getNumber()).willReturn(5).willReturn(5).willReturn(6).willReturn(6); + given(ghPullRequest.getNumber()).willReturn(5).willReturn(5); JSONObject jsonObject = GhprbTestUtil.provideConfiguration(); GhprbTrigger.getDscp().configure(null, jsonObject); project.addProperty(new GithubProjectProperty("https://github.com/user/dropwizard")); @@ -127,9 +125,45 @@ public void shouldBuildTriggersOnUpdatingRetestMessagePR() throws Exception { GitSCM scm = GhprbTestUtil.provideGitSCM(); project.setScm(scm); - // THEN - Thread.sleep(130000); + GhprbTestUtil.triggerRunAndWait(10, trigger, project); assertThat(project.getBuilds().toArray().length).isEqualTo(2); } + + + @Test + public void shouldNotBuildDisabledBuild() throws Exception { + // GIVEN + FreeStyleProject project = jenkinsRule.createFreeStyleProject("PRJ"); + GhprbTrigger trigger = GhprbTestUtil.getTrigger(null); + + given(commitPointer.getSha()).willReturn("sha"); + + GHIssueComment comment = mock(GHIssueComment.class); + given(comment.getBody()).willReturn("retest this please"); + given(comment.getUpdatedAt()).willReturn(new DateTime().plusDays(1).toDate()); + given(comment.getUser()).willReturn(ghUser); + given(ghPullRequest.getComments()).willReturn(newArrayList(comment)); + given(ghPullRequest.getNumber()).willReturn(5); + JSONObject jsonObject = GhprbTestUtil.provideConfiguration(); + GhprbTrigger.getDscp().configure(null, jsonObject); + project.addProperty(new GithubProjectProperty("https://github.com/user/dropwizard")); + + Ghprb ghprb = spy(trigger.createGhprb(project)); + doReturn(ghprbGitHub).when(ghprb).getGitHub(); + trigger.start(project, true); + trigger.setHelper(ghprb); + ghprb.getRepository().setHelper(ghprb); + project.addTrigger(trigger); + project.getTriggers().keySet().iterator().next().configure(null, jsonObject); + GitSCM scm = GhprbTestUtil.provideGitSCM(); + project.setScm(scm); + + project.disable(); + + GhprbTestUtil.triggerRunAndWait(10, trigger, project); + assertThat(project.getBuilds().toArray().length).isEqualTo(0); + + verify(ghRepository, times(0)).createCommitStatus(any(String.class), any(GHCommitState.class), any(String.class), any(String.class)); + } } diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbITBaseTestCase.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbITBaseTestCase.java index 56b892339..c0bb1a97b 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbITBaseTestCase.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbITBaseTestCase.java @@ -1,28 +1,19 @@ package org.jenkinsci.plugins.ghprb; import static com.google.common.collect.Lists.newArrayList; - import static org.kohsuke.github.GHIssueState.OPEN; - import static org.mockito.BDDMockito.given; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; - -import antlr.ANTLRException; - import hudson.model.AbstractProject; -import java.io.IOException; - import org.joda.time.DateTime; - import org.kohsuke.github.GHCommitPointer; import org.kohsuke.github.GHPullRequest; import org.kohsuke.github.GHRateLimit; import org.kohsuke.github.GHRepository; import org.kohsuke.github.GHUser; import org.kohsuke.github.GitHub; - import org.mockito.Mock; import org.mockito.Mockito; @@ -31,58 +22,54 @@ */ public abstract class GhprbITBaseTestCase { - @Mock - protected GHCommitPointer commitPointer; - @Mock - protected GHPullRequest ghPullRequest; - @Mock - protected GhprbGitHub ghprbGitHub; - @Mock - protected GHRepository ghRepository; - @Mock - protected GHUser ghUser; - @Mock - protected GitHub gitHub; - - // Stubs - protected GHRateLimit ghRateLimit = new GHRateLimit(); - - protected void beforeTest() throws IOException, ANTLRException { - given(ghprbGitHub.get()).willReturn(gitHub); - given(gitHub.getRateLimit()).willReturn(ghRateLimit); - given(gitHub.getRepository(anyString())).willReturn(ghRepository); - given(commitPointer.getRef()).willReturn("ref"); - given(ghRepository.getName()).willReturn("dropwizard"); - - GhprbTestUtil.mockPR( - ghPullRequest, commitPointer, new DateTime(), - new DateTime().plusDays(1)); - - given(ghRepository.getPullRequests(eq(OPEN))) - .willReturn(newArrayList(ghPullRequest)) - .willReturn(newArrayList(ghPullRequest)); - - given(ghPullRequest.getUser()).willReturn(ghUser); - given(ghUser.getEmail()).willReturn("email@email.com"); - given(ghUser.getLogin()).willReturn("user"); - - ghRateLimit.remaining = GhprbTestUtil.INITIAL_RATE_LIMIT; - - GhprbTestUtil.mockCommitList(ghPullRequest); - } - - protected void setRepositoryHelper(Ghprb ghprb) { - ghprb.getRepository().setHelper(ghprb); - } - - protected void setTriggerHelper(GhprbTrigger trigger, Ghprb ghprb) { - trigger.setHelper(ghprb); - } - - protected Ghprb spyCreatingGhprb( - GhprbTrigger trigger, AbstractProject project) { - - return Mockito.spy(trigger.createGhprb(project)); - } + @Mock + protected GHCommitPointer commitPointer; + @Mock + protected GHPullRequest ghPullRequest; + @Mock + protected GhprbGitHub ghprbGitHub; + @Mock + protected GHRepository ghRepository; + @Mock + protected GHUser ghUser; + @Mock + protected GitHub gitHub; + + // Stubs + protected GHRateLimit ghRateLimit = new GHRateLimit(); + + protected void beforeTest() throws Exception { + given(ghprbGitHub.get()).willReturn(gitHub); + given(gitHub.getRateLimit()).willReturn(ghRateLimit); + given(gitHub.getRepository(anyString())).willReturn(ghRepository); + given(commitPointer.getRef()).willReturn("ref"); + given(ghRepository.getName()).willReturn("dropwizard"); + + GhprbTestUtil.mockPR(ghPullRequest, commitPointer, new DateTime(), new DateTime().plusDays(1)); + + given(ghRepository.getPullRequests(eq(OPEN))).willReturn(newArrayList(ghPullRequest)).willReturn(newArrayList(ghPullRequest)); + + given(ghPullRequest.getUser()).willReturn(ghUser); + given(ghUser.getEmail()).willReturn("email@email.com"); + given(ghUser.getLogin()).willReturn("user"); + + ghRateLimit.remaining = GhprbTestUtil.INITIAL_RATE_LIMIT; + + GhprbTestUtil.mockCommitList(ghPullRequest); + } + + protected void setRepositoryHelper(Ghprb ghprb) { + ghprb.getRepository().setHelper(ghprb); + } + + protected void setTriggerHelper(GhprbTrigger trigger, Ghprb ghprb) { + trigger.setHelper(ghprb); + } + + protected Ghprb spyCreatingGhprb(GhprbTrigger trigger, AbstractProject project) { + + return Mockito.spy(trigger.createGhprb(project)); + } + } \ No newline at end of file diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestMergeTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestMergeTest.java index e6c9ad0a4..8b7b5aa20 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestMergeTest.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestMergeTest.java @@ -10,6 +10,7 @@ import hudson.model.AbstractBuild; import hudson.model.FreeStyleBuild; +import hudson.model.ItemGroup; import hudson.model.StreamBuildListener; import hudson.model.FreeStyleProject; import hudson.model.Run; @@ -46,266 +47,259 @@ @RunWith(MockitoJUnitRunner.class) public class GhprbPullRequestMergeTest { - + @Rule public JenkinsRule jenkinsRule = new JenkinsRule(); - private FreeStyleProject project = mock(FreeStyleProject.class); - private AbstractBuild build = mock(FreeStyleBuild.class); - - @Mock - private GhprbPullRequest pullRequest; - @Mock - private GHPullRequest pr; - + private FreeStyleProject project = mock(FreeStyleProject.class); + private AbstractBuild build = mock(FreeStyleBuild.class); + + @Mock + private GhprbPullRequest pullRequest; + @Mock + private GHPullRequest pr; + @Mock private GitUser committer; - @Mock + @Mock private GHUser triggerSender; @Mock private GhprbCause cause; @Mock private Ghprb helper; - @Mock + @Mock private GhprbRepository repo; - + @Mock private StreamBuildListener listener; - - + + @Mock + private ItemGroup parent; + private final String triggerPhrase = "ok to merge"; private final String nonTriggerPhrase = "This phrase is not the trigger phrase"; - + private final String adminList = "admin"; - + private final String adminLogin = "admin"; private final String nonAdminLogin = "nonadmin"; - + private final String committerName = "committer"; private final String nonCommitterName = "noncommitter"; - + private final String mergeComment = "merge"; - + private final Integer pullId = 1; - - - - @Before - public void beforeTest() throws Exception { - GhprbTrigger trigger = spy(new GhprbTrigger(adminList, "user", "", "*/1 * * * *", triggerPhrase, false, false, false, false, false, null, null, false, null, null, null, null)); - - ConcurrentMap pulls = new ConcurrentHashMap(1); - pulls.put(pullId, pullRequest); - Map> jobs = new HashMap>(1); - jobs.put("project", pulls); - - GithubProjectProperty projectProperty = new GithubProjectProperty("https://github.com/jenkinsci/ghprb-plugin"); - DescriptorImpl descriptor = trigger.getDescriptor(); - - PrintStream logger = mock(PrintStream.class); + private Map triggerValues; + + @Before + public void beforeTest() throws Exception { + triggerValues = new HashMap(10); + triggerValues.put("adminlist", adminList); + triggerValues.put("triggerPhrase", triggerPhrase); - given(project.getTrigger(GhprbTrigger.class)).willReturn(trigger); - given(project.getName()).willReturn("project"); - given(project.getProperty(GithubProjectProperty.class)).willReturn(projectProperty); - - given(build.getCause(GhprbCause.class)).willReturn(cause); - given(build.getResult()).willReturn(Result.SUCCESS); - given(build.getParent()).willCallRealMethod(); - - given(pullRequest.getPullRequest()).willReturn(pr); - - + GhprbTrigger trigger = spy(GhprbTestUtil.getTrigger(triggerValues)); + + ConcurrentMap pulls = new ConcurrentHashMap(1); + pulls.put(pullId, pullRequest); + Map> jobs = new HashMap>(1); + jobs.put("project", pulls); + + GithubProjectProperty projectProperty = new GithubProjectProperty("https://github.com/jenkinsci/ghprb-plugin"); + DescriptorImpl descriptor = trigger.getDescriptor(); + + PrintStream logger = mock(PrintStream.class); + + given(parent.getFullName()).willReturn(""); + + given(project.getParent()).willReturn(parent); + given(project.getTrigger(GhprbTrigger.class)).willReturn(trigger); + given(project.getName()).willReturn("project"); + given(project.getProperty(GithubProjectProperty.class)).willReturn(projectProperty); + given(project.isDisabled()).willReturn(false); + + given(build.getCause(GhprbCause.class)).willReturn(cause); + given(build.getResult()).willReturn(Result.SUCCESS); + given(build.getParent()).willCallRealMethod(); + + given(pullRequest.getPullRequest()).willReturn(pr); + given(cause.getPullID()).willReturn(pullId); given(cause.isMerged()).willReturn(true); given(cause.getTriggerSender()).willReturn(triggerSender); given(cause.getCommitAuthor()).willReturn(committer); - - + given(listener.getLogger()).willReturn(logger); - - + doNothing().when(repo).addComment(anyInt(), anyString()); doNothing().when(logger).println(); - - - Field parentField = Run.class.getDeclaredField("project"); - parentField.setAccessible(true); - parentField.set(build, project); - - - - Field jobsField = descriptor.getClass().getDeclaredField("jobs"); - jobsField.setAccessible(true); - jobsField.set(descriptor, jobs); - + + Field parentField = Run.class.getDeclaredField("project"); + parentField.setAccessible(true); + parentField.set(build, project); + + Field jobsField = descriptor.getClass().getDeclaredField("jobs"); + jobsField.setAccessible(true); + jobsField.set(descriptor, jobs); helper = spy(new Ghprb(project, trigger, pulls)); - trigger.setHelper(helper); + trigger.setHelper(helper); given(helper.getRepository()).willReturn(repo); - given(helper.isBotUser(any(GHUser.class))).willReturn(false); - } - - @After - public void afterClass() { - - } - - - private void setupConditions(String triggerLogin, String committerName, String comment) throws IOException { - given(triggerSender.getLogin()).willReturn(triggerLogin); - given(triggerSender.getName()).willReturn(committerName); - given(committer.getName()).willReturn(this.committerName); - - PagedIterator itr = Mockito.mock(PagedIterator.class); - PagedIterable pagedItr = Mockito.mock(PagedIterable.class); - - Commit commit = mock(Commit.class); - GHPullRequestCommitDetail commitDetail = mock(GHPullRequestCommitDetail.class); - - - given(pr.listCommits()).willReturn(pagedItr); - - given(pagedItr.iterator()).willReturn(itr); - - given(itr.hasNext()).willReturn(true, false); - given(itr.next()).willReturn(commitDetail); - - given(commitDetail.getCommit()).willReturn(commit); - given(commit.getCommitter()).willReturn(committer); - - given(cause.getCommentBody()).willReturn(comment); - } - - private GhprbPullRequestMerge setupMerger(boolean onlyTriggerPhrase, boolean onlyAdminsMerge, boolean disallowOwnCode) { - GhprbPullRequestMerge merger = spy(new GhprbPullRequestMerge(mergeComment, onlyTriggerPhrase, onlyAdminsMerge, disallowOwnCode)); - - merger.setHelper(helper); - - return merger; - } - - - @Test - public void testApproveMerge() throws Exception { - - boolean onlyTriggerPhrase = false; - boolean onlyAdminsMerge = false; - boolean disallowOwnCode = false; - - GhprbPullRequestMerge merger = setupMerger(onlyTriggerPhrase, onlyAdminsMerge, disallowOwnCode); - - setupConditions(nonAdminLogin, committerName, triggerPhrase); - assertThat(merger.perform(build, null, listener)).isEqualTo(true); - - setupConditions(adminLogin, nonCommitterName, triggerPhrase); - assertThat(merger.perform(build, null, listener)).isEqualTo(true); - - setupConditions(adminLogin, committerName, nonTriggerPhrase); - assertThat(merger.perform(build, null, listener)).isEqualTo(true); - - setupConditions(nonAdminLogin, nonCommitterName, triggerPhrase); - assertThat(merger.perform(build, null, listener)).isEqualTo(true); - - setupConditions(nonAdminLogin, nonCommitterName, nonTriggerPhrase); - assertThat(merger.perform(build, null, listener)).isEqualTo(true); - - setupConditions(adminLogin, nonCommitterName, nonTriggerPhrase); - assertThat(merger.perform(build, null, listener)).isEqualTo(true); - - setupConditions(nonAdminLogin, nonCommitterName, nonTriggerPhrase); - assertThat(merger.perform(build, null, listener)).isEqualTo(true); - - setupConditions(adminLogin, committerName, triggerPhrase); - assertThat(merger.perform(build, null, listener)).isEqualTo(true); - } - - - - @Test - public void testAdminMerge() throws Exception { - - boolean onlyTriggerPhrase = false; - boolean onlyAdminsMerge = true; - boolean disallowOwnCode = false; - - GhprbPullRequestMerge merger = setupMerger(onlyTriggerPhrase, onlyAdminsMerge, disallowOwnCode); - - setupConditions(adminLogin, committerName, triggerPhrase); - assertThat(merger.perform(build, null, listener)).isEqualTo(true); - - setupConditions(nonAdminLogin, committerName, triggerPhrase); - assertThat(merger.perform(build, null, listener)).isEqualTo(false); - } - - @Test - public void testTriggerMerge() throws Exception { - - boolean onlyTriggerPhrase = true; - boolean onlyAdminsMerge = false; - boolean disallowOwnCode = false; - - - GhprbPullRequestMerge merger = setupMerger(onlyTriggerPhrase, onlyAdminsMerge, disallowOwnCode); - - setupConditions(adminLogin, committerName, triggerPhrase); - assertThat(merger.perform(build, null, listener)).isEqualTo(true); - - setupConditions(adminLogin, committerName, nonTriggerPhrase); - assertThat(merger.perform(build, null, listener)).isEqualTo(false); - } - - - @Test - public void testOwnCodeMerge() throws Exception { - - boolean onlyTriggerPhrase = false; - boolean onlyAdminsMerge = false; - boolean disallowOwnCode = true; - - GhprbPullRequestMerge merger = setupMerger(onlyTriggerPhrase, onlyAdminsMerge, disallowOwnCode); - - setupConditions(adminLogin, nonCommitterName, triggerPhrase); - assertThat(merger.perform(build, null, listener)).isEqualTo(true); - - setupConditions(adminLogin, committerName, triggerPhrase); - assertThat(merger.perform(build, null, listener)).isEqualTo(false); - } - - - @Test - public void testDenyMerge() throws Exception { - - boolean onlyTriggerPhrase = true; - boolean onlyAdminsMerge = true; - boolean disallowOwnCode = true; - - GhprbPullRequestMerge merger = setupMerger(onlyTriggerPhrase, onlyAdminsMerge, disallowOwnCode); - - setupConditions(nonAdminLogin, nonCommitterName, triggerPhrase); - assertThat(merger.perform(build, null, listener)).isEqualTo(false); - - setupConditions(adminLogin, committerName, triggerPhrase); - assertThat(merger.perform(build, null, listener)).isEqualTo(false); - - setupConditions(adminLogin, nonCommitterName, nonTriggerPhrase); - assertThat(merger.perform(build, null, listener)).isEqualTo(false); - - setupConditions(nonAdminLogin, nonCommitterName, triggerPhrase); - assertThat(merger.perform(build, null, listener)).isEqualTo(false); - - setupConditions(nonAdminLogin, nonCommitterName, nonTriggerPhrase); - assertThat(merger.perform(build, null, listener)).isEqualTo(false); - - setupConditions(adminLogin, committerName, nonTriggerPhrase); - assertThat(merger.perform(build, null, listener)).isEqualTo(false); - - setupConditions(nonAdminLogin, committerName, nonTriggerPhrase); - assertThat(merger.perform(build, null, listener)).isEqualTo(false); - - setupConditions(adminLogin, nonCommitterName, triggerPhrase); - assertThat(merger.perform(build, null, listener)).isEqualTo(true); - } - + given(helper.isBotUser(any(GHUser.class))).willReturn(false); + } + + @After + public void afterClass() { + + } + + private void setupConditions(String triggerLogin, String committerName, String comment) throws IOException { + given(triggerSender.getLogin()).willReturn(triggerLogin); + given(triggerSender.getName()).willReturn(committerName); + given(committer.getName()).willReturn(this.committerName); + + PagedIterator itr = Mockito.mock(PagedIterator.class); + PagedIterable pagedItr = Mockito.mock(PagedIterable.class); + + Commit commit = mock(Commit.class); + GHPullRequestCommitDetail commitDetail = mock(GHPullRequestCommitDetail.class); + + given(pr.listCommits()).willReturn(pagedItr); + + given(pagedItr.iterator()).willReturn(itr); + + given(itr.hasNext()).willReturn(true, false); + given(itr.next()).willReturn(commitDetail); + + given(commitDetail.getCommit()).willReturn(commit); + given(commit.getCommitter()).willReturn(committer); + + given(cause.getCommentBody()).willReturn(comment); + } + + private GhprbPullRequestMerge setupMerger(boolean onlyTriggerPhrase, boolean onlyAdminsMerge, boolean disallowOwnCode) { + GhprbPullRequestMerge merger = spy(new GhprbPullRequestMerge(mergeComment, onlyTriggerPhrase, onlyAdminsMerge, disallowOwnCode)); + + merger.setHelper(helper); + + return merger; + } + + @Test + public void testApproveMerge() throws Exception { + + boolean onlyTriggerPhrase = false; + boolean onlyAdminsMerge = false; + boolean disallowOwnCode = false; + + GhprbPullRequestMerge merger = setupMerger(onlyTriggerPhrase, onlyAdminsMerge, disallowOwnCode); + + setupConditions(nonAdminLogin, committerName, triggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(true); + + setupConditions(adminLogin, nonCommitterName, triggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(true); + + setupConditions(adminLogin, committerName, nonTriggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(true); + + setupConditions(nonAdminLogin, nonCommitterName, triggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(true); + + setupConditions(nonAdminLogin, nonCommitterName, nonTriggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(true); + + setupConditions(adminLogin, nonCommitterName, nonTriggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(true); + + setupConditions(nonAdminLogin, nonCommitterName, nonTriggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(true); + + setupConditions(adminLogin, committerName, triggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(true); + } + + @Test + public void testAdminMerge() throws Exception { + + boolean onlyTriggerPhrase = false; + boolean onlyAdminsMerge = true; + boolean disallowOwnCode = false; + + GhprbPullRequestMerge merger = setupMerger(onlyTriggerPhrase, onlyAdminsMerge, disallowOwnCode); + + setupConditions(adminLogin, committerName, triggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(true); + + setupConditions(nonAdminLogin, committerName, triggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(false); + } + + @Test + public void testTriggerMerge() throws Exception { + + boolean onlyTriggerPhrase = true; + boolean onlyAdminsMerge = false; + boolean disallowOwnCode = false; + + GhprbPullRequestMerge merger = setupMerger(onlyTriggerPhrase, onlyAdminsMerge, disallowOwnCode); + + setupConditions(adminLogin, committerName, triggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(true); + + setupConditions(adminLogin, committerName, nonTriggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(false); + } + + @Test + public void testOwnCodeMerge() throws Exception { + + boolean onlyTriggerPhrase = false; + boolean onlyAdminsMerge = false; + boolean disallowOwnCode = true; + + GhprbPullRequestMerge merger = setupMerger(onlyTriggerPhrase, onlyAdminsMerge, disallowOwnCode); + + setupConditions(adminLogin, nonCommitterName, triggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(true); + + setupConditions(adminLogin, committerName, triggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(false); + } + + @Test + public void testDenyMerge() throws Exception { + + boolean onlyTriggerPhrase = true; + boolean onlyAdminsMerge = true; + boolean disallowOwnCode = true; + + GhprbPullRequestMerge merger = setupMerger(onlyTriggerPhrase, onlyAdminsMerge, disallowOwnCode); + + setupConditions(nonAdminLogin, nonCommitterName, triggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(false); + + setupConditions(adminLogin, committerName, triggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(false); + + setupConditions(adminLogin, nonCommitterName, nonTriggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(false); + + setupConditions(nonAdminLogin, nonCommitterName, triggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(false); + + setupConditions(nonAdminLogin, nonCommitterName, nonTriggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(false); + + setupConditions(adminLogin, committerName, nonTriggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(false); + + setupConditions(nonAdminLogin, committerName, nonTriggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(false); + + setupConditions(adminLogin, nonCommitterName, triggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(true); + } } diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestTest.java index 4184e30c4..fccf277d3 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestTest.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestTest.java @@ -95,7 +95,6 @@ public void testInitRepoNameNull() throws IOException { // Mocks for Ghprb given(helper.isWhitelisted(ghUser)).willReturn(true); - GhprbPullRequest ghprbPullRequest = new GhprbPullRequest(pr, helper, repo); GhprbRepository ghprbRepository = mock(GhprbRepository.class); given(ghprbRepository.getName()).willReturn("name"); @@ -133,7 +132,6 @@ public void testInitRepoNameNotNull() throws IOException { // Mocks for Ghprb given(helper.isWhitelisted(ghUser)).willReturn(true); - GhprbPullRequest ghprbPullRequest = new GhprbPullRequest(pr, helper, repo); GhprbRepository ghprbRepository = mock(GhprbRepository.class); given(ghprbRepository.getName()).willReturn("name"); diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java index 11e01d689..288a0ee9c 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java @@ -4,8 +4,10 @@ import org.joda.time.DateTime; import org.junit.Assert; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; +import org.jvnet.hudson.test.JenkinsRule; import org.kohsuke.github.GHCommitPointer; import org.kohsuke.github.GHCommitState; import org.kohsuke.github.GHIssueComment; @@ -23,6 +25,7 @@ import javax.crypto.Mac; import javax.crypto.spec.SecretKeySpec; +import hudson.model.AbstractProject; import java.io.FileNotFoundException; import java.io.IOException; import java.io.UnsupportedEncodingException; @@ -50,6 +53,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.spy; /** * Unit tests for {@link GhprbRepository}. @@ -79,15 +83,21 @@ public class GhprbRepositoryTest { private GHCommitPointer head; @Mock private GHUser ghUser; - @Mock + private GhprbTrigger trigger; private GhprbRepository ghprbRepository; - private ConcurrentMap pulls; + private ConcurrentMap pulls; private GhprbPullRequest ghprbPullRequest; + @Rule + public JenkinsRule jenkinsRule = new JenkinsRule(); + @Before - public void setUp() throws IOException { + @SuppressWarnings("unused") + public void setUp() throws Exception { + AbstractProject project = jenkinsRule.createFreeStyleProject("GhprbRepoTest"); + trigger = spy(GhprbTestUtil.getTrigger(null)); initGHPRWithTestData(); // Mock github API @@ -160,6 +170,7 @@ public void testCheckMethodWithOnlyExistingPRs() throws IOException { verify(helper).ifOnlyTriggerPhrase(); verify(helper).getWhiteListTargetBranches(); + verify(helper, times(3)).isProjectDisabled(); verifyNoMoreInteractions(helper); verifyNoMoreInteractions(gt); @@ -184,7 +195,7 @@ public void testCheckMethodWithNewPR() throws IOException { given(ghPullRequest.getMergeable()).willReturn(true); given(ghPullRequest.getTitle()).willReturn("title"); given(ghPullRequest.getUser()).willReturn(ghUser); - given(ghPullRequest.getUrl()).willReturn(new URL("https://github.com/org/repo/pull/100")); + given(ghPullRequest.getHtmlUrl()).willReturn(new URL("https://github.com/org/repo/pull/100")); given(ghPullRequest.getApiURL()).willReturn(new URL("https://github.com/org/repo/pull/100")); given(ghUser.getEmail()).willReturn("email"); @@ -206,8 +217,7 @@ public void testCheckMethodWithNewPR() throws IOException { /** GH PR verifications */ verify(builds, times(1)).build(any(GhprbPullRequest.class), any(GHUser.class), any(String.class)); verify(ghRepository, times(1)).getPullRequests(OPEN); // Call to Github API - verify(ghRepository, times(1)) - .createCommitStatus(eq("head sha"), eq(PENDING), isNull(String.class), eq("msg"), eq("default")); // Call to Github API + verify(ghRepository, times(1)).createCommitStatus(eq("head sha"), eq(PENDING), isNull(String.class), eq("msg"), eq("default")); // Call to Github API verifyNoMoreInteractions(ghRepository); verify(ghPullRequest, times(1)).getTitle(); @@ -217,19 +227,20 @@ public void testCheckMethodWithNewPR() throws IOException { verify(ghPullRequest, times(3)).getBase(); verify(ghPullRequest, times(5)).getNumber(); verify(ghPullRequest, times(3)).getUpdatedAt(); - verify(ghPullRequest, times(1)).getUrl(); + verify(ghPullRequest, times(1)).getHtmlUrl(); verify(ghPullRequest, times(1)).listCommits(); verify(ghPullRequest, times(2)).getBody(); verifyNoMoreInteractions(ghPullRequest); - verify(helper, times(1)).isWhitelisted(eq(ghUser)); // Call to Github API + verify(helper, times(1)).isWhitelisted(eq(ghUser)); // Call to Github API verify(helper, times(2)).ifOnlyTriggerPhrase(); verify(helper, times(1)).getBuilds(); verify(helper, times(2)).getWhiteListTargetBranches(); verify(helper, times(1)).getTrigger(); + verify(helper, times(5)).isProjectDisabled(); verifyNoMoreInteractions(helper); - verify(ghUser, times(1)).getEmail(); // Call to Github API + verify(ghUser, times(1)).getEmail(); // Call to Github API verify(ghUser, times(1)).getLogin(); verifyNoMoreInteractions(ghUser); } @@ -258,7 +269,7 @@ public void testCheckMethodWhenPrWasUpdatedWithNonKeyPhrase() throws IOException given(ghPullRequest.getMergeable()).willReturn(true); given(ghPullRequest.getTitle()).willReturn("title"); given(ghPullRequest.getUser()).willReturn(ghUser); - given(ghPullRequest.getUrl()).willReturn(new URL("https://github.com/org/repo/pull/100")); + given(ghPullRequest.getHtmlUrl()).willReturn(new URL("https://github.com/org/repo/pull/100")); given(ghPullRequest.getApiURL()).willReturn(new URL("https://github.com/org/repo/pull/100")); given(ghUser.getEmail()).willReturn("email"); @@ -269,8 +280,8 @@ public void testCheckMethodWhenPrWasUpdatedWithNonKeyPhrase() throws IOException given(helper.getTrigger()).willReturn(trigger); // WHEN - ghprbRepository.check(); // PR was created - ghprbRepository.check(); // PR was updated + ghprbRepository.check(); // PR was created + ghprbRepository.check(); // PR was updated // THEN verifyGetGithub(2); @@ -279,8 +290,7 @@ public void testCheckMethodWhenPrWasUpdatedWithNonKeyPhrase() throws IOException /** GH PR verifications */ verify(builds, times(1)).build(any(GhprbPullRequest.class), any(GHUser.class), any(String.class)); verify(ghRepository, times(2)).getPullRequests(eq(OPEN)); // Call to Github API - verify(ghRepository, times(1)) - .createCommitStatus(eq("head sha"), eq(PENDING), isNull(String.class), eq("msg")); // Call to Github API + verify(ghRepository, times(1)).createCommitStatus(eq("head sha"), eq(PENDING), isNull(String.class), eq("msg")); // Call to Github API verifyNoMoreInteractions(ghRepository); verify(ghPullRequest, times(2)).getTitle(); @@ -289,7 +299,7 @@ public void testCheckMethodWhenPrWasUpdatedWithNonKeyPhrase() throws IOException verify(ghPullRequest, times(8)).getHead(); verify(ghPullRequest, times(3)).getBase(); verify(ghPullRequest, times(5)).getNumber(); - verify(ghPullRequest, times(1)).getUrl(); + verify(ghPullRequest, times(1)).getHtmlUrl(); verify(ghPullRequest, times(4)).getUpdatedAt(); verify(ghPullRequest, times(1)).getComments(); @@ -297,20 +307,21 @@ public void testCheckMethodWhenPrWasUpdatedWithNonKeyPhrase() throws IOException verify(ghPullRequest, times(2)).getBody(); verifyNoMoreInteractions(ghPullRequest); - verify(helper, times(1)).isWhitelisted(eq(ghUser)); // Call to Github API + verify(helper, times(1)).isWhitelisted(eq(ghUser)); // Call to Github API verify(helper, times(2)).ifOnlyTriggerPhrase(); verify(helper, times(1)).getBuilds(); verify(helper, times(2)).getWhiteListTargetBranches(); verify(helper, times(1)).getTrigger(); -// verify(helper).isBotUser(eq(ghUser)); + // verify(helper).isBotUser(eq(ghUser)); verify(helper).isWhitelistPhrase(eq("comment body")); verify(helper).isOktotestPhrase(eq("comment body")); verify(helper).isRetestPhrase(eq("comment body")); verify(helper).isTriggerPhrase(eq("comment body")); + verify(helper, times(6)).isProjectDisabled(); verifyNoMoreInteractions(helper); - verify(ghUser, times(1)).getEmail(); // Call to Github API + verify(ghUser, times(1)).getEmail(); // Call to Github API verify(ghUser, times(1)).getLogin(); verifyNoMoreInteractions(ghUser); } @@ -337,7 +348,7 @@ public void testCheckMethodWhenPrWasUpdatedWithRetestPhrase() throws IOException given(ghPullRequest.getMergeable()).willReturn(true); given(ghPullRequest.getTitle()).willReturn("title"); given(ghPullRequest.getUser()).willReturn(ghUser); - given(ghPullRequest.getUrl()).willReturn(new URL("https://github.com/org/repo/pull/100")); + given(ghPullRequest.getHtmlUrl()).willReturn(new URL("https://github.com/org/repo/pull/100")); given(ghPullRequest.getApiURL()).willReturn(new URL("https://github.com/org/repo/pull/100")); given(ghUser.getEmail()).willReturn("email"); @@ -351,8 +362,8 @@ public void testCheckMethodWhenPrWasUpdatedWithRetestPhrase() throws IOException given(trigger.getCommitStatusContext()).willReturn("default"); // WHEN - ghprbRepository.check(); // PR was created - ghprbRepository.check(); // PR was updated + ghprbRepository.check(); // PR was created + ghprbRepository.check(); // PR was updated // THEN verifyGetGithub(2); @@ -361,8 +372,7 @@ public void testCheckMethodWhenPrWasUpdatedWithRetestPhrase() throws IOException /** GH PR verifications */ verify(builds, times(2)).build(any(GhprbPullRequest.class), any(GHUser.class), any(String.class)); verify(ghRepository, times(2)).getPullRequests(eq(OPEN)); // Call to Github API - verify(ghRepository, times(2)) - .createCommitStatus(eq("head sha"), eq(PENDING), isNull(String.class), eq("msg"), eq("default")); // Call to Github API + verify(ghRepository, times(2)).createCommitStatus(eq("head sha"), eq(PENDING), isNull(String.class), eq("msg"), eq("default")); // Call to Github API verifyNoMoreInteractions(ghRepository); verify(ghPullRequest, times(2)).getTitle(); @@ -372,28 +382,29 @@ public void testCheckMethodWhenPrWasUpdatedWithRetestPhrase() throws IOException verify(ghPullRequest, times(3)).getBase(); verify(ghPullRequest, times(5)).getNumber(); verify(ghPullRequest, times(4)).getUpdatedAt(); - verify(ghPullRequest, times(1)).getUrl(); + verify(ghPullRequest, times(1)).getHtmlUrl(); verify(ghPullRequest, times(1)).getComments(); verify(ghPullRequest, times(2)).listCommits(); - + verify(ghPullRequest, times(2)).getBody(); verifyNoMoreInteractions(ghPullRequest); - verify(helper, times(2)).isWhitelisted(eq(ghUser)); // Call to Github API + verify(helper, times(2)).isWhitelisted(eq(ghUser)); // Call to Github API verify(helper, times(2)).ifOnlyTriggerPhrase(); verify(helper, times(2)).getBuilds(); verify(helper, times(2)).getWhiteListTargetBranches(); verify(helper, times(2)).getTrigger(); -// verify(helper).isBotUser(eq(ghUser)); + // verify(helper).isBotUser(eq(ghUser)); verify(helper).isWhitelistPhrase(eq("test this please")); verify(helper).isOktotestPhrase(eq("test this please")); verify(helper).isRetestPhrase(eq("test this please")); verify(helper).isAdmin(eq(ghUser)); + verify(helper, times(6)).isProjectDisabled(); verifyNoMoreInteractions(helper); - verify(ghUser, times(1)).getEmail(); // Call to Github API + verify(ghUser, times(1)).getEmail(); // Call to Github API verify(ghUser, times(1)).getLogin(); verifyNoMoreInteractions(ghUser); @@ -410,7 +421,6 @@ private void mockComments(String commentBody) throws IOException { comments.add(comment); given(ghPullRequest.getComments()).willReturn(comments); } - private void mockHeadAndBase() { /** Mock head\base */ @@ -419,14 +429,15 @@ private void mockHeadAndBase() { given(ghPullRequest.getBase()).willReturn(base); given(head.getSha()).willReturn("head sha"); } - + + @SuppressWarnings({ "rawtypes", "unchecked" }) private void mockCommitList() { - PagedIterator itr = Mockito.mock(PagedIterator.class); - PagedIterable pagedItr = Mockito.mock(PagedIterable.class); - Mockito.when(ghPullRequest.listCommits()).thenReturn(pagedItr); - Mockito.when(pagedItr.iterator()).thenReturn(itr); - Mockito.when(itr.hasNext()).thenReturn(false); - } + PagedIterator itr = Mockito.mock(PagedIterator.class); + PagedIterable pagedItr = Mockito.mock(PagedIterable.class); + Mockito.when(ghPullRequest.listCommits()).thenReturn(pagedItr); + Mockito.when(pagedItr.iterator()).thenReturn(itr); + Mockito.when(itr.hasNext()).thenReturn(false); + } @Test public void testCheckMethodWithNoPR() throws IOException { @@ -436,6 +447,7 @@ public void testCheckMethodWithNoPR() throws IOException { // WHEN ghprbRepository.check(); + verify(helper).isProjectDisabled(); // THEN verifyGetGithub(1); diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRootActionTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRootActionTest.java index 1f094602c..2d521949d 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRootActionTest.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRootActionTest.java @@ -1,31 +1,64 @@ package org.jenkinsci.plugins.ghprb; - +import hudson.model.FreeStyleProject; +import hudson.plugins.git.GitSCM; import java.io.BufferedReader; import java.io.ByteArrayInputStream; import java.io.InputStream; import java.io.InputStreamReader; import java.io.StringReader; -import java.io.IOException; import java.net.URLEncoder; +import net.sf.json.JSONObject; + +import org.joda.time.DateTime; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.jvnet.hudson.test.JenkinsRule; +import org.kohsuke.github.GHCommitPointer; +import org.kohsuke.github.GHPullRequest; +import org.kohsuke.github.GHRateLimit; +import org.kohsuke.github.GHRepository; +import org.kohsuke.github.GHUser; +import org.kohsuke.github.GitHub; import org.kohsuke.stapler.StaplerRequest; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; +import com.coravy.hudson.plugins.github.GithubProjectProperty; + +import static com.google.common.collect.Lists.newArrayList; +import static org.fest.assertions.Assertions.assertThat; +import static org.kohsuke.github.GHIssueState.OPEN; import static org.mockito.BDDMockito.given; +import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @RunWith(MockitoJUnitRunner.class) public class GhprbRootActionTest { + + + @Mock + protected GHCommitPointer commitPointer; + @Mock + protected GHPullRequest ghPullRequest; + @Mock + protected GhprbGitHub ghprbGitHub; + @Mock + protected GHRepository ghRepository; + @Mock + protected GHUser ghUser; + + protected GitHub gitHub; + // Stubs + protected GHRateLimit ghRateLimit = new GHRateLimit(); @Rule @@ -33,28 +66,120 @@ public class GhprbRootActionTest { @Mock private StaplerRequest req; - + private BufferedReader br; - + @Before public void setup() throws Exception { + gitHub = spy(GitHub.connectAnonymously()); + given(ghprbGitHub.get()).willReturn(gitHub); + given(gitHub.getRateLimit()).willReturn(ghRateLimit); + doReturn(ghRepository).when(gitHub).getRepository(anyString()); + given(commitPointer.getRef()).willReturn("ref"); + given(ghRepository.getName()).willReturn("dropwizard"); + + GhprbTestUtil.mockPR(ghPullRequest, commitPointer, new DateTime(), new DateTime().plusDays(1)); + + given(ghRepository.getPullRequests(eq(OPEN))).willReturn(newArrayList(ghPullRequest)).willReturn(newArrayList(ghPullRequest)); + + given(ghPullRequest.getUser()).willReturn(ghUser); + given(ghUser.getEmail()).willReturn("email@email.com"); + given(ghUser.getLogin()).willReturn("user"); + ghRateLimit.remaining = GhprbTestUtil.INITIAL_RATE_LIMIT; + + GhprbTestUtil.mockCommitList(ghPullRequest); + ghprbGitHub.setGitHub(gitHub); } - + @Test - public void testUrlEncoded() throws IOException { + public void testUrlEncoded() throws Exception { + // GIVEN + FreeStyleProject project = jenkinsRule.createFreeStyleProject("testUrlEncoded"); + GhprbTrigger trigger = GhprbTestUtil.getTrigger(null); + given(commitPointer.getSha()).willReturn("sha1"); + JSONObject jsonObject = GhprbTestUtil.provideConfiguration(); + GhprbTrigger.getDscp().configure(null, jsonObject); + project.addProperty(new GithubProjectProperty("https://github.com/user/dropwizard")); + given(ghPullRequest.getNumber()).willReturn(1); + Ghprb ghprb = spy(trigger.createGhprb(project)); + doReturn(ghprbGitHub).when(ghprb).getGitHub(); + trigger.start(project, true); + trigger.setHelper(ghprb); + ghprb.getRepository().setHelper(ghprb); + project.addTrigger(trigger); + project.getTriggers().keySet().iterator().next().configure(null, jsonObject); + GitSCM scm = GhprbTestUtil.provideGitSCM(); + project.setScm(scm); + + GhprbTestUtil.triggerRunAndWait(10, trigger, project); + + assertThat(project.getBuilds().toArray().length).isEqualTo(1); + + GhprbTrigger.getDscp().setGitHub(ghprbGitHub); + doReturn(gitHub).when(ghprbGitHub).get(); + BufferedReader br = new BufferedReader(new StringReader( "payload=" + URLEncoder.encode(GhprbTestUtil.PAYLOAD, "UTF-8"))); given(req.getContentType()).willReturn("application/x-www-form-urlencoded"); + given(req.getParameter("payload")).willReturn(GhprbTestUtil.PAYLOAD); + given(req.getHeader("X-GitHub-Event")).willReturn("issue_comment"); given(req.getReader()).willReturn(br); given(req.getCharacterEncoding()).willReturn("UTF-8"); - given(req.getHeader("X-GitHub-Event")).willReturn("issue_comment"); GhprbRootAction ra = new GhprbRootAction(); ra.doIndex(req, null); + GhprbTestUtil.waitForBuildsToFinish(project); + + assertThat(project.getBuilds().toArray().length).isEqualTo(2); } + @Test + public void disabledJobsDontBuild() throws Exception { + // GIVEN + FreeStyleProject project = jenkinsRule.createFreeStyleProject("disabledJobsDontBuild"); + GhprbTrigger trigger = GhprbTestUtil.getTrigger(null); + given(commitPointer.getSha()).willReturn("sha1"); + JSONObject jsonObject = GhprbTestUtil.provideConfiguration(); + GhprbTrigger.getDscp().configure(null, jsonObject); + project.addProperty(new GithubProjectProperty("https://github.com/user/dropwizard")); + given(ghPullRequest.getNumber()).willReturn(1); + Ghprb ghprb = spy(trigger.createGhprb(project)); + doReturn(ghprbGitHub).when(ghprb).getGitHub(); + trigger.start(project, true); + trigger.setHelper(ghprb); + ghprb.getRepository().setHelper(ghprb); + project.addTrigger(trigger); + project.getTriggers().keySet().iterator().next().configure(null, jsonObject); + GitSCM scm = GhprbTestUtil.provideGitSCM(); + project.setScm(scm); + + GhprbTestUtil.triggerRunAndWait(10, trigger, project); + + assertThat(project.getBuilds().toArray().length).isEqualTo(1); + + project.disable(); + + GhprbTrigger.getDscp().setGitHub(ghprbGitHub); + doReturn(gitHub).when(ghprbGitHub).get(); + + BufferedReader br = new BufferedReader(new StringReader( + "payload=" + URLEncoder.encode(GhprbTestUtil.PAYLOAD, "UTF-8"))); + + given(req.getContentType()).willReturn("application/x-www-form-urlencoded"); + given(req.getParameter("payload")).willReturn(GhprbTestUtil.PAYLOAD); + given(req.getHeader("X-GitHub-Event")).willReturn("issue_comment"); + given(req.getReader()).willReturn(br); + given(req.getCharacterEncoding()).willReturn("UTF-8"); + + GhprbRootAction ra = new GhprbRootAction(); + ra.doIndex(req, null); + GhprbTestUtil.waitForBuildsToFinish(project); + + assertThat(project.getBuilds().toArray().length).isEqualTo(1); + } + @Test public void testJson() throws Exception { given(req.getContentType()).willReturn("application/json"); @@ -62,17 +187,15 @@ public void testJson() throws Exception { // convert String into InputStream InputStream is = new ByteArrayInputStream(GhprbTestUtil.PAYLOAD.getBytes()); - // read it with BufferedReader br = spy(new BufferedReader(new InputStreamReader(is))); - + given(req.getReader()).willReturn(br); - + GhprbRootAction ra = new GhprbRootAction(); ra.doIndex(req, null); - + verify(br, times(1)).close(); } - } diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTestUtil.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTestUtil.java index 0b7359d30..6405b9cb5 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTestUtil.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTestUtil.java @@ -17,8 +17,10 @@ import static com.google.common.collect.Lists.newArrayList; import static org.mockito.BDDMockito.given; -import java.net.MalformedURLException; import java.net.URL; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import org.joda.time.DateTime; import org.kohsuke.github.GHCommitPointer; @@ -26,10 +28,9 @@ import org.kohsuke.github.PagedIterable; import org.kohsuke.github.PagedIterator; import org.mockito.Mockito; -//import org.mockserver.client.server.MockServerClient; -//import org.mockserver.model.HttpRequest; -//import org.mockserver.model.HttpResponse; +import antlr.ANTLRException; +import hudson.model.AbstractProject; import hudson.plugins.git.BranchSpec; import hudson.plugins.git.GitSCM; import hudson.plugins.git.UserRemoteConfig; @@ -37,9 +38,9 @@ public class GhprbTestUtil { - public static final int INITIAL_RATE_LIMIT = 5000; - public static final String GHPRB_PLUGIN_NAME = "ghprb"; - public static final String PAYLOAD = "{" + + public static final int INITIAL_RATE_LIMIT = 5000; + public static final String GHPRB_PLUGIN_NAME = "ghprb"; + public static final String PAYLOAD = "{" + " \"action\": \"created\"," + " \"issue\": {" + " \"url\": \"https://api.github.com/repos/user/dropwizard/issues/1\"," + @@ -48,7 +49,7 @@ public class GhprbTestUtil { " \"events_url\": \"https://api.github.com/repos/user/dropwizard/issues/1/events\"," + " \"html_url\": \"https://github.com/user/dropwizard/pull/1\"," + " \"id\": 44444444," + - " \"number\": 12," + + " \"number\": 1," + " \"title\": \"Adding version command\"," + " \"user\": {" + " \"login\": \"user\"," + @@ -224,93 +225,144 @@ public class GhprbTestUtil { " }" + "}"; - // TODO: When anyone has time to investigate mocking the github request. -// public static void mockGithubUserPage() { -// new MockServerClient("https://api.github.com", 80) -// .when(new HttpRequest().withMethod("GET").withPath("/user")) -// .respond(new HttpResponse().withStatusCode(200)); -// -// } - public static void mockCommitList(GHPullRequest ghPullRequest) { - PagedIterator itr = Mockito.mock(PagedIterator.class); - PagedIterable pagedItr = Mockito.mock(PagedIterable.class); + @SuppressWarnings({ "rawtypes", "unchecked" }) + public static void mockCommitList(GHPullRequest ghPullRequest) { + PagedIterator itr = Mockito.mock(PagedIterator.class); + PagedIterable pagedItr = Mockito.mock(PagedIterable.class); - Mockito.when(ghPullRequest.listCommits()).thenReturn(pagedItr); - Mockito.when(pagedItr.iterator()).thenReturn(itr); - Mockito.when(itr.hasNext()).thenReturn(false); - } + Mockito.when(ghPullRequest.listCommits()).thenReturn(pagedItr); + Mockito.when(pagedItr.iterator()).thenReturn(itr); + Mockito.when(itr.hasNext()).thenReturn(false); + } - public static void mockPR( - GHPullRequest prToMock, GHCommitPointer commitPointer, - DateTime... updatedDate) - throws MalformedURLException { + public static void mockPR(GHPullRequest prToMock, GHCommitPointer commitPointer, DateTime... updatedDate) throws Exception { - given(prToMock.getHead()).willReturn(commitPointer); - given(prToMock.getBase()).willReturn(commitPointer); - given(prToMock.getUrl()).willReturn(new URL("http://127.0.0.1")); - given(prToMock.getApiURL()).willReturn(new URL("http://127.0.0.1")); + given(prToMock.getHead()).willReturn(commitPointer); + given(prToMock.getBase()).willReturn(commitPointer); + given(prToMock.getUrl()).willReturn(new URL("http://127.0.0.1")); + given(prToMock.getApiURL()).willReturn(new URL("http://127.0.0.1")); - if (updatedDate.length > 1) { - given(prToMock.getUpdatedAt()).willReturn(updatedDate[0].toDate()) - .willReturn(updatedDate[0].toDate()) - .willReturn(updatedDate[1].toDate()) - .willReturn(updatedDate[1].toDate()) - .willReturn(updatedDate[1].toDate()); - } - else { - given(prToMock.getUpdatedAt()).willReturn(updatedDate[0].toDate()); - } - } + if (updatedDate.length > 1) { + given(prToMock.getUpdatedAt()) + .willReturn(updatedDate[0].toDate()) + .willReturn(updatedDate[0].toDate()) + .willReturn(updatedDate[1].toDate()) + .willReturn(updatedDate[1].toDate()) + .willReturn(updatedDate[1].toDate()); + } else { + given(prToMock.getUpdatedAt()).willReturn(updatedDate[0].toDate()); + } + } - public static JSONObject provideConfiguration() { - JSONObject jsonObject = new JSONObject(); + public static JSONObject provideConfiguration() { + JSONObject jsonObject = new JSONObject(); - jsonObject.put("serverAPIUrl", "https://api.github.com"); - jsonObject.put("username", "user"); - jsonObject.put("password", "1111"); - jsonObject.put("accessToken", "accessToken"); - jsonObject.put("adminlist", "user"); - jsonObject.put("allowMembersOfWhitelistedOrgsAsAdmin", "false"); - jsonObject.put("publishedURL", ""); - jsonObject.put("requestForTestingPhrase", "test this"); - jsonObject.put("whitelistPhrase", ""); - jsonObject.put("okToTestPhrase", "ok to test"); - jsonObject.put("retestPhrase", "retest this please"); - jsonObject.put("skipBuildPhrase", "[skip ci]"); - jsonObject.put("cron", "*/1 * * * *"); - jsonObject.put("useComments", "true"); - jsonObject.put("useDetailedComments", "false"); - jsonObject.put("logExcerptLines", "0"); - jsonObject.put("unstableAs", ""); - jsonObject.put("testMode", "true"); - jsonObject.put("autoCloseFailedPullRequests", "false"); - jsonObject.put("displayBuildErrorsOnDownstreamBuilds", "false"); - jsonObject.put("msgSuccess", "Success"); - jsonObject.put("msgFailure", "Failure"); - jsonObject.put("commitStatusContext", "Status Context"); - jsonObject.put("secret", "111"); + jsonObject.put("serverAPIUrl", "https://api.github.com"); + jsonObject.put("username", "user"); + jsonObject.put("password", "1111"); + jsonObject.put("accessToken", "accessToken"); + jsonObject.put("adminlist", "user"); + jsonObject.put("allowMembersOfWhitelistedOrgsAsAdmin", "false"); + jsonObject.put("publishedURL", ""); + jsonObject.put("requestForTestingPhrase", "test this"); + jsonObject.put("whitelistPhrase", ""); + jsonObject.put("okToTestPhrase", "ok to test"); + jsonObject.put("retestPhrase", "retest this please"); + jsonObject.put("skipBuildPhrase", "[skip ci]"); + jsonObject.put("cron", "0 0 31 2 0"); + jsonObject.put("useComments", "true"); + jsonObject.put("useDetailedComments", "false"); + jsonObject.put("logExcerptLines", "0"); + jsonObject.put("unstableAs", ""); + jsonObject.put("testMode", "true"); + jsonObject.put("autoCloseFailedPullRequests", "false"); + jsonObject.put("displayBuildErrorsOnDownstreamBuilds", "false"); + jsonObject.put("msgSuccess", "Success"); + jsonObject.put("msgFailure", "Failure"); + jsonObject.put("commitStatusContext", "Status Context"); - return jsonObject; - } + return jsonObject; + } - public static GitSCM provideGitSCM() { - return new GitSCM( - newArrayList( - new UserRemoteConfig( - "https://github.com/user/dropwizard", "", - "+refs/pull/*:refs/remotes/origin/pr/*", "") - ), - newArrayList(new BranchSpec("${sha1}")), - false, - null, - null, - "", - null - ); - } + public static GitSCM provideGitSCM() { + return new GitSCM(newArrayList( + new UserRemoteConfig("https://github.com/user/dropwizard", + "", "+refs/pull/*:refs/remotes/origin/pr/*", "")), + newArrayList(new BranchSpec("${sha1}")), + false, + null, + null, + "", + null); + } + + @SuppressWarnings("unchecked") + public static GhprbTrigger getTrigger(Map values) throws ANTLRException { + if (values == null) { + values = new HashMap(); + } + Map defaultValues = new HashMap (){ + private static final long serialVersionUID = -6720840565156773837L; - private GhprbTestUtil() { - } + { + put("adminlist", "user"); + put("whitelist", "user"); + put("orgslist", ""); + put("cron", "0 0 31 2 0"); + put("triggerPhrase", "retest this please"); + put("onlyTriggerPhrase", false); + put("useGitHubHooks", false); + put("permitAll", false); + put("autoCloseFailedPullRequests", false); + put("displayBuildErrorsOnDownstreamBuilds", false); + put("commentFilePath", null); + put("whiteListTargetBranches", null); + put("allowMembersOfWhitelistedOrgsAsAdmin", false); + put("msgSuccess", null); + put("msgFailure", null); + put("secret", null); + put("commitStatusContext", null); + }}; + + defaultValues.putAll(values); + GhprbTrigger trigger = new GhprbTrigger( + (String)defaultValues.get("adminlist"), + (String)defaultValues.get("whitelist"), + (String)defaultValues.get("orgslist"), + (String)defaultValues.get("cron"), + (String)defaultValues.get("triggerPhrase"), + (Boolean)defaultValues.get("onlyTriggerPhrase"), + (Boolean)defaultValues.get("useGitHubHooks"), + (Boolean)defaultValues.get("permitAll"), + (Boolean)defaultValues.get("autoCloseFailedPullRequests"), + (Boolean)defaultValues.get("displayBuildErrorsOnDownstreamBuilds"), + (String)defaultValues.get("commentFilePath"), + (List)defaultValues.get("whiteListTargetBranches"), + (Boolean)defaultValues.get("allowMembersOfWhitelistedOrgsAsAdmin"), + (String)defaultValues.get("msgSuccess"), + (String)defaultValues.get("msgFailure"), + (String)defaultValues.get("secret"), + (String)defaultValues.get("commitStatusContext")); + return trigger; + } + + public static void waitForBuildsToFinish(AbstractProject project) throws InterruptedException { + while (project.isBuilding() || project.isInQueue()) { + // THEN + Thread.sleep(500); + } + } + + + public static void triggerRunAndWait(int numOfTriggers, GhprbTrigger trigger, AbstractProject project) throws InterruptedException { + for (int i = 0; i < numOfTriggers; ++i) { + trigger.run(); + waitForBuildsToFinish(project); + } + + } + + private GhprbTestUtil() {} } diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTriggerTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTriggerTest.java index 1944af7b8..0022e0412 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTriggerTest.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTriggerTest.java @@ -1,4 +1,3 @@ - package org.jenkinsci.plugins.ghprb; import static org.fest.assertions.Assertions.assertThat; @@ -24,44 +23,43 @@ */ @RunWith(MockitoJUnitRunner.class) public class GhprbTriggerTest { - - @Mock - private GhprbPullRequest pr; - @Test - public void testCheckSkipBuild() throws NoSuchFieldException, SecurityException, NoSuchMethodException, IllegalAccessException, IllegalArgumentException, InvocationTargetException { - GHIssue issue = mock(GHIssue.class); - - String[] comments = {"Some dumb comment\r\nThat shouldn't match", "[skip ci]"}; - String[] phraseArray = {"\\[skip\\W+ci\\]", "skip ci"}; + @Mock + private GhprbPullRequest pr; + + @Test + public void testCheckSkipBuild() throws Exception { + GHIssue issue = mock(GHIssue.class); + + String[] comments = { "Some dumb comment\r\nThat shouldn't match", "[skip ci]" }; + String[] phraseArray = { "\\[skip\\W+ci\\]", "skip ci" }; + + Method checkSkip = GhprbPullRequest.class.getDeclaredMethod("checkSkipBuild", GHIssue.class); + checkSkip.setAccessible(true); + + Field shouldRun = GhprbPullRequest.class.getDeclaredField("shouldRun"); + shouldRun.setAccessible(true); + + for (String phraseString : phraseArray) { + for (String comment : comments) { + + Set phrases = new HashSet(Arrays.asList(phraseString.split("[\\r\\n]+"))); + given(issue.getBody()).willReturn(comment); + given(pr.getSkipBuildPhrases()).willReturn(phrases); + boolean isMatch = false; + for (String phrase : phrases) { + isMatch = Pattern.matches(phrase, comment); + if (isMatch) { + break; + } + } + shouldRun.set(pr, true); + checkSkip.invoke(pr, issue); + assertThat(shouldRun.get(pr)).isEqualTo(!isMatch); + + } + } - Method checkSkip = GhprbPullRequest.class.getDeclaredMethod("checkSkipBuild", GHIssue.class); - checkSkip.setAccessible(true); - - Field shouldRun = GhprbPullRequest.class.getDeclaredField("shouldRun"); - shouldRun.setAccessible(true); - - for (String phraseString : phraseArray) { - for (String comment : comments) { - - Set phrases = new HashSet(Arrays.asList(phraseString.split("[\\r\\n]+"))); - given(issue.getBody()).willReturn(comment); - given(pr.getSkipBuildPhrases()).willReturn(phrases); - boolean isMatch = false; - for (String phrase : phrases) { - isMatch = Pattern.matches(phrase, comment); - if (isMatch) { - break; - } - } - shouldRun.set(pr, true); - checkSkip.invoke(pr, issue); - assertThat(shouldRun.get(pr)).isEqualTo(!isMatch); - - } - } - - } - + } } diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/manager/configuration/JobConfigurationTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/manager/configuration/JobConfigurationTest.java index 4ac58485f..1e86a2e75 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/manager/configuration/JobConfigurationTest.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/manager/configuration/JobConfigurationTest.java @@ -9,22 +9,20 @@ */ public class JobConfigurationTest { - @Test - public void shouldNotPrintStackTrace() { - JobConfiguration jobConfiguration = - JobConfiguration.builder().printStackTrace(false).build(); - - assertThat(jobConfiguration).isNotNull(); - assertThat(jobConfiguration.printStackTrace()).isFalse(); - } - - @Test - public void shouldPrintStackTrace() { - JobConfiguration jobConfiguration = - JobConfiguration.builder().printStackTrace(true).build(); - - assertThat(jobConfiguration).isNotNull(); - assertThat(jobConfiguration.printStackTrace()).isTrue(); - } + @Test + public void shouldNotPrintStackTrace() { + JobConfiguration jobConfiguration = JobConfiguration.builder().printStackTrace(false).build(); + + assertThat(jobConfiguration).isNotNull(); + assertThat(jobConfiguration.printStackTrace()).isFalse(); + } + + @Test + public void shouldPrintStackTrace() { + JobConfiguration jobConfiguration = JobConfiguration.builder().printStackTrace(true).build(); + + assertThat(jobConfiguration).isNotNull(); + assertThat(jobConfiguration.printStackTrace()).isTrue(); + } } \ No newline at end of file diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/manager/factory/GhprbBuildManagerFactoryUtilTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/manager/factory/GhprbBuildManagerFactoryUtilTest.java index 9dfdec731..d0c1d3974 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/manager/factory/GhprbBuildManagerFactoryUtilTest.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/manager/factory/GhprbBuildManagerFactoryUtilTest.java @@ -22,33 +22,29 @@ */ public class GhprbBuildManagerFactoryUtilTest { - @Rule - public JenkinsRuleWithBuildFlow jenkinsRule = new JenkinsRuleWithBuildFlow(); - - @Test - public void shouldReturnDefaultManager() throws Exception { - // GIVEN - MatrixProject project = jenkinsRule.createMatrixProject("PRJ"); - - GhprbBuildManager buildManager = - GhprbBuildManagerFactoryUtil.getBuildManager( - new MatrixBuild(project)); - - // THEN - assertThat(buildManager).isInstanceOf(GhprbDefaultBuildManager.class); - } - - @Test - public void shouldReturnBuildFlowManager() throws Exception { - // GIVEN - BuildFlow buildFlowProject = jenkinsRule.createBuildFlowProject("BFPRJ"); - - GhprbBuildManager buildManager = - GhprbBuildManagerFactoryUtil.getBuildManager( - new FlowRun(buildFlowProject)); - - // THEN - assertThat(buildManager).isInstanceOf(BuildFlowBuildManager.class); - } + @Rule + public JenkinsRuleWithBuildFlow jenkinsRule = new JenkinsRuleWithBuildFlow(); + + @Test + public void shouldReturnDefaultManager() throws Exception { + // GIVEN + MatrixProject project = jenkinsRule.createMatrixProject("PRJ"); + + GhprbBuildManager buildManager = GhprbBuildManagerFactoryUtil.getBuildManager(new MatrixBuild(project)); + + // THEN + assertThat(buildManager).isInstanceOf(GhprbDefaultBuildManager.class); + } + + @Test + public void shouldReturnBuildFlowManager() throws Exception { + // GIVEN + BuildFlow buildFlowProject = jenkinsRule.createBuildFlowProject("BFPRJ"); + + GhprbBuildManager buildManager = GhprbBuildManagerFactoryUtil.getBuildManager(new FlowRun(buildFlowProject)); + + // THEN + assertThat(buildManager).isInstanceOf(BuildFlowBuildManager.class); + } } \ No newline at end of file diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/manager/impl/GhprbDefaultBuildManagerTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/manager/impl/GhprbDefaultBuildManagerTest.java index 47959cbf8..4473a8a0f 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/manager/impl/GhprbDefaultBuildManagerTest.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/manager/impl/GhprbDefaultBuildManagerTest.java @@ -33,78 +33,71 @@ @RunWith(MockitoJUnitRunner.class) public class GhprbDefaultBuildManagerTest extends GhprbITBaseTestCase { - @Rule - public JenkinsRule jenkinsRule = new JenkinsRule(); + @Rule + public JenkinsRule jenkinsRule = new JenkinsRule(); - @Before - public void setUp() throws Exception { -// GhprbTestUtil.mockGithubUserPage(); - super.beforeTest(); - } + @Before + public void setUp() throws Exception { + // GhprbTestUtil.mockGithubUserPage(); + super.beforeTest(); + } - @Test - public void shouldCalculateUrlFromDefault() throws Exception { - // GIVEN - MatrixProject project = givenThatGhprbHasBeenTriggeredForAMatrixProject(); + @Test + public void shouldCalculateUrlFromDefault() throws Exception { + // GIVEN + MatrixProject project = givenThatGhprbHasBeenTriggeredForAMatrixProject(); - // THEN - assertThat(project.getBuilds().toArray().length).isEqualTo(1); + // THEN + assertThat(project.getBuilds().toArray().length).isEqualTo(1); - MatrixBuild matrixBuild = project.getBuilds().getFirstBuild(); + MatrixBuild matrixBuild = project.getBuilds().getFirstBuild(); - GhprbBuildManager buildManager = - GhprbBuildManagerFactoryUtil.getBuildManager(matrixBuild); + GhprbBuildManager buildManager = GhprbBuildManagerFactoryUtil.getBuildManager(matrixBuild); - assertThat(buildManager).isInstanceOf(GhprbDefaultBuildManager.class); + assertThat(buildManager).isInstanceOf(GhprbDefaultBuildManager.class); - assertThat(buildManager.calculateBuildUrl()).isEqualTo( - "defaultPublishedURL/" + matrixBuild.getUrl()); - } + assertThat(buildManager.calculateBuildUrl()).isEqualTo("defaultPublishedURL/" + matrixBuild.getUrl()); + } - private MatrixProject givenThatGhprbHasBeenTriggeredForAMatrixProject() throws Exception { - MatrixProject project = jenkinsRule.createMatrixProject("MTXPRJ"); + private MatrixProject givenThatGhprbHasBeenTriggeredForAMatrixProject() throws Exception { + MatrixProject project = jenkinsRule.createMatrixProject("MTXPRJ"); - GhprbTrigger trigger = new GhprbTrigger("user", "user", "", - "*/1 * * * *", "retest this please", false, false, false, false, - false, null, null, false, null, null, null, null); + GhprbTrigger trigger = GhprbTestUtil.getTrigger(null); - given(commitPointer.getSha()).willReturn("sha"); + given(commitPointer.getSha()).willReturn("sha"); - JSONObject jsonObject = GhprbTestUtil.provideConfiguration(); - + JSONObject jsonObject = GhprbTestUtil.provideConfiguration(); - jsonObject.put("publishedURL", "defaultPublishedURL"); - GhprbTrigger.DESCRIPTOR.configure(null, jsonObject); + jsonObject.put("publishedURL", "defaultPublishedURL"); + GhprbTrigger.DESCRIPTOR.configure(null, jsonObject); - project.addProperty(new GithubProjectProperty("https://github.com/user/dropwizard")); + project.addProperty(new GithubProjectProperty("https://github.com/user/dropwizard")); - given(ghPullRequest.getNumber()).willReturn(1); + given(ghPullRequest.getNumber()).willReturn(1); - // Creating spy on ghprb, configuring repo - Ghprb ghprb = spyCreatingGhprb(trigger, project); + // Creating spy on ghprb, configuring repo + Ghprb ghprb = spyCreatingGhprb(trigger, project); - doReturn(ghprbGitHub).when(ghprb).getGitHub(); + doReturn(ghprbGitHub).when(ghprb).getGitHub(); - setRepositoryHelper(ghprb); + setRepositoryHelper(ghprb); - given(ghRepository.getPullRequest(1)).willReturn(ghPullRequest); + given(ghRepository.getPullRequest(1)).willReturn(ghPullRequest); - // Configuring and adding Ghprb trigger - project.addTrigger(trigger); + // Configuring and adding Ghprb trigger + project.addTrigger(trigger); - project.getTriggers().keySet().iterator().next() - .configure(null, jsonObject); + project.getTriggers().keySet().iterator().next().configure(null, jsonObject); - // Configuring Git SCM - project.setScm(GhprbTestUtil.provideGitSCM()); + // Configuring Git SCM + project.setScm(GhprbTestUtil.provideGitSCM()); - trigger.start(project, true); + trigger.start(project, true); - setTriggerHelper(trigger, ghprb); + setTriggerHelper(trigger, ghprb); - // THEN - Thread.sleep(130000); + GhprbTestUtil.triggerRunAndWait(10, trigger, project); - return project; - } + return project; + } } \ No newline at end of file diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/manager/impl/downstreambuilds/BuildFlowBuildManagerTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/manager/impl/downstreambuilds/BuildFlowBuildManagerTest.java index a4ab8bb5f..8d24484fe 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/manager/impl/downstreambuilds/BuildFlowBuildManagerTest.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/manager/impl/downstreambuilds/BuildFlowBuildManagerTest.java @@ -35,117 +35,110 @@ @RunWith(MockitoJUnitRunner.class) public class BuildFlowBuildManagerTest extends GhprbITBaseTestCase { - @Rule - public JenkinsRuleWithBuildFlow jenkinsRule = new JenkinsRuleWithBuildFlow(); + @Rule + public JenkinsRuleWithBuildFlow jenkinsRule = new JenkinsRuleWithBuildFlow(); - @Before - public void setUp() throws Exception { - super.beforeTest(); - } + @Before + public void setUp() throws Exception { + super.beforeTest(); + } - @Test - public void shouldCalculateUrlWithDownstreamBuilds() throws Exception { - // GIVEN - BuildFlow buildFlowProject = - givenThatGhprbHasBeenTriggeredForABuildFlowProject(); + @Test + public void shouldCalculateUrlWithDownstreamBuilds() throws Exception { + // GIVEN + BuildFlow buildFlowProject = givenThatGhprbHasBeenTriggeredForABuildFlowProject(); - // THEN - assertThat(buildFlowProject.getBuilds().toArray().length).isEqualTo(1); + // THEN + assertThat(buildFlowProject.getBuilds().toArray().length).isEqualTo(1); - FlowRun flowRun = buildFlowProject.getBuilds().getFirstBuild(); + FlowRun flowRun = buildFlowProject.getBuilds().getFirstBuild(); - GhprbBuildManager buildManager = - GhprbBuildManagerFactoryUtil.getBuildManager(flowRun); + GhprbBuildManager buildManager = GhprbBuildManagerFactoryUtil.getBuildManager(flowRun); - assertThat(buildManager).isInstanceOf(BuildFlowBuildManager.class); + assertThat(buildManager).isInstanceOf(BuildFlowBuildManager.class); - Iterator iterator = buildManager.downstreamProjects(); + Iterator iterator = buildManager.downstreamProjects(); - StringBuilder expectedUrl = new StringBuilder(); + StringBuilder expectedUrl = new StringBuilder(); - int count = 0; + int count = 0; - while (iterator.hasNext()) { - Object downstreamBuild = iterator.next(); + while (iterator.hasNext()) { + Object downstreamBuild = iterator.next(); - assertThat(downstreamBuild).isInstanceOf(JobInvocation.class); + assertThat(downstreamBuild).isInstanceOf(JobInvocation.class); - JobInvocation jobInvocation = (JobInvocation)downstreamBuild; + JobInvocation jobInvocation = (JobInvocation) downstreamBuild; - String jobInvocationBuildUrl = jobInvocation.getBuildUrl(); + String jobInvocationBuildUrl = jobInvocation.getBuildUrl(); - expectedUrl.append("\n"); - expectedUrl.append(jobInvocationBuildUrl); - expectedUrl.append(""); + expectedUrl.append("\n"); + expectedUrl.append(jobInvocationBuildUrl); + expectedUrl.append(""); - count++; - } + count++; + } - assertThat(count).isEqualTo(4); + assertThat(count).isEqualTo(4); - assertThat(buildManager.calculateBuildUrl()).isEqualTo(expectedUrl.toString()); - } + assertThat(buildManager.calculateBuildUrl()).isEqualTo(expectedUrl.toString()); + } - private BuildFlow givenThatGhprbHasBeenTriggeredForABuildFlowProject() - throws Exception { + private BuildFlow givenThatGhprbHasBeenTriggeredForABuildFlowProject() throws Exception { - BuildFlow buildFlowProject = jenkinsRule.createBuildFlowProject(); + BuildFlow buildFlowProject = jenkinsRule.createBuildFlowProject(); - jenkinsRule.createFreeStyleProject("downstreamProject1"); - jenkinsRule.createFreeStyleProject("downstreamProject2"); - jenkinsRule.createFreeStyleProject("downstreamProject3"); + jenkinsRule.createFreeStyleProject("downstreamProject1"); + jenkinsRule.createFreeStyleProject("downstreamProject2"); + jenkinsRule.createFreeStyleProject("downstreamProject3"); - StringBuilder dsl = new StringBuilder(); + StringBuilder dsl = new StringBuilder(); - dsl.append("parallel ("); - dsl.append(" { build(\"downstreamProject1\") },"); - dsl.append(" { build(\"downstreamProject2\") }"); - dsl.append(")"); - dsl.append("{ build(\"downstreamProject3\") }"); + dsl.append("parallel ("); + dsl.append(" { build(\"downstreamProject1\") },"); + dsl.append(" { build(\"downstreamProject2\") }"); + dsl.append(")"); + dsl.append("{ build(\"downstreamProject3\") }"); - buildFlowProject.setDsl(dsl.toString()); + buildFlowProject.setDsl(dsl.toString()); - GhprbTrigger trigger = new GhprbTrigger("user", "user", "", - "*/1 * * * *", "retest this please", false, false, false, false, - false, null, null, false, null, null, null, null); + GhprbTrigger trigger = GhprbTestUtil.getTrigger(null); - given(commitPointer.getSha()).willReturn("sha"); - JSONObject jsonObject = GhprbTestUtil.provideConfiguration(); + given(commitPointer.getSha()).willReturn("sha"); + JSONObject jsonObject = GhprbTestUtil.provideConfiguration(); - GhprbTrigger.DESCRIPTOR.configure(null, jsonObject); + GhprbTrigger.DESCRIPTOR.configure(null, jsonObject); - buildFlowProject.addProperty(new GithubProjectProperty( - "https://github.com/user/dropwizard")); + buildFlowProject.addProperty(new GithubProjectProperty("https://github.com/user/dropwizard")); - given(ghPullRequest.getNumber()).willReturn(1); + given(ghPullRequest.getNumber()).willReturn(1); - // Creating spy on ghprb, configuring repo - Ghprb ghprb = spyCreatingGhprb(trigger, buildFlowProject); + // Creating spy on ghprb, configuring repo + Ghprb ghprb = spyCreatingGhprb(trigger, buildFlowProject); - doReturn(ghprbGitHub).when(ghprb).getGitHub(); + doReturn(ghprbGitHub).when(ghprb).getGitHub(); - setRepositoryHelper(ghprb); + setRepositoryHelper(ghprb); - given(ghRepository.getPullRequest(1)).willReturn(ghPullRequest); + given(ghRepository.getPullRequest(1)).willReturn(ghPullRequest); - // Configuring and adding Ghprb trigger - buildFlowProject.addTrigger(trigger); + // Configuring and adding Ghprb trigger + buildFlowProject.addTrigger(trigger); - buildFlowProject.getTriggers().keySet().iterator().next() - .configure(null, jsonObject); + buildFlowProject.getTriggers().keySet().iterator().next().configure(null, jsonObject); - // Configuring Git SCM - buildFlowProject.setScm(GhprbTestUtil.provideGitSCM()); + // Configuring Git SCM + buildFlowProject.setScm(GhprbTestUtil.provideGitSCM()); - trigger.start(buildFlowProject, true); + trigger.start(buildFlowProject, true); - setTriggerHelper(trigger, ghprb); + setTriggerHelper(trigger, ghprb); - Thread.sleep(130000); + GhprbTestUtil.triggerRunAndWait(10, trigger, buildFlowProject); - return buildFlowProject; - } + return buildFlowProject; + } } diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/rules/JenkinsRuleWithBuildFlow.java b/src/test/java/org/jenkinsci/plugins/ghprb/rules/JenkinsRuleWithBuildFlow.java index b54bb468b..19d02867a 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/rules/JenkinsRuleWithBuildFlow.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/rules/JenkinsRuleWithBuildFlow.java @@ -11,14 +11,13 @@ */ public class JenkinsRuleWithBuildFlow extends JenkinsRule { - public BuildFlow createBuildFlowProject() throws IOException { - return createBuildFlowProject(createUniqueProjectName()); - } + public BuildFlow createBuildFlowProject() throws IOException { + return createBuildFlowProject(createUniqueProjectName()); + } - public BuildFlow createBuildFlowProject(String name) - throws IOException { + public BuildFlow createBuildFlowProject(String name) throws IOException { - return jenkins.createProject(BuildFlow.class, name); - } + return jenkins.createProject(BuildFlow.class, name); + } } \ No newline at end of file From d7b8b444404d87dbd2f53e309c6545e886b40092 Mon Sep 17 00:00:00 2001 From: Robin Neatherway Date: Tue, 9 Jun 2015 11:48:37 +0100 Subject: [PATCH 05/14] Correct typo 'conext' -> 'context' --- .../plugins/ghprb/extensions/status/GhprbSimpleStatus.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus.java b/src/main/java/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus.java index e5c16e9c7..b58001181 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus.java @@ -85,7 +85,7 @@ private void createCommitStatus(AbstractBuild build, TaskListener listener listener.getLogger().println(String.format("Setting status of %s to %s with url %s and message: '%s'", sha1, state, url, message)); if (context != null) { - listener.getLogger().println(String.format("Using conext: " + context)); + listener.getLogger().println(String.format("Using context: " + context)); } try { repo.createCommitStatus(sha1, state, url, message, context); From b6b3766ea51cf9edcbaf2122536c7f2f06e878f6 Mon Sep 17 00:00:00 2001 From: David Tanner Date: Fri, 12 Jun 2015 11:26:54 -0600 Subject: [PATCH 06/14] [maven-release-plugin] prepare release ghprb-1.23 --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 56bcd0649..16f211f8c 100644 --- a/pom.xml +++ b/pom.xml @@ -10,7 +10,7 @@ ghprb GitHub Pull Request Builder - 1.23-SNAPSHOT + 1.23 hpi https://wiki.jenkins-ci.org/display/JENKINS/GitHub+pull+request+builder+plugin @@ -31,7 +31,7 @@ scm:git:ssh://github.com/jenkinsci/ghprb-plugin.git scm:git:ssh://git@github.com/jenkinsci/ghprb-plugin.git https://github.com/jenkinsci/ghprb-plugin - HEAD + ghprb-1.23 From 2ffd677fb76141b132b30653b887268944544b20 Mon Sep 17 00:00:00 2001 From: David Tanner Date: Fri, 12 Jun 2015 11:26:56 -0600 Subject: [PATCH 07/14] [maven-release-plugin] prepare for next development iteration --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 16f211f8c..20e22118e 100644 --- a/pom.xml +++ b/pom.xml @@ -10,7 +10,7 @@ ghprb GitHub Pull Request Builder - 1.23 + 1.24-SNAPSHOT hpi https://wiki.jenkins-ci.org/display/JENKINS/GitHub+pull+request+builder+plugin @@ -31,7 +31,7 @@ scm:git:ssh://github.com/jenkinsci/ghprb-plugin.git scm:git:ssh://git@github.com/jenkinsci/ghprb-plugin.git https://github.com/jenkinsci/ghprb-plugin - ghprb-1.23 + HEAD From 444e7e8af4ecbb48f7779aa7f90ca1937df9ecfc Mon Sep 17 00:00:00 2001 From: David Tanner Date: Sat, 13 Jun 2015 17:33:33 -0600 Subject: [PATCH 08/14] Change to just use the host name for the default domain --- src/main/java/org/jenkinsci/plugins/ghprb/Ghprb.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/Ghprb.java b/src/main/java/org/jenkinsci/plugins/ghprb/Ghprb.java index c8948b081..92da9df8e 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/Ghprb.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/Ghprb.java @@ -391,9 +391,7 @@ private static String createCredentials(String serverAPIUrl, StandardCredentials specifications.add(new SchemeSpecification(serverUri.getScheme())); specifications.add(new PathSpecification(serverUri.getPath(), null, false)); - - - Domain domain = new Domain(serverAPIUrl, "Auto generated credentials domain", specifications); + Domain domain = new Domain(serverUri.getHost(), "Auto generated credentials domain", specifications); CredentialsStore provider = new SystemCredentialsProvider.StoreImpl(); provider.addDomain(domain, credentials); return credentials.getId(); From d72a4cb97f872a054c668b172cbacbe3ed8f199c Mon Sep 17 00:00:00 2001 From: David Tanner Date: Sat, 13 Jun 2015 18:32:58 -0600 Subject: [PATCH 09/14] Fix bug in groovy config --- .../java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java | 2 +- .../jenkinsci/plugins/ghprb/GhprbTrigger/config.groovy | 8 ++++---- .../jenkinsci/plugins/ghprb/GhprbTrigger/global.groovy | 1 - 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java index 66dfe7adc..e977514d7 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java @@ -70,7 +70,7 @@ public class GhprbTrigger extends GhprbTriggerBackwardsCompatible { private GhprbGitHubAuth gitHubApiAuth; - private DescribableList extensions; + private DescribableList extensions = new DescribableList(Saveable.NOOP); public DescribableList getExtensions() { if (extensions == null) { diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbTrigger/config.groovy b/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbTrigger/config.groovy index cf7363a3a..67eb02cf5 100644 --- a/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbTrigger/config.groovy +++ b/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbTrigger/config.groovy @@ -1,4 +1,3 @@ -// Namespaces xml = namespace("http://www.w3.org/XML/1998/namespace") j = namespace("jelly:core") f = namespace("/lib/form") @@ -59,8 +58,9 @@ f.advanced() { } } } -f.advanced(title: _("Trigger Setup")) { +//f.advanced(title: _("Trigger Setup")) { f.entry(title: _("Trigger Setup")) { - f.hetero_list(items: instance.extensions, name: "extensions", oneEach: "true", hasHeader: "true", descriptors: descriptor.getExtensionDescriptors()) + f.hetero_list(items: instance == null ? null : instance.extensions, + name: "extensions", oneEach: "true", hasHeader: "true", descriptors: descriptor.getExtensionDescriptors()) } -} +//} diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbTrigger/global.groovy b/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbTrigger/global.groovy index 6b6884dd5..64786f59a 100644 --- a/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbTrigger/global.groovy +++ b/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbTrigger/global.groovy @@ -1,4 +1,3 @@ -// Namespaces xml = namespace("http://www.w3.org/XML/1998/namespace") j = namespace("jelly:core") f = namespace("/lib/form") From 30a06d6f5c8442556951bc52b49620000450b6c1 Mon Sep 17 00:00:00 2001 From: David Tanner Date: Sat, 13 Jun 2015 18:48:49 -0600 Subject: [PATCH 10/14] [maven-release-plugin] prepare release ghprb-1.23.1 --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 20e22118e..aea0059a9 100644 --- a/pom.xml +++ b/pom.xml @@ -10,7 +10,7 @@ ghprb GitHub Pull Request Builder - 1.24-SNAPSHOT + 1.23.1 hpi https://wiki.jenkins-ci.org/display/JENKINS/GitHub+pull+request+builder+plugin @@ -31,7 +31,7 @@ scm:git:ssh://github.com/jenkinsci/ghprb-plugin.git scm:git:ssh://git@github.com/jenkinsci/ghprb-plugin.git https://github.com/jenkinsci/ghprb-plugin - HEAD + ghprb-1.23.1 From 2083f43ed0be3184b6df84b146b346e58f469205 Mon Sep 17 00:00:00 2001 From: David Tanner Date: Sat, 13 Jun 2015 18:48:52 -0600 Subject: [PATCH 11/14] [maven-release-plugin] prepare for next development iteration --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index aea0059a9..20e22118e 100644 --- a/pom.xml +++ b/pom.xml @@ -10,7 +10,7 @@ ghprb GitHub Pull Request Builder - 1.23.1 + 1.24-SNAPSHOT hpi https://wiki.jenkins-ci.org/display/JENKINS/GitHub+pull+request+builder+plugin @@ -31,7 +31,7 @@ scm:git:ssh://github.com/jenkinsci/ghprb-plugin.git scm:git:ssh://git@github.com/jenkinsci/ghprb-plugin.git https://github.com/jenkinsci/ghprb-plugin - ghprb-1.23.1 + HEAD From d262961054039d1eba567e3324456256d65d2d91 Mon Sep 17 00:00:00 2001 From: David Tanner Date: Sat, 13 Jun 2015 20:50:17 -0600 Subject: [PATCH 12/14] config was still broken. Also fixed null pointer when there are no credentials set up yet. --- .../org/jenkinsci/plugins/ghprb/GhprbTrigger.java | 11 +++-------- .../plugins/ghprb/GhprbTrigger/config.groovy | 4 ++-- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java index e977514d7..7dca9e5d1 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java @@ -482,7 +482,7 @@ public GhprbGitHubAuth getGitHubAuth(String gitHubAuthId) { public List getGithubAuth() { if (githubAuth == null || githubAuth.size() == 0) { githubAuth = new ArrayList(1); - githubAuth.add(new GhprbGitHubAuth(null, null, "Blank description", null)); + githubAuth.add(new GhprbGitHubAuth(null, null, "Anonymous connection", null)); } return githubAuth; } @@ -491,16 +491,11 @@ public List getDefaultAuth(List githubAuth) { if (githubAuth != null && githubAuth.size() > 0) { return githubAuth; } - List defaults = new ArrayList(1); - defaults.add(new GhprbGitHubAuth(null, null, "Blank description", null)); - return defaults; + return getGithubAuth(); } - - private String adminlist; - private String requestForTestingPhrase; // map of jobs (by their fullName) and their map of pull requests @@ -655,7 +650,7 @@ public boolean isUseDetailedComments() { public ListBoxModel doFillGitHubAuthIdItems(@QueryParameter("gitHubAuth") String gitHubAuthId) { ListBoxModel model = new ListBoxModel(); - for (GhprbGitHubAuth auth : githubAuth) { + for (GhprbGitHubAuth auth : getGithubAuth()) { String description = Util.fixNull(auth.getDescription()); int length = description.length(); length = length > 50 ? 50 : length; diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbTrigger/config.groovy b/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbTrigger/config.groovy index 67eb02cf5..622a1926c 100644 --- a/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbTrigger/config.groovy +++ b/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbTrigger/config.groovy @@ -58,9 +58,9 @@ f.advanced() { } } } -//f.advanced(title: _("Trigger Setup")) { +f.advanced(title: _("Trigger Setup")) { f.entry(title: _("Trigger Setup")) { f.hetero_list(items: instance == null ? null : instance.extensions, name: "extensions", oneEach: "true", hasHeader: "true", descriptors: descriptor.getExtensionDescriptors()) } -//} +} From af6bb66ed05726ed39254000d3a28b33117d32b9 Mon Sep 17 00:00:00 2001 From: David Tanner Date: Sat, 13 Jun 2015 21:16:17 -0600 Subject: [PATCH 13/14] [maven-release-plugin] prepare release ghprb-1.23.2 --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 20e22118e..ae8175bf5 100644 --- a/pom.xml +++ b/pom.xml @@ -10,7 +10,7 @@ ghprb GitHub Pull Request Builder - 1.24-SNAPSHOT + 1.23.2 hpi https://wiki.jenkins-ci.org/display/JENKINS/GitHub+pull+request+builder+plugin @@ -31,7 +31,7 @@ scm:git:ssh://github.com/jenkinsci/ghprb-plugin.git scm:git:ssh://git@github.com/jenkinsci/ghprb-plugin.git https://github.com/jenkinsci/ghprb-plugin - HEAD + ghprb-1.23.2 From f43d5f9fb407ca754accd0affa12fd9644184619 Mon Sep 17 00:00:00 2001 From: David Tanner Date: Sat, 13 Jun 2015 21:16:20 -0600 Subject: [PATCH 14/14] [maven-release-plugin] prepare for next development iteration --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index ae8175bf5..20e22118e 100644 --- a/pom.xml +++ b/pom.xml @@ -10,7 +10,7 @@ ghprb GitHub Pull Request Builder - 1.23.2 + 1.24-SNAPSHOT hpi https://wiki.jenkins-ci.org/display/JENKINS/GitHub+pull+request+builder+plugin @@ -31,7 +31,7 @@ scm:git:ssh://github.com/jenkinsci/ghprb-plugin.git scm:git:ssh://git@github.com/jenkinsci/ghprb-plugin.git https://github.com/jenkinsci/ghprb-plugin - ghprb-1.23.2 + HEAD