HDDS-10208. Simplify tests using assertThrows in hadoop-hdds

hadoop-hdds/client/src/test/java/org/apache/hadoop/hdds/scm/client/TestHddsClientUtils.java

@@ -228,12 +228,8 @@ public void testVerifyResourceName() {
     invalidNames.add(tooShort);
 
     for (String name : invalidNames) {
-      try {
-        HddsClientUtils.verifyResourceName(name);
-        fail("Did not reject invalid string [" + name + "] as a name");
-      } catch (IllegalArgumentException e) {
-        // throwing up on an invalid name. we're good
-      }
+      assertThrows(IllegalArgumentException.class, () -> HddsClientUtils.verifyResourceName(name),
+          "Did not reject invalid string [" + name + "] as a name");
     }
   }
 
@@ -257,12 +253,8 @@ public void testVerifyKeyName() {
 
 
     for (String name : invalidNames) {
-      try {
-        HddsClientUtils.verifyKeyName(name);
-        fail("Did not reject invalid string [" + name + "] as a name");
-      } catch (IllegalArgumentException e) {
-        // throwing up on an invalid name. it's working.
-      }
+      assertThrows(IllegalArgumentException.class, () -> HddsClientUtils.verifyKeyName(name),
+          "Did not reject invalid string [" + name + "] as a name");
     }
 
     List<String> validNames = new ArrayList<>();

hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestSchemaOneBackwardsCompatibility.java

@@ -72,7 +72,6 @@
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.junit.jupiter.api.Assertions.fail;
 import static org.mockito.Mockito.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -370,13 +369,8 @@ public void testReadDeletedBlockChunkInfo(String schemaVersion)
 
       for (Table.KeyValue<String, ChunkInfoList> chunkListKV: deletedBlocks) {
         preUpgradeBlocks.add(chunkListKV.getKey());
-        try {
-          chunkListKV.getValue();
-          fail("No exception thrown when trying to retrieve old " +
-              "deleted blocks values as chunk lists.");
-        } catch (IOException ex) {
-          // Exception thrown as expected.
-        }
+        assertThrows(IOException.class, () -> chunkListKV.getValue(),
+            "No exception thrown when trying to retrieve old deleted blocks values as chunk lists.");
       }
 
       assertEquals(TestDB.NUM_DELETED_BLOCKS, preUpgradeBlocks.size());

hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerPersistence.java

@@ -98,7 +98,6 @@
 import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.junit.jupiter.api.Assertions.fail;
 import static org.junit.jupiter.api.Assumptions.assumeFalse;
 import static org.junit.jupiter.api.Assumptions.assumeTrue;
 
@@ -249,12 +248,8 @@ public void testCreateDuplicateContainer(ContainerTestVersionInfo versionInfo)
     long testContainerID = getTestContainerID();
 
     Container container = addContainer(containerSet, testContainerID);
-    try {
-      containerSet.addContainer(container);
-      fail("Expected Exception not thrown.");
-    } catch (IOException ex) {
-      assertNotNull(ex);
-    }
+    IOException ex = assertThrows(IOException.class, () -> containerSet.addContainer(container));
+    assertNotNull(ex);
   }
 
   @ContainerTestVersionInfo.ContainerTest
@@ -800,26 +795,23 @@ public void testPutBlockWithInvalidBCSId(ContainerTestVersionInfo versionInfo)
     blockData.setBlockCommitSequenceId(4);
     blockManager.putBlock(container, blockData);
     BlockData readBlockData;
-    try {
+    StorageContainerException sce = assertThrows(StorageContainerException.class, () -> {
       blockID1.setBlockCommitSequenceId(5);
       // read with bcsId higher than container bcsId
       blockManager.
           getBlock(container, blockID1);
-      fail("Expected exception not thrown");
-    } catch (StorageContainerException sce) {
-      assertSame(UNKNOWN_BCSID, sce.getResult());
-    }
+    });
+    assertSame(UNKNOWN_BCSID, sce.getResult());
 
-    try {
+    sce = assertThrows(StorageContainerException.class, () -> {
       blockID1.setBlockCommitSequenceId(4);
       // read with bcsId lower than container bcsId but greater than committed
       // bcsId.
       blockManager.
           getBlock(container, blockID1);
-      fail("Expected exception not thrown");
-    } catch (StorageContainerException sce) {
-      assertSame(BCSID_MISMATCH, sce.getResult());
-    }
+    });
+    assertSame(BCSID_MISMATCH, sce.getResult());
+
     readBlockData = blockManager.
         getBlock(container, blockData.getBlockID());
     ChunkInfo readChunk =

hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java

@@ -393,15 +393,14 @@ public void testContainerImportExport(ContainerTestVersionInfo versionInfo)
           containerData.getBytesUsed());
 
       //Can't overwrite existing container
