adoroszlai commented on code in PR #5844:
URL: https://github.com/apache/ozone/pull/5844#discussion_r1433902937


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java:
##########
@@ -350,8 +351,8 @@ public void testCreateDoesNotAddParentDirKeys() throws 
Exception {
     // Creating a child should not add parent keys to the bucket
     try {
       getKey(parent, true);
-    } catch (IOException ex) {
-      assertKeyNotFoundException(ex);
+    } catch (OMException ome) {
+      Assertions.assertEquals(KEY_NOT_FOUND, ome.getResult());

Review Comment:
   `TestRootedOzoneFileSystem` is yet to be migrated (HDDS-9921).  We can use 
existing static import until then:
   
   ```suggestion
         assertEquals(KEY_NOT_FOUND, ome.getResult());
   ```



##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerDataYaml.java:
##########
@@ -188,22 +189,19 @@ public void testCreateContainerFileWithoutReplicaIndex(
     cleanup();
   }
 
-
   @ContainerLayoutTestInfo.ContainerTest
-  public void testIncorrectContainerFile(ContainerLayoutVersion layout)
-      throws IOException {
+  public void testIncorrectContainerFile(ContainerLayoutVersion layout) {
     setLayoutVersion(layout);
-    try {
-      String containerFile = "incorrect.container";
-      //Get file from resources folder
+    String containerFile = "incorrect.container";
+
+    IllegalArgumentException exception = 
assertThrows(IllegalArgumentException.class, () -> {
+      // Get file from resource folder
       ClassLoader classLoader = getClass().getClassLoader();
       File file = new File(classLoader.getResource(containerFile).getFile());

Review Comment:
   nit: these lines can be moved outside of `assertThrows`, and then it can use 
a single statement instead of a block.



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java:
##########
@@ -76,6 +76,7 @@
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.jupiter.api.Assertions;

Review Comment:
   ```suggestion
   ```



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestSecureOzoneCluster.java:
##########
@@ -838,21 +840,12 @@ public void testGetSetRevokeS3Secret() throws Exception {
           OmTransportFactory.create(conf, ugiNonAdmin, null),
           RandomStringUtils.randomAscii(5));
 
-      try {
-        omClientNonAdmin.getS3Secret("HADOOP/JOHN");
-        // Expected to fail because current ugi isn't an admin
-        fail("non-admin getS3Secret didn't fail as intended");
-      } catch (IOException ex) {
-        GenericTestUtils.assertExceptionContains("USER_MISMATCH", ex);
-      }
-
-      try {
-        omClientNonAdmin.revokeS3Secret("HADOOP/DOE");
-        // Expected to fail because current ugi isn't an admin
-        fail("non-admin revokeS3Secret didn't fail as intended");
-      } catch (IOException ex) {
-        GenericTestUtils.assertExceptionContains("USER_MISMATCH", ex);
-      }
+      OMException omException = assertThrows(OMException.class,
+          () -> omClientNonAdmin.getS3Secret("HADOOP/JOHN"));
+      assertSame(USER_MISMATCH, omException.getResult());
+      omException = assertThrows(OMException.class,
+          () -> omClientNonAdmin.getS3Secret("HADOOP/DOE"));

Review Comment:
   This call was different in the original:
   
   ```suggestion
             () -> omClientNonAdmin.revokeS3Secret("HADOOP/DOE"));
   ```



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java:
##########
@@ -421,10 +422,11 @@ public void testDeleteCreatesFakeParentDir() throws 
Exception {
     // Creating a child should not add parent keys to the bucket
     try {
       getKey(parent, true);
-    } catch (IOException ex) {
-      assertKeyNotFoundException(ex);
+    } catch (OMException ome) {
+      Assertions.assertEquals(KEY_NOT_FOUND, ome.getResult());

Review Comment:
   ```suggestion
         assertEquals(KEY_NOT_FOUND, ome.getResult());
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to