From 7eadce16c26b951ab7e9a02ac271dc2cd2f0f965 Mon Sep 17 00:00:00 2001 From: Boni Garcia Date: Mon, 20 Jun 2022 14:02:42 +0200 Subject: [PATCH 1/3] [java] Migrate based for Closure tests to Jupiter (JUnit 5) Part of the migration of Java tests to JUnit 5 (issue #10196). --- .../openqa/selenium/javascript/BUILD.bazel | 6 +- .../javascript/ClosureTestStatement.java | 25 ++-- .../selenium/javascript/ClosureTestSuite.java | 13 +- .../javascript/JavaScriptTestSuite.java | 140 ++++++------------ 4 files changed, 64 insertions(+), 120 deletions(-) diff --git a/java/test/org/openqa/selenium/javascript/BUILD.bazel b/java/test/org/openqa/selenium/javascript/BUILD.bazel index 0c3afd1a4f820..ff4b12f6bc24d 100644 --- 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 index 9363ec97ed3d7..6d2af688f9b37 100644 --- 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 driverSupplier, - String testPath, - Function filePathToUrlFn, - long timeoutSeconds) { + Supplier driverSupplier, + String testPath, + Function 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 index ff3988ad4b25a..239bb4b808948 100644 --- 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 index f9e99a0e07e94..2e030c5769635 100644 --- a/java/test/org/openqa/selenium/javascript/JavaScriptTestSuite.java +++ b/java/test/org/openqa/selenium/javascript/JavaScriptTestSuite.java @@ -17,21 +17,16 @@ 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; @@ -39,34 +34,51 @@ 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 { +public class JavaScriptTestSuite { - private final List children; private final Supplier 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 createChildren( - final Supplier 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 instanceof Closeable) { + ((Closeable) driverSupplier).close(); + } + } + + @TestFactory + public Collection dynamicTests() throws IOException { final Path baseDir = InProject.findProjectRoot(); final Function pathToUrlFn = s -> { AppServer appServer = GlobalTestEnvironment.get().getAppServer(); @@ -83,83 +95,15 @@ private static List createChildren( List 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 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, Closeable { From ff3f92721f62e71b530bf50f618c43b5f193c738 Mon Sep 17 00:00:00 2001 From: Boni Garcia Date: Mon, 20 Jun 2022 16:07:40 +0200 Subject: [PATCH 2/3] Include SHA for bazel-contrib/rules_jvm --- WORKSPACE | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WORKSPACE b/WORKSPACE index 513a6cdb8e80d..3aa48f4b8aca9 100644 --- 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", ) From ef13e05628641d70124da4c8cf37d0150f05992f Mon Sep 17 00:00:00 2001 From: Boni Garcia Date: Mon, 20 Jun 2022 16:19:37 +0200 Subject: [PATCH 3/3] [java] smell-fix: DriverSupplier is always an instance of Closeable --- .../org/openqa/selenium/javascript/JavaScriptTestSuite.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/test/org/openqa/selenium/javascript/JavaScriptTestSuite.java b/java/test/org/openqa/selenium/javascript/JavaScriptTestSuite.java index 2e030c5769635..136be4aa20f89 100644 --- a/java/test/org/openqa/selenium/javascript/JavaScriptTestSuite.java +++ b/java/test/org/openqa/selenium/javascript/JavaScriptTestSuite.java @@ -72,7 +72,7 @@ public void teardown() throws IOException { if (testEnvironment != null) { testEnvironment.stop(); } - if (driverSupplier instanceof Closeable) { + if (driverSupplier != null) { ((Closeable) driverSupplier).close(); } }