-      try {
+      KeyValueContainer finalContainer = container;
+      assertThrows(IOException.class, () -> {
         try (FileInputStream fis = new FileInputStream(folderToExport)) {
-          container.importContainerData(fis, packer);
+          finalContainer.importContainerData(fis, packer);
         }
-        fail("Container is imported twice. Previous files are overwritten");
-      } catch (IOException ex) {
-        //all good
-        assertTrue(container.getContainerFile().exists());
-      }
+      }, "Container is imported twice. Previous files are overwritten");
+      //all good
+      assertTrue(container.getContainerFile().exists());
 
       //Import failure should cleanup the container directory
       containerData =
@@ -415,18 +414,18 @@ public void testContainerImportExport(ContainerTestVersionInfo versionInfo)
       containerVolume = volumeChoosingPolicy.chooseVolume(
           StorageVolumeUtil.getHddsVolumesList(volumeSet.getVolumesList()), 1);
       container.populatePathFields(scmId, containerVolume);
-      try {
-        FileInputStream fis = new FileInputStream(folderToExport);
-        fis.close();
-        container.importContainerData(fis, packer);
-        fail("Container import should fail");
-      } catch (Exception ex) {
-        assertInstanceOf(IOException.class, ex);
-      } finally {
-        File directory =
-            new File(container.getContainerData().getContainerPath());
-        assertFalse(directory.exists());
-      }
+      KeyValueContainer finalContainer1 = container;
+      assertThrows(IOException.class, () -> {
+        try {
+          FileInputStream fis = new FileInputStream(folderToExport);
+          fis.close();
+          finalContainer1.importContainerData(fis, packer);
+        } finally {
+          File directory =
+              new File(finalContainer1.getContainerData().getContainerPath());
+          assertFalse(directory.exists());
+        }
+      });
     }
   }
 

hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestFilePerBlockStrategy.java

