[ https://issues.apache.org/jira/browse/GEODE-8864?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17295378#comment-17295378 ]
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_r587608110 ########## 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: If this were running with native Redis, it would return all entries because it is such a small amount of data. In that case, removing duplicates would not have any effect (all elements would be returned at once without duplicates, so the amount of elements returned would always be equal to the size of the complete entry map rather than the count parameter [read `Why SCAN may return all the items of an aggregate data type in a single call?` in the [SCAN documentation](https://redis.io/commands/scan) or look at the Redis code for more on this]). However, this test is only run with Geode's Redis compatibility API. I thought we moved it to `HScanIntegrationTest.java` specifically so our method of returning with a count param (which only returns the exact number of elements specified by count) could be tested. ---------------------------------------------------------------- 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)