DonalEvans commented on a change in pull request #7278:
URL: https://github.com/apache/geode/pull/7278#discussion_r792193884



##########
File path: 
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSet.java
##########
@@ -208,42 +207,26 @@ static int setOpStoreResult(RegionProvider 
regionProvider, RedisKey destinationK
     return result.size();
   }
 
-  public Pair<BigInteger, List<Object>> sscan(GlobPattern matchPattern, int 
count,
-      BigInteger cursor) {
-    List<Object> returnList = new ArrayList<>();
-    int size = members.size();
-    BigInteger beforeCursor = new BigInteger("0");
-    int numElements = 0;
-    int i = -1;
-    for (byte[] value : members) {
-      i++;
-      if (beforeCursor.compareTo(cursor) < 0) {
-        beforeCursor = beforeCursor.add(new BigInteger("1"));
-        continue;
-      }
+  public Pair<Integer, List<byte[]>> sscan(GlobPattern matchPattern, int count,
+      int cursor) {
+    long maximumCapacity = Math.min(count, scard() + 1);

Review comment:
       This can be an `int` which removed the need for a cast on line 213.

##########
File path: 
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSScanIntegrationTest.java
##########
@@ -156,152 +215,172 @@ public void 
givenNonexistentKey_andCursorIsNotInteger_returnsInvalidCursorError(
 
   @Test
   public void 
givenExistentSetKey_andCursorIsNotAnInteger_returnsInvalidCursorError() {
-    jedis.set("a", "b");
+    jedis.set(KEY, "b");
 
-    assertThatThrownBy(() -> jedis.sendCommand("a", Protocol.Command.SSCAN, 
"a", "sjfls"))
+    assertThatThrownBy(() -> jedis.sendCommand(KEY, Protocol.Command.SSCAN, 
KEY, "sjfls"))
         .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void givenNonexistentKey_returnsEmptyArray() {
-    ScanResult<String> result = jedis.sscan("nonexistent", "0");
+    ScanResult<String> result = jedis.sscan("nonexistent", ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
     assertThat(result.getResult()).isEmpty();
   }
 
+  @Test
+  public void negativeCursor_doesNotError() {
+    List<String> set = initializeThreeMemberSet();
+
+    String cursor = "-100";
+    ScanResult<String> result;
+    List<String> allEntries = new ArrayList<>();
+
+    do {
+      result = jedis.sscan(KEY, cursor);
+      allEntries.addAll(result.getResult());
+      cursor = result.getCursor();
+    } while (!result.isCompleteIteration());
+
+    assertThat(allEntries).hasSize(3);
+    assertThat(allEntries).containsExactlyInAnyOrderElementsOf(set);
+  }
+
   @Test
   public void givenSetWithOneMember_returnsMember() {
-    jedis.sadd("a", "1");
-    ScanResult<String> result = jedis.sscan("a", "0");
+    jedis.sadd(KEY, "1");
+    ScanResult<String> result = jedis.sscan(KEY, ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
-    assertThat(result.getResult()).containsExactly("1");
+    assertThat(result.getResult()).containsOnly(FIELD_ONE);
   }
 
   @Test
-  public void givenSetWithMultipleMembers_returnsAllMembers() {
-    jedis.sadd("a", "1", "2", "3");
-    ScanResult<String> result = jedis.sscan("a", "0");
+  public void givenSetWithMultipleMembers_returnsFewMembers() {
+    final List<String> initialMemberData = makeSet();
+    jedis.sadd(KEY, initialMemberData.toArray(new 
String[initialMemberData.size()]));
+    ScanResult<String> result = jedis.sscan(KEY, ZERO_CURSOR);
 
-    assertThat(result.isCompleteIteration()).isTrue();
-    assertThat(result.getResult()).containsExactlyInAnyOrder("1", "2", "3");
+    assertThat(result.getResult()).containsAnyElementsOf(initialMemberData);

Review comment:
       This assertion should be `isSubsetOf()`

##########
File path: 
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSScanIntegrationTest.java
##########
@@ -323,26 +402,204 @@ public void 
givenNegativeCursor_returnsMembersUsingAbsoluteValueOfCursor() {
 
   @Test
   public void givenCursorGreaterThanUnsignedLongCapacity_returnsCursorError() {
-    assertThatThrownBy(() -> jedis.sscan("a", "18446744073709551616"))
-        .hasMessageContaining(ERROR_CURSOR);
+    assertThatThrownBy(
+        () -> jedis.sscan(KEY, 
UNSIGNED_LONG_CAPACITY.add(BigInteger.ONE).toString()))
+            .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void 
givenNegativeCursorGreaterThanUnsignedLongCapacity_returnsCursorError() {
-    assertThatThrownBy(() -> jedis.sscan("a", "-18446744073709551616"))
-        .hasMessageContaining(ERROR_CURSOR);
+    assertThatThrownBy(
+        () -> jedis.sscan(KEY, 
NEGATIVE_LONG_CAPACITY.add(BigInteger.valueOf(-1)).toString()))
+            .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void givenInvalidRegexSyntax_returnsEmptyArray() {
-    jedis.sadd("a", "1");
+    jedis.sadd(KEY, "1");
     ScanParams scanParams = new ScanParams();
     scanParams.count(1);
     scanParams.match("\\p");
 
     ScanResult<byte[]> result =
-        jedis.sscan("a".getBytes(), "0".getBytes(), scanParams);
+        jedis.sscan(KEY.getBytes(), ZERO_CURSOR.getBytes(), scanParams);
 
     assertThat(result.getResult()).isEmpty();
   }
+
+  @Test
+  public void should_notReturnValue_givenValueWasRemovedBeforeSSCANISCalled() {
+    List<String> set = initializeThreeMemberSet();
+
+    jedis.srem(KEY, FIELD_THREE);
+    set.remove(FIELD_THREE);
+
+    GeodeAwaitility.await().untilAsserted(
+        () -> assertThat(jedis.sismember(KEY, FIELD_THREE)).isFalse());
+
+    ScanResult<String> result = jedis.sscan(KEY, ZERO_CURSOR);
+
+    assertThat(result.getResult()).containsExactlyInAnyOrderElementsOf(set);
+  }
+
+  @Test
+  public void should_notErrorGivenNonzeroCursorOnFirstCall() {
+    List<String> set = initializeThreeMemberSet();
+
+    ScanResult<String> result = jedis.sscan(KEY, "5");
+
+    assertThat(result.getResult()).isSubsetOf(set);
+  }
+
+  @Test
+  public void should_notErrorGivenCountEqualToIntegerMaxValue() {
+    List<byte[]> set = initializeThreeMemberByteSet();
+
+    ScanParams scanParams = new ScanParams().count(Integer.MAX_VALUE);
+
+    ScanResult<byte[]> result =
+        jedis.sscan(KEY.getBytes(), ZERO_CURSOR.getBytes(), scanParams);
+    assertThat(result.getResult())
+        .containsExactlyInAnyOrderElementsOf(set);
+  }
+
+  @Test
+  public void should_notErrorGivenCountGreaterThanIntegerMaxValue() {
+    initializeThreeMemberByteSet();
+
+    String greaterThanInt = String.valueOf(2L * Integer.MAX_VALUE);
+    List<Object> result =
+        uncheckedCast(jedis.sendCommand(KEY.getBytes(), Protocol.Command.SSCAN,
+            KEY.getBytes(), ZERO_CURSOR.getBytes(),
+            "COUNT".getBytes(), greaterThanInt.getBytes()));
+
+    assertThat((byte[]) result.get(0)).isEqualTo(ZERO_CURSOR.getBytes());
+
+    List<byte[]> fields = uncheckedCast(result.get(1));
+    assertThat(fields).containsExactlyInAnyOrder(
+        FIELD_ONE.getBytes(),
+        FIELD_TWO.getBytes(),
+        FIELD_THREE.getBytes());
+  }
+
+  /**** Concurrency ***/
+
+  @Test
+  public void 
should_returnAllConsistentlyPresentMembers_givenConcurrentThreadsAddingAndRemovingMembers()
 {
+    final List<String> initialMemberData = makeSet();
+    jedis.sadd(KEY, initialMemberData.toArray(new 
String[initialMemberData.size()]));
+    final int iterationCount = 500;
+
+    Jedis jedis1 = jedis.getConnectionFromSlot(SLOT_FOR_KEY);
+    Jedis jedis2 = jedis.getConnectionFromSlot(SLOT_FOR_KEY);
+
+    new ConcurrentLoopingThreads(iterationCount,
+        (i) -> multipleSScanAndAssertOnContentOfResultSet(i, jedis1, 
initialMemberData),
+        (i) -> multipleSScanAndAssertOnContentOfResultSet(i, jedis2, 
initialMemberData),
+        (i) -> {
+          String field = "new_" + BASE_FIELD + i;
+          jedis.sadd(KEY, field);
+          jedis.srem(KEY, field);
+        }).run();
+
+    jedis1.close();
+    jedis2.close();
+  }
+
+  @Test
+  public void should_notAlterUnderlyingData_givenMultipleConcurrentSscans() {
+    final List<String> initialMemberData = makeSet();
+    jedis.sadd(KEY, initialMemberData.toArray(new 
String[initialMemberData.size()]));
+    final int iterationCount = 500;
+
+    Jedis jedis1 = jedis.getConnectionFromSlot(SLOT_FOR_KEY);
+    Jedis jedis2 = jedis.getConnectionFromSlot(SLOT_FOR_KEY);
+
+    new ConcurrentLoopingThreads(iterationCount,
+        (i) -> multipleSScanAndAssertOnContentOfResultSet(i, jedis1, 
initialMemberData),
+        (i) -> multipleSScanAndAssertOnContentOfResultSet(i, jedis2, 
initialMemberData))
+            .run();
+
+    initialMemberData
+        .forEach((member) -> assertThat(jedis.sismember(KEY, 
member)).isTrue());
+
+    jedis1.close();
+    jedis2.close();
+  }
+
+  private void multipleSScanAndAssertOnContentOfResultSet(int iteration, Jedis 
jedis,
+      final List<String> initialMemberData) {
+
+    List<String> allEntries = new ArrayList<>();
+    ScanResult<String> result;
+    String cursor = ZERO_CURSOR;
+
+    do {
+      result = jedis.sscan(KEY, cursor);
+      cursor = result.getCursor();
+      List<String> resultEntries = result.getResult();
+      resultEntries
+          .forEach((entry) -> allEntries.add(entry));
+    } while (!result.isCompleteIteration());
+
+    assertThat(allEntries).as("failed on iteration " + iteration)
+        .containsAll(initialMemberData);
+  }
+
+  private void multipleSScanAndAssertOnSizeOfResultSet(Jedis jedis,

Review comment:
       This method is never used and should be removed.

##########
File path: 
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSScanIntegrationTest.java
##########
@@ -323,26 +409,217 @@ public void 
givenNegativeCursor_returnsMembersUsingAbsoluteValueOfCursor() {
 
   @Test
   public void givenCursorGreaterThanUnsignedLongCapacity_returnsCursorError() {
-    assertThatThrownBy(() -> jedis.sscan("a", "18446744073709551616"))
-        .hasMessageContaining(ERROR_CURSOR);
+    assertThatThrownBy(
+        () -> jedis.sscan(KEY, 
UNSIGNED_LONG_CAPACITY.add(BigInteger.ONE).toString()))
+            .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void 
givenNegativeCursorGreaterThanUnsignedLongCapacity_returnsCursorError() {
-    assertThatThrownBy(() -> jedis.sscan("a", "-18446744073709551616"))
-        .hasMessageContaining(ERROR_CURSOR);
+    assertThatThrownBy(
+        () -> jedis.sscan(KEY, 
SIGNED_LONG_CAPACITY.add(BigInteger.valueOf(-1)).toString()))
+            .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void givenInvalidRegexSyntax_returnsEmptyArray() {

Review comment:
       > This test is flawed, as there is no such thing as an invalid syntax 
for glob-style patterns. This test is just testing that given a non-matching 
pattern, an empty array is returned, so the name should reflect that. In fact, 
if the test is modified to the below, then it fails, as the element `p` matches 
and is returned:
   > 
   > ```
   >   @Test
   >   public void givenInvalidRegexSyntax_returnsEmptyArray() {
   >     jedis.sadd(KEY, "\\p", "p");
   >     ScanParams scanParams = new ScanParams();
   >     scanParams.count(10);
   >     scanParams.match("\\p");
   > 
   >     ScanResult<byte[]> result =
   >         jedis.sscan(KEY.getBytes(), ZERO_CURSOR.getBytes(), scanParams);
   > 
   >     assertThat(result.getResult()).isEmpty();
   >   }
   > ```
   
   This comment still applies. The test should be renamed to something along 
the lines of "givenNonMatchingPattern_returnsEmptyResult()" and the matching 
pattern changed to something without backslashes in it, as they just complicate 
things unnecessarily.

##########
File path: 
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSScanIntegrationTest.java
##########
@@ -156,152 +215,172 @@ public void 
givenNonexistentKey_andCursorIsNotInteger_returnsInvalidCursorError(
 
   @Test
   public void 
givenExistentSetKey_andCursorIsNotAnInteger_returnsInvalidCursorError() {
-    jedis.set("a", "b");
+    jedis.set(KEY, "b");
 
-    assertThatThrownBy(() -> jedis.sendCommand("a", Protocol.Command.SSCAN, 
"a", "sjfls"))
+    assertThatThrownBy(() -> jedis.sendCommand(KEY, Protocol.Command.SSCAN, 
KEY, "sjfls"))
         .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void givenNonexistentKey_returnsEmptyArray() {
-    ScanResult<String> result = jedis.sscan("nonexistent", "0");
+    ScanResult<String> result = jedis.sscan("nonexistent", ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
     assertThat(result.getResult()).isEmpty();
   }
 
+  @Test
+  public void negativeCursor_doesNotError() {
+    List<String> set = initializeThreeMemberSet();
+
+    String cursor = "-100";
+    ScanResult<String> result;
+    List<String> allEntries = new ArrayList<>();
+
+    do {
+      result = jedis.sscan(KEY, cursor);
+      allEntries.addAll(result.getResult());
+      cursor = result.getCursor();
+    } while (!result.isCompleteIteration());
+
+    assertThat(allEntries).hasSize(3);
+    assertThat(allEntries).containsExactlyInAnyOrderElementsOf(set);
+  }
+
   @Test
   public void givenSetWithOneMember_returnsMember() {
-    jedis.sadd("a", "1");
-    ScanResult<String> result = jedis.sscan("a", "0");
+    jedis.sadd(KEY, "1");
+    ScanResult<String> result = jedis.sscan(KEY, ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
-    assertThat(result.getResult()).containsExactly("1");
+    assertThat(result.getResult()).containsOnly(FIELD_ONE);
   }
 
   @Test
-  public void givenSetWithMultipleMembers_returnsAllMembers() {
-    jedis.sadd("a", "1", "2", "3");
-    ScanResult<String> result = jedis.sscan("a", "0");
+  public void givenSetWithMultipleMembers_returnsFewMembers() {
+    final List<String> initialMemberData = makeSet();

Review comment:
       Since we're explicitly dealing with sets, `makeSet()` should probably 
return a Set rather than a List.

##########
File path: 
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSScanIntegrationTest.java
##########
@@ -156,152 +215,172 @@ public void 
givenNonexistentKey_andCursorIsNotInteger_returnsInvalidCursorError(
 
   @Test
   public void 
givenExistentSetKey_andCursorIsNotAnInteger_returnsInvalidCursorError() {
-    jedis.set("a", "b");
+    jedis.set(KEY, "b");
 
-    assertThatThrownBy(() -> jedis.sendCommand("a", Protocol.Command.SSCAN, 
"a", "sjfls"))
+    assertThatThrownBy(() -> jedis.sendCommand(KEY, Protocol.Command.SSCAN, 
KEY, "sjfls"))
         .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void givenNonexistentKey_returnsEmptyArray() {
-    ScanResult<String> result = jedis.sscan("nonexistent", "0");
+    ScanResult<String> result = jedis.sscan("nonexistent", ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
     assertThat(result.getResult()).isEmpty();
   }
 
+  @Test
+  public void negativeCursor_doesNotError() {
+    List<String> set = initializeThreeMemberSet();
+
+    String cursor = "-100";
+    ScanResult<String> result;
+    List<String> allEntries = new ArrayList<>();
+
+    do {
+      result = jedis.sscan(KEY, cursor);
+      allEntries.addAll(result.getResult());
+      cursor = result.getCursor();
+    } while (!result.isCompleteIteration());
+
+    assertThat(allEntries).hasSize(3);
+    assertThat(allEntries).containsExactlyInAnyOrderElementsOf(set);
+  }
+
   @Test
   public void givenSetWithOneMember_returnsMember() {
-    jedis.sadd("a", "1");
-    ScanResult<String> result = jedis.sscan("a", "0");
+    jedis.sadd(KEY, "1");
+    ScanResult<String> result = jedis.sscan(KEY, ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
-    assertThat(result.getResult()).containsExactly("1");
+    assertThat(result.getResult()).containsOnly(FIELD_ONE);
   }
 
   @Test
-  public void givenSetWithMultipleMembers_returnsAllMembers() {
-    jedis.sadd("a", "1", "2", "3");
-    ScanResult<String> result = jedis.sscan("a", "0");
+  public void givenSetWithMultipleMembers_returnsFewMembers() {

Review comment:
       This test might be better named something like 
"givenSetWithMultipleMembers_returnsSubsetOfMembers"

##########
File path: 
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSScanIntegrationTest.java
##########
@@ -156,152 +215,172 @@ public void 
givenNonexistentKey_andCursorIsNotInteger_returnsInvalidCursorError(
 
   @Test
   public void 
givenExistentSetKey_andCursorIsNotAnInteger_returnsInvalidCursorError() {
-    jedis.set("a", "b");
+    jedis.set(KEY, "b");
 
-    assertThatThrownBy(() -> jedis.sendCommand("a", Protocol.Command.SSCAN, 
"a", "sjfls"))
+    assertThatThrownBy(() -> jedis.sendCommand(KEY, Protocol.Command.SSCAN, 
KEY, "sjfls"))
         .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void givenNonexistentKey_returnsEmptyArray() {
-    ScanResult<String> result = jedis.sscan("nonexistent", "0");
+    ScanResult<String> result = jedis.sscan("nonexistent", ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
     assertThat(result.getResult()).isEmpty();
   }
 
+  @Test
+  public void negativeCursor_doesNotError() {
+    List<String> set = initializeThreeMemberSet();
+
+    String cursor = "-100";
+    ScanResult<String> result;
+    List<String> allEntries = new ArrayList<>();
+
+    do {
+      result = jedis.sscan(KEY, cursor);
+      allEntries.addAll(result.getResult());
+      cursor = result.getCursor();
+    } while (!result.isCompleteIteration());
+
+    assertThat(allEntries).hasSize(3);
+    assertThat(allEntries).containsExactlyInAnyOrderElementsOf(set);
+  }

Review comment:
       Given that this test is just intending to show that a negative cursor 
value does not cause an error (behaviour with a negative cursor is undefined) 
we should only be asserting that no error is seen. This test should therefore 
be just:
   ```
     @Test
     public void negativeCursor_doesNotError() {
       initializeThreeMemberSet();
   
       assertThatNoException().isThrownBy(() -> jedis.sscan(KEY, "-100"));
     }
   ```

##########
File path: 
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSScanIntegrationTest.java
##########
@@ -55,95 +73,136 @@ public void tearDown() {
 
   @Test
   public void givenNoKeyArgument_returnsWrongNumberOfArgumentsError() {
-    assertThatThrownBy(() -> jedis.sendCommand("key", Protocol.Command.SSCAN))
+    assertThatThrownBy(() -> jedis.sendCommand(KEY, Protocol.Command.SSCAN))
         .hasMessageContaining("ERR wrong number of arguments for 'sscan' 
command");
   }
 
   @Test
-  public void givenNoCursorArgument_returnsWrongNumberOfArgumentsError() {
-    assertThatThrownBy(() -> jedis.sendCommand("key", Protocol.Command.SSCAN, 
"key"))
+  public void givenIncorrectOptionalArgumentAndKeyExists_returnsSyntaxError() {
+    assertThatThrownBy(() -> jedis.sendCommand(KEY, Protocol.Command.SSCAN, 
KEY))
         .hasMessageContaining("ERR wrong number of arguments for 'sscan' 
command");
   }

Review comment:
       These tests should be replaced with a single test, 
"givenLessThanTwoArguments_returnsWrongNumberOfArgumentsError":
   ```
     @Test
     public void givenLessThanTwoArguments_returnsWrongNumberOfArgumentsError() 
{
       assertAtLeastNArgs(jedis, Protocol.Command.SSCAN, 2);
     }
   ```

##########
File path: 
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSScanIntegrationTest.java
##########
@@ -323,26 +402,204 @@ public void 
givenNegativeCursor_returnsMembersUsingAbsoluteValueOfCursor() {
 
   @Test
   public void givenCursorGreaterThanUnsignedLongCapacity_returnsCursorError() {
-    assertThatThrownBy(() -> jedis.sscan("a", "18446744073709551616"))
-        .hasMessageContaining(ERROR_CURSOR);
+    assertThatThrownBy(
+        () -> jedis.sscan(KEY, 
UNSIGNED_LONG_CAPACITY.add(BigInteger.ONE).toString()))
+            .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void 
givenNegativeCursorGreaterThanUnsignedLongCapacity_returnsCursorError() {
-    assertThatThrownBy(() -> jedis.sscan("a", "-18446744073709551616"))
-        .hasMessageContaining(ERROR_CURSOR);
+    assertThatThrownBy(
+        () -> jedis.sscan(KEY, 
NEGATIVE_LONG_CAPACITY.add(BigInteger.valueOf(-1)).toString()))
+            .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void givenInvalidRegexSyntax_returnsEmptyArray() {
-    jedis.sadd("a", "1");
+    jedis.sadd(KEY, "1");
     ScanParams scanParams = new ScanParams();
     scanParams.count(1);
     scanParams.match("\\p");
 
     ScanResult<byte[]> result =
-        jedis.sscan("a".getBytes(), "0".getBytes(), scanParams);
+        jedis.sscan(KEY.getBytes(), ZERO_CURSOR.getBytes(), scanParams);
 
     assertThat(result.getResult()).isEmpty();
   }
+
+  @Test
+  public void should_notReturnValue_givenValueWasRemovedBeforeSSCANISCalled() {
+    List<String> set = initializeThreeMemberSet();
+
+    jedis.srem(KEY, FIELD_THREE);
+    set.remove(FIELD_THREE);
+
+    GeodeAwaitility.await().untilAsserted(
+        () -> assertThat(jedis.sismember(KEY, FIELD_THREE)).isFalse());
+
+    ScanResult<String> result = jedis.sscan(KEY, ZERO_CURSOR);
+
+    assertThat(result.getResult()).containsExactlyInAnyOrderElementsOf(set);
+  }
+
+  @Test
+  public void should_notErrorGivenNonzeroCursorOnFirstCall() {

Review comment:
       This test should be changed to:
   ```
     @Test
     public void should_notErrorGivenNonzeroCursorOnFirstCall() {
       initializeThreeMemberSet();
   
       assertThatNoException().isThrownBy(() -> jedis.sscan(KEY, "5"));
     }
   ```

##########
File path: 
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSScanIntegrationTest.java
##########
@@ -323,26 +402,204 @@ public void 
givenNegativeCursor_returnsMembersUsingAbsoluteValueOfCursor() {
 
   @Test
   public void givenCursorGreaterThanUnsignedLongCapacity_returnsCursorError() {
-    assertThatThrownBy(() -> jedis.sscan("a", "18446744073709551616"))
-        .hasMessageContaining(ERROR_CURSOR);
+    assertThatThrownBy(
+        () -> jedis.sscan(KEY, 
UNSIGNED_LONG_CAPACITY.add(BigInteger.ONE).toString()))
+            .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void 
givenNegativeCursorGreaterThanUnsignedLongCapacity_returnsCursorError() {
-    assertThatThrownBy(() -> jedis.sscan("a", "-18446744073709551616"))
-        .hasMessageContaining(ERROR_CURSOR);
+    assertThatThrownBy(
+        () -> jedis.sscan(KEY, 
NEGATIVE_LONG_CAPACITY.add(BigInteger.valueOf(-1)).toString()))
+            .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void givenInvalidRegexSyntax_returnsEmptyArray() {
-    jedis.sadd("a", "1");
+    jedis.sadd(KEY, "1");
     ScanParams scanParams = new ScanParams();
     scanParams.count(1);
     scanParams.match("\\p");
 
     ScanResult<byte[]> result =
-        jedis.sscan("a".getBytes(), "0".getBytes(), scanParams);
+        jedis.sscan(KEY.getBytes(), ZERO_CURSOR.getBytes(), scanParams);
 
     assertThat(result.getResult()).isEmpty();
   }
+
+  @Test
+  public void should_notReturnValue_givenValueWasRemovedBeforeSSCANISCalled() {
+    List<String> set = initializeThreeMemberSet();
+
+    jedis.srem(KEY, FIELD_THREE);
+    set.remove(FIELD_THREE);
+
+    GeodeAwaitility.await().untilAsserted(
+        () -> assertThat(jedis.sismember(KEY, FIELD_THREE)).isFalse());
+
+    ScanResult<String> result = jedis.sscan(KEY, ZERO_CURSOR);
+
+    assertThat(result.getResult()).containsExactlyInAnyOrderElementsOf(set);
+  }
+
+  @Test
+  public void should_notErrorGivenNonzeroCursorOnFirstCall() {
+    List<String> set = initializeThreeMemberSet();
+
+    ScanResult<String> result = jedis.sscan(KEY, "5");
+
+    assertThat(result.getResult()).isSubsetOf(set);
+  }
+
+  @Test
+  public void should_notErrorGivenCountEqualToIntegerMaxValue() {
+    List<byte[]> set = initializeThreeMemberByteSet();
+
+    ScanParams scanParams = new ScanParams().count(Integer.MAX_VALUE);
+
+    ScanResult<byte[]> result =
+        jedis.sscan(KEY.getBytes(), ZERO_CURSOR.getBytes(), scanParams);
+    assertThat(result.getResult())
+        .containsExactlyInAnyOrderElementsOf(set);
+  }
+
+  @Test
+  public void should_notErrorGivenCountGreaterThanIntegerMaxValue() {
+    initializeThreeMemberByteSet();
+
+    String greaterThanInt = String.valueOf(2L * Integer.MAX_VALUE);
+    List<Object> result =
+        uncheckedCast(jedis.sendCommand(KEY.getBytes(), Protocol.Command.SSCAN,
+            KEY.getBytes(), ZERO_CURSOR.getBytes(),
+            "COUNT".getBytes(), greaterThanInt.getBytes()));
+
+    assertThat((byte[]) result.get(0)).isEqualTo(ZERO_CURSOR.getBytes());
+
+    List<byte[]> fields = uncheckedCast(result.get(1));
+    assertThat(fields).containsExactlyInAnyOrder(
+        FIELD_ONE.getBytes(),
+        FIELD_TWO.getBytes(),
+        FIELD_THREE.getBytes());
+  }
+
+  /**** Concurrency ***/
+
+  @Test
+  public void 
should_returnAllConsistentlyPresentMembers_givenConcurrentThreadsAddingAndRemovingMembers()
 {
+    final List<String> initialMemberData = makeSet();
+    jedis.sadd(KEY, initialMemberData.toArray(new 
String[initialMemberData.size()]));

Review comment:
       This should be `jedis.sadd(KEY, initialMemberData.toArray(new 
String[0]));`

##########
File path: 
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSScanIntegrationTest.java
##########
@@ -323,26 +402,204 @@ public void 
givenNegativeCursor_returnsMembersUsingAbsoluteValueOfCursor() {
 
   @Test
   public void givenCursorGreaterThanUnsignedLongCapacity_returnsCursorError() {
-    assertThatThrownBy(() -> jedis.sscan("a", "18446744073709551616"))
-        .hasMessageContaining(ERROR_CURSOR);
+    assertThatThrownBy(
+        () -> jedis.sscan(KEY, 
UNSIGNED_LONG_CAPACITY.add(BigInteger.ONE).toString()))
+            .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void 
givenNegativeCursorGreaterThanUnsignedLongCapacity_returnsCursorError() {
-    assertThatThrownBy(() -> jedis.sscan("a", "-18446744073709551616"))
-        .hasMessageContaining(ERROR_CURSOR);
+    assertThatThrownBy(
+        () -> jedis.sscan(KEY, 
NEGATIVE_LONG_CAPACITY.add(BigInteger.valueOf(-1)).toString()))
+            .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void givenInvalidRegexSyntax_returnsEmptyArray() {
-    jedis.sadd("a", "1");
+    jedis.sadd(KEY, "1");
     ScanParams scanParams = new ScanParams();
     scanParams.count(1);
     scanParams.match("\\p");
 
     ScanResult<byte[]> result =
-        jedis.sscan("a".getBytes(), "0".getBytes(), scanParams);
+        jedis.sscan(KEY.getBytes(), ZERO_CURSOR.getBytes(), scanParams);
 
     assertThat(result.getResult()).isEmpty();
   }
+
+  @Test
+  public void should_notReturnValue_givenValueWasRemovedBeforeSSCANISCalled() {

Review comment:
       For consistency, this should be 
"should_notReturnValue_givenValueWasRemovedBeforeSscanIsCalled"

##########
File path: 
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSScanIntegrationTest.java
##########
@@ -156,152 +215,172 @@ public void 
givenNonexistentKey_andCursorIsNotInteger_returnsInvalidCursorError(
 
   @Test
   public void 
givenExistentSetKey_andCursorIsNotAnInteger_returnsInvalidCursorError() {
-    jedis.set("a", "b");
+    jedis.set(KEY, "b");
 
-    assertThatThrownBy(() -> jedis.sendCommand("a", Protocol.Command.SSCAN, 
"a", "sjfls"))
+    assertThatThrownBy(() -> jedis.sendCommand(KEY, Protocol.Command.SSCAN, 
KEY, "sjfls"))
         .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void givenNonexistentKey_returnsEmptyArray() {
-    ScanResult<String> result = jedis.sscan("nonexistent", "0");
+    ScanResult<String> result = jedis.sscan("nonexistent", ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
     assertThat(result.getResult()).isEmpty();
   }
 
+  @Test
+  public void negativeCursor_doesNotError() {
+    List<String> set = initializeThreeMemberSet();
+
+    String cursor = "-100";
+    ScanResult<String> result;
+    List<String> allEntries = new ArrayList<>();
+
+    do {
+      result = jedis.sscan(KEY, cursor);
+      allEntries.addAll(result.getResult());
+      cursor = result.getCursor();
+    } while (!result.isCompleteIteration());
+
+    assertThat(allEntries).hasSize(3);
+    assertThat(allEntries).containsExactlyInAnyOrderElementsOf(set);
+  }
+
   @Test
   public void givenSetWithOneMember_returnsMember() {
-    jedis.sadd("a", "1");
-    ScanResult<String> result = jedis.sscan("a", "0");
+    jedis.sadd(KEY, "1");
+    ScanResult<String> result = jedis.sscan(KEY, ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
-    assertThat(result.getResult()).containsExactly("1");
+    assertThat(result.getResult()).containsOnly(FIELD_ONE);
   }
 
   @Test
-  public void givenSetWithMultipleMembers_returnsAllMembers() {
-    jedis.sadd("a", "1", "2", "3");
-    ScanResult<String> result = jedis.sscan("a", "0");
+  public void givenSetWithMultipleMembers_returnsFewMembers() {
+    final List<String> initialMemberData = makeSet();
+    jedis.sadd(KEY, initialMemberData.toArray(new 
String[initialMemberData.size()]));

Review comment:
       This should be:
   ```
   jedis.sadd(KEY, initialMemberData.toArray(new String[0]));
   ```
   From the IntelliJ inspection description:
   >In older Java versions using pre-sized array was recommended, as the 
reflection call which is necessary to create an array of proper size was quite 
slow. However since late updates of OpenJDK 6 this call was intrinsified, 
making the performance of the empty array version the same and sometimes even 
better, compared to the pre-sized version. Also passing pre-sized array is 
dangerous for a concurrent or synchronized collection as a data race is 
possible between the size and toArray call which may result in extra nulls at 
the end of the array, if the collection was concurrently shrunk during the 
operation.

##########
File path: 
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSScanIntegrationTest.java
##########
@@ -323,26 +402,204 @@ public void 
givenNegativeCursor_returnsMembersUsingAbsoluteValueOfCursor() {
 
   @Test
   public void givenCursorGreaterThanUnsignedLongCapacity_returnsCursorError() {
-    assertThatThrownBy(() -> jedis.sscan("a", "18446744073709551616"))
-        .hasMessageContaining(ERROR_CURSOR);
+    assertThatThrownBy(
+        () -> jedis.sscan(KEY, 
UNSIGNED_LONG_CAPACITY.add(BigInteger.ONE).toString()))
+            .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void 
givenNegativeCursorGreaterThanUnsignedLongCapacity_returnsCursorError() {
-    assertThatThrownBy(() -> jedis.sscan("a", "-18446744073709551616"))
-        .hasMessageContaining(ERROR_CURSOR);
+    assertThatThrownBy(
+        () -> jedis.sscan(KEY, 
NEGATIVE_LONG_CAPACITY.add(BigInteger.valueOf(-1)).toString()))
+            .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void givenInvalidRegexSyntax_returnsEmptyArray() {
-    jedis.sadd("a", "1");
+    jedis.sadd(KEY, "1");
     ScanParams scanParams = new ScanParams();
     scanParams.count(1);
     scanParams.match("\\p");
 
     ScanResult<byte[]> result =
-        jedis.sscan("a".getBytes(), "0".getBytes(), scanParams);
+        jedis.sscan(KEY.getBytes(), ZERO_CURSOR.getBytes(), scanParams);
 
     assertThat(result.getResult()).isEmpty();
   }
+
+  @Test
+  public void should_notReturnValue_givenValueWasRemovedBeforeSSCANISCalled() {
+    List<String> set = initializeThreeMemberSet();
+
+    jedis.srem(KEY, FIELD_THREE);
+    set.remove(FIELD_THREE);
+
+    GeodeAwaitility.await().untilAsserted(
+        () -> assertThat(jedis.sismember(KEY, FIELD_THREE)).isFalse());
+
+    ScanResult<String> result = jedis.sscan(KEY, ZERO_CURSOR);
+
+    assertThat(result.getResult()).containsExactlyInAnyOrderElementsOf(set);
+  }
+
+  @Test
+  public void should_notErrorGivenNonzeroCursorOnFirstCall() {
+    List<String> set = initializeThreeMemberSet();
+
+    ScanResult<String> result = jedis.sscan(KEY, "5");
+
+    assertThat(result.getResult()).isSubsetOf(set);
+  }
+
+  @Test
+  public void should_notErrorGivenCountEqualToIntegerMaxValue() {
+    List<byte[]> set = initializeThreeMemberByteSet();
+
+    ScanParams scanParams = new ScanParams().count(Integer.MAX_VALUE);
+
+    ScanResult<byte[]> result =
+        jedis.sscan(KEY.getBytes(), ZERO_CURSOR.getBytes(), scanParams);
+    assertThat(result.getResult())
+        .containsExactlyInAnyOrderElementsOf(set);
+  }
+
+  @Test
+  public void should_notErrorGivenCountGreaterThanIntegerMaxValue() {
+    initializeThreeMemberByteSet();
+
+    String greaterThanInt = String.valueOf(2L * Integer.MAX_VALUE);
+    List<Object> result =
+        uncheckedCast(jedis.sendCommand(KEY.getBytes(), Protocol.Command.SSCAN,
+            KEY.getBytes(), ZERO_CURSOR.getBytes(),
+            "COUNT".getBytes(), greaterThanInt.getBytes()));
+
+    assertThat((byte[]) result.get(0)).isEqualTo(ZERO_CURSOR.getBytes());
+
+    List<byte[]> fields = uncheckedCast(result.get(1));
+    assertThat(fields).containsExactlyInAnyOrder(
+        FIELD_ONE.getBytes(),
+        FIELD_TWO.getBytes(),
+        FIELD_THREE.getBytes());
+  }
+
+  /**** Concurrency ***/
+
+  @Test
+  public void 
should_returnAllConsistentlyPresentMembers_givenConcurrentThreadsAddingAndRemovingMembers()
 {
+    final List<String> initialMemberData = makeSet();
+    jedis.sadd(KEY, initialMemberData.toArray(new 
String[initialMemberData.size()]));
+    final int iterationCount = 500;
+
+    Jedis jedis1 = jedis.getConnectionFromSlot(SLOT_FOR_KEY);
+    Jedis jedis2 = jedis.getConnectionFromSlot(SLOT_FOR_KEY);
+
+    new ConcurrentLoopingThreads(iterationCount,
+        (i) -> multipleSScanAndAssertOnContentOfResultSet(i, jedis1, 
initialMemberData),
+        (i) -> multipleSScanAndAssertOnContentOfResultSet(i, jedis2, 
initialMemberData),
+        (i) -> {
+          String field = "new_" + BASE_FIELD + i;
+          jedis.sadd(KEY, field);
+          jedis.srem(KEY, field);
+        }).run();
+
+    jedis1.close();
+    jedis2.close();
+  }
+
+  @Test
+  public void should_notAlterUnderlyingData_givenMultipleConcurrentSscans() {
+    final List<String> initialMemberData = makeSet();
+    jedis.sadd(KEY, initialMemberData.toArray(new 
String[initialMemberData.size()]));

Review comment:
       This should be `jedis.sadd(KEY, initialMemberData.toArray(new 
String[0]));`

##########
File path: 
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSScanIntegrationTest.java
##########
@@ -323,26 +402,204 @@ public void 
givenNegativeCursor_returnsMembersUsingAbsoluteValueOfCursor() {
 
   @Test
   public void givenCursorGreaterThanUnsignedLongCapacity_returnsCursorError() {
-    assertThatThrownBy(() -> jedis.sscan("a", "18446744073709551616"))
-        .hasMessageContaining(ERROR_CURSOR);
+    assertThatThrownBy(
+        () -> jedis.sscan(KEY, 
UNSIGNED_LONG_CAPACITY.add(BigInteger.ONE).toString()))
+            .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void 
givenNegativeCursorGreaterThanUnsignedLongCapacity_returnsCursorError() {
-    assertThatThrownBy(() -> jedis.sscan("a", "-18446744073709551616"))
-        .hasMessageContaining(ERROR_CURSOR);
+    assertThatThrownBy(
+        () -> jedis.sscan(KEY, 
NEGATIVE_LONG_CAPACITY.add(BigInteger.valueOf(-1)).toString()))
+            .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void givenInvalidRegexSyntax_returnsEmptyArray() {
-    jedis.sadd("a", "1");
+    jedis.sadd(KEY, "1");
     ScanParams scanParams = new ScanParams();
     scanParams.count(1);
     scanParams.match("\\p");
 
     ScanResult<byte[]> result =
-        jedis.sscan("a".getBytes(), "0".getBytes(), scanParams);
+        jedis.sscan(KEY.getBytes(), ZERO_CURSOR.getBytes(), scanParams);
 
     assertThat(result.getResult()).isEmpty();
   }
+
+  @Test
+  public void should_notReturnValue_givenValueWasRemovedBeforeSSCANISCalled() {
+    List<String> set = initializeThreeMemberSet();
+
+    jedis.srem(KEY, FIELD_THREE);
+    set.remove(FIELD_THREE);
+
+    GeodeAwaitility.await().untilAsserted(
+        () -> assertThat(jedis.sismember(KEY, FIELD_THREE)).isFalse());
+
+    ScanResult<String> result = jedis.sscan(KEY, ZERO_CURSOR);
+
+    assertThat(result.getResult()).containsExactlyInAnyOrderElementsOf(set);

Review comment:
       To be more explicit, this assert should be changed to: 
`assertThat(result.getResult()).doesNotContain(FIELD_THREE);`
   
   This also renders the `set` variable redundant, so it can be removed 
(although the call to `initializeThreeMemberSet()` will have to stay).

##########
File path: 
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSScanIntegrationTest.java
##########
@@ -323,26 +402,204 @@ public void 
givenNegativeCursor_returnsMembersUsingAbsoluteValueOfCursor() {
 
   @Test
   public void givenCursorGreaterThanUnsignedLongCapacity_returnsCursorError() {
-    assertThatThrownBy(() -> jedis.sscan("a", "18446744073709551616"))
-        .hasMessageContaining(ERROR_CURSOR);
+    assertThatThrownBy(
+        () -> jedis.sscan(KEY, 
UNSIGNED_LONG_CAPACITY.add(BigInteger.ONE).toString()))
+            .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void 
givenNegativeCursorGreaterThanUnsignedLongCapacity_returnsCursorError() {
-    assertThatThrownBy(() -> jedis.sscan("a", "-18446744073709551616"))
-        .hasMessageContaining(ERROR_CURSOR);
+    assertThatThrownBy(
+        () -> jedis.sscan(KEY, 
NEGATIVE_LONG_CAPACITY.add(BigInteger.valueOf(-1)).toString()))
+            .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void givenInvalidRegexSyntax_returnsEmptyArray() {
-    jedis.sadd("a", "1");
+    jedis.sadd(KEY, "1");
     ScanParams scanParams = new ScanParams();
     scanParams.count(1);
     scanParams.match("\\p");
 
     ScanResult<byte[]> result =
-        jedis.sscan("a".getBytes(), "0".getBytes(), scanParams);
+        jedis.sscan(KEY.getBytes(), ZERO_CURSOR.getBytes(), scanParams);
 
     assertThat(result.getResult()).isEmpty();
   }
+
+  @Test
+  public void should_notReturnValue_givenValueWasRemovedBeforeSSCANISCalled() {
+    List<String> set = initializeThreeMemberSet();
+
+    jedis.srem(KEY, FIELD_THREE);
+    set.remove(FIELD_THREE);
+
+    GeodeAwaitility.await().untilAsserted(
+        () -> assertThat(jedis.sismember(KEY, FIELD_THREE)).isFalse());
+
+    ScanResult<String> result = jedis.sscan(KEY, ZERO_CURSOR);
+
+    assertThat(result.getResult()).containsExactlyInAnyOrderElementsOf(set);
+  }
+
+  @Test
+  public void should_notErrorGivenNonzeroCursorOnFirstCall() {
+    List<String> set = initializeThreeMemberSet();
+
+    ScanResult<String> result = jedis.sscan(KEY, "5");
+
+    assertThat(result.getResult()).isSubsetOf(set);
+  }
+
+  @Test
+  public void should_notErrorGivenCountEqualToIntegerMaxValue() {
+    List<byte[]> set = initializeThreeMemberByteSet();
+
+    ScanParams scanParams = new ScanParams().count(Integer.MAX_VALUE);
+
+    ScanResult<byte[]> result =
+        jedis.sscan(KEY.getBytes(), ZERO_CURSOR.getBytes(), scanParams);
+    assertThat(result.getResult())
+        .containsExactlyInAnyOrderElementsOf(set);
+  }
+
+  @Test
+  public void should_notErrorGivenCountGreaterThanIntegerMaxValue() {
+    initializeThreeMemberByteSet();
+
+    String greaterThanInt = String.valueOf(2L * Integer.MAX_VALUE);
+    List<Object> result =
+        uncheckedCast(jedis.sendCommand(KEY.getBytes(), Protocol.Command.SSCAN,
+            KEY.getBytes(), ZERO_CURSOR.getBytes(),
+            "COUNT".getBytes(), greaterThanInt.getBytes()));
+
+    assertThat((byte[]) result.get(0)).isEqualTo(ZERO_CURSOR.getBytes());
+
+    List<byte[]> fields = uncheckedCast(result.get(1));
+    assertThat(fields).containsExactlyInAnyOrder(
+        FIELD_ONE.getBytes(),
+        FIELD_TWO.getBytes(),
+        FIELD_THREE.getBytes());
+  }
+
+  /**** Concurrency ***/
+
+  @Test
+  public void 
should_returnAllConsistentlyPresentMembers_givenConcurrentThreadsAddingAndRemovingMembers()
 {
+    final List<String> initialMemberData = makeSet();
+    jedis.sadd(KEY, initialMemberData.toArray(new 
String[initialMemberData.size()]));
+    final int iterationCount = 500;
+
+    Jedis jedis1 = jedis.getConnectionFromSlot(SLOT_FOR_KEY);
+    Jedis jedis2 = jedis.getConnectionFromSlot(SLOT_FOR_KEY);
+
+    new ConcurrentLoopingThreads(iterationCount,
+        (i) -> multipleSScanAndAssertOnContentOfResultSet(i, jedis1, 
initialMemberData),
+        (i) -> multipleSScanAndAssertOnContentOfResultSet(i, jedis2, 
initialMemberData),
+        (i) -> {
+          String field = "new_" + BASE_FIELD + i;
+          jedis.sadd(KEY, field);
+          jedis.srem(KEY, field);
+        }).run();
+
+    jedis1.close();
+    jedis2.close();
+  }
+
+  @Test
+  public void should_notAlterUnderlyingData_givenMultipleConcurrentSscans() {
+    final List<String> initialMemberData = makeSet();
+    jedis.sadd(KEY, initialMemberData.toArray(new 
String[initialMemberData.size()]));
+    final int iterationCount = 500;
+
+    Jedis jedis1 = jedis.getConnectionFromSlot(SLOT_FOR_KEY);
+    Jedis jedis2 = jedis.getConnectionFromSlot(SLOT_FOR_KEY);
+
+    new ConcurrentLoopingThreads(iterationCount,
+        (i) -> multipleSScanAndAssertOnContentOfResultSet(i, jedis1, 
initialMemberData),
+        (i) -> multipleSScanAndAssertOnContentOfResultSet(i, jedis2, 
initialMemberData))
+            .run();
+
+    initialMemberData
+        .forEach((member) -> assertThat(jedis.sismember(KEY, 
member)).isTrue());
+
+    jedis1.close();
+    jedis2.close();
+  }
+
+  private void multipleSScanAndAssertOnContentOfResultSet(int iteration, Jedis 
jedis,
+      final List<String> initialMemberData) {
+
+    List<String> allEntries = new ArrayList<>();
+    ScanResult<String> result;
+    String cursor = ZERO_CURSOR;
+
+    do {
+      result = jedis.sscan(KEY, cursor);
+      cursor = result.getCursor();
+      List<String> resultEntries = result.getResult();
+      resultEntries
+          .forEach((entry) -> allEntries.add(entry));
+    } while (!result.isCompleteIteration());
+
+    assertThat(allEntries).as("failed on iteration " + iteration)
+        .containsAll(initialMemberData);
+  }
+
+  private void multipleSScanAndAssertOnSizeOfResultSet(Jedis jedis,
+      final List<String> initialMemberData) {
+    List<String> allEntries = new ArrayList<>();
+    ScanResult<String> result;
+    String cursor = ZERO_CURSOR;
+
+    do {
+      result = jedis.sscan(KEY, cursor);
+      cursor = result.getCursor();
+      allEntries.addAll(result.getResult());
+    } while (!result.isCompleteIteration());
+
+    List<String> allDistinctEntries =
+        allEntries
+            .stream()
+            .distinct()
+            .collect(Collectors.toList());
+
+    assertThat(allDistinctEntries.size())
+        .isEqualTo(initialMemberData.size());
+  }
+
+  private List<String> initializeThreeMemberSet() {
+    List<String> set = new ArrayList<>();
+    set.add(FIELD_ONE);
+    set.add(FIELD_TWO);
+    set.add(FIELD_THREE);
+    jedis.sadd(KEY, FIELD_ONE, FIELD_TWO, FIELD_THREE);
+    return set;
+  }
+
+  private List<byte[]> initializeThreeMemberByteSet() {
+    List<byte[]> set = new ArrayList<>();
+    set.add(FIELD_ONE.getBytes());
+    set.add(FIELD_TWO.getBytes());
+    set.add(FIELD_THREE.getBytes());
+    jedis.sadd(KEY.getBytes(), FIELD_ONE.getBytes(), FIELD_TWO.getBytes(), 
FIELD_THREE.getBytes());
+    return set;
+  }
+
+  private List<String> makeSet() {
+    List<String> set = new ArrayList<>();
+    for (int i = 0; i < SIZE_OF_SET; i++) {
+      set.add((BASE_FIELD + i));
+    }
+    return set;
+  }
+
+  private List<byte[]> makeByteSet() {

Review comment:
       This method is never used and should be removed.

##########
File path: 
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSScanIntegrationTest.java
##########
@@ -323,26 +402,204 @@ public void 
givenNegativeCursor_returnsMembersUsingAbsoluteValueOfCursor() {
 
   @Test
   public void givenCursorGreaterThanUnsignedLongCapacity_returnsCursorError() {
-    assertThatThrownBy(() -> jedis.sscan("a", "18446744073709551616"))
-        .hasMessageContaining(ERROR_CURSOR);
+    assertThatThrownBy(
+        () -> jedis.sscan(KEY, 
UNSIGNED_LONG_CAPACITY.add(BigInteger.ONE).toString()))
+            .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void 
givenNegativeCursorGreaterThanUnsignedLongCapacity_returnsCursorError() {
-    assertThatThrownBy(() -> jedis.sscan("a", "-18446744073709551616"))
-        .hasMessageContaining(ERROR_CURSOR);
+    assertThatThrownBy(
+        () -> jedis.sscan(KEY, 
NEGATIVE_LONG_CAPACITY.add(BigInteger.valueOf(-1)).toString()))
+            .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void givenInvalidRegexSyntax_returnsEmptyArray() {
-    jedis.sadd("a", "1");
+    jedis.sadd(KEY, "1");
     ScanParams scanParams = new ScanParams();
     scanParams.count(1);
     scanParams.match("\\p");
 
     ScanResult<byte[]> result =
-        jedis.sscan("a".getBytes(), "0".getBytes(), scanParams);
+        jedis.sscan(KEY.getBytes(), ZERO_CURSOR.getBytes(), scanParams);
 
     assertThat(result.getResult()).isEmpty();
   }
+
+  @Test
+  public void should_notReturnValue_givenValueWasRemovedBeforeSSCANISCalled() {
+    List<String> set = initializeThreeMemberSet();
+
+    jedis.srem(KEY, FIELD_THREE);
+    set.remove(FIELD_THREE);
+
+    GeodeAwaitility.await().untilAsserted(
+        () -> assertThat(jedis.sismember(KEY, FIELD_THREE)).isFalse());
+
+    ScanResult<String> result = jedis.sscan(KEY, ZERO_CURSOR);
+
+    assertThat(result.getResult()).containsExactlyInAnyOrderElementsOf(set);
+  }
+
+  @Test
+  public void should_notErrorGivenNonzeroCursorOnFirstCall() {
+    List<String> set = initializeThreeMemberSet();
+
+    ScanResult<String> result = jedis.sscan(KEY, "5");
+
+    assertThat(result.getResult()).isSubsetOf(set);
+  }
+
+  @Test
+  public void should_notErrorGivenCountEqualToIntegerMaxValue() {
+    List<byte[]> set = initializeThreeMemberByteSet();
+
+    ScanParams scanParams = new ScanParams().count(Integer.MAX_VALUE);
+
+    ScanResult<byte[]> result =
+        jedis.sscan(KEY.getBytes(), ZERO_CURSOR.getBytes(), scanParams);
+    assertThat(result.getResult())
+        .containsExactlyInAnyOrderElementsOf(set);
+  }
+
+  @Test
+  public void should_notErrorGivenCountGreaterThanIntegerMaxValue() {
+    initializeThreeMemberByteSet();
+
+    String greaterThanInt = String.valueOf(2L * Integer.MAX_VALUE);
+    List<Object> result =
+        uncheckedCast(jedis.sendCommand(KEY.getBytes(), Protocol.Command.SSCAN,
+            KEY.getBytes(), ZERO_CURSOR.getBytes(),
+            "COUNT".getBytes(), greaterThanInt.getBytes()));
+
+    assertThat((byte[]) result.get(0)).isEqualTo(ZERO_CURSOR.getBytes());
+
+    List<byte[]> fields = uncheckedCast(result.get(1));
+    assertThat(fields).containsExactlyInAnyOrder(
+        FIELD_ONE.getBytes(),
+        FIELD_TWO.getBytes(),
+        FIELD_THREE.getBytes());
+  }
+
+  /**** Concurrency ***/
+
+  @Test
+  public void 
should_returnAllConsistentlyPresentMembers_givenConcurrentThreadsAddingAndRemovingMembers()
 {
+    final List<String> initialMemberData = makeSet();
+    jedis.sadd(KEY, initialMemberData.toArray(new 
String[initialMemberData.size()]));
+    final int iterationCount = 500;
+
+    Jedis jedis1 = jedis.getConnectionFromSlot(SLOT_FOR_KEY);
+    Jedis jedis2 = jedis.getConnectionFromSlot(SLOT_FOR_KEY);
+
+    new ConcurrentLoopingThreads(iterationCount,
+        (i) -> multipleSScanAndAssertOnContentOfResultSet(i, jedis1, 
initialMemberData),
+        (i) -> multipleSScanAndAssertOnContentOfResultSet(i, jedis2, 
initialMemberData),
+        (i) -> {
+          String field = "new_" + BASE_FIELD + i;
+          jedis.sadd(KEY, field);
+          jedis.srem(KEY, field);
+        }).run();
+
+    jedis1.close();
+    jedis2.close();
+  }
+
+  @Test
+  public void should_notAlterUnderlyingData_givenMultipleConcurrentSscans() {
+    final List<String> initialMemberData = makeSet();
+    jedis.sadd(KEY, initialMemberData.toArray(new 
String[initialMemberData.size()]));
+    final int iterationCount = 500;
+
+    Jedis jedis1 = jedis.getConnectionFromSlot(SLOT_FOR_KEY);
+    Jedis jedis2 = jedis.getConnectionFromSlot(SLOT_FOR_KEY);
+
+    new ConcurrentLoopingThreads(iterationCount,
+        (i) -> multipleSScanAndAssertOnContentOfResultSet(i, jedis1, 
initialMemberData),
+        (i) -> multipleSScanAndAssertOnContentOfResultSet(i, jedis2, 
initialMemberData))
+            .run();
+
+    initialMemberData
+        .forEach((member) -> assertThat(jedis.sismember(KEY, 
member)).isTrue());

Review comment:
       This would be better as:
   ```
   
assertThat(jedis.smembers(KEY)).containsExactlyInAnyOrderElementsOf(initialMemberData);
   ```
   as we're currently only asserting that all the initial members are still 
present, but not that we haven't added any extra members somehow.

##########
File path: 
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSScanIntegrationTest.java
##########
@@ -156,152 +222,172 @@ public void 
givenNonexistentKey_andCursorIsNotInteger_returnsInvalidCursorError(
 
   @Test
   public void 
givenExistentSetKey_andCursorIsNotAnInteger_returnsInvalidCursorError() {
-    jedis.set("a", "b");
+    jedis.set(KEY, "b");
 
-    assertThatThrownBy(() -> jedis.sendCommand("a", Protocol.Command.SSCAN, 
"a", "sjfls"))
+    assertThatThrownBy(() -> jedis.sendCommand(KEY, Protocol.Command.SSCAN, 
KEY, "sjfls"))
         .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void givenNonexistentKey_returnsEmptyArray() {
-    ScanResult<String> result = jedis.sscan("nonexistent", "0");
+    ScanResult<String> result = jedis.sscan("nonexistent", ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
     assertThat(result.getResult()).isEmpty();
   }
 
+  @Test
+  public void givenNegativeCursor_returnsEntriesUsingAbsoluteValueOfCursor() {
+    List<String> set = initializeThreeMemberSet();
+
+    String cursor = "-100";
+    ScanResult<String> result;
+    List<String> allEntries = new ArrayList<>();
+
+    do {
+      result = jedis.sscan(KEY, cursor);
+      allEntries.addAll(result.getResult());
+      cursor = result.getCursor();
+    } while (!result.isCompleteIteration());
+
+    assertThat(allEntries).hasSize(3);
+    assertThat(allEntries).containsExactlyInAnyOrderElementsOf(set);
+  }
+
   @Test
   public void givenSetWithOneMember_returnsMember() {
-    jedis.sadd("a", "1");
-    ScanResult<String> result = jedis.sscan("a", "0");
+    jedis.sadd(KEY, FIELD_ONE);
+    ScanResult<String> result = jedis.sscan(KEY, ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
-    assertThat(result.getResult()).containsExactly("1");
+    assertThat(result.getResult()).containsExactly(FIELD_ONE);
   }
 
   @Test
   public void givenSetWithMultipleMembers_returnsAllMembers() {
-    jedis.sadd("a", "1", "2", "3");
-    ScanResult<String> result = jedis.sscan("a", "0");
+    jedis.sadd(KEY, FIELD_ONE, FIELD_TWO, FIELD_THREE);
+    ScanResult<String> result = jedis.sscan(KEY, ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
-    assertThat(result.getResult()).containsExactlyInAnyOrder("1", "2", "3");
+    assertThat(result.getResult()).containsExactlyInAnyOrder(FIELD_ONE, 
FIELD_TWO, FIELD_THREE);
   }
 
   @Test
   public void givenCount_returnsAllMembersWithoutDuplicates() {
-    jedis.sadd("a", "1", "2", "3");
+    jedis.sadd(KEY, FIELD_ONE, FIELD_TWO, FIELD_THREE);
 
     ScanParams scanParams = new ScanParams();
     scanParams.count(1);
-    String cursor = "0";
+    String cursor = ZERO_CURSOR;
     ScanResult<byte[]> result;
     List<byte[]> allMembersFromScan = new ArrayList<>();
 
     do {
-      result = jedis.sscan("a".getBytes(), cursor.getBytes(), scanParams);
+      result = jedis.sscan(KEY.getBytes(), cursor.getBytes(), scanParams);
       allMembersFromScan.addAll(result.getResult());
       cursor = result.getCursor();
     } while (!result.isCompleteIteration());
 
-    assertThat(allMembersFromScan).containsExactlyInAnyOrder("1".getBytes(),
-        "2".getBytes(),
-        "3".getBytes());
+    
assertThat(allMembersFromScan).containsExactlyInAnyOrder(FIELD_ONE.getBytes(),
+        FIELD_TWO.getBytes(),
+        FIELD_THREE.getBytes());
   }
 
   @Test
   @SuppressWarnings("unchecked")
   public void givenMultipleCounts_returnsAllEntriesWithoutDuplicates() {

Review comment:
       > This test is flawed, as for small set sizes, native Redis will return 
all elements in one SSCAN regardless of the value of COUNT, and duplicate 
elements are allowed. A significant improvement would be to populate a set with 
1000 elements, then call SSCAN once and assert that the number of elements 
returned is greater than or equal to the appropriate COUNT value and that they 
are a subset of the total set contents.
   
   This comment still applies

##########
File path: 
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSScanIntegrationTest.java
##########
@@ -53,97 +73,143 @@ public void tearDown() {
     jedis.close();
   }
 
+  @Test
+  public void givenLessThanTwoArguments_returnsWrongNumberOfArgumentsError() {
+    assertAtLeastNArgs(jedis, Protocol.Command.SSCAN, 2);
+  }
+
   @Test
   public void givenNoKeyArgument_returnsWrongNumberOfArgumentsError() {
-    assertThatThrownBy(() -> jedis.sendCommand("key", Protocol.Command.SSCAN))
+    assertThatThrownBy(() -> jedis.sendCommand(KEY, Protocol.Command.SSCAN))
         .hasMessageContaining("ERR wrong number of arguments for 'sscan' 
command");
   }
 
   @Test
   public void givenNoCursorArgument_returnsWrongNumberOfArgumentsError() {
-    assertThatThrownBy(() -> jedis.sendCommand("key", Protocol.Command.SSCAN, 
"key"))
+    assertThatThrownBy(() -> jedis.sendCommand(KEY, Protocol.Command.SSCAN, 
KEY))
         .hasMessageContaining("ERR wrong number of arguments for 'sscan' 
command");
   }
 
   @Test
   public void givenArgumentsAreNotOddAndKeyExists_returnsSyntaxError() {

Review comment:
       >This test name could be more accurate as 
"givenIncorrectOptionalArgumentAndKeyExists_returnsSyntaxError" since the 
number of arguments isn't actually the relevant factor here, just that there is 
a syntax error of some kind.
   
   This comment still applies. This test name should be changed.

##########
File path: 
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSScanIntegrationTest.java
##########
@@ -156,152 +222,172 @@ public void 
givenNonexistentKey_andCursorIsNotInteger_returnsInvalidCursorError(
 
   @Test
   public void 
givenExistentSetKey_andCursorIsNotAnInteger_returnsInvalidCursorError() {
-    jedis.set("a", "b");
+    jedis.set(KEY, "b");
 
-    assertThatThrownBy(() -> jedis.sendCommand("a", Protocol.Command.SSCAN, 
"a", "sjfls"))
+    assertThatThrownBy(() -> jedis.sendCommand(KEY, Protocol.Command.SSCAN, 
KEY, "sjfls"))
         .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void givenNonexistentKey_returnsEmptyArray() {
-    ScanResult<String> result = jedis.sscan("nonexistent", "0");
+    ScanResult<String> result = jedis.sscan("nonexistent", ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
     assertThat(result.getResult()).isEmpty();
   }
 
+  @Test
+  public void givenNegativeCursor_returnsEntriesUsingAbsoluteValueOfCursor() {
+    List<String> set = initializeThreeMemberSet();
+
+    String cursor = "-100";
+    ScanResult<String> result;
+    List<String> allEntries = new ArrayList<>();
+
+    do {
+      result = jedis.sscan(KEY, cursor);
+      allEntries.addAll(result.getResult());
+      cursor = result.getCursor();
+    } while (!result.isCompleteIteration());
+
+    assertThat(allEntries).hasSize(3);
+    assertThat(allEntries).containsExactlyInAnyOrderElementsOf(set);
+  }
+
   @Test
   public void givenSetWithOneMember_returnsMember() {
-    jedis.sadd("a", "1");
-    ScanResult<String> result = jedis.sscan("a", "0");
+    jedis.sadd(KEY, FIELD_ONE);
+    ScanResult<String> result = jedis.sscan(KEY, ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
-    assertThat(result.getResult()).containsExactly("1");
+    assertThat(result.getResult()).containsExactly(FIELD_ONE);
   }
 
   @Test
   public void givenSetWithMultipleMembers_returnsAllMembers() {
-    jedis.sadd("a", "1", "2", "3");
-    ScanResult<String> result = jedis.sscan("a", "0");
+    jedis.sadd(KEY, FIELD_ONE, FIELD_TWO, FIELD_THREE);
+    ScanResult<String> result = jedis.sscan(KEY, ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
-    assertThat(result.getResult()).containsExactlyInAnyOrder("1", "2", "3");
+    assertThat(result.getResult()).containsExactlyInAnyOrder(FIELD_ONE, 
FIELD_TWO, FIELD_THREE);
   }
 
   @Test
   public void givenCount_returnsAllMembersWithoutDuplicates() {

Review comment:
       > This test is flawed, as for small set sizes, native Redis will return 
all elements in one SSCAN regardless of the value of COUNT, and duplicate 
elements are allowed. A significant improvement would be to populate a set with 
1000 elements, then call SSCAN once and assert that the number of elements 
returned is greater than or equal to the appropriate COUNT value and that they 
are a subset of the total set contents.
   
   This comment still applies

##########
File path: 
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSScanIntegrationTest.java
##########
@@ -323,26 +402,204 @@ public void 
givenNegativeCursor_returnsMembersUsingAbsoluteValueOfCursor() {
 
   @Test
   public void givenCursorGreaterThanUnsignedLongCapacity_returnsCursorError() {
-    assertThatThrownBy(() -> jedis.sscan("a", "18446744073709551616"))
-        .hasMessageContaining(ERROR_CURSOR);
+    assertThatThrownBy(
+        () -> jedis.sscan(KEY, 
UNSIGNED_LONG_CAPACITY.add(BigInteger.ONE).toString()))
+            .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void 
givenNegativeCursorGreaterThanUnsignedLongCapacity_returnsCursorError() {
-    assertThatThrownBy(() -> jedis.sscan("a", "-18446744073709551616"))
-        .hasMessageContaining(ERROR_CURSOR);
+    assertThatThrownBy(
+        () -> jedis.sscan(KEY, 
NEGATIVE_LONG_CAPACITY.add(BigInteger.valueOf(-1)).toString()))
+            .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void givenInvalidRegexSyntax_returnsEmptyArray() {
-    jedis.sadd("a", "1");
+    jedis.sadd(KEY, "1");
     ScanParams scanParams = new ScanParams();
     scanParams.count(1);
     scanParams.match("\\p");
 
     ScanResult<byte[]> result =
-        jedis.sscan("a".getBytes(), "0".getBytes(), scanParams);
+        jedis.sscan(KEY.getBytes(), ZERO_CURSOR.getBytes(), scanParams);
 
     assertThat(result.getResult()).isEmpty();
   }
+
+  @Test
+  public void should_notReturnValue_givenValueWasRemovedBeforeSSCANISCalled() {
+    List<String> set = initializeThreeMemberSet();
+
+    jedis.srem(KEY, FIELD_THREE);
+    set.remove(FIELD_THREE);
+
+    GeodeAwaitility.await().untilAsserted(
+        () -> assertThat(jedis.sismember(KEY, FIELD_THREE)).isFalse());
+
+    ScanResult<String> result = jedis.sscan(KEY, ZERO_CURSOR);
+
+    assertThat(result.getResult()).containsExactlyInAnyOrderElementsOf(set);
+  }
+
+  @Test
+  public void should_notErrorGivenNonzeroCursorOnFirstCall() {
+    List<String> set = initializeThreeMemberSet();
+
+    ScanResult<String> result = jedis.sscan(KEY, "5");
+
+    assertThat(result.getResult()).isSubsetOf(set);
+  }
+
+  @Test
+  public void should_notErrorGivenCountEqualToIntegerMaxValue() {
+    List<byte[]> set = initializeThreeMemberByteSet();
+
+    ScanParams scanParams = new ScanParams().count(Integer.MAX_VALUE);
+
+    ScanResult<byte[]> result =
+        jedis.sscan(KEY.getBytes(), ZERO_CURSOR.getBytes(), scanParams);
+    assertThat(result.getResult())
+        .containsExactlyInAnyOrderElementsOf(set);
+  }
+
+  @Test
+  public void should_notErrorGivenCountGreaterThanIntegerMaxValue() {
+    initializeThreeMemberByteSet();
+
+    String greaterThanInt = String.valueOf(2L * Integer.MAX_VALUE);
+    List<Object> result =
+        uncheckedCast(jedis.sendCommand(KEY.getBytes(), Protocol.Command.SSCAN,
+            KEY.getBytes(), ZERO_CURSOR.getBytes(),
+            "COUNT".getBytes(), greaterThanInt.getBytes()));
+
+    assertThat((byte[]) result.get(0)).isEqualTo(ZERO_CURSOR.getBytes());
+
+    List<byte[]> fields = uncheckedCast(result.get(1));
+    assertThat(fields).containsExactlyInAnyOrder(
+        FIELD_ONE.getBytes(),
+        FIELD_TWO.getBytes(),
+        FIELD_THREE.getBytes());
+  }
+
+  /**** Concurrency ***/
+
+  @Test
+  public void 
should_returnAllConsistentlyPresentMembers_givenConcurrentThreadsAddingAndRemovingMembers()
 {
+    final List<String> initialMemberData = makeSet();
+    jedis.sadd(KEY, initialMemberData.toArray(new 
String[initialMemberData.size()]));
+    final int iterationCount = 500;
+
+    Jedis jedis1 = jedis.getConnectionFromSlot(SLOT_FOR_KEY);
+    Jedis jedis2 = jedis.getConnectionFromSlot(SLOT_FOR_KEY);
+
+    new ConcurrentLoopingThreads(iterationCount,
+        (i) -> multipleSScanAndAssertOnContentOfResultSet(i, jedis1, 
initialMemberData),
+        (i) -> multipleSScanAndAssertOnContentOfResultSet(i, jedis2, 
initialMemberData),
+        (i) -> {
+          String field = "new_" + BASE_FIELD + i;
+          jedis.sadd(KEY, field);
+          jedis.srem(KEY, field);
+        }).run();
+
+    jedis1.close();
+    jedis2.close();
+  }
+
+  @Test
+  public void should_notAlterUnderlyingData_givenMultipleConcurrentSscans() {
+    final List<String> initialMemberData = makeSet();
+    jedis.sadd(KEY, initialMemberData.toArray(new 
String[initialMemberData.size()]));
+    final int iterationCount = 500;
+
+    Jedis jedis1 = jedis.getConnectionFromSlot(SLOT_FOR_KEY);
+    Jedis jedis2 = jedis.getConnectionFromSlot(SLOT_FOR_KEY);
+
+    new ConcurrentLoopingThreads(iterationCount,
+        (i) -> multipleSScanAndAssertOnContentOfResultSet(i, jedis1, 
initialMemberData),
+        (i) -> multipleSScanAndAssertOnContentOfResultSet(i, jedis2, 
initialMemberData))
+            .run();
+
+    initialMemberData
+        .forEach((member) -> assertThat(jedis.sismember(KEY, 
member)).isTrue());
+
+    jedis1.close();
+    jedis2.close();
+  }
+
+  private void multipleSScanAndAssertOnContentOfResultSet(int iteration, Jedis 
jedis,
+      final List<String> initialMemberData) {
+
+    List<String> allEntries = new ArrayList<>();
+    ScanResult<String> result;
+    String cursor = ZERO_CURSOR;
+
+    do {
+      result = jedis.sscan(KEY, cursor);
+      cursor = result.getCursor();
+      List<String> resultEntries = result.getResult();
+      resultEntries
+          .forEach((entry) -> allEntries.add(entry));

Review comment:
       This can be simplified to:
   ```
   allEntries.addAll(result.getResult());
   ```

##########
File path: 
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSScanIntegrationTest.java
##########
@@ -156,152 +222,172 @@ public void 
givenNonexistentKey_andCursorIsNotInteger_returnsInvalidCursorError(
 
   @Test
   public void 
givenExistentSetKey_andCursorIsNotAnInteger_returnsInvalidCursorError() {
-    jedis.set("a", "b");
+    jedis.set(KEY, "b");
 
-    assertThatThrownBy(() -> jedis.sendCommand("a", Protocol.Command.SSCAN, 
"a", "sjfls"))
+    assertThatThrownBy(() -> jedis.sendCommand(KEY, Protocol.Command.SSCAN, 
KEY, "sjfls"))
         .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void givenNonexistentKey_returnsEmptyArray() {
-    ScanResult<String> result = jedis.sscan("nonexistent", "0");
+    ScanResult<String> result = jedis.sscan("nonexistent", ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
     assertThat(result.getResult()).isEmpty();
   }
 
+  @Test
+  public void givenNegativeCursor_returnsEntriesUsingAbsoluteValueOfCursor() {
+    List<String> set = initializeThreeMemberSet();
+
+    String cursor = "-100";
+    ScanResult<String> result;
+    List<String> allEntries = new ArrayList<>();
+
+    do {
+      result = jedis.sscan(KEY, cursor);
+      allEntries.addAll(result.getResult());
+      cursor = result.getCursor();
+    } while (!result.isCompleteIteration());
+
+    assertThat(allEntries).hasSize(3);
+    assertThat(allEntries).containsExactlyInAnyOrderElementsOf(set);
+  }
+
   @Test
   public void givenSetWithOneMember_returnsMember() {
-    jedis.sadd("a", "1");
-    ScanResult<String> result = jedis.sscan("a", "0");
+    jedis.sadd(KEY, FIELD_ONE);
+    ScanResult<String> result = jedis.sscan(KEY, ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
-    assertThat(result.getResult()).containsExactly("1");
+    assertThat(result.getResult()).containsExactly(FIELD_ONE);
   }
 
   @Test
   public void givenSetWithMultipleMembers_returnsAllMembers() {
-    jedis.sadd("a", "1", "2", "3");
-    ScanResult<String> result = jedis.sscan("a", "0");
+    jedis.sadd(KEY, FIELD_ONE, FIELD_TWO, FIELD_THREE);
+    ScanResult<String> result = jedis.sscan(KEY, ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
-    assertThat(result.getResult()).containsExactlyInAnyOrder("1", "2", "3");
+    assertThat(result.getResult()).containsExactlyInAnyOrder(FIELD_ONE, 
FIELD_TWO, FIELD_THREE);
   }
 
   @Test
   public void givenCount_returnsAllMembersWithoutDuplicates() {
-    jedis.sadd("a", "1", "2", "3");
+    jedis.sadd(KEY, FIELD_ONE, FIELD_TWO, FIELD_THREE);
 
     ScanParams scanParams = new ScanParams();
     scanParams.count(1);
-    String cursor = "0";
+    String cursor = ZERO_CURSOR;
     ScanResult<byte[]> result;
     List<byte[]> allMembersFromScan = new ArrayList<>();
 
     do {
-      result = jedis.sscan("a".getBytes(), cursor.getBytes(), scanParams);
+      result = jedis.sscan(KEY.getBytes(), cursor.getBytes(), scanParams);
       allMembersFromScan.addAll(result.getResult());
       cursor = result.getCursor();
     } while (!result.isCompleteIteration());
 
-    assertThat(allMembersFromScan).containsExactlyInAnyOrder("1".getBytes(),
-        "2".getBytes(),
-        "3".getBytes());
+    
assertThat(allMembersFromScan).containsExactlyInAnyOrder(FIELD_ONE.getBytes(),
+        FIELD_TWO.getBytes(),
+        FIELD_THREE.getBytes());
   }
 
   @Test
   @SuppressWarnings("unchecked")
   public void givenMultipleCounts_returnsAllEntriesWithoutDuplicates() {
-    jedis.sadd("a", "1", "12", "3");
+    jedis.sadd(KEY, FIELD_ONE, FIELD_TWO, FIELD_THREE);
 
     List<Object> result;
 
     List<byte[]> allEntries = new ArrayList<>();
-    String cursor = "0";
+    String cursor = ZERO_CURSOR;
 
     do {
       result =
-          (List<Object>) jedis.sendCommand("a", Protocol.Command.SSCAN, "a", 
cursor, "COUNT", "2",
-              "COUNT", "1");
+          (List<Object>) jedis.sendCommand(KEY, Protocol.Command.SSCAN, KEY, 
cursor, "COUNT",
+              FIELD_TWO,
+              "COUNT", FIELD_ONE);
       allEntries.addAll((List<byte[]>) result.get(1));
       cursor = new String((byte[]) result.get(0));
-    } while (!Arrays.equals((byte[]) result.get(0), "0".getBytes()));
+    } while (!Arrays.equals((byte[]) result.get(0), ZERO_CURSOR.getBytes()));
 
-    assertThat((byte[]) result.get(0)).isEqualTo("0".getBytes());
-    assertThat(allEntries).containsExactlyInAnyOrder("1".getBytes(),
-        "12".getBytes(),
-        "3".getBytes());
+    assertThat((byte[]) result.get(0)).isEqualTo(ZERO_CURSOR.getBytes());
+    assertThat(allEntries).containsExactlyInAnyOrder(FIELD_ONE.getBytes(),
+        FIELD_TWO.getBytes(),
+        FIELD_THREE.getBytes());
   }
 
   @Test
   public void givenMatch_returnsAllMatchingMembersWithoutDuplicates() {
-    jedis.sadd("a", "1", "12", "3");
+    jedis.sadd(KEY, FIELD_ONE, FIELD_TWO, FIELD_THREE);
 
     ScanParams scanParams = new ScanParams();
     scanParams.match("1*");
 
     ScanResult<byte[]> result =
-        jedis.sscan("a".getBytes(), "0".getBytes(), scanParams);
+        jedis.sscan(KEY.getBytes(), ZERO_CURSOR.getBytes(), scanParams);
 
     assertThat(result.isCompleteIteration()).isTrue();
-    assertThat(result.getResult()).containsExactlyInAnyOrder("1".getBytes(),
-        "12".getBytes());
+    
assertThat(result.getResult()).containsExactlyInAnyOrder(FIELD_ONE.getBytes(),
+        FIELD_TWO.getBytes());
   }
 
   @Test
   @SuppressWarnings("unchecked")
   public void givenMultipleMatches_returnsMembersMatchingLastMatchParameter() {
-    jedis.sadd("a", "1", "12", "3");
+    jedis.sadd(KEY, FIELD_ONE, FIELD_TWO, FIELD_THREE);
 
-    List<Object> result = (List<Object>) jedis.sendCommand("a", 
Protocol.Command.SSCAN, "a", "0",
-        "MATCH", "3*", "MATCH", "1*");
+    List<Object> result =
+        (List<Object>) jedis.sendCommand(KEY, Protocol.Command.SSCAN, KEY, 
ZERO_CURSOR,
+            "MATCH", "3*", "MATCH", "1*");
 
-    assertThat((byte[]) result.get(0)).isEqualTo("0".getBytes());
-    assertThat((List<byte[]>) 
result.get(1)).containsExactlyInAnyOrder("1".getBytes(),
-        "12".getBytes());
+    assertThat((byte[]) result.get(0)).isEqualTo(ZERO_CURSOR.getBytes());
+    assertThat((List<byte[]>) 
result.get(1)).containsExactlyInAnyOrder(FIELD_ONE.getBytes(),
+        FIELD_TWO.getBytes());
   }
 
   @Test
   public void givenMatchAndCount_returnsAllMembersWithoutDuplicates() {
-    jedis.sadd("a", "1", "12", "3");
+    jedis.sadd(KEY, FIELD_ONE, FIELD_TWO, FIELD_THREE);
 
     ScanParams scanParams = new ScanParams();
     scanParams.count(1);
     scanParams.match("1*");
     ScanResult<byte[]> result;
     List<byte[]> allMembersFromScan = new ArrayList<>();
-    String cursor = "0";
+    String cursor = ZERO_CURSOR;
 
     do {
-      result = jedis.sscan("a".getBytes(), cursor.getBytes(), scanParams);
+      result = jedis.sscan(KEY.getBytes(), cursor.getBytes(), scanParams);
       allMembersFromScan.addAll(result.getResult());
       cursor = result.getCursor();
     } while (!result.isCompleteIteration());
 
-    assertThat(allMembersFromScan).containsExactlyInAnyOrder("1".getBytes(),
-        "12".getBytes());
+    
assertThat(allMembersFromScan).containsExactlyInAnyOrder(FIELD_ONE.getBytes(),
+        FIELD_TWO.getBytes());
   }
 
   @Test
   @SuppressWarnings("unchecked")
   public void 
givenMultipleCountsAndMatches_returnsAllEntriesWithoutDuplicates() {

Review comment:
       > This test would be better named 
"givenSetWithThreeMembersAndMultipleMatchAndCountArguments_returnsAllMatchingMembers"
 with the assertions changed to `containsOnly()` to allow duplicate entries.
   
   This comment still applies

##########
File path: 
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSScanIntegrationTest.java
##########
@@ -156,152 +222,172 @@ public void 
givenNonexistentKey_andCursorIsNotInteger_returnsInvalidCursorError(
 
   @Test
   public void 
givenExistentSetKey_andCursorIsNotAnInteger_returnsInvalidCursorError() {
-    jedis.set("a", "b");
+    jedis.set(KEY, "b");
 
-    assertThatThrownBy(() -> jedis.sendCommand("a", Protocol.Command.SSCAN, 
"a", "sjfls"))
+    assertThatThrownBy(() -> jedis.sendCommand(KEY, Protocol.Command.SSCAN, 
KEY, "sjfls"))
         .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void givenNonexistentKey_returnsEmptyArray() {
-    ScanResult<String> result = jedis.sscan("nonexistent", "0");
+    ScanResult<String> result = jedis.sscan("nonexistent", ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
     assertThat(result.getResult()).isEmpty();
   }
 
+  @Test
+  public void givenNegativeCursor_returnsEntriesUsingAbsoluteValueOfCursor() {
+    List<String> set = initializeThreeMemberSet();
+
+    String cursor = "-100";
+    ScanResult<String> result;
+    List<String> allEntries = new ArrayList<>();
+
+    do {
+      result = jedis.sscan(KEY, cursor);
+      allEntries.addAll(result.getResult());
+      cursor = result.getCursor();
+    } while (!result.isCompleteIteration());
+
+    assertThat(allEntries).hasSize(3);
+    assertThat(allEntries).containsExactlyInAnyOrderElementsOf(set);
+  }
+
   @Test
   public void givenSetWithOneMember_returnsMember() {
-    jedis.sadd("a", "1");
-    ScanResult<String> result = jedis.sscan("a", "0");
+    jedis.sadd(KEY, FIELD_ONE);
+    ScanResult<String> result = jedis.sscan(KEY, ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
-    assertThat(result.getResult()).containsExactly("1");
+    assertThat(result.getResult()).containsExactly(FIELD_ONE);
   }
 
   @Test
   public void givenSetWithMultipleMembers_returnsAllMembers() {
-    jedis.sadd("a", "1", "2", "3");
-    ScanResult<String> result = jedis.sscan("a", "0");
+    jedis.sadd(KEY, FIELD_ONE, FIELD_TWO, FIELD_THREE);
+    ScanResult<String> result = jedis.sscan(KEY, ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
-    assertThat(result.getResult()).containsExactlyInAnyOrder("1", "2", "3");
+    assertThat(result.getResult()).containsExactlyInAnyOrder(FIELD_ONE, 
FIELD_TWO, FIELD_THREE);
   }
 
   @Test
   public void givenCount_returnsAllMembersWithoutDuplicates() {
-    jedis.sadd("a", "1", "2", "3");
+    jedis.sadd(KEY, FIELD_ONE, FIELD_TWO, FIELD_THREE);
 
     ScanParams scanParams = new ScanParams();
     scanParams.count(1);
-    String cursor = "0";
+    String cursor = ZERO_CURSOR;
     ScanResult<byte[]> result;
     List<byte[]> allMembersFromScan = new ArrayList<>();
 
     do {
-      result = jedis.sscan("a".getBytes(), cursor.getBytes(), scanParams);
+      result = jedis.sscan(KEY.getBytes(), cursor.getBytes(), scanParams);
       allMembersFromScan.addAll(result.getResult());
       cursor = result.getCursor();
     } while (!result.isCompleteIteration());
 
-    assertThat(allMembersFromScan).containsExactlyInAnyOrder("1".getBytes(),
-        "2".getBytes(),
-        "3".getBytes());
+    
assertThat(allMembersFromScan).containsExactlyInAnyOrder(FIELD_ONE.getBytes(),
+        FIELD_TWO.getBytes(),
+        FIELD_THREE.getBytes());
   }
 
   @Test
   @SuppressWarnings("unchecked")
   public void givenMultipleCounts_returnsAllEntriesWithoutDuplicates() {
-    jedis.sadd("a", "1", "12", "3");
+    jedis.sadd(KEY, FIELD_ONE, FIELD_TWO, FIELD_THREE);
 
     List<Object> result;
 
     List<byte[]> allEntries = new ArrayList<>();
-    String cursor = "0";
+    String cursor = ZERO_CURSOR;
 
     do {
       result =
-          (List<Object>) jedis.sendCommand("a", Protocol.Command.SSCAN, "a", 
cursor, "COUNT", "2",
-              "COUNT", "1");
+          (List<Object>) jedis.sendCommand(KEY, Protocol.Command.SSCAN, KEY, 
cursor, "COUNT",
+              FIELD_TWO,
+              "COUNT", FIELD_ONE);
       allEntries.addAll((List<byte[]>) result.get(1));
       cursor = new String((byte[]) result.get(0));
-    } while (!Arrays.equals((byte[]) result.get(0), "0".getBytes()));
+    } while (!Arrays.equals((byte[]) result.get(0), ZERO_CURSOR.getBytes()));
 
-    assertThat((byte[]) result.get(0)).isEqualTo("0".getBytes());
-    assertThat(allEntries).containsExactlyInAnyOrder("1".getBytes(),
-        "12".getBytes(),
-        "3".getBytes());
+    assertThat((byte[]) result.get(0)).isEqualTo(ZERO_CURSOR.getBytes());
+    assertThat(allEntries).containsExactlyInAnyOrder(FIELD_ONE.getBytes(),
+        FIELD_TWO.getBytes(),
+        FIELD_THREE.getBytes());
   }
 
   @Test
   public void givenMatch_returnsAllMatchingMembersWithoutDuplicates() {
-    jedis.sadd("a", "1", "12", "3");
+    jedis.sadd(KEY, FIELD_ONE, FIELD_TWO, FIELD_THREE);
 
     ScanParams scanParams = new ScanParams();
     scanParams.match("1*");
 
     ScanResult<byte[]> result =
-        jedis.sscan("a".getBytes(), "0".getBytes(), scanParams);
+        jedis.sscan(KEY.getBytes(), ZERO_CURSOR.getBytes(), scanParams);
 
     assertThat(result.isCompleteIteration()).isTrue();
-    assertThat(result.getResult()).containsExactlyInAnyOrder("1".getBytes(),
-        "12".getBytes());
+    
assertThat(result.getResult()).containsExactlyInAnyOrder(FIELD_ONE.getBytes(),
+        FIELD_TWO.getBytes());
   }
 
   @Test
   @SuppressWarnings("unchecked")
   public void givenMultipleMatches_returnsMembersMatchingLastMatchParameter() {

Review comment:
       > This test is also flawed for the same reasons as earlier tests. A 
better name would be 
"givenSetWithThreeEntriesAndMultipleMatchArguments_returnsOnlyElementsMatchingLastMatchArgument"
 with the assertions changed to `containsOnly()` to allow duplicate entries.
   
   This comment still applies

##########
File path: 
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSScanIntegrationTest.java
##########
@@ -156,152 +222,172 @@ public void 
givenNonexistentKey_andCursorIsNotInteger_returnsInvalidCursorError(
 
   @Test
   public void 
givenExistentSetKey_andCursorIsNotAnInteger_returnsInvalidCursorError() {
-    jedis.set("a", "b");
+    jedis.set(KEY, "b");
 
-    assertThatThrownBy(() -> jedis.sendCommand("a", Protocol.Command.SSCAN, 
"a", "sjfls"))
+    assertThatThrownBy(() -> jedis.sendCommand(KEY, Protocol.Command.SSCAN, 
KEY, "sjfls"))
         .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void givenNonexistentKey_returnsEmptyArray() {
-    ScanResult<String> result = jedis.sscan("nonexistent", "0");
+    ScanResult<String> result = jedis.sscan("nonexistent", ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
     assertThat(result.getResult()).isEmpty();
   }
 
+  @Test
+  public void givenNegativeCursor_returnsEntriesUsingAbsoluteValueOfCursor() {
+    List<String> set = initializeThreeMemberSet();
+
+    String cursor = "-100";
+    ScanResult<String> result;
+    List<String> allEntries = new ArrayList<>();
+
+    do {
+      result = jedis.sscan(KEY, cursor);
+      allEntries.addAll(result.getResult());
+      cursor = result.getCursor();
+    } while (!result.isCompleteIteration());
+
+    assertThat(allEntries).hasSize(3);
+    assertThat(allEntries).containsExactlyInAnyOrderElementsOf(set);
+  }
+
   @Test
   public void givenSetWithOneMember_returnsMember() {
-    jedis.sadd("a", "1");
-    ScanResult<String> result = jedis.sscan("a", "0");
+    jedis.sadd(KEY, FIELD_ONE);
+    ScanResult<String> result = jedis.sscan(KEY, ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
-    assertThat(result.getResult()).containsExactly("1");
+    assertThat(result.getResult()).containsExactly(FIELD_ONE);
   }
 
   @Test
   public void givenSetWithMultipleMembers_returnsAllMembers() {
-    jedis.sadd("a", "1", "2", "3");
-    ScanResult<String> result = jedis.sscan("a", "0");
+    jedis.sadd(KEY, FIELD_ONE, FIELD_TWO, FIELD_THREE);
+    ScanResult<String> result = jedis.sscan(KEY, ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
-    assertThat(result.getResult()).containsExactlyInAnyOrder("1", "2", "3");
+    assertThat(result.getResult()).containsExactlyInAnyOrder(FIELD_ONE, 
FIELD_TWO, FIELD_THREE);
   }
 
   @Test
   public void givenCount_returnsAllMembersWithoutDuplicates() {
-    jedis.sadd("a", "1", "2", "3");
+    jedis.sadd(KEY, FIELD_ONE, FIELD_TWO, FIELD_THREE);
 
     ScanParams scanParams = new ScanParams();
     scanParams.count(1);
-    String cursor = "0";
+    String cursor = ZERO_CURSOR;
     ScanResult<byte[]> result;
     List<byte[]> allMembersFromScan = new ArrayList<>();
 
     do {
-      result = jedis.sscan("a".getBytes(), cursor.getBytes(), scanParams);
+      result = jedis.sscan(KEY.getBytes(), cursor.getBytes(), scanParams);
       allMembersFromScan.addAll(result.getResult());
       cursor = result.getCursor();
     } while (!result.isCompleteIteration());
 
-    assertThat(allMembersFromScan).containsExactlyInAnyOrder("1".getBytes(),
-        "2".getBytes(),
-        "3".getBytes());
+    
assertThat(allMembersFromScan).containsExactlyInAnyOrder(FIELD_ONE.getBytes(),
+        FIELD_TWO.getBytes(),
+        FIELD_THREE.getBytes());
   }
 
   @Test
   @SuppressWarnings("unchecked")
   public void givenMultipleCounts_returnsAllEntriesWithoutDuplicates() {
-    jedis.sadd("a", "1", "12", "3");
+    jedis.sadd(KEY, FIELD_ONE, FIELD_TWO, FIELD_THREE);
 
     List<Object> result;
 
     List<byte[]> allEntries = new ArrayList<>();
-    String cursor = "0";
+    String cursor = ZERO_CURSOR;
 
     do {
       result =
-          (List<Object>) jedis.sendCommand("a", Protocol.Command.SSCAN, "a", 
cursor, "COUNT", "2",
-              "COUNT", "1");
+          (List<Object>) jedis.sendCommand(KEY, Protocol.Command.SSCAN, KEY, 
cursor, "COUNT",
+              FIELD_TWO,
+              "COUNT", FIELD_ONE);
       allEntries.addAll((List<byte[]>) result.get(1));
       cursor = new String((byte[]) result.get(0));
-    } while (!Arrays.equals((byte[]) result.get(0), "0".getBytes()));
+    } while (!Arrays.equals((byte[]) result.get(0), ZERO_CURSOR.getBytes()));
 
-    assertThat((byte[]) result.get(0)).isEqualTo("0".getBytes());
-    assertThat(allEntries).containsExactlyInAnyOrder("1".getBytes(),
-        "12".getBytes(),
-        "3".getBytes());
+    assertThat((byte[]) result.get(0)).isEqualTo(ZERO_CURSOR.getBytes());
+    assertThat(allEntries).containsExactlyInAnyOrder(FIELD_ONE.getBytes(),
+        FIELD_TWO.getBytes(),
+        FIELD_THREE.getBytes());
   }
 
   @Test
   public void givenMatch_returnsAllMatchingMembersWithoutDuplicates() {

Review comment:
       > This test is also flawed for the same reasons as earlier tests. A 
better name would be 
"givenSetWithThreeEntriesAndMatch_returnsOnlyMatchingElements" with the 
assertions changed to `containsOnly()` to allow duplicate entries.
   
   This comment still applies

##########
File path: 
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSScanIntegrationTest.java
##########
@@ -156,152 +222,172 @@ public void 
givenNonexistentKey_andCursorIsNotInteger_returnsInvalidCursorError(
 
   @Test
   public void 
givenExistentSetKey_andCursorIsNotAnInteger_returnsInvalidCursorError() {
-    jedis.set("a", "b");
+    jedis.set(KEY, "b");
 
-    assertThatThrownBy(() -> jedis.sendCommand("a", Protocol.Command.SSCAN, 
"a", "sjfls"))
+    assertThatThrownBy(() -> jedis.sendCommand(KEY, Protocol.Command.SSCAN, 
KEY, "sjfls"))
         .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void givenNonexistentKey_returnsEmptyArray() {
-    ScanResult<String> result = jedis.sscan("nonexistent", "0");
+    ScanResult<String> result = jedis.sscan("nonexistent", ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
     assertThat(result.getResult()).isEmpty();
   }
 
+  @Test
+  public void givenNegativeCursor_returnsEntriesUsingAbsoluteValueOfCursor() {
+    List<String> set = initializeThreeMemberSet();
+
+    String cursor = "-100";
+    ScanResult<String> result;
+    List<String> allEntries = new ArrayList<>();
+
+    do {
+      result = jedis.sscan(KEY, cursor);
+      allEntries.addAll(result.getResult());
+      cursor = result.getCursor();
+    } while (!result.isCompleteIteration());
+
+    assertThat(allEntries).hasSize(3);
+    assertThat(allEntries).containsExactlyInAnyOrderElementsOf(set);
+  }
+
   @Test
   public void givenSetWithOneMember_returnsMember() {
-    jedis.sadd("a", "1");
-    ScanResult<String> result = jedis.sscan("a", "0");
+    jedis.sadd(KEY, FIELD_ONE);
+    ScanResult<String> result = jedis.sscan(KEY, ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
-    assertThat(result.getResult()).containsExactly("1");
+    assertThat(result.getResult()).containsExactly(FIELD_ONE);
   }
 
   @Test
   public void givenSetWithMultipleMembers_returnsAllMembers() {
-    jedis.sadd("a", "1", "2", "3");
-    ScanResult<String> result = jedis.sscan("a", "0");
+    jedis.sadd(KEY, FIELD_ONE, FIELD_TWO, FIELD_THREE);
+    ScanResult<String> result = jedis.sscan(KEY, ZERO_CURSOR);
 
     assertThat(result.isCompleteIteration()).isTrue();
-    assertThat(result.getResult()).containsExactlyInAnyOrder("1", "2", "3");
+    assertThat(result.getResult()).containsExactlyInAnyOrder(FIELD_ONE, 
FIELD_TWO, FIELD_THREE);
   }
 
   @Test
   public void givenCount_returnsAllMembersWithoutDuplicates() {
-    jedis.sadd("a", "1", "2", "3");
+    jedis.sadd(KEY, FIELD_ONE, FIELD_TWO, FIELD_THREE);
 
     ScanParams scanParams = new ScanParams();
     scanParams.count(1);
-    String cursor = "0";
+    String cursor = ZERO_CURSOR;
     ScanResult<byte[]> result;
     List<byte[]> allMembersFromScan = new ArrayList<>();
 
     do {
-      result = jedis.sscan("a".getBytes(), cursor.getBytes(), scanParams);
+      result = jedis.sscan(KEY.getBytes(), cursor.getBytes(), scanParams);
       allMembersFromScan.addAll(result.getResult());
       cursor = result.getCursor();
     } while (!result.isCompleteIteration());
 
-    assertThat(allMembersFromScan).containsExactlyInAnyOrder("1".getBytes(),
-        "2".getBytes(),
-        "3".getBytes());
+    
assertThat(allMembersFromScan).containsExactlyInAnyOrder(FIELD_ONE.getBytes(),
+        FIELD_TWO.getBytes(),
+        FIELD_THREE.getBytes());
   }
 
   @Test
   @SuppressWarnings("unchecked")
   public void givenMultipleCounts_returnsAllEntriesWithoutDuplicates() {
-    jedis.sadd("a", "1", "12", "3");
+    jedis.sadd(KEY, FIELD_ONE, FIELD_TWO, FIELD_THREE);
 
     List<Object> result;
 
     List<byte[]> allEntries = new ArrayList<>();
-    String cursor = "0";
+    String cursor = ZERO_CURSOR;
 
     do {
       result =
-          (List<Object>) jedis.sendCommand("a", Protocol.Command.SSCAN, "a", 
cursor, "COUNT", "2",
-              "COUNT", "1");
+          (List<Object>) jedis.sendCommand(KEY, Protocol.Command.SSCAN, KEY, 
cursor, "COUNT",
+              FIELD_TWO,
+              "COUNT", FIELD_ONE);
       allEntries.addAll((List<byte[]>) result.get(1));
       cursor = new String((byte[]) result.get(0));
-    } while (!Arrays.equals((byte[]) result.get(0), "0".getBytes()));
+    } while (!Arrays.equals((byte[]) result.get(0), ZERO_CURSOR.getBytes()));
 
-    assertThat((byte[]) result.get(0)).isEqualTo("0".getBytes());
-    assertThat(allEntries).containsExactlyInAnyOrder("1".getBytes(),
-        "12".getBytes(),
-        "3".getBytes());
+    assertThat((byte[]) result.get(0)).isEqualTo(ZERO_CURSOR.getBytes());
+    assertThat(allEntries).containsExactlyInAnyOrder(FIELD_ONE.getBytes(),
+        FIELD_TWO.getBytes(),
+        FIELD_THREE.getBytes());
   }
 
   @Test
   public void givenMatch_returnsAllMatchingMembersWithoutDuplicates() {
-    jedis.sadd("a", "1", "12", "3");
+    jedis.sadd(KEY, FIELD_ONE, FIELD_TWO, FIELD_THREE);
 
     ScanParams scanParams = new ScanParams();
     scanParams.match("1*");
 
     ScanResult<byte[]> result =
-        jedis.sscan("a".getBytes(), "0".getBytes(), scanParams);
+        jedis.sscan(KEY.getBytes(), ZERO_CURSOR.getBytes(), scanParams);
 
     assertThat(result.isCompleteIteration()).isTrue();
-    assertThat(result.getResult()).containsExactlyInAnyOrder("1".getBytes(),
-        "12".getBytes());
+    
assertThat(result.getResult()).containsExactlyInAnyOrder(FIELD_ONE.getBytes(),
+        FIELD_TWO.getBytes());
   }
 
   @Test
   @SuppressWarnings("unchecked")
   public void givenMultipleMatches_returnsMembersMatchingLastMatchParameter() {
-    jedis.sadd("a", "1", "12", "3");
+    jedis.sadd(KEY, FIELD_ONE, FIELD_TWO, FIELD_THREE);
 
-    List<Object> result = (List<Object>) jedis.sendCommand("a", 
Protocol.Command.SSCAN, "a", "0",
-        "MATCH", "3*", "MATCH", "1*");
+    List<Object> result =
+        (List<Object>) jedis.sendCommand(KEY, Protocol.Command.SSCAN, KEY, 
ZERO_CURSOR,
+            "MATCH", "3*", "MATCH", "1*");
 
-    assertThat((byte[]) result.get(0)).isEqualTo("0".getBytes());
-    assertThat((List<byte[]>) 
result.get(1)).containsExactlyInAnyOrder("1".getBytes(),
-        "12".getBytes());
+    assertThat((byte[]) result.get(0)).isEqualTo(ZERO_CURSOR.getBytes());
+    assertThat((List<byte[]>) 
result.get(1)).containsExactlyInAnyOrder(FIELD_ONE.getBytes(),
+        FIELD_TWO.getBytes());
   }
 
   @Test
   public void givenMatchAndCount_returnsAllMembersWithoutDuplicates() {

Review comment:
       > This test would be better named 
"givenSetWithThreeMembersAndMatchAndCount_returnsAllMatchingMembers" with the 
assertions changed to `containsOnly()` to allow duplicate entries.
   
   This comment still applies

##########
File path: 
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSScanIntegrationTest.java
##########
@@ -323,26 +409,217 @@ public void 
givenNegativeCursor_returnsMembersUsingAbsoluteValueOfCursor() {
 
   @Test
   public void givenCursorGreaterThanUnsignedLongCapacity_returnsCursorError() {
-    assertThatThrownBy(() -> jedis.sscan("a", "18446744073709551616"))
-        .hasMessageContaining(ERROR_CURSOR);
+    assertThatThrownBy(
+        () -> jedis.sscan(KEY, 
UNSIGNED_LONG_CAPACITY.add(BigInteger.ONE).toString()))
+            .hasMessageContaining(ERROR_CURSOR);
   }
 
   @Test
   public void 
givenNegativeCursorGreaterThanUnsignedLongCapacity_returnsCursorError() {

Review comment:
       > It would be good to add tests that show the behaviour when the cursor 
value is outside the range `Long.MIN_VALUE > x > Long.MAX_VALUE`, and tests for 
the behaviour when the cursor in just inside that range.
   > 
   > The behaviour of geode-for-redis differs from native Redis here, as they 
accept cursor values up to `UNSIGNED_LONG_CAPACITY` but we only accept ones up 
to `Long.MAX_VALUE`, so in order to test this, the test cases for failing with 
values outside the range should be put in `SScanIntegrationTest` (not the 
Abstract parent class). Also, looking at that class, the test 
`givenDifferentCursorThanSpecifiedByPreviousSscan_returnsAllMembers` is wrong 
and should be removed.
   
   This comment still applies




-- 
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: notifications-unsubscr...@geode.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to