From 1d69d924cfbeb1d6be8a9899d8c1e78eb3f77ec4 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Tue, 7 Sep 2021 12:59:28 -0700 Subject: [PATCH] fix: respect predicate in waitFor* methods (#596) --- .../playwright/impl/BrowserContextImpl.java | 6 +- .../microsoft/playwright/impl/PageImpl.java | 27 ++-- .../playwright/impl/WebSocketImpl.java | 3 +- .../playwright/TestPageEventConsole.java | 133 ++++++++++++++++++ 4 files changed, 149 insertions(+), 20 deletions(-) create mode 100644 playwright/src/test/java/com/microsoft/playwright/TestPageEventConsole.java diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/BrowserContextImpl.java b/playwright/src/main/java/com/microsoft/playwright/impl/BrowserContextImpl.java index e0d53e3f..f37cfae9 100644 --- a/playwright/src/main/java/com/microsoft/playwright/impl/BrowserContextImpl.java +++ b/playwright/src/main/java/com/microsoft/playwright/impl/BrowserContextImpl.java @@ -144,9 +144,9 @@ class BrowserContextImpl extends ChannelOwner implements BrowserContext { listeners.remove(EventType.RESPONSE, handler); } - private T waitForEventWithTimeout(EventType eventType, Runnable code, Double timeout) { + private T waitForEventWithTimeout(EventType eventType, Runnable code, Predicate predicate, Double timeout) { List> waitables = new ArrayList<>(); - waitables.add(new WaitableEvent<>(listeners, eventType)); + waitables.add(new WaitableEvent<>(listeners, eventType, predicate)); waitables.add(new WaitableContextClose<>()); waitables.add(timeoutSettings.createWaitable(timeout)); return runUntil(code, new WaitableRace<>(waitables)); @@ -161,7 +161,7 @@ class BrowserContextImpl extends ChannelOwner implements BrowserContext { if (options == null) { options = new WaitForPageOptions(); } - return waitForEventWithTimeout(EventType.PAGE, code, options.timeout); + return waitForEventWithTimeout(EventType.PAGE, code, options.predicate, options.timeout); } @Override diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/PageImpl.java b/playwright/src/main/java/com/microsoft/playwright/impl/PageImpl.java index ace2f984..20663a86 100644 --- a/playwright/src/main/java/com/microsoft/playwright/impl/PageImpl.java +++ b/playwright/src/main/java/com/microsoft/playwright/impl/PageImpl.java @@ -445,7 +445,7 @@ public class PageImpl extends ChannelOwner implements Page { if (options == null) { options = new WaitForCloseOptions(); } - return waitForEventWithTimeout(EventType.CLOSE, code, options.timeout); + return waitForEventWithTimeout(EventType.CLOSE, code, null, options.timeout); } @Override @@ -457,7 +457,7 @@ public class PageImpl extends ChannelOwner implements Page { if (options == null) { options = new WaitForConsoleMessageOptions(); } - return waitForEventWithTimeout(EventType.CONSOLE, code, options.timeout); + return waitForEventWithTimeout(EventType.CONSOLE, code, options.predicate, options.timeout); } @Override @@ -469,7 +469,7 @@ public class PageImpl extends ChannelOwner implements Page { if (options == null) { options = new WaitForDownloadOptions(); } - return waitForEventWithTimeout(EventType.DOWNLOAD, code, options.timeout); + return waitForEventWithTimeout(EventType.DOWNLOAD, code, options.predicate, options.timeout); } @Override @@ -482,7 +482,7 @@ public class PageImpl extends ChannelOwner implements Page { if (options == null) { options = new WaitForFileChooserOptions(); } - return waitForEventWithTimeout(EventType.FILECHOOSER, code, options.timeout); + return waitForEventWithTimeout(EventType.FILECHOOSER, code, options.predicate, options.timeout); } @Override @@ -494,7 +494,7 @@ public class PageImpl extends ChannelOwner implements Page { if (options == null) { options = new WaitForPopupOptions(); } - return waitForEventWithTimeout(EventType.POPUP, code, options.timeout); + return waitForEventWithTimeout(EventType.POPUP, code, options.predicate, options.timeout); } @Override @@ -506,7 +506,7 @@ public class PageImpl extends ChannelOwner implements Page { if (options == null) { options = new WaitForWebSocketOptions(); } - return waitForEventWithTimeout(EventType.WEBSOCKET, code, options.timeout); + return waitForEventWithTimeout(EventType.WEBSOCKET, code, options.predicate, options.timeout); } @Override @@ -518,12 +518,12 @@ public class PageImpl extends ChannelOwner implements Page { if (options == null) { options = new WaitForWorkerOptions(); } - return waitForEventWithTimeout(EventType.WORKER, code, options.timeout); + return waitForEventWithTimeout(EventType.WORKER, code, options.predicate, options.timeout); } - private T waitForEventWithTimeout(EventType eventType, Runnable code, Double timeout) { + private T waitForEventWithTimeout(EventType eventType, Runnable code, Predicate predicate, Double timeout) { List> waitables = new ArrayList<>(); - waitables.add(new WaitableEvent<>(listeners, eventType)); + waitables.add(new WaitableEvent<>(listeners, eventType, predicate)); waitables.add(createWaitForCloseHelper()); waitables.add(createWaitableTimeout(timeout)); return runUntil(code, new WaitableRace<>(waitables)); @@ -1313,8 +1313,7 @@ public class PageImpl extends ChannelOwner implements Page { options = new WaitForRequestOptions(); } List> waitables = new ArrayList<>(); - waitables.add(new WaitableEvent<>(listeners, EventType.REQUEST, - request -> predicate == null || predicate.test(request))); + waitables.add(new WaitableEvent<>(listeners, EventType.REQUEST, predicate)); waitables.add(createWaitForCloseHelper()); waitables.add(createWaitableTimeout(options.timeout)); return runUntil(code, new WaitableRace<>(waitables)); @@ -1331,8 +1330,7 @@ public class PageImpl extends ChannelOwner implements Page { } List> waitables = new ArrayList<>(); Predicate predicate = options.predicate; - waitables.add(new WaitableEvent<>(listeners, EventType.REQUESTFINISHED, - request -> predicate == null || predicate.test(request))); + waitables.add(new WaitableEvent<>(listeners, EventType.REQUESTFINISHED, predicate)); waitables.add(createWaitForCloseHelper()); waitables.add(createWaitableTimeout(options.timeout)); return runUntil(code, new WaitableRace<>(waitables)); @@ -1362,8 +1360,7 @@ public class PageImpl extends ChannelOwner implements Page { options = new WaitForResponseOptions(); } List> waitables = new ArrayList<>(); - waitables.add(new WaitableEvent<>(listeners, EventType.RESPONSE, - response -> predicate == null || predicate.test(response))); + waitables.add(new WaitableEvent<>(listeners, EventType.RESPONSE, predicate)); waitables.add(createWaitForCloseHelper()); waitables.add(createWaitableTimeout(options.timeout)); return runUntil(code, new WaitableRace<>(waitables)); diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/WebSocketImpl.java b/playwright/src/main/java/com/microsoft/playwright/impl/WebSocketImpl.java index e27aa87d..3a1a1a68 100644 --- a/playwright/src/main/java/com/microsoft/playwright/impl/WebSocketImpl.java +++ b/playwright/src/main/java/com/microsoft/playwright/impl/WebSocketImpl.java @@ -143,8 +143,7 @@ class WebSocketImpl extends ChannelOwner implements WebSocket { private WebSocketFrame waitForEventWithTimeout(EventType eventType, Runnable code, Predicate predicate, Double timeout) { List> waitables = new ArrayList<>(); - waitables.add(new WaitableEvent<>(listeners, eventType, - frame -> predicate == null || predicate.test(frame))); + waitables.add(new WaitableEvent<>(listeners, eventType, predicate)); waitables.add(new WaitableWebSocketClose<>()); waitables.add(new WaitableWebSocketError<>()); waitables.add(page.createWaitForCloseHelper()); diff --git a/playwright/src/test/java/com/microsoft/playwright/TestPageEventConsole.java b/playwright/src/test/java/com/microsoft/playwright/TestPageEventConsole.java new file mode 100644 index 00000000..feeb8bfd --- /dev/null +++ b/playwright/src/test/java/com/microsoft/playwright/TestPageEventConsole.java @@ -0,0 +1,133 @@ +/* + * Copyright (c) Microsoft Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.microsoft.playwright; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledIf; + +import java.util.ArrayList; +import java.util.List; + +import static com.microsoft.playwright.Utils.getOS; +import static com.microsoft.playwright.Utils.mapOf; +import static java.util.Arrays.asList; +import static java.util.stream.Collectors.toList; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class TestPageEventConsole extends TestBase { + @Test + void shouldWork() { + ConsoleMessage[] event = {null}; + page.onConsoleMessage(m -> event[0] = m); + ConsoleMessage message = page.waitForConsoleMessage(() -> page.evaluate("() => console.log('hello', 5, {foo: 'bar'});")); + if (isFirefox()) { + assertEquals("hello 5 JSHandle@object", message.text()); + } else { + assertEquals("hello 5 {foo: bar}", message.text()); + } + assertEquals("log", message.type()); + assertEquals("hello", message.args().get(0).jsonValue()); + assertEquals(5, message.args().get(1).jsonValue()); + assertEquals(mapOf("foo", "bar"), message.args().get(2).jsonValue()); + assertEquals(message, event[0]); + } + + @Test + void shouldEmitSameLogTwice() { + List messages = new ArrayList<>(); + page.onConsoleMessage(m -> messages.add(m.text())); + page.evaluate("() => { for (let i = 0; i < 2; ++i) console.log('hello'); }"); + assertEquals(asList("hello", "hello"), messages); + } + + @Test + void shouldWorkForDifferentConsoleAPICalls() { + List messages = new ArrayList<>(); + page.onConsoleMessage(msg -> messages.add(msg)); + // All console events will be reported before "page.evaluate" is finished. + page.evaluate("() => {\n" + + " // A pair of time/timeEnd generates only one Console API call.\n" + + " console.time('calling console.time');\n" + + " console.timeEnd('calling console.time');\n" + + " console.trace('calling console.trace');\n" + + " console.dir('calling console.dir');\n" + + " console.warn('calling console.warn');\n" + + " console.error('calling console.error');\n" + + " console.log(Promise.resolve('should not wait until resolved!'));\n" + + " }"); + assertEquals(asList("timeEnd", "trace", "dir", "warning", "error", "log"), + messages.stream().map(msg -> msg.type()).collect(toList())); + assertTrue(messages.get(0).text().contains("calling console.time")); + + assertEquals(asList( + "calling console.trace", + "calling console.dir", + "calling console.warn", + "calling console.error", + "Promise"), messages.subList(1, messages.size()).stream().map(msg -> msg.text()).collect(toList())); + } + + @Test + void shouldNotFailForWindowObject() { + ConsoleMessage message = page.waitForConsoleMessage(() -> page.evaluate("console.error(window)")); + if (isFirefox()) { + assertEquals("JSHandle@object", message.text()); + } else { + assertEquals("Window", message.text()); + } + } + + static boolean isWebKitWindows() { + return isWebKit() && getOS() == Utils.OS.WINDOWS; + } + + @Test + @DisabledIf(value="isWebKitWindows", disabledReason="Upstream issue https://bugs.webkit.org/show_bug.cgi?id=229515") + void shouldTriggerCorrectLog() { + page.navigate("about:blank"); + ConsoleMessage message = page.waitForConsoleMessage(() -> { + page.evaluate("async url => fetch(url).catch(e => {})", server.EMPTY_PAGE); + }); + assertTrue(message.text().contains("Access-Control-Allow-Origin")); + assertEquals("error", message.type()); + } + + @Test + void shouldHaveLocationForConsoleAPICalls() { + page.navigate(server.EMPTY_PAGE); + ConsoleMessage message = page.waitForConsoleMessage( + new Page.WaitForConsoleMessageOptions().setPredicate(m -> "yellow".equals(m.text())), + () -> page.navigate(server.PREFIX + "/consolelog.html")); + assertEquals("log", message.type()); + // Engines have different column notion. + assertTrue(message.location().startsWith(server.PREFIX + "/consolelog.html:7:"), message.location()); + } + + @Test + void shouldSupportPredicate() { + page.navigate(server.EMPTY_PAGE); + ConsoleMessage message = page.waitForConsoleMessage( + new Page.WaitForConsoleMessageOptions().setPredicate(m -> "info".equals(m.type())), + () -> { + page.evaluate("console.log(1)"); + page.evaluate("console.info(2)"); + }); + assertEquals("2", message.text()); + assertEquals("info", message.type()); + } +}