[
https://issues.apache.org/jira/browse/GEODE-8864?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17272966#comment-17272966
]
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_r565440062
##########
File path:
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHScanIntegrationTest.java
##########
@@ -60,34 +65,56 @@ public void tearDown() {
jedis.close();
}
+ /********* Parameter Checks **************/
+
@Test
- public void givenNoKeyArgument_returnsWrongNumberOfArgumentsError() {
+ public void givenLessThanTwoArguments_returnsWrongNumberOfArgumentsError() {
assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSCAN))
.hasMessageContaining("ERR wrong number of arguments for 'hscan'
command");
- }
- @Test
- public void givenNoCursorArgument_returnsWrongNumberOfArgumentsError() {
assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSCAN, "key!"))
.hasMessageContaining("ERR wrong number of arguments for 'hscan'
command");
}
@Test
- public void givenArgumentsAreNotOddAndKeyExists_returnsSyntaxError() {
+ public void
givenMatchArgumentWithoutPatternOnExistingKey_returnsSyntaxError() {
+ jedis.hset("key", "b", "1");
+
+ assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSCAN, "key",
"0", "Match"))
+ .hasMessageContaining(ERROR_SYNTAX);
+ }
+
+ @Test
+
+ public void
givenMatchArgumentWithoutPatternOnNonExistentKey_returnsEmptyArray() {
+ jedis.hset("key", "b", "1");
+
+ ScanParams scanParams = new ScanParams();
+ scanParams.match("");
+
+ ScanResult<Map.Entry<String, String>> result =
+ jedis.hscan("key", "0", scanParams);
+
+ assertThat(result.getResult()).isEmpty();
+ }
Review comment:
The name of this test seems inaccurate. It says it is a
`NonExistentKey` however, we do an hset beforehand with the key, so it should
exist with data in the cache. Also, we say we don't provide a pattern, but
empty string actually is a pattern. What we could do (if we want to test a
match argument without a pattern on a nonexistent key), is not do the hset,
then do:
```
List<Object> result =
(List<Object>) jedis.sendCommand(Protocol.Command.HSCAN, "key1",
"0", "Match");
assertThat((List<String>) result.get(1)).isEmpty();
```
##########
File path:
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHScanIntegrationTest.java
##########
@@ -60,34 +65,56 @@ public void tearDown() {
jedis.close();
}
+ /********* Parameter Checks **************/
+
@Test
- public void givenNoKeyArgument_returnsWrongNumberOfArgumentsError() {
+ public void givenLessThanTwoArguments_returnsWrongNumberOfArgumentsError() {
assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSCAN))
.hasMessageContaining("ERR wrong number of arguments for 'hscan'
command");
- }
- @Test
- public void givenNoCursorArgument_returnsWrongNumberOfArgumentsError() {
assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSCAN, "key!"))
.hasMessageContaining("ERR wrong number of arguments for 'hscan'
command");
}
@Test
- public void givenArgumentsAreNotOddAndKeyExists_returnsSyntaxError() {
+ public void
givenMatchArgumentWithoutPatternOnExistingKey_returnsSyntaxError() {
+ jedis.hset("key", "b", "1");
+
+ assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSCAN, "key",
"0", "Match"))
+ .hasMessageContaining(ERROR_SYNTAX);
+ }
+
+ @Test
+
+ public void
givenMatchArgumentWithoutPatternOnNonExistentKey_returnsEmptyArray() {
+ jedis.hset("key", "b", "1");
+
+ ScanParams scanParams = new ScanParams();
+ scanParams.match("");
+
+ ScanResult<Map.Entry<String, String>> result =
+ jedis.hscan("key", "0", scanParams);
+
+ assertThat(result.getResult()).isEmpty();
+ }
+
+ @Test
+ public void
givenCountArgumentWithoutNumberOnExistingKey_returnsSyntaxError() {
jedis.hset("a", "b", "1");
- assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSCAN, "a",
"0", "a*"))
+ assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSCAN, "a",
"0", "Count"))
.hasMessageContaining(ERROR_SYNTAX);
}
@Test
@SuppressWarnings("unchecked")
- public void givenArgumentsAreNotOddAndKeyDoesNotExist_returnsEmptyArray() {
+ public void
givenCountArgumentWithoutNumberOnNonExistentKey_returnsEmptyArray() {
+ jedis.hset("a", "b", "1");
Review comment:
We don't need to do an hset here.
##########
File path:
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHScanIntegrationTest.java
##########
@@ -121,17 +156,10 @@ public void
givenCount_whenCountParameterIsNegative_returnsSyntaxError() {
}
@Test
- public void
givenMultipleCounts_whenAnyCountParameterIsNotAnInteger_returnsNotIntegerError()
{
- jedis.hset("a", "b", "1");
- assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.HSCAN, "a",
"0", "COUNT", "3",
- "COUNT", "sjlfs", "COUNT", "1"))
- .hasMessageContaining(ERROR_NOT_INTEGER);
- }
+ public void
givenMultipleCounts_whenAnyCountParameterIsLessThanOne_DoesNotError() {
+ jedis.hset("key", "b", "1");
Review comment:
The title of this test seems incorrect. When any count parameter is
less than 0, we return a syntax error. maybe instead of saying `DoesNotError`
we could change it back to `returnsSyntaxError`
----------------------------------------------------------------
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: pull-request-available
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
