Refactor unit tests to use ExpectedException

Instead of catching the expected exception and calling fail() when
the exception is not raised, use the ExpectedException.expect() rule
to confirm that the expected exception is raised.

In cases where a specific exception message is expected, instead of
catching the exception and then asserting on the value returned from
its getMessage(), use the ExpectedException.expectMessage() rule.

Change-Id: I15a61bbed33e11f77c5d0fac02374630f540a68e

gerrit-gwtexpui/src/test/java/com/google/gwtexpui/safehtml/client/LinkFindReplaceTest.java

@@ -18,11 +18,15 @@ import static com.google.gwtexpui.safehtml.client.LinkFindReplace.hasValidScheme
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
 
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 public class LinkFindReplaceTest {
+  @Rule
+  public ExpectedException exception = ExpectedException.none();
+
   @Test
   public void testNoEscaping() {
     String find = "find";
@@ -55,21 +59,15 @@ public class LinkFindReplaceTest {
 
   @Test
   public void testInvalidSchemeInReplace() {
-    try {
+    exception.expect(IllegalArgumentException.class);
     new LinkFindReplace("find", "javascript:alert(1)").replace("find");
-      fail("Expected IllegalStateException");
-    } catch (IllegalArgumentException expected) {
-    }
   }
 
   @Test
   public void testInvalidSchemeWithBackreference() {
-    try {
+    exception.expect(IllegalArgumentException.class);
     new LinkFindReplace(".*(script:[^;]*)", "java$1")
         .replace("Look at this script: alert(1);");
-      fail("Expected IllegalStateException");
-    } catch (IllegalArgumentException expected) {
-    }
   }
 
   @Test

gerrit-gwtexpui/src/test/java/com/google/gwtexpui/safehtml/client/SafeHtmlBuilderTest.java

@@ -20,11 +20,15 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
 
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 public class SafeHtmlBuilderTest {
+  @Rule
+  public ExpectedException exception = ExpectedException.none();
+
   @Test
   public void testEmpty() {
     final SafeHtmlBuilder b = new SafeHtmlBuilder();
@@ -258,34 +262,25 @@ public class SafeHtmlBuilderTest {
   @Test
   public void testRejectJavaScript_AnchorHref() {
     final String href = "javascript:window.close();";
-    try {
+    exception.expect(RuntimeException.class);
+    exception.expectMessage("javascript unsafe in href: " + href);
     new SafeHtmlBuilder().openAnchor().setAttribute("href", href);
-      fail("accepted javascript in a href");
-    } catch (RuntimeException e) {
-      assertEquals("javascript unsafe in href: " + href, e.getMessage());
-    }
   }
 
   @Test
   public void testRejectJavaScript_ImgSrc() {
     final String href = "javascript:window.close();";
-    try {
+    exception.expect(RuntimeException.class);
+    exception.expectMessage("javascript unsafe in href: " + href);
     new SafeHtmlBuilder().openElement("img").setAttribute("src", href);
-      fail("accepted javascript in img src");
-    } catch (RuntimeException e) {
-      assertEquals("javascript unsafe in href: " + href, e.getMessage());
-    }
   }
 
   @Test
   public void testRejectJavaScript_FormAction() {
     final String href = "javascript:window.close();";
-    try {
+    exception.expect(RuntimeException.class);
+    exception.expectMessage("javascript unsafe in href: " + href);
     new SafeHtmlBuilder().openElement("form").setAttribute("action", href);
-      fail("accepted javascript in form action");
-    } catch (RuntimeException e) {
-      assertEquals("javascript unsafe in href: " + href, e.getMessage());
-    }
   }
 
   private static String escape(final char c) {

gerrit-server/src/test/java/com/google/gerrit/rules/GerritCommonTest.java

@@ -14,12 +14,10 @@
 
 package com.google.gerrit.rules;
 
-import static com.google.common.truth.Truth.assertThat;
 import static com.google.gerrit.common.data.Permission.LABEL;
 import static com.google.gerrit.server.project.Util.allow;
 import static com.google.gerrit.server.project.Util.category;
 import static com.google.gerrit.server.project.Util.value;
-import static org.junit.Assert.fail;
 
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.common.data.LabelType;
@@ -42,7 +40,9 @@ import com.googlecode.prolog_cafe.lang.SymbolTerm;
 
 import org.eclipse.jgit.lib.Config;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 import java.io.PushbackReader;
 import java.io.StringReader;
@@ -62,6 +62,9 @@ public class GerritCommonTest extends PrologTestCase {
   private ProjectConfig local;
   private Util util;
 
+  @Rule
+  public ExpectedException exception = ExpectedException.none();
+
   @Before
   public void setUp() throws Exception {
     util = new Util();
@@ -125,12 +128,9 @@ public class GerritCommonTest extends PrologTestCase {
       throw new CompileException("Cannot consult " + nameTerm);
     }
 
-    try {
+    exception.expect(ReductionLimitException.class);
+    exception.expectMessage("exceeded reduction limit of 1300");
     env.once(Prolog.BUILTIN, "call", new StructureTerm(":",
         SymbolTerm.create("user"), SymbolTerm.create("loopy")));
-      fail("long running loop did not abort with ReductionLimitException");
-    } catch (ReductionLimitException e) {
-      assertThat(e.getMessage()).isEqualTo("exceeded reduction limit of 1300");
-    }
   }
 }

gerrit-server/src/test/java/com/google/gerrit/server/config/SitePathsTest.java

@@ -19,11 +19,12 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
 
 import com.google.gerrit.server.util.HostPlatform;
 
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 import java.io.IOException;
 import java.nio.file.Files;
@@ -32,6 +33,9 @@ import java.nio.file.Path;
 import java.nio.file.Paths;
 
 public class SitePathsTest {
+  @Rule
+  public ExpectedException exception = ExpectedException.none();
+
   @Test
   public void testCreate_NotExisting() throws IOException {
     final Path root = random();
@@ -77,12 +81,8 @@ public class SitePathsTest {
     final Path root = random();
     try {
       Files.createFile(root);
-      try {
+      exception.expect(NotDirectoryException.class);
       new SitePaths(root);
-        fail("Did not throw exception");
-      } catch (NotDirectoryException e) {
-        // Expected.
-      }
     } finally {
       Files.delete(root);
     }

gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java

@@ -14,8 +14,6 @@
 
 package com.google.gerrit.server.notedb;
 
-import static org.junit.Assert.fail;
-
 import com.google.gerrit.common.TimeUtil;
 
 import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -29,12 +27,17 @@ import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
 import org.junit.After;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 public class ChangeNotesParserTest extends AbstractChangeNotesTest {
   private TestRepository<InMemoryRepository> testRepo;
   private RevWalk walk;
 
+  @Rule
+  public ExpectedException exception = ExpectedException.none();
+
   @Before
   public void setUpTestRepo() throws Exception {
     testRepo = new TestRepository<>(repo);
@@ -202,10 +205,8 @@ public class ChangeNotesParserTest extends AbstractChangeNotesTest {
 
   private void assertParseFails(RevCommit commit) throws Exception {
     try (ChangeNotesParser parser = newParser(commit)) {
+      exception.expect(ConfigInvalidException.class);
       parser.parseAll();
-      fail("Expected parse to fail:\n" + commit.getFullMessage());
-    } catch (ConfigInvalidException e) {
-      // Expected.
     }
   }
 

gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java

@@ -20,7 +20,6 @@ import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.concurrent.TimeUnit.HOURS;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static java.util.concurrent.TimeUnit.MINUTES;
-import static org.junit.Assert.fail;
 
 import com.google.common.base.Function;
 import com.google.common.base.MoreObjects;
@@ -74,7 +73,9 @@ import org.joda.time.DateTimeUtils.MillisProvider;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Ignore;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.junit.runner.RunWith;
 
 import java.util.Arrays;
@@ -94,6 +95,9 @@ public abstract class AbstractQueryChangesTest {
     return updateConfig(NotesMigration.allEnabledConfig());
   }
 
+  @Rule
+  public ExpectedException exception = ExpectedException.none();
+
   private static Config updateConfig(Config cfg) {
     cfg.setInt("index", null, "maxPages", 10);
     return cfg;
@@ -1100,12 +1104,8 @@ public abstract class AbstractQueryChangesTest {
   }
 
   protected void assertBadQuery(QueryRequest query) throws Exception {
-    try {
+    exception.expect(BadRequestException.class);
     query.get();
-      fail("expected BadRequestException for query: " + query);
-    } catch (BadRequestException e) {
-      // Expected.
-    }
   }
 
   protected TestRepository<Repo> createProject(String name) throws Exception {

gerrit-server/src/test/java/com/google/gerrit/server/util/SocketUtilTest.java

@@ -23,9 +23,10 @@ import static java.net.InetSocketAddress.createUnresolved;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
 
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 import java.net.Inet4Address;
 import java.net.Inet6Address;
@@ -34,6 +35,9 @@ import java.net.InetSocketAddress;
 import java.net.UnknownHostException;
 
 public class SocketUtilTest {
+  @Rule
+  public ExpectedException exception = ExpectedException.none();
+
   @Test
   public void testIsIPv6() throws UnknownHostException {
     final InetAddress ipv6 = getByName("1:2:3:4:5:6:7:8");
@@ -92,20 +96,20 @@ public class SocketUtilTest {
         parse("[foo.bar.example.com]:1234", 80));
     assertEquals(createUnresolved("foo.bar.example.com", 80), //
         parse("[foo.bar.example.com]", 80));
+  }
 
-    try {
+  @Test
+  public void testParseInvalidIPv6() {
+    exception.expect(IllegalArgumentException.class);
+    exception.expectMessage("invalid IPv6: [:3");
     parse("[:3", 80);
-      fail("did not throw exception");
-    } catch (IllegalArgumentException e) {
-      assertEquals("invalid IPv6: [:3", e.getMessage());
   }
 
-    try {
+  @Test
+  public void testParseInvalidPort() {
+    exception.expect(IllegalArgumentException.class);
+    exception.expectMessage("invalid port: localhost:A");
     parse("localhost:A", 80);
-      fail("did not throw exception");
-    } catch (IllegalArgumentException e) {
-      assertEquals("invalid port: localhost:A", e.getMessage());
-    }
   }
 
   @Test