cameronlee314 commented on a change in pull request #1404:
URL: https://github.com/apache/samza/pull/1404#discussion_r461727748
##########
File path:
samza-kv-inmemory/src/test/java/org/apache/samza/storage/kv/inmemory/TestInMemoryKeyValueStore.java
##########
@@ -19,66 +19,501 @@
package org.apache.samza.storage.kv.inmemory;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
import com.google.common.collect.Iterators;
import com.google.common.primitives.Ints;
-import org.apache.samza.metrics.MetricsRegistryMap;
+import org.apache.samza.SamzaException;
+import org.apache.samza.metrics.Counter;
import org.apache.samza.storage.kv.Entry;
import org.apache.samza.storage.kv.KeyValueIterator;
import org.apache.samza.storage.kv.KeyValueSnapshot;
import org.apache.samza.storage.kv.KeyValueStoreMetrics;
+import org.junit.Before;
import org.junit.Test;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
-import java.io.ByteArrayOutputStream;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
-import java.util.concurrent.ThreadLocalRandom;
-import java.util.stream.Collectors;
-import java.util.stream.IntStream;
-
+import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.verifyZeroInteractions;
+import static org.mockito.Mockito.when;
+
public class TestInMemoryKeyValueStore {
+ private static final String DEFAULT_KEY_PREFIX = "key_prefix";
+ private static final String OTHER_KEY_PREFIX = "other_key_prefix";
+ /**
+ * Keep the lengths of the values longer so that metrics validations for key
and value sizes don't collide.
+ */
+ private static final String DEFAULT_VALUE_PREFIX =
"value_prefix_value_prefix";
+ private static final String OTHER_VALUE_PREFIX =
"other_value_prefix_value_prefix";
+
+ @Mock
+ private KeyValueStoreMetrics keyValueStoreMetrics;
+ @Mock
+ private Counter getsCounter;
+ @Mock
+ private Counter bytesReadCounter;
+ @Mock
+ private Counter putsCounter;
+ @Mock
+ private Counter bytesWrittenCounter;
+ @Mock
+ private Counter deletesCounter;
+
+ private InMemoryKeyValueStore inMemoryKeyValueStore;
+
+ @Before
+ public void setup() {
+ MockitoAnnotations.initMocks(this);
+ when(this.keyValueStoreMetrics.gets()).thenReturn(this.getsCounter);
+
when(this.keyValueStoreMetrics.bytesRead()).thenReturn(this.bytesReadCounter);
+ when(this.keyValueStoreMetrics.puts()).thenReturn(this.putsCounter);
+
when(this.keyValueStoreMetrics.bytesWritten()).thenReturn(this.bytesWrittenCounter);
+ when(this.keyValueStoreMetrics.deletes()).thenReturn(this.deletesCounter);
+ this.inMemoryKeyValueStore = new
InMemoryKeyValueStore(this.keyValueStoreMetrics);
+ }
+
+ @Test
+ public void testGet() {
+ this.inMemoryKeyValueStore.put(key(0), value(0));
+ this.inMemoryKeyValueStore.put(key(OTHER_KEY_PREFIX, 1),
value(OTHER_VALUE_PREFIX, 1));
+
+ assertArrayEquals(value(0), this.inMemoryKeyValueStore.get(key(0)));
+ assertArrayEquals(value(OTHER_VALUE_PREFIX, 1),
this.inMemoryKeyValueStore.get(key(OTHER_KEY_PREFIX, 1)));
+ verify(this.getsCounter, times(2)).inc();
+ verify(this.bytesReadCounter).inc(value(0).length);
+ verify(this.bytesReadCounter).inc(value(OTHER_VALUE_PREFIX, 1).length);
+ }
+
+ @Test
+ public void testGetEmpty() {
+ assertNull(this.inMemoryKeyValueStore.get(key(0)));
+ verify(this.getsCounter).inc();
+ verifyZeroInteractions(this.bytesReadCounter);
+ }
+
+ @Test
+ public void testGetAfterDelete() {
+ this.inMemoryKeyValueStore.put(key(0), value(0));
+ this.inMemoryKeyValueStore.delete(key(0));
+
+ assertNull(this.inMemoryKeyValueStore.get(key(0)));
+ verify(this.getsCounter).inc();
+ verifyZeroInteractions(this.bytesReadCounter);
+ }
+
+ @Test
+ public void testPut() {
+ this.inMemoryKeyValueStore.put(key(0), value(0));
+ this.inMemoryKeyValueStore.put(key(OTHER_KEY_PREFIX, 1),
value(OTHER_VALUE_PREFIX, 1));
+
+ assertArrayEquals(value(0), this.inMemoryKeyValueStore.get(key(0)));
+ assertArrayEquals(value(OTHER_VALUE_PREFIX, 1),
this.inMemoryKeyValueStore.get(key(OTHER_KEY_PREFIX, 1)));
+ verify(this.putsCounter, times(2)).inc();
+ verify(this.bytesWrittenCounter).inc(key(0).length + value(0).length);
+ verify(this.bytesWrittenCounter).inc(key(OTHER_KEY_PREFIX, 1).length +
value(OTHER_VALUE_PREFIX, 1).length);
+ }
+
+ @Test
+ public void testPutExistingEntry() {
+ this.inMemoryKeyValueStore.put(key(0), value(0));
+ this.inMemoryKeyValueStore.put(key(0), value(OTHER_VALUE_PREFIX, 1));
+
+ assertArrayEquals(value(OTHER_VALUE_PREFIX, 1),
this.inMemoryKeyValueStore.get(key(0)));
+ verify(this.putsCounter, times(2)).inc();
+ verify(this.bytesWrittenCounter).inc(key(0).length + value(0).length);
+ verify(this.bytesWrittenCounter).inc(key(0).length +
value(OTHER_VALUE_PREFIX, 1).length);
+ }
+
+ @Test
+ public void testPutEmpty() {
+ byte[] emptyValue = new byte[0];
+ this.inMemoryKeyValueStore.put(key(0), emptyValue);
+
+ assertEquals(0, this.inMemoryKeyValueStore.get(key(0)).length);
+ verify(this.putsCounter).inc();
+ verify(this.bytesWrittenCounter).inc(key(0).length);
+ }
Review comment:
Yes, this tests when the value has zero length. `testPut` is puts a
non-empty value.
##########
File path:
samza-kv-inmemory/src/test/java/org/apache/samza/storage/kv/inmemory/TestInMemoryKeyValueStore.java
##########
@@ -19,66 +19,501 @@
package org.apache.samza.storage.kv.inmemory;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
import com.google.common.collect.Iterators;
import com.google.common.primitives.Ints;
-import org.apache.samza.metrics.MetricsRegistryMap;
+import org.apache.samza.SamzaException;
+import org.apache.samza.metrics.Counter;
import org.apache.samza.storage.kv.Entry;
import org.apache.samza.storage.kv.KeyValueIterator;
import org.apache.samza.storage.kv.KeyValueSnapshot;
import org.apache.samza.storage.kv.KeyValueStoreMetrics;
+import org.junit.Before;
import org.junit.Test;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
-import java.io.ByteArrayOutputStream;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
-import java.util.concurrent.ThreadLocalRandom;
-import java.util.stream.Collectors;
-import java.util.stream.IntStream;
-
+import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.verifyZeroInteractions;
+import static org.mockito.Mockito.when;
+
public class TestInMemoryKeyValueStore {
+ private static final String DEFAULT_KEY_PREFIX = "key_prefix";
+ private static final String OTHER_KEY_PREFIX = "other_key_prefix";
+ /**
+ * Keep the lengths of the values longer so that metrics validations for key
and value sizes don't collide.
+ */
+ private static final String DEFAULT_VALUE_PREFIX =
"value_prefix_value_prefix";
+ private static final String OTHER_VALUE_PREFIX =
"other_value_prefix_value_prefix";
+
+ @Mock
+ private KeyValueStoreMetrics keyValueStoreMetrics;
+ @Mock
+ private Counter getsCounter;
+ @Mock
+ private Counter bytesReadCounter;
+ @Mock
+ private Counter putsCounter;
+ @Mock
+ private Counter bytesWrittenCounter;
+ @Mock
+ private Counter deletesCounter;
+
+ private InMemoryKeyValueStore inMemoryKeyValueStore;
+
+ @Before
+ public void setup() {
+ MockitoAnnotations.initMocks(this);
+ when(this.keyValueStoreMetrics.gets()).thenReturn(this.getsCounter);
+
when(this.keyValueStoreMetrics.bytesRead()).thenReturn(this.bytesReadCounter);
+ when(this.keyValueStoreMetrics.puts()).thenReturn(this.putsCounter);
+
when(this.keyValueStoreMetrics.bytesWritten()).thenReturn(this.bytesWrittenCounter);
+ when(this.keyValueStoreMetrics.deletes()).thenReturn(this.deletesCounter);
+ this.inMemoryKeyValueStore = new
InMemoryKeyValueStore(this.keyValueStoreMetrics);
+ }
+
+ @Test
+ public void testGet() {
+ this.inMemoryKeyValueStore.put(key(0), value(0));
+ this.inMemoryKeyValueStore.put(key(OTHER_KEY_PREFIX, 1),
value(OTHER_VALUE_PREFIX, 1));
+
+ assertArrayEquals(value(0), this.inMemoryKeyValueStore.get(key(0)));
+ assertArrayEquals(value(OTHER_VALUE_PREFIX, 1),
this.inMemoryKeyValueStore.get(key(OTHER_KEY_PREFIX, 1)));
+ verify(this.getsCounter, times(2)).inc();
+ verify(this.bytesReadCounter).inc(value(0).length);
+ verify(this.bytesReadCounter).inc(value(OTHER_VALUE_PREFIX, 1).length);
+ }
+
+ @Test
+ public void testGetEmpty() {
+ assertNull(this.inMemoryKeyValueStore.get(key(0)));
+ verify(this.getsCounter).inc();
+ verifyZeroInteractions(this.bytesReadCounter);
+ }
+
+ @Test
+ public void testGetAfterDelete() {
+ this.inMemoryKeyValueStore.put(key(0), value(0));
+ this.inMemoryKeyValueStore.delete(key(0));
+
+ assertNull(this.inMemoryKeyValueStore.get(key(0)));
+ verify(this.getsCounter).inc();
+ verifyZeroInteractions(this.bytesReadCounter);
+ }
+
+ @Test
+ public void testPut() {
+ this.inMemoryKeyValueStore.put(key(0), value(0));
+ this.inMemoryKeyValueStore.put(key(OTHER_KEY_PREFIX, 1),
value(OTHER_VALUE_PREFIX, 1));
+
+ assertArrayEquals(value(0), this.inMemoryKeyValueStore.get(key(0)));
+ assertArrayEquals(value(OTHER_VALUE_PREFIX, 1),
this.inMemoryKeyValueStore.get(key(OTHER_KEY_PREFIX, 1)));
+ verify(this.putsCounter, times(2)).inc();
+ verify(this.bytesWrittenCounter).inc(key(0).length + value(0).length);
+ verify(this.bytesWrittenCounter).inc(key(OTHER_KEY_PREFIX, 1).length +
value(OTHER_VALUE_PREFIX, 1).length);
+ }
+
+ @Test
+ public void testPutExistingEntry() {
+ this.inMemoryKeyValueStore.put(key(0), value(0));
+ this.inMemoryKeyValueStore.put(key(0), value(OTHER_VALUE_PREFIX, 1));
+
+ assertArrayEquals(value(OTHER_VALUE_PREFIX, 1),
this.inMemoryKeyValueStore.get(key(0)));
+ verify(this.putsCounter, times(2)).inc();
+ verify(this.bytesWrittenCounter).inc(key(0).length + value(0).length);
+ verify(this.bytesWrittenCounter).inc(key(0).length +
value(OTHER_VALUE_PREFIX, 1).length);
+ }
+
+ @Test
+ public void testPutEmpty() {
+ byte[] emptyValue = new byte[0];
+ this.inMemoryKeyValueStore.put(key(0), emptyValue);
+
+ assertEquals(0, this.inMemoryKeyValueStore.get(key(0)).length);
+ verify(this.putsCounter).inc();
+ verify(this.bytesWrittenCounter).inc(key(0).length);
+ }
+
+ @Test
+ public void testPutNull() {
+ this.inMemoryKeyValueStore.put(key(0), value(0));
+ this.inMemoryKeyValueStore.put(key(0), null);
+
+ assertNull(this.inMemoryKeyValueStore.get(key(0)));
+ verify(this.putsCounter, times(2)).inc();
+ verify(this.deletesCounter).inc();
+ verify(this.bytesWrittenCounter).inc(key(0).length + value(0).length);
+ }
+
+ @Test
+ public void testPutAll() {
+ List<Entry<byte[], byte[]>> entries = new ArrayList<>();
+ for (int i = 0; i < 10; i++) {
+ entries.add(new Entry<>(key(i), value(i)));
+ }
+ this.inMemoryKeyValueStore.putAll(entries);
+
+ for (int i = 0; i < 10; i++) {
+ assertArrayEquals(value(i), this.inMemoryKeyValueStore.get(key(i)));
+ }
+ verify(this.putsCounter, times(10)).inc();
+ verify(this.bytesWrittenCounter, times(10)).inc(key(0).length +
value(0).length);
+ }
+
+ @Test
+ public void testPutAllUpdate() {
+ // check that an existing value is overridden
+ this.inMemoryKeyValueStore.put(key(0), value(1234));
+ List<Entry<byte[], byte[]>> entries = new ArrayList<>();
+ for (int i = 0; i < 10; i++) {
+ entries.add(new Entry<>(key(i), value(i)));
+ }
+ this.inMemoryKeyValueStore.putAll(entries);
+
+ for (int i = 0; i < 10; i++) {
+ assertArrayEquals(value(i), this.inMemoryKeyValueStore.get(key(i)));
+ }
+ // 1 time for initial value to be overridden, 10 times for "regular" puts
+ verify(this.putsCounter, times(11)).inc();
+ verify(this.bytesWrittenCounter).inc(key(0).length + value(1234).length);
+ verify(this.bytesWrittenCounter, times(10)).inc(key(0).length +
value(0).length);
Review comment:
Yep, it does assume that, good point. Thanks for the suggestion. I
defined a constant and added some more javadocs to clarify this.
##########
File path:
samza-kv-inmemory/src/test/java/org/apache/samza/storage/kv/inmemory/TestInMemoryKeyValueStore.java
##########
@@ -19,66 +19,501 @@
package org.apache.samza.storage.kv.inmemory;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
import com.google.common.collect.Iterators;
import com.google.common.primitives.Ints;
-import org.apache.samza.metrics.MetricsRegistryMap;
+import org.apache.samza.SamzaException;
+import org.apache.samza.metrics.Counter;
import org.apache.samza.storage.kv.Entry;
import org.apache.samza.storage.kv.KeyValueIterator;
import org.apache.samza.storage.kv.KeyValueSnapshot;
import org.apache.samza.storage.kv.KeyValueStoreMetrics;
+import org.junit.Before;
import org.junit.Test;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
-import java.io.ByteArrayOutputStream;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
-import java.util.concurrent.ThreadLocalRandom;
-import java.util.stream.Collectors;
-import java.util.stream.IntStream;
-
+import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.verifyZeroInteractions;
+import static org.mockito.Mockito.when;
+
public class TestInMemoryKeyValueStore {
+ private static final String DEFAULT_KEY_PREFIX = "key_prefix";
+ private static final String OTHER_KEY_PREFIX = "other_key_prefix";
+ /**
+ * Keep the lengths of the values longer so that metrics validations for key
and value sizes don't collide.
+ */
+ private static final String DEFAULT_VALUE_PREFIX =
"value_prefix_value_prefix";
+ private static final String OTHER_VALUE_PREFIX =
"other_value_prefix_value_prefix";
+
+ @Mock
+ private KeyValueStoreMetrics keyValueStoreMetrics;
+ @Mock
+ private Counter getsCounter;
+ @Mock
+ private Counter bytesReadCounter;
+ @Mock
+ private Counter putsCounter;
+ @Mock
+ private Counter bytesWrittenCounter;
+ @Mock
+ private Counter deletesCounter;
+
+ private InMemoryKeyValueStore inMemoryKeyValueStore;
+
+ @Before
+ public void setup() {
+ MockitoAnnotations.initMocks(this);
+ when(this.keyValueStoreMetrics.gets()).thenReturn(this.getsCounter);
+
when(this.keyValueStoreMetrics.bytesRead()).thenReturn(this.bytesReadCounter);
+ when(this.keyValueStoreMetrics.puts()).thenReturn(this.putsCounter);
+
when(this.keyValueStoreMetrics.bytesWritten()).thenReturn(this.bytesWrittenCounter);
+ when(this.keyValueStoreMetrics.deletes()).thenReturn(this.deletesCounter);
+ this.inMemoryKeyValueStore = new
InMemoryKeyValueStore(this.keyValueStoreMetrics);
+ }
+
+ @Test
+ public void testGet() {
+ this.inMemoryKeyValueStore.put(key(0), value(0));
+ this.inMemoryKeyValueStore.put(key(OTHER_KEY_PREFIX, 1),
value(OTHER_VALUE_PREFIX, 1));
+
+ assertArrayEquals(value(0), this.inMemoryKeyValueStore.get(key(0)));
+ assertArrayEquals(value(OTHER_VALUE_PREFIX, 1),
this.inMemoryKeyValueStore.get(key(OTHER_KEY_PREFIX, 1)));
+ verify(this.getsCounter, times(2)).inc();
+ verify(this.bytesReadCounter).inc(value(0).length);
+ verify(this.bytesReadCounter).inc(value(OTHER_VALUE_PREFIX, 1).length);
+ }
+
+ @Test
+ public void testGetEmpty() {
+ assertNull(this.inMemoryKeyValueStore.get(key(0)));
+ verify(this.getsCounter).inc();
+ verifyZeroInteractions(this.bytesReadCounter);
+ }
+
+ @Test
+ public void testGetAfterDelete() {
+ this.inMemoryKeyValueStore.put(key(0), value(0));
+ this.inMemoryKeyValueStore.delete(key(0));
+
+ assertNull(this.inMemoryKeyValueStore.get(key(0)));
+ verify(this.getsCounter).inc();
+ verifyZeroInteractions(this.bytesReadCounter);
+ }
+
+ @Test
+ public void testPut() {
+ this.inMemoryKeyValueStore.put(key(0), value(0));
+ this.inMemoryKeyValueStore.put(key(OTHER_KEY_PREFIX, 1),
value(OTHER_VALUE_PREFIX, 1));
+
+ assertArrayEquals(value(0), this.inMemoryKeyValueStore.get(key(0)));
+ assertArrayEquals(value(OTHER_VALUE_PREFIX, 1),
this.inMemoryKeyValueStore.get(key(OTHER_KEY_PREFIX, 1)));
+ verify(this.putsCounter, times(2)).inc();
+ verify(this.bytesWrittenCounter).inc(key(0).length + value(0).length);
+ verify(this.bytesWrittenCounter).inc(key(OTHER_KEY_PREFIX, 1).length +
value(OTHER_VALUE_PREFIX, 1).length);
+ }
+
+ @Test
+ public void testPutExistingEntry() {
+ this.inMemoryKeyValueStore.put(key(0), value(0));
+ this.inMemoryKeyValueStore.put(key(0), value(OTHER_VALUE_PREFIX, 1));
+
+ assertArrayEquals(value(OTHER_VALUE_PREFIX, 1),
this.inMemoryKeyValueStore.get(key(0)));
+ verify(this.putsCounter, times(2)).inc();
+ verify(this.bytesWrittenCounter).inc(key(0).length + value(0).length);
+ verify(this.bytesWrittenCounter).inc(key(0).length +
value(OTHER_VALUE_PREFIX, 1).length);
+ }
+
+ @Test
+ public void testPutEmpty() {
+ byte[] emptyValue = new byte[0];
+ this.inMemoryKeyValueStore.put(key(0), emptyValue);
+
+ assertEquals(0, this.inMemoryKeyValueStore.get(key(0)).length);
+ verify(this.putsCounter).inc();
+ verify(this.bytesWrittenCounter).inc(key(0).length);
+ }
Review comment:
With the current implementation, it doesn't test a different code path.
It just seemed like an edge case that would be good to cover.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
