SOLR-12555: Use `expectThrows` for expected exceptions

This commit replaces the `try { doX(); fail(); }` pattern with the
`expectThrows` test helper, which was created for this purpose.  This
commit makes these changes in the core package: `o.a.solr.cloud`.

Closes #425

solr/core/src/test/org/apache/solr/BasicFunctionalityTest.java

@@ -356,42 +356,39 @@ public class BasicFunctionalityTest extends SolrTestCaseJ4 {
 
     // test that malformed numerics cause client error not server error
     for (String field : FIELDS) {
-      try {
+      SolrException e1 = expectThrows(SolrException.class,
-        h.update(add( doc("id","100", field, BAD_VALUE)));
+          "Didn't encounter an error trying to add a bad date: " + field,
-        fail("Didn't encounter an error trying to add a bad date: " + field);
+          () -> h.update(add( doc("id","100", field, BAD_VALUE))));
-      } catch (SolrException e) {
+      String msg1 = e1.getMessage();
-        String msg = e.toString();
+      assertTrue("not an (update) client error on field: " + field +" : "+ msg1,
-        assertTrue("not an (update) client error on field: " + field +" : "+ msg,
+          400 <= e1.code() && e1.code() < 500);
-                   400 <= e.code() && e.code() < 500);
+      assertTrue("(update) client error does not mention bad value: " + msg1,
-        assertTrue("(update) client error does not mention bad value: " + msg,
+          msg1.contains(BAD_VALUE));
-                   msg.contains(BAD_VALUE));
+      assertTrue("client error does not mention document id: " + msg1,
-        assertTrue("client error does not mention document id: " + msg,
+          msg1.contains("[doc=100]"));
-                   msg.contains("[doc=100]"));
+      SchemaField sf = h.getCore().getLatestSchema().getField(field);
-      }
-      SchemaField sf = h.getCore().getLatestSchema().getField(field); 
       if (!sf.hasDocValues() && !sf.indexed()) {
         continue;
       }
-      try {
+      SolrException e2 = expectThrows(SolrException.class,
-        h.query(req("q",field + ":" + BAD_VALUE));
+          "Didn't encounter an error trying to add a bad date: " + field,
-        fail("Didn't encounter an error trying to query a bad date: " + field);
+          () -> h.query(req("q",field + ":" + BAD_VALUE))
-      } catch (SolrException e) {
+      );
-        String msg = e.toString();
+      String msg2 = e2.toString();
-        assertTrue("not a (search) client error on field: " + field +" : "+ msg,
+      assertTrue("not a (search) client error on field: " + field +" : "+ msg2,
-                   400 <= e.code() && e.code() < 500);
+          400 <= e2.code() && e2.code() < 500);
-        assertTrue("(search) client error does not mention bad value: " + msg,
+      assertTrue("(search) client error does not mention bad value: " + msg2,
-                   msg.contains(BAD_VALUE));
+          msg2.contains(BAD_VALUE));
-      }
+
-      try {
+      SolrException e3 = expectThrows(SolrException.class,
-        h.query(req("q",field + ":[NOW TO " + BAD_VALUE + "]"));
+          "Didn't encounter an error trying to add a bad date: " + field,
-        fail("Didn't encounter an error trying to query a bad date: " + field);
+          () -> h.query(req("q",field + ":[NOW TO " + BAD_VALUE + "]"))
-      } catch (SolrException e) {
+      );
-        String msg = e.toString();
+      String msg3 = e3.toString();
-        assertTrue("not a (search) client error on field: " + field +" : "+ msg,
+      assertTrue("not a (search) client error on field: " + field +" : "+ msg3,
-                   400 <= e.code() && e.code() < 500);
+          400 <= e3.code() && e3.code() < 500);
-        assertTrue("(search) client error does not mention bad value: " + msg,
+      assertTrue("(search) client error does not mention bad value: " + msg3,
-                   msg.contains(BAD_VALUE));
+          msg3.contains(BAD_VALUE));
-      }
     }
   }
 
@@ -414,42 +411,40 @@ public class BasicFunctionalityTest extends SolrTestCaseJ4 {
 
     // test that malformed numerics cause client error not server error
     for (String field : FIELDS) {
-      try {
+      SolrException e1 = expectThrows(SolrException.class,
-        h.update(add( doc("id","100", field, BAD_VALUE)));
+          "Didn't encounter an error trying to add a non-number: " + field,
-        fail("Didn't encounter an error trying to add a non-number: " + field);
+          () -> h.update(add( doc("id","100", field, BAD_VALUE))));
-      } catch (SolrException e) {
+      String msg1 = e1.toString();
-        String msg = e.toString();
+      assertTrue("not an (update) client error on field: " + field +" : "+ msg1,
-        assertTrue("not an (update) client error on field: " + field +" : "+ msg,
+          400 <= e1.code() && e1.code() < 500);
-                   400 <= e.code() && e.code() < 500);
+      assertTrue("(update) client error does not mention bad value: " + msg1,
-        assertTrue("(update) client error does not mention bad value: " + msg,
+          msg1.contains(BAD_VALUE));
-                   msg.contains(BAD_VALUE));
+      assertTrue("client error does not mention document id",
-        assertTrue("client error does not mention document id",
+          msg1.contains("[doc=100]"));
-                   msg.contains("[doc=100]"));
-      }
       SchemaField sf = h.getCore().getLatestSchema().getField(field); 
       if (!sf.hasDocValues() && !sf.indexed()) {
         continue;
       }
-      try {
+
-        h.query(req("q",field + ":" + BAD_VALUE));
+      SolrException e2 = expectThrows(SolrException.class,
-        fail("Didn't encounter an error trying to query a non-number: " + field);
+          "Didn't encounter an error trying to add a non-number: " + field,
-      } catch (SolrException e) {
+          () -> h.query(req("q",field + ":" + BAD_VALUE))
-        String msg = e.toString();
+      );
-        assertTrue("not a (search) client error on field: " + field +" : "+ msg,
+      String msg2 = e2.toString();
-                   400 <= e.code() && e.code() < 500);
+      assertTrue("not a (search) client error on field: " + field +" : "+ msg2,
-        assertTrue("(search) client error does not mention bad value: " + msg,
+          400 <= e2.code() && e2.code() < 500);
-                   msg.contains(BAD_VALUE));
+      assertTrue("(search) client error does not mention bad value: " + msg2,
-      }
+          msg2.contains(BAD_VALUE));
-      try {
+
-        h.query(req("q",field + ":[10 TO " + BAD_VALUE + "]"));
+      SolrException e3 = expectThrows(SolrException.class,
-        fail("Didn't encounter an error trying to query a non-number: " + field);
+          "Didn't encounter an error trying to add a non-number: " + field,
-      } catch (SolrException e) {
+          () -> h.query(req("q",field + ":[10 TO " + BAD_VALUE + "]"))
-        String msg = e.toString();
+      );
-        assertTrue("not a (search) client error on field: " + field +" : "+ msg,
+      String msg3 = e3.toString();
-                   400 <= e.code() && e.code() < 500);
+      assertTrue("not a (search) client error on field: " + field +" : "+ msg3,
-        assertTrue("(search) client error does not mention bad value: " + msg,
+          400 <= e3.code() && e3.code() < 500);
-                   msg.contains(BAD_VALUE));
+      assertTrue("(search) client error does not mention bad value: " + msg3,
-      }
+          msg3.contains(BAD_VALUE));
     }
   }
   
@@ -1001,26 +996,23 @@ public class BasicFunctionalityTest extends SolrTestCaseJ4 {
                  "sortabuse_t", "zzz xxx ccc vvv bbb nnn qqq www eee rrr ttt"));
 
     assertU(commit());
-  
+
-    try {
+    RuntimeException outerEx = expectThrows(RuntimeException.class, () -> {
       ignoreException("can not sort on multivalued field: sortabuse_t");
       assertQ("sort on something that shouldn't work",
-              req("q", "sortabuse_b:true",
+          req("q", "sortabuse_b:true",
-                  "sort", "sortabuse_t asc"),
+              "sort", "sortabuse_t asc"),
-              "*[count(//doc)=2]");
+          "*[count(//doc)=2]");
-      fail("no error encountered when sorting on sortabuse_t");
+    });
-    } catch (Exception outer) {
+    Throwable root = getRootCause(outerEx);
-      // EXPECTED
+    assertEquals("sort exception root cause",
-      Throwable root = getRootCause(outer);
+        SolrException.class, root.getClass());
-      assertEquals("sort exception root cause", 
+    SolrException e = (SolrException) root;
-                   SolrException.class, root.getClass());
+    assertEquals("incorrect error type",
-      SolrException e = (SolrException) root;
+        SolrException.ErrorCode.BAD_REQUEST,
-      assertEquals("incorrect error type", 
+        SolrException.ErrorCode.getErrorCode(e.code()));
-                   SolrException.ErrorCode.BAD_REQUEST,
+    assertTrue("exception doesn't contain field name",
-                   SolrException.ErrorCode.getErrorCode(e.code()));
+        e.getMessage().contains("sortabuse_t"));
-      assertTrue("exception doesn't contain field name",
-                 -1 != e.getMessage().indexOf("sortabuse_t"));
-    }
   }
 
 //   /** this doesn't work, but if it did, this is how we'd test it. */

solr/core/src/test/org/apache/solr/CursorPagingTest.java

@@ -842,13 +842,13 @@ public class CursorPagingTest extends SolrTestCaseJ4 {
     throws Exception {
 
     try {
-      ignoreException(expSubstr);
+      SolrException e = expectThrows(SolrException.class, () -> {
-      assertJQ(req(p));
+        ignoreException(expSubstr);
-      fail("no exception matching expected: " + expCode.code + ": " + expSubstr);
+        assertJQ(req(p));
-    } catch (SolrException e) {
+      });
       assertEquals(expCode.code, e.code());
       assertTrue("Expected substr not found: " + expSubstr + " <!< " + e.getMessage(),
-                 e.getMessage().contains(expSubstr));
+          e.getMessage().contains(expSubstr));
     } finally {
       unIgnoreException(expSubstr);
     }

solr/core/src/test/org/apache/solr/TestCursorMarkWithoutUniqueKey.java

@@ -53,11 +53,12 @@ public class TestCursorMarkWithoutUniqueKey extends SolrTestCaseJ4 {
 
     try {
       ignoreException("Cursor functionality is not available unless the IndexSchema defines a uniqueKey field");
-      assertQ(req("q", "*:*", "sort", "fld desc", "cursorMark", CURSOR_MARK_START));
+      expectThrows(RuntimeException.class,
-      fail("No exception when querying with a cursorMark with no uniqueKey defined.");
+          "No exception when querying with a cursorMark with no uniqueKey defined.",
-    } catch (Exception e) {
+          () -> assertQ(req("q", "*:*", "sort", "fld desc", "cursorMark", CURSOR_MARK_START))
+      );
+    } finally {
       unIgnoreException("Cursor functionality is not available unless the IndexSchema defines a uniqueKey field");
     }
-
   }
 }

