From 7447f6d9dc1fefe31eca15771704b4d547ff9585 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Wed, 20 Mar 2024 16:36:32 -0700 Subject: [PATCH] chore: get rid of driver wrapper script 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 04f2328de..2ac8bbebf 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 @@ 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 @@ protected void initialize(Boolean installBrowsers) throws Exception { 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 @@ private void installBrowsers(Map env) throws IOException, Interr 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 @@ private static String platformDir() { } @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 f68416479..953549bf1 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.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 @@ void shouldThrowWhenBrowserPathIsInvalid(@TempDir Path tmpDir) throws NoSuchFiel @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 @@ void playwrightCliInstalled() throws Exception { 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 @@ void canSpecifyPreinstalledNodeJsAsEnv(@TempDir Path tmpDir) throws IOException, 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 @@ private static boolean isWindows() { } 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 9ee059eaf..a4165c989 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 @@ */ 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 @@ protected void initialize(Boolean installBrowsers) { } @Override - protected Path driverDir() { + public Path driverDir() { return driverDir; } } @@ -65,14 +66,14 @@ private void initialize(Map env, Boolean installBrowsers) throws } 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 @@ private static Driver newInstance() throws Exception { 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 9becc7efd..4a23eedc6 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 @@ void kill() throws InterruptedException { 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