[
https://issues.apache.org/jira/browse/GEODE-8934?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17285285#comment-17285285
]
ASF GitHub Bot commented on GEODE-8934:
---------------------------------------
sabbey37 commented on a change in pull request #6022:
URL: https://github.com/apache/geode/pull/6022#discussion_r576919656
##########
File path:
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractIncrByIntegrationTest.java
##########
@@ -18,79 +18,124 @@
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.concurrent.atomic.AtomicInteger;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import redis.clients.jedis.Jedis;
import redis.clients.jedis.Protocol;
+import org.apache.geode.redis.ConcurrentLoopingThreads;
import org.apache.geode.test.awaitility.GeodeAwaitility;
import org.apache.geode.test.dunit.rules.RedisPortSupplier;
public abstract class AbstractIncrByIntegrationTest implements
RedisPortSupplier {
- private Jedis jedis;
+ private Jedis jedis1;
+ private Jedis jedis2;
private Random rand;
private static final int REDIS_CLIENT_TIMEOUT =
Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
@Before
public void setUp() {
rand = new Random();
- jedis = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
+
+ jedis1 = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
+ jedis2 = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
}
@After
public void tearDown() {
- jedis.flushAll();
- jedis.close();
+ jedis1.flushAll();
+ jedis1.close();
}
@Test
public void givenKeyNotProvided_returnsWrongNumberOfArgumentsError() {
- assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.INCRBY))
+ assertThatThrownBy(() -> jedis1.sendCommand(Protocol.Command.INCRBY))
.hasMessageContaining("ERR wrong number of arguments for 'incrby'
command");
}
@Test
public void givenIncrementNotProvided_returnsWrongNumberOfArgumentsError() {
- assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.INCRBY, "key"))
+ assertThatThrownBy(() -> jedis1.sendCommand(Protocol.Command.INCRBY,
"key"))
.hasMessageContaining("ERR wrong number of arguments for 'incrby'
command");
}
@Test
public void
givenMoreThanThreeArgumentsProvided_returnsWrongNumberOfArgumentsError() {
- assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.INCRBY, "key",
"5", "extraArg"))
+ assertThatThrownBy(() -> jedis1.sendCommand(Protocol.Command.INCRBY,
"key", "5", "extraArg"))
.hasMessageContaining("ERR wrong number of arguments for 'incrby'
command");
}
@Test
- public void testIncrBy() {
- String key1 = randString();
- String key2 = randString();
- String key3 = randString();
- int incr1 = rand.nextInt(100);
- int incr2 = rand.nextInt(100);
- Long incr3 = Long.MAX_VALUE / 2;
- int num1 = 100;
- int num2 = -100;
- jedis.set(key1, "" + num1);
- jedis.set(key2, "" + num2);
- jedis.set(key3, "" + Long.MAX_VALUE);
-
- jedis.incrBy(key1, incr1);
- jedis.incrBy(key2, incr2);
- assertThat(jedis.get(key1)).isEqualTo("" + (num1 + incr1 * 1));
- assertThat(jedis.get(key2)).isEqualTo("" + (num2 + incr2 * 1));
-
- Exception ex = null;
- try {
- jedis.incrBy(key3, incr3);
- } catch (Exception e) {
- ex = e;
- }
- assertThat(ex).isNotNull();
+ public void testIncrBy_failsWhenPerformedOnNonIntegerValue() {
+ jedis1.sadd("key", "member");
+ assertThatThrownBy(() -> jedis1.incrBy("key", 1))
+ .hasMessageContaining("WRONGTYPE Operation against a key holding the
wrong kind of value");
+ }
+
+ @Test
+ public void testIncrBy_createsAndIncrementsNonExistentKey() {
+ assertThat(jedis1.incrBy("nonexistentkey", 1)).isEqualTo(1);
+ assertThat(jedis1.incrBy("otherNonexistentKey", -1)).isEqualTo(-1);
+ }
+
+ @Test
+ public void incrBy_incrementsPositiveIntegerValue() {
+ String key = "key";
+ int num = 100;
+ int increment = rand.nextInt(100);
+
+ jedis1.set(key, String.valueOf(num));
+ jedis1.incrBy(key, increment);
+ assertThat(jedis1.get(key)).isEqualTo(String.valueOf(num + increment));
+ }
+
+ @Test
+ public void incrBy_incrementsNegativeValue() {
+ String key = "key";
+ int num = -100;
+ int increment = rand.nextInt(100);
+
+ jedis1.set(key, "" + num);
+ jedis1.incrBy(key, increment);
+ assertThat(jedis1.get(key)).isEqualTo(String.valueOf(num + increment));
+ }
+
+ @Test
+ public void testIncrBy_IncrementingMaxValueThrowsError() {
+ String key = "key";
+ Long increment = Long.MAX_VALUE / 2;
+
+ jedis1.set(key, String.valueOf(Long.MAX_VALUE));
+ assertThatThrownBy(() -> jedis1.incrBy(key, increment))
+ .hasMessageContaining("ERR increment or decrement would overflow");
+ }
Review comment:
Thanks for adding this test and merging the files! We get a different
error when incrementing a key that is set above the max long value values (so
setting a key to 9223372036854775808 then trying to incrby 1 (which results in
`ERR value is not an integer or out of range`). Could we add a test for that as
well?
----------------------------------------------------------------
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
> Redis: **INCRBY** command supported
> -----------------------------------
>
> Key: GEODE-8934
> URL: https://issues.apache.org/jira/browse/GEODE-8934
> Project: Geode
> Issue Type: Test
> Components: redis
> Reporter: Hale Bales
> Assignee: Hale Bales
> Priority: Major
> Labels: pull-request-available
>
> Write unit/integration tests that run against both Geode Redis and native
> Redis, and dunit tests which test multiple concurrent clients accessing
> different servers for the following command:
> INCRBY
> A.C.
> Unit/integration tests for both Geode and native Redis passing
> DUNIT tests passing
> README/redis_api_for_geode.html.md.erb updated to make command "supported"
> or
> Stories in the backlog to fix the identified issues (with JIRA tickets)
> and problem tests ignoredMGET
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
