Skip to content

Commit d6a181e

Browse files
committed
fix: waitForCondition should not call predicate after it returned true
1 parent eb08046 commit d6a181e

File tree

2 files changed

+17
-8
lines changed

2 files changed

+17
-8
lines changed

playwright/src/main/java/com/microsoft/playwright/impl/WaitableRace.java

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,20 @@
2020

2121
class WaitableRace<T> implements Waitable<T> {
2222
private final Collection<Waitable<T>> waitables;
23+
private Waitable<T> firstReady;
2324

2425
WaitableRace(Collection<Waitable<T>> waitables) {
2526
this.waitables = waitables;
2627
}
2728

2829
@Override
2930
public boolean isDone() {
30-
for (Waitable w : waitables) {
31+
if (firstReady != null) {
32+
return true;
33+
}
34+
for (Waitable<T> w : waitables) {
3135
if (w.isDone()) {
36+
firstReady = w;
3237
return true;
3338
}
3439
}
@@ -37,14 +42,11 @@ public boolean isDone() {
3742

3843
@Override
3944
public T get() {
40-
assert isDone();
41-
dispose();
42-
for (Waitable<T> w : waitables) {
43-
if (w.isDone()) {
44-
return w.get();
45-
}
45+
try {
46+
return firstReady.get();
47+
} finally {
48+
dispose();
4649
}
47-
throw new IllegalStateException("At least one element must be ready");
4850
}
4951

5052
@Override

playwright/src/test/java/com/microsoft/playwright/TestBrowserContextBasic.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,13 @@ void waitForConditionPageClosed(BrowserContext context) {
299299
assertTrue(e.getMessage().contains("Target page, context or browser has been closed"), e.getMessage());
300300
}
301301

302+
@Test
303+
void waitForConditionThatMayChangeToFalse(BrowserContext context) {
304+
int[] var = {0};
305+
context.waitForCondition(() -> ++var[0] == 1);
306+
assertEquals(1, var[0], "The predicate should be called only once.");
307+
}
308+
302309
@Test
303310
void shouldPropagateCloseReasonToPendingActions(Browser browser) {
304311
BrowserContext context = browser.newContext();

0 commit comments

Comments
 (0)