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
diff --git a/gerrit-gwtexpui/src/test/java/com/google/gwtexpui/safehtml/client/LinkFindReplaceTest.java b/gerrit-gwtexpui/src/test/java/com/google/gwtexpui/safehtml/client/LinkFindReplaceTest.java index 0d8336e..e4c801f7 100644 --- a/gerrit-gwtexpui/src/test/java/com/google/gwtexpui/safehtml/client/LinkFindReplaceTest.java +++ b/gerrit-gwtexpui/src/test/java/com/google/gwtexpui/safehtml/client/LinkFindReplaceTest.java
@@ -18,11 +18,15 @@ 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 @@ @Test public void testInvalidSchemeInReplace() { - try { - new LinkFindReplace("find", "javascript:alert(1)").replace("find"); - fail("Expected IllegalStateException"); - } catch (IllegalArgumentException expected) { - } + exception.expect(IllegalArgumentException.class); + new LinkFindReplace("find", "javascript:alert(1)").replace("find"); } @Test public void testInvalidSchemeWithBackreference() { - try { - new LinkFindReplace(".*(script:[^;]*)", "java$1") - .replace("Look at this script: alert(1);"); - fail("Expected IllegalStateException"); - } catch (IllegalArgumentException expected) { - } + exception.expect(IllegalArgumentException.class); + new LinkFindReplace(".*(script:[^;]*)", "java$1") + .replace("Look at this script: alert(1);"); } @Test
diff --git a/gerrit-gwtexpui/src/test/java/com/google/gwtexpui/safehtml/client/SafeHtmlBuilderTest.java b/gerrit-gwtexpui/src/test/java/com/google/gwtexpui/safehtml/client/SafeHtmlBuilderTest.java index 0163d7f..b2cbc92 100644 --- a/gerrit-gwtexpui/src/test/java/com/google/gwtexpui/safehtml/client/SafeHtmlBuilderTest.java +++ b/gerrit-gwtexpui/src/test/java/com/google/gwtexpui/safehtml/client/SafeHtmlBuilderTest.java
@@ -20,11 +20,15 @@ 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 @@ @Test public void testRejectJavaScript_AnchorHref() { final String href = "javascript:window.close();"; - try { - new SafeHtmlBuilder().openAnchor().setAttribute("href", href); - fail("accepted javascript in a href"); - } catch (RuntimeException e) { - assertEquals("javascript unsafe in href: " + href, e.getMessage()); - } + exception.expect(RuntimeException.class); + exception.expectMessage("javascript unsafe in href: " + href); + new SafeHtmlBuilder().openAnchor().setAttribute("href", href); } @Test public void testRejectJavaScript_ImgSrc() { final String href = "javascript:window.close();"; - try { - new SafeHtmlBuilder().openElement("img").setAttribute("src", href); - fail("accepted javascript in img src"); - } catch (RuntimeException e) { - assertEquals("javascript unsafe in href: " + href, e.getMessage()); - } + exception.expect(RuntimeException.class); + exception.expectMessage("javascript unsafe in href: " + href); + new SafeHtmlBuilder().openElement("img").setAttribute("src", href); } @Test public void testRejectJavaScript_FormAction() { final String href = "javascript:window.close();"; - try { - new SafeHtmlBuilder().openElement("form").setAttribute("action", href); - fail("accepted javascript in form action"); - } catch (RuntimeException e) { - assertEquals("javascript unsafe in href: " + href, e.getMessage()); - } + exception.expect(RuntimeException.class); + exception.expectMessage("javascript unsafe in href: " + href); + new SafeHtmlBuilder().openElement("form").setAttribute("action", href); } private static String escape(final char c) {
diff --git a/gerrit-server/src/test/java/com/google/gerrit/rules/GerritCommonTest.java b/gerrit-server/src/test/java/com/google/gerrit/rules/GerritCommonTest.java index 74ab572..d6a5c67 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/rules/GerritCommonTest.java +++ b/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 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 @@ 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 @@ throw new CompileException("Cannot consult " + nameTerm); } - try { - 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"); - } + exception.expect(ReductionLimitException.class); + exception.expectMessage("exceeded reduction limit of 1300"); + env.once(Prolog.BUILTIN, "call", new StructureTerm(":", + SymbolTerm.create("user"), SymbolTerm.create("loopy"))); } }
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/config/SitePathsTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/config/SitePathsTest.java index 5533d53..743e43d 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/config/SitePathsTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/config/SitePathsTest.java
@@ -19,11 +19,12 @@ 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.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 @@ final Path root = random(); try { Files.createFile(root); - try { - new SitePaths(root); - fail("Did not throw exception"); - } catch (NotDirectoryException e) { - // Expected. - } + exception.expect(NotDirectoryException.class); + new SitePaths(root); } finally { Files.delete(root); }
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java index c41c4ec..bac72f0 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java +++ b/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.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 @@ 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. } }
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index fd36097..4cee5c7 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -20,7 +20,6 @@ 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.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 @@ 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 @@ } protected void assertBadQuery(QueryRequest query) throws Exception { - try { - query.get(); - fail("expected BadRequestException for query: " + query); - } catch (BadRequestException e) { - // Expected. - } + exception.expect(BadRequestException.class); + query.get(); } protected TestRepository<Repo> createProject(String name) throws Exception {
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/util/SocketUtilTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/util/SocketUtilTest.java index 5ce8882..c060aaf 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/util/SocketUtilTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/util/SocketUtilTest.java
@@ -23,9 +23,10 @@ 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.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 @@ parse("[foo.bar.example.com]:1234", 80)); assertEquals(createUnresolved("foo.bar.example.com", 80), // parse("[foo.bar.example.com]", 80)); + } - try { - parse("[:3", 80); - fail("did not throw exception"); - } catch (IllegalArgumentException e) { - assertEquals("invalid IPv6: [:3", e.getMessage()); - } + @Test + public void testParseInvalidIPv6() { + exception.expect(IllegalArgumentException.class); + exception.expectMessage("invalid IPv6: [:3"); + parse("[:3", 80); + } - try { - parse("localhost:A", 80); - fail("did not throw exception"); - } catch (IllegalArgumentException e) { - assertEquals("invalid port: localhost:A", e.getMessage()); - } + @Test + public void testParseInvalidPort() { + exception.expect(IllegalArgumentException.class); + exception.expectMessage("invalid port: localhost:A"); + parse("localhost:A", 80); } @Test