You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by je...@apache.org on 2020/04/13 16:02:24 UTC
[geode] branch develop updated: GEODE-7978: Improve tests for Redis
Module SREM Command (#4948)
This is an automated email from the ASF dual-hosted git repository.
jensdeppe pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 435cf3f GEODE-7978: Improve tests for Redis Module SREM Command (#4948)
435cf3f is described below
commit 435cf3fdce8ea372ae1e8ea4bfcad330e924efff
Author: Jens Deppe <jd...@pivotal.io>
AuthorDate: Mon Apr 13 09:01:48 2020 -0700
GEODE-7978: Improve tests for Redis Module SREM Command (#4948)
Co-authored-by: John Hutchison <jh...@pivotal.io>
---
.../org/apache/geode/redis/GeoIntegrationTest.java | 3 +-
.../apache/geode/redis/SetsIntegrationTest.java | 93 +++++++++++++++++++---
.../redis/internal/executor/set/SAddExecutor.java | 6 +-
.../executor/set/SetExecutorJUnitTest.java | 18 ++---
4 files changed, 96 insertions(+), 24 deletions(-)
diff --git a/geode-redis/src/integrationTest/java/org/apache/geode/redis/GeoIntegrationTest.java b/geode-redis/src/integrationTest/java/org/apache/geode/redis/GeoIntegrationTest.java
index b38e17f..844b9e5 100755
--- a/geode-redis/src/integrationTest/java/org/apache/geode/redis/GeoIntegrationTest.java
+++ b/geode-redis/src/integrationTest/java/org/apache/geode/redis/GeoIntegrationTest.java
@@ -23,7 +23,6 @@ import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
-import java.io.IOException;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -52,7 +51,7 @@ public class GeoIntegrationTest {
private static int port = 6379;
@BeforeClass
- public static void setUp() throws IOException {
+ public static void setUp() {
CacheFactory cf = new CacheFactory();
// cf.set("log-file", "redis.log");
cf.set(LOG_LEVEL, "error");
diff --git a/geode-redis/src/integrationTest/java/org/apache/geode/redis/SetsIntegrationTest.java b/geode-redis/src/integrationTest/java/org/apache/geode/redis/SetsIntegrationTest.java
index 53422bc..7d8da16 100755
--- a/geode-redis/src/integrationTest/java/org/apache/geode/redis/SetsIntegrationTest.java
+++ b/geode-redis/src/integrationTest/java/org/apache/geode/redis/SetsIntegrationTest.java
@@ -18,6 +18,7 @@ import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
import static org.apache.geode.distributed.ConfigurationProperties.LOG_LEVEL;
import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.catchThrowable;
import java.util.ArrayList;
import java.util.HashSet;
@@ -733,19 +734,91 @@ public class SetsIntegrationTest {
}
@Test
- public void testSRem() {
- jedis.sadd("master", "field1", "field2");
+ public void testSRem_should_ReturnTheNumberOfElementsRemoved() {
+ String key = "key";
+ String field1 = "field1";
+ String field2 = "field2";
+ jedis.sadd(key, field1, field2);
+
+ Long removedElementsCount = jedis.srem(key, field1, field2);
+
+ assertThat(removedElementsCount).isEqualTo(2);
+ }
+
+ @Test
+ public void testSRem_should_RemoveSpecifiedElementsFromSet() {
+ String key = "key";
+ String field1 = "field1";
+ String field2 = "field2";
+ jedis.sadd(key, field1, field2);
+
+ jedis.srem(key, field2);
+
+ Set<String> membersInSet = jedis.smembers(key);
+ assertThat(membersInSet).doesNotContain(field2);
+ }
+
+ @Test
+ public void testSRem_should_ThrowError_givenOnlyKey() {
+ String key = "key";
+ String field1 = "field1";
+ String field2 = "field2";
+ jedis.sadd(key, field1, field2);
+
+ Throwable caughtException = catchThrowable(
+ () -> jedis.srem(key));
+
+ assertThat(caughtException).hasMessageContaining("wrong number of arguments");
+ }
+
+ @Test
+ public void testSRem_should_notRemoveMembersOfSetNotSpecified() {
+ String key = "key";
+ String field1 = "field1";
+ String field2 = "field2";
+ jedis.sadd(key, field1, field2);
+
+ jedis.srem(key, field1);
+
+ Set<String> membersInSet = jedis.smembers(key);
+ assertThat(membersInSet).containsExactly(field2);
+ }
+
+ @Test
+ public void testSRem_should_ignoreMembersNotInSpecifiedSet() {
+ String key = "key";
+ String field1 = "field1";
+ String field2 = "field2";
+ String unkownField = "random-guy";
+ jedis.sadd(key, field1, field2);
+
+ long result = jedis.srem(key, unkownField);
+
+ assertThat(result).isEqualTo(0);
+ }
+
+ @Test
+ public void testSRem_should_throwError_givenKeyThatIsNotASet() {
+ String key = "key";
+ String value = "value";
+ jedis.set(key, value);
+
+ Throwable caughtException = catchThrowable(
+ () -> jedis.srem(key, value));
- Long sremCount = jedis.srem("master", "field1", "field2", "unknown");
- Set<String> sremSet = jedis.smembers("master");
- assertThat(sremCount).isEqualTo(2);
- assertThat(sremSet).isEmpty();
+ assertThat(caughtException)
+ .hasMessageContaining(
+ "WRONGTYPE Operation against a key holding the wrong kind of value");
+ }
+
+ @Test
+ public void testSRem_shouldReturnZero_givenNonExistingKey() {
+ String key = "key";
+ String field = "field";
- sremCount = jedis.srem("master", "field1", "field2", "unknown");
- assertThat(sremCount).isEqualTo(0);
+ long result = jedis.srem(key, field);
- sremCount = jedis.srem("unknownkey", "field1");
- assertThat(sremCount).isEqualTo(0);
+ assertThat(result).isEqualTo(0);
}
@Test
diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/set/SAddExecutor.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/set/SAddExecutor.java
index 49b3b3a..15d2c99 100755
--- a/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/set/SAddExecutor.java
+++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/executor/set/SAddExecutor.java
@@ -37,6 +37,9 @@ public class SAddExecutor extends SetExecutor {
return;
}
+ // Save key
+ context.getKeyRegistrar().register(command.getKey(), RedisDataType.REDIS_SET);
+
ByteArrayWrapper key = command.getKey();
RedisSet geodeRedisSet = new GeodeRedisSetSynchronized(key, context);
Set<ByteArrayWrapper> membersToAdd =
@@ -44,8 +47,5 @@ public class SAddExecutor extends SetExecutor {
long entriesAdded = geodeRedisSet.sadd(membersToAdd);
command.setResponse(Coder.getIntegerResponse(context.getByteBufAllocator(), entriesAdded));
-
- // Save key
- context.getKeyRegistrar().register(command.getKey(), RedisDataType.REDIS_SET);
}
}
diff --git a/geode-redis/src/test/java/org/apache/geode/redis/internal/executor/set/SetExecutorJUnitTest.java b/geode-redis/src/test/java/org/apache/geode/redis/internal/executor/set/SetExecutorJUnitTest.java
index d185afb..897aba5 100644
--- a/geode-redis/src/test/java/org/apache/geode/redis/internal/executor/set/SetExecutorJUnitTest.java
+++ b/geode-redis/src/test/java/org/apache/geode/redis/internal/executor/set/SetExecutorJUnitTest.java
@@ -349,26 +349,26 @@ public class SetExecutorJUnitTest {
}
@Test
- public void verifyErrorMessageWhenWrongArgsPassedToSRem() {
- Executor sRemExecutor = new SRemExecutor();
+ public void verifyErrorMessage_WhenTooFewArgsPassedToSRem() {
+ Executor subject = new SRemExecutor();
List<byte[]> commandsAsBytes = new ArrayList<>();
commandsAsBytes.add("SREM".getBytes());
- ArgumentCaptor<ByteBuf> argsErrorCaptor = ArgumentCaptor.forClass(ByteBuf.class);
+
+ ArgumentCaptor<ByteBuf> argsErrorCaptor =
+ ArgumentCaptor.forClass(ByteBuf.class);
when(context.getByteBufAllocator()).thenReturn(byteBuf);
- when(command.getProcessedCommand()).thenReturn(commandsAsBytes);
- sRemExecutor.executeCommand(command, context);
+ when(command.getProcessedCommand()).thenReturn(commandsAsBytes);
commandsAsBytes.add("key1".getBytes());
- sRemExecutor.executeCommand(command, context);
+ subject.executeCommand(command, context);
- verify(command, times(2)).setResponse(argsErrorCaptor.capture());
+ verify(command).setResponse(argsErrorCaptor.capture());
List<ByteBuf> capturedErrors = argsErrorCaptor.getAllValues();
+
assertThat(capturedErrors.get(0).toString(Charset.defaultCharset()))
.startsWith("-ERR The wrong number of arguments or syntax was provided");
- assertThat(capturedErrors.get(1).toString(Charset.defaultCharset()))
- .startsWith("-ERR The wrong number of arguments or syntax was provided");
}
}