[GitHub] [accumulo] keith-turner commented on a diff in pull request #3318: Improvements to ManagerAssignmentIT

["keith-turner (via GitHub)" <gi...@apache.org>]

keith-turner commented on code in PR #3318:
URL: https://github.com/apache/accumulo/pull/3318#discussion_r1170631769


##########
test/src/main/java/org/apache/accumulo/test/functional/ManagerAssignmentIT.java:
##########
@@ -146,36 +148,39 @@ public void test() throws Exception {
 
       // set the hosting goal to always
       c.tableOperations().setTabletHostingGoal(tableName, new Range(), TabletHostingGoal.ALWAYS);
-      TabletLocationState always;
-      do {
-        UtilWaitThread.sleep(250);
-        always = getTabletLocationState(c, tableId);
-      } while (always.goal != TabletHostingGoal.ALWAYS && always.current == null);
 
+      Predicate<TabletLocationState> alwaysHostedOrCurrentNotNull =
+          t -> (t.goal == TabletHostingGoal.ALWAYS) || (t.current != null);
+
+      assertTrue(Wait.waitFor(
+          () -> alwaysHostedOrCurrentNotNull.test(getTabletLocationState(c, tableId)), 60000, 250));
+
+      final TabletLocationState always = getTabletLocationState(c, tableId);
+      assertTrue(alwaysHostedOrCurrentNotNull.test(always));
       assertNull(always.future);
       assertEquals(flushed.getCurrentServer(), always.getLastServer());
       assertEquals(TabletHostingGoal.ALWAYS, always.goal);
 
       // set the hosting goal to never
       c.tableOperations().setTabletHostingGoal(tableName, new Range(), TabletHostingGoal.NEVER);
-      TabletLocationState never;
-      do {
-        UtilWaitThread.sleep(250);
-        never = getTabletLocationState(c, tableId);
-      } while (never.goal != TabletHostingGoal.NEVER && never.current != null);
+      Predicate<TabletLocationState> neverHostedOrCurrentNull =
+          t -> (t.goal == TabletHostingGoal.NEVER) || (t.current == null);
+      assertTrue(Wait.waitFor(

Review Comment:
   Not sure if this is workable.  I was wondering if this could be shortened with a new waitfor method like the following.
   
   ```java
     public static <T> T waitFor(Supplier<T> supplier, Predicate<T> predicate) {
       var obj = supplier.get();
       while(!predicate.test(obj)) {
         TimeUnit.MILLISECONDS.sleep(10);
         obj = supplier.get();
       }
       return obj;
     }
   ```
   
   Then maybe could do something like the following.
   
   ```java
   TabletLocationState never = Wait.waitFor(()->getTabletLocationState(c, tableId), t -> (t.goal == TabletHostingGoal.NEVER) || (t.current == null));
   ```



-- 
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: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org