YARN-5560. Clean up bad exception catching practices in TestYarnClient. Contributed by Sean Po

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestYarnClient.java

@@ -69,8 +69,6 @@
 import org.apache.hadoop.yarn.api.protocolrecords.GetContainersResponse;
 import org.apache.hadoop.yarn.api.protocolrecords.GetLabelsToNodesRequest;
 import org.apache.hadoop.yarn.api.protocolrecords.GetLabelsToNodesResponse;
-import org.apache.hadoop.yarn.api.protocolrecords.GetNewReservationRequest;
-import org.apache.hadoop.yarn.api.protocolrecords.GetNewReservationResponse;
 import org.apache.hadoop.yarn.api.protocolrecords.GetNodesToLabelsRequest;
 import org.apache.hadoop.yarn.api.protocolrecords.GetNodesToLabelsResponse;
 import org.apache.hadoop.yarn.api.protocolrecords.KillApplicationRequest;
@@ -158,7 +156,7 @@ public void testClientStop() {
 
   @SuppressWarnings("deprecation")
   @Test (timeout = 30000)
-  public void testSubmitApplication() {
+  public void testSubmitApplication() throws Exception {
     Configuration conf = new Configuration();
     conf.setLong(YarnConfiguration.YARN_CLIENT_APP_SUBMISSION_POLL_INTERVAL_MS,
         100); // speed up tests
@@ -184,8 +182,6 @@ public void testSubmitApplication() {
       Assert.assertTrue(e instanceof ApplicationIdNotProvidedException);
       Assert.assertTrue(e.getMessage().contains(
           "ApplicationId is not provided in ApplicationSubmissionContext"));
-    } catch (IOException e) {
-      Assert.fail("IOException is not expected.");
     }
 
     // Submit the application with applicationId provided
@@ -197,13 +193,7 @@ public void testSubmitApplication() {
           System.currentTimeMillis(), i);
       when(context.getApplicationId()).thenReturn(applicationId);
       ((MockYarnClient) client).setYarnApplicationState(exitStates[i]);
-      try {
       client.submitApplication(context);
-      } catch (YarnException e) {
-        Assert.fail("Exception is not expected.");
-      } catch (IOException e) {
-        Assert.fail("Exception is not expected.");
-      }
       verify(((MockYarnClient) client).mockReport,times(4 * i + 4))
           .getYarnApplicationState();
     }
@@ -584,11 +574,10 @@ public void start() {
 
         historyClient = mock(AHSClient.class);
 
-      } catch (YarnException e) {
+      } catch (Exception e) {
-        Assert.fail("Exception is not expected.");
+        Assert.fail("Unexpected exception caught: " + e);
-      } catch (IOException e) {
-        Assert.fail("Exception is not expected.");
       }
+
       when(mockResponse.getApplicationReport()).thenReturn(mockReport);
     }
 
@@ -993,36 +982,20 @@ private ApplicationId createApp(YarnClient rmClient, boolean unmanaged)
     return appId;
   }
 
-  private GetNewReservationResponse getNewReservation(YarnClient rmClient) {
-    GetNewReservationRequest newReservationRequest = GetNewReservationRequest
-        .newInstance();
-    GetNewReservationResponse getNewReservationResponse = null;
-    try {
-      getNewReservationResponse = rmClient.createReservation();
-    } catch (Exception e) {
-      Assert.fail(e.getMessage());
-    }
-    return getNewReservationResponse;
-  }
-
   private void waitTillAccepted(YarnClient rmClient, ApplicationId appId,
       boolean unmanagedApplication)
     throws Exception {
-    try {
     long start = System.currentTimeMillis();
     ApplicationReport report = rmClient.getApplicationReport(appId);
     while (YarnApplicationState.ACCEPTED != report.getYarnApplicationState()) {
       if (System.currentTimeMillis() - start > 20 * 1000) {
-          throw new Exception("App '" + appId + 
+        throw new Exception(
-            "' time out, failed to reach ACCEPTED state");
+            "App '" + appId + "' time out, failed to reach ACCEPTED state");
       }
       Thread.sleep(200);
       report = rmClient.getApplicationReport(appId);
     }
     Assert.assertEquals(unmanagedApplication, report.isUnmanagedApp());
-    } catch (Exception ex) {
-      throw new Exception(ex);
-    }
   }
 
   @Test
@@ -1071,13 +1044,11 @@ TimelineClient createTimelineClient() throws IOException, YarnException {
     });
 
     client.init(conf);
