Do not build accepted merge requests unless specified so in the configuration.

The combination of state and action upon approval is "updated" and "approved". Therefore
such combination cannot be excluded by using logical or.

Therefore the meaning of the arguments to MergeRequestHookTriggerHandlerImpl changed,
and tests were adjusted properly.
This commit is contained in:
Patrik Dudits 2018-03-13 13:03:10 +01:00
parent 087796e0af
commit 554bb618a5
5 changed files with 107 additions and 6 deletions

View File

@ -32,7 +32,7 @@ public final class MergeRequestHookTriggerHandlerFactory {
}
private static Set<Action> retrieveAllowedActions(boolean triggerOnApprovedMergeRequest) {
Set<Action> allowedActions = EnumSet.noneOf(Action.class);
Set<Action> allowedActions = EnumSet.of(Action.open, Action.update);
if (triggerOnApprovedMergeRequest)
allowedActions.add(Action.approved);
return allowedActions;

View File

@ -38,7 +38,7 @@ class MergeRequestHookTriggerHandlerImpl extends AbstractWebHookTriggerHandler<M
private final Collection<Action> allowedActions;
MergeRequestHookTriggerHandlerImpl(Collection<State> allowedStates, boolean skipWorkInProgressMergeRequest) {
this(allowedStates, EnumSet.noneOf(Action.class),skipWorkInProgressMergeRequest);
this(allowedStates, EnumSet.allOf(Action.class),skipWorkInProgressMergeRequest);
}
MergeRequestHookTriggerHandlerImpl(Collection<State> allowedStates, Collection<Action> allowedActions, boolean skipWorkInProgressMergeRequest) {
@ -159,7 +159,7 @@ class MergeRequestHookTriggerHandlerImpl extends AbstractWebHookTriggerHandler<M
private boolean isAllowedByConfig(MergeRequestObjectAttributes objectAttributes) {
return allowedStates.contains(objectAttributes.getState())
|| allowedActions.contains(objectAttributes.getAction());
&& allowedActions.contains(objectAttributes.getAction());
}
private boolean isNotSkipWorkInProgressMergeRequest(MergeRequestObjectAttributes objectAttributes) {

View File

@ -121,11 +121,19 @@ public class MergeRequestHookTriggerHandlerImplTest {
@Test
public void mergeRequest_build_when_approved() throws IOException, InterruptedException, GitAPIException, ExecutionException {
MergeRequestHookTriggerHandler mergeRequestHookTriggerHandler = new MergeRequestHookTriggerHandlerImpl(EnumSet.noneOf(State.class), EnumSet.of(Action.approved), false);
MergeRequestHookTriggerHandler mergeRequestHookTriggerHandler = new MergeRequestHookTriggerHandlerImpl(EnumSet.allOf(State.class), EnumSet.of(Action.approved), false);
OneShotEvent buildTriggered = doHandle(mergeRequestHookTriggerHandler, Action.approved);
assertThat(buildTriggered.isSignaled(), is(true));
}
@Test
public void mergeRequest_do_not_build_when_when_approved() throws Exception {
MergeRequestHookTriggerHandler mergeRequestHookTriggerHandler = new MergeRequestHookTriggerHandlerImpl(EnumSet.allOf(State.class), EnumSet.of(Action.update), false);
OneShotEvent buildTriggered = doHandle(mergeRequestHookTriggerHandler, defaultMergeRequestObjectAttributes().withState(State.opened).withAction(Action.approved));
assertThat(buildTriggered.isSignaled(), is (false));
}
@Test
public void mergeRequest_build_only_when_approved_and_not_when_opened() throws IOException, InterruptedException, GitAPIException, ExecutionException {
@ -140,14 +148,14 @@ public class MergeRequestHookTriggerHandlerImplTest {
private void mergeRequest_build_only_when_approved(Action action)
throws GitAPIException, IOException, InterruptedException {
MergeRequestHookTriggerHandler mergeRequestHookTriggerHandler = new MergeRequestHookTriggerHandlerImpl(EnumSet.noneOf(State.class), EnumSet.of(Action.approved), false);
MergeRequestHookTriggerHandler mergeRequestHookTriggerHandler = new MergeRequestHookTriggerHandlerImpl(EnumSet.allOf(State.class), EnumSet.of(Action.approved), false);
OneShotEvent buildTriggered = doHandle(mergeRequestHookTriggerHandler, action);
assertThat(buildTriggered.isSignaled(), is(false));
}
private OneShotEvent doHandle(MergeRequestHookTriggerHandler mergeRequestHookTriggerHandler, Action action) throws GitAPIException, IOException, InterruptedException {
return doHandle(mergeRequestHookTriggerHandler, defaultMergeRequestObjectAttributes().withState(State.opened).withAction(action));
return doHandle(mergeRequestHookTriggerHandler, defaultMergeRequestObjectAttributes().withAction(action));
}
private OneShotEvent doHandle(MergeRequestHookTriggerHandler mergeRequestHookTriggerHandler, State state) throws GitAPIException, IOException, InterruptedException {
@ -196,6 +204,8 @@ public class MergeRequestHookTriggerHandlerImplTest {
private MergeRequestObjectAttributesBuilder defaultMergeRequestObjectAttributes() {
return mergeRequestObjectAttributes()
.withIid(1)
.withAction(Action.update)
.withState(State.opened)
.withTitle("test")
.withTargetProjectId(1)
.withSourceProjectId(1)

View File

@ -100,6 +100,20 @@ public class MergeRequestBuildActionTest {
verify(trigger, never()).onPost(any(MergeRequestHook.class));
}
@Test
public void skip_approvedMR() throws IOException, ExecutionException, InterruptedException {
FreeStyleProject testProject = jenkins.createFreeStyleProject();
testProject.addTrigger(trigger);
testProject.setScm(new GitSCM(gitRepoUrl));
QueueTaskFuture<?> future = testProject.scheduleBuild2(0, new ParametersAction(new StringParameterValue("gitlabTargetBranch", "master")));
future.get();
exception.expect(HttpResponses.HttpResponseException.class);
new MergeRequestBuildAction(testProject, getJson("MergeRequestEvent_approvedMR.json"), null).execute(response);
verify(trigger, never()).onPost(any(MergeRequestHook.class));
}
@Test
public void skip_alreadyBuiltMR() throws IOException, ExecutionException, InterruptedException {
FreeStyleProject testProject = jenkins.createFreeStyleProject();

View File

@ -0,0 +1,77 @@
{
"object_kind": "merge_request",
"user": {
"name": "Administrator",
"username": "root",
"avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=40\u0026d=identicon"
},
"object_attributes": {
"id": 99,
"target_branch": "master",
"source_branch": "ms-viewport",
"source_project_id": 14,
"author_id": 51,
"assignee_id": 6,
"title": "MS-Viewport",
"created_at": "2013-12-03T17:23:34.123Z",
"updated_at": "2013-12-03T17:23:34.123Z",
"st_commits": null,
"st_diffs": null,
"milestone_id": null,
"state": "opened",
"merge_status": "unchecked",
"target_project_id": 14,
"iid": 1,
"description": "",
"source": {
"name": "Awesome Project",
"description": "Aut reprehenderit ut est.",
"web_url": "http://example.com/awesome_space/awesome_project",
"avatar_url": null,
"git_ssh_url": "git@example.com:awesome_space/awesome_project.git",
"git_http_url": "http://example.com/awesome_space/awesome_project.git",
"namespace": "Awesome Space",
"visibility_level": 20,
"path_with_namespace": "awesome_space/awesome_project",
"default_branch": "master",
"homepage": "http://example.com/awesome_space/awesome_project",
"url": "http://example.com/awesome_space/awesome_project.git",
"ssh_url": "git@example.com:awesome_space/awesome_project.git",
"http_url": "http://example.com/awesome_space/awesome_project.git"
},
"target": {
"name": "Awesome Project",
"description": "Aut reprehenderit ut est.",
"web_url": "http://example.com/awesome_space/awesome_project",
"avatar_url": null,
"git_ssh_url": "git@example.com:awesome_space/awesome_project.git",
"git_http_url": "http://example.com/awesome_space/awesome_project.git",
"namespace": "Awesome Space",
"visibility_level": 20,
"path_with_namespace": "awesome_space/awesome_project",
"default_branch": "master",
"homepage": "http://example.com/awesome_space/awesome_project",
"url": "http://example.com/awesome_space/awesome_project.git",
"ssh_url": "git@example.com:awesome_space/awesome_project.git",
"http_url": "http://example.com/awesome_space/awesome_project.git"
},
"last_commit": {
"id": "${commitSha1}",
"message": "fixed readme",
"timestamp": "2012-01-03T23:36:29+02:00",
"url": "http://example.com/awesome_space/awesome_project/commits/da1560886d4f094c3e6c9ef40349f7d38b5d27d7",
"author": {
"name": "GitLab dev user",
"email": "gitlabdev@dv6700.(none)"
}
},
"work_in_progress": false,
"url": "http://example.com/diaspora/merge_requests/1",
"action": "approved",
"assignee": {
"name": "User1",
"username": "user1",
"avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=40\u0026d=identicon"
}
}
}