Refactor acceptance tests to use ExpectedException

Move the declaration of ExpectedException out of AccountIT up to the
AbstractDaemonTest class, so it can be reused in all the derived test
classes.

Refactor the acceptance tests to use:

 exception.expect(Exception.class);

rather than surrounding the code under test in a try-catch-block and
explicitly calling fail() if the exception is not raised.

and:

 exception.expectMessage("Expected message");

rather than catching the exception and then checking the message with:

 assertThat(exception.getMessage()).isEqualTo("Expected message");

Change-Id: I6bbd476265898b19d1336385bc428ec488a5e47e

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java

@@ -71,6 +71,7 @@ import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.transport.Transport;
 import org.junit.AfterClass;
 import org.junit.Rule;
+import org.junit.rules.ExpectedException;
 import org.junit.rules.TestRule;
 import org.junit.runner.Description;
 import org.junit.runner.RunWith;
@@ -157,6 +158,9 @@ public abstract class AbstractDaemonTest {
   protected ReviewDb db;
   protected Project.NameKey project;
 
+  @Rule
+  public ExpectedException exception = ExpectedException.none();
+
   private String resourcePrefix;
   private List<Repository> toClose;
 

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java

@@ -23,16 +23,11 @@ import com.google.gerrit.extensions.api.accounts.EmailInput;
 import com.google.gerrit.extensions.common.AccountInfo;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 
-import org.junit.Rule;
 import org.junit.Test;
-import org.junit.rules.ExpectedException;
 
 import java.util.List;
 
 public class AccountIT extends AbstractDaemonTest {
-  @Rule
-  public ExpectedException exception = ExpectedException.none();
-
   @Test
   public void get() throws Exception {
     AccountInfo info = gApi

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java

@@ -18,7 +18,6 @@ import static com.google.common.truth.Truth.assertThat;
 import static com.google.gerrit.acceptance.PushOneCommit.FILE_CONTENT;
 import static com.google.gerrit.acceptance.PushOneCommit.FILE_NAME;
 import static org.eclipse.jgit.lib.Constants.HEAD;
-import static org.junit.Assert.fail;
 
 import com.google.common.base.Predicate;
 import com.google.common.collect.Iterables;
@@ -282,12 +281,9 @@ public class RevisionIT extends AbstractDaemonTest {
     cherry.current().review(ReviewInput.approve());
     cherry.current().submit();
 
-    try {
+    exception.expect(RestApiException.class);
+    exception.expectMessage("Cherry pick failed: identical tree");
     orig.revision(r.getCommit().name()).cherryPick(in);
-      fail("Cherry-pick identical tree error expected");
-    } catch (RestApiException e) {
-      assertThat(e.getMessage()).isEqualTo("Cherry pick failed: identical tree");
-    }
   }
 
   @Test
@@ -310,12 +306,9 @@ public class RevisionIT extends AbstractDaemonTest {
     ChangeApi orig = gApi.changes().id(triplet);
     assertThat(orig.get().messages).hasSize(1);
 
-    try {
+    exception.expect(RestApiException.class);
+    exception.expectMessage("Cherry pick failed: merge conflict");
     orig.revision(r.getCommit().name()).cherryPick(in);
-      fail("Cherry-pick merge conflict error expected");
-    } catch (RestApiException e) {
-      assertThat(e.getMessage()).isEqualTo("Cherry pick failed: merge conflict");
-    }
   }
 
   @Test

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java

@@ -23,7 +23,6 @@ import static org.apache.http.HttpStatus.SC_CONFLICT;
 import static org.apache.http.HttpStatus.SC_NOT_FOUND;
 import static org.apache.http.HttpStatus.SC_NO_CONTENT;
 import static org.apache.http.HttpStatus.SC_OK;
-import static org.junit.Assert.fail;
 
 import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableMap;
@@ -288,21 +287,25 @@ public class ChangeEditIT extends AbstractDaemonTest {
   }
 
   @Test
-  public void updateMessage() throws Exception {
+  public void updateMessageNoChange() throws Exception {
     assertThat(modifier.createEdit(change, getCurrentPatchSet(changeId)))
         .isEqualTo(RefUpdate.Result.NEW);
     Optional<ChangeEdit> edit = editUtil.byChange(change);
 
-    try {
+    exception.expect(UnchangedCommitMessageException.class);
+    exception.expectMessage(
+        "New commit message cannot be same as existing commit message");
     modifier.modifyMessage(
         edit.get(),
         edit.get().getEditCommit().getFullMessage());
-      fail("UnchangedCommitMessageException expected");
-    } catch (UnchangedCommitMessageException ex) {
-      assertThat(ex.getMessage()).isEqualTo(
-          "New commit message cannot be same as existing commit message");
   }
 
+  @Test
+  public void updateMessage() throws Exception {
+    assertThat(modifier.createEdit(change, getCurrentPatchSet(changeId)))
+        .isEqualTo(RefUpdate.Result.NEW);
+    Optional<ChangeEdit> edit = editUtil.byChange(change);
+
     String msg = String.format("New commit message\n\nChange-Id: %s",
         change.getKey());
     assertThat(modifier.modifyMessage(edit.get(), msg)).isEqualTo(
@@ -385,12 +388,9 @@ public class ChangeEditIT extends AbstractDaemonTest {
     assertThat(modifier.deleteFile(edit.get(), FILE_NAME)).isEqualTo(
         RefUpdate.Result.FORCED);
     edit = editUtil.byChange(change);
-    try {
+    exception.expect(ResourceNotFoundException.class);
     fileUtil.getContent(projectCache.get(edit.get().getChange().getProject()),
         ObjectId.fromString(edit.get().getRevision().get()), FILE_NAME);
-      fail("ResourceNotFoundException expected");
-    } catch (ResourceNotFoundException rnfe) {
-    }
   }
 
   @Test
@@ -402,12 +402,9 @@ public class ChangeEditIT extends AbstractDaemonTest {
     edit = editUtil.byChange(change);
     assertByteArray(fileUtil.getContent(projectCache.get(edit.get().getChange().getProject()),
         ObjectId.fromString(edit.get().getRevision().get()), FILE_NAME3), CONTENT_OLD);
-    try {
+    exception.expect(ResourceNotFoundException.class);
     fileUtil.getContent(projectCache.get(edit.get().getChange().getProject()),
         ObjectId.fromString(edit.get().getRevision().get()), FILE_NAME);
-      fail("ResourceNotFoundException expected");
-    } catch (ResourceNotFoundException rnfe) {
-    }
   }
 
   @Test
@@ -415,12 +412,9 @@ public class ChangeEditIT extends AbstractDaemonTest {
     RestResponse r = adminSession.delete(urlEditFile());
     assertThat(r.getStatusCode()).isEqualTo(SC_NO_CONTENT);
     Optional<ChangeEdit> edit = editUtil.byChange(change);
-    try {
+    exception.expect(ResourceNotFoundException.class);
     fileUtil.getContent(projectCache.get(edit.get().getChange().getProject()),
         ObjectId.fromString(edit.get().getRevision().get()), FILE_NAME);
-      fail("ResourceNotFoundException expected");
-    } catch (ResourceNotFoundException rnfe) {
-    }
   }
 
   @Test
@@ -435,12 +429,9 @@ public class ChangeEditIT extends AbstractDaemonTest {
     assertThat(adminSession.delete(urlEditFile()).getStatusCode()).isEqualTo(
         SC_NO_CONTENT);
     Optional<ChangeEdit> edit = editUtil.byChange(change);
-    try {
+    exception.expect(ResourceNotFoundException.class);
     fileUtil.getContent(projectCache.get(edit.get().getChange().getProject()),
         ObjectId.fromString(edit.get().getRevision().get()), FILE_NAME);
-      fail("ResourceNotFoundException expected");
-    } catch (ResourceNotFoundException rnfe) {
-    }
   }
 
   @Test
@@ -490,12 +481,9 @@ public class ChangeEditIT extends AbstractDaemonTest {
     Optional<ChangeEdit> edit = editUtil.byChange(change);
     assertByteArray(fileUtil.getContent(projectCache.get(edit.get().getChange().getProject()),
         ObjectId.fromString(edit.get().getRevision().get()), FILE_NAME3), CONTENT_OLD);
-    try {
+    exception.expect(ResourceNotFoundException.class);
     fileUtil.getContent(projectCache.get(edit.get().getChange().getProject()),
         ObjectId.fromString(edit.get().getRevision().get()), FILE_NAME);
-      fail("ResourceNotFoundException expected");
-    } catch (ResourceNotFoundException rnfe) {
-    }
   }
 
   @Test
@@ -595,14 +583,11 @@ public class ChangeEditIT extends AbstractDaemonTest {
     assertThat(adminSession.delete(urlEditFile()).getStatusCode()).isEqualTo(
         SC_NO_CONTENT);
     Optional<ChangeEdit> edit = editUtil.byChange(change);
-    try {
-      fileUtil.getContent(projectCache.get(edit.get().getChange().getProject()),
-          ObjectId.fromString(edit.get().getRevision().get()), FILE_NAME);
-      fail("ResourceNotFoundException expected");
-    } catch (ResourceNotFoundException rnfe) {
-    }
     RestResponse r = adminSession.get(urlEditFile());
     assertThat(r.getStatusCode()).isEqualTo(SC_NO_CONTENT);
+    exception.expect(ResourceNotFoundException.class);
+    fileUtil.getContent(projectCache.get(edit.get().getChange().getProject()),
+        ObjectId.fromString(edit.get().getRevision().get()), FILE_NAME);
   }
 
   @Test
@@ -635,15 +620,12 @@ public class ChangeEditIT extends AbstractDaemonTest {
   @Test
   public void writeNoChanges() throws Exception {
     assertThat(modifier.createEdit(change, ps)).isEqualTo(RefUpdate.Result.NEW);
-    try {
+    exception.expect(InvalidChangeOperationException.class);
+    exception.expectMessage("no changes were made");
     modifier.modifyFile(
         editUtil.byChange(change).get(),
         FILE_NAME,
         RestSession.newRawInput(CONTENT_OLD));
-      fail();
-    } catch (InvalidChangeOperationException e) {
-      assertThat(e.getMessage()).isEqualTo("no changes were made");
-    }
   }
 
   @Test

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java

@@ -16,7 +16,6 @@ package com.google.gerrit.acceptance.rest.change;
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.TruthJUnit.assume;
-import static org.junit.Assert.fail;
 
 import com.google.common.collect.Iterables;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
@@ -105,13 +104,9 @@ public class CreateChangeIT extends AbstractDaemonTest {
   private void assertCreateFails(ChangeInfo in,
       Class<? extends RestApiException> errType, String errSubstring)
       throws Exception {
-    try {
+    exception.expect(errType);
+    exception.expectMessage(errSubstring);
     gApi.changes().create(in);
-      fail("Expected " + errType.getSimpleName());
-    } catch (RestApiException expected) {
-      assertThat(expected).isInstanceOf(errType);
-      assertThat(expected.getMessage()).contains(errSubstring);
-    }
   }
 
   private ChangeStatus booleanToDraftStatus(Boolean draft) {

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java

@@ -18,7 +18,6 @@ import static com.google.common.truth.Truth.assertThat;
 import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
 import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
 import static com.google.gerrit.server.project.Util.block;
-import static org.junit.Assert.fail;
 
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.NoHttpd;
@@ -108,11 +107,7 @@ public class CreateBranchIT extends AbstractDaemonTest {
 
   private void assertCreateFails(Class<? extends RestApiException> errType)
       throws Exception {
-    try {
+    exception.expect(errType);
     branch().create(new BranchInput());
-      fail("Expected " + errType.getSimpleName());
-    } catch (RestApiException expected) {
-      assertThat(expected).isInstanceOf(errType);
-    }
   }
 }

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/CreateProjectIT.java

@@ -17,7 +17,6 @@ package com.google.gerrit.acceptance.rest.project;
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.gerrit.acceptance.rest.project.ProjectAssert.assertProjectInfo;
 import static com.google.gerrit.acceptance.rest.project.ProjectAssert.assertProjectOwners;
-import static org.junit.Assert.fail;
 
 import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
@@ -292,11 +291,7 @@ public class CreateProjectIT extends AbstractDaemonTest {
 
   private void assertCreateFails(ProjectInput in,
       Class<? extends RestApiException> errType) throws Exception {
-    try {
+    exception.expect(errType);
     gApi.projects().create(in);
-      fail("Expected " + errType.getSimpleName());
-    } catch (RestApiException expected) {
-      assertThat(expected).isInstanceOf(errType);
-    }
   }
 }

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/DeleteBranchIT.java

@@ -17,7 +17,6 @@ package com.google.gerrit.acceptance.rest.project;
 import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
 import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
 import static com.google.gerrit.server.project.Util.block;
-import static org.junit.Assert.fail;
 
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.NoHttpd;
@@ -94,21 +93,13 @@ public class DeleteBranchIT extends AbstractDaemonTest {
 
   private void assertDeleteSucceeds() throws Exception {
     branch().delete();
-    try {
+    exception.expect(ResourceNotFoundException.class);
     branch().get();
-      fail("Expected ResourceNotFoundException");
-    } catch (ResourceNotFoundException expected) {
-      // Expected.
-    }
   }
 
   private void assertDeleteForbidden() throws Exception {
-    try {
+    exception.expect(AuthException.class);
     branch().delete();
-      fail("Expected AuthException");
-    } catch (AuthException expected) {
-      // Expected.
-    }
     branch().get();
   }
 }

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/project/ListBranchesIT.java

@@ -16,7 +16,6 @@ package com.google.gerrit.acceptance.rest.project;
 
 import static com.google.gerrit.acceptance.rest.project.BranchAssert.assertBranches;
 import static com.google.gerrit.acceptance.rest.project.BranchAssert.assertRefNames;
-import static org.junit.Assert.fail;
 
 import com.google.common.collect.ImmutableList;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
@@ -32,24 +31,16 @@ import org.junit.Test;
 public class ListBranchesIT extends AbstractDaemonTest {
   @Test
   public void listBranchesOfNonExistingProject_NotFound() throws Exception {
-    try {
+    exception.expect(ResourceNotFoundException.class);
     gApi.projects().name("non-existing").branches().get();
-      fail("Expected ResourceNotFoundException");
-    } catch (ResourceNotFoundException expected) {
-      // Expected.
-    }
   }
 
   @Test
   public void listBranchesOfNonVisibleProject_NotFound() throws Exception {
     blockRead(project, "refs/*");
     setApiUser(user);
-    try {
+    exception.expect(ResourceNotFoundException.class);
     gApi.projects().name(project.get()).branches().get();
-      fail("Expected ResourceNotFoundException");
-    } catch (ResourceNotFoundException expected) {
-      // Expected.
-    }
   }
 
   @Test