Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pull request merge method #749

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
import hudson.tasks.Publisher;
import hudson.tasks.Recorder;
import hudson.util.FormValidation;
import hudson.util.ListBoxModel;
import jenkins.tasks.SimpleBuildStep;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.github.GHIssue;
import org.kohsuke.github.GHPullRequest;
import org.kohsuke.github.GHPullRequestCommitDetail;
Expand Down Expand Up @@ -42,17 +44,20 @@ public class GhprbPullRequestMerge extends Recorder implements SimpleBuildStep {

private final String mergeComment;

private final String mergeMethod;

private final Boolean failOnNonMerge;

private final Boolean deleteOnMerge;

private final Boolean allowMergeWithoutTriggerPhrase;

@DataBoundConstructor
public GhprbPullRequestMerge(String mergeComment, boolean onlyAdminsMerge, boolean disallowOwnCode, boolean failOnNonMerge,
boolean deleteOnMerge, boolean allowMergeWithoutTriggerPhrase) {
public GhprbPullRequestMerge(String mergeComment, String mergeMethod, boolean onlyAdminsMerge, boolean disallowOwnCode,
boolean failOnNonMerge, boolean deleteOnMerge, boolean allowMergeWithoutTriggerPhrase) {

this.mergeComment = mergeComment;
this.mergeMethod = mergeMethod;
this.onlyAdminsMerge = onlyAdminsMerge;
this.disallowOwnCode = disallowOwnCode;
this.failOnNonMerge = failOnNonMerge;
Expand All @@ -64,6 +69,10 @@ public String getMergeComment() {
return mergeComment;
}

public String getMergeMethod() {
return mergeMethod;
}

public boolean getOnlyAdminsMerge() {
return onlyAdminsMerge == null ? false : onlyAdminsMerge;
}
Expand Down Expand Up @@ -216,7 +225,7 @@ public void perform(
e.printStackTrace(listener.getLogger());
}
String mergeComment = Ghprb.replaceMacros(run, listener, getMergeComment());
pr.merge(mergeComment);
pr.merge(mergeComment, null, GHPullRequest.MergeMethod.valueOf(getMergeMethod()));
listener.getLogger().println("Pull request successfully merged");
deleteBranch(run, launcher, listener);
}
Expand Down Expand Up @@ -309,6 +318,18 @@ public FormValidation doCheck(@AncestorInPath Job<?, ?> project, @QueryParameter
FilePath buildDirectory = new FilePath(project.getBuildDir());
return FilePath.validateFileMask(buildDirectory, value);
}

public ListBoxModel doFillMergeMethodItems() {
ListBoxModel items = new ListBoxModel();
for (GHPullRequest.MergeMethod mergeMethod: GHPullRequest.MergeMethod.values()) {
items.add(StringUtils.capitalize(mergeMethod.name().toLowerCase()), mergeMethod.name());
}
return items;
}

public String defaultMergeMethod() {
return GHPullRequest.MergeMethod.MERGE.toString();
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ public Object mergeGithubPullRequest(Runnable closure) {

return new GhprbPullRequestMerge(
context.mergeComment,
context.mergeMethod,
context.onlyAdminsMerge,
context.disallowOwnCode,
context.failOnNonMerge,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
public class GhprbPullRequestMergeContext implements Context {
String mergeComment;

String mergeMethod;

boolean onlyAdminsMerge;

boolean disallowOwnCode;
Expand All @@ -22,6 +24,13 @@ public void mergeComment(String mergeComment) {
this.mergeComment = mergeComment;
}

/**
* @param mergeMethod Sets a merge method that should be used when the merge command is sent to GitHub.
*/
public void mergeMethod(String mergeMethod) {
this.mergeMethod = mergeMethod;
}

/**
* @param onlyAdminsMerge Allows only admin users to trigger a pull request merge. Defaults to {@code false}.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
<f:entry title="Merge comment" field="mergeComment">
<f:textarea />
</f:entry>
<f:entry title="Merge method" field="mergeMethod">
<f:select default="${descriptor.defaultMergeMethod()}"/>
</f:entry>
<f:entry title="Only Admin can merge code" field="onlyAdminsMerge">
<f:checkbox />
</f:entry>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<div>
Merge method to use (<a href="https://help.github.com/en/articles/about-merge-methods-on-github">About merge methods</a>).
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ public class GhprbPullRequestMergeTest {

private final String mergeComment = "merge";

private final GHPullRequest.MergeMethod mergeMethod = GHPullRequest.MergeMethod.MERGE;

private final Integer pullId = 1;

private Map<String, Object> triggerValues;
Expand Down Expand Up @@ -244,6 +246,7 @@ private GhprbPullRequestMerge setupMerger(

GhprbPullRequestMerge merger = spy(new GhprbPullRequestMerge(
mergeComment,
mergeMethod.name(),
onlyAdminsMerge,
disallowOwnCode,
failOnNonMerge,
Expand Down Expand Up @@ -272,35 +275,35 @@ public void testApproveMerge() throws Exception {

setupConditions(nonAdminLogin, committerName, committerEmail, triggerPhrase);
merger.perform(build, mockFilePath, launcher, listener);
verify(pr, times(1)).merge(mergeComment);
verify(pr, times(1)).merge(mergeComment, null, mergeMethod);

setupConditions(adminLogin, nonCommitterName, nonCommitterEmail, triggerPhrase);
merger.perform(build, mockFilePath, launcher, listener);
verify(pr, times(2)).merge(mergeComment);
verify(pr, times(2)).merge(mergeComment, null, mergeMethod);

setupConditions(adminLogin, committerName, committerEmail, nonTriggerPhrase);
merger.perform(build, mockFilePath, launcher, listener);
verify(pr, times(2)).merge(mergeComment);
verify(pr, times(2)).merge(mergeComment, null, mergeMethod);

setupConditions(nonAdminLogin, nonCommitterName, nonCommitterEmail, triggerPhrase);
merger.perform(build, mockFilePath, launcher, listener);
verify(pr, times(3)).merge(mergeComment);
verify(pr, times(3)).merge(mergeComment, null, mergeMethod);

setupConditions(nonAdminLogin, nonCommitterName, nonCommitterEmail, nonTriggerPhrase);
merger.perform(build, mockFilePath, launcher, listener);
verify(pr, times(3)).merge(mergeComment);
verify(pr, times(3)).merge(mergeComment, null, mergeMethod);

setupConditions(adminLogin, nonCommitterName, nonCommitterEmail, nonTriggerPhrase);
merger.perform(build, mockFilePath, launcher, listener);
verify(pr, times(3)).merge(mergeComment);
verify(pr, times(3)).merge(mergeComment, null, mergeMethod);

setupConditions(nonAdminLogin, nonCommitterName, nonCommitterEmail, nonTriggerPhrase);
merger.perform(build, mockFilePath, launcher, listener);
verify(pr, times(3)).merge(mergeComment);
verify(pr, times(3)).merge(mergeComment, null, mergeMethod);

setupConditions(adminLogin, committerName, committerEmail, triggerPhrase);
merger.perform(build, mockFilePath, launcher, listener);
verify(pr, times(4)).merge(mergeComment);
verify(pr, times(4)).merge(mergeComment, null, mergeMethod);
}

@Test
Expand All @@ -313,11 +316,11 @@ public void testAdminMerge() throws Exception {

setupConditions(adminLogin, committerName, committerEmail, triggerPhrase);
merger.perform(build, mockFilePath, launcher, listener);
verify(pr, times(1)).merge(mergeComment);
verify(pr, times(1)).merge(mergeComment, null, mergeMethod);

setupConditions(nonAdminLogin, committerName, committerEmail, triggerPhrase);
merger.perform(build, mockFilePath, launcher, listener);
verify(pr, times(1)).merge(mergeComment);
verify(pr, times(1)).merge(mergeComment, null, mergeMethod);
}

@Test
Expand All @@ -330,11 +333,11 @@ public void testTriggerMerge() throws Exception {

setupConditions(adminLogin, committerName, committerEmail, triggerPhrase);
merger.perform(build, mockFilePath, launcher, listener);
verify(pr, times(1)).merge(mergeComment);
verify(pr, times(1)).merge(mergeComment, null, mergeMethod);

setupConditions(adminLogin, committerName, committerEmail, nonTriggerPhrase);
merger.perform(build, mockFilePath, launcher, listener);
verify(pr, times(1)).merge(mergeComment);
verify(pr, times(1)).merge(mergeComment, null, mergeMethod);
}

@Test
Expand All @@ -347,11 +350,11 @@ public void testOwnCodeMerge() throws Exception {

setupConditions(adminLogin, nonCommitterName, nonCommitterEmail, triggerPhrase);
merger.perform(build, mockFilePath, launcher, listener);
verify(pr, times(1)).merge(mergeComment);
verify(pr, times(1)).merge(mergeComment, null, mergeMethod);

setupConditions(adminLogin, committerName, committerEmail, triggerPhrase);
merger.perform(build, mockFilePath, launcher, listener);
verify(pr, times(1)).merge(mergeComment);
verify(pr, times(1)).merge(mergeComment, null, mergeMethod);
}

@Test
Expand All @@ -364,39 +367,39 @@ public void testDenyMerge() throws Exception {

setupConditions(nonAdminLogin, nonAdminLogin, nonCommitterName, nonCommitterEmail, triggerPhrase);
merger.perform(build, mockFilePath, launcher, listener);
verify(pr, times(0)).merge(mergeComment);
verify(pr, times(0)).merge(mergeComment, null, mergeMethod);

setupConditions(nonAdminLogin, adminLogin, committerName, committerEmail, triggerPhrase);
merger.perform(build, mockFilePath, launcher, listener);
verify(pr, times(0)).merge(mergeComment);
verify(pr, times(0)).merge(mergeComment, null, mergeMethod);

setupConditions(nonAdminLogin, adminLogin, nonCommitterName, nonCommitterEmail, nonTriggerPhrase);
merger.perform(build, mockFilePath, launcher, listener);
verify(pr, times(0)).merge(mergeComment);
verify(pr, times(0)).merge(mergeComment, null, mergeMethod);

setupConditions(nonAdminLogin, nonAdminLogin, nonCommitterName, nonCommitterEmail, triggerPhrase);
merger.perform(build, mockFilePath, launcher, listener);
verify(pr, times(0)).merge(mergeComment);
verify(pr, times(0)).merge(mergeComment, null, mergeMethod);

setupConditions(nonAdminLogin, nonAdminLogin, nonCommitterName, nonCommitterEmail, nonTriggerPhrase);
merger.perform(build, mockFilePath, launcher, listener);
verify(pr, times(0)).merge(mergeComment);
verify(pr, times(0)).merge(mergeComment, null, mergeMethod);

setupConditions(nonAdminLogin, adminLogin, committerName, committerEmail, nonTriggerPhrase);
merger.perform(build, mockFilePath, launcher, listener);
verify(pr, times(0)).merge(mergeComment);
verify(pr, times(0)).merge(mergeComment, null, mergeMethod);

setupConditions(nonAdminLogin, nonAdminLogin, committerName, committerEmail, nonTriggerPhrase);
merger.perform(build, mockFilePath, launcher, listener);
verify(pr, times(0)).merge(mergeComment);
verify(pr, times(0)).merge(mergeComment, null, mergeMethod);

setupConditions(adminLogin, adminLogin, nonCommitterName, nonCommitterEmail, triggerPhrase);
merger.perform(build, mockFilePath, launcher, listener);
verify(pr, times(0)).merge(mergeComment);
verify(pr, times(0)).merge(mergeComment, null, mergeMethod);

setupConditions(nonAdminLogin, adminLogin, nonCommitterName, nonCommitterEmail, triggerPhrase);
merger.perform(build, mockFilePath, launcher, listener);
verify(pr, times(1)).merge(mergeComment);
verify(pr, times(1)).merge(mergeComment, null, mergeMethod);
}

@Test
Expand Down