[java] Migrate base for Closure tests to JUnit Jupiter (JUnit 5) (#10786

) * [java] Migrate based for Closure tests to Jupiter (JUnit 5) Part of the migration of Java tests to JUnit 5 (issue #10196). * Include SHA for bazel-contrib/rules_jvm * [java] smell-fix: DriverSupplier is always an instance of Closeable
SeleniumHQ · Jun 20, 2022 · 43361aa · 43361aa
1 parent a268142
commit 43361aa
Show file tree

Hide file tree

Showing 5 changed files with 65 additions and 121 deletions.
diff --git a/WORKSPACE b/WORKSPACE
@@ -132,7 +132,7 @@ rules_jvm_external_setup()
 
 http_archive(
     name = "contrib_rules_jvm",
-    sha256 = None,
+    sha256 = "c561a011b3005ccb3d6903bf60d655e2539189e3ed9b3595ecffd64e2061a338",
     strip_prefix = "rules_jvm-35d89231aed8527b49dd2caba966e9cb097b4da6",
     url = "https://github.com/bazel-contrib/rules_jvm/archive/35d89231aed8527b49dd2caba966e9cb097b4da6.zip",
 )

diff --git a/java/test/org/openqa/selenium/javascript/BUILD.bazel b/java/test/org/openqa/selenium/javascript/BUILD.bazel
@@ -1,5 +1,5 @@
 load("@rules_jvm_external//:defs.bzl", "artifact")
-load("//java:defs.bzl", "java_library")
+load("//java:defs.bzl", "JUNIT5_DEPS", "java_library")
 
 java_library(
     name = "javascript",
@@ -13,7 +13,7 @@ java_library(
         "//java/test/org/openqa/selenium/testing:test-base",
         "//java/test/org/openqa/selenium/testing/drivers",
         artifact("com.google.guava:guava"),
-        artifact("junit:junit"),
+        artifact("org.junit.jupiter:junit-jupiter-api"),
         artifact("org.assertj:assertj-core"),
-    ],
+    ] + JUNIT5_DEPS,
 )
diff --git a/java/test/org/openqa/selenium/javascript/ClosureTestStatement.java b/java/test/org/openqa/selenium/javascript/ClosureTestStatement.java
@@ -17,12 +17,7 @@
 
 package org.openqa.selenium.javascript;
 
-import static org.junit.Assert.fail;
-import static org.openqa.selenium.testing.TestUtilities.isOnTravis;
-
 import com.google.common.base.Stopwatch;
-
-import org.junit.runners.model.Statement;
 import org.openqa.selenium.JavascriptExecutor;
 import org.openqa.selenium.OutputType;
 import org.openqa.selenium.TakesScreenshot;
@@ -35,7 +30,10 @@
 import java.util.function.Supplier;
 import java.util.logging.Logger;
 
