From ac1599cc2c5fb7de909e0930d0f4aed5cf1b1158 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Fri, 12 Feb 2021 11:12:06 -0800 Subject: [PATCH] fix(registry): handle relative registry path (#5406) We get relative registry path when PLAYWRIGHT_BROWSERS_PATH or HOME is relative. In this case, it would be good to resolve to the same absolute path during installation and execution, and we can usually do that using INIT_CWD. --- .../installation-tests/installation-tests.sh | 35 ++++++++++++++ packages/installation-tests/sanity.js | 5 +- src/server/firefox/firefox.ts | 2 + src/utils/registry.ts | 46 +++++++++++-------- 4 files changed, 69 insertions(+), 19 deletions(-) diff --git a/packages/installation-tests/installation-tests.sh b/packages/installation-tests/installation-tests.sh index 0fab37ceba..7c599e4262 100755 --- a/packages/installation-tests/installation-tests.sh +++ b/packages/installation-tests/installation-tests.sh @@ -15,14 +15,22 @@ unset PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD export PLAYWRIGHT_BROWSERS_PATH=0 # Pack all packages and put them in our output folder. +echo "Building packages..." PACKAGE_BUILDER="../../../packages/build_package.js" PLAYWRIGHT_CORE_TGZ="$(node ${PACKAGE_BUILDER} playwright-core ./playwright-core.tgz)" +echo "playwright-core built" PLAYWRIGHT_TGZ="$(node ${PACKAGE_BUILDER} playwright ./playwright.tgz)" +echo "playwright built" PLAYWRIGHT_CHROMIUM_TGZ="$(node ${PACKAGE_BUILDER} playwright-chromium ./playwright-chromium.tgz)" +echo "playwright-chromium built" PLAYWRIGHT_WEBKIT_TGZ="$(node ${PACKAGE_BUILDER} playwright-webkit ./playwright-webkit.tgz)" +echo "playwright-webkit built" PLAYWRIGHT_FIREFOX_TGZ="$(node ${PACKAGE_BUILDER} playwright-firefox ./playwright-firefox.tgz)" +echo "playwright-firefox built" PLAYWRIGHT_ELECTRON_TGZ="$(node ${PACKAGE_BUILDER} playwright-electron ./playwright-electron.tgz)" +echo "playwright-electron built" PLAYWRIGHT_ANDROID_TGZ="$(node ${PACKAGE_BUILDER} playwright-android ./playwright-android.tgz)" +echo "playwright-android built" SCRIPTS_PATH="$(pwd -P)/.." TEST_ROOT="$(pwd -P)" @@ -47,6 +55,8 @@ function run_tests { test_skip_browser_download test_playwright_global_installation_subsequent_installs test_playwright_should_work + test_playwright_should_work_with_relative_home_path + test_playwright_should_work_with_relative_browsers_path test_playwright_chromium_should_work test_playwright_webkit_should_work test_playwright_firefox_should_work @@ -210,6 +220,31 @@ function test_playwright_should_work { echo "${FUNCNAME[0]} success" } +function test_playwright_should_work_with_relative_home_path { + initialize_test "${FUNCNAME[0]}" + PLAYWRIGHT_BROWSERS_PATH="" HOME=. npm install ${PLAYWRIGHT_TGZ} + copy_test_scripts + echo "Running sanity.js" + # Firefox does not work with relative HOME. + PLAYWRIGHT_BROWSERS_PATH="" HOME=. node sanity.js playwright chromium webkit + echo "${FUNCNAME[0]} success" +} + +function test_playwright_should_work_with_relative_browsers_path { + initialize_test "${FUNCNAME[0]}" + + # Make sure that browsers path is resolved relative to the `npm install` call location. + mkdir foo + cd foo + PLAYWRIGHT_BROWSERS_PATH="../relative" npm install ${PLAYWRIGHT_TGZ} + cd .. + + copy_test_scripts + echo "Running sanity.js" + PLAYWRIGHT_BROWSERS_PATH="./relative" node sanity.js playwright + echo "${FUNCNAME[0]} success" +} + function test_playwright_chromium_should_work { initialize_test "${FUNCNAME[0]}" diff --git a/packages/installation-tests/sanity.js b/packages/installation-tests/sanity.js index 8a548edd5e..42138e120a 100644 --- a/packages/installation-tests/sanity.js +++ b/packages/installation-tests/sanity.js @@ -23,8 +23,10 @@ let success = { }[requireName]; if (process.argv[3] === 'none') success = []; -if (process.argv[3] === 'all') +else if (process.argv[3] === 'all') success = ['chromium', 'firefox', 'webkit']; +else if (process.argv[3]) + success = process.argv.slice(3) const playwright = require(requireName); @@ -54,6 +56,7 @@ const installer = require(requireName + '/lib/install/installer'); process.exit(1); } catch (e) { // All good. + console.log(`Expected error while launching ${browserType}: ${e}`); } } console.log(`require SUCCESS`); diff --git a/src/server/firefox/firefox.ts b/src/server/firefox/firefox.ts index dc6b5b583a..93fa665a3d 100644 --- a/src/server/firefox/firefox.ts +++ b/src/server/firefox/firefox.ts @@ -40,6 +40,8 @@ export class Firefox extends BrowserType { } _amendEnvironment(env: Env, userDataDir: string, executable: string, browserArguments: string[]): Env { + if (!path.isAbsolute(os.homedir())) + throw new Error(`Cannot launch Firefox with relative home directory. Did you set ${os.platform() === 'win32' ? 'USERPROFILE' : 'HOME'} to a relative path?`); return os.platform() === 'linux' ? { ...env, // On linux Juggler ships the libstdc++ it was linked against. diff --git a/src/utils/registry.ts b/src/utils/registry.ts index 86203e235e..610966e34e 100644 --- a/src/utils/registry.ts +++ b/src/utils/registry.ts @@ -158,12 +158,35 @@ export const hostPlatform = ((): BrowserPlatform => { })(); export const registryDirectory = (() => { + let result: string; + const envDefined = getFromENV('PLAYWRIGHT_BROWSERS_PATH'); - if (envDefined === '0') - return path.join(__dirname, '..', '..', '.local-browsers'); - if (envDefined) - return path.resolve(process.cwd(), envDefined); - return path.join(cacheDirectory(), 'ms-playwright'); + if (envDefined === '0') { + result = path.join(__dirname, '..', '..', '.local-browsers'); + } else if (envDefined) { + result = envDefined; + } else { + let cacheDirectory: string; + if (process.platform === 'linux') + cacheDirectory = process.env.XDG_CACHE_HOME || path.join(os.homedir(), '.cache'); + else if (process.platform === 'darwin') + cacheDirectory = path.join(os.homedir(), 'Library', 'Caches'); + else if (process.platform === 'win32') + cacheDirectory = process.env.LOCALAPPDATA || path.join(os.homedir(), 'AppData', 'Local'); + else + throw new Error('Unsupported platform: ' + process.platform); + result = path.join(cacheDirectory, 'ms-playwright'); + } + + if (!path.isAbsolute(result)) { + // It is important to resolve to the absolute path: + // - for unzipping to work correctly; + // - so that registry directory matches between installation and execution. + // INIT_CWD points to the root of `npm/yarn install` and is probably what + // the user meant when typing the relative path. + result = path.resolve(getFromENV('INIT_CWD') || process.cwd(), result); + } + return result; })(); export function isBrowserDirectory(browserDirectory: string): boolean { @@ -255,16 +278,3 @@ export class Registry { return !!browser && browser.download !== false; } } - -function cacheDirectory() { - if (process.platform === 'linux') - return process.env.XDG_CACHE_HOME || path.join(os.homedir(), '.cache'); - - if (process.platform === 'darwin') - return path.join(os.homedir(), 'Library', 'Caches'); - - if (process.platform === 'win32') - return process.env.LOCALAPPDATA || path.join(os.homedir(), 'AppData', 'Local'); - throw new Error('Unsupported platform: ' + process.platform); -} -