You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2020/10/22 19:36:15 UTC
[GitHub] [geode] nonbinaryprogrammer opened a new pull request #5654: add integration tests for strlen
nonbinaryprogrammer opened a new pull request #5654:
URL: https://github.com/apache/geode/pull/5654
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
- [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?
- [ ] Is your initial contribution a single, squashed commit?
- [ ] Does `gradlew build` run cleanly?
- [ ] Have you written or updated unit tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
### Note:
Please ensure that once the PR is submitted, check Concourse for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [geode] sabbey37 commented on a change in pull request #5654: add integration tests for strlen
Posted by GitBox <gi...@apache.org>.
sabbey37 commented on a change in pull request #5654:
URL: https://github.com/apache/geode/pull/5654#discussion_r512165642
##########
File path: geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractIncrIntegrationTest.java
##########
@@ -104,7 +104,7 @@ public void testIncr_whenWrongType_shouldReturnError() {
try {
jedis.incr(key);
} catch (JedisDataException e) {
- assertThat(e.getMessage()).contains(RedisConstants.ERROR_NOT_INTEGER);
+ assertThat(e.getMessage()).contains("out of range");
Review comment:
This one was not out of sync until this PR. The reason it still passes is because it was changed to only check that the error message contains "out of range" rather than the full message "value is not an integer or out of range". I'm sure there are other messages throughout our code that are not the same as Redis, but we haven't found them yet because the commands are not fully 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:
users@infra.apache.org
[GitHub] [geode] dschneider-pivotal commented on a change in pull request #5654: add integration tests for strlen
Posted by GitBox <gi...@apache.org>.
dschneider-pivotal commented on a change in pull request #5654:
URL: https://github.com/apache/geode/pull/5654#discussion_r512145200
##########
File path: geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractIncrIntegrationTest.java
##########
@@ -104,7 +104,7 @@ public void testIncr_whenWrongType_shouldReturnError() {
try {
jedis.incr(key);
} catch (JedisDataException e) {
- assertThat(e.getMessage()).contains(RedisConstants.ERROR_NOT_INTEGER);
+ assertThat(e.getMessage()).contains("out of range");
Review comment:
If our messages are out of sync with native redis shouldn't we be seeing our native acceptance tests failing on this branch? It looks like they all passed. Does this indicate something is wrong with our native redis testing?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [geode] sabbey37 commented on a change in pull request #5654: add integration tests for strlen
Posted by GitBox <gi...@apache.org>.
sabbey37 commented on a change in pull request #5654:
URL: https://github.com/apache/geode/pull/5654#discussion_r512165642
##########
File path: geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractIncrIntegrationTest.java
##########
@@ -104,7 +104,7 @@ public void testIncr_whenWrongType_shouldReturnError() {
try {
jedis.incr(key);
} catch (JedisDataException e) {
- assertThat(e.getMessage()).contains(RedisConstants.ERROR_NOT_INTEGER);
+ assertThat(e.getMessage()).contains("out of range");
Review comment:
This one was not out of sync until this PR. The reason it still passes is because it was changed to only check that the error message contains "out of range" rather than the full message "value is not an integer or out of range". I'm sure there are some messages throughout our code that are not the same as Redis, but we haven't found them yet because the commands are not fully 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:
users@infra.apache.org
[GitHub] [geode] sabbey37 commented on a change in pull request #5654: add integration tests for strlen
Posted by GitBox <gi...@apache.org>.
sabbey37 commented on a change in pull request #5654:
URL: https://github.com/apache/geode/pull/5654#discussion_r511997258
##########
File path: geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractIncrIntegrationTest.java
##########
@@ -104,7 +104,7 @@ public void testIncr_whenWrongType_shouldReturnError() {
try {
jedis.incr(key);
} catch (JedisDataException e) {
- assertThat(e.getMessage()).contains(RedisConstants.ERROR_NOT_INTEGER);
+ assertThat(e.getMessage()).contains("out of range");
Review comment:
Could we change this back to the `ERROR_NOT_INTEGER` constant? Geode Redis should return the same errors as Redis.
##########
File path: geode-redis/src/main/java/org/apache/geode/redis/internal/RedisConstants.java
##########
@@ -42,6 +42,7 @@
public static final String ERROR_WRONG_TYPE =
"Operation against a key holding the wrong kind of value";
public static final String ERROR_NOT_INTEGER = "value is not an integer or out of range";
+ public static final String ERROR_NOT_LONG = "value is not a long or out of range";
Review comment:
I don't think Redis ever returns this error.
##########
File path: geode-redis/src/main/java/org/apache/geode/redis/internal/data/RedisString.java
##########
@@ -134,7 +132,7 @@ private long parseValueAsLong() {
try {
return Long.parseLong(value.toString());
} catch (NumberFormatException ex) {
- throw new NumberFormatException(RedisConstants.ERROR_NOT_INTEGER);
+ throw new NumberFormatException(RedisConstants.ERROR_NOT_LONG);
Review comment:
I definitely understand your thinking here. However, we want to match the errors returned by Redis, which is why we are returning the not integer error. We had to use a Java long here instead of an integer because the integer's storage capacity was not large enough, according to the Redis [INCR docs](https://redis.io/commands/incr) `the string stored at the key is interpreted as a base-10 64 bit signed integer`.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [geode] sabbey37 commented on a change in pull request #5654: add integration tests for strlen
Posted by GitBox <gi...@apache.org>.
sabbey37 commented on a change in pull request #5654:
URL: https://github.com/apache/geode/pull/5654#discussion_r512084273
##########
File path: geode-redis/src/test/java/org/apache/geode/redis/internal/data/RedisStringTest.java
##########
@@ -41,16 +42,228 @@ public static void beforeClass() {
}
@Test
- public void confirmSerializationIsStable() throws IOException, ClassNotFoundException {
- RedisString o1 = new RedisString(new ByteArrayWrapper(new byte[] {0, 1, 2, 3}));
- o1.setExpirationTimestampNoDelta(1000);
+ public void constructorSetsValue() {
+ ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {0, 1, 2});
+ RedisString string = new RedisString(byteArrayWrapper);
+ ByteArrayWrapper returnedByteArrayWrapper = string.get();
+ assertThat(returnedByteArrayWrapper).isNotNull();
+ assertThat(returnedByteArrayWrapper.value).isEqualTo(byteArrayWrapper.value);
+ }
+
+ @Test
+ public void setSetsValue() {
+ RedisString string = new RedisString();
+ string.set(new ByteArrayWrapper(new byte[] {0, 1, 2}));
+ ByteArrayWrapper returnedByteArrayWrapper = string.get();
+ assertThat(returnedByteArrayWrapper).isNotNull();
+ assertThat(returnedByteArrayWrapper.value)
+ .isEqualTo(new ByteArrayWrapper(new byte[] {0, 1, 2}).value);
+ }
+
+ @Test
+ public void getReturnsSetValue() {
+ RedisString string = new RedisString(new ByteArrayWrapper(new byte[] {0, 1}));
+ ByteArrayWrapper returnedByteArrayWrapper = string.get();
+ assertThat(returnedByteArrayWrapper).isNotNull();
+ assertThat(returnedByteArrayWrapper.value)
+ .isEqualTo(new ByteArrayWrapper(new byte[] {0, 1}).value);
+ }
+
+ @Test
+ public void appendResizesByteArray() {
+ // allows unchecked cast of mock to Region<ByteArrayWrapper, RedisData>
+ @SuppressWarnings("unchecked")
+ Region<ByteArrayWrapper, RedisData> region = mock(Region.class);
+ RedisString redisString = new RedisString(new ByteArrayWrapper(new byte[] {0, 1}));
+ ByteArrayWrapper part2 = new ByteArrayWrapper(new byte[] {2, 3, 4, 5});
+ int redisStringSize = redisString.strlen();
+ int part2Size = part2.length();
+ int appendedStringSize = redisString.append(part2, region, null);
+ assertThat(appendedStringSize).isEqualTo(redisStringSize + part2Size);
+ }
+
+ @Test
+ public void appendStoresStableDelta() throws IOException {
+ // allows unchecked cast of mock to Region<ByteArrayWrapper, RedisData>
+ @SuppressWarnings("unchecked")
+ Region<ByteArrayWrapper, RedisData> region = mock(Region.class);
+ RedisString o1 = new RedisString(new ByteArrayWrapper(new byte[] {0, 1}));
+ ByteArrayWrapper part2 = new ByteArrayWrapper(new byte[] {2, 3});
+ o1.append(part2, region, null);
+ assertThat(o1.hasDelta()).isTrue();
+ assertThat(o1.get()).isEqualTo(new ByteArrayWrapper(new byte[] {0, 1, 2, 3}));
HeapDataOutputStream out = new HeapDataOutputStream(100);
- DataSerializer.writeObject(o1, out);
+ o1.toDelta(out);
+ assertThat(o1.hasDelta()).isFalse();
ByteArrayDataInput in = new ByteArrayDataInput(out.toByteArray());
- RedisString o2 = DataSerializer.readObject(in);
+ RedisString o2 = new RedisString(new ByteArrayWrapper(new byte[] {0, 1}));
+ assertThat(o2).isNotEqualTo(o1);
+ o2.fromDelta(in);
+ assertThat(o2.get()).isEqualTo(new ByteArrayWrapper(new byte[] {0, 1, 2, 3}));
assertThat(o2).isEqualTo(o1);
}
+ @Test
+ public void serializationIsStable() throws IOException, ClassNotFoundException {
+ RedisString o1 = new RedisString(new ByteArrayWrapper(new byte[] {0, 1, 2, 3}));
+ o1.setExpirationTimestampNoDelta(1000);
+ HeapDataOutputStream outputStream = new HeapDataOutputStream(100);
+ DataSerializer.writeObject(o1, outputStream);
+ ByteArrayDataInput dataInput = new ByteArrayDataInput(outputStream.toByteArray());
+ RedisString o2 = DataSerializer.readObject(dataInput);
+ assertThat(o2).isEqualTo(o1);
+ }
+
+ @Test
+ public void incrThrowsArithmeticErrorWhenNotALong() {
+ // allows unchecked cast of mock to Region<ByteArrayWrapper, RedisData>
+ @SuppressWarnings("unchecked")
+ Region<ByteArrayWrapper, RedisData> region = mock(Region.class);
+ ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {'1', '0', ' ', '1'});
+ RedisString string = new RedisString(byteArrayWrapper);
+ assertThatThrownBy(() -> string.incr(region, byteArrayWrapper))
+ .isInstanceOf(NumberFormatException.class);
+ }
+
+ @Test
+ public void incrErrorsWhenValueOverflows() {
+ // allows unchecked cast of mock to Region<ByteArrayWrapper, RedisData>
+ @SuppressWarnings("unchecked")
+ Region<ByteArrayWrapper, RedisData> region = mock(Region.class);
+ ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(
+ // max value for signed long
+ new byte[] {'9', '2', '2', '3', '3', '7', '2', '0', '3', '6', '8', '5', '4', '7', '7', '5',
+ '8', '0', '7'});
+ RedisString string = new RedisString(byteArrayWrapper);
+ assertThatThrownBy(() -> string.incr(region, byteArrayWrapper))
+ .isInstanceOf(ArithmeticException.class);
+ }
+
+ @Test
+ public void incrIncrementsValueAtGivenKey() {
+ // allows unchecked cast of mock to Region<ByteArrayWrapper, RedisData>
+ @SuppressWarnings("unchecked")
+ Region<ByteArrayWrapper, RedisData> region = mock(Region.class);
+ ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {'1', '0'});
+ RedisString string = new RedisString(byteArrayWrapper);
+ string.incr(region, byteArrayWrapper);
+ assertThat(string.get().toString()).isEqualTo("11");
+ }
+
+ @Test
+ public void incrbyThrowsNumberFormatExceptionWhenNotALong() {
+ // allows unchecked cast of mock to Region<ByteArrayWrapper, RedisData>
+ @SuppressWarnings("unchecked")
+ Region<ByteArrayWrapper, RedisData> region = mock(Region.class);
+ ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {'1', '0', ' ', '1'});
+ RedisString string = new RedisString(byteArrayWrapper);
+ assertThatThrownBy(() -> string.incrby(region, byteArrayWrapper, 2L))
+ .isInstanceOf(NumberFormatException.class);
+ }
+
+ @Test
+ public void incrbyErrorsWhenValueOverflows() {
+ // allows unchecked cast of mock to Region<ByteArrayWrapper, RedisData>
+ @SuppressWarnings("unchecked")
+ Region<ByteArrayWrapper, RedisData> region = mock(Region.class);
+ ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(
+ // max value for signed long
+ new byte[] {'9', '2', '2', '3', '3', '7', '2', '0', '3', '6', '8', '5', '4', '7', '7', '5',
+ '8', '0', '7'});
+ RedisString string = new RedisString(byteArrayWrapper);
+ assertThatThrownBy(() -> string.incrby(region, byteArrayWrapper, 2L))
+ .isInstanceOf(ArithmeticException.class);
+ }
+
+ @Test
+ public void incrbyIncrementsValueByGivenLong() {
+ // allows unchecked cast of mock to Region<ByteArrayWrapper, RedisData>
+ @SuppressWarnings("unchecked")
+ Region<ByteArrayWrapper, RedisData> region = mock(Region.class);
+ ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {'1', '0'});
+ RedisString string = new RedisString(byteArrayWrapper);
+ string.incrby(region, byteArrayWrapper, 2L);
+ assertThat(string.get().toString()).isEqualTo("12");
+ }
+
+ @Test
+ public void incrbyfloatThrowsArithmeticErrorWhenNotADouble() {
+ // allows unchecked cast of mock to Region<ByteArrayWrapper, RedisData>
+ @SuppressWarnings("unchecked")
+ Region<ByteArrayWrapper, RedisData> region = mock(Region.class);
+ ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {'1', '0', ' ', '1'});
+ RedisString string = new RedisString(byteArrayWrapper);
+ assertThatThrownBy(() -> string.incrbyfloat(region, byteArrayWrapper, 1.1))
+ .isInstanceOf(NumberFormatException.class);
+ }
+
+ @Test
+ public void incrbyfloatIncrementsValueByGivenFloat() {
+ // allows unchecked cast of mock to Region<ByteArrayWrapper, RedisData>
+ @SuppressWarnings("unchecked")
+ Region<ByteArrayWrapper, RedisData> region = mock(Region.class);
+ ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {'1', '0'});
+ RedisString string = new RedisString(byteArrayWrapper);
+ string.incrbyfloat(region, byteArrayWrapper, 2.20);
+ assertThat(string.get().toString()).isEqualTo("12.2");
+ }
+
+ @Test
+ public void decrErrorsWhenOverflows() {
+ // allows unchecked cast of mock to Region<ByteArrayWrapper, RedisData>
+ @SuppressWarnings("unchecked")
+ Region<ByteArrayWrapper, RedisData> region = mock(Region.class);
+ ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {0});
+ RedisString string = new RedisString(byteArrayWrapper);
+ assertThatThrownBy(() -> string.decr(region, byteArrayWrapper))
+ .isInstanceOf(NumberFormatException.class);
Review comment:
This seems to be an error due to the inability to parse the value as a long rather than due to overflow. We could change the test name or make it error due to overflow instead.
##########
File path: geode-redis/src/test/java/org/apache/geode/redis/internal/data/RedisStringTest.java
##########
@@ -41,16 +42,228 @@ public static void beforeClass() {
}
@Test
- public void confirmSerializationIsStable() throws IOException, ClassNotFoundException {
- RedisString o1 = new RedisString(new ByteArrayWrapper(new byte[] {0, 1, 2, 3}));
- o1.setExpirationTimestampNoDelta(1000);
+ public void constructorSetsValue() {
+ ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {0, 1, 2});
+ RedisString string = new RedisString(byteArrayWrapper);
+ ByteArrayWrapper returnedByteArrayWrapper = string.get();
+ assertThat(returnedByteArrayWrapper).isNotNull();
+ assertThat(returnedByteArrayWrapper.value).isEqualTo(byteArrayWrapper.value);
+ }
+
+ @Test
+ public void setSetsValue() {
+ RedisString string = new RedisString();
+ string.set(new ByteArrayWrapper(new byte[] {0, 1, 2}));
+ ByteArrayWrapper returnedByteArrayWrapper = string.get();
+ assertThat(returnedByteArrayWrapper).isNotNull();
+ assertThat(returnedByteArrayWrapper.value)
+ .isEqualTo(new ByteArrayWrapper(new byte[] {0, 1, 2}).value);
+ }
+
+ @Test
+ public void getReturnsSetValue() {
+ RedisString string = new RedisString(new ByteArrayWrapper(new byte[] {0, 1}));
+ ByteArrayWrapper returnedByteArrayWrapper = string.get();
+ assertThat(returnedByteArrayWrapper).isNotNull();
+ assertThat(returnedByteArrayWrapper.value)
+ .isEqualTo(new ByteArrayWrapper(new byte[] {0, 1}).value);
+ }
+
+ @Test
+ public void appendResizesByteArray() {
+ // allows unchecked cast of mock to Region<ByteArrayWrapper, RedisData>
+ @SuppressWarnings("unchecked")
+ Region<ByteArrayWrapper, RedisData> region = mock(Region.class);
+ RedisString redisString = new RedisString(new ByteArrayWrapper(new byte[] {0, 1}));
+ ByteArrayWrapper part2 = new ByteArrayWrapper(new byte[] {2, 3, 4, 5});
+ int redisStringSize = redisString.strlen();
+ int part2Size = part2.length();
+ int appendedStringSize = redisString.append(part2, region, null);
+ assertThat(appendedStringSize).isEqualTo(redisStringSize + part2Size);
+ }
+
+ @Test
+ public void appendStoresStableDelta() throws IOException {
+ // allows unchecked cast of mock to Region<ByteArrayWrapper, RedisData>
+ @SuppressWarnings("unchecked")
+ Region<ByteArrayWrapper, RedisData> region = mock(Region.class);
+ RedisString o1 = new RedisString(new ByteArrayWrapper(new byte[] {0, 1}));
+ ByteArrayWrapper part2 = new ByteArrayWrapper(new byte[] {2, 3});
+ o1.append(part2, region, null);
+ assertThat(o1.hasDelta()).isTrue();
+ assertThat(o1.get()).isEqualTo(new ByteArrayWrapper(new byte[] {0, 1, 2, 3}));
HeapDataOutputStream out = new HeapDataOutputStream(100);
- DataSerializer.writeObject(o1, out);
+ o1.toDelta(out);
+ assertThat(o1.hasDelta()).isFalse();
ByteArrayDataInput in = new ByteArrayDataInput(out.toByteArray());
- RedisString o2 = DataSerializer.readObject(in);
+ RedisString o2 = new RedisString(new ByteArrayWrapper(new byte[] {0, 1}));
+ assertThat(o2).isNotEqualTo(o1);
+ o2.fromDelta(in);
+ assertThat(o2.get()).isEqualTo(new ByteArrayWrapper(new byte[] {0, 1, 2, 3}));
assertThat(o2).isEqualTo(o1);
}
+ @Test
+ public void serializationIsStable() throws IOException, ClassNotFoundException {
+ RedisString o1 = new RedisString(new ByteArrayWrapper(new byte[] {0, 1, 2, 3}));
+ o1.setExpirationTimestampNoDelta(1000);
+ HeapDataOutputStream outputStream = new HeapDataOutputStream(100);
+ DataSerializer.writeObject(o1, outputStream);
+ ByteArrayDataInput dataInput = new ByteArrayDataInput(outputStream.toByteArray());
+ RedisString o2 = DataSerializer.readObject(dataInput);
+ assertThat(o2).isEqualTo(o1);
+ }
+
+ @Test
+ public void incrThrowsArithmeticErrorWhenNotALong() {
+ // allows unchecked cast of mock to Region<ByteArrayWrapper, RedisData>
+ @SuppressWarnings("unchecked")
+ Region<ByteArrayWrapper, RedisData> region = mock(Region.class);
+ ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {'1', '0', ' ', '1'});
+ RedisString string = new RedisString(byteArrayWrapper);
+ assertThatThrownBy(() -> string.incr(region, byteArrayWrapper))
+ .isInstanceOf(NumberFormatException.class);
+ }
+
+ @Test
+ public void incrErrorsWhenValueOverflows() {
+ // allows unchecked cast of mock to Region<ByteArrayWrapper, RedisData>
+ @SuppressWarnings("unchecked")
+ Region<ByteArrayWrapper, RedisData> region = mock(Region.class);
+ ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(
+ // max value for signed long
+ new byte[] {'9', '2', '2', '3', '3', '7', '2', '0', '3', '6', '8', '5', '4', '7', '7', '5',
+ '8', '0', '7'});
+ RedisString string = new RedisString(byteArrayWrapper);
+ assertThatThrownBy(() -> string.incr(region, byteArrayWrapper))
+ .isInstanceOf(ArithmeticException.class);
+ }
+
+ @Test
+ public void incrIncrementsValueAtGivenKey() {
+ // allows unchecked cast of mock to Region<ByteArrayWrapper, RedisData>
+ @SuppressWarnings("unchecked")
+ Region<ByteArrayWrapper, RedisData> region = mock(Region.class);
+ ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {'1', '0'});
+ RedisString string = new RedisString(byteArrayWrapper);
+ string.incr(region, byteArrayWrapper);
+ assertThat(string.get().toString()).isEqualTo("11");
+ }
+
+ @Test
+ public void incrbyThrowsNumberFormatExceptionWhenNotALong() {
+ // allows unchecked cast of mock to Region<ByteArrayWrapper, RedisData>
+ @SuppressWarnings("unchecked")
+ Region<ByteArrayWrapper, RedisData> region = mock(Region.class);
+ ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {'1', '0', ' ', '1'});
+ RedisString string = new RedisString(byteArrayWrapper);
+ assertThatThrownBy(() -> string.incrby(region, byteArrayWrapper, 2L))
+ .isInstanceOf(NumberFormatException.class);
+ }
+
+ @Test
+ public void incrbyErrorsWhenValueOverflows() {
+ // allows unchecked cast of mock to Region<ByteArrayWrapper, RedisData>
+ @SuppressWarnings("unchecked")
+ Region<ByteArrayWrapper, RedisData> region = mock(Region.class);
+ ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(
+ // max value for signed long
+ new byte[] {'9', '2', '2', '3', '3', '7', '2', '0', '3', '6', '8', '5', '4', '7', '7', '5',
+ '8', '0', '7'});
+ RedisString string = new RedisString(byteArrayWrapper);
+ assertThatThrownBy(() -> string.incrby(region, byteArrayWrapper, 2L))
+ .isInstanceOf(ArithmeticException.class);
+ }
+
+ @Test
+ public void incrbyIncrementsValueByGivenLong() {
+ // allows unchecked cast of mock to Region<ByteArrayWrapper, RedisData>
+ @SuppressWarnings("unchecked")
+ Region<ByteArrayWrapper, RedisData> region = mock(Region.class);
+ ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {'1', '0'});
+ RedisString string = new RedisString(byteArrayWrapper);
+ string.incrby(region, byteArrayWrapper, 2L);
+ assertThat(string.get().toString()).isEqualTo("12");
+ }
+
+ @Test
+ public void incrbyfloatThrowsArithmeticErrorWhenNotADouble() {
+ // allows unchecked cast of mock to Region<ByteArrayWrapper, RedisData>
+ @SuppressWarnings("unchecked")
+ Region<ByteArrayWrapper, RedisData> region = mock(Region.class);
+ ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {'1', '0', ' ', '1'});
+ RedisString string = new RedisString(byteArrayWrapper);
+ assertThatThrownBy(() -> string.incrbyfloat(region, byteArrayWrapper, 1.1))
+ .isInstanceOf(NumberFormatException.class);
+ }
+
+ @Test
+ public void incrbyfloatIncrementsValueByGivenFloat() {
+ // allows unchecked cast of mock to Region<ByteArrayWrapper, RedisData>
+ @SuppressWarnings("unchecked")
+ Region<ByteArrayWrapper, RedisData> region = mock(Region.class);
+ ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {'1', '0'});
+ RedisString string = new RedisString(byteArrayWrapper);
+ string.incrbyfloat(region, byteArrayWrapper, 2.20);
+ assertThat(string.get().toString()).isEqualTo("12.2");
+ }
+
+ @Test
+ public void decrErrorsWhenOverflows() {
+ // allows unchecked cast of mock to Region<ByteArrayWrapper, RedisData>
+ @SuppressWarnings("unchecked")
+ Region<ByteArrayWrapper, RedisData> region = mock(Region.class);
+ ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {0});
+ RedisString string = new RedisString(byteArrayWrapper);
+ assertThatThrownBy(() -> string.decr(region, byteArrayWrapper))
+ .isInstanceOf(NumberFormatException.class);
+ }
+
+ @Test
+ public void decrDecrementsValue() {
+ // allows unchecked cast of mock to Region<ByteArrayWrapper, RedisData>
+ @SuppressWarnings("unchecked")
+ Region<ByteArrayWrapper, RedisData> region = mock(Region.class);
+ ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {'1', '0'});
+ RedisString string = new RedisString(byteArrayWrapper);
+ string.decr(region, byteArrayWrapper);
+ assertThat(string.get().toString()).isEqualTo("9");
+ }
+
+ @Test
+ public void decrbyErrorsWhenOverflows() {
+ // allows unchecked cast of mock to Region<ByteArrayWrapper, RedisData>
+ @SuppressWarnings("unchecked")
+ Region<ByteArrayWrapper, RedisData> region = mock(Region.class);
+ ByteArrayWrapper byteArrayWrapper = new ByteArrayWrapper(new byte[] {1});
+ RedisString string = new RedisString(byteArrayWrapper);
+ assertThatThrownBy(() -> string.decrby(region, byteArrayWrapper, 2))
+ .isInstanceOf(NumberFormatException.class);
+ }
Review comment:
This also seems to be an error due to the inability to parse the value as a long rather than due to overflow. We could change the test name or make it error due to overflow instead.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [geode] nonbinaryprogrammer merged pull request #5654: GEODE-8662: add integration tests for strlen
Posted by GitBox <gi...@apache.org>.
nonbinaryprogrammer merged pull request #5654:
URL: https://github.com/apache/geode/pull/5654
----------------------------------------------------------------
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:
users@infra.apache.org