-public class ClosureTestStatement extends Statement {
+import static org.junit.jupiter.api.Assertions.fail;
+import static org.openqa.selenium.testing.TestUtilities.isOnTravis;
+
+public class ClosureTestStatement {
 
   private static final Logger LOG = Logger.getLogger(ClosureTestStatement.class.getName());
 
@@ -45,17 +43,16 @@ public class ClosureTestStatement extends Statement {
   private final long timeoutSeconds;
 
   public ClosureTestStatement(
-      Supplier<WebDriver> driverSupplier,
-      String testPath,
-      Function<String, URL> filePathToUrlFn,
-      long timeoutSeconds) {
+    Supplier<WebDriver> driverSupplier,
+    String testPath,
+    Function<String, URL> filePathToUrlFn,
+    long timeoutSeconds) {
     this.driverSupplier = driverSupplier;
     this.testPath = testPath;
     this.filePathToUrlFn = filePathToUrlFn;
     this.timeoutSeconds = Math.max(0, timeoutSeconds);
   }
 
-  @Override
   public void evaluate() throws Throwable {
     URL testUrl = filePathToUrlFn.apply(testPath);
     LOG.info("Running: " + testUrl);
@@ -87,9 +84,9 @@ public void evaluate() throws Throwable {
       long elapsedTime = stopwatch.elapsed(TimeUnit.SECONDS);
       if (timeoutSeconds > 0 && elapsedTime > timeoutSeconds) {
         throw new JavaScriptAssertionError("Tests timed out after " + elapsedTime + " s. \nCaptured Errors: " +
-                                           ((JavascriptExecutor) driver).executeScript("return window.errors;")
-                                           + "\nPageSource: " + driver.getPageSource() + "\nScreenshot: " +
-                                           ((TakesScreenshot)driver).getScreenshotAs(OutputType.BASE64));
+          ((JavascriptExecutor) driver).executeScript("return window.errors;")
+          + "\nPageSource: " + driver.getPageSource() + "\nScreenshot: " +
+          ((TakesScreenshot) driver).getScreenshotAs(OutputType.BASE64));
       }
       TimeUnit.MILLISECONDS.sleep(100);
     }

diff --git a/java/test/org/openqa/selenium/javascript/ClosureTestSuite.java b/java/test/org/openqa/selenium/javascript/ClosureTestSuite.java
@@ -17,16 +17,19 @@
 
 package org.openqa.selenium.javascript;
 
-import org.junit.BeforeClass;
-import org.junit.runner.RunWith;
+import org.junit.jupiter.api.BeforeAll;
+
+import java.io.IOException;
 
 import static org.assertj.core.api.Assumptions.assumeThat;
 
+public class ClosureTestSuite extends JavaScriptTestSuite {
 
-@RunWith(JavaScriptTestSuite.class)
-public class ClosureTestSuite {
+  public ClosureTestSuite() throws IOException {
+    super();
+  }
 
-  @BeforeClass
+  @BeforeAll
   public static void checkShouldRun() {
     assumeThat(Boolean.getBoolean("selenium.skiptest")).isFalse();
   }

diff --git a/java/test/org/openqa/selenium/javascript/JavaScriptTestSuite.java b/java/test/org/openqa/selenium/javascript/JavaScriptTestSuite.java
@@ -17,56 +17,68 @@
 
 package org.openqa.selenium.javascript;
 
-import static java.util.stream.Collectors.toList;
-
-import org.junit.runner.Description;
-import org.junit.runner.Runner;
-import org.junit.runner.notification.Failure;
-import org.junit.runner.notification.RunNotifier;
-import org.junit.runners.ParentRunner;
-import org.junit.runners.model.InitializationError;
-import org.junit.runners.model.Statement;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.DynamicTest;
+import org.junit.jupiter.api.TestFactory;
 import org.openqa.selenium.WebDriver;
+import org.openqa.selenium.build.InProject;
 import org.openqa.selenium.environment.GlobalTestEnvironment;
 import org.openqa.selenium.environment.InProcessTestEnvironment;
 import org.openqa.selenium.environment.TestEnvironment;
 import org.openqa.selenium.environment.webserver.AppServer;
-import org.openqa.selenium.build.InProject;
 import org.openqa.selenium.testing.drivers.WebDriverBuilder;
 
 import java.io.Closeable;
 import java.io.IOException;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.nio.file.Path;
+import java.util.Collection;
 import java.util.List;
 import java.util.function.Function;
 import java.util.function.Supplier;
 
+import static java.util.stream.Collectors.toList;
+import static org.junit.jupiter.api.DynamicTest.dynamicTest;
+
 /**
- * JUnit4 test runner for Closure-based JavaScript tests.
+ * JUnit5 test base for Closure-based JavaScript tests.
  */
-public class JavaScriptTestSuite extends ParentRunner<Runner> {
+public class JavaScriptTestSuite {
 
-  private final List<Runner> children;
   private final Supplier<WebDriver> driverSupplier;
 
-  public JavaScriptTestSuite(Class<?> testClass) throws InitializationError, IOException {
-    super(testClass);
-
-    long timeout = Math.max(0, Long.getLong("js.test.timeout", 0));
+  private final long timeout;
 
-    driverSupplier = new DriverSupplier();
+  private TestEnvironment testEnvironment;
 
-    children = createChildren(driverSupplier, timeout);
+  public JavaScriptTestSuite() {
+    this.timeout = Math.max(0, Long.getLong("js.test.timeout", 0));
+    this.driverSupplier = new DriverSupplier();
   }
 
   private static boolean isBazel() {
     return InProject.findRunfilesRoot() != null;
   }
 
-  private static List<Runner> createChildren(
-      final Supplier<WebDriver> driverSupplier, final long timeout) throws IOException {
+  @BeforeEach
+  public void setup() {
+    testEnvironment = GlobalTestEnvironment.getOrCreate(InProcessTestEnvironment::new);
+  }
+
+  @AfterEach
+  public void teardown() throws IOException {
+    if (testEnvironment != null) {
+      testEnvironment.stop();
+    }
+    if (driverSupplier != null) {
+      ((Closeable) driverSupplier).close();
+    }
+  }
+
+  @TestFactory
+  public Collection<DynamicTest> dynamicTests() throws IOException {
     final Path baseDir = InProject.findProjectRoot();
     final Function<String, URL> pathToUrlFn = s -> {
       AppServer appServer = GlobalTestEnvironment.get().getAppServer();
@@ -83,83 +95,15 @@ private static List<Runner> createChildren(
 
     List<Path> tests = TestFileLocator.findTestFiles();
     return tests.stream()
-        .map(file -> {
-          final String path = TestFileLocator.getTestFilePath(baseDir, file);
-          Description description = Description.createSuiteDescription(
-              path.replaceAll(".html$", ""));
-
-          Statement testStatement = new ClosureTestStatement(
-              driverSupplier, path, pathToUrlFn, timeout);
-          return new StatementRunner(testStatement, description);
-        })
-        .collect(toList());
-  }
-
-  @Override
-  protected List<Runner> getChildren() {
-    return children;
-  }
-
-  @Override
-  protected Description describeChild(Runner child) {
-    return child.getDescription();
-  }
-
-  @Override
-  protected void runChild(Runner child, RunNotifier notifier) {
-    child.run(notifier);
-  }
-
-  @Override
-  protected Statement classBlock(RunNotifier notifier) {
-    final Statement suite = super.classBlock(notifier);
-
-    return new Statement() {
-      @Override
-      public void evaluate() throws Throwable {
-        TestEnvironment testEnvironment = null;
-        try {
-          testEnvironment = GlobalTestEnvironment.getOrCreate(InProcessTestEnvironment::new);
-          suite.evaluate();
-        } finally {
-          if (testEnvironment != null) {
-            testEnvironment.stop();
-          }
-          if (driverSupplier instanceof Closeable) {
-            ((Closeable) driverSupplier).close();
-          }
-        }
-      }
-    };
-  }
-
-  private static class StatementRunner extends Runner {
-
-    private final Description description;
-    private final Statement testStatement;
-
-    private StatementRunner(Statement testStatement, Description description) {
-      this.testStatement = testStatement;
-      this.description = description;
-    }
-
-    @Override
-    public Description getDescription() {
-      return description;
-    }
-
-    @Override
-    public void run(RunNotifier notifier) {
-      notifier.fireTestStarted(description);
-      try {
-        testStatement.evaluate();
-      } catch (Throwable throwable) {
-        Failure failure = new Failure(description, throwable);
-        notifier.fireTestFailure(failure);
-      } finally {
-        notifier.fireTestFinished(description);
-      }
-    }
+      .map(file -> {
+        final String path = TestFileLocator.getTestFilePath(baseDir, file);
+        String title = path.replaceAll(".html$", "");
+        ClosureTestStatement testStatement = new ClosureTestStatement(
+          driverSupplier, path, pathToUrlFn, timeout);
+
+        return dynamicTest(title, testStatement::evaluate);
+      })
+      .collect(toList());
   }
 
   private static class DriverSupplier implements Supplier<WebDriver>, Closeable {