From c9c6236400af20caa9af43fa93d4db3d65a3444c Mon Sep 17 00:00:00 2001 From: Viet Nguyen Duc Date: Mon, 21 Oct 2024 02:59:06 +0000 Subject: [PATCH 1/4] [grid] Update for node shutdown gracefully Signed-off-by: Viet Nguyen Duc --- .../grid/node/ForwardWebDriverCommand.java | 15 +++- .../org/openqa/selenium/grid/node/Node.java | 2 +- .../selenium/grid/node/local/LocalNode.java | 17 ++++- .../org/openqa/selenium/grid/e2e/BUILD.bazel | 20 +++++ .../selenium/grid/e2e/SessionTimeoutTest.java | 51 +++++++++++++ .../node/ForwardWebDriverCommandTest.java | 76 +++++++++++++++++++ .../openqa/selenium/grid/node/NodeTest.java | 44 ++++++++++- 7 files changed, 216 insertions(+), 9 deletions(-) create mode 100644 java/test/org/openqa/selenium/grid/e2e/BUILD.bazel create mode 100644 java/test/org/openqa/selenium/grid/e2e/SessionTimeoutTest.java create mode 100644 java/test/org/openqa/selenium/grid/node/ForwardWebDriverCommandTest.java diff --git a/java/src/org/openqa/selenium/grid/node/ForwardWebDriverCommand.java b/java/src/org/openqa/selenium/grid/node/ForwardWebDriverCommand.java index 458dfff372cbe..4c8a03796c7db 100644 --- a/java/src/org/openqa/selenium/grid/node/ForwardWebDriverCommand.java +++ b/java/src/org/openqa/selenium/grid/node/ForwardWebDriverCommand.java @@ -17,7 +17,11 @@ package org.openqa.selenium.grid.node; +import static org.openqa.selenium.remote.HttpSessionId.getSessionId; + +import org.openqa.selenium.NoSuchSessionException; import org.openqa.selenium.internal.Require; +import org.openqa.selenium.remote.SessionId; import org.openqa.selenium.remote.http.HttpHandler; import org.openqa.selenium.remote.http.HttpRequest; import org.openqa.selenium.remote.http.HttpResponse; @@ -30,8 +34,17 @@ class ForwardWebDriverCommand implements HttpHandler { this.node = Require.nonNull("Node", node); } + public boolean matches(HttpRequest req) { + return getSessionId(req.getUri()) + .map(id -> node.isSessionOwner(new SessionId(id))) + .orElse(false); + } + @Override public HttpResponse execute(HttpRequest req) { - return node.executeWebDriverCommand(req); + if (matches(req)) { + return node.executeWebDriverCommand(req); + } + throw new NoSuchSessionException(String.format("Session not found in node %s", node.getId())); } } diff --git a/java/src/org/openqa/selenium/grid/node/Node.java b/java/src/org/openqa/selenium/grid/node/Node.java index 5d54bb3e2981d..09fe7d02ae9f5 100644 --- a/java/src/org/openqa/selenium/grid/node/Node.java +++ b/java/src/org/openqa/selenium/grid/node/Node.java @@ -152,7 +152,7 @@ protected Node( req -> getSessionId(req.getUri()) .map(SessionId::new) - .map(this::isSessionOwner) + .map(sessionId -> this.getSession(sessionId) != null) .orElse(false)) .to(() -> new ForwardWebDriverCommand(this)) .with(spanDecorator("node.forward_command")), diff --git a/java/src/org/openqa/selenium/grid/node/local/LocalNode.java b/java/src/org/openqa/selenium/grid/node/local/LocalNode.java index 2283e8573042d..7304db8a87847 100644 --- a/java/src/org/openqa/selenium/grid/node/local/LocalNode.java +++ b/java/src/org/openqa/selenium/grid/node/local/LocalNode.java @@ -297,7 +297,13 @@ protected LocalNode( heartbeatPeriod.getSeconds(), TimeUnit.SECONDS); - Runtime.getRuntime().addShutdownHook(new Thread(this::stopAllSessions)); + Runtime.getRuntime() + .addShutdownHook( + new Thread( + () -> { + stopAllSessions(); + drain(); + })); new JMXHelper().register(this); } @@ -316,7 +322,6 @@ private void stopTimedOutSession(RemovalNotification not } // Attempt to stop the session slot.stop(); - this.sessionToDownloadsDir.invalidate(id); // Decrement pending sessions if Node is draining if (this.isDraining()) { int done = pendingSessions.decrementAndGet(); @@ -473,8 +478,6 @@ public Either newSession( sessionToDownloadsDir.put(session.getId(), uuidForSessionDownloads); currentSessions.put(session.getId(), slotToUse); - checkSessionCount(); - SessionId sessionId = session.getId(); Capabilities caps = session.getCapabilities(); SESSION_ID.accept(span, sessionId); @@ -513,6 +516,8 @@ public Either newSession( span.addEvent("Unable to create session with the driver", attributeMap); return Either.left(possibleSession.left()); } + } finally { + checkSessionCount(); } } @@ -765,6 +770,10 @@ public HttpResponse uploadFile(HttpRequest req, SessionId id) { public void stop(SessionId id) throws NoSuchSessionException { Require.nonNull("Session ID", id); + if (sessionToDownloadsDir.getIfPresent(id) != null) { + sessionToDownloadsDir.invalidate(id); + } + SessionSlot slot = currentSessions.getIfPresent(id); if (slot == null) { throw new NoSuchSessionException("Cannot find session with id: " + id); diff --git a/java/test/org/openqa/selenium/grid/e2e/BUILD.bazel b/java/test/org/openqa/selenium/grid/e2e/BUILD.bazel new file mode 100644 index 0000000000000..da013678c69c1 --- /dev/null +++ b/java/test/org/openqa/selenium/grid/e2e/BUILD.bazel @@ -0,0 +1,20 @@ +load("@rules_jvm_external//:defs.bzl", "artifact") +load("//java:defs.bzl", "JUNIT5_DEPS", "java_selenium_test_suite") + +java_selenium_test_suite( + name = "large-tests", + size = "large", + srcs = glob(["*.java"]), + browsers = [ + "chrome", + ], + deps = [ + "//java/src/org/openqa/selenium:core", + "//java/src/org/openqa/selenium/chrome", + "//java/src/org/openqa/selenium/grid", + "//java/src/org/openqa/selenium/remote/http", + artifact("com.google.guava:guava"), + artifact("org.junit.jupiter:junit-jupiter-api"), + artifact("org.assertj:assertj-core"), + ] + JUNIT5_DEPS, +) diff --git a/java/test/org/openqa/selenium/grid/e2e/SessionTimeoutTest.java b/java/test/org/openqa/selenium/grid/e2e/SessionTimeoutTest.java new file mode 100644 index 0000000000000..cf9df66fdd91e --- /dev/null +++ b/java/test/org/openqa/selenium/grid/e2e/SessionTimeoutTest.java @@ -0,0 +1,51 @@ +// Licensed to the Software Freedom Conservancy (SFC) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The SFC licenses this file +// to you 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 org.openqa.selenium.grid.e2e; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.net.URI; +import org.assertj.core.api.Assumptions; +import org.junit.jupiter.api.Test; +import org.openqa.selenium.NoSuchSessionException; +import org.openqa.selenium.WebDriver; +import org.openqa.selenium.chrome.ChromeOptions; +import org.openqa.selenium.grid.Bootstrap; +import org.openqa.selenium.remote.RemoteWebDriver; + +public class SessionTimeoutTest { + @Test + void testSessionTimeout() throws Exception { + Assumptions.assumeThat(System.getProperty("webdriver.chrome.binary")).isNull(); + Bootstrap.main(("hub --host 127.0.0.1 --port 4444").split(" ")); + Bootstrap.main( + ("node --host 127.0.0.1 --port 5555 --session-timeout 12 --selenium-manager true") + .split(" ")); + + var options = new ChromeOptions(); + options.addArguments("--disable-search-engine-choice-screen"); + options.addArguments("--headless=new"); + + WebDriver driver = new RemoteWebDriver(URI.create("http://localhost:4444").toURL(), options); + driver.get("http://localhost:4444"); + Thread.sleep(12000); + NoSuchSessionException exception = assertThrows(NoSuchSessionException.class, driver::getTitle); + assertTrue(exception.getMessage().startsWith("Cannot find session with id:")); + } +} diff --git a/java/test/org/openqa/selenium/grid/node/ForwardWebDriverCommandTest.java b/java/test/org/openqa/selenium/grid/node/ForwardWebDriverCommandTest.java new file mode 100644 index 0000000000000..721db0b32c7d2 --- /dev/null +++ b/java/test/org/openqa/selenium/grid/node/ForwardWebDriverCommandTest.java @@ -0,0 +1,76 @@ +// Licensed to the Software Freedom Conservancy (SFC) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The SFC licenses this file +// to you 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 org.openqa.selenium.grid.node; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.*; + +import java.util.UUID; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.openqa.selenium.NoSuchSessionException; +import org.openqa.selenium.grid.data.NodeId; +import org.openqa.selenium.remote.SessionId; +import org.openqa.selenium.remote.http.HttpRequest; +import org.openqa.selenium.remote.http.HttpResponse; + +class ForwardWebDriverCommandTest { + + private Node mockNode; + private ForwardWebDriverCommand command; + + @BeforeEach + void setUp() { + mockNode = mock(Node.class); + when(mockNode.getId()).thenReturn(new NodeId(UUID.randomUUID())); + command = new ForwardWebDriverCommand(mockNode); + } + + @Test + void testExecuteWithValidSessionOwner() { + HttpRequest mockRequest = mock(HttpRequest.class); + when(mockRequest.getUri()).thenReturn("/session/1234"); + + SessionId sessionId = new SessionId("1234"); + when(mockNode.isSessionOwner(sessionId)).thenReturn(true); + + HttpResponse expectedResponse = new HttpResponse(); + when(mockNode.executeWebDriverCommand(mockRequest)).thenReturn(expectedResponse); + + HttpResponse actualResponse = command.execute(mockRequest); + assertEquals(expectedResponse, actualResponse); + } + + @Test + void testExecuteWithInvalidSessionOwner() { + HttpRequest mockRequest = mock(HttpRequest.class); + when(mockRequest.getUri()).thenReturn("/session/5678"); + + SessionId sessionId = new SessionId("5678"); + when(mockNode.isSessionOwner(sessionId)).thenReturn(false); + + NoSuchSessionException exception = + assertThrows(NoSuchSessionException.class, () -> command.execute(mockRequest)); + assertTrue( + exception + .getMessage() + .startsWith(String.format("Session not found in node %s", mockNode.getId()))); + } +} diff --git a/java/test/org/openqa/selenium/grid/node/NodeTest.java b/java/test/org/openqa/selenium/grid/node/NodeTest.java index b1c50ed0dc65a..c2a66d2162c4b 100644 --- a/java/test/org/openqa/selenium/grid/node/NodeTest.java +++ b/java/test/org/openqa/selenium/grid/node/NodeTest.java @@ -23,6 +23,8 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.InstanceOfAssertFactories.MAP; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.openqa.selenium.json.Json.MAP_TYPE; import static org.openqa.selenium.remote.http.Contents.string; import static org.openqa.selenium.remote.http.HttpMethod.DELETE; @@ -102,7 +104,9 @@ class NodeTest { private Tracer tracer; private EventBus bus; private LocalNode local; + private LocalNode local2; private Node node; + private Node node2; private ImmutableCapabilities stereotype; private ImmutableCapabilities caps; private URI uri; @@ -150,6 +154,7 @@ public HttpResponse execute(HttpRequest req) throws UncheckedIOException { builder = builder.enableManagedDownloads(true).sessionTimeout(Duration.ofSeconds(1)); } local = builder.build(); + local2 = builder.build(); node = new RemoteNode( @@ -160,6 +165,16 @@ public HttpResponse execute(HttpRequest req) throws UncheckedIOException { registrationSecret, local.getSessionTimeout(), ImmutableSet.of(caps)); + + node2 = + new RemoteNode( + tracer, + new PassthroughHttpClient.Factory(local2), + new NodeId(UUID.randomUUID()), + uri, + registrationSecret, + local2.getSessionTimeout(), + ImmutableSet.of(caps)); } @Test @@ -371,13 +386,36 @@ void shouldOnlyRespondToWebDriverCommandsForSessionsTheNodeOwns() { assertThatEither(response).isRight(); Session session = response.right().getSession(); + Either response2 = + node2.newSession(createSessionRequest(caps)); + assertThatEither(response2).isRight(); + Session session2 = response2.right().getSession(); + + // Assert that should respond to commands for sessions Node 1 owns HttpRequest req = new HttpRequest(POST, String.format("/session/%s/url", session.getId())); assertThat(local.matches(req)).isTrue(); assertThat(node.matches(req)).isTrue(); - req = new HttpRequest(POST, String.format("/session/%s/url", UUID.randomUUID())); - assertThat(local.matches(req)).isFalse(); - assertThat(node.matches(req)).isFalse(); + // Assert that should respond to commands for sessions Node 2 owns + HttpRequest req2 = new HttpRequest(POST, String.format("/session/%s/url", session2.getId())); + assertThat(local2.matches(req2)).isTrue(); + assertThat(node2.matches(req2)).isTrue(); + + // Assert that should not respond to commands for sessions Node 1 does not own + NoSuchSessionException exception = + assertThrows(NoSuchSessionException.class, () -> node.execute(req2)); + assertTrue( + exception + .getMessage() + .startsWith(String.format("Cannot find session with id: %s", session2.getId()))); + + // Assert that should not respond to commands for sessions Node 2 does not own + NoSuchSessionException exception2 = + assertThrows(NoSuchSessionException.class, () -> node2.execute(req)); + assertTrue( + exception2 + .getMessage() + .startsWith(String.format("Cannot find session with id: %s", session.getId()))); } @Test From 411df2567125b6bdddf1f0975989625be1ca9245 Mon Sep 17 00:00:00 2001 From: Viet Nguyen Duc Date: Tue, 22 Oct 2024 06:23:21 +0000 Subject: [PATCH 2/4] Fixed tests and race condition potential Signed-off-by: Viet Nguyen Duc --- .../org/openqa/selenium/grid/e2e/BUILD.bazel | 20 -------- .../selenium/grid/e2e/SessionTimeoutTest.java | 51 ------------------- .../selenium/grid/router/StressTest.java | 21 +++++++- 3 files changed, 20 insertions(+), 72 deletions(-) delete mode 100644 java/test/org/openqa/selenium/grid/e2e/BUILD.bazel delete mode 100644 java/test/org/openqa/selenium/grid/e2e/SessionTimeoutTest.java diff --git a/java/test/org/openqa/selenium/grid/e2e/BUILD.bazel b/java/test/org/openqa/selenium/grid/e2e/BUILD.bazel deleted file mode 100644 index da013678c69c1..0000000000000 --- a/java/test/org/openqa/selenium/grid/e2e/BUILD.bazel +++ /dev/null @@ -1,20 +0,0 @@ -load("@rules_jvm_external//:defs.bzl", "artifact") -load("//java:defs.bzl", "JUNIT5_DEPS", "java_selenium_test_suite") - -java_selenium_test_suite( - name = "large-tests", - size = "large", - srcs = glob(["*.java"]), - browsers = [ - "chrome", - ], - deps = [ - "//java/src/org/openqa/selenium:core", - "//java/src/org/openqa/selenium/chrome", - "//java/src/org/openqa/selenium/grid", - "//java/src/org/openqa/selenium/remote/http", - artifact("com.google.guava:guava"), - artifact("org.junit.jupiter:junit-jupiter-api"), - artifact("org.assertj:assertj-core"), - ] + JUNIT5_DEPS, -) diff --git a/java/test/org/openqa/selenium/grid/e2e/SessionTimeoutTest.java b/java/test/org/openqa/selenium/grid/e2e/SessionTimeoutTest.java deleted file mode 100644 index cf9df66fdd91e..0000000000000 --- a/java/test/org/openqa/selenium/grid/e2e/SessionTimeoutTest.java +++ /dev/null @@ -1,51 +0,0 @@ -// Licensed to the Software Freedom Conservancy (SFC) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The SFC licenses this file -// to you 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 org.openqa.selenium.grid.e2e; - -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; - -import java.net.URI; -import org.assertj.core.api.Assumptions; -import org.junit.jupiter.api.Test; -import org.openqa.selenium.NoSuchSessionException; -import org.openqa.selenium.WebDriver; -import org.openqa.selenium.chrome.ChromeOptions; -import org.openqa.selenium.grid.Bootstrap; -import org.openqa.selenium.remote.RemoteWebDriver; - -public class SessionTimeoutTest { - @Test - void testSessionTimeout() throws Exception { - Assumptions.assumeThat(System.getProperty("webdriver.chrome.binary")).isNull(); - Bootstrap.main(("hub --host 127.0.0.1 --port 4444").split(" ")); - Bootstrap.main( - ("node --host 127.0.0.1 --port 5555 --session-timeout 12 --selenium-manager true") - .split(" ")); - - var options = new ChromeOptions(); - options.addArguments("--disable-search-engine-choice-screen"); - options.addArguments("--headless=new"); - - WebDriver driver = new RemoteWebDriver(URI.create("http://localhost:4444").toURL(), options); - driver.get("http://localhost:4444"); - Thread.sleep(12000); - NoSuchSessionException exception = assertThrows(NoSuchSessionException.class, driver::getTitle); - assertTrue(exception.getMessage().startsWith("Cannot find session with id:")); - } -} diff --git a/java/test/org/openqa/selenium/grid/router/StressTest.java b/java/test/org/openqa/selenium/grid/router/StressTest.java index a2c432229297f..d9fb088504e97 100644 --- a/java/test/org/openqa/selenium/grid/router/StressTest.java +++ b/java/test/org/openqa/selenium/grid/router/StressTest.java @@ -20,6 +20,8 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.concurrent.TimeUnit.MINUTES; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.StringReader; import java.util.LinkedList; @@ -33,6 +35,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.openqa.selenium.By; +import org.openqa.selenium.NoSuchSessionException; import org.openqa.selenium.WebDriver; import org.openqa.selenium.grid.config.MapConfig; import org.openqa.selenium.grid.config.MemoizedConfig; @@ -65,7 +68,12 @@ public void setupServers() { DeploymentTypes.DISTRIBUTED.start( browser.getCapabilities(), new TomlConfig( - new StringReader("[node]\n" + "driver-implementation = " + browser.displayName()))); + new StringReader( + "[node]\n" + + "driver-implementation = " + + browser.displayName() + + "\n" + + "session-timeout = 15"))); tearDowns.add(deployment); server = deployment.getServer(); @@ -124,4 +132,15 @@ void multipleSimultaneousSessions() throws Exception { CompletableFuture.allOf(futures).get(4, MINUTES); } + + @Test + void testStopTimedOutSession() throws Exception { + assertThat(server.isStarted()).isTrue(); + WebDriver driver = + RemoteWebDriver.builder().oneOf(browser.getCapabilities()).address(server.getUrl()).build(); + driver.get(appServer.getUrl().toString()); + Thread.sleep(15000); + NoSuchSessionException exception = assertThrows(NoSuchSessionException.class, driver::getTitle); + assertTrue(exception.getMessage().startsWith("Cannot find session with id:")); + } } From fca3b20e1e565a5594dc6e148bef6f0c16b05b0e Mon Sep 17 00:00:00 2001 From: Viet Nguyen Duc Date: Wed, 23 Oct 2024 07:49:03 +0000 Subject: [PATCH 3/4] Return HTTP response in case session is not owned Signed-off-by: Viet Nguyen Duc --- .../grid/node/ForwardWebDriverCommand.java | 11 ++++++++-- .../node/ForwardWebDriverCommandTest.java | 22 +++++++++++-------- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/java/src/org/openqa/selenium/grid/node/ForwardWebDriverCommand.java b/java/src/org/openqa/selenium/grid/node/ForwardWebDriverCommand.java index 4c8a03796c7db..fadcfff4c80fd 100644 --- a/java/src/org/openqa/selenium/grid/node/ForwardWebDriverCommand.java +++ b/java/src/org/openqa/selenium/grid/node/ForwardWebDriverCommand.java @@ -17,9 +17,11 @@ package org.openqa.selenium.grid.node; +import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR; import static org.openqa.selenium.remote.HttpSessionId.getSessionId; +import static org.openqa.selenium.remote.http.Contents.asJson; -import org.openqa.selenium.NoSuchSessionException; +import com.google.common.collect.ImmutableMap; import org.openqa.selenium.internal.Require; import org.openqa.selenium.remote.SessionId; import org.openqa.selenium.remote.http.HttpHandler; @@ -45,6 +47,11 @@ public HttpResponse execute(HttpRequest req) { if (matches(req)) { return node.executeWebDriverCommand(req); } - throw new NoSuchSessionException(String.format("Session not found in node %s", node.getId())); + return new HttpResponse() + .setStatus(HTTP_INTERNAL_ERROR) + .setContent( + asJson( + ImmutableMap.of( + "error", String.format("Session not found in node %s", node.getId())))); } } diff --git a/java/test/org/openqa/selenium/grid/node/ForwardWebDriverCommandTest.java b/java/test/org/openqa/selenium/grid/node/ForwardWebDriverCommandTest.java index 721db0b32c7d2..8f0df29251870 100644 --- a/java/test/org/openqa/selenium/grid/node/ForwardWebDriverCommandTest.java +++ b/java/test/org/openqa/selenium/grid/node/ForwardWebDriverCommandTest.java @@ -17,15 +17,15 @@ package org.openqa.selenium.grid.node; +import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.*; +import static org.openqa.selenium.remote.http.Contents.asJson; +import com.google.common.collect.ImmutableMap; import java.util.UUID; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.openqa.selenium.NoSuchSessionException; import org.openqa.selenium.grid.data.NodeId; import org.openqa.selenium.remote.SessionId; import org.openqa.selenium.remote.http.HttpRequest; @@ -66,11 +66,15 @@ void testExecuteWithInvalidSessionOwner() { SessionId sessionId = new SessionId("5678"); when(mockNode.isSessionOwner(sessionId)).thenReturn(false); - NoSuchSessionException exception = - assertThrows(NoSuchSessionException.class, () -> command.execute(mockRequest)); - assertTrue( - exception - .getMessage() - .startsWith(String.format("Session not found in node %s", mockNode.getId()))); + HttpResponse actualResponse = command.execute(mockRequest); + HttpResponse expectResponse = + new HttpResponse() + .setStatus(HTTP_INTERNAL_ERROR) + .setContent( + asJson( + ImmutableMap.of( + "error", String.format("Session not found in node %s", mockNode.getId())))); + assertEquals(expectResponse.getStatus(), actualResponse.getStatus()); + assertEquals(expectResponse.getContentEncoding(), actualResponse.getContentEncoding()); } } From b5d02d9218becb662c1ebb95067d549cb43a107f Mon Sep 17 00:00:00 2001 From: Viet Nguyen Duc Date: Thu, 24 Oct 2024 02:15:05 +0000 Subject: [PATCH 4/4] Fix test Signed-off-by: Viet Nguyen Duc --- .../selenium/grid/router/StressTest.java | 54 +++++++++++++++---- 1 file changed, 45 insertions(+), 9 deletions(-) diff --git a/java/test/org/openqa/selenium/grid/router/StressTest.java b/java/test/org/openqa/selenium/grid/router/StressTest.java index d9fb088504e97..3be76395e4f01 100644 --- a/java/test/org/openqa/selenium/grid/router/StressTest.java +++ b/java/test/org/openqa/selenium/grid/router/StressTest.java @@ -35,8 +35,10 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.openqa.selenium.By; +import org.openqa.selenium.MutableCapabilities; import org.openqa.selenium.NoSuchSessionException; import org.openqa.selenium.WebDriver; +import org.openqa.selenium.WebDriverException; import org.openqa.selenium.grid.config.MapConfig; import org.openqa.selenium.grid.config.MemoizedConfig; import org.openqa.selenium.grid.config.TomlConfig; @@ -73,7 +75,9 @@ public void setupServers() { + "driver-implementation = " + browser.displayName() + "\n" - + "session-timeout = 15"))); + + "session-timeout = 11" + + "\n" + + "enable-managed-downloads = true"))); tearDowns.add(deployment); server = deployment.getServer(); @@ -114,7 +118,10 @@ void multipleSimultaneousSessions() throws Exception { try { WebDriver driver = RemoteWebDriver.builder() - .oneOf(browser.getCapabilities()) + .oneOf( + browser + .getCapabilities() + .merge(new MutableCapabilities(Map.of("se:downloadsEnabled", true)))) .address(server.getUrl()) .build(); @@ -134,13 +141,42 @@ void multipleSimultaneousSessions() throws Exception { } @Test - void testStopTimedOutSession() throws Exception { + void multipleSimultaneousSessionsTimedOut() throws Exception { assertThat(server.isStarted()).isTrue(); - WebDriver driver = - RemoteWebDriver.builder().oneOf(browser.getCapabilities()).address(server.getUrl()).build(); - driver.get(appServer.getUrl().toString()); - Thread.sleep(15000); - NoSuchSessionException exception = assertThrows(NoSuchSessionException.class, driver::getTitle); - assertTrue(exception.getMessage().startsWith("Cannot find session with id:")); + + CompletableFuture[] futures = new CompletableFuture[10]; + for (int i = 0; i < futures.length; i++) { + CompletableFuture future = new CompletableFuture<>(); + futures[i] = future; + + executor.submit( + () -> { + try { + WebDriver driver = + RemoteWebDriver.builder() + .oneOf(browser.getCapabilities()) + .address(server.getUrl()) + .build(); + driver.get(appServer.getUrl().toString()); + Thread.sleep(11000); + NoSuchSessionException exception = + assertThrows(NoSuchSessionException.class, driver::getTitle); + assertTrue(exception.getMessage().startsWith("Cannot find session with id:")); + WebDriverException webDriverException = + assertThrows( + WebDriverException.class, + () -> ((RemoteWebDriver) driver).getDownloadableFiles()); + assertTrue( + webDriverException + .getMessage() + .startsWith("Cannot find downloads file system for session id:")); + future.complete(true); + } catch (Exception e) { + future.completeExceptionally(e); + } + }); + } + + CompletableFuture.allOf(futures).get(5, MINUTES); } }