@@ -37,7 +37,7 @@
 import static org.apache.hadoop.ozone.container.ContainerTestHelper.setDataChecksum;
 import static org.apache.hadoop.ozone.container.common.ContainerTestUtils.WRITE_STAGE;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.fail;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 /**
  * Test for FilePerBlockStrategy.
@@ -48,7 +48,7 @@ public class TestFilePerBlockStrategy extends CommonChunkManagerTestCases {
   public void testDeletePartialChunkWithOffsetUnsupportedRequest() {
     // GIVEN
     ChunkManager chunkManager = createTestSubject();
-    try {
+    StorageContainerException e = assertThrows(StorageContainerException.class, () -> {
       KeyValueContainer container = getKeyValueContainer();
       BlockID blockID = getBlockID();
       chunkManager.writeChunk(container, blockID,
@@ -58,12 +58,8 @@ public void testDeletePartialChunkWithOffsetUnsupportedRequest() {
 
       // WHEN
       chunkManager.deleteChunk(container, blockID, chunkInfo);
-
-      // THEN
-      fail("testDeleteChunkUnsupportedRequest");
-    } catch (StorageContainerException ex) {
-      assertEquals(ContainerProtos.Result.UNSUPPORTED_REQUEST, ex.getResult());
-    }
+    });
+    assertEquals(ContainerProtos.Result.UNSUPPORTED_REQUEST, e.getResult());
   }
 
   /**

hadoop-hdds/erasurecode/src/test/java/org/apache/ozone/erasurecode/rawcoder/TestRawCoderBase.java

@@ -93,13 +93,9 @@ protected void testCoding(boolean usingDirectBuffer) {
   protected void testCodingWithBadInput(boolean usingDirectBuffer) {
     this.usingDirectBuffer = usingDirectBuffer;
     prepareCoders(true);
-
-    try {
-      performTestCoding(baseChunkSize, false, true, false);
-      fail("Encoding test with bad input should fail");
-    } catch (Exception e) {
-      // Expected
-    }
+    assertThrows(Exception.class,
+        () -> performTestCoding(baseChunkSize, false, true, false),
+        "Encoding test with bad input should fail");
   }
 
   /**
@@ -109,13 +105,9 @@ protected void testCodingWithBadInput(boolean usingDirectBuffer) {
   protected void testCodingWithBadOutput(boolean usingDirectBuffer) {
     this.usingDirectBuffer = usingDirectBuffer;
     prepareCoders(true);
-
-    try {
-      performTestCoding(baseChunkSize, false, false, true);
-      fail("Decoding test with bad output should fail");
-    } catch (Exception e) {
-      // Expected
-    }
+    assertThrows(Exception.class,
+        () -> performTestCoding(baseChunkSize, false, false, true),
+        "Decoding test with bad output should fail");
   }
 
   /**
@@ -143,19 +135,8 @@ void testAfterRelease() throws Exception {
 
   @Test
   public void testCodingWithErasingTooMany() {
-    try {
-      testCoding(true);
-      fail("Decoding test erasing too many should fail");
-    } catch (Exception e) {
-      // Expected
-    }
-
-    try {
-      testCoding(false);
-      fail("Decoding test erasing too many should fail");
-    } catch (Exception e) {
-      // Expected
-    }
+    assertThrows(Exception.class, () -> testCoding(true), "Decoding test erasing too many should fail");
+    assertThrows(Exception.class, () -> testCoding(false), "Decoding test erasing too many should fail");
   }
 
   @Test

hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/conf/TestHddsConfServlet.java

@@ -19,8 +19,8 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.junit.jupiter.api.Assertions.fail;
 import static org.mockito.Mockito.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
@@ -170,12 +170,8 @@ public void testWriteXml() throws Exception {
   @Test
   public void testBadFormat() throws Exception {
     StringWriter sw = new StringWriter();
-    try {
-      HddsConfServlet.writeResponse(getTestConf(), sw, "not a format", null);
-      fail("writeResponse with bad format didn't throw!");
-    } catch (HddsConfServlet.BadFormatException bfe) {
-      // expected
-    }
+    assertThrows(HddsConfServlet.BadFormatException.class,
+        () -> HddsConfServlet.writeResponse(getTestConf(), sw, "not a format", null));
     assertEquals("", sw.toString());
   }
 

hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/authority/TestDefaultCAServer.java

@@ -124,15 +124,11 @@ public void testMissingCertificate() {
         ((DefaultCAServer) testCA).processVerificationStatus(
             DefaultCAServer.VerificationStatus.MISSING_CERTIFICATE,
             CAType.ROOT);
-    try {
-
-      caInitializer.accept(securityConfig);
-      fail("code should not reach here, exception should have been thrown.");
-    } catch (IllegalStateException e) {
-      // This also is a runtime exception. Hence not caught by junit expected
-      // exception.
-      assertThat(e.toString()).contains("Missing Root Certs");
-    }
+    IllegalStateException e =
+        assertThrows(IllegalStateException.class, () -> caInitializer.accept(securityConfig));
+    // This also is a runtime exception. Hence not caught by junit expected
+    // exception.
+    assertThat(e.toString()).contains("Missing Root Certs");
   }
 
   @Test
@@ -145,15 +141,11 @@ public void testMissingKey() {
     Consumer<SecurityConfig> caInitializer =
         ((DefaultCAServer) testCA).processVerificationStatus(
             DefaultCAServer.VerificationStatus.MISSING_KEYS, CAType.ROOT);
-    try {
-
-      caInitializer.accept(securityConfig);
-      fail("code should not reach here, exception should have been thrown.");
-    } catch (IllegalStateException e) {
-      // This also is a runtime exception. Hence not caught by junit expected
-      // exception.
-      assertThat(e.toString()).contains("Missing Keys");
-    }
+    IllegalStateException e =
+        assertThrows(IllegalStateException.class, () -> caInitializer.accept(securityConfig));
+    // This also is a runtime exception. Hence not caught by junit expected
+    // exception.
+    assertThat(e.toString()).contains("Missing Keys");
   }
 
   /**

hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/utils/TestCertificateSignRequest.java

@@ -54,6 +54,7 @@
 import static org.apache.hadoop.hdds.security.x509.certificate.utils.CertificateSignRequest.getPkcs9Extensions;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 
@@ -206,29 +207,23 @@ public void testGenerateCSRWithInvalidParams() throws NoSuchProviderException,
     }
 
     // Now try with blank/null Subject.
-    try {
+    assertThrows(IllegalArgumentException.class, () -> {
       builder.setSubject(null);
       builder.build();
-      fail("Null/Blank Subject should have thrown.");
-    } catch (IllegalArgumentException e) {
-      builder.setSubject(subject);
-    }
+    });
+    builder.setSubject(subject);
 
-    try {
+    assertThrows(IllegalArgumentException.class, () -> {
       builder.setSubject("");
       builder.build();
-      fail("Null/Blank Subject should have thrown.");
-    } catch (IllegalArgumentException e) {
-      builder.setSubject(subject);
-    }
+    });
+    builder.setSubject(subject);
 
     // Now try with invalid IP address
-    try {
+    assertThrows(IllegalArgumentException.class, () -> {
       builder.addIpAddress("255.255.255.*");
       builder.build();
-      fail("Invalid ip address");
-    } catch (IllegalArgumentException e) {
-    }
+    });
 
     PKCS10CertificationRequest csr = builder.build();