fix: waitForCondition should not call predicate after it returned true (

#1721)
microsoft · Dec 19, 2024 · 42d0203 · 42d0203
1 parent c591a14
commit 42d0203
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 8 deletions.
diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/WaitableRace.java b/playwright/src/main/java/com/microsoft/playwright/impl/WaitableRace.java
@@ -20,15 +20,20 @@
 
 class WaitableRace<T> implements Waitable<T> {
   private final Collection<Waitable<T>> waitables;
+  private Waitable<T> firstReady;
 
   WaitableRace(Collection<Waitable<T>> waitables) {
     this.waitables = waitables;
   }
 
   @Override
   public boolean isDone() {
-    for (Waitable w : waitables) {
+    if (firstReady != null) {
+      return true;
+    }
+    for (Waitable<T> w : waitables) {
       if (w.isDone()) {
+        firstReady = w;
         return true;
       }
     }
@@ -37,14 +42,11 @@ public boolean isDone() {
 
   @Override
   public T get() {
-    assert isDone();
-    dispose();
-    for (Waitable<T> w : waitables) {
-      if (w.isDone()) {
-        return w.get();
-      }
+    try {
+      return firstReady.get();
+    } finally {
+      dispose();
     }
-    throw new IllegalStateException("At least one element must be ready");
   }
 
   @Override

diff --git a/playwright/src/test/java/com/microsoft/playwright/TestBrowserContextBasic.java b/playwright/src/test/java/com/microsoft/playwright/TestBrowserContextBasic.java
@@ -299,6 +299,13 @@ void waitForConditionPageClosed(BrowserContext context) {
     assertTrue(e.getMessage().contains("Target page, context or browser has been closed"), e.getMessage());
   }
 
+  @Test
+  void waitForConditionThatMayChangeToFalse(BrowserContext context) {
+    int[] var = {0};
+    context.waitForCondition(() -> ++var[0] == 1);
+    assertEquals(1, var[0], "The predicate should be called only once.");
+  }
+
   @Test
   void shouldPropagateCloseReasonToPendingActions(Browser browser) {
     BrowserContext context = browser.newContext();