swamirishi commented on code in PR #9571:
URL: https://github.com/apache/ozone/pull/9571#discussion_r2652561387


##########
hadoop-hdds/managed-rocksdb/src/test/java/org/apache/hadoop/hdds/utils/db/managed/TestManagedDirectSlice.java:
##########
@@ -20,30 +20,58 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
 import java.nio.ByteBuffer;
-import java.util.Arrays;
-import org.apache.commons.lang3.RandomUtils;
-import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.CsvSource;
+import java.util.Random;
+import org.apache.hadoop.hdds.utils.db.CodecBuffer;
+import org.junit.jupiter.api.Test;
 
 /**
  * Tests for ManagedDirectSlice.
  */
 public class TestManagedDirectSlice {
-
   static {
     ManagedRocksObjectUtils.loadRocksDBLibrary();
   }
 
-  @ParameterizedTest
-  @CsvSource({"0, 1024", "1024, 1024", "512, 1024", "0, 100", "10, 512", "0, 
0"})
-  public void testManagedDirectSliceWithOffset(int offset, int 
numberOfBytesWritten) {
-    ByteBuffer byteBuffer = ByteBuffer.allocateDirect(1024);
-    byte[] randomBytes = RandomUtils.secure().nextBytes(numberOfBytesWritten);
-    byteBuffer.put(randomBytes);
-    byteBuffer.flip();
-    byteBuffer.position(offset);
-    try (ManagedDirectSlice directSlice = new ManagedDirectSlice(byteBuffer);
-         ManagedSlice slice = new ManagedSlice(Arrays.copyOfRange(randomBytes, 
offset, numberOfBytesWritten))) {
+  static final Random RANDOM = new Random();
+
+  @Test
+  public void testManagedDirectSlice() {
+    // test all values <= 16
+    for (int size = 0; size <= 16; size++) {
+      testManagedDirectSlice(size);
+    }
+
+    // test power of 2
+    for (int i = 0; i <= 10; i++) {
+      final int size = 32 << i;
+      testManagedDirectSlice(size - 1);
+      testManagedDirectSlice(size);
+      testManagedDirectSlice(size + 1);
+    }
+
+    // test random
+    for (int i = 0; i < 10; i++) {
+      final int size = RANDOM.nextInt(1 << 20);
+      testManagedDirectSlice(size);
+    }
+  }
+
+  static void testManagedDirectSlice(int size) {
+    try {
+      runTestManagedDirectSlice(size);
+    } catch (Throwable e) {
+      System.out.printf("Failed for size %d%n", size);
+      throw e;
+    }
+  }
+
+  static void runTestManagedDirectSlice(int size) {
+    final byte[] bytes = new byte[size];
+    RANDOM.nextBytes(bytes);
+    try (CodecBuffer buffer = 
CodecBuffer.allocateDirect(size).put(ByteBuffer.wrap(bytes));
+         ManagedDirectSlice directSlice = new 
ManagedDirectSlice(buffer.asReadOnlyByteBuffer());

Review Comment:
   We should also test a byte buffer with a non zero position to test the 
[slice initiailization in the 
constructor](https://github.com/apache/ozone/blob/329719836d515a25afbfe4b05328476d456387f8/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedDirectSlice.java#L38).
 Consider the case if someone removes the slice logic then things would break. 
If there is some logic change we should ensure we have test case for that.
   BTW I have raised a patch on rocksdb which gives us a way to avoid the 
slicing and with this constructor we can directly pass the position and 
remaining to the JNI layer in a single call.
   https://github.com/facebook/rocksdb/pull/14208



-- 
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