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/10 23:54:27 UTC

[geode] branch develop updated: GEODE-7978: Improve tests for Redis Module SREM Command (#4937)

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 b664be6  GEODE-7978: Improve tests for Redis Module SREM Command (#4937)
b664be6 is described below

commit b664be6654f70e7832f01c1fddf05ac2da102f95
Author: John Hutchison <jh...@gmail.com>
AuthorDate: Fri Apr 10 19:53:57 2020 -0400

    GEODE-7978: Improve tests for Redis Module SREM Command (#4937)
    
    
    Co-authored-by: John Hutchison <jh...@pivotal.io>
    Co-authored-by: Jens Deppe <jd...@pivotal.io>
---
 .../org/apache/geode/redis/GeoIntegrationTest.java |  3 +-
 .../apache/geode/redis/SetsIntegrationTest.java    | 93 +++++++++++++++++++---
 .../executor/set/SetExecutorJUnitTest.java         | 18 ++---
 3 files changed, 93 insertions(+), 21 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/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");
   }
 }