-    try {
+    conf.setBoolean(YarnConfiguration.TIMELINE_SERVICE_CLIENT_BEST_EFFORT,
-      conf.setBoolean(YarnConfiguration.TIMELINE_SERVICE_CLIENT_BEST_EFFORT, true);
+        true);
     client.serviceInit(conf);
     client.getTimelineDelegationToken();
-    } catch (Exception e) {
+
-      Assert.fail("Should not have thrown an exception");
-    }
     try {
       conf.setBoolean(YarnConfiguration.TIMELINE_SERVICE_CLIENT_BEST_EFFORT, false);
       client.serviceInit(conf);
@@ -1229,16 +1200,13 @@ private YarnClient setupYarnClient(MiniYARNCluster cluster) {
   }
 
   private ReservationSubmissionRequest submitReservationTestHelper(
-      YarnClient client, long arrival, long deadline, long duration) {
+      YarnClient client, long arrival, long deadline, long duration)
-    ReservationId reservationID = getNewReservation(client).getReservationId();
+      throws IOException, YarnException {
+    ReservationId reservationID = client.createReservation().getReservationId();
     ReservationSubmissionRequest sRequest = createSimpleReservationRequest(
         reservationID, 4, arrival, deadline, duration);
-    ReservationSubmissionResponse sResponse = null;
+    ReservationSubmissionResponse sResponse =
-    try {
+        client.submitReservation(sRequest);
-      sResponse = client.submitReservation(sRequest);
-    } catch (Exception e) {
-      Assert.fail(e.getMessage());
-    }
     Assert.assertNotNull(sResponse);
     Assert.assertNotNull(reservationID);
     System.out.println("Submit reservation response: " + reservationID);
@@ -1260,11 +1228,7 @@ public void testCreateReservation() throws Exception {
 
       // Submit the reservation again with the same request and make sure it
       // passes.
-      try {
       client.submitReservation(sRequest);
-      } catch (Exception e) {
-        Assert.fail(e.getMessage());
-      }
 
       // Submit the reservation with the same reservation id but different
       // reservation definition, and ensure YarnException is thrown.
@@ -1314,12 +1278,7 @@ public void testUpdateReservation() throws Exception {
       rDef.setDeadline(deadline);
       ReservationUpdateRequest uRequest =
           ReservationUpdateRequest.newInstance(rDef, reservationID);
-      ReservationUpdateResponse uResponse = null;
+      ReservationUpdateResponse uResponse = client.updateReservation(uRequest);
-      try {
-        uResponse = client.updateReservation(uRequest);
-      } catch (Exception e) {
-        Assert.fail(e.getMessage());
-      }
       Assert.assertNotNull(uResponse);
       System.out.println("Update reservation response: " + uResponse);
     } finally {
@@ -1344,15 +1303,10 @@ public void testListReservationsByReservationId() throws Exception{
           submitReservationTestHelper(client, arrival, deadline, duration);
 
       ReservationId reservationID = sRequest.getReservationId();
-      ReservationListResponse response = null;
       ReservationListRequest request = ReservationListRequest.newInstance(
           ReservationSystemTestUtil.reservationQ, reservationID.toString(), -1,
           -1, false);
-      try {
+      ReservationListResponse response = client.listReservations(request);
-        response = client.listReservations(request);
-      } catch (Exception e) {
-        Assert.fail(e.getMessage());
-      }
       Assert.assertNotNull(response);
       Assert.assertEquals(1, response.getReservationAllocationState().size());
       Assert.assertEquals(response.getReservationAllocationState().get(0)
@@ -1388,12 +1342,7 @@ public void testListReservationsByTimeInterval() throws Exception {
           ReservationSystemTestUtil.reservationQ, "", arrival + duration / 2,
           arrival + duration / 2, true);
 
-      ReservationListResponse response = null;
+      ReservationListResponse response = client.listReservations(request);
-      try {
-        response = client.listReservations(request);
-      } catch (Exception e) {
-        Assert.fail(e.getMessage());
-      }
       Assert.assertNotNull(response);
       Assert.assertEquals(1, response.getReservationAllocationState().size());
       Assert.assertEquals(response.getReservationAllocationState().get(0)
@@ -1402,12 +1351,7 @@ public void testListReservationsByTimeInterval() throws Exception {
       request = ReservationListRequest.newInstance(
           ReservationSystemTestUtil.reservationQ, "", 1, Long.MAX_VALUE, true);
 
-      response = null;
-      try {
       response = client.listReservations(request);
-      } catch (Exception e) {
-        Assert.fail(e.getMessage());
-      }
       Assert.assertNotNull(response);
       Assert.assertEquals(1, response.getReservationAllocationState().size());
       Assert.assertEquals(response.getReservationAllocationState().get(0)
@@ -1449,12 +1393,7 @@ public void testListReservationsByInvalidTimeInterval() throws Exception {
       ReservationListRequest request = ReservationListRequest
           .newInstance(ReservationSystemTestUtil.reservationQ, "", 1, -1, true);
 
-      ReservationListResponse response = null;
+      ReservationListResponse response = client.listReservations(request);
-      try {
-        response = client.listReservations(request);
-      } catch (Exception e) {
-        Assert.fail(e.getMessage());
-      }
       Assert.assertNotNull(response);
       Assert.assertEquals(1, response.getReservationAllocationState().size());
       Assert.assertEquals(response.getReservationAllocationState().get(0)
@@ -1464,12 +1403,7 @@ public void testListReservationsByInvalidTimeInterval() throws Exception {
       request = ReservationListRequest.newInstance(
           ReservationSystemTestUtil.reservationQ, "", 1, -10, true);
 
-      response = null;
-      try {
       response = client.listReservations(request);
-      } catch (Exception e) {
-        Assert.fail(e.getMessage());
-      }
       Assert.assertNotNull(response);
       Assert.assertEquals(1, response.getReservationAllocationState().size());
       Assert.assertEquals(response.getReservationAllocationState().get(0)
@@ -1501,12 +1435,7 @@ public void testListReservationsByTimeIntervalContainingNoReservations()
           ReservationSystemTestUtil.reservationQ, "", Long.MAX_VALUE, -1,
           false);
 
-      ReservationListResponse response = null;
+      ReservationListResponse response = client.listReservations(request);
-      try {
-        response = client.listReservations(request);
-      } catch (Exception e) {
-        Assert.fail(e.getMessage());
-      }
 
       // Ensure all reservations are filtered out.
       Assert.assertNotNull(response);
@@ -1521,12 +1450,7 @@ public void testListReservationsByTimeIntervalContainingNoReservations()
           ReservationSystemTestUtil.reservationQ, "", deadline + duration,
           deadline + 2 * duration, false);
 
-      response = null;
-      try {
       response = client.listReservations(request);
-      } catch (Exception e) {
-        Assert.fail(e.getMessage());
-      }
 
       // Ensure all reservations are filtered out.
       Assert.assertNotNull(response);
@@ -1539,12 +1463,7 @@ public void testListReservationsByTimeIntervalContainingNoReservations()
           ReservationSystemTestUtil.reservationQ, "", 0, arrival - duration,
           false);
 
-      response = null;
-      try {
       response = client.listReservations(request);
-      } catch (Exception e) {
-        Assert.fail(e.getMessage());
-      }
 
       // Ensure all reservations are filtered out.
       Assert.assertNotNull(response);
@@ -1554,12 +1473,7 @@ public void testListReservationsByTimeIntervalContainingNoReservations()
       request = ReservationListRequest
           .newInstance(ReservationSystemTestUtil.reservationQ, "", 0, 1, false);
 
-      response = null;
-      try {
       response = client.listReservations(request);
-      } catch (Exception e) {
-        Assert.fail(e.getMessage());
-      }
 
       // Ensure all reservations are filtered out.
       Assert.assertNotNull(response);
@@ -1590,12 +1504,7 @@ public void testReservationDelete() throws Exception {
       // Delete the reservation
       ReservationDeleteRequest dRequest =
           ReservationDeleteRequest.newInstance(reservationID);
-      ReservationDeleteResponse dResponse = null;
+      ReservationDeleteResponse dResponse = client.deleteReservation(dRequest);
-      try {
-        dResponse = client.deleteReservation(dRequest);
-      } catch (Exception e) {
-        Assert.fail(e.getMessage());
-      }
       Assert.assertNotNull(dResponse);
       System.out.println("Delete reservation response: " + dResponse);
 
@@ -1604,12 +1513,7 @@ public void testReservationDelete() throws Exception {
           ReservationSystemTestUtil.reservationQ, reservationID.toString(), -1,
           -1, false);
 
-      ReservationListResponse response = null;
+      ReservationListResponse response =  client.listReservations(request);
-      try {
-        response = client.listReservations(request);
-      } catch (Exception e) {
-        Assert.fail(e.getMessage());
-      }
       Assert.assertNotNull(response);
       Assert.assertEquals(0, response.getReservationAllocationState().size());
     } finally {