fix: fix PLAYWRIGHT_BROWSERS_PATH treatment (#1662)

Drive-by: introduce installation tests that use `npm pack`
to simulate installation from NPM registry.

Fixes #1651

.github/workflows/tests.yml

@@ -210,30 +210,8 @@ jobs:
         sudo apt-get install libwoff1 libopus0 libwebp6 libwebpdemux2 libenchant1c2a libgudev-1.0-0 libsecret-1-0 libhyphen0 libgdk-pixbuf2.0-0 libegl1 libgles2 libevent-2.1-6 libnotify4 libvpx5 libxslt1.1
     - uses: actions/setup-node@v1
       with:
-        node-version: 10.15
+        # New node.js version supports `--unhandled-rejections=strict` flag that
+        # we use in `installation-tests.sh`.
+        node-version: 12
     - run: npm install
-    - run: npm link
-    - name: Running e2e test for playwright
-      run: |
-        npm link playwright-core
-        node install.js
-        node ../../test/e2e/stub.js chromium firefox webkit
-      working-directory: packages/playwright
-    - name: Running e2e test for playwright-chromium
-      run: |
-        npm link playwright-core
-        node install.js
-        node ../../test/e2e/stub.js chromium
-      working-directory: packages/playwright-chromium
-    - name: Running e2e test for playwright-webkit
-      run: |
-        npm link playwright-core
-        node install.js
-        node ../../test/e2e/stub.js webkit
-      working-directory: packages/playwright-webkit
-    - name: Running e2e test for playwright-firefox
-      run: |
-        npm link playwright-core
-        node install.js
-        node ../../test/e2e/stub.js firefox
-      working-directory: packages/playwright-firefox
+    - run: bash test/installation-tests/installation-tests.sh

src/server/browserFetcher.ts

@@ -139,13 +139,12 @@ export async function downloadBrowser(options: DownloadOptions): Promise<void> {
     progress,
   } = options;
   assert(downloadPath, '`downloadPath` must be provided');
+  if (await existsAsync(downloadPath))
+    return;
   const url = revisionURL(options);
   const zipPath = path.join(os.tmpdir(), `playwright-download-${browser}-${platform}-${revision}.zip`);
-  if (await existsAsync(downloadPath))
-    throw new Error('ERROR: downloadPath folder already exists!');
   try {
     await downloadFile(url, zipPath, progress);
-    // await mkdirAsync(downloadPath, {recursive: true});
     await extractZip(zipPath, downloadPath);
   } finally {
     if (await existsAsync(zipPath))

test/installation-tests/.gitignore

@@ -0,0 +1 @@
+output

test/installation-tests/README.md

@@ -0,0 +1,4 @@
+# Installation Tests
+
+File `installation-tests.sh` tests that installation flow for all
+Playwright packages works as expected.

test/installation-tests/installation-tests.sh

@@ -0,0 +1,127 @@
+#!/bin/bash
+set -e
+set +x
+
+trap "cd $(pwd -P)" EXIT
+cd "$(dirname $0)"
+
+# 1. Pack all packages.
+
+rm -rf ./output
+mkdir ./output
+cd ./output
+
+npm pack ../../..
+npm pack ../../../packages/playwright
+npm pack ../../../packages/playwright-chromium
+npm pack ../../../packages/playwright-webkit
+npm pack ../../../packages/playwright-firefox
+
+# There is no option to specify output for `npm pack`, but the format is
+# fixed.
+PACKAGE_VERSION=$(node -e 'console.log(require("../../../package.json").version)')
+PLAYWRIGHT_CORE_TGZ="$(pwd -P)/playwright-core-${PACKAGE_VERSION}.tgz"
+PLAYWRIGHT_TGZ="$(pwd -P)/playwright-${PACKAGE_VERSION}.tgz"
+PLAYWRIGHT_CHROMIUM_TGZ="$(pwd -P)/playwright-chromium-${PACKAGE_VERSION}.tgz"
+PLAYWRIGHT_WEBKIT_TGZ="$(pwd -P)/playwright-webkit-${PACKAGE_VERSION}.tgz"
+PLAYWRIGHT_FIREFOX_TGZ="$(pwd -P)/playwright-firefox-${PACKAGE_VERSION}.tgz"
+
+SANITY_JS="$(pwd -P)/../sanity.js"
+TEST_ROOT="$(pwd -P)"
+
+function run_tests {
+  test_playwright_global_installation_subsequent_installs
+  test_playwright_should_work
+  test_playwright_chromium_should_work
+  test_playwright_webkit_should_work
+  test_playwright_firefox_should_work
+  test_playwright_global_installation
+}
+
+function test_playwright_global_installation {
+  initialize_test "${FUNCNAME[0]}"
+
+  local BROWSERS="$(pwd -P)/browsers"
+  PLAYWRIGHT_BROWSERS_PATH="${BROWSERS}" npm install ${PLAYWRIGHT_CORE_TGZ}
+  PLAYWRIGHT_BROWSERS_PATH="${BROWSERS}" npm install ${PLAYWRIGHT_TGZ}
+  if [[ ! -d "${BROWSERS}" ]]; then
+    echo "Directory for shared browsers was not created!"
+    exit 1
+  fi
+  cp ${SANITY_JS} .
+  if node sanity.js playwright chromium 2>/dev/null; then
+    echo "Should not be able to launch chromium without PLAYWRIGHT_BROWSERS_PATH variable!"
+    exit 1
+  fi
+  PLAYWRIGHT_BROWSERS_PATH="${BROWSERS}" node sanity.js playwright chromium
+}
+
+
+# @see https://github.com/microsoft/playwright/issues/1651
+function test_playwright_global_installation_subsequent_installs {
+  initialize_test "${FUNCNAME[0]}"
+
+  local BROWSERS="$(pwd -P)/browsers"
+
+  mkdir install-1 && pushd install-1 && npm init -y
+  PLAYWRIGHT_BROWSERS_PATH="${BROWSERS}" npm install ${PLAYWRIGHT_CORE_TGZ}
+  PLAYWRIGHT_BROWSERS_PATH="${BROWSERS}" npm install ${PLAYWRIGHT_TGZ}
+  # Note: the `npm install` would not actually crash, the error
+  # is merely logged to the console. To reproduce the error, we should make
+  # sure that script's install.js can be run subsequently without unhandled promise rejections.
+  # Note: the flag `--unahdnled-rejections=strict` will force node to terminate in case
+  # of UnhandledPromiseRejection.
+  PLAYWRIGHT_BROWSERS_PATH="${BROWSERS}" node --unhandled-rejections=strict node_modules/playwright/install.js
+}
+
+function test_playwright_should_work {
+  initialize_test "${FUNCNAME[0]}"
+
+  npm install ${PLAYWRIGHT_CORE_TGZ}
+  npm install ${PLAYWRIGHT_TGZ}
+  cp ${SANITY_JS} . && node sanity.js playwright chromium firefox webkit
+}
+
+function test_playwright_chromium_should_work {
+  initialize_test "${FUNCNAME[0]}"
+
+  npm install ${PLAYWRIGHT_CORE_TGZ}
+  npm install ${PLAYWRIGHT_CHROMIUM_TGZ}
+  cp ${SANITY_JS} . && node sanity.js playwright-chromium chromium
+}
+
+function test_playwright_webkit_should_work {
+  initialize_test "${FUNCNAME[0]}"
+
+  npm install ${PLAYWRIGHT_CORE_TGZ}
+  npm install ${PLAYWRIGHT_WEBKIT_TGZ}
+  cp ${SANITY_JS} . && node sanity.js playwright-webkit webkit
+}
+
+function test_playwright_firefox_should_work {
+  initialize_test "${FUNCNAME[0]}"
+
+  npm install ${PLAYWRIGHT_CORE_TGZ}
+  npm install ${PLAYWRIGHT_FIREFOX_TGZ}
+  cp ${SANITY_JS} . && node sanity.js playwright-firefox firefox
+}
+
+function initialize_test {
+  cd ${TEST_ROOT}
+  local TEST_NAME="./$1"
+  mkdir ${TEST_NAME} && cd ${TEST_NAME} && npm init -y
+  echo "====================================================================================="
+  echo "====================================================================================="
+  echo
+  echo "  RUNNING TEST:  ${TEST_NAME}"
+  echo
+  echo "====================================================================================="
+  echo "====================================================================================="
+}
+
+# Run all tests
+# Script will terminate if there's some error somewhere.
+run_tests
+
+echo
+echo "SUCCESS!"

test/e2e/stub.js → test/installation-tests/sanity.js

@@ -1,11 +1,7 @@
-const playwright = require(process.cwd());
+const requireName = process.argv[2];
+const browsers = process.argv.slice(3);
 
-if (process.argv.length === 2) {
-  console.error("Usage stub.js <browser-types-space-separated>");
-  process.exit(1);
-}
-
-const browsers = process.argv.slice(2, process.argv.length);
+const playwright = require(requireName);
 
 (async () => {
   for (const browserType of browsers) {