From ec3ba7a32040bb047a9e7ea70325416f5f1db511 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Thu, 9 Dec 2021 07:55:25 +0100 Subject: [PATCH] smoke - rewrite killing --- test/automation/src/code.ts | 46 +++++++++++++++++++++---- test/automation/src/electronDriver.ts | 38 ++++++++++---------- test/automation/src/playwrightDriver.ts | 33 +++++++----------- test/integration/browser/src/index.ts | 10 ++++-- 4 files changed, 79 insertions(+), 48 deletions(-) diff --git a/test/automation/src/code.ts b/test/automation/src/code.ts index c2c85b2a263..1e77d290685 100644 --- a/test/automation/src/code.ts +++ b/test/automation/src/code.ts @@ -28,10 +28,38 @@ export interface LaunchOptions { browser?: 'chromium' | 'webkit' | 'firefox'; } +interface ICodeInstance { + kill: () => Promise +} + +const instances = new Set(); + +function registerInstance(process: cp.ChildProcess, logger: Logger, type: string, kill: () => Promise) { + const instance = { kill }; + instances.add(instance); + process.once('exit', (code, signal) => { + logger.log(`Process terminated (type: ${type}, pid: ${process.pid}, code: ${code}, signal: ${signal})`); + + instances.delete(instance); + }); +} + +async function teardown(signal?: number) { + stopped = true; + + for (const instance of instances) { + await instance.kill(); + } + + if (typeof signal === 'number') { + process.exit(signal); + } +} + let stopped = false; -process.on('exit', () => stopped = true); -process.on('SIGINT', () => stopped = true); -process.on('SIGTERM', () => stopped = true); +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 export async function launch(options: LaunchOptions): Promise { if (stopped) { @@ -42,13 +70,19 @@ export async function launch(options: LaunchOptions): Promise { // Browser smoke tests if (options.web) { - const { serverProcess, client, driver } = await launchPlaywright(options); + const { serverProcess, client, driver, kill } = await measureAndLog(launchPlaywright(options), 'launch playwright', options.logger); + registerInstance(serverProcess, options.logger, 'server', kill); + return new Code(client, driver, options.logger, serverProcess); } // Electron smoke tests - const { electronProcess, client, driver } = await launchElectron(options); - return new Code(client, driver, options.logger, electronProcess); + else { + const { electronProcess, client, driver, kill } = await measureAndLog(launchElectron(options), 'launch electron', options.logger); + registerInstance(electronProcess, options.logger, 'electron', kill); + + return new Code(client, driver, options.logger, electronProcess); + } } async function poll( diff --git a/test/automation/src/electronDriver.ts b/test/automation/src/electronDriver.ts index fdbac5d6f53..8bf0b64beb5 100644 --- a/test/automation/src/electronDriver.ts +++ b/test/automation/src/electronDriver.ts @@ -18,7 +18,7 @@ import type { LaunchOptions } from './code'; const repoPath = path.join(__dirname, '../../..'); -export async function launch(options: LaunchOptions): Promise<{ electronProcess: ChildProcess, client: IDisposable, driver: IDriver }> { +export async function launch(options: LaunchOptions): Promise<{ electronProcess: ChildProcess, client: IDisposable, driver: IDriver, kill: () => Promise }> { const { codePath, workspacePath, extensionsPath, userDataDir, remote, logger, verbose, extraArgs } = options; const env = { ...process.env }; const logsPath = path.join(repoPath, '.build', 'logs', remote ? 'smoke-tests-remote' : 'smoke-tests'); @@ -89,30 +89,30 @@ export async function launch(options: LaunchOptions): Promise<{ electronProcess: const electronPath = codePath ? getBuildElectronPath(codePath) : getDevElectronPath(); const electronProcess = spawn(electronPath, args, spawnOptions); - if (verbose) { - logger.log(`Started electron for desktop smoke tests on pid ${electronProcess.pid}`); - } - - let electronProcessDidExit = false; - electronProcess.once('exit', (code, signal) => { - if (verbose) { - logger.log(`Electron for desktop smoke tests terminated (pid: ${electronProcess.pid}, code: ${code}, signal: ${signal})`); - } - electronProcessDidExit = true; - }); - - process.once('exit', () => { - if (!electronProcessDidExit) { - electronProcess.kill(); - } - }); + logger.log(`Started electron for desktop smoke tests on pid ${electronProcess.pid}`); let retries = 0; while (true) { try { const { client, driver } = await measureAndLog(connectElectronDriver(outPath, driverIPCHandle), 'connectElectronDriver()', logger); - return { electronProcess, client, driver }; + return { + electronProcess, + client, + driver, + kill: async () => { + try { + return promisify(kill)(electronProcess.pid!); + } catch (error) { + try { + process.kill(electronProcess.pid!, 0); // throws an exception if the process doesn't exist anymore + logger.log(`Error tearing down electron client (pid: ${electronProcess.pid}): ${error}`); + } catch (error) { + return; // Expected when process is gone + } + } + } + }; } catch (err) { // give up diff --git a/test/automation/src/playwrightDriver.ts b/test/automation/src/playwrightDriver.ts index c137bb22486..ae1770948f5 100644 --- a/test/automation/src/playwrightDriver.ts +++ b/test/automation/src/playwrightDriver.ts @@ -199,7 +199,7 @@ class PlaywrightDriver implements IDriver { let port = 9000; -export async function launch(options: LaunchOptions): Promise<{ serverProcess: ChildProcess, client: IDisposable, driver: IDriver }> { +export async function launch(options: LaunchOptions): Promise<{ serverProcess: ChildProcess, client: IDisposable, driver: IDriver, kill: () => Promise }> { // Launch server const { serverProcess, endpoint } = await launchServer(options); @@ -212,12 +212,13 @@ 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(serverProcess, browser, context, page, options.logger) + driver: new PlaywrightDriver(serverProcess, browser, context, page, options.logger), + kill: () => teardown(serverProcess, options.logger) }; } async function launchServer(options: LaunchOptions) { - const { userDataDir, codePath, extensionsPath, verbose, logger } = options; + const { userDataDir, codePath, extensionsPath, logger } = options; const codeServerPath = codePath ?? process.env.VSCODE_REMOTE_SERVER_PATH; const agentFolder = userDataDir; await measureAndLog(promisify(mkdir)(agentFolder), `mkdir(${agentFolder})`, logger); @@ -234,18 +235,14 @@ async function launchServer(options: LaunchOptions) { serverLocation = join(codeServerPath, `server.${process.platform === 'win32' ? 'cmd' : 'sh'}`); args.push(`--logsPath=${logsPath}`); - if (verbose) { - logger.log(`Starting built server from '${serverLocation}'`); - logger.log(`Storing log files into '${logsPath}'`); - } + logger.log(`Starting built server from '${serverLocation}'`); + logger.log(`Storing log files into '${logsPath}'`); } else { serverLocation = join(root, `resources/server/web.${process.platform === 'win32' ? 'bat' : 'sh'}`); args.push('--logsPath', logsPath); - if (verbose) { - logger.log(`Starting server out of sources from '${serverLocation}'`); - logger.log(`Storing log files into '${logsPath}'`); - } + logger.log(`Starting server out of sources from '${serverLocation}'`); + logger.log(`Storing log files into '${logsPath}'`); } const serverProcess = spawn( @@ -254,17 +251,11 @@ async function launchServer(options: LaunchOptions) { { env } ); - if (verbose) { - logger.log(`*** Started server for browser smoke tests (pid: ${serverProcess.pid})`); - serverProcess.once('exit', (code, signal) => logger.log(`Server for browser smoke tests terminated (pid: ${serverProcess.pid}, code: ${code}, signal: ${signal})`)); + logger.log(`Started server for browser smoke tests (pid: ${serverProcess.pid})`); + serverProcess.once('exit', (code, signal) => logger.log(`Server for browser smoke tests terminated (pid: ${serverProcess.pid}, code: ${code}, signal: ${signal})`)); - serverProcess.stderr?.on('data', error => logger.log(`Server stderr: ${error}`)); - serverProcess.stdout?.on('data', data => logger.log(`Server stdout: ${data}`)); - } - - process.on('exit', () => teardown(serverProcess, logger)); - process.on('SIGINT', () => teardown(serverProcess, logger)); - process.on('SIGTERM', () => teardown(serverProcess, logger)); + serverProcess.stderr?.on('data', error => logger.log(`Server stderr: ${error}`)); + serverProcess.stdout?.on('data', data => logger.log(`Server stdout: ${data}`)); return { serverProcess, diff --git a/test/integration/browser/src/index.ts b/test/integration/browser/src/index.ts index 262faad5f06..df9f5dc8191 100644 --- a/test/integration/browser/src/index.ts +++ b/test/integration/browser/src/index.ts @@ -162,8 +162,14 @@ async function launchServer(browserType: BrowserType): Promise<{ endpoint: url.U } process.on('exit', () => serverProcess.kill()); - process.on('SIGINT', () => serverProcess.kill()); - process.on('SIGTERM', () => serverProcess.kill()); + process.on('SIGINT', () => { + serverProcess.kill(); + process.exit(128 + 2); // https://nodejs.org/docs/v14.16.0/api/process.html#process_signal_events + }); + process.on('SIGTERM', () => { + serverProcess.kill(); + process.exit(128 + 15); // https://nodejs.org/docs/v14.16.0/api/process.html#process_signal_events + }); return new Promise(c => { serverProcess.stdout!.on('data', data => {