kezhuw commented on a change in pull request #15344:
URL: https://github.com/apache/flink/pull/15344#discussion_r601500588
##########
File path:
flink-test-utils-parent/flink-test-utils-junit/src/main/java/org/apache/flink/core/testutils/CommonTestUtils.java
##########
@@ -198,16 +205,17 @@ public static void assertThrows(
@SuppressWarnings("BusyWait")
public static void waitUtil(Supplier<Boolean> condition, Duration timeout,
String errorMsg)
throws TimeoutException, InterruptedException {
- long timeoutMs = timeout.toMillis();
- if (timeoutMs <= 0) {
+ long timeoutNanos = timeout.toNanos();
+ if (timeoutNanos <= 0) {
throw new IllegalArgumentException("The timeout must be
positive.");
}
- long startingTime = System.currentTimeMillis();
- while (!condition.get() && System.currentTimeMillis() - startingTime <
timeoutMs) {
+ long startingTime = System.nanoTime();
Review comment:
Hmmm, this `CommonTestUtils` resides in `flink-test-utils-junit` package
which does not depend on `flink-core` where `Deadline` lives in. More worse,
`flink-core` depends on `-junit`. I guess we encounter this due to fact that
`flink-test-utils` is more flink related(with scala dependencies) while
`-junit` has not flink dependencies.
I guess we have to stop here for this `Duration` version for now.
I think there are candidates for follow up:
* Move `CommonTestUtils`(and other no junit related ?) to
`flink-test-utils`. Dependants of `flink-test-utils-junit` are more than
`flink-test-utils`. No sure whether some of them might want flink agnostic on
purpose.
* Simple renaming to avoid naming clashing on usages.
* Others basing on more through investigation.
--
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]
