sabbey37 commented on a change in pull request #6022:
URL: https://github.com/apache/geode/pull/6022#discussion_r574607774
##########
File path:
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractStringIntegrationTest.java
##########
@@ -26,141 +28,190 @@
import redis.clients.jedis.Protocol;
import redis.clients.jedis.exceptions.JedisDataException;
+import org.apache.geode.redis.ConcurrentLoopingThreads;
import org.apache.geode.redis.internal.RedisConstants;
import org.apache.geode.test.awaitility.GeodeAwaitility;
import org.apache.geode.test.dunit.rules.RedisPortSupplier;
public abstract class AbstractStringIntegrationTest implements
RedisPortSupplier {
- private Jedis jedis;
+ private Jedis jedis1;
+ private Jedis jedis2;
+
private static final int REDIS_CLIENT_TIMEOUT =
Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
@Before
public void setUp() {
- 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.STRLEN))
+ assertThatThrownBy(() -> jedis1.sendCommand(Protocol.Command.STRLEN))
.hasMessageContaining("ERR wrong number of arguments for 'strlen'
command");
}
@Test
public void
givenMoreThanTwoArgumentsProvided_returnsWrongNumberOfArgumentsError() {
- assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.STRLEN, "key",
"extraArg"))
+ assertThatThrownBy(() -> jedis1.sendCommand(Protocol.Command.STRLEN,
"key", "extraArg"))
.hasMessageContaining("ERR wrong number of arguments for 'strlen'
command");
}
@Test
public void testStrlen_requestNonexistentKey_returnsZero() {
- Long result = jedis.strlen("Nohbdy");
+ Long result = jedis1.strlen("Nohbdy");
assertThat(result).isEqualTo(0);
}
@Test
public void testStrlen_requestKey_returnsLengthOfStringValue() {
String value = "byGoogle";
- jedis.set("golang", value);
+ jedis1.set("golang", value);
- Long result = jedis.strlen("golang");
+ Long result = jedis1.strlen("golang");
assertThat(result).isEqualTo(value.length());
}
@Test
public void testStrlen_requestWrongType_shouldReturnError() {
String key = "hashKey";
- jedis.hset(key, "field", "this value doesn't matter");
+ jedis1.hset(key, "field", "this value doesn't matter");
- assertThatThrownBy(() -> jedis.strlen(key))
+ assertThatThrownBy(() -> jedis1.strlen(key))
.isInstanceOf(JedisDataException.class)
.hasMessageContaining(RedisConstants.ERROR_WRONG_TYPE);
}
@Test
public void testStrlen_withEmptyByte() {
byte[] key = new byte[] {0};
- jedis.set(key, new byte[] {});
+ jedis1.set(key, new byte[] {});
- assertThat(jedis.strlen(key)).isEqualTo(0);
+ assertThat(jedis1.strlen(key)).isEqualTo(0);
}
@Test
public void testStrlen_withBinaryData() {
byte[] zero = new byte[] {0};
- jedis.set(zero, zero);
+ jedis1.set(zero, zero);
- assertThat(jedis.strlen(zero)).isEqualTo(1);
+ assertThat(jedis1.strlen(zero)).isEqualTo(1);
}
@Test
public void testStrlen_withUTF16BinaryData() {
String test_utf16_string = "最𐐷𤭢";
byte[] testBytes = test_utf16_string.getBytes(StandardCharsets.UTF_16);
- jedis.set(testBytes, testBytes);
+ jedis1.set(testBytes, testBytes);
- assertThat(jedis.strlen(testBytes)).isEqualTo(12);
+ assertThat(jedis1.strlen(testBytes)).isEqualTo(12);
}
@Test
public void testStrlen_withIntData() {
byte[] key = new byte[] {0};
byte[] value = new byte[] {1, 0, 0};
- jedis.set(key, value);
+ jedis1.set(key, value);
- assertThat(jedis.strlen(key)).isEqualTo(value.length);
+ assertThat(jedis1.strlen(key)).isEqualTo(value.length);
}
@Test
public void testStrlen_withFloatData() {
byte[] key = new byte[] {0};
byte[] value = new byte[] {'0', '.', '9'};
- jedis.set(key, value);
+ jedis1.set(key, value);
- assertThat(jedis.strlen(key)).isEqualTo(value.length);
+ assertThat(jedis1.strlen(key)).isEqualTo(value.length);
}
@Test
public void testDecr_ErrorsWithWrongNumberOfArguments() {
- assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.DECR))
+ assertThatThrownBy(() -> jedis1.sendCommand(Protocol.Command.DECR))
.hasMessageContaining("ERR wrong number of arguments for 'decr'
command");
- assertThatThrownBy(() -> jedis.sendCommand(Protocol.Command.DECR, "1",
"2"))
+ assertThatThrownBy(() -> jedis1.sendCommand(Protocol.Command.DECR, "1",
"2"))
.hasMessageContaining("ERR wrong number of arguments for 'decr'
command");
}
@Test
public void testDecr_withWrongType_shouldError() {
String key = "hashKey";
- jedis.hset(key, "field", "non-int value");
+ jedis1.hset(key, "field", "non-int value");
- assertThatThrownBy(() -> jedis.decr(key))
+ assertThatThrownBy(() -> jedis1.decr(key))
.isInstanceOf(JedisDataException.class)
.hasMessageContaining(RedisConstants.ERROR_WRONG_TYPE);
}
@Test
public void testDecr_decrementsPositiveIntegerValues() {
String key = "key";
- jedis.set(key, "10");
+ jedis1.set(key, "10");
- assertThat(jedis.decr(key)).isEqualTo(9L);
- assertThat(jedis.get(key)).isEqualTo("9");
+ assertThat(jedis1.decr(key)).isEqualTo(9L);
+ assertThat(jedis1.get(key)).isEqualTo("9");
}
@Test
public void testDecr_returnsValueWhenDecrementingResultsInNegativeNumber() {
String key = "key";
- jedis.set(key, "0");
+ jedis1.set(key, "0");
+
+ assertThat(jedis1.decr(key)).isEqualTo(-1L);
+ assertThat(jedis1.get(key)).isEqualTo("-1");
+ }
+
+ @Test
+ public void testIncrBy_returnsErrorMessageForWrongNumberOfParameters() {
+ assertThatThrownBy(() -> jedis1.sendCommand(Protocol.Command.INCRBY))
+ .hasMessageContaining("ERR wrong number of arguments for 'incrby'
command");
+ assertThatThrownBy(() -> jedis1.sendCommand(Protocol.Command.INCRBY, "1"))
+ .hasMessageContaining("ERR wrong number of arguments for 'incrby'
command");
+ assertThatThrownBy(() -> jedis1.sendCommand(Protocol.Command.INCRBY, "1",
"2", "3"))
+ .hasMessageContaining("ERR wrong number of arguments for 'incrby'
command");
+ }
+
+ @Test
+ 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");
+ }
- assertThat(jedis.decr(key)).isEqualTo(-1L);
- assertThat(jedis.get(key)).isEqualTo("-1");
+ @Test
+ public void testIncrBy_createsAndIncrementsNonExistentKey() {
+ assertThat(jedis1.incrBy("nonexistentkey", 1)).isEqualTo(1);
+ assertThat(jedis1.incrBy("otherNonexistentKey", -1)).isEqualTo(-1);
+ }
+
+ @Test
+ public void testConcurrentIncrBy_performsAllIncrBys() {
+ String key = "key";
+ AtomicInteger expectedValue = new AtomicInteger(0);
+
+ jedis1.set(key, "0");
+
+ new ConcurrentLoopingThreads(1000,
+ (i) -> {
+ int increment = ThreadLocalRandom.current().nextInt(-50, 50);
+ expectedValue.addAndGet(increment);
+ jedis1.incrBy(key, increment);
+ },
+ (i) -> {
+ int increment = ThreadLocalRandom.current().nextInt(-50, 50);
+ expectedValue.addAndGet(increment);
+ jedis2.incrBy(key, increment);
+ }).run();
+
+
assertThat(Integer.parseInt(jedis1.get(key))).isEqualTo(expectedValue.get());
}
Review comment:
There's actually an `AbstractIncrByIntegrationTest` file. Some of these
tests could be merged into that file. Others are duplicates. A test that isn't
in either file: testing the max and min 64 bit signed integer value
(9,223,372,036,854,775,807 and -9223372036854775808, respectively). We should
test both trying to increment a key that is set above/below these 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`)) as well as incrementing a key
that is right at the max/min value (so setting a key to `9223372036854775807`
then trying to incrby 1 (which results in `ERR increment or decrement would
overflow`))
----------------------------------------------------------------
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]
