[java] start the secure server only when needed in unit tests (SeleniumHQ#14717)

joerg1985 · pujagani · web-flow · commit 9f5ee9f425b2 · 2024-11-06T11:09:21.000+05:30
Co-authored-by: Puja Jagani &lt;puja.jagani93@gmail.com&gt;
diff --git a/java/test/org/openqa/selenium/CookieImplementationTest.java b/java/test/org/openqa/selenium/CookieImplementationTest.java
@@ -35,10 +35,12 @@
 import org.openqa.selenium.environment.DomainHelper;
 import org.openqa.selenium.testing.Ignore;
 import org.openqa.selenium.testing.JupiterTestBase;
+import org.openqa.selenium.testing.NeedsSecureServer;
 import org.openqa.selenium.testing.NotWorkingInRemoteBazelBuilds;
 import org.openqa.selenium.testing.NotYetImplemented;
 import org.openqa.selenium.testing.SwitchToTopAfterTest;
 
+@NeedsSecureServer
 class CookieImplementationTest extends JupiterTestBase {
 
   private DomainHelper domainHelper;
diff --git a/java/test/org/openqa/selenium/PageLoadingTest.java b/java/test/org/openqa/selenium/PageLoadingTest.java
@@ -36,10 +36,12 @@
 import org.openqa.selenium.testing.Ignore;
 import org.openqa.selenium.testing.JupiterTestBase;
 import org.openqa.selenium.testing.NeedsFreshDriver;
+import org.openqa.selenium.testing.NeedsSecureServer;
 import org.openqa.selenium.testing.NoDriverAfterTest;
 import org.openqa.selenium.testing.NotYetImplemented;
 import org.openqa.selenium.testing.SwitchToTopAfterTest;
 
+@NeedsSecureServer
 class PageLoadingTest extends JupiterTestBase {
 
   @Test
diff --git a/java/test/org/openqa/selenium/environment/InProcessTestEnvironment.java b/java/test/org/openqa/selenium/environment/InProcessTestEnvironment.java
@@ -24,8 +24,8 @@ public class InProcessTestEnvironment implements TestEnvironment {
 
   private final AppServer appServer;
 
-  public InProcessTestEnvironment() {
-    appServer = new NettyAppServer();
+  public InProcessTestEnvironment(boolean secureServer) {
+    appServer = new NettyAppServer(secureServer);
     appServer.start();
   }
 
@@ -40,6 +40,6 @@ public void stop() {
   }
 
   public static void main(String[] args) {
-    new InProcessTestEnvironment();
+    new InProcessTestEnvironment(true);
   }
 }
diff --git a/java/test/org/openqa/selenium/environment/webserver/NettyAppServer.java b/java/test/org/openqa/selenium/environment/webserver/NettyAppServer.java
@@ -68,13 +68,13 @@ public class NettyAppServer implements AppServer {
                 LOG.log(
                     Level.WARNING,
                     String.format("NettyAppServer retry #%s. ", e.getAttemptCount()));
-                initValues();
+                initValues(secure != null);
               })
           .onRetriesExceeded(e -> LOG.log(Level.WARNING, "NettyAppServer start aborted."))
           .build();
 
-  public NettyAppServer() {
-    initValues();
+  public NettyAppServer(boolean secureServer) {
+    initValues(secureServer);
   }
 
   public NettyAppServer(HttpHandler handler) {
@@ -116,7 +116,7 @@ public static void main(String[] args) {
     System.out.printf("Server started. Root URL: %s%n", server.whereIs("/"));
   }
 
-  private void initValues() {
+  private void initValues(boolean secureServer) {
     Config config = createDefaultConfig();
     BaseServerOptions options = new BaseServerOptions(config);
 
@@ -128,16 +128,18 @@ private void initValues() {
 
     server = new NettyServer(options, handler);
 
-    Config secureConfig = new CompoundConfig(sslConfig, createDefaultConfig());
-    BaseServerOptions secureOptions = new BaseServerOptions(secureConfig);
+    if (secureServer) {
+      Config secureConfig = new CompoundConfig(sslConfig, createDefaultConfig());
+      BaseServerOptions secureOptions = new BaseServerOptions(secureConfig);
 
-    HttpHandler secureHandler =
-        new HandlersForTests(
-            secureOptions.getHostname().orElse("localhost"),
-            secureOptions.getPort(),
-            tempDir.toPath());
+      HttpHandler secureHandler =
+          new HandlersForTests(
+              secureOptions.getHostname().orElse("localhost"),
+              secureOptions.getPort(),
+              tempDir.toPath());
 
-    secure = new NettyServer(secureOptions, secureHandler);
+      secure = new NettyServer(secureOptions, secureHandler);
+    }
   }
 
   @Override
diff --git a/java/test/org/openqa/selenium/environment/webserver/NettyAppServerTest.java b/java/test/org/openqa/selenium/environment/webserver/NettyAppServerTest.java
@@ -21,6 +21,6 @@ class NettyAppServerTest extends AppServerTestBase {
 
   @Override
   protected AppServer createAppServer() {
-    return new NettyAppServer();
+    return new NettyAppServer(false);
   }
 }
diff --git a/java/test/org/openqa/selenium/federatedcredentialmanagement/FederatedCredentialManagementTest.java b/java/test/org/openqa/selenium/federatedcredentialmanagement/FederatedCredentialManagementTest.java
@@ -36,8 +36,10 @@
 import org.openqa.selenium.chrome.ChromeOptions;
 import org.openqa.selenium.support.ui.WebDriverWait;
 import org.openqa.selenium.testing.JupiterTestBase;
+import org.openqa.selenium.testing.NeedsSecureServer;
 import org.openqa.selenium.testing.NotYetImplemented;
 
+@NeedsSecureServer
 class FederatedCredentialManagementTest extends JupiterTestBase {
 
   private JavascriptExecutor jsAwareDriver;
diff --git a/java/test/org/openqa/selenium/grid/router/RemoteWebDriverBiDiTest.java b/java/test/org/openqa/selenium/grid/router/RemoteWebDriverBiDiTest.java
@@ -61,7 +61,7 @@ class RemoteWebDriverBiDiTest {
 
   @BeforeAll
   static void serverSetup() {
-    server = new NettyAppServer();
+    server = new NettyAppServer(false);
     server.start();
   }
 
diff --git a/java/test/org/openqa/selenium/grid/router/RemoteWebDriverDownloadTest.java b/java/test/org/openqa/selenium/grid/router/RemoteWebDriverDownloadTest.java
@@ -88,7 +88,7 @@ public void setupServers() {
     tearDowns.add(deployment);
 
     server = deployment.getServer();
-    appServer = new NettyAppServer();
+    appServer = new NettyAppServer(false);
     tearDowns.add(() -> appServer.stop());
     appServer.start();
   }
diff --git a/java/test/org/openqa/selenium/javascript/JavaScriptTestSuite.java b/java/test/org/openqa/selenium/javascript/JavaScriptTestSuite.java
@@ -62,7 +62,7 @@ private static boolean isBazel() {
   @BeforeEach
   public void setup() {
     // this field is actually in use, javascript test do access it
-    testEnvironment = GlobalTestEnvironment.getOrCreate(InProcessTestEnvironment::new);
+    testEnvironment = GlobalTestEnvironment.getOrCreate(() -> new InProcessTestEnvironment(true));
   }
 
   @AfterEach
diff --git a/java/test/org/openqa/selenium/safari/CrossDomainTest.java b/java/test/org/openqa/selenium/safari/CrossDomainTest.java
@@ -41,7 +41,7 @@ class CrossDomainTest extends JupiterTestBase {
 
   @BeforeAll
   public static void startSecondServer() {
-    otherServer = new NettyAppServer();
+    otherServer = new NettyAppServer(false);
     otherServer.start();
 
     otherPages = new Pages(otherServer);
diff --git a/java/test/org/openqa/selenium/testing/BUILD.bazel b/java/test/org/openqa/selenium/testing/BUILD.bazel
@@ -20,6 +20,7 @@ java_library(
         "Ignore.java",
         "IgnoreList.java",
         "NeedsFreshDriver.java",
+        "NeedsSecureServer.java",
         "NoDriverAfterTest.java",
         "NoDriverBeforeTest.java",
         "NotWorkingInRemoteBazelBuilds.java",
diff --git a/java/test/org/openqa/selenium/testing/JupiterTestBase.java b/java/test/org/openqa/selenium/testing/JupiterTestBase.java
@@ -22,6 +22,8 @@
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.time.Duration;
+import java.util.Optional;
+import java.util.logging.Logger;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
@@ -37,6 +39,8 @@
 
 public abstract class JupiterTestBase {
 
+  private static final Logger LOG = Logger.getLogger(JupiterTestBase.class.getName());
+
   @RegisterExtension protected static SeleniumExtension seleniumExtension = new SeleniumExtension();
 
   protected TestEnvironment environment;
@@ -54,9 +58,27 @@ public static void shouldTestBeRunAtAll() {
 
   @BeforeEach
   public void prepareEnvironment() {
-    environment = GlobalTestEnvironment.getOrCreate(InProcessTestEnvironment::new);
+    boolean needsSecureServer =
+        Optional.ofNullable(this.getClass().getAnnotation(NeedsSecureServer.class))
+            .map(NeedsSecureServer::value)
+            .orElse(false);
+
+    environment =
+        GlobalTestEnvironment.getOrCreate(() -> new InProcessTestEnvironment(needsSecureServer));
     appServer = environment.getAppServer();
 
+    if (needsSecureServer) {
+      try {
+        appServer.whereIsSecure("/");
+      } catch (IllegalStateException ex) {
+        // this should not happen with bazel, a new JVM is used for each class
+        // the annotation is on class level, so we should never see this
+        LOG.info("appServer is restarted with secureServer=true");
+        environment.stop();
+        environment = new InProcessTestEnvironment(true);
+      }
+    }
+
     pages = new Pages(appServer);
 
     driver = seleniumExtension.getDriver();
diff --git a/java/test/org/openqa/selenium/testing/NeedsSecureServer.java b/java/test/org/openqa/selenium/testing/NeedsSecureServer.java
@@ -0,0 +1,30 @@
+// 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.testing;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+@Retention(RetentionPolicy.RUNTIME)
+@Target(ElementType.TYPE)
+public @interface NeedsSecureServer {
+
+  boolean value() default true;
+}

Original file line number	Diff line number	Diff line change
`@@ -24,8 +24,8 @@ public class InProcessTestEnvironment implements TestEnvironment {`
`24`	`24`
`25`	`25`	`private final AppServer appServer;`
`26`	`26`
`27`		`- public InProcessTestEnvironment() {`
`28`		`- appServer = new NettyAppServer();`
	`27`	`+ public InProcessTestEnvironment(boolean secureServer) {`
	`28`	`+ appServer = new NettyAppServer(secureServer);`
`29`	`29`	`appServer.start();`
`30`	`30`	`}`
`31`	`31`
`@@ -40,6 +40,6 @@ public void stop() {`
`40`	`40`	`}`
`41`	`41`
`42`	`42`	`public static void main(String[] args) {`
`43`		`- new InProcessTestEnvironment();`
	`43`	`+ new InProcessTestEnvironment(true);`
`44`	`44`	`}`
`45`	`45`	`}`
Original file line number	Diff line number	Diff line change
`@@ -21,6 +21,6 @@ class NettyAppServerTest extends AppServerTestBase {`
`21`	`21`
`22`	`22`	`@Override`
`23`	`23`	`protected AppServer createAppServer() {`
`24`		`- return new NettyAppServer();`
	`24`	`+ return new NettyAppServer(false);`
`25`	`25`	`}`
`26`	`26`	`}`
Original file line number	Diff line number	Diff line change
`@@ -61,7 +61,7 @@ class RemoteWebDriverBiDiTest {`
`61`	`61`
`62`	`62`	`@BeforeAll`
`63`	`63`	`static void serverSetup() {`
`64`		`- server = new NettyAppServer();`
	`64`	`+ server = new NettyAppServer(false);`
`65`	`65`	`server.start();`
`66`	`66`	`}`
`67`	`67`
Original file line number	Diff line number	Diff line change
`@@ -88,7 +88,7 @@ public void setupServers() {`
`88`	`88`	`tearDowns.add(deployment);`
`89`	`89`
`90`	`90`	`server = deployment.getServer();`
`91`		`- appServer = new NettyAppServer();`
	`91`	`+ appServer = new NettyAppServer(false);`
`92`	`92`	`tearDowns.add(() -> appServer.stop());`
`93`	`93`	`appServer.start();`
`94`	`94`	`}`
Original file line number	Diff line number	Diff line change
`@@ -62,7 +62,7 @@ private static boolean isBazel() {`
`62`	`62`	`@BeforeEach`
`63`	`63`	`public void setup() {`
`64`	`64`	`// this field is actually in use, javascript test do access it`
`65`		`- testEnvironment = GlobalTestEnvironment.getOrCreate(InProcessTestEnvironment::new);`
	`65`	`+ testEnvironment = GlobalTestEnvironment.getOrCreate(() -> new InProcessTestEnvironment(true));`
`66`	`66`	`}`
`67`	`67`
`68`	`68`	`@AfterEach`