nastra commented on code in PR #8804:
URL: https://github.com/apache/iceberg/pull/8804#discussion_r1362441102
##########
aws/src/integration/java/org/apache/iceberg/aws/dynamodb/TestDynamoDbLockManager.java:
##########
@@ -141,11 +142,8 @@ public void testAcquireSingleProcess() throws Exception {
CompletableFuture.supplyAsync(
() -> {
- try {
- Thread.sleep(5000);
- } catch (InterruptedException e) {
- throw new RuntimeException(e);
- }
+ TestHelpers.delayInMilliseconds(
Review Comment:
this goes back to an earlier comment I had on
https://github.com/apache/iceberg/pull/8715#issuecomment-1748190639. We don't
just want to replace `Thread.sleep()` usage blindly with Awaitility by waiting
the same amount of time. The important piece is that we'd need to understand
what kind of condition the test is trying to _eventually_ reach, which we can
then check by using Awaitility (rather than just sleeping X seconds).
I've opened https://github.com/apache/iceberg/pull/8853 and
https://github.com/apache/iceberg/pull/8852 to give an idea how that might look
for other places in the code
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
