You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "wzhallright (via GitHub)" <gi...@apache.org> on 2024/02/05 11:03:37 UTC

[PR] HDDS-10277. Remove unnecessary fail() from tests [ozone]

wzhallright opened a new pull request, #6166:
URL: https://github.com/apache/ozone/pull/6166

   ## What changes were proposed in this pull request?
   Remove unnecessary fail() from tests
   
   ## What is the link to the Apache JIRA
   [HDDS-10277](https://issues.apache.org/jira/browse/HDDS-10277)
   ## CI
   https://github.com/wzhallright/ozone/actions/runs/7783088108
   
   
   


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-10277. Remove unnecessary fail() from tests [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #6166:
URL: https://github.com/apache/ozone/pull/6166


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-10277. Remove unnecessary fail() from tests [ozone]

Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #6166:
URL: https://github.com/apache/ozone/pull/6166#discussion_r1478996319


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestContainerStateMachineFailures.java:
##########
@@ -735,21 +725,16 @@ public void testContainerStateMachineSingleFailureRetry()
     OmKeyLocationInfo omKeyLocationInfo = locationInfoList.get(0);
 
     induceFollowerFailure(omKeyLocationInfo, 2);
+    key.flush();
+    key.write("ratis".getBytes(UTF_8));
+    key.flush();
+    key.close();

Review Comment:
   nit: should be in `finally` block or use `try-with-resources`.



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneClientRetriesOnExceptions.java:
##########
@@ -246,11 +245,7 @@ public void testMaxRetriesByOzoneClient() throws Exception {
     ioe = assertThrows(IOException.class, () -> key.flush());
     assertThat(ioe.getMessage()).contains("Stream is closed");
 
-    try {
-      key.close();
-    } catch (IOException e) {
-      fail("Expected should not be thrown");
-    }
+    key.close();

Review Comment:
   nit: use `try-with-resources`.



##########
hadoop-ozone/interface-storage/src/test/java/org/apache/hadoop/ozone/om/helpers/TestRepeatedOmKeyInfoCodec.java:
##########
@@ -87,47 +86,40 @@ private OmKeyInfo getKeyInfo(int chunkNum) {
   }
 
   @Test
-  public void test() throws InterruptedException {
+  void test() throws Exception {
     threadSafety();
     testWithoutPipeline(1);
     testWithoutPipeline(2);
     testCompatibility(1);
     testCompatibility(2);
   }
 
-  public void testWithoutPipeline(int chunkNum) {
+  public void testWithoutPipeline(int chunkNum) throws IOException {
     final Codec<RepeatedOmKeyInfo> codec = RepeatedOmKeyInfo.getCodec(true);
     OmKeyInfo originKey = getKeyInfo(chunkNum);
     RepeatedOmKeyInfo repeatedOmKeyInfo = new RepeatedOmKeyInfo(originKey);
-    try {
-      byte[] rawData = codec.toPersistedFormat(repeatedOmKeyInfo);
-      RepeatedOmKeyInfo key = codec.fromPersistedFormat(rawData);
-      System.out.println("Chunk number = " + chunkNum +
-          ", Serialized key size without pipeline = " + rawData.length);
-      assertNull(key.getOmKeyInfoList().get(0).getLatestVersionLocations()
-          .getLocationList().get(0).getPipeline());
-    } catch (IOException e) {
-      fail("Should success");
-    }
+
+    byte[] rawData = codec.toPersistedFormat(repeatedOmKeyInfo);
+    RepeatedOmKeyInfo key = codec.fromPersistedFormat(rawData);
+    System.out.println("Chunk number = " + chunkNum +

Review Comment:
   nit: this can be removed.



##########
hadoop-ozone/interface-storage/src/test/java/org/apache/hadoop/ozone/om/helpers/TestRepeatedOmKeyInfoCodec.java:
##########
@@ -87,47 +86,40 @@ private OmKeyInfo getKeyInfo(int chunkNum) {
   }
 
   @Test
-  public void test() throws InterruptedException {
+  void test() throws Exception {
     threadSafety();
     testWithoutPipeline(1);
     testWithoutPipeline(2);
     testCompatibility(1);
     testCompatibility(2);
   }
 
-  public void testWithoutPipeline(int chunkNum) {
+  public void testWithoutPipeline(int chunkNum) throws IOException {
     final Codec<RepeatedOmKeyInfo> codec = RepeatedOmKeyInfo.getCodec(true);
     OmKeyInfo originKey = getKeyInfo(chunkNum);
     RepeatedOmKeyInfo repeatedOmKeyInfo = new RepeatedOmKeyInfo(originKey);
-    try {
-      byte[] rawData = codec.toPersistedFormat(repeatedOmKeyInfo);
-      RepeatedOmKeyInfo key = codec.fromPersistedFormat(rawData);
-      System.out.println("Chunk number = " + chunkNum +
-          ", Serialized key size without pipeline = " + rawData.length);
-      assertNull(key.getOmKeyInfoList().get(0).getLatestVersionLocations()
-          .getLocationList().get(0).getPipeline());
-    } catch (IOException e) {
-      fail("Should success");
-    }
+
+    byte[] rawData = codec.toPersistedFormat(repeatedOmKeyInfo);
+    RepeatedOmKeyInfo key = codec.fromPersistedFormat(rawData);
+    System.out.println("Chunk number = " + chunkNum +
+        ", Serialized key size without pipeline = " + rawData.length);
+    assertNull(key.getOmKeyInfoList().get(0).getLatestVersionLocations()
+        .getLocationList().get(0).getPipeline());
   }
 
-  public void testCompatibility(int chunkNum) {
+  public void testCompatibility(int chunkNum) throws IOException {
     final Codec<RepeatedOmKeyInfo> codecWithoutPipeline
         = RepeatedOmKeyInfo.getCodec(true);
     final Codec<RepeatedOmKeyInfo> codecWithPipeline
         = RepeatedOmKeyInfo.getCodec(false);
     OmKeyInfo originKey = getKeyInfo(chunkNum);
     RepeatedOmKeyInfo repeatedOmKeyInfo = new RepeatedOmKeyInfo(originKey);
-    try {
-      byte[] rawData = codecWithPipeline.toPersistedFormat(repeatedOmKeyInfo);
-      RepeatedOmKeyInfo key = codecWithoutPipeline.fromPersistedFormat(rawData);
-      System.out.println("Chunk number = " + chunkNum +
-          ", Serialized key size with pipeline = " + rawData.length);
-      assertNotNull(key.getOmKeyInfoList().get(0).getLatestVersionLocations()
-          .getLocationList().get(0).getPipeline());
-    } catch (IOException e) {
-      fail("Should success");
-    }
+    byte[] rawData = codecWithPipeline.toPersistedFormat(repeatedOmKeyInfo);
+    RepeatedOmKeyInfo key = codecWithoutPipeline.fromPersistedFormat(rawData);
+    System.out.println("Chunk number = " + chunkNum +

Review Comment:
   same as previous.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-10277. Remove unnecessary fail() from tests [ozone]

Posted by "wzhallright (via GitHub)" <gi...@apache.org>.
wzhallright commented on PR #6166:
URL: https://github.com/apache/ozone/pull/6166#issuecomment-1928784729

   @hemantk-12 Thanks for your review! I have fixed the comments. PTAL.


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-10277. Remove unnecessary fail() from tests [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #6166:
URL: https://github.com/apache/ozone/pull/6166#issuecomment-1928989982

   Thanks @wzhallright for the patch, @hemantk-12 for the review.


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org