Add assertThrows

This is a new assertion that will be introduced in JUnit 4.13. Unlike
ExpectedException rule, this makes it easy to test other aspects of the
thrown exception, such like ServiceMayNotContinueException's status
code. Introduce this as before making changes to UploadPackTest more.

Change-Id: Ied7b3071ffcd0e93eece35b01e0abc5ff65645f2
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>

org.eclipse.jgit.test/src/org/eclipse/jgit/lib/MoreAsserts.java

@@ -0,0 +1,79 @@
+/*
+ * Copyright (C) 2019, Google LLC
+ * and other copyright owners as documented in the project's IP log.
+ *
+ * This program and the accompanying materials are made available
+ * under the terms of the Eclipse Distribution License v1.0 which
+ * accompanies this distribution, is reproduced below, and is
+ * available at http://www.eclipse.org/org/documents/edl-v10.php
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Eclipse Foundation, Inc. nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.eclipse.jgit.lib;
+
+/** Assertion methods. */
+public class MoreAsserts {
+	/**
+	 * Simple version of assertThrows that will be introduced in JUnit 4.13.
+	 *
+	 * @param expected
+	 *            Expected throwable class
+	 * @param r
+	 *            Runnable that is expected to throw an exception.
+	 * @return The thrown exception.
+	 */
+	public static <T extends Throwable> T assertThrows(Class<T> expected,
+			ThrowingRunnable r) {
+		try {
+			r.run();
+		} catch (Throwable actual) {
+			if (expected.isAssignableFrom(actual.getClass())) {
+				@SuppressWarnings("unchecked")
+				T toReturn = (T) actual;
+				return toReturn;
+			}
+			throw new AssertionError("Expected " + expected.getSimpleName()
+					+ ", but got " + actual.getClass().getSimpleName(), actual);
+		}
+		throw new AssertionError(
+				"Expected " + expected.getSimpleName() + " to be thrown");
+	}
+
+	public interface ThrowingRunnable {
+		void run() throws Throwable;
+	}
+
+	private MoreAsserts() {
+	}
+}

org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/UploadPackTest.java

@@ -1,5 +1,6 @@
 package org.eclipse.jgit.transport;
 
+import static org.eclipse.jgit.lib.MoreAsserts.assertThrows;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.hasItems;
 import static org.hamcrest.Matchers.is;
@@ -50,20 +51,14 @@ import org.eclipse.jgit.revwalk.RevTree;
 import org.eclipse.jgit.storage.pack.PackStatistics;
 import org.eclipse.jgit.transport.UploadPack.RequestPolicy;
 import org.eclipse.jgit.util.io.NullOutputStream;
-import org.hamcrest.Matchers;
 import org.junit.After;
 import org.junit.Before;
-import org.junit.Rule;
 import org.junit.Test;
