From f15147ba33639cfb5749d6c4692ec039ce7a179a Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Wed, 20 Mar 2024 16:52:36 -0700 Subject: [PATCH] chore: get rid of driver wrapper script (#1519) Fixes https://github.com/microsoft/playwright-java/issues/1518 --- .../playwright/impl/driver/jar/DriverJar.java | 7 +++---- .../impl/driver/jar/TestInstall.java | 12 ++++++------ .../playwright/impl/driver/Driver.java | 19 ++++++++++--------- .../playwright/TestBrowserTypeConnect.java | 2 +- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/driver-bundle/src/main/java/com/microsoft/playwright/impl/driver/jar/DriverJar.java b/driver-bundle/src/main/java/com/microsoft/playwright/impl/driver/jar/DriverJar.java index 04f2328d..2ac8bbeb 100644 --- a/driver-bundle/src/main/java/com/microsoft/playwright/impl/driver/jar/DriverJar.java +++ b/driver-bundle/src/main/java/com/microsoft/playwright/impl/driver/jar/DriverJar.java @@ -29,7 +29,6 @@ import java.util.concurrent.TimeUnit; public class DriverJar extends Driver { private static final String PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD = "PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD"; private static final String SELENIUM_REMOTE_URL = "SELENIUM_REMOTE_URL"; - static final String PLAYWRIGHT_NODEJS_PATH = "PLAYWRIGHT_NODEJS_PATH"; private final Path driverTempDir; private Path preinstalledNodePath; @@ -64,7 +63,7 @@ public class DriverJar extends Driver { env.put(PLAYWRIGHT_NODEJS_PATH, preinstalledNodePath.toString()); } extractDriverToTempDir(); - logMessage("extracted driver from jar to " + driverPath()); + logMessage("extracted driver from jar to " + driverDir()); if (installBrowsers) installBrowsers(env); } @@ -82,7 +81,7 @@ public class DriverJar extends Driver { logMessage("Skipping browsers download because `SELENIUM_REMOTE_URL` env variable is set"); return; } - Path driver = driverPath(); + Path driver = driverDir(); if (!Files.exists(driver)) { throw new RuntimeException("Failed to find driver: " + driver); } @@ -204,7 +203,7 @@ public class DriverJar extends Driver { } @Override - protected Path driverDir() { + public Path driverDir() { return driverTempDir; } } diff --git a/driver-bundle/src/test/java/com/microsoft/playwright/impl/driver/jar/TestInstall.java b/driver-bundle/src/test/java/com/microsoft/playwright/impl/driver/jar/TestInstall.java index f6841647..953549bf 100644 --- a/driver-bundle/src/test/java/com/microsoft/playwright/impl/driver/jar/TestInstall.java +++ b/driver-bundle/src/test/java/com/microsoft/playwright/impl/driver/jar/TestInstall.java @@ -32,7 +32,7 @@ import java.util.HashMap; import java.util.Map; import java.util.concurrent.TimeUnit; -import static com.microsoft.playwright.impl.driver.jar.DriverJar.PLAYWRIGHT_NODEJS_PATH; +import static com.microsoft.playwright.impl.driver.Driver.PLAYWRIGHT_NODEJS_PATH; import static java.util.Collections.singletonMap; import static org.junit.jupiter.api.Assertions.*; @@ -83,7 +83,7 @@ public class TestInstall { @Test void playwrightCliInstalled() throws Exception { Driver driver = Driver.createAndInstall(Collections.emptyMap(), false); - assertTrue(Files.exists(driver.driverPath())); + assertTrue(Files.exists(driver.driverDir())); ProcessBuilder pb = driver.createProcessBuilder(); pb.command().add("install"); @@ -98,7 +98,7 @@ public class TestInstall { void playwrightDriverInAlternativeTmpdir(@TempDir Path tmpdir) throws Exception { System.setProperty("playwright.driver.tmpdir", tmpdir.toString()); DriverJar driver = new DriverJar(); - assertTrue(driver.driverPath().startsWith(tmpdir), "Driver path: " + driver.driverPath() + " tmp: " + tmpdir); + assertTrue(driver.driverDir().startsWith(tmpdir), "Driver path: " + driver.driverDir() + " tmp: " + tmpdir); } @Test @@ -135,7 +135,7 @@ public class TestInstall { private static String extractNodeJsToTemp() throws URISyntaxException, IOException { DriverJar auxDriver = new DriverJar(); auxDriver.extractDriverToTempDir(); - String nodePath = auxDriver.driverPath().getParent().resolve(isWindows() ? "node.exe" : "node").toString(); + String nodePath = auxDriver.driverDir().resolve(isWindows() ? "node.exe" : "node").toString(); return nodePath; } @@ -145,9 +145,9 @@ public class TestInstall { } private static void canSpecifyPreinstalledNodeJsShared(Driver driver, Path tmpDir) throws IOException, URISyntaxException, InterruptedException { - Path builtinNode = driver.driverPath().getParent().resolve("node"); + Path builtinNode = driver.driverDir().resolve("node"); assertFalse(Files.exists(builtinNode), builtinNode.toString()); - Path builtinNodeExe = driver.driverPath().getParent().resolve("node.exe"); + Path builtinNodeExe = driver.driverDir().resolve("node.exe"); assertFalse(Files.exists(builtinNodeExe), builtinNodeExe.toString()); ProcessBuilder pb = driver.createProcessBuilder(); diff --git a/driver/src/main/java/com/microsoft/playwright/impl/driver/Driver.java b/driver/src/main/java/com/microsoft/playwright/impl/driver/Driver.java index 9ee059ea..a4165c98 100644 --- a/driver/src/main/java/com/microsoft/playwright/impl/driver/Driver.java +++ b/driver/src/main/java/com/microsoft/playwright/impl/driver/Driver.java @@ -31,6 +31,7 @@ import static com.microsoft.playwright.impl.driver.DriverLogging.logWithTimestam */ public abstract class Driver { protected final Map env = new LinkedHashMap<>(); + public static final String PLAYWRIGHT_NODEJS_PATH = "PLAYWRIGHT_NODEJS_PATH"; private static Driver instance; @@ -47,7 +48,7 @@ public abstract class Driver { } @Override - protected Path driverDir() { + public Path driverDir() { return driverDir; } } @@ -65,14 +66,14 @@ public abstract class Driver { } protected abstract void initialize(Boolean installBrowsers) throws Exception; - public Path driverPath() { - String cliFileName = System.getProperty("os.name").toLowerCase().contains("windows") ? - "playwright.cmd" : "playwright.sh"; - return driverDir().resolve(cliFileName); - } - public ProcessBuilder createProcessBuilder() { - ProcessBuilder pb = new ProcessBuilder(driverPath().toString()); + String nodePath = env.get("PLAYWRIGHT_NODEJS_PATH"); + if (nodePath == null) { + String node = System.getProperty("os.name").toLowerCase().contains("windows") ? "node.exe" : "node"; + nodePath = driverDir().resolve(node).toAbsolutePath().toString(); + } + ProcessBuilder pb = new ProcessBuilder(nodePath); + pb.command().add(driverDir().resolve("package").resolve("cli.js").toAbsolutePath().toString()); pb.environment().putAll(env); pb.environment().put("PW_LANG_NAME", "java"); pb.environment().put("PW_LANG_NAME_VERSION", getMajorJavaVersion()); @@ -118,7 +119,7 @@ public abstract class Driver { return (Driver) jarDriver.getDeclaredConstructor().newInstance(); } - protected abstract Path driverDir(); + public abstract Path driverDir(); protected static void logMessage(String message) { // This matches log format produced by the server. diff --git a/playwright/src/test/java/com/microsoft/playwright/TestBrowserTypeConnect.java b/playwright/src/test/java/com/microsoft/playwright/TestBrowserTypeConnect.java index 9becc7ef..4a23eedc 100644 --- a/playwright/src/test/java/com/microsoft/playwright/TestBrowserTypeConnect.java +++ b/playwright/src/test/java/com/microsoft/playwright/TestBrowserTypeConnect.java @@ -64,7 +64,7 @@ public class TestBrowserTypeConnect extends TestBase { private static BrowserServer launchBrowserServer(BrowserType browserType) { try { Driver driver = Driver.ensureDriverInstalled(Collections.emptyMap(), false); - Path dir = driver.driverPath().getParent(); + Path dir = driver.driverDir(); String node = dir.resolve(isWindows ? "node.exe" : "node").toString(); String cliJs = dir.resolve("package/cli.js").toString(); // We launch node process directly instead of using playwright.sh script as killing the script