[
https://issues.apache.org/jira/browse/GEODE-8864?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17295344#comment-17295344
]
ASF GitHub Bot commented on GEODE-8864:
---------------------------------------
sabbey37 commented on a change in pull request #5954:
URL: https://github.com/apache/geode/pull/5954#discussion_r587561626
##########
File path:
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/HScanIntegrationTest.java
##########
@@ -37,22 +44,58 @@ public int getPort() {
return server.getPort();
}
+
+ // Note: these tests will not pass native redis, so included here in
concrete test class
+ @Test
+ public void givenCursorGreaterThanIntMaxValue_returnsCursorError() {
+ int largestCursorValue = Integer.MAX_VALUE;
+
+ BigInteger tooBigCursor =
+ new
BigInteger(String.valueOf(largestCursorValue)).add(BigInteger.valueOf(1));
+
+ assertThatThrownBy(() -> jedis.hscan("a", tooBigCursor.toString()))
+ .hasMessageContaining(ERROR_CURSOR);
+ }
+
+ @Test
+ public void givenCursorLessThanIntMinValue_returnsCursorError() {
+ int smallestCursorValue = Integer.MIN_VALUE;
+
+ BigInteger tooSmallCursor =
+ new
BigInteger(String.valueOf(smallestCursorValue)).subtract(BigInteger.valueOf(1));
+
+ assertThatThrownBy(() -> jedis.hscan("a", tooSmallCursor.toString()))
+ .hasMessageContaining(ERROR_CURSOR);
+ }
+
+
@Test
- public void
givenDifferentCursorThanSpecifiedByPreviousHscan_returnsAllEntries() {
+ public void givenCount_shouldReturnsExpectedNumberOfEntries() {
Review comment:
If you end up making other changes, the grammar here could be corrected
(either changed to `givenCount_shouldReturnExpectedNumberOfEntries` or (more
consistent with the tests above it) `givenCount_returnsExpectedNumberOfEntries`.
##########
File path:
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/HScanIntegrationTest.java
##########
@@ -37,22 +44,58 @@ public int getPort() {
return server.getPort();
}
+
+ // Note: these tests will not pass native redis, so included here in
concrete test class
+ @Test
+ public void givenCursorGreaterThanIntMaxValue_returnsCursorError() {
+ int largestCursorValue = Integer.MAX_VALUE;
+
+ BigInteger tooBigCursor =
+ new
BigInteger(String.valueOf(largestCursorValue)).add(BigInteger.valueOf(1));
+
+ assertThatThrownBy(() -> jedis.hscan("a", tooBigCursor.toString()))
+ .hasMessageContaining(ERROR_CURSOR);
+ }
+
+ @Test
+ public void givenCursorLessThanIntMinValue_returnsCursorError() {
+ int smallestCursorValue = Integer.MIN_VALUE;
+
+ BigInteger tooSmallCursor =
+ new
BigInteger(String.valueOf(smallestCursorValue)).subtract(BigInteger.valueOf(1));
+
+ assertThatThrownBy(() -> jedis.hscan("a", tooSmallCursor.toString()))
+ .hasMessageContaining(ERROR_CURSOR);
+ }
+
+
@Test
- public void
givenDifferentCursorThanSpecifiedByPreviousHscan_returnsAllEntries() {
+ public void givenCount_shouldReturnsExpectedNumberOfEntries() {
Map<String, String> entryMap = new HashMap<>();
- for (int i = 0; i < 10; i++) {
- entryMap.put(String.valueOf(i), String.valueOf(i));
- }
- jedis.hmset("a", entryMap);
+ entryMap.put("1", "yellow");
+ entryMap.put("2", "green");
+ entryMap.put("3", "orange");
+ jedis.hmset("colors", entryMap);
+
+ int COUNT_PARAM = 2;
ScanParams scanParams = new ScanParams();
- scanParams.count(5);
- ScanResult<Map.Entry<String, String>> result = jedis.hscan("a", "0",
scanParams);
- assertThat(result.isCompleteIteration()).isFalse();
+ scanParams.count(COUNT_PARAM);
+ ScanResult<Map.Entry<String, String>> result;
+
+ List<Map.Entry<String, String>> allEntries = new ArrayList<>();
+ String cursor = "0";
- result = jedis.hscan("a", "100");
+ result = jedis.hscan("colors", cursor, scanParams);
+ allEntries.addAll(result.getResult());
- assertThat(result.getResult()).hasSize(10);
- assertThat(new
HashSet<>(result.getResult())).isEqualTo(entryMap.entrySet());
+ List<Map.Entry<String, String>> allDistinctEntries =
+ allEntries
+ .stream()
+ .distinct()
+ .collect(Collectors.toList());
+
+ assertThat(allDistinctEntries.size()).isEqualTo(COUNT_PARAM);
Review comment:
I like this test. Thanks for thinking of it! Because we are making sure
that we return the same number of entries as the `COUNT_PARAM`, it doesn't make
sense to take out duplicates (we should return the same number of entries as
the `COUNT_PARAM`, including duplicates). In that case, lines 90-96 could be
removed and we could just assert the following:
`assertThat(result.getResult().size()).isEqualTo(COUNT_PARAM)`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> finish implementation of Redis HScan Command
> --------------------------------------------
>
> Key: GEODE-8864
> URL: https://issues.apache.org/jira/browse/GEODE-8864
> Project: Geode
> Issue Type: New Feature
> Components: redis
> Reporter: John Hutchison
> Priority: Major
> Labels: blocks-1.14.0, pull-request-available
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
