diff --git a/java/src/org/openqa/selenium/bidi/Connection.java b/java/src/org/openqa/selenium/bidi/Connection.java index 45376716bec5c..29b661ad05fb3 100644 --- a/java/src/org/openqa/selenium/bidi/Connection.java +++ b/java/src/org/openqa/selenium/bidi/Connection.java @@ -47,6 +47,7 @@ import org.jspecify.annotations.Nullable; import org.openqa.selenium.Beta; import org.openqa.selenium.WebDriverException; +import org.openqa.selenium.internal.Debug; import org.openqa.selenium.internal.Either; import org.openqa.selenium.internal.Require; import org.openqa.selenium.json.Json; @@ -77,9 +78,23 @@ public class Connection implements Closeable { private final WebSocket socket; private final AtomicBoolean underlyingSocketClosed = new AtomicBoolean(false); + /** + * Creates a new BiDi connection to the given URL using the given HTTP client. Before the socket + * opens, the current Selenium debug switches are reflected onto the {@code org.openqa.selenium} + * logger via {@link Debug#configureLogger()}, so connections constructed directly (bypassing + * {@code RemoteWebDriver}/{@code DriverFinder}) still honor {@code -Dselenium.debug} and friends. + * + * @param client the HTTP client used to open the underlying web socket; must not be null + * @param url the URL to open the web socket connection to; must not be null + */ public Connection(HttpClient client, String url) { + // Reflect the current debug switches before this connection starts logging its wire + // diagnostics at FINE -- callers that construct a Connection directly (never going through + // RemoteWebDriver or DriverFinder) would otherwise never trigger the raise. Idempotent and + // cheap, same pattern as DriverFinder.getBinaryPaths(). Require.nonNull("HTTP client", client); Require.nonNull("URL to connect to", url); + Debug.configureLogger(); this.client = client; this.socket = this.client.openSocket(new HttpRequest(GET, url), new Listener()); diff --git a/java/src/org/openqa/selenium/devtools/Connection.java b/java/src/org/openqa/selenium/devtools/Connection.java index 9a91094d85b8f..cd7387314bc87 100644 --- a/java/src/org/openqa/selenium/devtools/Connection.java +++ b/java/src/org/openqa/selenium/devtools/Connection.java @@ -51,6 +51,7 @@ import org.jspecify.annotations.Nullable; import org.openqa.selenium.WebDriverException; import org.openqa.selenium.devtools.idealized.target.model.SessionID; +import org.openqa.selenium.internal.Debug; import org.openqa.selenium.internal.Either; import org.openqa.selenium.internal.Require; import org.openqa.selenium.json.Json; @@ -91,9 +92,27 @@ public Connection(HttpClient client, String url) { this(client, url, ClientConfig.defaultConfig()); } + /** + * Creates a new CDP connection to the given URL using the given HTTP client and client + * configuration. Before the socket opens, the current Selenium debug switches are reflected onto + * the {@code org.openqa.selenium} logger via {@link Debug#configureLogger()}, so connections + * constructed directly (bypassing {@code RemoteWebDriver}/{@code DriverFinder}) still honor + * {@code -Dselenium.debug} and friends. The deprecated 2-arg constructor delegates here, so this + * single call point covers both. + * + * @param client the HTTP client used to open the underlying web socket; must not be null + * @param url the URL to open the web socket connection to + * @param clientConfig the client configuration to use when opening the connection + */ public Connection(HttpClient client, String url, ClientConfig clientConfig) { + // Reflect the current debug switches before this connection starts logging its wire + // diagnostics at FINE -- callers that construct a Connection directly (never going through + // RemoteWebDriver or DriverFinder) would otherwise never trigger the raise. Idempotent and + // cheap, same pattern as DriverFinder.getBinaryPaths(). The deprecated 2-arg constructor + // delegates here, so this single call point covers both. this.client = Require.nonNull("HTTP client", client); this.wsConfig = wsClientConfig(clientConfig, url); + Debug.configureLogger(); this.socket = this.client.openSocket(new HttpRequest(GET, wsConfig.baseUri()), new Listener()); this.isClosed = new AtomicBoolean(); } diff --git a/java/src/org/openqa/selenium/internal/Debug.java b/java/src/org/openqa/selenium/internal/Debug.java index 0b012f180f59e..da585ca389a4c 100644 --- a/java/src/org/openqa/selenium/internal/Debug.java +++ b/java/src/org/openqa/selenium/internal/Debug.java @@ -17,37 +17,67 @@ package org.openqa.selenium.internal; +import java.util.Arrays; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.logging.ConsoleHandler; +import java.util.logging.Filter; +import java.util.logging.Handler; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.logging.SimpleFormatter; -import java.util.logging.StreamHandler; +import org.jspecify.annotations.Nullable; /** Used to provide information about whether Selenium is running under debug mode. */ public class Debug { - private static final boolean IS_DEBUG; private static final AtomicBoolean DEBUG_WARNING_LOGGED = new AtomicBoolean(false); private static final Logger SELENIUM_LOGGER = Logger.getLogger("org.openqa.selenium"); - private static boolean loggerConfigured = false; - static { - IS_DEBUG = - Boolean.getBoolean("selenium.debug") || Boolean.getBoolean("selenium.webdriver.verbose"); - } + private static boolean loggerConfigured = false; + private static Handler installedHandler = null; + private static Level previousLevel = null; + private static boolean levelRaisedByDebug = false; + private static Level configuredLevel = null; + private static Level levelSetByDebug = null; private Debug() { // Utility class } + /** + * Reports whether Selenium debug logging has been requested via the {@code selenium.debug} or the + * legacy {@code selenium.webdriver.verbose} system property. Read live on every call, so a + * property change made at runtime is reflected immediately. + * + * @return true when either the {@code selenium.debug} or the {@code selenium.webdriver.verbose} + * system property is set to {@code true}; false otherwise + */ public static boolean isDebugging() { - return IS_DEBUG; + return Boolean.getBoolean("selenium.debug") || Boolean.getBoolean("selenium.webdriver.verbose"); } + /** + * Returns the log level that debug output should be reported at: {@link Level#INFO} when {@link + * #isDebugging()} is true, {@link Level#FINE} otherwise. + * + * @deprecated Individual log statements no longer change what severity they report at based on + * this switch; {@link #configureLogger()} raises the real {@code org.openqa.selenium} logger + * to {@link Level#FINE} instead, which is the ordinary way to see Selenium's debug output. + * Enable it with {@code -Dselenium.debug=true}, the {@code SE_DEBUG} environment variable, or + * directly via {@code Logger.getLogger("org.openqa.selenium").setLevel(Level.FINE)}. This + * method's own behavior is unchanged and kept only for existing call sites still comparing + * against it. + * @return {@link Level#INFO} when debugging is enabled; {@link Level#FINE} otherwise + */ + @Deprecated(forRemoval = true) public static Level getDebugLogLevel() { return isDebugging() ? Level.INFO : Level.FINE; } + static synchronized boolean isHandlerCurrentlyInstalled() { + return installedHandler != null + && Arrays.asList(SELENIUM_LOGGER.getHandlers()).contains(installedHandler); + } + public static boolean isDebugAll() { boolean everything = Boolean.parseBoolean(System.getenv("SE_DEBUG")); if (everything && DEBUG_WARNING_LOGGED.compareAndSet(false, true)) { @@ -59,16 +89,131 @@ public static boolean isDebugAll() { return everything; } - public static void configureLogger() { - if (!isDebugAll() || loggerConfigured) { + /** + * Computes {@code logger}'s effective level: its own level if set, otherwise the first non-null + * level found walking up its {@link Logger#getParent()} chain, falling back to {@link Level#INFO} + * (JUL's own root default) if none is ever set. {@link Logger} has no single built-in method for + * this, but walking the parent chain is how the JVM itself resolves it internally when deciding + * whether a record is loggable. + * + * @param logger the logger to compute the effective level of + * @return the effective level; never {@code null} + */ + private static Level effectiveLevel(Logger logger) { + for (Logger current = logger; current != null; current = current.getParent()) { + Level level = current.getLevel(); + if (level != null) { + return level; + } + } + return Level.INFO; + } + + @Nullable + private static Level getRequestedLogLevel() { + if (isDebugAll()) { + return Level.FINE; + } + if (isDebugging()) { + return Level.FINE; + } + return null; + } + + /** + * Reflects the current debug switches ({@code -Dselenium.debug=true}, {@code + * -Dselenium.webdriver.verbose=true}, {@code SE_DEBUG}) onto the real {@code org.openqa.selenium} + * logger: raises it to {@link Level#FINE} when it is currently less verbose than {@link + * Level#FINE}; a level already at {@link Level#FINE} or more verbose is left untouched. It also + * attaches a handler Selenium owns, filtered to exclude {@link Level#INFO} and above. Caller-owned + * direct or ancestor handlers that accept {@link Level#FINE} can still receive and print FINE + * records, so FINE output can be duplicated. Calls are no-ops when the requested configuration + * is already consistent, but repair the Selenium-owned handler and FINE loggability after + * external divergence. Reversible: once every switch is off, the + * next call removes exactly the handler this method installed and restores the logger's level to + * what it was before debugging turned on, only when this method was the one that raised it and + * unless something else changed the level in the meantime -- that change is left alone rather + * than clobbered. This can't distinguish an external override that happens to also set exactly + * {@link Level#FINE}: since JUL has no level-change listener to tell the two apart, that specific + * case still restores the pre-debug level. Safe to call from concurrent driver construction. + * + *

Cross-binding note: the Python binding does the analogous thing at import time (the {@code + * SE_DEBUG} block at the top of {@code py/selenium/webdriver/__init__.py}): when the {@code + * SE_DEBUG} environment variable is set it puts the {@code selenium} logger at {@code DEBUG} and + * attaches an unfiltered {@code StreamHandler} if the logger has none of its own. Two deliberate + * differences here: Java only raises the level when the logger is currently less verbose than + * {@link Level#FINE} (Python sets {@code DEBUG} unconditionally), and Java's Selenium-owned + * handler is filtered to records below {@link Level#INFO}. Caller-owned direct or ancestor + * handlers accepting {@link Level#FINE} can still print FINE records, so duplicates remain + * possible. + */ + public static synchronized void configureLogger() { + Level requestedLevel = getRequestedLogLevel(); + boolean shouldDebug = requestedLevel != null; + // When shouldDebug is on and already configured, only skip if the handler this method + // installed is still actually attached -- something outside this class (e.g. a LogManager + // reset, or unrelated code calling removeHandler() directly) can remove it without ever + // going through configureLogger(), and that divergence must be repaired here rather than + // silently left until the debug switch itself changes. + if (shouldDebug == loggerConfigured + && (!shouldDebug + || (isHandlerCurrentlyInstalled() + && requestedLevel.equals(configuredLevel) + && effectiveLevel(SELENIUM_LOGGER).intValue() <= requestedLevel.intValue()))) { return; } - SELENIUM_LOGGER.setLevel(Level.FINE); + if (shouldDebug) { + // Capture the original own level on a genuine off->on transition. A repair call must not + // overwrite this snapshot with the level it is repairing. + if (!loggerConfigured) { + configuredLevel = requestedLevel; + previousLevel = SELENIUM_LOGGER.getLevel(); + levelRaisedByDebug = false; + levelSetByDebug = null; + } + + if (effectiveLevel(SELENIUM_LOGGER).intValue() > requestedLevel.intValue()) { + SELENIUM_LOGGER.setLevel(requestedLevel); + levelSetByDebug = requestedLevel; + levelRaisedByDebug = true; + } + + configuredLevel = requestedLevel; + if (isHandlerCurrentlyInstalled()) { + installedHandler.setLevel(requestedLevel); + } else { + Handler handler = new ConsoleHandler(); + handler.setLevel(requestedLevel); + Filter belowInfo = record -> record.getLevel().intValue() < Level.INFO.intValue(); + handler.setFilter(belowInfo); + SELENIUM_LOGGER.addHandler(handler); + installedHandler = handler; + } + } else { + // installedHandler can already be null here if it was removed externally and debugging + // turned off before any repair call ever ran -- Logger.removeHandler(null) throws NPE per + // its javadoc, so guard against that. + if (installedHandler != null) { + SELENIUM_LOGGER.removeHandler(installedHandler); + installedHandler.close(); + installedHandler = null; + } + // Restore only when Debug itself raised the level AND nothing else changed it since. The + // equality guard keeps the existing "external override while debugging" protection; + // levelRaisedByDebug additionally covers the case where Debug never touched the level at + // all and so has nothing to restore. + if (levelRaisedByDebug + && levelSetByDebug != null + && levelSetByDebug.equals(SELENIUM_LOGGER.getLevel())) { + SELENIUM_LOGGER.setLevel(previousLevel); + } + levelRaisedByDebug = false; + previousLevel = null; + configuredLevel = null; + levelSetByDebug = null; + } - StreamHandler handler = new StreamHandler(System.err, new SimpleFormatter()); - handler.setLevel(Level.FINE); - SELENIUM_LOGGER.addHandler(handler); - loggerConfigured = true; + loggerConfigured = shouldDebug; } } diff --git a/java/src/org/openqa/selenium/remote/RemoteWebDriver.java b/java/src/org/openqa/selenium/remote/RemoteWebDriver.java index b80dfd0eb655d..2419da32974cd 100644 --- a/java/src/org/openqa/selenium/remote/RemoteWebDriver.java +++ b/java/src/org/openqa/selenium/remote/RemoteWebDriver.java @@ -122,8 +122,15 @@ public class RemoteWebDriver PrintsPage, TakesScreenshot { + // Guarantees (JLS 12.4.2) that debug logging is configured before ANY subclass constructor + // body runs -- including argument expressions passed to a subclass's own super(...) call, e.g. + // ChromeDriver/FirefoxDriver's DriverFinder/SeleniumManager discovery, which logs at FINE + // before super(...) is ever reached. configureLogger() is idempotent, so this and the call in + // the canonical instance constructor below are both safe to keep: this one covers logging that + // happens before an instance exists, the other picks up a property changed after this class + // already loaded. static { - org.openqa.selenium.internal.Debug.configureLogger(); + Debug.configureLogger(); } private static final Logger LOG = Logger.getLogger(RemoteWebDriver.class.getName()); @@ -203,14 +210,30 @@ public RemoteWebDriver(CommandExecutor executor, Capabilities capabilities) { this(executor, capabilities, ClientConfig.defaultConfig()); } + /** + * Creates a new driver that runs its commands through the given executor, requesting a new + * session with the given capabilities. Before the session starts, the current Selenium debug + * switches are reflected onto the {@code org.openqa.selenium} logger via {@link + * Debug#configureLogger()}, so a debug property changed at runtime takes effect for every + * driver constructed afterwards. + * + * @param executor the command executor used to communicate with the remote end; must not be + * null + * @param capabilities the capabilities requested for the new session; null is treated as an + * empty set of capabilities + * @param clientConfig the HTTP client configuration for the connection; must not be null + */ public RemoteWebDriver( CommandExecutor executor, Capabilities capabilities, ClientConfig clientConfig) { + // Instance-time (not class-load-time) so a property change made after this class has already + // loaded still takes effect for drivers constructed afterwards. this.clientConfig = Require.nonNull("Client config", clientConfig); this.executor = Require.nonNull("Command executor", executor); + Debug.configureLogger(); this.capabilities = requireNonNullElseGet(capabilities, () -> new ImmutableCapabilities()); try { - startSession(capabilities); + startSession(this.capabilities); } catch (RuntimeException e) { try { quit(); diff --git a/java/src/org/openqa/selenium/remote/http/RetryRequest.java b/java/src/org/openqa/selenium/remote/http/RetryRequest.java index 17a3f7e0ed25c..00efadec5e7f8 100644 --- a/java/src/org/openqa/selenium/remote/http/RetryRequest.java +++ b/java/src/org/openqa/selenium/remote/http/RetryRequest.java @@ -23,12 +23,10 @@ import java.net.ConnectException; import java.util.logging.Level; import java.util.logging.Logger; -import org.openqa.selenium.internal.Debug; public class RetryRequest implements Filter { private static final Logger LOG = Logger.getLogger(RetryRequest.class.getName()); - private static final Level LOG_LEVEL = Debug.getDebugLogLevel(); private static final int RETRIES_ON_CONNECTION_FAILURE = 3; private static final int RETRIES_ON_SERVER_ERROR = 2; @@ -50,7 +48,7 @@ public HttpHandler apply(HttpHandler next) { // must be a connection failure and check whether we have retries left for this if (isConnectionFailure && i < RETRIES_ON_CONNECTION_FAILURE) { - LOG.log(LOG_LEVEL, "Retry #" + (i + 1) + " on ConnectException", ex); + LOG.log(Level.FINE, "Retry #" + (i + 1) + " on ConnectException", ex); continue; } @@ -65,7 +63,7 @@ public HttpHandler apply(HttpHandler next) { // must be a server error and check whether we have retries left for this if (isServerError && i < RETRIES_ON_SERVER_ERROR) { - LOG.log(LOG_LEVEL, "Retry #" + (i + 1) + " on ServerError: " + response.getStatus()); + LOG.log(Level.FINE, "Retry #" + (i + 1) + " on ServerError: " + response.getStatus()); continue; } diff --git a/java/test/org/openqa/selenium/devtools/BUILD.bazel b/java/test/org/openqa/selenium/devtools/BUILD.bazel index 7c0b8f68509b9..e1a402a87424e 100644 --- a/java/test/org/openqa/selenium/devtools/BUILD.bazel +++ b/java/test/org/openqa/selenium/devtools/BUILD.bazel @@ -4,6 +4,7 @@ load("//java:defs.bzl", "JUNIT5_DEPS", "java_library", "java_selenium_test_suite SMALL_TESTS = [ "CdpEndpointFinderTest.java", "CdpVersionFinderTest.java", + "ConnectionTest.java", ] java_test_suite( diff --git a/java/test/org/openqa/selenium/devtools/ConnectionTest.java b/java/test/org/openqa/selenium/devtools/ConnectionTest.java new file mode 100644 index 0000000000000..1dd317dfd588c --- /dev/null +++ b/java/test/org/openqa/selenium/devtools/ConnectionTest.java @@ -0,0 +1,116 @@ +// 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.devtools; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.logging.Level; +import java.util.logging.Logger; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.openqa.selenium.internal.Debug; +import org.openqa.selenium.remote.http.ClientConfig; +import org.openqa.selenium.remote.http.HttpClient; +import org.openqa.selenium.remote.http.HttpRequest; +import org.openqa.selenium.remote.http.HttpResponse; +import org.openqa.selenium.remote.http.Message; +import org.openqa.selenium.remote.http.WebSocket; + +@Tag("UnitTests") +class ConnectionTest { + + private String oldDebugProperty; + private Level oldLoggerLevel; + + @BeforeEach + void storeSystemProperty() { + oldDebugProperty = System.getProperty("selenium.debug"); + oldLoggerLevel = seleniumLogger().getLevel(); + System.clearProperty("selenium.debug"); + } + + @AfterEach + void restoreSystemProperty() { + if (oldDebugProperty != null) { + System.setProperty("selenium.debug", oldDebugProperty); + } else { + System.clearProperty("selenium.debug"); + } + // Re-sync configureLogger's internal state/handler with the now-restored property so a + // handler installed by this test never leaks into the next. + Debug.configureLogger(); + seleniumLogger().setLevel(oldLoggerLevel); + } + + private static Logger seleniumLogger() { + return Logger.getLogger("org.openqa.selenium"); + } + + @Test + void constructingConnectionDirectlyConfiguresTheSeleniumLoggerWhenDebugging() { + // devtools.Connection is sometimes constructed directly rather than through RemoteWebDriver or + // DriverFinder -- neither of which would run in that path to trigger Debug.configureLogger() + // otherwise. No test previously constructed a Connection directly and checked that its own + // constructor actually configures the shared org.openqa.selenium logger. + System.setProperty("selenium.debug", "true"); + + try (Connection connection = + new Connection( + new NoOpHttpClient(), + "ws://localhost:9222/devtools/page/1", + ClientConfig.defaultConfig())) { + assertThat(seleniumLogger().getLevel()).isEqualTo(Level.FINE); + } + } + + /** Minimal real (not mocked) {@link HttpClient} whose socket never talks to the network. */ + private static class NoOpHttpClient implements HttpClient { + @Override + public HttpResponse execute(HttpRequest request) { + throw new UnsupportedOperationException("execute"); + } + + @Override + public WebSocket openSocket(HttpRequest request, WebSocket.Listener listener) { + return new WebSocket() { + @Override + public WebSocket send(Message message) { + return this; + } + + @Override + public void close() {} + }; + } + + @Override + public java.util.concurrent.CompletableFuture> + sendAsyncNative( + java.net.http.HttpRequest request, java.net.http.HttpResponse.BodyHandler handler) { + throw new UnsupportedOperationException("sendAsyncNative"); + } + + @Override + public java.net.http.HttpResponse sendNative( + java.net.http.HttpRequest request, java.net.http.HttpResponse.BodyHandler handler) { + throw new UnsupportedOperationException("sendNative"); + } + } +} diff --git a/java/test/org/openqa/selenium/internal/BUILD.bazel b/java/test/org/openqa/selenium/internal/BUILD.bazel index e0e9427d6cb5d..4a70406ee6f39 100644 --- a/java/test/org/openqa/selenium/internal/BUILD.bazel +++ b/java/test/org/openqa/selenium/internal/BUILD.bazel @@ -9,5 +9,7 @@ java_test_suite( "//java/src/org/openqa/selenium:core", artifact("org.assertj:assertj-core"), artifact("org.junit.jupiter:junit-jupiter-api"), + artifact("uk.org.webcompere:system-stubs-core"), + artifact("uk.org.webcompere:system-stubs-jupiter"), ] + JUNIT5_DEPS, ) diff --git a/java/test/org/openqa/selenium/internal/DebugTest.java b/java/test/org/openqa/selenium/internal/DebugTest.java new file mode 100644 index 0000000000000..9b91a02c46b29 --- /dev/null +++ b/java/test/org/openqa/selenium/internal/DebugTest.java @@ -0,0 +1,553 @@ +// 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.internal; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.ByteArrayOutputStream; +import java.io.PrintStream; +import java.io.UnsupportedEncodingException; +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; +import java.util.logging.ConsoleHandler; +import java.util.logging.ErrorManager; +import java.util.logging.Filter; +import java.util.logging.Formatter; +import java.util.logging.Handler; +import java.util.logging.Level; +import java.util.logging.LogRecord; +import java.util.logging.Logger; +import java.util.logging.SimpleFormatter; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import uk.org.webcompere.systemstubs.environment.EnvironmentVariables; +import uk.org.webcompere.systemstubs.jupiter.SystemStub; +import uk.org.webcompere.systemstubs.jupiter.SystemStubsExtension; + +@Tag("UnitTests") +@ExtendWith(SystemStubsExtension.class) +class DebugTest { + + /** + * The shared {@code org.openqa.selenium} logger whose state {@link Debug#configureLogger()} + * manages -- deliberately not this test class's own logger, because the behavior under test lives + * on the shared category. + */ + private static Logger seleniumLogger() { + return Logger.getLogger("org.openqa.selenium"); + } + + private String oldDebugProperty; + private String oldVerboseProperty; + private Level oldLoggerLevel; + + @SystemStub private EnvironmentVariables environment; + + @BeforeEach + void storeSystemProperties() { + oldDebugProperty = System.getProperty("selenium.debug"); + oldVerboseProperty = System.getProperty("selenium.webdriver.verbose"); + oldLoggerLevel = seleniumLogger().getLevel(); + System.clearProperty("selenium.debug"); + System.clearProperty("selenium.webdriver.verbose"); + } + + @AfterEach + void restoreSystemProperties() { + if (oldDebugProperty != null) { + System.setProperty("selenium.debug", oldDebugProperty); + } else { + System.clearProperty("selenium.debug"); + } + if (oldVerboseProperty != null) { + System.setProperty("selenium.webdriver.verbose", oldVerboseProperty); + } else { + System.clearProperty("selenium.webdriver.verbose"); + } + // Re-sync configureLogger's internal state/handler with the now-restored properties so a + // handler installed by one test never leaks into the next. + Debug.configureLogger(); + // A test may have changed the logger's level directly (simulating code other than Debug + // touching it); put it back exactly as found so tests stay isolated regardless of what + // configureLogger()'s own restore logic decided to do. + seleniumLogger().setLevel(oldLoggerLevel); + } + + @Test + void isDebuggingReflectsPropertySetAfterClassLoad() { + assertThat(Debug.isDebugging()).isFalse(); + + System.setProperty("selenium.debug", "true"); + + assertThat(Debug.isDebugging()).isTrue(); + } + + @Test + void isDebuggingHonoursTheLegacyVerboseProperty() { + assertThat(Debug.isDebugging()).isFalse(); + + System.setProperty("selenium.webdriver.verbose", "true"); + + assertThat(Debug.isDebugging()).isTrue(); + } + + @Test + void configureLoggerRaisesSeleniumLoggerToFine() { + System.setProperty("selenium.debug", "true"); + + Debug.configureLogger(); + + assertThat(seleniumLogger().getLevel()).isEqualTo(Level.FINE); + } + + @Test + void configureLoggerDoesNotClobberALevelChangedWhileDebuggingWasOn() { + System.setProperty("selenium.debug", "true"); + Debug.configureLogger(); + assertThat(seleniumLogger().getLevel()).isEqualTo(Level.FINE); + + // Something other than Debug changes the level while debugging is still on -- e.g. the user's + // own logging config. + seleniumLogger().setLevel(Level.WARNING); + + System.clearProperty("selenium.debug"); + Debug.configureLogger(); + + // The externally-set WARNING must survive. Debug must not clobber it with the level that was + // ambient before IT turned debugging on -- that snapshot is stale the moment anything else + // changes the level in between. + assertThat(seleniumLogger().getLevel()).isEqualTo(Level.WARNING); + } + + @Test + void configureLoggerRestoresPreDebugLevelAndRemovesHandlerWhenTurnedOff() { + Level preDebugLevel = seleniumLogger().getLevel(); + List handlersBeforeDebug = new ArrayList<>(List.of(seleniumLogger().getHandlers())); + + System.setProperty("selenium.debug", "true"); + Debug.configureLogger(); + + List handlersWhileDebugging = new ArrayList<>(List.of(seleniumLogger().getHandlers())); + handlersWhileDebugging.removeAll(handlersBeforeDebug); + assertThat(handlersWhileDebugging).hasSize(1); + Handler installedHandler = handlersWhileDebugging.get(0); + + // No external override happens in between -- this is the plain turn-on/turn-off round trip. + System.clearProperty("selenium.debug"); + Debug.configureLogger(); + + assertThat(seleniumLogger().getLevel()).isEqualTo(preDebugLevel); + assertThat(seleniumLogger().getHandlers()).doesNotContain(installedHandler); + } + + @Test + void configureLoggerIsIdempotent() { + int before = seleniumLogger().getHandlers().length; + System.setProperty("selenium.debug", "true"); + + for (int i = 0; i < 5; i++) { + Debug.configureLogger(); + } + + assertThat(seleniumLogger().getHandlers().length - before).isEqualTo(1); + } + + @Test + void configureLoggerLeavesUserHandlerConfigurationAlone() throws UnsupportedEncodingException { + Handler userHandler = new ConsoleHandler(); + Filter userFilter = record -> false; + Formatter userFormatter = new SimpleFormatter(); + ErrorManager userErrorManager = new ErrorManager(); + userHandler.setLevel(Level.WARNING); + userHandler.setFilter(userFilter); + userHandler.setFormatter(userFormatter); + userHandler.setEncoding("UTF-8"); + userHandler.setErrorManager(userErrorManager); + seleniumLogger().addHandler(userHandler); + try { + System.setProperty("selenium.debug", "true"); + Debug.configureLogger(); + assertThat(seleniumLogger().getHandlers()).contains(userHandler); + assertThat(userHandler.getLevel()).isEqualTo(Level.WARNING); + assertThat(userHandler.getFilter()).isSameAs(userFilter); + assertThat(userHandler.getFormatter()).isSameAs(userFormatter); + assertThat(userHandler.getEncoding()).isEqualTo("UTF-8"); + assertThat(userHandler.getErrorManager()).isSameAs(userErrorManager); + + System.clearProperty("selenium.debug"); + Debug.configureLogger(); + assertThat(seleniumLogger().getHandlers()).contains(userHandler); + assertThat(userHandler.getLevel()).isEqualTo(Level.WARNING); + assertThat(userHandler.getFilter()).isSameAs(userFilter); + assertThat(userHandler.getFormatter()).isSameAs(userFormatter); + assertThat(userHandler.getEncoding()).isEqualTo("UTF-8"); + assertThat(userHandler.getErrorManager()).isSameAs(userErrorManager); + } finally { + seleniumLogger().removeHandler(userHandler); + } + } + + @Test + void infoRecordsAreNotDuplicatedWhenDebuggingIsEnabled() { + List userHandlerRecords = new ArrayList<>(); + Handler userHandler = + new Handler() { + @Override + public void publish(LogRecord record) { + userHandlerRecords.add(record); + } + + @Override + public void flush() {} + + @Override + public void close() {} + }; + // Simulates a handler the caller already has attached directly to this logger (e.g. their + // own handler at INFO) that already prints INFO-and-above records on its own. + userHandler.setLevel(Level.INFO); + seleniumLogger().addHandler(userHandler); + + boolean oldUseParentHandlers = seleniumLogger().getUseParentHandlers(); + // Isolate this check to handlers attached directly to org.openqa.selenium. Propagation to the + // JVM's own root logger handler is a separate, legitimate print channel this test isn't + // about, and it would otherwise be indistinguishable from a real duplicate here. + seleniumLogger().setUseParentHandlers(false); + + PrintStream originalErr = System.err; + ByteArrayOutputStream capturedErr = new ByteArrayOutputStream(); + String marker = "duplicate-check-" + UUID.randomUUID(); + try { + System.setErr(new PrintStream(capturedErr)); + System.setProperty("selenium.debug", "true"); + Debug.configureLogger(); + + seleniumLogger().log(Level.INFO, marker); + for (Handler handler : seleniumLogger().getHandlers()) { + handler.flush(); + } + } finally { + System.setErr(originalErr); + seleniumLogger().setUseParentHandlers(oldUseParentHandlers); + seleniumLogger().removeHandler(userHandler); + } + + // The caller's own handler must still see the record: Selenium never suppresses records for + // handlers it doesn't own. + assertThat(userHandlerRecords).extracting(LogRecord::getMessage).containsExactly(marker); + // Selenium's own handler must not ALSO print it to stderr -- otherwise the exact same line + // the caller's handler just printed would appear a second time, straight from Selenium's own + // console handler. + assertThat(capturedErr.toString()).doesNotContain(marker); + } + + @Test + void configureLoggerDoesNotLowerAnAlreadyMoreVerboseLevel() { + // The application already asked for MORE verbosity than the debug switch provides, e.g. to + // see W3CHttpResponseCodec's FINER response-decoding diagnostics. + seleniumLogger().setLevel(Level.FINER); + + System.setProperty("selenium.debug", "true"); + Debug.configureLogger(); + // Turning debug on must never make the logger LESS verbose than it already was. + assertThat(seleniumLogger().getLevel()).isEqualTo(Level.FINER); + + System.clearProperty("selenium.debug"); + Debug.configureLogger(); + assertThat(seleniumLogger().getLevel()).isEqualTo(Level.FINER); + } + + @Test + void configureLoggerDoesNotClampAnInheritedMoreVerboseEffectiveLevel() { + // org.openqa.selenium's OWN level stays null/unset (inheriting), but its parent logger + // (org.openqa) is explicitly more verbose than FINE -- the real EFFECTIVE level right now is + // already FINER, and configureLogger() must not clobber that down to FINE just because the + // child logger's own level happens to be null rather than explicitly set. + Logger parentLogger = Logger.getLogger("org.openqa"); + Level oldParentLevel = parentLogger.getLevel(); + parentLogger.setLevel(Level.FINER); + try { + // Enforce the starting precondition rather than merely asserting it: @AfterEach already + // restores this logger's own level after every test, so forcing it to null here is safe + // and can't leak into other tests -- but without this, a leftover explicit level from + // elsewhere in the same JVM run could fail this precondition before the real behavior under + // test ever runs. + seleniumLogger().setLevel(null); + assertThat(seleniumLogger().getLevel()).isNull(); + + System.setProperty("selenium.debug", "true"); + Debug.configureLogger(); + + // The logger's OWN level must stay untouched: configureLogger() had nothing to raise since + // the EFFECTIVE level was already more verbose than FINE. + assertThat(seleniumLogger().getLevel()).isNull(); + // The inherited FINER effective level must still be in force. + assertThat(seleniumLogger().isLoggable(Level.FINER)).isTrue(); + + // Nothing was raised, so turning debug back off must be a no-op for the level. + System.clearProperty("selenium.debug"); + Debug.configureLogger(); + assertThat(seleniumLogger().getLevel()).isNull(); + } finally { + parentLogger.setLevel(oldParentLevel); + } + } + + @Test + void configureLoggerDoesNotRestoreALevelItNeverChanged() { + seleniumLogger().setLevel(Level.FINER); + System.setProperty("selenium.debug", "true"); + Debug.configureLogger(); // debug on, level untouched (already more verbose than FINE) + + // Something else deliberately drops verbosity to FINE while debugging is on. + seleniumLogger().setLevel(Level.FINE); + + System.clearProperty("selenium.debug"); + Debug.configureLogger(); + + // Debug never changed the level (it was already more verbose when debug turned on), so + // turning debug off must not "restore" a pre-debug snapshot it never took either. + assertThat(seleniumLogger().getLevel()).isEqualTo(Level.FINE); + } + + @Test + @SuppressWarnings({"deprecation", "removal"}) + void getDebugLogLevelStillReportsInfoWhileDeprecated() { + System.setProperty("selenium.debug", "true"); + assertThat(Debug.getDebugLogLevel()).isEqualTo(Level.INFO); + + System.clearProperty("selenium.debug"); + assertThat(Debug.getDebugLogLevel()).isEqualTo(Level.FINE); + } + + @Test + void configureLoggerRepairsAnExternallyRemovedHandlerWithoutCorruptingRestoreBookkeeping() { + Level preDebugLevel = seleniumLogger().getLevel(); + List handlersBeforeDebug = new ArrayList<>(List.of(seleniumLogger().getHandlers())); + + System.setProperty("selenium.debug", "true"); + Debug.configureLogger(); + + List handlersWhileDebugging = new ArrayList<>(List.of(seleniumLogger().getHandlers())); + handlersWhileDebugging.removeAll(handlersBeforeDebug); + assertThat(handlersWhileDebugging).hasSize(1); + Handler installedHandler = handlersWhileDebugging.get(0); + + // Something outside Debug removes the handler directly while debugging stays on -- e.g. + // LogManager.getLogManager().reset() or a direct removeHandler() call by unrelated code. + seleniumLogger().removeHandler(installedHandler); + assertThat(Debug.isHandlerCurrentlyInstalled()).isFalse(); + + // The property is unchanged (still true) -- a naive fast-path keyed only on + // shouldDebug == loggerConfigured would return early here and never repair the handler. + Debug.configureLogger(); + assertThat(Debug.isHandlerCurrentlyInstalled()) + .as("the repair call must reinstall a handler even though the debug switch never changed") + .isTrue(); + + // Turning debug back off after the repair call must still restore the ORIGINAL pre-debug + // level -- proving the repair call didn't re-run the level-raising bookkeeping and corrupt + // levelRaisedByDebug/previousLevel. + System.clearProperty("selenium.debug"); + Debug.configureLogger(); + assertThat(seleniumLogger().getLevel()).isEqualTo(preDebugLevel); + } + + @Test + void configureLoggerRepairRestoresHandlerAndFineLoggabilityWithoutReplacingSnapshot() { + Level preDebugLevel = seleniumLogger().getLevel(); + List handlersBeforeDebug = new ArrayList<>(List.of(seleniumLogger().getHandlers())); + + System.setProperty("selenium.debug", "true"); + Debug.configureLogger(); + + List handlersWhileDebugging = new ArrayList<>(List.of(seleniumLogger().getHandlers())); + handlersWhileDebugging.removeAll(handlersBeforeDebug); + seleniumLogger().removeHandler(handlersWhileDebugging.get(0)); + seleniumLogger().setLevel(Level.INFO); + + Debug.configureLogger(); + + assertThat(Debug.isHandlerCurrentlyInstalled()).isTrue(); + assertThat(seleniumLogger().isLoggable(Level.FINE)).isTrue(); + + System.clearProperty("selenium.debug"); + Debug.configureLogger(); + + assertThat(seleniumLogger().getLevel()).isEqualTo(preDebugLevel); + } + + @Test + void configureLoggerRepairsLevelWithoutReplacingItsHandler() { + Level preDebugLevel = seleniumLogger().getLevel(); + List handlersBeforeDebug = new ArrayList<>(List.of(seleniumLogger().getHandlers())); + + System.setProperty("selenium.debug", "true"); + Debug.configureLogger(); + + List handlersWhileDebugging = new ArrayList<>(List.of(seleniumLogger().getHandlers())); + handlersWhileDebugging.removeAll(handlersBeforeDebug); + Handler installedHandler = handlersWhileDebugging.get(0); + seleniumLogger().setLevel(Level.INFO); + + Debug.configureLogger(); + + assertThat(seleniumLogger().getHandlers()).contains(installedHandler); + assertThat(seleniumLogger().isLoggable(Level.FINE)).isTrue(); + + System.clearProperty("selenium.debug"); + Debug.configureLogger(); + + assertThat(seleniumLogger().getLevel()).isEqualTo(preDebugLevel); + } + + @Test + void configureLoggerDoesNotChangeRootLoggerForSystemPropertyDebugging() { + Logger rootLogger = Logger.getLogger(""); + Level rootLevel = rootLogger.getLevel(); + List rootHandlers = List.of(rootLogger.getHandlers()); + + System.setProperty("selenium.debug", "true"); + Debug.configureLogger(); + + assertThat(rootLogger.getLevel()).isEqualTo(rootLevel); + assertThat(rootLogger.getHandlers()).containsExactlyElementsOf(rootHandlers); + } + + @Test + void seDebugConfiguresFineHandlerAndRestoresLoggerLevel() { + environment.set("SE_DEBUG", "false"); + Debug.configureLogger(); + Level preDebugLevel = seleniumLogger().getLevel(); + List handlersBeforeDebug = new ArrayList<>(List.of(seleniumLogger().getHandlers())); + + environment.set("SE_DEBUG", "true"); + Debug.configureLogger(); + + List handlersWhileDebugging = new ArrayList<>(List.of(seleniumLogger().getHandlers())); + handlersWhileDebugging.removeAll(handlersBeforeDebug); + assertThat(handlersWhileDebugging) + .singleElement() + .extracting(Handler::getLevel) + .isEqualTo(Level.FINE); + + environment.set("SE_DEBUG", "false"); + Debug.configureLogger(); + + assertThat(seleniumLogger().getLevel()).isEqualTo(preDebugLevel); + } + + @Test + void configureLoggerLeavesExplicitMoreVerboseLevelsEffective() { + for (Level level : List.of(Level.FINER, Level.FINEST, Level.ALL)) { + seleniumLogger().setLevel(level); + System.setProperty("selenium.debug", "true"); + + Debug.configureLogger(); + + assertThat(seleniumLogger().getLevel()).isEqualTo(level); + assertThat(seleniumLogger().isLoggable(level)).isTrue(); + + System.clearProperty("selenium.debug"); + Debug.configureLogger(); + } + } + + @Test + void configureLoggerLeavesInheritedMoreVerboseLevelsEffective() { + Logger parentLogger = Logger.getLogger("org.openqa"); + Level oldParentLevel = parentLogger.getLevel(); + try { + for (Level level : List.of(Level.FINER, Level.FINEST, Level.ALL)) { + parentLogger.setLevel(level); + seleniumLogger().setLevel(null); + System.setProperty("selenium.debug", "true"); + + Debug.configureLogger(); + + assertThat(seleniumLogger().getLevel()).isNull(); + assertThat(seleniumLogger().isLoggable(level)).isTrue(); + + System.clearProperty("selenium.debug"); + Debug.configureLogger(); + } + } finally { + parentLogger.setLevel(oldParentLevel); + } + } + + @Test + void configureLoggerRestoresMoreVerboseLevelAfterLateRepair() { + Logger parentLogger = Logger.getLogger("org.openqa"); + Level oldParentLevel = parentLogger.getLevel(); + parentLogger.setLevel(Level.FINER); + try { + for (boolean inherited : List.of(false, true)) { + seleniumLogger().setLevel(inherited ? null : Level.FINER); + System.setProperty("selenium.debug", "true"); + Debug.configureLogger(); + + seleniumLogger().setLevel(Level.INFO); + Debug.configureLogger(); + + assertThat(seleniumLogger().isLoggable(Level.FINE)).isTrue(); + + System.clearProperty("selenium.debug"); + Debug.configureLogger(); + + assertThat(seleniumLogger().getLevel()).isEqualTo(inherited ? null : Level.FINER); + assertThat(seleniumLogger().isLoggable(Level.FINER)).isTrue(); + } + } finally { + parentLogger.setLevel(oldParentLevel); + } + } + + @Test + void isHandlerCurrentlyInstalledReflectsExternalHandlerRemoval() { + // isHandlerCurrentlyInstalled() must answer whether Debug's handler is REALLY still attached + // to org.openqa.selenium, not just whether Debug's own bookkeeping thinks it installed one and + // was never told otherwise. Something outside Debug entirely can remove that handler without + // going through configureLogger() -- e.g. LogManager.getLogManager().reset() (routine in + // embedding scenarios: Spring Boot's JavaLoggingSystem, a Log4j-JUL bridge, a container + // shutdown hook) or a direct removeHandler() call by unrelated code -- and Debug has no way to + // be told when that happens. + List handlersBeforeDebug = new ArrayList<>(List.of(seleniumLogger().getHandlers())); + + System.setProperty("selenium.debug", "true"); + Debug.configureLogger(); + assertThat(Debug.isHandlerCurrentlyInstalled()).isTrue(); + + List handlersWhileDebugging = new ArrayList<>(List.of(seleniumLogger().getHandlers())); + handlersWhileDebugging.removeAll(handlersBeforeDebug); + assertThat(handlersWhileDebugging).hasSize(1); + Handler installedHandler = handlersWhileDebugging.get(0); + + // Simulates the external-actor scenario: something other than Debug removes the handler + // directly, without ever calling configureLogger(). + seleniumLogger().removeHandler(installedHandler); + + assertThat(Debug.isHandlerCurrentlyInstalled()) + .as("the handler was removed out from under Debug's bookkeeping by something else") + .isFalse(); + } +} diff --git a/java/test/org/openqa/selenium/remote/RemoteWebDriverInitializationTest.java b/java/test/org/openqa/selenium/remote/RemoteWebDriverInitializationTest.java index 5759b4f253ea8..25e4291499e5e 100644 --- a/java/test/org/openqa/selenium/remote/RemoteWebDriverInitializationTest.java +++ b/java/test/org/openqa/selenium/remote/RemoteWebDriverInitializationTest.java @@ -41,7 +41,12 @@ import java.time.Duration; import java.util.Map; import java.util.UUID; +import java.util.concurrent.atomic.AtomicReference; +import java.util.logging.Level; +import java.util.logging.Logger; import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; @@ -49,6 +54,7 @@ import org.openqa.selenium.ImmutableCapabilities; import org.openqa.selenium.Platform; import org.openqa.selenium.SessionNotCreatedException; +import org.openqa.selenium.internal.Debug; import org.openqa.selenium.remote.http.ClientConfig; import org.openqa.selenium.remote.http.Contents; import org.openqa.selenium.remote.http.HttpClient; @@ -58,7 +64,66 @@ @Tag("UnitTests") class RemoteWebDriverInitializationTest { + /** + * The shared {@code org.openqa.selenium} logger that {@code Debug.configureLogger()} manages -- + * deliberately not this test class's own logger, because the assertion is about the shared + * category's state. + */ + private static Logger seleniumLogger() { + return Logger.getLogger("org.openqa.selenium"); + } + private boolean quitCalled = false; + private String oldDebugProperty; + // Legacy alias for selenium.debug -- Debug.isDebugging() honors either, so a test JVM that + // happens to have this set externally must not leak into the "no switch" baseline assertions. + private String oldVerboseProperty; + private Level oldLoggerLevel; + + @BeforeEach + void storeDebugState() { + oldDebugProperty = System.getProperty("selenium.debug"); + oldVerboseProperty = System.getProperty("selenium.webdriver.verbose"); + oldLoggerLevel = seleniumLogger().getLevel(); + System.clearProperty("selenium.debug"); + System.clearProperty("selenium.webdriver.verbose"); + } + + @AfterEach + void restoreDebugState() { + if (oldDebugProperty != null) { + System.setProperty("selenium.debug", oldDebugProperty); + } else { + System.clearProperty("selenium.debug"); + } + if (oldVerboseProperty != null) { + System.setProperty("selenium.webdriver.verbose", oldVerboseProperty); + } else { + System.clearProperty("selenium.webdriver.verbose"); + } + Debug.configureLogger(); + seleniumLogger().setLevel(oldLoggerLevel); + } + + @Test + void constructingASecondDriverPicksUpADebugPropertyChangedAfterTheFirst() { + // A plain in-memory executor (no mocking framework): answers the single NEW_SESSION command + // each construction issues by echoing the requested capabilities back. + CommandExecutor inMemoryExecutor = command -> echoCapabilities.apply(command); + + // First construction: touches (and, the first time in this JVM, initializes) the class while + // debugging is off -- exercises the static initializer with nothing to react to yet. + new RemoteWebDriver(inMemoryExecutor, new ImmutableCapabilities()); + + System.setProperty("selenium.debug", "true"); + + // Second construction, after the property changed. The class's static initializer already + // ran once and won't run again, so picking this up can only be the canonical constructor's + // own call to Debug.configureLogger(). + new RemoteWebDriver(inMemoryExecutor, new ImmutableCapabilities()); + + assertThat(seleniumLogger().getLevel()).isEqualTo(Level.FINE); + } @Test void testQuitsIfStartSessionFails() { @@ -155,6 +220,29 @@ && singleton(capabilities) assertThat(driver.getSessionId()).isNotNull(); } + @Test + void constructorTreatsNullCapabilitiesAsEmptyCapabilities() { + // Javadoc on the canonical constructor promises "null is treated as an empty set of + // capabilities" -- verify startSession() actually receives the coalesced empty + // ImmutableCapabilities, not the raw null parameter, and that this does not NPE. + // A plain in-memory executor (no mocking framework): records the single NEW_SESSION command + // this construction issues, then answers it by echoing the requested capabilities back. + AtomicReference sentCommand = new AtomicReference<>(); + CommandExecutor executor = + command -> { + sentCommand.set(command); + return echoCapabilities.apply(command); + }; + + RemoteWebDriver driver = new RemoteWebDriver(executor, null); + + assertThat(sentCommand.get().getName()).isEqualTo(DriverCommand.NEW_SESSION); + assertThat(sentCommand.get().getSessionId()).isNull(); + assertThat(sentCommand.get().getParameters().get("capabilities")) + .isEqualTo(singleton(new ImmutableCapabilities())); + assertThat(driver.getSessionId()).isNotNull(); + } + @Test void canHandlePlatformNameCapability() { WebDriverFixture fixture = diff --git a/java/test/org/openqa/selenium/remote/http/RetryRequestTest.java b/java/test/org/openqa/selenium/remote/http/RetryRequestTest.java index aec6358cdf088..4ff358714f19d 100644 --- a/java/test/org/openqa/selenium/remote/http/RetryRequestTest.java +++ b/java/test/org/openqa/selenium/remote/http/RetryRequestTest.java @@ -41,6 +41,10 @@ import java.util.concurrent.Future; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import java.util.logging.Handler; +import java.util.logging.Level; +import java.util.logging.LogRecord; +import java.util.logging.Logger; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.openqa.selenium.TimeoutException; @@ -355,4 +359,47 @@ void shouldDeliverUnmodifiedServerErrors() { assertThat(handler.execute(new HttpRequest(GET, "/"))).isSameAs(lastResponse.get()); assertThat(count).hasValue(3); } + + @Test + void retryRecordsStayFineWhenSystemPropertyDebuggingIsEnabled() { + Logger logger = Logger.getLogger(RetryRequest.class.getName()); + Level oldLevel = logger.getLevel(); + String originalDebugProperty = System.getProperty("selenium.debug"); + List records = new ArrayList<>(); + Handler handler = + new Handler() { + @Override + public void publish(LogRecord record) { + records.add(record); + } + + @Override + public void flush() {} + + @Override + public void close() {} + }; + handler.setLevel(Level.ALL); + logger.setLevel(Level.ALL); + logger.addHandler(handler); + System.setProperty("selenium.debug", "false"); + try { + System.setProperty("selenium.debug", "true"); + new RetryRequest() + .andFinally(request -> new HttpResponse().setStatus(HTTP_UNAVAILABLE)) + .execute(new HttpRequest(GET, "/")); + + assertThat(records).extracting(LogRecord::getLevel).contains(Level.FINE); + } finally { + System.setProperty("selenium.debug", "false"); + assertThat(System.getProperty("selenium.debug")).isEqualTo("false"); + logger.removeHandler(handler); + logger.setLevel(oldLevel); + if (originalDebugProperty == null) { + System.clearProperty("selenium.debug"); + } else { + System.setProperty("selenium.debug", originalDebugProperty); + } + } + } }