Skip to content

Commit ec836e1

Browse files
test: fix CDP WebSocket connection leaks in DevToolsWrapper (#23553) (#23633)
Cache DevTools and Domains instances with lazy initialization so only one CDP WebSocket is opened per wrapper lifetime, instead of creating a new Augmenter and connection on every call to getDevTools()/getDomains(). Add close() method to DevToolsWrapper and call it from an @after hook in ChromeDeviceTest to ensure the WebSocket is properly closed after each test. Fix TargetID filtering in attachToTargets() to compare by string value instead of object identity, which could cause duplicate session attachments. Pass ClientConfig.defaultConfig() to SeleniumCdpConnection.create() for correct connection configuration. Pin Chrome 143.0 for remote Grid tests as a temporary workaround for CDP WebSocket connection issues in CI. Co-authored-by: Marco Collovati <marco@vaadin.com>
1 parent e0aaff4 commit ec836e1

File tree

2 files changed

+44
-11
lines changed

2 files changed

+44
-11
lines changed

flow-test-util/src/main/java/com/vaadin/flow/testutil/ChromeDeviceTest.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.util.List;
2323
import java.util.Map;
2424

25+
import org.junit.After;
2526
import org.junit.Assert;
2627
import org.junit.Before;
2728
import org.junit.experimental.categories.Category;
@@ -32,6 +33,8 @@
3233
import org.openqa.selenium.chrome.ChromeOptions;
3334
import org.openqa.selenium.remote.DesiredCapabilities;
3435
import org.openqa.selenium.remote.RemoteWebDriver;
36+
import org.slf4j.Logger;
37+
import org.slf4j.LoggerFactory;
3538

3639
import com.vaadin.flow.testcategory.ChromeTests;
3740
import com.vaadin.testbench.TestBench;
@@ -56,6 +59,8 @@
5659
*/
5760
@Category(ChromeTests.class)
5861
public class ChromeDeviceTest extends ViewOrUITest {
62+
private static final Logger log = LoggerFactory
63+
.getLogger(ChromeDeviceTest.class);
5964
private DevToolsWrapper devTools = null;
6065

6166
protected DevToolsWrapper getDevTools() {
@@ -77,6 +82,11 @@ public void setup() throws Exception {
7782
if (Browser.CHROME == getRunLocallyBrowser()) {
7883
driver = new ChromeDriver(chromeOptions);
7984
} else {
85+
// Temporary workaround for dev tools websocket connection errors
86+
// in the CI environment.
87+
log.warn(
88+
"Forcing Chrome 143.0 for tests using Selenium dev tools to avoid websocket connection issues in CI");
89+
chromeOptions.setBrowserVersion("143.0");
8090
URL remoteURL = new URL(getHubURL());
8191
driver = new RemoteWebDriver(remoteURL, chromeOptions);
8292
setDevToolsRuntimeCapabilities((RemoteWebDriver) driver, remoteURL);
@@ -87,6 +97,11 @@ public void setup() throws Exception {
8797
setDriver(TestBench.createDriver(driver));
8898
}
8999

100+
@After
101+
public void closeDevTools() {
102+
devTools.close();
103+
}
104+
90105
/**
91106
* Customizes given Chrome options to enable network connection emulation.
92107
*

flow-test-util/src/main/java/com/vaadin/flow/testutil/DevToolsWrapper.java

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,15 @@
3030
import org.openqa.selenium.devtools.idealized.target.model.TargetID;
3131
import org.openqa.selenium.devtools.v144.network.Network;
3232
import org.openqa.selenium.remote.Augmenter;
33-
import org.openqa.selenium.remote.RemoteWebDriver;
33+
import org.openqa.selenium.remote.http.ClientConfig;
3434

3535
public class DevToolsWrapper {
3636
private final WebDriver driver;
3737
private final Duration timeout = Duration.ofSeconds(3);
3838
private final HashMap<TargetID, SessionID> attachedTargets = new HashMap<TargetID, SessionID>();
3939
private Connection connection = null;
40+
private DevTools devTools = null;
41+
private Domains domains = null;
4042

4143
public DevToolsWrapper(WebDriver driver) {
4244
this.driver = driver;
@@ -70,27 +72,37 @@ public void setCacheDisabled(Boolean isDisabled) {
7072
sendToAllTargets(Network.setCacheDisabled(isDisabled));
7173
}
7274

75+
public void close() {
76+
if (devTools != null) {
77+
devTools.close();
78+
}
79+
if (connection != null) {
80+
connection.close();
81+
}
82+
}
83+
7384
/**
7485
* Creates a custom DevTools CDP connection if there is not one yet.
75-
*
86+
* <p>
7687
* Note, there is already a CDP connection provided by {@link DevTools} but
7788
* it allows sending commands only to the page session whereas we need to
7889
* also send commands to service workers. Therefore a custom connection is
7990
* necessary.
8091
*/
8192
private void createConnectionIfThereIsNotOne() {
8293
if (connection == null) {
83-
connection = SeleniumCdpConnection.create(driver).get();
94+
connection = SeleniumCdpConnection
95+
.create(driver, ClientConfig.defaultConfig()).get();
8496
}
8597
}
8698

8799
/**
88100
* Attaches to all the available targets by creating a session per each.
89101
* These sessions can be later used for sending commands to the
90102
* corresponding targets.
91-
*
103+
* <p>
92104
* Every target represents a certain browser page, service worker and etc.
93-
*
105+
* <p>
94106
* Read more about targets and sessions here:
95107
* https://github.com/aslushnikov/getting-started-with-cdp#targets--sessions
96108
*/
@@ -100,8 +112,9 @@ private void attachToAllTargets() {
100112
connection
101113
.sendAndWait(null, getDomains().target().getTargets(), timeout)
102114
.stream()
103-
.filter((target) -> !attachedTargets
104-
.containsKey(target.getTargetId()))
115+
.filter((target) -> attachedTargets.keySet().stream()
116+
.noneMatch(t -> t.toString()
117+
.equals(target.getTargetId().toString())))
105118
.forEach((target) -> {
106119
TargetID targetId = target.getTargetId();
107120
SessionID sessionId = connection.sendAndWait(null,
@@ -123,12 +136,17 @@ private <X> void sendToAllTargets(Command<X> command) {
123136
}
124137

125138
private DevTools getDevTools() {
126-
WebDriver driver = new Augmenter()
127-
.augment((RemoteWebDriver) this.driver);
128-
return ((HasDevTools) driver).getDevTools();
139+
if (devTools == null) {
140+
WebDriver augmented = new Augmenter().augment(this.driver);
141+
devTools = ((HasDevTools) augmented).getDevTools();
142+
}
143+
return devTools;
129144
}
130145

131146
private Domains getDomains() {
132-
return getDevTools().getDomains();
147+
if (domains == null) {
148+
domains = getDevTools().getDomains();
149+
}
150+
return domains;
133151
}
134152
}

0 commit comments

Comments
 (0)