m1a2st commented on code in PR #23391:
URL: https://github.com/apache/kafka/pull/23391#discussion_r3956439668
##########
storage/src/test/java/org/apache/kafka/storage/internals/log/OffsetMapTest.java:
##########
@@ -18,30 +18,45 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.ValueSource;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
import java.nio.ByteBuffer;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
import java.util.stream.IntStream;
+import java.util.stream.Stream;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
-public class OffsetMapTest {
+class OffsetMapTest {
Review Comment:
Why we need this change?
##########
storage/src/test/java/org/apache/kafka/storage/internals/log/OffsetMapTest.java:
##########
@@ -18,30 +18,45 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.ValueSource;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
import java.nio.ByteBuffer;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
import java.util.stream.IntStream;
+import java.util.stream.Stream;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
-public class OffsetMapTest {
+class OffsetMapTest {
private static final int MEMORY_SIZE = 4096;
+ static Stream<String> hashAlgorithms() {
+ return Stream.of("MD5", "SHA-1", "SHA-256", "SHA-512");
Review Comment:
`SkimpyOffsetMap` documents support for MD2, MD5, SHA-1, SHA-256, SHA-384,
and SHA-512, but here only covers four of them. Should we add MD2 and SHA-384
as well?
##########
storage/src/test/java/org/apache/kafka/storage/internals/log/OffsetMapTest.java:
##########
@@ -18,30 +18,45 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.ValueSource;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
import java.nio.ByteBuffer;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
import java.util.stream.IntStream;
+import java.util.stream.Stream;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
-public class OffsetMapTest {
+class OffsetMapTest {
private static final int MEMORY_SIZE = 4096;
+ static Stream<String> hashAlgorithms() {
+ return Stream.of("MD5", "SHA-1", "SHA-256", "SHA-512");
+ }
+
+ static Stream<Arguments> algorithmsAndItems() {
+ return hashAlgorithms().flatMap(algorithm ->
+ IntStream.of(10, 100, 1000, 5000).mapToObj(items ->
Arguments.of(algorithm, items)));
+ }
+
@ParameterizedTest
- @ValueSource(ints = {10, 100, 1000, 5000})
- public void testBasicValidation(int items) throws Exception {
- SkimpyOffsetMap map = new SkimpyOffsetMap(items * 48);
+ @MethodSource("algorithmsAndItems")
+ void testBasicValidation(String algorithm, int items) throws Exception {
+ int bytesPerEntry =
MessageDigest.getInstance(algorithm).getDigestLength() + 8;
+ SkimpyOffsetMap map = new SkimpyOffsetMap(items * bytesPerEntry * 2,
algorithm);
IntStream.range(0, items).forEach(i -> assertDoesNotThrow(() ->
map.put(key(i), i)));
for (int i = 0; i < items; i++) {
assertEquals(i, map.get(key(i)));
}
}
@Test
- public void testClear() throws Exception {
+ void testClear() throws Exception {
Review Comment:
ditto
##########
storage/src/test/java/org/apache/kafka/storage/internals/log/OffsetMapTest.java:
##########
@@ -89,9 +105,9 @@ public void testLatestOffset() throws Exception {
}
assertEquals(i - 1, map.latestOffset());
}
-
+
@Test
- public void testUtilization() throws Exception {
+ void testUtilization() throws Exception {
Review Comment:
ditto
##########
storage/src/test/java/org/apache/kafka/storage/internals/log/OffsetMapTest.java:
##########
@@ -80,7 +96,7 @@ public void testUpdateLatestOffset() throws Exception {
}
@Test
- public void testLatestOffset() throws Exception {
+ void testLatestOffset() throws Exception {
Review Comment:
ditto
##########
storage/src/test/java/org/apache/kafka/storage/internals/log/OffsetMapTest.java:
##########
@@ -66,7 +82,7 @@ public void testGetWhenFull() throws Exception {
}
@Test
- public void testUpdateLatestOffset() throws Exception {
Review Comment:
ditto
--
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]
