From 21594d33e8a97b70c4b9446c4c20fa57e31e4f94 Mon Sep 17 00:00:00 2001
From: Paul Gschwendtner <paulgschwendtner@gmail.com>
Date: Thu, 20 Jun 2019 10:21:29 +0200
Subject: [PATCH] build: properly wait for external browser tunnels to
 terminate

We currently wait for browser tunnels of Saucelabs and Browserstack
to terminate by repeatedly checking with `ps -p`. This is not reliable and
can break if the host system is not removing forcibly killed child processes
from the process table. This is valid since the tunnels are launched as child
processes and we ideally wait for those to terminate using the `wait` command.
---
 scripts/browserstack/start-tunnel.sh       |  4 ++++
 scripts/browserstack/stop-tunnel.sh        |  7 +------
 scripts/circleci/run-browserstack-tests.sh | 10 ++++++++--
 scripts/circleci/run-saucelabs-tests.sh    |  5 ++++-
 scripts/saucelabs/start-tunnel.sh          |  2 +-
 scripts/saucelabs/stop-tunnel.sh           |  7 +------
 6 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/scripts/browserstack/start-tunnel.sh b/scripts/browserstack/start-tunnel.sh
index bddbbfec0ba..e31e832c077 100755
--- a/scripts/browserstack/start-tunnel.sh
+++ b/scripts/browserstack/start-tunnel.sh
@@ -68,3 +68,7 @@ browserstack-tunnel/BrowserStackLocal -k ${BROWSER_STACK_ACCESS_KEY} ${ARGS} 2>&
 
 # Wait for the tunnel to be ready and create the readyfile with the Browserstack PID
 create_ready_file ${!} &
+
+# Wait for all sub processes to finish. This ensures that no zombie
+# processes are left over.
+wait
diff --git a/scripts/browserstack/stop-tunnel.sh b/scripts/browserstack/stop-tunnel.sh
index 2f19808b3b4..9fbc0ba435f 100755
--- a/scripts/browserstack/stop-tunnel.sh
+++ b/scripts/browserstack/stop-tunnel.sh
@@ -19,10 +19,5 @@ tunnelProcessId=$(cat ${tunnelReadyFile})
 # we cannot use killall because CircleCI base container images don't have it installed.
 kill ${tunnelProcessId}
 
-while (ps -p ${tunnelProcessId} &> /dev/null); do
-  printf "."
-  sleep .5
-done
-
 echo ""
-echo "Browserstack tunnel has been shut down"
+echo "Browserstack tunnel interrupt signal has been sent."
diff --git a/scripts/circleci/run-browserstack-tests.sh b/scripts/circleci/run-browserstack-tests.sh
index 55f4309ef19..7d117320846 100755
--- a/scripts/circleci/run-browserstack-tests.sh
+++ b/scripts/circleci/run-browserstack-tests.sh
@@ -4,14 +4,17 @@
 # proper exit code.
 set -e
 
+# Path to the project directory.
+projectDir="$(dirname ${0})/../.."
+
 # Go to project directory.
-cd $(dirname ${0})/../..
+cd ${projectDir}
 
 # Decode access token and make it accessible for child processes.
 export BROWSER_STACK_ACCESS_KEY=`echo ${BROWSER_STACK_ACCESS_KEY} | rev`
 
 # Start tunnel and wait for it being ready.
-./scripts/browserstack/start-tunnel.sh
+./scripts/browserstack/start-tunnel.sh &
 ./scripts/browserstack/wait-tunnel.sh
 
 # Setup the test platform environment variable that will be read
@@ -24,3 +27,6 @@ yarn gulp ci:test
 # Kill the Browserstack tunnel. This is necessary in order to avoid rate-limit
 # errors that cause the unit tests to be flaky.
 ./scripts/browserstack/stop-tunnel.sh
+
+# Wait for all sub processes to terminate properly.
+wait
diff --git a/scripts/circleci/run-saucelabs-tests.sh b/scripts/circleci/run-saucelabs-tests.sh
index 659b15494a1..62ca0592324 100755
--- a/scripts/circleci/run-saucelabs-tests.sh
+++ b/scripts/circleci/run-saucelabs-tests.sh
@@ -11,7 +11,7 @@ cd $(dirname ${0})/../..
 export SAUCE_ACCESS_KEY=`echo ${SAUCE_ACCESS_KEY} | rev`
 
 # Start tunnel and wait for it being ready.
-./scripts/saucelabs/start-tunnel.sh
+./scripts/saucelabs/start-tunnel.sh &
 ./scripts/saucelabs/wait-tunnel.sh
 
 # Setup the test platform environment variable that will be read
@@ -24,3 +24,6 @@ yarn gulp ci:test
 # Kill the Saucelabs tunnel. This is necessary in order to avoid rate-limit
 # errors that cause the unit tests to be flaky.
 ./scripts/saucelabs/stop-tunnel.sh
+
+# Wait for all sub processes to terminate properly.
+wait
diff --git a/scripts/saucelabs/start-tunnel.sh b/scripts/saucelabs/start-tunnel.sh
index 18582c38732..1466f683755 100755
--- a/scripts/saucelabs/start-tunnel.sh
+++ b/scripts/saucelabs/start-tunnel.sh
@@ -35,4 +35,4 @@ fi
 
 echo "Starting Sauce Connect in the background. Passed arguments: ${sauceArgs}"
 
-sauce-connect/bin/sc -u ${SAUCE_USERNAME} -k ${SAUCE_ACCESS_KEY} ${sauceArgs} &
+sauce-connect/bin/sc -u ${SAUCE_USERNAME} -k ${SAUCE_ACCESS_KEY} ${sauceArgs}
diff --git a/scripts/saucelabs/stop-tunnel.sh b/scripts/saucelabs/stop-tunnel.sh
index 8123fa04b5d..9b28e783474 100755
--- a/scripts/saucelabs/stop-tunnel.sh
+++ b/scripts/saucelabs/stop-tunnel.sh
@@ -19,10 +19,5 @@ tunnelProcessId=$(cat ${tunnelPidFile})
 # we cannot use killall because CircleCI base container images don't have it installed.
 kill ${tunnelProcessId}
 
-while (ps -p ${tunnelProcessId} &> /dev/null); do
-  printf "."
-  sleep .5
-done
-
 echo ""
-echo "Sauce Connect tunnel has been shut down"
+echo "Sauce Connect tunnel interrupt signal has been sent."