-import org.junit.rules.ExpectedException;
 
 /**
  * Tests for server upload-pack utilities.
  */
 public class UploadPackTest {
-	@Rule
-	public ExpectedException thrown = ExpectedException.none();
-
 	private URIish uri;
 
 	private TestProtocol<Object> testProtocol;
@@ -147,11 +142,11 @@ public class UploadPackTest {
 		assertFalse(client.getObjectDatabase().has(blob.toObjectId()));
 
 		try (Transport tn = testProtocol.open(uri, client, "server")) {
-			thrown.expect(TransportException.class);
+			TransportException e = assertThrows(TransportException.class,
-			thrown.expectMessage(Matchers.containsString(
+					() -> tn.fetch(NullProgressMonitor.INSTANCE, Collections
-						"want " + blob.name() + " not valid"));
+							.singletonList(new RefSpec(blob.name()))));
-			tn.fetch(NullProgressMonitor.INSTANCE,
+			assertThat(e.getMessage(),
-					Collections.singletonList(new RefSpec(blob.name())));
+					containsString("want " + blob.name() + " not valid"));
 		}
 	}
 
@@ -186,11 +181,12 @@ public class UploadPackTest {
 		assertFalse(client.getObjectDatabase().has(blob.toObjectId()));
 
 		try (Transport tn = testProtocol.open(uri, client, "server")) {
-			thrown.expect(TransportException.class);
+			TransportException e = assertThrows(TransportException.class,
-			thrown.expectMessage(Matchers.containsString(
+					() -> tn.fetch(NullProgressMonitor.INSTANCE, Collections
+							.singletonList(new RefSpec(blob.name()))));
+			assertThat(e.getMessage(),
+					containsString(
 						"want " + blob.name() + " not valid"));
-			tn.fetch(NullProgressMonitor.INSTANCE,
-					Collections.singletonList(new RefSpec(blob.name())));
 		}
 	}
 
@@ -416,12 +412,11 @@ public class UploadPackTest {
 			try (Transport tn = testProtocol.open(uri, client, "server2")) {
 				tn.setFilterSpec(FilterSpec.withBlobLimit(0));
 
-				thrown.expect(TransportException.class);
+				TransportException e = assertThrows(TransportException.class,
-				thrown.expectMessage(
+						() -> tn.fetch(NullProgressMonitor.INSTANCE, Collections
-						"filter requires server to advertise that capability");
+								.singletonList(new RefSpec(commit.name()))));
-
+				assertThat(e.getMessage(), containsString(
-				tn.fetch(NullProgressMonitor.INSTANCE,
+						"filter requires server to advertise that capability"));
-						Collections.singletonList(new RefSpec(commit.name())));
 			}
 		}
 	}
@@ -739,13 +734,12 @@ public class UploadPackTest {
 
 	@Test
 	public void testV2LsRefsUnrecognizedArgument() throws Exception {
-		thrown.expect(PackProtocolException.class);
+		PackProtocolException e = assertThrows(PackProtocolException.class,
-		thrown.expectMessage("unexpected invalid-argument");
+				() -> uploadPackV2("command=ls-refs\n",
-		uploadPackV2(
+						PacketLineIn.delimiter(), "invalid-argument\n",
-			"command=ls-refs\n",
+						PacketLineIn.end()));
-			PacketLineIn.delimiter(),
+		assertThat(e.getMessage(),
-			"invalid-argument\n",
+				containsString("unexpected invalid-argument"));
-				PacketLineIn.end());
 	}
 
 	@Test
@@ -792,119 +786,88 @@ public class UploadPackTest {
 		remote.update("branch1", advertized);
 
 		// This works
-		uploadPackV2(
+		uploadPackV2(RequestPolicy.ADVERTISED, null, null, "command=fetch\n",
-			RequestPolicy.ADVERTISED,
+				PacketLineIn.delimiter(), "want " + advertized.name() + "\n",
-			null,
-			null,
-			"command=fetch\n",
-			PacketLineIn.delimiter(),
-			"want " + advertized.name() + "\n",
 				PacketLineIn.end());
 
 		// This doesn't
-		thrown.expect(TransportException.class);
+		TransportException e = assertThrows(TransportException.class,
-		thrown.expectMessage(Matchers.containsString(
+				() -> uploadPackV2(RequestPolicy.ADVERTISED, null, null,
-					"want " + unadvertized.name() + " not valid"));
+						"command=fetch\n", PacketLineIn.delimiter(),
-		uploadPackV2(
-			RequestPolicy.ADVERTISED,
-			null,
-			null,
-			"command=fetch\n",
-			PacketLineIn.delimiter(),
 						"want " + unadvertized.name() + "\n",
-				PacketLineIn.end());
+						PacketLineIn.end()));
+		assertThat(e.getMessage(),
+				containsString("want " + unadvertized.name() + " not valid"));
 	}
 
 	@Test
 	public void testV2FetchRequestPolicyReachableCommit() throws Exception {
 		RevCommit reachable = remote.commit().message("x").create();
-		RevCommit advertized = remote.commit().message("x").parent(reachable).create();
+		RevCommit advertized = remote.commit().message("x").parent(reachable)
+				.create();
 		RevCommit unreachable = remote.commit().message("y").create();
 		remote.update("branch1", advertized);
 
 		// This works
-		uploadPackV2(
+		uploadPackV2(RequestPolicy.REACHABLE_COMMIT, null, null,
-			RequestPolicy.REACHABLE_COMMIT,
+				"command=fetch\n", PacketLineIn.delimiter(),
-			null,
+				"want " + reachable.name() + "\n", PacketLineIn.end());
-			null,
-			"command=fetch\n",
-			PacketLineIn.delimiter(),
-			"want " + reachable.name() + "\n",
-				PacketLineIn.end());
 
 		// This doesn't
-		thrown.expect(TransportException.class);
+		TransportException e = assertThrows(TransportException.class,
-		thrown.expectMessage(Matchers.containsString(
+				() -> uploadPackV2(RequestPolicy.REACHABLE_COMMIT, null, null,
-					"want " + unreachable.name() + " not valid"));
+						"command=fetch\n", PacketLineIn.delimiter(),
-		uploadPackV2(
-			RequestPolicy.REACHABLE_COMMIT,
-			null,
-			null,
-			"command=fetch\n",
-			PacketLineIn.delimiter(),
 						"want " + unreachable.name() + "\n",
-				PacketLineIn.end());
+						PacketLineIn.end()));
+		assertThat(e.getMessage(),
+				containsString("want " + unreachable.name() + " not valid"));
 	}
 
 	@Test
 	public void testV2FetchRequestPolicyTip() throws Exception {
 		RevCommit parentOfTip = remote.commit().message("x").create();
-		RevCommit tip = remote.commit().message("y").parent(parentOfTip).create();
+		RevCommit tip = remote.commit().message("y").parent(parentOfTip)
+				.create();
 		remote.update("secret", tip);
 
 		// This works
-		uploadPackV2(
+		uploadPackV2(RequestPolicy.TIP, new RejectAllRefFilter(), null,
-			RequestPolicy.TIP,
+				"command=fetch\n", PacketLineIn.delimiter(),
-			new RejectAllRefFilter(),
+				"want " + tip.name() + "\n", PacketLineIn.end());
-			null,
-			"command=fetch\n",
-			PacketLineIn.delimiter(),
-			"want " + tip.name() + "\n",
-				PacketLineIn.end());
 
 		// This doesn't
-		thrown.expect(TransportException.class);
+		TransportException e = assertThrows(TransportException.class,
-		thrown.expectMessage(Matchers.containsString(
+				() -> uploadPackV2(RequestPolicy.TIP, new RejectAllRefFilter(),
-					"want " + parentOfTip.name() + " not valid"));
+						null, "command=fetch\n", PacketLineIn.delimiter(),
-		uploadPackV2(
-			RequestPolicy.TIP,
-			new RejectAllRefFilter(),
-			null,
-			"command=fetch\n",
-			PacketLineIn.delimiter(),
 						"want " + parentOfTip.name() + "\n",
-				PacketLineIn.end());
+						PacketLineIn.end()));
+		assertThat(e.getMessage(),
+				containsString("want " + parentOfTip.name() + " not valid"));
 	}
 
 	@Test
 	public void testV2FetchRequestPolicyReachableCommitTip() throws Exception {
 		RevCommit parentOfTip = remote.commit().message("x").create();
-		RevCommit tip = remote.commit().message("y").parent(parentOfTip).create();
+		RevCommit tip = remote.commit().message("y").parent(parentOfTip)
+				.create();
 		RevCommit unreachable = remote.commit().message("y").create();
 		remote.update("secret", tip);
 
 		// This works
-		uploadPackV2(
+		uploadPackV2(RequestPolicy.REACHABLE_COMMIT_TIP,
-			RequestPolicy.REACHABLE_COMMIT_TIP,
+				new RejectAllRefFilter(), null, "command=fetch\n",
-			new RejectAllRefFilter(),
+				PacketLineIn.delimiter(), "want " + parentOfTip.name() + "\n",
-			null,
-			"command=fetch\n",
-			PacketLineIn.delimiter(),
-			"want " + parentOfTip.name() + "\n",
 				PacketLineIn.end());
 
 		// This doesn't
-		thrown.expect(TransportException.class);
+		TransportException e = assertThrows(TransportException.class,
-		thrown.expectMessage(Matchers.containsString(
+				() -> uploadPackV2(RequestPolicy.REACHABLE_COMMIT_TIP,
-					"want " + unreachable.name() + " not valid"));
+						new RejectAllRefFilter(), null, "command=fetch\n",
-		uploadPackV2(
-			RequestPolicy.REACHABLE_COMMIT_TIP,
-			new RejectAllRefFilter(),
-			null,
-			"command=fetch\n",
 						PacketLineIn.delimiter(),
 						"want " + unreachable.name() + "\n",
-				PacketLineIn.end());
+						PacketLineIn.end()));
+		assertThat(e.getMessage(),
+				containsString("want " + unreachable.name() + " not valid"));
 	}
 
 	@Test
@@ -1012,29 +975,29 @@ public class UploadPackTest {
 		String commonInBlob = "abcdefghijklmnopqrstuvwxyz";
 
 		RevBlob parentBlob = remote.blob(commonInBlob + "a");
-		RevCommit parent = remote.commit(remote.tree(remote.file("foo", parentBlob)));
+		RevCommit parent = remote
+				.commit(remote.tree(remote.file("foo", parentBlob)));
 		RevBlob childBlob = remote.blob(commonInBlob + "b");
-		RevCommit child = remote.commit(remote.tree(remote.file("foo", childBlob)), parent);
+		RevCommit child = remote
+				.commit(remote.tree(remote.file("foo", childBlob)), parent);
 		remote.update("branch1", child);
 
 		// Pretend that we have parent to get a thin pack based on it.
-		ByteArrayInputStream recvStream = uploadPackV2(
+		ByteArrayInputStream recvStream = uploadPackV2("command=fetch\n",
-			"command=fetch\n",
 				PacketLineIn.delimiter(),
 				"want " + child.toObjectId().getName() + "\n",
-			"have " + parent.toObjectId().getName() + "\n",
+				"have " + parent.toObjectId().getName() + "\n", "thin-pack\n",
-			"thin-pack\n",
+				"done\n", PacketLineIn.end());
-			"done\n",
-				PacketLineIn.end());
 		PacketLineIn pckIn = new PacketLineIn(recvStream);
 
 		assertThat(pckIn.readString(), is("packfile"));
 
 		// Verify that we received a thin pack by trying to apply it
 		// against the client repo, which does not have parent.
-		thrown.expect(IOException.class);
+		IOException e = assertThrows(IOException.class,
-		thrown.expectMessage("pack has unresolved deltas");
+				() -> parsePack(recvStream));
-		parsePack(recvStream);
+		assertThat(e.getMessage(),
+				containsString("pack has unresolved deltas"));
 	}
 
 	@Test
@@ -1339,15 +1302,13 @@ public class UploadPackTest {
 
 		remote.update("branch1", tooOld);
 
-		thrown.expect(PackProtocolException.class);
+		PackProtocolException e = assertThrows(PackProtocolException.class,
-		thrown.expectMessage("No commits selected for shallow request");
+				() -> uploadPackV2("command=fetch\n", PacketLineIn.delimiter(),
-		uploadPackV2(
-			"command=fetch\n",
-			PacketLineIn.delimiter(),
 						"deepen-since 1510000\n",
 						"want " + tooOld.toObjectId().getName() + "\n",
-			"done\n",
+						"done\n", PacketLineIn.end()));
-				PacketLineIn.end());
+		assertThat(e.getMessage(),
+				containsString("No commits selected for shallow request"));
 	}
 
 	@Test
@@ -1406,7 +1367,8 @@ public class UploadPackTest {
 	}
 
 	@Test
-	public void testV2FetchDeepenNot_excludeDescendantOfWant() throws Exception {
+	public void testV2FetchDeepenNot_excludeDescendantOfWant()
+			throws Exception {
 		RevCommit one = remote.commit().message("one").create();
 		RevCommit two = remote.commit().message("two").parent(one).create();
 		RevCommit three = remote.commit().message("three").parent(two).create();
@@ -1415,15 +1377,13 @@ public class UploadPackTest {
 		remote.update("two", two);
 		remote.update("four", four);
 
-		thrown.expect(PackProtocolException.class);
+		PackProtocolException e = assertThrows(PackProtocolException.class,
-		thrown.expectMessage("No commits selected for shallow request");
+				() -> uploadPackV2("command=fetch\n", PacketLineIn.delimiter(),
-		uploadPackV2(
-			"command=fetch\n",
-			PacketLineIn.delimiter(),
 						"deepen-not four\n",
-			"want " + two.toObjectId().getName() + "\n",
+						"want " + two.toObjectId().getName() + "\n", "done\n",
-			"done\n",
+						PacketLineIn.end()));
-				PacketLineIn.end());
+		assertThat(e.getMessage(),
+				containsString("No commits selected for shallow request"));
 	}
 
 	@Test
@@ -1501,13 +1461,11 @@ public class UploadPackTest {
 
 	@Test
 	public void testV2FetchUnrecognizedArgument() throws Exception {
-		thrown.expect(PackProtocolException.class);
+		PackProtocolException e = assertThrows(PackProtocolException.class,
-		thrown.expectMessage("unexpected invalid-argument");
+				() -> uploadPackV2("command=fetch\n", PacketLineIn.delimiter(),
-		uploadPackV2(
+						"invalid-argument\n", PacketLineIn.end()));
-			"command=fetch\n",
+		assertThat(e.getMessage(),
-			PacketLineIn.delimiter(),
+				containsString("unexpected invalid-argument"));
-			"invalid-argument\n",
-				PacketLineIn.end());
 	}
 
 	@Test
@@ -1876,15 +1834,12 @@ public class UploadPackTest {
 
 		server.getConfig().setBoolean("uploadpack", null, "allowfilter", false);
 
-		thrown.expect(PackProtocolException.class);
+		PackProtocolException e = assertThrows(PackProtocolException.class,
-		thrown.expectMessage("unexpected filter blob:limit=5");
+				() -> uploadPackV2("command=fetch\n", PacketLineIn.delimiter(),
-		uploadPackV2(
-			"command=fetch\n",
-			PacketLineIn.delimiter(),
 						"want " + commit.toObjectId().getName() + "\n",
-			"filter blob:limit=5\n",
+						"filter blob:limit=5\n", "done\n", PacketLineIn.end()));
-			"done\n",
+		assertThat(e.getMessage(),
-				PacketLineIn.end());
+				containsString("unexpected filter blob:limit=5"));
 	}
 
 	@Test