RevisionIT: Extend 'submit' and 'submit on behalf of' tests

The existing test named 'submitOnBehalfOf' tests the case where the
user does not have permission.

Rename it to 'submitOnBehalfOfNotPermitted' to better reflect its
purpose.

Use ExpectedException rather than the 'expected' argument on the @Test
annotation, and add an additional check for the expected message.

Add a new test 'submitOnBehalfOf' that tests the case where the user
does have permission.

Add a new test that tests the case where the 'on behalf of' user is
not a valid account.

Add an assertion in the submit test that the change was merged.

There are still some cases that don't have coverage, but they can be
done in separate commits later:

- Caller does not have submit permission
- Destination branch does not exist
- Destination branch is not visible to on-behalf-of user
- Revision is not the current revision

Change-Id: Iad0df1b9262b9031293c7f2b42eb5fd1a9c95ecb
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java
index 7add9a2..def8317 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -19,6 +19,7 @@
 import static com.google.gerrit.acceptance.PushOneCommit.FILE_NAME;
 import static com.google.gerrit.acceptance.PushOneCommit.PATCH;
 import static com.google.gerrit.acceptance.PushOneCommit.SUBJECT;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.eclipse.jgit.lib.Constants.HEAD;
 import static org.junit.Assert.fail;
@@ -29,6 +30,7 @@
 import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.extensions.api.changes.ChangeApi;
 import com.google.gerrit.extensions.api.changes.CherryPickInput;
 import com.google.gerrit.extensions.api.changes.DraftApi;
@@ -38,6 +40,7 @@
 import com.google.gerrit.extensions.api.changes.RevisionApi;
 import com.google.gerrit.extensions.api.changes.SubmitInput;
 import com.google.gerrit.extensions.api.projects.BranchInput;
+import com.google.gerrit.extensions.client.ChangeStatus;
 import com.google.gerrit.extensions.client.SubmitType;
 import com.google.gerrit.extensions.common.ChangeInfo;
 import com.google.gerrit.extensions.common.ChangeMessageInfo;
@@ -49,9 +52,13 @@
 import com.google.gerrit.extensions.restapi.BinaryResult;
 import com.google.gerrit.extensions.restapi.ETagView;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 import com.google.gerrit.reviewdb.client.Patch;
 import com.google.gerrit.server.change.GetRevisionActions;
 import com.google.gerrit.server.change.RevisionResource;
+import com.google.gerrit.server.git.ProjectConfig;
+import com.google.gerrit.server.group.SystemGroupBackend;
+import com.google.gerrit.server.project.Util;
 import com.google.inject.Inject;
 
 import org.eclipse.jgit.lib.ObjectId;
@@ -119,18 +126,68 @@
   @Test
   public void submit() throws Exception {
     PushOneCommit.Result r = createChange();
+    String changeId = project.get() + "~master~" + r.getChangeId();
     gApi.changes()
-        .id(project.get() + "~master~" + r.getChangeId())
+        .id(changeId)
         .current()
         .review(ReviewInput.approve());
     gApi.changes()
-        .id(project.get() + "~master~" + r.getChangeId())
+        .id(changeId)
         .current()
         .submit();
+    assertThat(gApi.changes().id(changeId).get().status)
+        .isEqualTo(ChangeStatus.MERGED);
   }
 
-  @Test(expected = AuthException.class)
+  private void allowSubmitOnBehalfOf() throws Exception {
+    ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
+    Util.allow(cfg,
+        Permission.SUBMIT_AS,
+        SystemGroupBackend.getGroup(REGISTERED_USERS).getUUID(),
+        "refs/heads/*");
+    saveProjectConfig(project, cfg);
+  }
+
+  @Test
   public void submitOnBehalfOf() throws Exception {
+    allowSubmitOnBehalfOf();
+    PushOneCommit.Result r = createChange();
+    String changeId = project.get() + "~master~" + r.getChangeId();
+    gApi.changes()
+        .id(changeId)
+        .current()
+        .review(ReviewInput.approve());
+    SubmitInput in = new SubmitInput();
+    in.onBehalfOf = admin2.email;
+    gApi.changes()
+        .id(changeId)
+        .current()
+        .submit(in);
+    assertThat(gApi.changes().id(changeId).get().status)
+        .isEqualTo(ChangeStatus.MERGED);
+  }
+
+  @Test
+  public void submitOnBehalfOfInvalidUser() throws Exception {
+    allowSubmitOnBehalfOf();
+    PushOneCommit.Result r = createChange();
+    String changeId = project.get() + "~master~" + r.getChangeId();
+    gApi.changes()
+        .id(changeId)
+        .current()
+        .review(ReviewInput.approve());
+    SubmitInput in = new SubmitInput();
+    in.onBehalfOf = "doesnotexist";
+    exception.expect(UnprocessableEntityException.class);
+    exception.expectMessage("Account Not Found: doesnotexist");
+    gApi.changes()
+        .id(changeId)
+        .current()
+        .submit(in);
+  }
+
+  @Test
+  public void submitOnBehalfOfNotPermitted() throws Exception {
     PushOneCommit.Result r = createChange();
     gApi.changes()
         .id(project.get() + "~master~" + r.getChangeId())
@@ -138,6 +195,8 @@
         .review(ReviewInput.approve());
     SubmitInput in = new SubmitInput();
     in.onBehalfOf = admin2.email;
+    exception.expect(AuthException.class);
+    exception.expectMessage("submit on behalf of not permitted");
     gApi.changes()
         .id(project.get() + "~master~" + r.getChangeId())
         .current()