solr/core/src/test/org/apache/solr/TestDistributedSearch.java

@@ -911,13 +911,11 @@ public class TestDistributedSearch extends BaseDistributedSearchTestCase {
 
     //SOLR 3161 ensure shards.qt=/update fails (anything but search handler really)
     // Also see TestRemoteStreaming#testQtUpdateFails()
-    try {
+
-      ignoreException("isShard is only acceptable");
+    //SolrException e = expectThrows(SolrException.class, () -> {
-      // query("q","*:*","shards.qt","/update","stream.body","<delete><query>*:*</query></delete>");
+    //  ignoreException("isShard is only acceptable");
-      // fail();
+    //  query("q","*:*","shards.qt","/update","stream.body","<delete><query>*:*</query></delete>");
-    } catch (SolrException e) {
+    //});
-      //expected
-    }
     unIgnoreException("isShard is only acceptable");
 
     // test debugging
@@ -1219,41 +1217,34 @@ public class TestDistributedSearch extends BaseDistributedSearchTestCase {
   private void validateCommonQueryParameters() throws Exception {
     ignoreException("parameter cannot be negative");
 
-    try {
+    SolrException e1 = expectThrows(SolrException.class, () -> {
       SolrQuery query = new SolrQuery();
       query.setParam("start", "non_numeric_value").setQuery("*");
       QueryResponse resp = query(query);
-      fail("Expected the last query to fail, but got response: " + resp);
+    });
-    } catch (SolrException e) {
+    assertEquals(ErrorCode.BAD_REQUEST.code, e1.code());
-      assertEquals(ErrorCode.BAD_REQUEST.code, e.code());
-    }
 
-    try {
+    SolrException e2 = expectThrows(SolrException.class, () -> {
       SolrQuery query = new SolrQuery();
       query.setStart(-1).setQuery("*");
       QueryResponse resp = query(query);
-      fail("Expected the last query to fail, but got response: " + resp);
+    });
-    } catch (SolrException e) {
+    assertEquals(ErrorCode.BAD_REQUEST.code, e2.code());
-      assertEquals(ErrorCode.BAD_REQUEST.code, e.code());
-    }
 
-    try {
+    SolrException e3 = expectThrows(SolrException.class, () -> {
       SolrQuery query = new SolrQuery();
       query.setRows(-1).setStart(0).setQuery("*");
       QueryResponse resp = query(query);
-      fail("Expected the last query to fail, but got response: " + resp);
+    });
-    } catch (SolrException e) {
+    assertEquals(ErrorCode.BAD_REQUEST.code, e3.code());
-      assertEquals(ErrorCode.BAD_REQUEST.code, e.code());
-    }
 
-    try {
+    SolrException e4 = expectThrows(SolrException.class, () -> {
       SolrQuery query = new SolrQuery();
       query.setParam("rows", "non_numeric_value").setQuery("*");
       QueryResponse resp = query(query);
-      fail("Expected the last query to fail, but got response: " + resp);
+    });
-    } catch (SolrException e) {
+    assertEquals(ErrorCode.BAD_REQUEST.code, e4.code());
-      assertEquals(ErrorCode.BAD_REQUEST.code, e.code());
+
-    }
     resetExceptionIgnores();
   }
 }

solr/core/src/test/org/apache/solr/TestTolerantSearch.java

@@ -131,12 +131,9 @@ public class TestTolerantSearch extends SolrJettyTestBase {
     query.setFacet(true);
     
     ignoreException("Dummy exception in BadResponseWriter");
-    try {
+
-      collection1.query(query);
+    expectThrows(SolrException.class, () -> collection1.query(query));
-      fail("Should get an exception");
+
-    } catch (Exception e) {
-      //expected
-    }
     query.set(ShardParams.SHARDS_TOLERANT, "true");
     QueryResponse response = collection1.query(query);
     assertTrue(response.getResponseHeader().getBooleanArg(SolrQueryResponse.RESPONSE_HEADER_PARTIAL_RESULTS_KEY));
@@ -179,12 +176,9 @@ public class TestTolerantSearch extends SolrJettyTestBase {
     query.setFacet(true);
     
     ignoreException("Dummy exception in BadResponseWriter");
-    try {
+
-      collection1.query(query);
+    expectThrows(Exception.class, () -> collection1.query(query));
-      fail("Should get an exception");
+
-    } catch (Exception e) {
-      //expected
-    }
     query.set(ShardParams.SHARDS_TOLERANT, "true");
     QueryResponse response = collection1.query(query);
     assertTrue(response.getResponseHeader().getBooleanArg(SolrQueryResponse.RESPONSE_HEADER_PARTIAL_RESULTS_KEY));

solr/core/src/test/org/apache/solr/cloud/BasicDistributedZk2Test.java

@@ -116,12 +116,7 @@ public class BasicDistributedZk2Test extends AbstractFullDistribZkTestBase {
       long docId = testUpdateAndDelete();
       
       // index a bad doc...
-      try {
+      expectThrows(SolrException.class, () -> indexr(t1, "a doc with no id"));
-        indexr(t1, "a doc with no id");
-        fail("this should fail");
-      } catch (SolrException e) {
-        // expected
-      }
       
       // TODO: bring this to its own method?
       // try indexing to a leader that has no replicas up
@@ -255,29 +250,26 @@ public class BasicDistributedZk2Test extends AbstractFullDistribZkTestBase {
   private void brindDownShardIndexSomeDocsAndRecover() throws Exception {
     SolrQuery query = new SolrQuery("*:*");
     query.set("distrib", false);
-    
+
     commit();
-    
+
     long deadShardCount = shardToJetty.get(SHARD2).get(0).client.solrClient
         .query(query).getResults().getNumFound();
 
     query("q", "*:*", "sort", "n_tl1 desc");
-    
+
     int oldLiveNodes = cloudClient.getZkStateReader().getZkClient().getChildren(ZkStateReader.LIVE_NODES_ZKNODE, null, true).size();
-    
+
     assertEquals(5, oldLiveNodes);
-    
+
     // kill a shard
     CloudJettyRunner deadShard = chaosMonkey.stopShard(SHARD1, 0);
-    
+
     // ensure shard is dead
-    try {
+    expectThrows(SolrServerException.class,
-      index_specific(deadShard.client.solrClient, id, 999, i1, 107, t1,
+        "This server should be down and this update should have failed",
-          "specific doc!");
+        () -> index_specific(deadShard.client.solrClient, id, 999, i1, 107, t1, "specific doc!")
-      fail("This server should be down and this update should have failed");
+    );
-    } catch (SolrServerException e) {
-      // expected..
-    }
     
     commit();
     

solr/core/src/test/org/apache/solr/cloud/BasicDistributedZkTest.java

@@ -713,24 +713,20 @@ public class BasicDistributedZkTest extends AbstractFullDistribZkTestBase {
     log.info("### STARTING doOptimisticLockingAndUpdating");
     printLayout();
     
-    SolrInputDocument sd =  sdoc("id", 1000, "_version_", -1);
+    final SolrInputDocument sd =  sdoc("id", 1000, "_version_", -1);
     indexDoc(sd);
 
     ignoreException("version conflict");
     for (SolrClient client : clients) {
-      try {
+      SolrException e = expectThrows(SolrException.class, () -> client.add(sd));
-        client.add(sd);
+      assertEquals(409, e.code());
-        fail();
-      } catch (SolrException e) {
-        assertEquals(409, e.code());
-      }
     }
     unIgnoreException("version conflict");
 
     // TODO: test deletes.  SolrJ needs a good way to pass version for delete...
 
-    sd =  sdoc("id", 1000, "foo_i",5);
+    final SolrInputDocument sd2 =  sdoc("id", 1000, "foo_i",5);
-    clients.get(0).add(sd);
+    clients.get(0).add(sd2);
 
     List<Integer> expected = new ArrayList<>();
     int val = 0;

solr/core/src/test/org/apache/solr/cloud/BasicZkTest.java

@@ -150,15 +150,15 @@ public class BasicZkTest extends AbstractZkTestCase {
     zkController.getZkClient().setData("/configs/conf1/solrconfig.xml", new byte[0], true);
  
     // we set the solrconfig to nothing, so this reload should fail
-    try {
+    SolrException e = expectThrows(SolrException.class,
+        "The reloaded SolrCore did not pick up configs from zookeeper",
+        () -> {
       ignoreException("solrconfig.xml");
       h.getCoreContainer().reload(h.getCore().getName());
-      fail("The reloaded SolrCore did not pick up configs from zookeeper");
+    });
-    } catch(SolrException e) {
+    resetExceptionIgnores();
-      resetExceptionIgnores();
+    assertTrue(e.getMessage().contains("Unable to reload core [collection1]"));
-      assertTrue(e.getMessage().contains("Unable to reload core [collection1]"));
+    assertTrue(e.getCause().getMessage().contains("Error loading solr config from solrconfig.xml"));
-      assertTrue(e.getCause().getMessage().contains("Error loading solr config from solrconfig.xml"));
-    }
     
     // test stats call
     Map<String, Metric> metrics = h.getCore().getCoreMetricManager().getRegistry().getMetrics();

solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java

@@ -514,31 +514,25 @@ public class CollectionsAPISolrJTest extends SolrCloudTestCase {
     waitForState("Expecting attribute 'maxShardsPerNode' to be deleted", collection,
         (n, c) -> null == c.get("maxShardsPerNode"));
 
-    try {
+    expectThrows(IllegalArgumentException.class,
-      CollectionAdminRequest.modifyCollection(collection, null)
+        "An attempt to set unknown collection attribute should have failed",
-          .setAttribute("non_existent_attr", 25)
+        () -> CollectionAdminRequest.modifyCollection(collection, null)
-          .process(cluster.getSolrClient());
+            .setAttribute("non_existent_attr", 25)
-      fail("An attempt to set unknown collection attribute should have failed");
+            .process(cluster.getSolrClient())
-    } catch (IllegalArgumentException e) {
+    );
-      // expected
-    }
 
-    try {
+    expectThrows(IllegalArgumentException.class,
-      CollectionAdminRequest.modifyCollection(collection, null)
+        "An attempt to set null value should have failed",
-          .setAttribute("non_existent_attr", null)
+        () -> CollectionAdminRequest.modifyCollection(collection, null)
-          .process(cluster.getSolrClient());
+            .setAttribute("non_existent_attr", null)
-      fail("An attempt to set null value should have failed");
+            .process(cluster.getSolrClient())
-    } catch (IllegalArgumentException e) {
+    );
-      // expected
-    }
 
-    try {
+    expectThrows(IllegalArgumentException.class,
-      CollectionAdminRequest.modifyCollection(collection, null)
+        "An attempt to unset unknown collection attribute should have failed",
-          .unsetAttribute("non_existent_attr")
+        () -> CollectionAdminRequest.modifyCollection(collection, null)
-          .process(cluster.getSolrClient());
+            .unsetAttribute("non_existent_attr")
-      fail("An attempt to unset unknown collection attribute should have failed");
+            .process(cluster.getSolrClient())
-    } catch (IllegalArgumentException e) {
+    );
-      // expected
-    }
   }
 }

solr/core/src/test/org/apache/solr/cloud/DeleteReplicaTest.java

@@ -144,20 +144,16 @@ public class DeleteReplicaTest extends SolrCloudTestCase {
 
     CollectionAdminRequest.deleteReplicasFromShard(collectionName, "shard1", 2).process(cluster.getSolrClient());
     waitForState("Expected a single shard with a single replica", collectionName, clusterShape(1, 1));
-    
+
-    try {
+    SolrException e = expectThrows(SolrException.class,
-      CollectionAdminRequest.deleteReplicasFromShard(collectionName, "shard1", 1).process(cluster.getSolrClient());
+        "Can't delete the last replica by count",
-      fail("Expected Exception, Can't delete the last replica by count");
+        () -> CollectionAdminRequest.deleteReplicasFromShard(collectionName, "shard1", 1).process(cluster.getSolrClient())
-    } catch (SolrException e) {
+    );
-      // expected
+    assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code());
-      assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code());
+    assertTrue(e.getMessage().contains("There is only one replica available"));
-      assertTrue(e.getMessage().contains("There is only one replica available"));
-    }
     DocCollection docCollection = getCollectionState(collectionName);
     // We know that since leaders are preserved, PULL replicas should not be left alone in the shard
     assertEquals(0, docCollection.getSlice("shard1").getReplicas(EnumSet.of(Replica.Type.PULL)).size());
-    
-
   }
 
   @Test
@@ -257,6 +253,7 @@ public class DeleteReplicaTest extends SolrCloudTestCase {
           return false;
         }
         LOG.info("Running delete core {}",cd);
+
         try {
           ZkNodeProps m = new ZkNodeProps(
               Overseer.QUEUE_OPERATION, OverseerAction.DELETECORE.toLower(),

solr/core/src/test/org/apache/solr/cloud/ForceLeaderTest.java

@@ -317,13 +317,9 @@ public class ForceLeaderTest extends HttpPartitionTest {
 
   private void assertSendDocFails(int docId) throws Exception {
     // sending a doc in this state fails
-    try {
+    expectThrows(SolrException.class,
-      sendDoc(docId);
+        "Should've failed indexing during a down state.",
-      log.error("Should've failed indexing during a down state. Cluster state: " + printClusterStateInfo());
+        () -> sendDoc(docId));
-      fail("Should've failed indexing during a down state.");
-    } catch (SolrException ex) {
-      log.info("Document couldn't be sent, which is expected.");
-    }
   }
 
   private void putNonLeadersIntoLIR(String collectionName, String shard, ZkController zkController, Replica leader, List<Replica> notLeaders) throws Exception {

solr/core/src/test/org/apache/solr/cloud/OverseerStatusTest.java

@@ -16,6 +16,7 @@
  */
 package org.apache.solr.cloud;
 
+import org.apache.solr.client.solrj.impl.HttpSolrClient;
 import org.apache.solr.client.solrj.request.CollectionAdminRequest;
 import org.apache.solr.common.params.CollectionParams;
 import org.apache.solr.common.util.NamedList;
@@ -58,14 +59,14 @@ public class OverseerStatusTest extends SolrCloudTestCase {
     SimpleOrderedMap<Object> reload = (SimpleOrderedMap<Object>) collection_operations.get(CollectionParams.CollectionAction.RELOAD.toLower());
     assertEquals("No stats for reload in OverseerCollectionProcessor", 1, reload.get("requests"));
 
-    try {
+    HttpSolrClient.RemoteSolrException e = expectThrows(HttpSolrClient.RemoteSolrException.class,
-      CollectionAdminRequest.splitShard("non_existent_collection")
+        "Split shard for non existent collection should have failed",
-          .setShardName("non_existent_shard")
+        () -> CollectionAdminRequest
-          .process(cluster.getSolrClient());
+            .splitShard("non_existent_collection")
-      fail("Split shard for non existent collection should have failed");
+            .setShardName("non_existent_shard")
-    } catch (Exception e) {
+            .process(cluster.getSolrClient())
-      // expected because we did not correctly specify required params for split
+    );
-    }
+
     resp = new CollectionAdminRequest.OverseerStatus().process(cluster.getSolrClient()).getResponse();
     collection_operations = (NamedList<Object>) resp.get("collection_operations");
     SimpleOrderedMap<Object> split = (SimpleOrderedMap<Object>) collection_operations.get(CollectionParams.CollectionAction.SPLITSHARD.toLower());

solr/core/src/test/org/apache/solr/cloud/SolrXmlInZkTest.java

@@ -140,12 +140,12 @@ public class SolrXmlInZkTest extends SolrTestCaseJ4 {
   @Test
   public void testNotInZkOrOnDisk() throws Exception {
     try {
-      System.setProperty("hostPort", "8787");
+      SolrException e = expectThrows(SolrException.class, () -> {
-      setUpZkAndDiskXml(false, false); // solr.xml not on disk either
+        System.setProperty("hostPort", "8787");
-      fail("Should have thrown an exception here");
+        setUpZkAndDiskXml(false, false); // solr.xml not on disk either
-    } catch (SolrException solre) {
+      });
       assertTrue("Should be failing to create default solr.xml in code",
-          solre.getMessage().contains("solr.xml does not exist"));
+          e.getMessage().contains("solr.xml does not exist"));
     } finally {
       closeZK();
     }

solr/core/src/test/org/apache/solr/cloud/TestAuthenticationFramework.java

@@ -29,6 +29,7 @@ import org.apache.lucene.util.LuceneTestCase;
 import org.apache.solr.client.solrj.SolrQuery;
 import org.apache.solr.client.solrj.impl.CloudSolrClient;
 import org.apache.solr.client.solrj.impl.HttpClientUtil;
+import org.apache.solr.client.solrj.impl.HttpSolrClient;
 import org.apache.solr.client.solrj.impl.SolrHttpClientBuilder;
 import org.apache.solr.client.solrj.request.CollectionAdminRequest;
 import org.apache.solr.client.solrj.request.UpdateRequest;
@@ -79,12 +80,9 @@ public class TestAuthenticationFramework extends SolrCloudTestCase {
 
     // Should fail with 401
     try {
-      collectionCreateSearchDeleteTwice();
+      HttpSolrClient.RemoteSolrException e = expectThrows(HttpSolrClient.RemoteSolrException.class,
-      fail("Should've returned a 401 error");
+          this::collectionCreateSearchDeleteTwice);
-    } catch (Exception ex) {
+      assertTrue("Should've returned a 401 error", e.getMessage().contains("Error 401"));
-      if (!ex.getMessage().contains("Error 401")) {
-        fail("Should've returned a 401 error");
-      }
     } finally {
       MockAuthenticationPlugin.expectedUsername = null;
       MockAuthenticationPlugin.expectedPassword = null;

solr/core/src/test/org/apache/solr/cloud/TestCloudDeleteByQuery.java

@@ -30,7 +30,6 @@ import org.apache.solr.client.solrj.impl.CloudSolrClient;
 import org.apache.solr.client.solrj.impl.HttpSolrClient;
 import org.apache.solr.client.solrj.request.CollectionAdminRequest;
 import org.apache.solr.client.solrj.request.UpdateRequest;
-import org.apache.solr.client.solrj.response.UpdateResponse;
 import org.apache.solr.common.SolrDocument;
 import org.apache.solr.common.SolrDocumentList;
 import org.apache.solr.common.SolrException;
@@ -199,12 +198,10 @@ public class TestCloudDeleteByQuery extends SolrCloudTestCase {
 
   public void testMalformedDBQ(SolrClient client) throws Exception {
     assertNotNull("client not initialized", client);
-    try {
+    SolrException e = expectThrows(SolrException.class,
-      UpdateResponse rsp = update(params()).deleteByQuery("foo_i:not_a_num").process(client);
+        "Expected DBQ failure",
-      fail("Expected DBQ failure: " + rsp.toString());
+        () -> update(params()).deleteByQuery("foo_i:not_a_num").process(client));
-    } catch (SolrException e) {
+    assertEquals("not the expected DBQ failure: " + e.getMessage(), 400, e.code());
-      assertEquals("not the expected DBQ failure: " + e.getMessage(), 400, e.code());
-    }
   }
 
   //

solr/core/src/test/org/apache/solr/cloud/TestCloudInspectUtil.java

@@ -100,20 +100,15 @@ public class TestCloudInspectUtil extends SolrTestCaseJ4 {
     
     // ################################
     
-    addFails = new HashSet<String>();
+    final HashSet<String> addFailsExpectEx = new HashSet<String>();
-    deleteFails = new HashSet<String>();
+    final HashSet<String> deleteFailsExpectEx = new HashSet<String>();
     
-    a = getDocList("2", "3", "4");
+    final SolrDocumentList aExpectEx = getDocList("2", "3", "4");
-    b = getDocList("2", "3", "4");
+    final SolrDocumentList bExpectEx = getDocList("2", "3", "4");
-
-    try {
-      legal = CloudInspectUtil.checkIfDiffIsLegal(a, b, "control", "cloud",
-          addFails, deleteFails);
-      fail("Expected exception because lists have no diff");
-    } catch (IllegalArgumentException e) {
-      // expected
-    }
 
+    expectThrows(IllegalArgumentException.class, "Expected exception because lists have no diff",
+        () -> CloudInspectUtil.checkIfDiffIsLegal(aExpectEx, bExpectEx,
+            "control", "cloud", addFailsExpectEx, deleteFailsExpectEx));
   }
 
   private SolrDocumentList getDocList(String ... ids) {

solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java

@@ -92,7 +92,6 @@ import org.apache.solr.util.ExternalPaths;
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
 import org.junit.After;
-import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.noggit.JSONParser;
@@ -632,13 +631,9 @@ public class TestConfigSetsAPI extends SolrTestCaseJ4 {
 
   private void verifyException(SolrClient solrClient, ConfigSetAdminRequest request,
       String errorContains) throws Exception {
-    try {
+    Exception e = expectThrows(Exception.class, () -> solrClient.request(request));
-      solrClient.request(request);
+    assertTrue("Expected exception message to contain: " + errorContains
-      Assert.fail("Expected exception");
+        + " got: " + e.getMessage(), e.getMessage().contains(errorContains));
-    } catch (Exception e) {
-      assertTrue("Expected exception message to contain: " + errorContains
-          + " got: " + e.getMessage(), e.getMessage().contains(errorContains));
-    }
   }
 
   @Test

solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPIZkFailure.java

@@ -38,7 +38,6 @@ import org.apache.solr.SolrTestCaseJ4;
 import org.apache.solr.client.solrj.SolrClient;
 import org.apache.solr.client.solrj.impl.HttpSolrClient.RemoteSolrException;
 import org.apache.solr.client.solrj.request.ConfigSetAdminRequest.Create;
-import org.apache.solr.client.solrj.response.ConfigSetAdminResponse;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.cloud.SolrZkClient;
 import org.apache.solr.common.cloud.ZkConfigManager;
@@ -58,7 +57,6 @@ import org.apache.zookeeper.server.ZKDatabase;
 import org.apache.zookeeper.server.quorum.Leader.Proposal;
 import org.apache.zookeeper.txn.TxnHeader;
 import org.junit.After;
-import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -112,14 +110,10 @@ public class TestConfigSetsAPIZkFailure extends SolrTestCaseJ4 {
 
       Create create = new Create();
       create.setBaseConfigSetName(BASE_CONFIGSET_NAME).setConfigSetName(CONFIGSET_NAME);
-      try {
+      RemoteSolrException se = expectThrows(RemoteSolrException.class, () -> create.process(solrClient));
-        ConfigSetAdminResponse response = create.process(solrClient);
+      // partial creation should have been cleaned up
-        Assert.fail("Expected solr exception");
+      assertFalse(configManager.configExists(CONFIGSET_NAME));
-      } catch (RemoteSolrException se) {
+      assertEquals(SolrException.ErrorCode.SERVER_ERROR.code, se.code());
-        // partial creation should have been cleaned up
-        assertFalse(configManager.configExists(CONFIGSET_NAME));
-        assertEquals(SolrException.ErrorCode.SERVER_ERROR.code, se.code());
-      }
     } finally {
       zkClient.close();
     }

solr/core/src/test/org/apache/solr/cloud/TestDownShardTolerantSearch.java

@@ -69,14 +69,13 @@ public class TestDownShardTolerantSearch extends SolrCloudTestCase {
     assertThat(response.getStatus(), is(0));
     assertTrue(response.getResults().getNumFound() > 0);
 
-    try {
+    SolrServerException e = expectThrows(SolrServerException.class,
-      cluster.getSolrClient().query("tolerant", new SolrQuery("*:*").setRows(1).setParam(ShardParams.SHARDS_TOLERANT, false));
+        "Request should have failed because we killed shard1 jetty",
-      fail("Request should have failed because we killed shard1 jetty");
+        () -> cluster.getSolrClient().query("tolerant", new SolrQuery("*:*").setRows(1)
-    } catch (SolrServerException e) {
+            .setParam(ShardParams.SHARDS_TOLERANT, false))
-      log.info("error from server", e);
+    );
-      assertNotNull(e.getCause());
+    assertNotNull(e.getCause());
-      assertTrue("Error message from server should have the name of the down shard",
+    assertTrue("Error message from server should have the name of the down shard",
-          e.getCause().getMessage().contains("shard"));
+        e.getCause().getMessage().contains("shard"));
-    }
   }
 }

solr/core/src/test/org/apache/solr/cloud/TestLeaderInitiatedRecoveryThread.java

@@ -51,8 +51,8 @@ public class TestLeaderInitiatedRecoveryThread extends AbstractFullDistribZkTest
 
     final String leaderCoreNodeName = shardToLeaderJetty.get(SHARD1).coreNodeName;
     final CloudJettyRunner leaderRunner = shardToLeaderJetty.get(SHARD1);
-    CoreContainer coreContainer = leaderRunner.jetty.getCoreContainer();
+    final CoreContainer coreContainer1 = leaderRunner.jetty.getCoreContainer();
-    ZkController zkController = coreContainer.getZkController();
+    final ZkController zkController1 = coreContainer1.getZkController();
 
     CloudJettyRunner notLeader = null;
     for (CloudJettyRunner cloudJettyRunner : shardToJetty.get(SHARD1)) {
@@ -83,22 +83,21 @@ public class TestLeaderInitiatedRecoveryThread extends AbstractFullDistribZkTest
     /*
      1. Test that publishDownState throws exception when zkController.isReplicaInRecoveryHandling == false
       */
-    try {
-      
-      LeaderInitiatedRecoveryThread thread = new LeaderInitiatedRecoveryThread(zkController, coreContainer,
-          DEFAULT_COLLECTION, SHARD1, replicaCoreNodeProps, 1, cd);
-      assertFalse(zkController.isReplicaInRecoveryHandling(replicaCoreNodeProps.getCoreUrl()));
-      thread.run();
-      fail("publishDownState should not have succeeded because replica url is not marked in leader initiated recovery in ZkController");
-    } catch (SolrException e) {
-      assertTrue(e.code() == SolrException.ErrorCode.INVALID_STATE.code);
-    }
 
+    SolrException e = expectThrows(SolrException.class,
+        "publishDownState should not have succeeded because replica url is not marked in leader initiated recovery in ZkController",
+        () -> {
+      LeaderInitiatedRecoveryThread thread = new LeaderInitiatedRecoveryThread(zkController1, coreContainer1,
+          DEFAULT_COLLECTION, SHARD1, replicaCoreNodeProps, 1, cd);
+      assertFalse(zkController1.isReplicaInRecoveryHandling(replicaCoreNodeProps.getCoreUrl()));
+      thread.run();
+    });
+    assertEquals(e.code(), SolrException.ErrorCode.INVALID_STATE.code);
 
     /*
      2. Test that a non-live replica cannot be put into LIR or down state
       */
-    LeaderInitiatedRecoveryThread thread = new LeaderInitiatedRecoveryThread(zkController, coreContainer,
+    LeaderInitiatedRecoveryThread thread = new LeaderInitiatedRecoveryThread(zkController1, coreContainer1,
         DEFAULT_COLLECTION, SHARD1, replicaCoreNodeProps, 1, cd);
     // kill the replica
     int children = cloudClient.getZkStateReader().getZkClient().getChildren("/live_nodes", null, true).size();
@@ -129,7 +128,7 @@ public class TestLeaderInitiatedRecoveryThread extends AbstractFullDistribZkTest
     ChaosMonkey.start(notLeader.jetty);
     waitForRecoveriesToFinish(true);
 
-    thread = new LeaderInitiatedRecoveryThread(zkController, coreContainer,
+    thread = new LeaderInitiatedRecoveryThread(zkController1, coreContainer1,
         DEFAULT_COLLECTION, SHARD1, replicaCoreNodeProps, 1, cd) {
       @Override
       protected void updateLIRState(String replicaCoreNodeName) {
@@ -138,13 +137,13 @@ public class TestLeaderInitiatedRecoveryThread extends AbstractFullDistribZkTest
     };
     assertFalse(thread.publishDownState(replicaCoreNodeProps.getCoreName(), replica.getName(), replica.getNodeName(), replicaCoreNodeProps.getCoreUrl(), false));
     assertFalse(thread.publishDownState(replicaCoreNodeProps.getCoreName(), replica.getName(), replica.getNodeName(), replicaCoreNodeProps.getCoreUrl(), true));
-    assertNull(zkController.getLeaderInitiatedRecoveryState(DEFAULT_COLLECTION, SHARD1, replica.getName()));
+    assertNull(zkController1.getLeaderInitiatedRecoveryState(DEFAULT_COLLECTION, SHARD1, replica.getName()));
 
 
     /*
      4. Test that if ZK connection loss or session expired then thread should not attempt to publish down state even if forcePublish=true
       */
-    thread = new LeaderInitiatedRecoveryThread(zkController, coreContainer,
+    thread = new LeaderInitiatedRecoveryThread(zkController1, coreContainer1,
         DEFAULT_COLLECTION, SHARD1, replicaCoreNodeProps, 1, cd) {
       @Override
       protected void updateLIRState(String replicaCoreNodeName) {
@@ -153,13 +152,13 @@ public class TestLeaderInitiatedRecoveryThread extends AbstractFullDistribZkTest
     };
     assertFalse(thread.publishDownState(replicaCoreNodeProps.getCoreName(), replica.getName(), replica.getNodeName(), replicaCoreNodeProps.getCoreUrl(), false));
     assertFalse(thread.publishDownState(replicaCoreNodeProps.getCoreName(), replica.getName(), replica.getNodeName(), replicaCoreNodeProps.getCoreUrl(), true));
-    assertNull(zkController.getLeaderInitiatedRecoveryState(DEFAULT_COLLECTION, SHARD1, replica.getName()));
+    assertNull(zkController1.getLeaderInitiatedRecoveryState(DEFAULT_COLLECTION, SHARD1, replica.getName()));
 
 
     /*
      5. Test that any exception other then ZK connection loss or session expired should publish down state only if forcePublish=true
       */
-    thread = new LeaderInitiatedRecoveryThread(zkController, coreContainer,
+    thread = new LeaderInitiatedRecoveryThread(zkController1, coreContainer1,
         DEFAULT_COLLECTION, SHARD1, replicaCoreNodeProps, 1, cd) {
       @Override
       protected void updateLIRState(String replicaCoreNodeName) {
@@ -187,26 +186,21 @@ public class TestLeaderInitiatedRecoveryThread extends AbstractFullDistribZkTest
       Thread.sleep(500);
     }
 
-    assertNull(zkController.getLeaderInitiatedRecoveryState(DEFAULT_COLLECTION, SHARD1, replica.getName()));
+    assertNull(zkController1.getLeaderInitiatedRecoveryState(DEFAULT_COLLECTION, SHARD1, replica.getName()));
     assertEquals(Replica.State.DOWN, cloudClient.getZkStateReader().getClusterState().getCollection(DEFAULT_COLLECTION).getReplica(replica.getName()).getState());
 
     /*
     6. Test that non-leader cannot set LIR nodes
      */
 
-    coreContainer = notLeader.jetty.getCoreContainer();
+    final CoreContainer coreContainer2 = notLeader.jetty.getCoreContainer();
-    zkController = coreContainer.getZkController();
+    final ZkController zkController2 = coreContainer2.getZkController();
 
-    thread = new LeaderInitiatedRecoveryThread(zkController, coreContainer,
+    thread = new LeaderInitiatedRecoveryThread(zkController2, coreContainer2,
-        DEFAULT_COLLECTION, SHARD1, replicaCoreNodeProps, 1, coreContainer.getCores().iterator().next().getCoreDescriptor()) {
+        DEFAULT_COLLECTION, SHARD1, replicaCoreNodeProps, 1, coreContainer2.getCores().iterator().next().getCoreDescriptor()) {
       @Override
       protected void updateLIRState(String replicaCoreNodeName) {
-        try {
+        throw expectThrows(ZkController.NotLeaderException.class, () -> super.updateLIRState(replicaCoreNodeName));
-          super.updateLIRState(replicaCoreNodeName);
-        } catch (Exception e) {
-          assertTrue(e instanceof ZkController.NotLeaderException);
-          throw e;
-        }
       }
     };
     cversion = getOverseerCversion();
@@ -217,21 +211,21 @@ public class TestLeaderInitiatedRecoveryThread extends AbstractFullDistribZkTest
      7. assert that we can write a LIR state if everything else is fine
       */
     // reset the zkcontroller to the one from the leader
-    coreContainer = leaderRunner.jetty.getCoreContainer();
+    final CoreContainer coreContainer3 = leaderRunner.jetty.getCoreContainer();
-    zkController = coreContainer.getZkController();
+    final ZkController zkController3 = coreContainer3.getZkController();
-    thread = new LeaderInitiatedRecoveryThread(zkController, coreContainer,
+    thread = new LeaderInitiatedRecoveryThread(zkController3, coreContainer3,
-        DEFAULT_COLLECTION, SHARD1, replicaCoreNodeProps, 1, coreContainer.getCores().iterator().next().getCoreDescriptor());
+        DEFAULT_COLLECTION, SHARD1, replicaCoreNodeProps, 1, coreContainer3.getCores().iterator().next().getCoreDescriptor());
     thread.publishDownState(replicaCoreNodeProps.getCoreName(), replica.getName(), replica.getNodeName(), replicaCoreNodeProps.getCoreUrl(), false);
     timeOut = new TimeOut(30, TimeUnit.SECONDS, TimeSource.NANO_TIME);
     while (!timeOut.hasTimedOut()) {
-      Replica.State state = zkController.getLeaderInitiatedRecoveryState(DEFAULT_COLLECTION, SHARD1, replica.getName());
+      Replica.State state = zkController3.getLeaderInitiatedRecoveryState(DEFAULT_COLLECTION, SHARD1, replica.getName());
       if (state == Replica.State.DOWN) {
         break;
       }
       Thread.sleep(500);
     }
-    assertNotNull(zkController.getLeaderInitiatedRecoveryStateObject(DEFAULT_COLLECTION, SHARD1, replica.getName()));
+    assertNotNull(zkController3.getLeaderInitiatedRecoveryStateObject(DEFAULT_COLLECTION, SHARD1, replica.getName()));
-    assertEquals(Replica.State.DOWN, zkController.getLeaderInitiatedRecoveryState(DEFAULT_COLLECTION, SHARD1, replica.getName()));
+    assertEquals(Replica.State.DOWN, zkController3.getLeaderInitiatedRecoveryState(DEFAULT_COLLECTION, SHARD1, replica.getName()));
 
     /*
     7. Test that

solr/core/src/test/org/apache/solr/cloud/TestPullReplicaErrorHandling.java

@@ -152,12 +152,13 @@ public void testCantConnectToPullReplica() throws Exception {
           assertNumDocs(10 + i, leaderClient);
         }
       }
-      try (HttpSolrClient pullReplicaClient = getHttpSolrClient(s.getReplicas(EnumSet.of(Replica.Type.PULL)).get(0).getCoreUrl())) {
+
-        pullReplicaClient.query(new SolrQuery("*:*")).getResults().getNumFound();
+      SolrServerException e = expectThrows(SolrServerException.class, () -> {
-        fail("Shouldn't be able to query the pull replica");
+        try(HttpSolrClient pullReplicaClient = getHttpSolrClient(s.getReplicas(EnumSet.of(Replica.Type.PULL)).get(0).getCoreUrl())) {
-      } catch (SolrServerException e) {
+          pullReplicaClient.query(new SolrQuery("*:*")).getResults().getNumFound();
-        //expected
+        }
-      }
+      });
+      
       assertNumberOfReplicas(numShards, 0, numShards, true, true);// Replica should still be active, since it doesn't disconnect from ZooKeeper
       {
         long numFound = 0;

solr/core/src/test/org/apache/solr/cloud/TestSolrCloudWithDelegationTokens.java

@@ -16,7 +16,6 @@
  */
 package org.apache.solr.cloud;
 
-import junit.framework.Assert;
 import org.apache.hadoop.util.Time;
 import org.apache.lucene.util.LuceneTestCase;
 import org.apache.solr.SolrTestCaseJ4;
@@ -383,33 +382,31 @@ public class TestSolrCloudWithDelegationTokens extends SolrTestCaseJ4 {
     SolrRequest request = getAdminRequest(new ModifiableSolrParams());
 
     // test without token
-    HttpSolrClient ss =
+    final HttpSolrClient ssWoToken =
-        new HttpSolrClient.Builder(solrClientPrimary.getBaseURL().toString())
+      new HttpSolrClient.Builder(solrClientPrimary.getBaseURL().toString())
-            .withResponseParser(solrClientPrimary.getParser())
+          .withResponseParser(solrClientPrimary.getParser())
-            .build();
+          .build();
     try {
-      doSolrRequest(ss, request, ErrorCode.UNAUTHORIZED.code);
+      doSolrRequest(ssWoToken, request, ErrorCode.UNAUTHORIZED.code);
     } finally {
-      ss.close();
+      ssWoToken.close();
     }
 
-    ss = new HttpSolrClient.Builder(solrClientPrimary.getBaseURL().toString())
+    final HttpSolrClient ssWToken = new HttpSolrClient.Builder(solrClientPrimary.getBaseURL().toString())
         .withKerberosDelegationToken(token)
         .withResponseParser(solrClientPrimary.getParser())
         .build();
     try {
       // test with token via property
-      doSolrRequest(ss, request, HttpStatus.SC_OK);
+      doSolrRequest(ssWToken, request, HttpStatus.SC_OK);
 
       // test with param -- should throw an exception
       ModifiableSolrParams tokenParam = new ModifiableSolrParams();
       tokenParam.set("delegation", "invalidToken");
-      try {
+      expectThrows(IllegalArgumentException.class,
-        doSolrRequest(ss, getAdminRequest(tokenParam), ErrorCode.FORBIDDEN.code);
+          () -> doSolrRequest(ssWToken, getAdminRequest(tokenParam), ErrorCode.FORBIDDEN.code));
-        Assert.fail("Expected exception");
-      } catch (IllegalArgumentException ex) {}
     } finally {
-      ss.close();
+      ssWToken.close();
     }
   }
 }

solr/core/src/test/org/apache/solr/cloud/TestSolrCloudWithSecureImpersonation.java

@@ -223,37 +223,28 @@ public class TestSolrCloudWithSecureImpersonation extends SolrTestCaseJ4 {
 
   @Test
   public void testProxyNoConfigGroups() throws Exception {
-    try {
+    HttpSolrClient.RemoteSolrException e = expectThrows(HttpSolrClient.RemoteSolrException.class,
-      solrClient.request(getProxyRequest("noGroups","bar"));
+        () -> solrClient.request(getProxyRequest("noGroups","bar"))
-      fail("Expected RemoteSolrException");
+    );
-    }
+    assertTrue(e.getMessage().contains(getExpectedGroupExMsg("noGroups", "bar")));
-    catch (HttpSolrClient.RemoteSolrException ex) {
-      assertTrue(ex.getMessage().contains(getExpectedGroupExMsg("noGroups", "bar")));
-    }
   }
 
   @Test
   public void testProxyWrongHost() throws Exception {
-    try {
+    HttpSolrClient.RemoteSolrException e = expectThrows(HttpSolrClient.RemoteSolrException.class,
-      solrClient.request(getProxyRequest("wrongHost","bar"));
+        () -> solrClient.request(getProxyRequest("wrongHost","bar"))
-      fail("Expected RemoteSolrException");
+    );
-    }
+    assertTrue(e.getMessage().contains(getExpectedHostExMsg("wrongHost")));
-    catch (HttpSolrClient.RemoteSolrException ex) {
-      assertTrue(ex.getMessage().contains(getExpectedHostExMsg("wrongHost")));
-    }
   }
 
   @Test
   public void testProxyNoConfigHosts() throws Exception {
-    try {
+    HttpSolrClient.RemoteSolrException e = expectThrows(HttpSolrClient.RemoteSolrException.class,
-      solrClient.request(getProxyRequest("noHosts","bar"));
+        () -> solrClient.request(getProxyRequest("noHosts","bar"))
-      fail("Expected RemoteSolrException");
+    );
-    }
+    // FixMe: this should return an exception about the host being invalid,
-    catch (HttpSolrClient.RemoteSolrException ex) {
+    // but a bug (HADOOP-11077) causes an NPE instead.
-      // FixMe: this should return an exception about the host being invalid,
+    // assertTrue(ex.getMessage().contains(getExpectedHostExMsg("noHosts")));
-      // but a bug (HADOOP-11077) causes an NPE instead.
-      //assertTrue(ex.getMessage().contains(getExpectedHostExMsg("noHosts")));
-    }
   }
 
   @Test
@@ -264,14 +255,11 @@ public class TestSolrCloudWithSecureImpersonation extends SolrTestCaseJ4 {
 
   @Test
   public void testProxyInvalidProxyUser() throws Exception {
-    try {
+    // wrong direction, should fail
-      // wrong direction, should fail
+    HttpSolrClient.RemoteSolrException e = expectThrows(HttpSolrClient.RemoteSolrException.class,
-      solrClient.request(getProxyRequest("bar","anyHostAnyUser"));
+        () -> solrClient.request(getProxyRequest("bar","anyHostAnyUser"))
-      fail("Expected RemoteSolrException");
+    );
-    }
+    assertTrue(e.getMessage().contains(getExpectedGroupExMsg("bar", "anyHostAnyUser")));
-    catch (HttpSolrClient.RemoteSolrException ex) {
-      assertTrue(ex.getMessage().contains(getExpectedGroupExMsg("bar", "anyHostAnyUser")));
-    }
   }
 
   @Test
@@ -290,49 +278,38 @@ public class TestSolrCloudWithSecureImpersonation extends SolrTestCaseJ4 {
 
   @Test
   public void testProxyUnknownRemote() throws Exception {
-    try {
+    HttpSolrClient.RemoteSolrException e = expectThrows(HttpSolrClient.RemoteSolrException.class,
-      // Use a reserved ip address
+        () -> {
-      String nonProxyUserConfiguredIpAddress = "255.255.255.255";
+          // Use a reserved ip address
-      solrClient.request(getProxyRequest("localHostAnyGroup", "bar", "unknownhost.bar.foo", nonProxyUserConfiguredIpAddress));
+          String nonProxyUserConfiguredIpAddress = "255.255.255.255";
-      fail("Expected RemoteSolrException");
+          solrClient.request(getProxyRequest("localHostAnyGroup", "bar", "unknownhost.bar.foo", nonProxyUserConfiguredIpAddress));
-    }
+    });
-    catch (HttpSolrClient.RemoteSolrException ex) {
+    assertTrue(e.getMessage().contains(getExpectedHostExMsg("localHostAnyGroup")));
-      assertTrue(ex.getMessage().contains(getExpectedHostExMsg("localHostAnyGroup")));
-    }
   }
 
   @Test
   public void testProxyInvalidRemote() throws Exception {
-    try {
+    HttpSolrClient.RemoteSolrException e = expectThrows(HttpSolrClient.RemoteSolrException.class,
-      String invalidIpAddress = "-127.-128";
+        () -> {
-      solrClient.request(getProxyRequest("localHostAnyGroup","bar", "[ff01::114]", invalidIpAddress));
+          String invalidIpAddress = "-127.-128";
-      fail("Expected RemoteSolrException");
+          solrClient.request(getProxyRequest("localHostAnyGroup","bar", "[ff01::114]", invalidIpAddress));
-    }
+    });
-    catch (HttpSolrClient.RemoteSolrException ex) {
+    assertTrue(e.getMessage().contains(getExpectedHostExMsg("localHostAnyGroup")));
-      assertTrue(ex.getMessage().contains(getExpectedHostExMsg("localHostAnyGroup")));
-    }
   }
 
   @Test
   public void testProxyInvalidGroup() throws Exception {
-    try {
+    HttpSolrClient.RemoteSolrException e = expectThrows(HttpSolrClient.RemoteSolrException.class,
-      solrClient.request(getProxyRequest("bogusGroup","bar", null));
+        () -> solrClient.request(getProxyRequest("bogusGroup","bar", null))
-      fail("Expected RemoteSolrException");
+    );
-    }
+    assertTrue(e.getMessage().contains(getExpectedGroupExMsg("bogusGroup", "bar")));
-    catch (HttpSolrClient.RemoteSolrException ex) {
-      assertTrue(ex.getMessage().contains(getExpectedGroupExMsg("bogusGroup", "bar")));
-    }
   }
 
   @Test
   public void testProxyNullProxyUser() throws Exception {
-    try {
+    expectThrows(HttpSolrClient.RemoteSolrException.class,
-      solrClient.request(getProxyRequest("","bar"));
+        () -> solrClient.request(getProxyRequest("","bar"))
-      fail("Expected RemoteSolrException");
+    );
-    }
-    catch (HttpSolrClient.RemoteSolrException ex) {
-      // this exception is specific to our implementation, don't check a specific message.
-    }
   }
 
   @Test

solr/core/src/test/org/apache/solr/cloud/TestTolerantUpdateProcessorCloud.java

@@ -239,38 +239,32 @@ public class TestTolerantUpdateProcessorCloud extends SolrCloudTestCase {
       assertQueryDocIds(c, false, "id_not_exists");
 
       // verify adding 2 broken docs causes a clint exception
-      try {
+      SolrException e = expectThrows(SolrException.class,
-        UpdateResponse rsp = update(params(),
+          "did not get a top level exception when more then 10 docs failed", () ->
-                                    doc(f("id", S_ONE_PRE + "X"), f("foo_i", "bogus_val_X")),
+              update(params(),
-                                    doc(f("id", S_TWO_PRE + "Y"), f("foo_i", "bogus_val_Y"))
+                  doc(f("id", S_ONE_PRE + "X"), f("foo_i", "bogus_val_X")),
-                                    ).process(c);
+                  doc(f("id", S_TWO_PRE + "Y"), f("foo_i", "bogus_val_Y"))
-        fail("did not get a top level exception when more then 10 docs failed: " + rsp.toString());
+              ).process(c)
-      } catch (SolrException e) {
+      );
-        assertEquals("not the type of error we were expecting ("+e.code()+"): " + e.toString(),
+      assertEquals("not the type of error we were expecting ("+e.code()+"): " + e.toString(),
-                     400, e.code());
+          400, e.code());
-      }
         
       // verify malformed deleteByQuerys fail
-      try {
+      e = expectThrows(SolrException.class,
-        UpdateResponse rsp = update(params()).deleteByQuery("foo_i:not_a_num").process(c);
+          "sanity check for malformed DBQ didn't fail",
-        fail("sanity check for malformed DBQ didn't fail: " + rsp.toString());
+          () -> update(params()).deleteByQuery("foo_i:not_a_num").process(c));
-      } catch (SolrException e) {
+      assertEquals("not the expected DBQ failure: " + e.getMessage(), 400, e.code());
-        assertEquals("not the expected DBQ failure: " + e.getMessage(), 400, e.code());
-      }
       
       // verify opportunistic concurrency deletions fail as we expect when docs are / aren't present
       for (UpdateRequest r : new UpdateRequest[] {
           update(params("commit", "true")).deleteById(S_ONE_PRE + "1", -1L),
           update(params("commit", "true")).deleteById(S_TWO_PRE + "2", -1L),
           update(params("commit", "true")).deleteById("id_not_exists",  1L)    }) {
-        try {
+        e = expectThrows(SolrException.class, "sanity check for opportunistic concurrency delete didn't fail",
-          UpdateResponse rsp = r.process(c);
+            () -> r.process(c)
-          fail("sanity check for opportunistic concurrency delete didn't fail: "
+        );
-               + r.toString() + " => " + rsp.toString());
+        assertEquals("not the expected opportunistic concurrency failure code: "
-        } catch (SolrException e) {
+            + r.toString() + " => " + e.getMessage(), 409, e.code());
-          assertEquals("not the expected opportunistic concurrency failure code: "
-                       + r.toString() + " => " + e.getMessage(), 409, e.code());
-        }
       }
     }
   }
@@ -538,51 +532,49 @@ public class TestTolerantUpdateProcessorCloud extends SolrCloudTestCase {
     assertEquals(0, client.deleteByQuery("*:*").getStatus());
     
     // many docs from diff shards, more then 10 (total) should fail
-
+    SolrException e = expectThrows(SolrException.class,
-    try {
+        "did not get a top level exception when more then 10 docs failed",
-      rsp = update(params("update.chain", "tolerant-chain-max-errors-10",
+        () -> update(params("update.chain", "tolerant-chain-max-errors-10", "commit", "true"),
-                          "commit", "true"),
+            doc(f("id", S_ONE_PRE + "11")),
-                   doc(f("id", S_ONE_PRE + "11")),
+            doc(f("id", S_TWO_PRE + "21"), f("foo_i", "bogus_val")),
-                   doc(f("id", S_TWO_PRE + "21"), f("foo_i", "bogus_val")),
+            doc(f("id", S_ONE_PRE + "12")),
-                   doc(f("id", S_ONE_PRE + "12")),
+            doc(f("id", S_TWO_PRE + "22"), f("foo_i", "bogus_val")),
-                   doc(f("id", S_TWO_PRE + "22"), f("foo_i", "bogus_val")),
+            doc(f("id", S_ONE_PRE + "13")),
-                   doc(f("id", S_ONE_PRE + "13")),
+            doc(f("id", S_TWO_PRE + "23"), f("foo_i", "bogus_val")),
-                   doc(f("id", S_TWO_PRE + "23"), f("foo_i", "bogus_val")),
+            doc(f("id", S_ONE_PRE + "14"), f("foo_i", "bogus_val")),
-                   doc(f("id", S_ONE_PRE + "14"), f("foo_i", "bogus_val")),
+            doc(f("id", S_TWO_PRE + "24")),
-                   doc(f("id", S_TWO_PRE + "24")),
+            doc(f("id", S_ONE_PRE + "15"), f("foo_i", "bogus_val")),
-                   doc(f("id", S_ONE_PRE + "15"), f("foo_i", "bogus_val")),
+            doc(f("id", S_TWO_PRE + "25")),
-                   doc(f("id", S_TWO_PRE + "25")),
+            doc(f("id", S_ONE_PRE + "16"), f("foo_i", "bogus_val")),
-                   doc(f("id", S_ONE_PRE + "16"), f("foo_i", "bogus_val")),
+            doc(f("id", S_TWO_PRE + "26"), f("foo_i", "bogus_val")),
-                   doc(f("id", S_TWO_PRE + "26"), f("foo_i", "bogus_val")),
+            doc(f("id", S_ONE_PRE + "17")),
-                   doc(f("id", S_ONE_PRE + "17")),
+            doc(f("id", S_TWO_PRE + "27")),
-                   doc(f("id", S_TWO_PRE + "27")),
+            doc(f("id", S_ONE_PRE + "18"), f("foo_i", "bogus_val")),
-                   doc(f("id", S_ONE_PRE + "18"), f("foo_i", "bogus_val")),
+            doc(f("id", S_TWO_PRE + "28"), f("foo_i", "bogus_val")),
-                   doc(f("id", S_TWO_PRE + "28"), f("foo_i", "bogus_val")),
+            doc(f("id", S_ONE_PRE + "19"), f("foo_i", "bogus_val")),
-                   doc(f("id", S_ONE_PRE + "19"), f("foo_i", "bogus_val")),
+            doc(f("id", S_TWO_PRE + "29"), f("foo_i", "bogus_val")),
-                   doc(f("id", S_TWO_PRE + "29"), f("foo_i", "bogus_val")),
+            doc(f("id", S_ONE_PRE + "10")), // may be skipped, more then 10 fails
-                   doc(f("id", S_ONE_PRE + "10")), // may be skipped, more then 10 fails
+            doc(f("id", S_TWO_PRE + "20"))  // may be skipped, more then 10 fails
-                   doc(f("id", S_TWO_PRE + "20"))  // may be skipped, more then 10 fails
+            ).process(client)
-                   ).process(client);
+    );
-      
+    {
-      fail("did not get a top level exception when more then 10 docs failed: " + rsp.toString());
-    } catch (SolrException e) {
       // we can't make any reliable assertions about the error message, because
       // it varies based on how the request was routed -- see SOLR-8830
-      assertEquals("not the type of error we were expecting ("+e.code()+"): " + e.toString(),
+      assertEquals("not the type of error we were expecting (" + e.code() + "): " + e.toString(),
-                   // NOTE: we always expect a 400 because we know that's what we would get from these types of errors
+          // NOTE: we always expect a 400 because we know that's what we would get from these types of errors
-                   // on a single node setup -- a 5xx type error isn't something we should have triggered
+          // on a single node setup -- a 5xx type error isn't something we should have triggered
-                   400, e.code());
+          400, e.code());
 
       // verify that the Exceptions metadata can tell us what failed.
       NamedList<String> remoteErrMetadata = e.getMetadata();
       assertNotNull("no metadata in: " + e.toString(), remoteErrMetadata);
       Set<ToleratedUpdateError> actualKnownErrs
-        = new LinkedHashSet<ToleratedUpdateError>(remoteErrMetadata.size());
+          = new LinkedHashSet<ToleratedUpdateError>(remoteErrMetadata.size());
       int actualKnownErrsCount = 0;
       for (int i = 0; i < remoteErrMetadata.size(); i++) {
         ToleratedUpdateError err =
-          ToleratedUpdateError.parseMetadataIfToleratedUpdateError(remoteErrMetadata.getName(i),
+            ToleratedUpdateError.parseMetadataIfToleratedUpdateError(remoteErrMetadata.getName(i),
-                                                                   remoteErrMetadata.getVal(i));
+                remoteErrMetadata.getVal(i));
         if (null == err) {
           // some metadata unrelated to this update processor
           continue;
@@ -591,16 +583,17 @@ public class TestTolerantUpdateProcessorCloud extends SolrCloudTestCase {
         actualKnownErrs.add(err);
       }
       assertEquals("wrong number of errors in metadata: " + remoteErrMetadata.toString(),
-                   11, actualKnownErrsCount);
+          11, actualKnownErrsCount);
       assertEquals("at least one dup error in metadata: " + remoteErrMetadata.toString(),
-                   actualKnownErrsCount, actualKnownErrs.size());
+          actualKnownErrsCount, actualKnownErrs.size());
       for (ToleratedUpdateError err : actualKnownErrs) {
         assertEquals("only expected type of error is ADD: " + err,
-                     CmdType.ADD, err.getType());
+            CmdType.ADD, err.getType());
         assertTrue("failed err msg didn't match expected value: " + err,
-                   err.getMessage().contains("bogus_val"));
+            err.getMessage().contains("bogus_val"));
       }
     }
+
     assertEquals(0, client.commit().getStatus()); // need to force since update didn't finish
     assertQueryDocIds(client, false
                       // explicitly failed
@@ -621,7 +614,8 @@ public class TestTolerantUpdateProcessorCloud extends SolrCloudTestCase {
     
     // many docs from diff shards, more then 10 from a single shard (two) should fail
 
-    try {
+    e = expectThrows(SolrException.class, "did not get a top level exception when more then 10 docs failed",
+        () -> {
       ArrayList<SolrInputDocument> docs = new ArrayList<SolrInputDocument>(30);
       docs.add(doc(f("id", S_ONE_PRE + "z")));
       docs.add(doc(f("id", S_TWO_PRE + "z")));
@@ -633,30 +627,30 @@ public class TestTolerantUpdateProcessorCloud extends SolrCloudTestCase {
       }
       docs.add(doc(f("id", S_ONE_PRE + "x"))); // may be skipped, more then 10 fails
       docs.add(doc(f("id", S_TWO_PRE + "x"))); // may be skipped, more then 10 fails
-          
+
-      rsp = update(params("update.chain", "tolerant-chain-max-errors-10",
+      update(params("update.chain", "tolerant-chain-max-errors-10",
-                          "commit", "true"),
+          "commit", "true"),
-                   docs.toArray(new SolrInputDocument[docs.size()])).process(client);
+          docs.toArray(new SolrInputDocument[docs.size()])).process(client);
-      
+    });
-      fail("did not get a top level exception when more then 10 docs failed: " + rsp.toString());
+
-    } catch (SolrException e) {
+    {
       // we can't make any reliable assertions about the error message, because
       // it varies based on how the request was routed -- see SOLR-8830
       assertEquals("not the type of error we were expecting ("+e.code()+"): " + e.toString(),
-                   // NOTE: we always expect a 400 because we know that's what we would get from these types of errors
+          // NOTE: we always expect a 400 because we know that's what we would get from these types of errors
-                   // on a single node setup -- a 5xx type error isn't something we should have triggered
+          // on a single node setup -- a 5xx type error isn't something we should have triggered
-                   400, e.code());
+          400, e.code());
 
       // verify that the Exceptions metadata can tell us what failed.
       NamedList<String> remoteErrMetadata = e.getMetadata();
       assertNotNull("no metadata in: " + e.toString(), remoteErrMetadata);
       Set<ToleratedUpdateError> actualKnownErrs
-        = new LinkedHashSet<ToleratedUpdateError>(remoteErrMetadata.size());
+          = new LinkedHashSet<ToleratedUpdateError>(remoteErrMetadata.size());
       int actualKnownErrsCount = 0;
       for (int i = 0; i < remoteErrMetadata.size(); i++) {
         ToleratedUpdateError err =
-          ToleratedUpdateError.parseMetadataIfToleratedUpdateError(remoteErrMetadata.getName(i),
+            ToleratedUpdateError.parseMetadataIfToleratedUpdateError(remoteErrMetadata.getName(i),
-                                                                   remoteErrMetadata.getVal(i));
+                remoteErrMetadata.getVal(i));
         if (null == err) {
           // some metadata unrelated to this update processor
           continue;
@@ -665,19 +659,19 @@ public class TestTolerantUpdateProcessorCloud extends SolrCloudTestCase {
         actualKnownErrs.add(err);
       }
       assertEquals("wrong number of errors in metadata: " + remoteErrMetadata.toString(),
-                   11, actualKnownErrsCount);
+          11, actualKnownErrsCount);
       assertEquals("at least one dup error in metadata: " + remoteErrMetadata.toString(),
-                   actualKnownErrsCount, actualKnownErrs.size());
+          actualKnownErrsCount, actualKnownErrs.size());
       for (ToleratedUpdateError err : actualKnownErrs) {
         assertEquals("only expected type of error is ADD: " + err,
-                     CmdType.ADD, err.getType());
+            CmdType.ADD, err.getType());
         assertTrue("failed id had unexpected prefix: " + err,
-                   err.getId().startsWith(S_TWO_PRE));
+            err.getId().startsWith(S_TWO_PRE));
         assertTrue("failed err msg didn't match expected value: " + err,
-                   err.getMessage().contains("bogus_val"));
+            err.getMessage().contains("bogus_val"));
       }
-           
     }
+
     assertEquals(0, client.commit().getStatus()); // need to force since update didn't finish
     assertQueryDocIds(client, true
                       , S_ONE_PRE + "z", S_ONE_PRE + "y", S_TWO_PRE + "z", S_TWO_PRE + "y" // first
@@ -700,7 +694,9 @@ public class TestTolerantUpdateProcessorCloud extends SolrCloudTestCase {
     
     // many docs from diff shards, more then 10 don't have any uniqueKey specified
 
-    try {
+    e = expectThrows(SolrException.class,
+        "did not get a top level exception when more then 10 docs mising uniqueKey",
+        () -> {
       ArrayList<SolrInputDocument> docs = new ArrayList<SolrInputDocument>(30);
       docs.add(doc(f("id", S_ONE_PRE + "z")));
       docs.add(doc(f("id", S_TWO_PRE + "z")));
@@ -712,19 +708,19 @@ public class TestTolerantUpdateProcessorCloud extends SolrCloudTestCase {
       }
       docs.add(doc(f("id", S_ONE_PRE + "x"))); // may be skipped, more then 10 fails
       docs.add(doc(f("id", S_TWO_PRE + "x"))); // may be skipped, more then 10 fails
-          
+
-      rsp = update(params("update.chain", "tolerant-chain-max-errors-10",
+      update(params("update.chain", "tolerant-chain-max-errors-10",
-                          "commit", "true"),
+          "commit", "true"),
-                   docs.toArray(new SolrInputDocument[docs.size()])).process(client);
+          docs.toArray(new SolrInputDocument[docs.size()])).process(client);
-      
+    });
-      fail("did not get a top level exception when more then 10 docs mising uniqueKey: " + rsp.toString());
+
-    } catch (SolrException e) {
+    {
       // we can't make any reliable assertions about the error message, because
       // it varies based on how the request was routed -- see SOLR-8830
       assertEquals("not the type of error we were expecting ("+e.code()+"): " + e.toString(),
-                   // NOTE: we always expect a 400 because we know that's what we would get from these types of errors
+          // NOTE: we always expect a 400 because we know that's what we would get from these types of errors
-                   // on a single node setup -- a 5xx type error isn't something we should have triggered
+          // on a single node setup -- a 5xx type error isn't something we should have triggered
-                   400, e.code());
+          400, e.code());
 
       // verify that the Exceptions metadata can tell us what failed.
       NamedList<String> remoteErrMetadata = e.getMetadata();
@@ -732,21 +728,22 @@ public class TestTolerantUpdateProcessorCloud extends SolrCloudTestCase {
       int actualKnownErrsCount = 0;
       for (int i = 0; i < remoteErrMetadata.size(); i++) {
         ToleratedUpdateError err =
-          ToleratedUpdateError.parseMetadataIfToleratedUpdateError(remoteErrMetadata.getName(i),
+            ToleratedUpdateError.parseMetadataIfToleratedUpdateError(remoteErrMetadata.getName(i),
-                                                                   remoteErrMetadata.getVal(i));
+                remoteErrMetadata.getVal(i));
         if (null == err) {
           // some metadata unrelated to this update processor
           continue;
         }
         actualKnownErrsCount++;
         assertEquals("only expected type of error is ADD: " + err,
-                     CmdType.ADD, err.getType());
+            CmdType.ADD, err.getType());
         assertTrue("failed id didn't match 'unknown': " + err,
-                   err.getId().contains("unknown"));
+            err.getId().contains("unknown"));
       }
       assertEquals("wrong number of errors in metadata: " + remoteErrMetadata.toString(),
-                   11, actualKnownErrsCount);
+          11, actualKnownErrsCount);
     }
+
     assertEquals(0, client.commit().getStatus()); // need to force since update didn't finish
     assertQueryDocIds(client, true
                       , S_ONE_PRE + "z", S_ONE_PRE + "y", S_TWO_PRE + "z", S_TWO_PRE + "y" // first
@@ -857,36 +854,39 @@ public class TestTolerantUpdateProcessorCloud extends SolrCloudTestCase {
     }
     
     // attempt a request containing 4 errors of various types (add, delI, delQ) .. 1 too many
-    try {
+
-      rsp = update(params("update.chain", "tolerant-chain-max-errors-10",
+    SolrException e = expectThrows(SolrException.class,
-                          "maxErrors", "3",
+        "did not get a top level exception when more then 4 updates failed",
-                          "commit", "true"),
+        () -> update(params("update.chain", "tolerant-chain-max-errors-10",
-                   doc(f("id", docId22), f("foo_i", "bogus_val")))
+            "maxErrors", "3",
-        .deleteById(docId1, -1L)
+            "commit", "true"),
-        .deleteByQuery("malformed:[")
+            doc(f("id", docId22), f("foo_i", "bogus_val")))
-        .deleteById(docId21, -1L)
+            .deleteById(docId1, -1L)
-        .process(client);
+            .deleteByQuery("malformed:[")
-      fail("did not get a top level exception when more then 4 updates failed: " + rsp.toString());
+            .deleteById(docId21, -1L)
-    } catch (SolrException e) {
+            .process(client)
+    );
+
+    {
       // we can't make any reliable assertions about the error message, because
       // it varies based on how the request was routed -- see SOLR-8830
-      
+
       // likewise, we can't make a firm(er) assertion about the response code...
       assertTrue("not the type of error we were expecting ("+e.code()+"): " + e.toString(),
-                 // should be one these 2 depending on order that the async errors were hit...
+          // should be one these 2 depending on order that the async errors were hit...
-                 // on a single node setup -- a 5xx type error isn't something we should have triggered
+          // on a single node setup -- a 5xx type error isn't something we should have triggered
-                 400 == e.code() || 409 == e.code());
+          400 == e.code() || 409 == e.code());
 
       // verify that the Exceptions metadata can tell us what failed.
       NamedList<String> remoteErrMetadata = e.getMetadata();
       assertNotNull("no metadata in: " + e.toString(), remoteErrMetadata);
       Set<ToleratedUpdateError> actualKnownErrs
-        = new LinkedHashSet<ToleratedUpdateError>(remoteErrMetadata.size());
+          = new LinkedHashSet<ToleratedUpdateError>(remoteErrMetadata.size());
       int actualKnownErrsCount = 0;
       for (int i = 0; i < remoteErrMetadata.size(); i++) {
         ToleratedUpdateError err =
-          ToleratedUpdateError.parseMetadataIfToleratedUpdateError(remoteErrMetadata.getName(i),
+            ToleratedUpdateError.parseMetadataIfToleratedUpdateError(remoteErrMetadata.getName(i),
-                                                                   remoteErrMetadata.getVal(i));
+                remoteErrMetadata.getVal(i));
         if (null == err) {
           // some metadata unrelated to this update processor
           continue;
@@ -895,9 +895,9 @@ public class TestTolerantUpdateProcessorCloud extends SolrCloudTestCase {
         actualKnownErrs.add(err);
       }
       assertEquals("wrong number of errors in metadata: " + remoteErrMetadata.toString(),
-                   4, actualKnownErrsCount);
+          4, actualKnownErrsCount);
       assertEquals("at least one dup error in metadata: " + remoteErrMetadata.toString(),
-                   actualKnownErrsCount, actualKnownErrs.size());
+          actualKnownErrsCount, actualKnownErrs.size());
     }
 
     // sanity check our 2 existing docs are still here

solr/core/src/test/org/apache/solr/cloud/TestZkChroot.java

@@ -99,23 +99,20 @@ public class TestZkChroot extends SolrTestCaseJ4 {
     
     System.setProperty("bootstrap_conf", "false");
     System.setProperty("zkHost", zkServer.getZkHost() + chroot);
-    
+
-    SolrZkClient zkClient = null;
+    try(SolrZkClient zkClient = new SolrZkClient(zkServer.getZkHost(),
-    
+        AbstractZkTestCase.TIMEOUT)) {
-    try {
+      expectThrows(ZooKeeperException.class,
-      zkClient = new SolrZkClient(zkServer.getZkHost(),
+          "did not get a top level exception when more then 4 updates failed",
-          AbstractZkTestCase.TIMEOUT);
+          () -> {
-      assertFalse("Path '" + chroot + "' should not exist before the test",
+        assertFalse("Path '" + chroot + "' should not exist before the test",
-          zkClient.exists(chroot, true));
+            zkClient.exists(chroot, true));
-      cores = CoreContainer.createAndLoad(home);
+        cores = CoreContainer.createAndLoad(home);
-      fail("There should be a zk exception, as the initial path doesn't exist");
+      });
-    } catch (ZooKeeperException e) {
-      // expected
       assertFalse("Path shouldn't have been created",
           zkClient.exists(chroot, true));// check the path was not created
     } finally {
       if (cores != null) cores.shutdown();
-      if (zkClient != null) zkClient.close();
     }
   }
   

solr/core/src/test/org/apache/solr/cloud/ZkCLITest.java

@@ -178,14 +178,9 @@ public class ZkCLITest extends SolrTestCaseJ4 {
     // test put file
     String[] args = new String[] {"-zkhost", zkServer.getZkAddress(), "-cmd",
         "putfile", "/solr.xml", SOLR_HOME + File.separator + "not-there.xml"};
-    try {
+    FileNotFoundException e = expectThrows(FileNotFoundException.class, () -> ZkCLI.main(args));
-      ZkCLI.main(args);
+    assertTrue("Didn't find expected error message containing 'not-there.xml' in " + e.getMessage(),
-      fail("Should have had a file not found exception");
+        e.getMessage().indexOf("not-there.xml") != -1);
-    } catch (FileNotFoundException fne) {
-      String msg = fne.getMessage();
-      assertTrue("Didn't find expected error message containing 'not-there.xml' in " + msg,
-          msg.indexOf("not-there.xml") != -1);
-    }
   }
 
   @Test
@@ -332,11 +327,8 @@ public class ZkCLITest extends SolrTestCaseJ4 {
     File file = createTempFile("newfile", null).toFile();
     String[] args = new String[] {"-zkhost", zkServer.getZkAddress(), "-cmd",
         "getfile", getNode, file.getAbsolutePath()};
-    try {
+    KeeperException e = expectThrows(KeeperException.class, () -> ZkCLI.main(args));
-      ZkCLI.main(args);
+    assertEquals(e.code(), KeeperException.Code.NONODE);
-      fail("Expected NoNodeException");
-    } catch (KeeperException.NoNodeException ex) {
-    }
   }
 
   @Test(expected = SolrException.class)

solr/core/src/test/org/apache/solr/cloud/ZkSolrClientTest.java

@@ -21,7 +21,6 @@ import java.util.Set;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
-import junit.framework.Assert;
 import org.apache.solr.SolrTestCaseJ4;
 import org.apache.solr.common.cloud.SolrZkClient;
 import org.apache.solr.common.cloud.ZkCmdExecutor;
@@ -117,7 +116,8 @@ public class ZkSolrClientTest extends SolrTestCaseJ4 {
       AbstractZkTestCase.tryCleanSolrZkNode(server.getZkHost());
       AbstractZkTestCase.makeSolrZkNode(server.getZkHost());
 
-      zkClient = new SolrZkClient(server.getZkAddress(), AbstractZkTestCase.TIMEOUT);
+      final SolrZkClient zkClientConLoss = new SolrZkClient(server.getZkAddress(), AbstractZkTestCase.TIMEOUT);
+      zkClient = zkClientConLoss;
       String shardsPath = "/collections/collection1/shards";
       zkClient.makePath(shardsPath, false, true);
 
@@ -129,12 +129,10 @@ public class ZkSolrClientTest extends SolrTestCaseJ4 {
       Thread.sleep(80);
 
 
-      try {
+      expectThrows(KeeperException.class,
-        zkClient.makePath("collections/collection2", false);
+          "Server should be down",
-        Assert.fail("Server should be down here");
+          () -> zkClientConLoss.makePath("collections/collection2", false)
-      } catch (KeeperException.ConnectionLossException e) {
+      );
-
-      }
 
       // bring server back up
       server = new ZkTestServer(zkDir, zkServerPort);
@@ -204,18 +202,14 @@ public class ZkSolrClientTest extends SolrTestCaseJ4 {
       
       ZkCmdExecutor zkCmdExecutor = new ZkCmdExecutor(timeout);
       final long start = System.nanoTime();
-      try {
+      expectThrows(KeeperException.SessionExpiredException.class, () -> {
-      zkCmdExecutor.retryOperation(() -> {
+        zkCmdExecutor.retryOperation(() -> {
-        if (System.nanoTime() - start > TimeUnit.NANOSECONDS.convert(timeout, TimeUnit.MILLISECONDS)) {
+          if (System.nanoTime() - start > TimeUnit.NANOSECONDS.convert(timeout, TimeUnit.MILLISECONDS)) {
-          throw new KeeperException.SessionExpiredException();
+            throw new KeeperException.SessionExpiredException();
-        }
+          }
-        throw new KeeperException.ConnectionLossException();
+          throw new KeeperException.ConnectionLossException();
+        });
       });
-      } catch(KeeperException.SessionExpiredException e) {
-        
-      } catch (Exception e) {
-        fail("Expected " + KeeperException.SessionExpiredException.class.getSimpleName() + " but got " + e.getClass().getSimpleName());
-      }
     } finally {
       if (server != null) {
         server.shutdown();
@@ -334,31 +328,22 @@ public class ZkSolrClientTest extends SolrTestCaseJ4 {
       zkClient.clean("/");
 
       // should not work
-      try {
+      KeeperException e =expectThrows(KeeperException.NoNodeException.class,
-        zkClient.makePath("/test/path/here", (byte[]) null, CreateMode.PERSISTENT, (Watcher) null, true, true, 1);
+          "We should not be able to create this path",
-        fail("We should not be able to create this path");
+          () -> zkClient.makePath("/test/path/here", (byte[]) null, CreateMode.PERSISTENT, (Watcher) null, true, true, 1));
-      } catch (Exception e) {
-
-      }
 
       zkClient.clean("/");
 
       ZkCmdExecutor zkCmdExecutor = new ZkCmdExecutor(30000);
-      try {
+      expectThrows(KeeperException.NoNodeException.class,
-        zkCmdExecutor.ensureExists("/collection/collection/leader", (byte[]) null, CreateMode.PERSISTENT, zkClient, 2);
+          "We should not be able to create this path",
-        fail("We should not be able to create this path");
+          () -> zkCmdExecutor.ensureExists("/collection/collection/leader", (byte[]) null, CreateMode.PERSISTENT, zkClient, 2));
-      } catch (Exception e) {
-
-      }
 
       zkClient.makePath("/collection", true);
 
-      try {
+      expectThrows(KeeperException.NoNodeException.class,
-        zkCmdExecutor.ensureExists("/collections/collection/leader", (byte[]) null, CreateMode.PERSISTENT, zkClient, 2);
+          "We should not be able to create this path",
-        fail("We should not be able to create this path");
+          () -> zkCmdExecutor.ensureExists("/collections/collection/leader", (byte[]) null, CreateMode.PERSISTENT, zkClient, 2));
-      } catch (Exception e) {
-
-      }
       zkClient.makePath("/collection/collection", true);
  
       byte[] bytes = new byte[10];