DonalEvans commented on a change in pull request #7429:
URL: https://github.com/apache/geode/pull/7429#discussion_r822142385
##########
File path:
geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/string/StringsDUnitTest.java
##########
@@ -296,43 +306,27 @@ private void appendPerformAndVerify(int index, String
hashtag, AtomicBoolean isR
iterationCount++;
}
+ return iterationCount;
+ }
+
+ private void verifyAppendResult(String key, int iterationCount) {
String storedString = jedisCluster.get(key);
- int startIndex = 0;
+ int idx = 0;
int i = 0;
- boolean lastWasDuplicate = false;
- boolean reachedEnd = false;
- while (!reachedEnd) {
+ while (i < iterationCount) {
String expectedValue = "-" + key + "-" + i + "-";
-
- int endIndex = startIndex + expectedValue.length();
- reachedEnd = endIndex == storedString.length();
- String foundValue = storedString.substring(startIndex, endIndex);
-
- int subsectionStart = Math.max(0, startIndex - (2 *
expectedValue.length()));
- int subsectionEnd = Math.min(storedString.length(), endIndex + (2 *
expectedValue.length()));
-
- try {
- assertThat(foundValue).as("unexpected " + foundValue
- + " at index " + i
- + " iterationCount=" + iterationCount
- + " in String subsection: " +
storedString.substring(subsectionStart, subsectionEnd))
- .isEqualTo(expectedValue);
- lastWasDuplicate = false;
- } catch (AssertionError error) {
- // Jedis client retries can lead to duplicated appends, so check if
the append is a
- // duplicate of the previous one, but only allow one duplicated append
in a row
- String previousAppend = "-" + key + "-" + (i - 1) + "-";
- if (lastWasDuplicate || !foundValue.equals(previousAppend)) {
- throw error;
- }
-
- // Decrement the counter to account for the duplicated append and
reset the flag to detect
- // multiple duplicated appends in a row
- lastWasDuplicate = true;
- i--;
+ String foundValue = storedString.substring(idx, idx +
expectedValue.length());
+ int subsectionStart = Math.max(0, idx - (2 * expectedValue.length()));
+ int subsectionEnd = Math.min(storedString.length(), idx + (2 *
expectedValue.length()));
+
+ if (!foundValue.equals(expectedValue)) {
+ Assert.fail("unexpected " + foundValue + " at index " + i + "
iterationCount="
+ + iterationCount
+ + " in String subsection: " +
storedString.substring(subsectionStart, subsectionEnd));
+ break;
}
Review comment:
This might be better as:
```
assertThat(foundValue).as("unexpected " + foundValue
+ " at index " + i
+ " iterationCount=" + iterationCount
+ " in String subsection: " +
storedString.substring(subsectionStart, subsectionEnd))
.isEqualTo(expectedValue);
```
I always try to avoid using `Assert.fail()` if it's possible to assert on
something concrete instead, and `as()` allows us to provide a descriptive
message.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
