Re: [PR] MINOR : Replaced the while loop with TestUtils.waitForCondition [kafka]

2024-04-16 Thread via GitHub
chia7712 merged PR #15678: URL: https://github.com/apache/kafka/pull/15678 -- 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:

Re: [PR] MINOR : Replaced the while loop with TestUtils.waitForCondition [kafka]

2024-04-15 Thread via GitHub
chiacyu commented on PR #15678: URL: https://github.com/apache/kafka/pull/15678#issuecomment-2055793708 Thanks for the comments. Just applied. -- 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

Re: [PR] MINOR : Replaced the while loop with TestUtils.waitForCondition [kafka]

2024-04-14 Thread via GitHub
showuon commented on PR #15678: URL: https://github.com/apache/kafka/pull/15678#issuecomment-2054458671 @chiacyu , thanks for the update. LGTM! Comments: 1. Please resolve merge conflict 2. In L163, we did: ``` if (readyToAssert.getCount() > 0) { readyToAssert.countDown();

Re: [PR] MINOR : Replaced the while loop with TestUtils.waitForCondition [kafka]

2024-04-12 Thread via GitHub
chiacyu commented on code in PR #15678: URL: https://github.com/apache/kafka/pull/15678#discussion_r1562574967 ## server/src/test/java/org/apache/kafka/server/AssignmentsManagerTest.java: ## @@ -172,10 +172,11 @@ public void testAssignmentAggregation() throws

Re: [PR] MINOR : Replaced the while loop with TestUtils.waitForCondition [kafka]

2024-04-11 Thread via GitHub
showuon commented on code in PR #15678: URL: https://github.com/apache/kafka/pull/15678#discussion_r1560863382 ## server/src/test/java/org/apache/kafka/server/AssignmentsManagerTest.java: ## @@ -172,10 +172,11 @@ public void testAssignmentAggregation() throws

Re: [PR] MINOR : Replaced the while loop with TestUtils.waitForCondition [kafka]

2024-04-11 Thread via GitHub
showuon commented on code in PR #15678: URL: https://github.com/apache/kafka/pull/15678#discussion_r1560863382 ## server/src/test/java/org/apache/kafka/server/AssignmentsManagerTest.java: ## @@ -172,10 +172,11 @@ public void testAssignmentAggregation() throws

Re: [PR] MINOR : Replaced the while loop with TestUtils.waitForCondition [kafka]

2024-04-11 Thread via GitHub
chiacyu commented on PR #15678: URL: https://github.com/apache/kafka/pull/15678#issuecomment-2049461096 Thanks for the reminder, let me check on that. -- 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

Re: [PR] MINOR : Replaced the while loop with TestUtils.waitForCondition [kafka]

2024-04-11 Thread via GitHub
chia7712 commented on PR #15678: URL: https://github.com/apache/kafka/pull/15678#issuecomment-2049107538 @chiacyu it seems `AssignmentsManagerTest` become unstable. could you check them? -- This is an automated message from the Apache Git Service. To respond to the message, please log on

Re: [PR] MINOR : Replaced the while loop with TestUtils.waitForCondition [kafka]

2024-04-10 Thread via GitHub
chia7712 commented on PR #15678: URL: https://github.com/apache/kafka/pull/15678#issuecomment-2047578617 > If the condition doesn't meet within the maxWaitMs, the waitForCondition would throw the assertion failure. Do we need another timeout to handle that? My origin thought was to

Re: [PR] MINOR : Replaced the while loop with TestUtils.waitForCondition [kafka]

2024-04-10 Thread via GitHub
chiacyu commented on PR #15678: URL: https://github.com/apache/kafka/pull/15678#issuecomment-2047169130 If the condition doesn't meet within the maxWaitMs, the waitForCondition would throw the assertion failure. Do we need another timeout to handle that? -- This is an automated message

Re: [PR] MINOR : Replaced the while loop with TestUtils.waitForCondition [kafka]

2024-04-09 Thread via GitHub
chia7712 commented on PR #15678: URL: https://github.com/apache/kafka/pull/15678#issuecomment-2045642331 It seems what we want to fix is the infinite waiting, and so maybe we can just add `Timeout` to this class to fix that? -- This is an automated message from the Apache Git Service. To

Re: [PR] MINOR : Replaced the while loop with TestUtils.waitForCondition [kafka]

2024-04-09 Thread via GitHub
chia7712 commented on code in PR #15678: URL: https://github.com/apache/kafka/pull/15678#discussion_r1557970841 ## server/src/test/java/org/apache/kafka/server/AssignmentsManagerTest.java: ## @@ -172,10 +172,11 @@ public void testAssignmentAggregation() throws

Re: [PR] MINOR : Replaced the while loop with TestUtils.waitForCondition [kafka]

2024-04-09 Thread via GitHub
chiacyu commented on code in PR #15678: URL: https://github.com/apache/kafka/pull/15678#discussion_r1557842174 ## server/src/test/java/org/apache/kafka/server/AssignmentsManagerTest.java: ## @@ -172,10 +172,11 @@ public void testAssignmentAggregation() throws

Re: [PR] MINOR : Replaced the while loop with TestUtils.waitForCondition [kafka]

2024-04-09 Thread via GitHub
chia7712 commented on PR #15678: URL: https://github.com/apache/kafka/pull/15678#issuecomment-2045082221 @chiacyu Could you take a look at my comments? thanks! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL

Re: [PR] MINOR : Replaced the while loop with TestUtils.waitForCondition [kafka]

2024-04-08 Thread via GitHub
chia7712 commented on code in PR #15678: URL: https://github.com/apache/kafka/pull/15678#discussion_r1556390789 ## server/src/test/java/org/apache/kafka/server/AssignmentsManagerTest.java: ## @@ -172,10 +172,11 @@ public void testAssignmentAggregation() throws

[PR] MINOR : Replaced the while loop with TestUtils.waitForCondition [kafka]

2024-04-08 Thread via GitHub
chiacyu opened a new pull request, #15678: URL: https://github.com/apache/kafka/pull/15678 We should replace the while loop in some test cases with waitForCondition to prevent infinite looping conditions. -- This is an automated message from the Apache Git Service. To respond