🆙 1.21.0 (#147348)

* 🆙 `playwright`

* fix install

* adopt latest apis

* comment

test/automation/src/code.ts

@@ -13,6 +13,7 @@ import { launch as launchPlaywrightElectron } from './playwrightElectron';
 import { Logger, measureAndLog } from './logger';
 import { copyExtension } from './extensions';
 import * as treekill from 'tree-kill';
+import { teardown } from './processes';
 
 const rootPath = join(__dirname, '../../..');
 
@@ -39,8 +40,8 @@ interface ICodeInstance {
 
 const instances = new Set<ICodeInstance>();
 
-function registerInstance(process: cp.ChildProcess, logger: Logger, type: string, kill: () => Promise<void>) {
-	const instance = { kill };
+function registerInstance(process: cp.ChildProcess, logger: Logger, type: string) {
+	const instance = { kill: () => teardown(process, logger) };
 	instances.add(instance);
 
 	process.stdout?.on('data', data => logger.log(`[${type}] stdout: ${data}`));
@@ -53,7 +54,7 @@ function registerInstance(process: cp.ChildProcess, logger: Logger, type: string
 	});
 }
 
-async function teardown(signal?: number) {
+async function teardownAll(signal?: number) {
 	stopped = true;
 
 	for (const instance of instances) {
@@ -66,9 +67,9 @@ async function teardown(signal?: number) {
 }
 
 let stopped = false;
-process.on('exit', () => teardown());
-process.on('SIGINT', () => teardown(128 + 2)); 	 // https://nodejs.org/docs/v14.16.0/api/process.html#process_signal_events
-process.on('SIGTERM', () => teardown(128 + 15)); // same as above
+process.on('exit', () => teardownAll());
+process.on('SIGINT', () => teardownAll(128 + 2)); 	 // https://nodejs.org/docs/v14.16.0/api/process.html#process_signal_events
+process.on('SIGTERM', () => teardownAll(128 + 15)); // same as above
 
 export async function launch(options: LaunchOptions): Promise<Code> {
 	if (stopped) {
@@ -79,25 +80,26 @@ export async function launch(options: LaunchOptions): Promise<Code> {
 
 	// Browser smoke tests
 	if (options.web) {
-		const { serverProcess, client, driver, kill } = await measureAndLog(launchPlaywrightBrowser(options), 'launch playwright (browser)', options.logger);
-		registerInstance(serverProcess, options.logger, 'server', kill);
+		const { serverProcess, client, driver } = await measureAndLog(launchPlaywrightBrowser(options), 'launch playwright (browser)', options.logger);
+		registerInstance(serverProcess, options.logger, 'server');
 
-		return new Code(client, driver, options.logger);
+		return new Code(client, driver, options.logger, serverProcess);
 	}
 
 	// Electron smoke tests (playwright)
 	else if (!options.legacy) {
-		const { client, driver } = await measureAndLog(launchPlaywrightElectron(options), 'launch playwright (electron)', options.logger);
+		const { electronProcess, client, driver } = await measureAndLog(launchPlaywrightElectron(options), 'launch playwright (electron)', options.logger);
+		registerInstance(electronProcess, options.logger, 'electron');
 
-		return new Code(client, driver, options.logger);
+		return new Code(client, driver, options.logger, electronProcess);
 	}
 
 	// Electron smoke tests (legacy driver)
 	else {
-		const { electronProcess, client, driver, kill } = await measureAndLog(launchElectron(options), 'launch electron', options.logger);
-		registerInstance(electronProcess, options.logger, 'electron', kill);
+		const { electronProcess, client, driver } = await measureAndLog(launchElectron(options), 'launch electron', options.logger);
+		registerInstance(electronProcess, options.logger, 'electron');
 
-		return new Code(client, driver, options.logger);
+		return new Code(client, driver, options.logger, electronProcess);
 	}
 }
 
@@ -109,7 +111,8 @@ export class Code {
 	constructor(
 		private client: IDisposable,
 		driver: IDriver,
-		readonly logger: Logger
+		readonly logger: Logger,
+		private readonly mainProcess: cp.ChildProcess
 	) {
 		this.driver = new Proxy(driver, {
 			get(target, prop) {
@@ -150,13 +153,15 @@ export class Code {
 	}
 
 	async exit(): Promise<void> {
-
-		// Start the exit flow via driver
-		const pid = await measureAndLog(this.driver.exitApplication(), 'driver.exitApplication()', this.logger);
-
 		return measureAndLog(new Promise<void>((resolve, reject) => {
+			const pid = this.mainProcess.pid!;
+
 			let done = false;
 
+			// Start the exit flow via driver
+			this.driver.exitApplication();
+
+			// Await the exit of the application
 			(async () => {
 				let retries = 0;
 				while (!done) {

test/automation/src/electron.ts

@@ -10,10 +10,10 @@ import { connect as connectElectronDriver, IDisposable, IDriver } from './driver
 import { ChildProcess, spawn, SpawnOptions } from 'child_process';
 import * as mkdirp from 'mkdirp';
 import { promisify } from 'util';
-import * as kill from 'tree-kill';
+import { teardown } from './processes';
 import { copyExtension } from './extensions';
 import { URI } from 'vscode-uri';
-import { Logger, measureAndLog } from './logger';
+import { measureAndLog } from './logger';
 import type { LaunchOptions } from './code';
 
 const root = join(__dirname, '..', '..', '..');
@@ -99,7 +99,7 @@ export async function resolveElectronConfiguration(options: LaunchOptions): Prom
 /**
  * @deprecated should use the playwright based electron support instead
  */
-export async function launch(options: LaunchOptions): Promise<{ electronProcess: ChildProcess; client: IDisposable; driver: IDriver; kill: () => Promise<void> }> {
+export async function launch(options: LaunchOptions): Promise<{ electronProcess: ChildProcess; client: IDisposable; driver: IDriver }> {
 	const { codePath, logger, verbose } = options;
 	const { env, args, electronPath } = await resolveElectronConfiguration(options);
 
@@ -126,8 +126,7 @@ export async function launch(options: LaunchOptions): Promise<{ electronProcess:
 			return {
 				electronProcess,
 				client,
-				driver,
-				kill: () => teardown(electronProcess, options.logger)
+				driver
 			};
 		} catch (err) {
 
@@ -152,31 +151,6 @@ export async function launch(options: LaunchOptions): Promise<{ electronProcess:
 	}
 }
 
-async function teardown(electronProcess: ChildProcess, logger: Logger): Promise<void> {
-	const electronPid = electronProcess.pid;
-	if (typeof electronPid !== 'number') {
-		return;
-	}
-
-	let retries = 0;
-	while (retries < 3) {
-		retries++;
-
-		try {
-			return await promisify(kill)(electronPid);
-		} catch (error) {
-			try {
-				process.kill(electronPid, 0); // throws an exception if the process doesn't exist anymore
-				logger.log(`Error tearing down electron client (pid: ${electronPid}, attempt: ${retries}): ${error}`);
-			} catch (error) {
-				return; // Expected when process is gone
-			}
-		}
-	}
-
-	logger.log(`Gave up tearing down electron client after ${retries} attempts...`);
-}
-
 export function getDevElectronPath(): string {
 	const buildPath = join(root, '.build');
 	const product = require(join(root, 'product.json'));

test/automation/src/playwrightBrowser.ts

@@ -10,7 +10,6 @@ import { mkdir } from 'fs';
 import { promisify } from 'util';
 import { IDriver, IDisposable } from './driver';
 import { URI } from 'vscode-uri';
-import * as kill from 'tree-kill';
 import { Logger, measureAndLog } from './logger';
 import type { LaunchOptions } from './code';
 import { PlaywrightDriver } from './playwrightDriver';
@@ -19,7 +18,7 @@ const root = join(__dirname, '..', '..', '..');
 
 let port = 9000;
 
-export async function launch(options: LaunchOptions): Promise<{ serverProcess: ChildProcess; client: IDisposable; driver: IDriver; kill: () => Promise<void> }> {
+export async function launch(options: LaunchOptions): Promise<{ serverProcess: ChildProcess; client: IDisposable; driver: IDriver }> {
 
 	// Launch server
 	const { serverProcess, endpoint } = await launchServer(options);
@@ -32,8 +31,7 @@ export async function launch(options: LaunchOptions): Promise<{ serverProcess: C
 		client: {
 			dispose: () => { /* there is no client to dispose for browser, teardown is triggered via exitApplication call */ }
 		},
-		driver: new PlaywrightDriver(browser, context, page, serverProcess.pid, options),
-		kill: () => teardown(serverProcess.pid, options.logger)
+		driver: new PlaywrightDriver(browser, context, page, serverProcess, options)
 	};
 }
 
@@ -145,30 +143,6 @@ async function launchBrowser(options: LaunchOptions, endpoint: string) {
 	return { browser, context, page };
 }
 
-export async function teardown(serverPid: number | undefined, logger: Logger): Promise<void> {
-	if (typeof serverPid !== 'number') {
-		return;
-	}
-
-	let retries = 0;
-	while (retries < 3) {
-		retries++;
-
-		try {
-			return await promisify(kill)(serverPid);
-		} catch (error) {
-			try {
-				process.kill(serverPid, 0); // throws an exception if the process doesn't exist anymore
-				logger.log(`Error tearing down server (pid: ${serverPid}, attempt: ${retries}): ${error}`);
-			} catch (error) {
-				return; // Expected when process is gone
-			}
-		}
-	}
-
-	logger.log(`Gave up tearing down server after ${retries} attempts...`);
-}
-
 function waitForEndpoint(server: ChildProcess, logger: Logger): Promise<string> {
 	return new Promise<string>((resolve, reject) => {
 		let endpointFound = false;

test/automation/src/playwrightDriver.ts

@@ -9,7 +9,8 @@ import { IDriver, IWindowDriver } from './driver';
 import { PageFunction } from 'playwright-core/types/structs';
 import { measureAndLog } from './logger';
 import { LaunchOptions } from './code';
-import { teardown } from './playwrightBrowser';
+import { teardown } from './processes';
+import { ChildProcess } from 'child_process';
 
 export class PlaywrightDriver implements IDriver {
 
@@ -36,7 +37,7 @@ export class PlaywrightDriver implements IDriver {
 		private readonly application: playwright.Browser | playwright.ElectronApplication,
 		private readonly context: playwright.BrowserContext,
 		private readonly page: playwright.Page,
-		private readonly serverPid: number | undefined,
+		private readonly serverProcess: ChildProcess | undefined,
 		private readonly options: LaunchOptions
 	) {
 	}
@@ -107,32 +108,28 @@ export class PlaywrightDriver implements IDriver {
 			// Ignore
 		}
 
-		// VSCode shutdown (desktop only)
-		let mainPid: number | undefined = undefined;
-		if (!this.options.web) {
+		// Web: exit via `close` method
+		if (this.options.web) {
 			try {
-				mainPid = await measureAndLog(this._evaluateWithDriver(([driver]) => (driver as unknown as IDriver).exitApplication()), 'driver.exitApplication()', this.options.logger);
+				await measureAndLog(this.application.close(), 'playwright.close()', this.options.logger);
+			} catch (error) {
+				this.options.logger.log(`Error closing appliction (${error})`);
+			}
+		}
+
+		// Desktop: exit via `driver.exitApplication`
+		else {
+			try {
+				await measureAndLog(this._evaluateWithDriver(([driver]) => driver.exitApplication()), 'driver.exitApplication()', this.options.logger);
 			} catch (error) {
 				this.options.logger.log(`Error exiting appliction (${error})`);
 			}
 		}
 
-		// Playwright shutdown
-		try {
-			await Promise.race([
-				measureAndLog(this.application.close(), 'playwright.close()', this.options.logger),
-				new Promise<void>(resolve => setTimeout(() => resolve(), 10000)) // TODO@bpasero mitigate https://github.com/microsoft/vscode/issues/146803
-			]);
-		} catch (error) {
-			this.options.logger.log(`Error closing appliction (${error})`);
+		// Server: via `teardown`
+		if (this.serverProcess) {
+			await measureAndLog(teardown(this.serverProcess, this.options.logger), 'teardown server process', this.options.logger);
 		}
-
-		// Server shutdown
-		if (typeof this.serverPid === 'number') {
-			await measureAndLog(teardown(this.serverPid, this.options.logger), 'teardown server', this.options.logger);
-		}
-
-		return mainPid ?? this.serverPid! /* when running web we must have a server Pid */;
 	}
 
 	async dispatchKeybinding(windowId: number, keybinding: string) {

test/automation/src/playwrightElectron.ts

@@ -9,8 +9,9 @@ import type { LaunchOptions } from './code';
 import { PlaywrightDriver } from './playwrightDriver';
 import { IElectronConfiguration, resolveElectronConfiguration } from './electron';
 import { measureAndLog } from './logger';
+import { ChildProcess } from 'child_process';
 
-export async function launch(options: LaunchOptions): Promise<{ client: IDisposable; driver: IDriver }> {
+export async function launch(options: LaunchOptions): Promise<{ electronProcess: ChildProcess; client: IDisposable; driver: IDriver }> {
 
 	// Resolve electron config and update
 	const { electronPath, args, env } = await resolveElectronConfiguration(options);
@@ -18,12 +19,14 @@ export async function launch(options: LaunchOptions): Promise<{ client: IDisposa
 
 	// Launch electron via playwright
 	const { electron, context, page } = await launchElectron({ electronPath, args, env }, options);
+	const electronProcess = electron.process();
 
 	return {
+		electronProcess,
 		client: {
 			dispose: () => { /* there is no client to dispose for electron, teardown is triggered via exitApplication call */ }
 		},
-		driver: new PlaywrightDriver(electron, context, page, undefined /* no server */, options)
+		driver: new PlaywrightDriver(electron, context, page, undefined /* no server process */, options)
 	};
 }
 

test/automation/src/processes.ts

@@ -0,0 +1,34 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import { ChildProcess } from 'child_process';
+import { promisify } from 'util';
+import * as treekill from 'tree-kill';
+import { Logger } from './logger';
+
+export async function teardown(p: ChildProcess, logger: Logger, retryCount = 3): Promise<void> {
+	const pid = p.pid;
+	if (typeof pid !== 'number') {
+		return;
+	}
+
+	let retries = 0;
+	while (retries < retryCount) {
+		retries++;
+
+		try {
+			return await promisify(treekill)(pid);
+		} catch (error) {
+			try {
+				process.kill(pid, 0); // throws an exception if the process doesn't exist anymore
+				logger.log(`Error tearing down process (pid: ${pid}, attempt: ${retries}): ${error}`);
+			} catch (error) {
+				return; // Expected when process is gone
+			}
+		}
+	}
+
+	logger.log(`Gave up tearing down process client after ${retries} attempts...`);
+}