From df4b5d6d046c51a7bcbb46f10b5c0d4f8ee43174 Mon Sep 17 00:00:00 2001
From: Benjamin Pasero <benjamin.pasero@microsoft.com>
Date: Fri, 26 Nov 2021 18:08:23 +0100
Subject: [PATCH] smoke - strengthen shutdown path

---
 test/automation/src/code.ts             | 11 ++++-------
 test/automation/src/playwrightDriver.ts | 16 +++++++++++-----
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/test/automation/src/code.ts b/test/automation/src/code.ts
index a5ab213ff84..a48ec0fee13 100644
--- a/test/automation/src/code.ts
+++ b/test/automation/src/code.ts
@@ -63,7 +63,7 @@ function getBuildOutPath(root: string): string {
 	}
 }
 
-async function connect(connectDriver: typeof connectElectronDriver, child: cp.ChildProcess | undefined, outPath: string, handlePath: string, logger: Logger): Promise<Code> {
+async function connect(connectDriver: typeof connectElectronDriver | typeof connectPlaywrightDriver, child: cp.ChildProcess | undefined, outPath: string, handlePath: string, logger: Logger): Promise<Code> {
 	let errCount = 0;
 
 	while (true) {
@@ -79,7 +79,7 @@ async function connect(connectDriver: typeof connectElectronDriver, child: cp.Ch
 			}
 
 			// retry
-			await new Promise(c => setTimeout(c, 100));
+			await new Promise(resolve => setTimeout(resolve, 100));
 		}
 	}
 }
@@ -116,14 +116,12 @@ export async function spawn(options: SpawnOptions): Promise<Code> {
 	const handle = await createDriverHandle();
 
 	let child: cp.ChildProcess | undefined;
-	let connectDriver: typeof connectElectronDriver;
 
 	copyExtension(options.extensionsPath, 'vscode-notebook-tests');
 
 	if (options.web) {
 		await launch(options.userDataDir, options.workspacePath, options.codePath, options.extensionsPath, Boolean(options.verbose));
-		connectDriver = connectPlaywrightDriver.bind(connectPlaywrightDriver, options);
-		return connect(connectDriver, child, '', handle, options.logger);
+		return connect(connectPlaywrightDriver.bind(connectPlaywrightDriver, options), child, '', handle, options.logger);
 	}
 
 	const env = { ...process.env };
@@ -199,8 +197,7 @@ export async function spawn(options: SpawnOptions): Promise<Code> {
 	child = cp.spawn(electronPath, args, spawnOptions);
 	instances.add(child);
 	child.once('exit', () => instances.delete(child!));
-	connectDriver = connectElectronDriver;
-	return connect(connectDriver, child, outPath, handle, options.logger);
+	return connect(connectElectronDriver, child, outPath, handle, options.logger);
 }
 
 async function copyExtension(extensionsPath: string, extId: string): Promise<void> {
diff --git a/test/automation/src/playwrightDriver.ts b/test/automation/src/playwrightDriver.ts
index 9fd2c83b8e7..da6a1aecec5 100644
--- a/test/automation/src/playwrightDriver.ts
+++ b/test/automation/src/playwrightDriver.ts
@@ -57,9 +57,15 @@ class PlaywrightDriver implements IDriver {
 		try {
 			await this._context.tracing.stop({ path: join(logsPath, `playwright-trace-${traceCounter++}.zip`) });
 		} catch (error) {
-			console.warn(`Failed to stop playwright tracing.`); // do not fail the build when this fails
+			console.warn(`Failed to stop playwright tracing: ${error}`);
 		}
-		await this._browser.close();
+
+		try {
+			await this._browser.close();
+		} catch (error) {
+			console.warn(`Failed to close browser: ${error}`);
+		}
+
 		await teardown();
 
 		return false;
@@ -207,9 +213,9 @@ export async function launch(userDataDir: string, _workspacePath: string, codeSe
 async function teardown(): Promise<void> {
 	if (server) {
 		try {
-			await new Promise<void>((c, e) => kill(server!.pid, err => err ? e(err) : c()));
-		} catch {
-			// noop
+			await new Promise<void>((resolve, reject) => kill(server!.pid, err => err ? reject(err) : resolve()));
+		} catch (error) {
+			console.warn(`Error tearing down server: ${error}`);
 		}
 
 		server = undefined;