[refactor][test] Move assert equals and retry to a base class - #14815
Conversation
|
/pulsarbot rerun-failure-checks |
1 similar comment
|
/pulsarbot rerun-failure-checks |
codelipenghui
added
area/test
type/refactor
|
|
||
| public static void assertEqualsAndRetry(Supplier<Object> actual, | ||
| Object expected, | ||
| Object expectedAndRetry) throws Exception { | ||
| assertEqualsAndRetry(actual, expected, expectedAndRetry, 5, 100); | ||
| } | ||
|
|
||
| public static void assertEqualsAndRetry(Supplier<Object> actual, | ||
| Object expected, | ||
| Object expectedAndRetry, | ||
| int retryCount, | ||
| long intSleepTimeInMillis) throws Exception { | ||
| assertTrue(retryStrategically((__) -> { | ||
| if (actual.get().equals(expectedAndRetry)) { | ||
| return false; | ||
| } | ||
| assertEquals(actual.get(), expected); | ||
| return true; | ||
| }, retryCount, intSleepTimeInMillis)); | ||
| } | ||
|
|
||
| public static boolean retryStrategically(Predicate<Void> predicate, int retryCount, long intSleepTimeInMillis) | ||
| throws Exception { | ||
| for (int i = 0; i < retryCount; i++) { | ||
| if (predicate.test(null)) { | ||
| return true; | ||
| } | ||
| Thread.sleep(intSleepTimeInMillis + (intSleepTimeInMillis * i)); | ||
| } | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
Please don't expand the usage of these methods.
Awaitability should be used instead of this approach.
There was a problem hiding this comment.
+1 - I think we should consolidate and only use Awaitility for these kinds of tests.
There was a problem hiding this comment.
If we use Awaitility, then we need to use Fail-Fast Conditions feature.
But we need to upgrade the awaitility dependency to 4.1.0 or higher first. (Current Pulsar are using 4.0.3)
Example:
Awaitility.await().failFast(() -> {
Optional<Map<String, String>> cachedValue = objCache.getIfCached(key1);
// Need ensure objCache.getIfCached(key1) don't return a wrong value.
// Only retry when objCache.getIfCached(key1) return Optional.empty() or Optional.of(v)
return cachedValue.isPresent() && !Optional.of(v).equals(cachedValue);
}).untilAsserted(() -> assertEquals(objCache.getIfCached(key1), Optional.of(v)));@lhotari @michaeljmarshall Do you have a better idea when using the current version of awaitility? Thanks!
|
@Demogorgon314 A better approach would be to remove retryStrategically completely, as I did in #14518 (which didn't get merged since it was obsolete) |
|
There was a similar discussion about the desire to replace retryStrategically with Availability in #14373 (comment) . |
(cherry picked from commit d0d9aa5)
4 participants
Motivation
Move assert equals and retry to a base class.
Documentation
Need to update docs?
doc-requiredno-need-docdoc