From 9cd89ae0525b327ce435ae3e4aadcba035621ee9 Mon Sep 17 00:00:00 2001 From: Max Schmitt Date: Tue, 20 Apr 2021 18:54:53 +0200 Subject: [PATCH] fix: host dependency validation (#6227) --- .../installation-tests/installation-tests.sh | 34 +++++++++++++++++ ...idate-dependencies-skip-executable-path.js | 10 +++++ .../validate-dependencies.js | 8 ++++ src/install/installer.ts | 2 +- src/server/browserType.ts | 7 ++-- src/utils/registry.ts | 37 ++++++++++--------- 6 files changed, 77 insertions(+), 21 deletions(-) create mode 100644 packages/installation-tests/validate-dependencies-skip-executable-path.js create mode 100644 packages/installation-tests/validate-dependencies.js diff --git a/packages/installation-tests/installation-tests.sh b/packages/installation-tests/installation-tests.sh index 1881112d99..8fc9039a86 100755 --- a/packages/installation-tests/installation-tests.sh +++ b/packages/installation-tests/installation-tests.sh @@ -42,6 +42,8 @@ function copy_test_scripts { cp "${SCRIPTS_PATH}/inspector-custom-executable.js" . cp "${SCRIPTS_PATH}/sanity.js" . cp "${SCRIPTS_PATH}/screencast.js" . + cp "${SCRIPTS_PATH}/validate-dependencies.js" . + cp "${SCRIPTS_PATH}/validate-dependencies-skip-executable-path.js" . cp "${SCRIPTS_PATH}/esm.mjs" . cp "${SCRIPTS_PATH}/esm-playwright.mjs" . cp "${SCRIPTS_PATH}/esm-playwright-chromium.mjs" . @@ -64,6 +66,8 @@ function run_tests { test_playwright_chromium_should_work test_playwright_webkit_should_work test_playwright_firefox_should_work + test_playwright_validate_dependencies + test_playwright_validate_dependencies_skip_executable_path test_playwright_global_installation test_playwright_global_installation_cross_package test_playwright_electron_should_work @@ -359,6 +363,36 @@ function test_playwright_firefox_should_work { echo "${FUNCNAME[0]} success" } +function test_playwright_validate_dependencies { + initialize_test "${FUNCNAME[0]}" + + npm install ${PLAYWRIGHT_TGZ} + copy_test_scripts + + OUTPUT="$(node validate-dependencies.js)" + if [[ "${OUTPUT}" != *"PLAYWRIGHT_SKIP_VALIDATE_HOST_REQUIREMENTS"* ]]; then + echo "ERROR: validateDependencies was not called" + exit 1 + fi + + echo "${FUNCNAME[0]} success" +} + +function test_playwright_validate_dependencies_skip_executable_path { + initialize_test "${FUNCNAME[0]}" + + npm install ${PLAYWRIGHT_TGZ} + copy_test_scripts + + OUTPUT="$(node validate-dependencies-skip-executable-path.js)" + if [[ "${OUTPUT}" == *"PLAYWRIGHT_SKIP_VALIDATE_HOST_REQUIREMENTS"* ]]; then + echo "ERROR: validateDependencies was called" + exit 1 + fi + + echo "${FUNCNAME[0]} success" +} + function test_playwright_electron_should_work { initialize_test "${FUNCNAME[0]}" diff --git a/packages/installation-tests/validate-dependencies-skip-executable-path.js b/packages/installation-tests/validate-dependencies-skip-executable-path.js new file mode 100644 index 0000000000..09a9a55321 --- /dev/null +++ b/packages/installation-tests/validate-dependencies-skip-executable-path.js @@ -0,0 +1,10 @@ +const playwright = require('playwright'); + +process.env.PLAYWRIGHT_SKIP_VALIDATE_HOST_REQUIREMENTS = 1; + +(async () => { + const browser = await playwright.chromium.launch({ + executablePath: playwright.chromium.executablePath() + }); + await browser.close(); +})(); \ No newline at end of file diff --git a/packages/installation-tests/validate-dependencies.js b/packages/installation-tests/validate-dependencies.js new file mode 100644 index 0000000000..b63d4a9357 --- /dev/null +++ b/packages/installation-tests/validate-dependencies.js @@ -0,0 +1,8 @@ +const playwright = require('playwright'); + +process.env.PLAYWRIGHT_SKIP_VALIDATE_HOST_REQUIREMENTS = 1; + +(async () => { + const browser = await playwright.chromium.launch(); + await browser.close(); +})(); \ No newline at end of file diff --git a/src/install/installer.ts b/src/install/installer.ts index 182c81b3b2..620c131837 100644 --- a/src/install/installer.ts +++ b/src/install/installer.ts @@ -74,7 +74,7 @@ async function validateCache(linksDir: string, browserNames: BrowserName[]) { linkTarget = (await fsReadFileAsync(linkPath)).toString(); const linkRegistry = new Registry(linkTarget); for (const browserName of allBrowserNames) { - if (!linkRegistry.shouldRetain(browserName)) + if (!linkRegistry.isSupportedBrowser(browserName)) continue; const usedBrowserPath = linkRegistry.browserDirectory(browserName); const browserRevision = linkRegistry.revision(browserName); diff --git a/src/server/browserType.ts b/src/server/browserType.ts index b71de74cc6..9b9173d2e4 100644 --- a/src/server/browserType.ts +++ b/src/server/browserType.ts @@ -178,10 +178,11 @@ export abstract class BrowserType extends SdkObject { throw new Error(errorMessageLines.join('\n')); } - if (!executable) { - // Only validate dependencies for bundled browsers. + // Only validate dependencies for downloadable browsers. + if (!executablePath && !options.channel) await validateHostRequirements(this._registry, this._name); - } + else if (!executablePath && options.channel && this._registry.isSupportedBrowser(options.channel)) + await validateHostRequirements(this._registry, options.channel as registry.BrowserName); let wsEndpointCallback: ((wsEndpoint: string) => void) | undefined; const wsEndpoint = options.useWebSocket ? new Promise(f => wsEndpointCallback = f) : undefined; diff --git a/src/utils/registry.ts b/src/utils/registry.ts index 9cf20b6391..2c14aef6fc 100644 --- a/src/utils/registry.ts +++ b/src/utils/registry.ts @@ -292,21 +292,25 @@ export class Registry { linuxLddDirectories(browserName: BrowserName): string[] { const browserDirectory = this.browserDirectory(browserName); - if (browserName === 'chromium') - return [path.join(browserDirectory, 'chrome-linux')]; - if (browserName === 'firefox') - return [path.join(browserDirectory, 'firefox')]; - if (browserName === 'webkit') { - return [ - path.join(browserDirectory, 'minibrowser-gtk'), - path.join(browserDirectory, 'minibrowser-gtk', 'bin'), - path.join(browserDirectory, 'minibrowser-gtk', 'lib'), - path.join(browserDirectory, 'minibrowser-wpe'), - path.join(browserDirectory, 'minibrowser-wpe', 'bin'), - path.join(browserDirectory, 'minibrowser-wpe', 'lib'), - ]; + switch (browserName) { + case 'chromium': + return [path.join(browserDirectory, 'chrome-linux')]; + case 'webkit': + case 'webkit-technology-preview': + return [ + path.join(browserDirectory, 'minibrowser-gtk'), + path.join(browserDirectory, 'minibrowser-gtk', 'bin'), + path.join(browserDirectory, 'minibrowser-gtk', 'lib'), + path.join(browserDirectory, 'minibrowser-wpe'), + path.join(browserDirectory, 'minibrowser-wpe', 'bin'), + path.join(browserDirectory, 'minibrowser-wpe', 'lib'), + ]; + case 'firefox': + case 'firefox-stable': + return [path.join(browserDirectory, 'firefox')]; + default: + return []; } - return []; } windowsExeAndDllDirectories(browserName: BrowserName): string[] { @@ -345,14 +349,13 @@ export class Registry { return util.format(urlTemplate, downloadHost, browser.revision); } - shouldRetain(browserName: BrowserName): boolean { + isSupportedBrowser(browserName: string): boolean { // We retain browsers if they are found in the descriptor. // Note, however, that there are older versions out in the wild that rely on // the "download" field in the browser descriptor and use its value // to retain and download browsers. // As of v1.10, we decided to abandon "download" field. - const browser = this._descriptors.find(browser => browser.name === browserName); - return !!browser; + return this._descriptors.some(browser => browser.name === browserName); } installByDefault(): BrowserName[] {