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 value (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:
[email protected]
