yujunjun/playwright
1
0
Fork 0
mirror of https://github.com/microsoft/playwright.git synced 2025-06-26 21:40:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

test: unflake fixtures test (#2313)

Browse Source 
Drive-by: ensure we call onkill before manually exiting the process.
This commit is contained in:
Dmitry Gozman 2020-05-20 14:58:27 -07:00 committed by GitHub
parent 9808d8bc03
commit 48440f7ed7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 170 additions and 115 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/server/chromium.ts
View File

@ -142,6 +142,8 @@ export class Chromium extends AbstractBrowserType<CRBrowser> {
pipe: true,
tempDir: temporaryUserDataDir || undefined,
attemptToGracefullyClose: async () => {
if ((options as any).__testHookGracefullyClose)
await (options as any).__testHookGracefullyClose();
debugAssert(browserServer._isInitialized());
// We try to gracefully close to prevent crash reporting and core dumps.
// Note that it's fine to reuse the pipe transport, since

2
src/server/firefox.ts
View File

@ -137,6 +137,8 @@ export class Firefox extends AbstractBrowserType<FFBrowser> {
pipe: false,
tempDir: temporaryProfileDir || undefined,
attemptToGracefullyClose: async () => {
if ((options as any).__testHookGracefullyClose)
await (options as any).__testHookGracefullyClose();
debugAssert(browserServer._isInitialized());
// We try to gracefully close to prevent crash reporting and core dumps.
const transport = await WebSocketTransport.connect(browserWSEndpoint!, async transport => transport);

34
src/server/processLauncher.ts
View File

@ -111,18 +111,21 @@ export async function launchProcess(options: LaunchProcessOptions): Promise<Laun
const downloadsPath = options.omitDownloads ? '' : await mkdtempAsync(DOWNLOADS_FOLDER);
let processClosed = false;
const waitForProcessToClose = new Promise((fulfill, reject) => {
spawnedProcess.once('exit', (exitCode, signal) => {
logger._log(browserLog, `<process did exit ${exitCode}, ${signal}>`);
processClosed = true;
helper.removeEventListeners(listeners);
options.onkill(exitCode, signal);
// Cleanup as processes exit.
Promise.all([
options.omitDownloads ? Promise.resolve() : removeFolderAsync(downloadsPath),
options.tempDir ? removeFolderAsync(options.tempDir) : Promise.resolve()
]).catch((err: Error) => console.error(err)).then(fulfill);
});
let fulfillClose = () => {};
const waitForClose = new Promise(f => fulfillClose = f);
let fulfillCleanup = () => {};
const waitForCleanup = new Promise(f => fulfillCleanup = f);
spawnedProcess.once('exit', (exitCode, signal) => {
logger._log(browserLog, `<process did exit ${exitCode}, ${signal}>`);
processClosed = true;
helper.removeEventListeners(listeners);
options.onkill(exitCode, signal);
fulfillClose();
// Cleanup as processes exit.
Promise.all([
options.omitDownloads ? Promise.resolve() : removeFolderAsync(downloadsPath),
options.tempDir ? removeFolderAsync(options.tempDir) : Promise.resolve()
]).catch((err: Error) => console.error(err)).then(fulfillCleanup);
});
const listeners = [ helper.addEventListener(process, 'exit', killProcess) ];
@ -145,12 +148,13 @@ export async function launchProcess(options: LaunchProcessOptions): Promise<Laun
if (gracefullyClosing) {
logger._log(browserLog, `<forecefully close>`);
killProcess();
await waitForClose; // Ensure the process is dead and we called options.onkill.
return;
}
gracefullyClosing = true;
logger._log(browserLog, `<gracefully close start>`);
await options.attemptToGracefullyClose().catch(() => killProcess());
await waitForProcessToClose;
await waitForCleanup; // Ensure the process is dead and we have cleaned up.
logger._log(browserLog, `<gracefully close end>`);
}
@ -169,10 +173,12 @@ export async function launchProcess(options: LaunchProcessOptions): Promise<Laun
// the process might have already stopped
}
}
// Attempt to remove temporary profile directory to avoid littering.
// Attempt to remove temporary directories to avoid littering.
try {
if (options.tempDir)
removeFolder.sync(options.tempDir);
if (!options.omitDownloads)
removeFolder.sync(downloadsPath);
} catch (e) { }
}

2
src/server/webkit.ts
View File

@ -125,6 +125,8 @@ export class WebKit extends AbstractBrowserType<WKBrowser> {
pipe: true,
tempDir: temporaryUserDataDir || undefined,
attemptToGracefullyClose: async () => {
if ((options as any).__testHookGracefullyClose)
await (options as any).__testHookGracefullyClose();
assert(transport);
// We try to gracefully close to prevent crash reporting and core dumps.
// Note that it's fine to reuse the pipe transport, since

226
test/fixtures.spec.js
View File

@ -19,57 +19,90 @@ const path = require('path');
const {spawn, execSync} = require('child_process');
const {FFOX, CHROMIUM, WEBKIT, WIN, LINUX} = require('./utils').testOptions(browserType);
async function testSignal(state, action, exitOnClose) {
const options = Object.assign({}, state.defaultBrowserOptions, {
handleSIGINT: true,
handleSIGTERM: true,
handleSIGHUP: true,
executablePath: state.browserType.executablePath(),
logger: undefined,
});
const res = spawn('node', [path.join(__dirname, 'fixtures', 'closeme.js'), path.join(state.playwrightPath, 'index-for-dev'), state.browserType.name(), JSON.stringify(options), exitOnClose ? 'true' : '']);
let wsEndPointCallback;
const wsEndPointPromise = new Promise(x => wsEndPointCallback = x);
let output = '';
let browserExitCode = 'none';
let browserSignal = 'none';
let browserPid;
res.stdout.on('data', data => {
output += data.toString();
// Uncomment to debug these tests.
// console.log(data.toString());
let match = output.match(/browserWS:(.+):browserWS/);
if (match)
wsEndPointCallback(match[1]);
match = output.match(/browserClose:([^:]+):([^:]+):browserClose/);
if (match) {
browserExitCode = match[1];
browserSignal = match[2];
}
match = output.match(/browserPid:([^:]+):browserPid/);
if (match)
browserPid = +match[1];
});
res.on('error', (...args) => console.log("ERROR", ...args));
const browser = await state.browserType.connect({ wsEndpoint: await wsEndPointPromise });
const promises = [
new Promise(resolve => browser.once('disconnected', resolve)),
new Promise(resolve => res.on('exit', resolve)),
];
action(res, browserPid);
const [, exitCode] = await Promise.all(promises);
return { exitCode, browserSignal, browserExitCode, output };
class Wrapper {
constructor(state, extraOptions) {
this._output = new Map();
this._outputCallback = new Map();
this._browserType = state.browserType;
const launchOptions = {...state.defaultBrowserOptions,
handleSIGINT: true,
handleSIGTERM: true,
handleSIGHUP: true,
executablePath: state.browserType.executablePath(),
logger: undefined,
};
const options = {
playwrightFile: path.join(state.playwrightPath, 'index-for-dev'),
browserTypeName: state.browserType.name(),
launchOptions,
...extraOptions,
};
this._child = spawn('node', [path.join(__dirname, 'fixtures', 'closeme.js'), JSON.stringify(options)]);
this._child.on('error', (...args) => console.log("ERROR", ...args));
this._exitPromise = new Promise(resolve => this._child.on('exit', resolve));
let outputString = '';
this._child.stdout.on('data', data => {
outputString += data.toString();
// Uncomment to debug.
// console.log(data.toString());
let match;
while (match = outputString.match(/\(([^()]+)=>([^()]+)\)/)) {
const key = match[1];
const value = match[2];
this._addOutput(key, value);
outputString = outputString.substring(match.index + match[0].length);
}
});
}
_addOutput(key, value) {
this._output.set(key, value);
const cb = this._outputCallback.get(key);
this._outputCallback.delete(key);
if (cb)
cb();
}
async out(key) {
if (!this._output.has(key))
await new Promise(f => this._outputCallback.set(key, f));
return this._output.get(key);
}
async connect() {
const wsEndpoint = await this.out('wsEndpoint');
const browser = await this._browserType.connect({ wsEndpoint });
this._exitAndDisconnectPromise = Promise.all([
this._exitPromise,
new Promise(resolve => browser.once('disconnected', resolve)),
]).then(([exitCode]) => exitCode);
}
child() {
return this._child;
}
async childExitCode() {
return await this._exitAndDisconnectPromise;
}
}
async function setup(state, options = {}) {
const wrapper = new Wrapper(state, options);
await wrapper.connect();
return wrapper;
}
describe('Fixtures', function() {
it.slow()('should close the browser when the node process closes', async state => {
const result = await testSignal(state, child => {
if (WIN)
execSync(`taskkill /pid ${child.pid} /T /F`);
else
process.kill(child.pid);
});
expect(result.exitCode).toBe(WIN ? 1 : 0);
const wrapper = await setup(state);
if (WIN)
execSync(`taskkill /pid ${wrapper.child().pid} /T /F`);
else
process.kill(wrapper.child().pid);
expect(await wrapper.childExitCode()).toBe(WIN ? 1 : 0);
// We might not get browser exitCode in time when killing the parent node process,
// so we don't check it here.
});
@ -77,65 +110,70 @@ describe('Fixtures', function() {
describe.skip(WIN).skip(!HEADLESS)('signals', () => {
// Cannot reliably send signals on Windows.
it.slow()('should report browser close signal', async state => {
const result = await testSignal(state, (child, browserPid) => process.kill(browserPid), true);
expect(result.exitCode).toBe(0);
expect(result.browserExitCode).toBe('null');
expect(result.browserSignal).toBe('SIGTERM');
const wrapper = await setup(state);
const pid = await wrapper.out('pid');
process.kill(-pid, 'SIGTERM');
expect(await wrapper.out('exitCode')).toBe('null');
expect(await wrapper.out('signal')).toBe('SIGTERM');
process.kill(wrapper.child().pid);
await wrapper.childExitCode();
});
it.slow()('should report browser close signal 2', async state => {
const result = await testSignal(state, (child, browserPid) => process.kill(browserPid, 'SIGKILL'), true);
expect(result.exitCode).toBe(0);
expect(result.browserExitCode).toBe('null');
expect(result.browserSignal).toBe('SIGKILL');
const wrapper = await setup(state);
const pid = await wrapper.out('pid');
process.kill(-pid, 'SIGKILL');
expect(await wrapper.out('exitCode')).toBe('null');
expect(await wrapper.out('signal')).toBe('SIGKILL');
process.kill(wrapper.child().pid);
await wrapper.childExitCode();
});
it.slow()('should close the browser on SIGINT', async state => {
const result = await testSignal(state, child => process.kill(child.pid, 'SIGINT'));
expect(result.exitCode).toBe(130);
expect(result.browserExitCode).toBe('0');
expect(result.browserSignal).toBe('null');
const wrapper = await setup(state);
process.kill(wrapper.child().pid, 'SIGINT');
expect(await wrapper.out('exitCode')).toBe('0');
expect(await wrapper.out('signal')).toBe('null');
expect(await wrapper.childExitCode()).toBe(130);
});
it.slow()('should close the browser on SIGTERM', async state => {
const result = await testSignal(state, child => process.kill(child.pid, 'SIGTERM'));
expect(result.exitCode).toBe(0);
expect(result.browserExitCode).toBe('0');
expect(result.browserSignal).toBe('null');
const wrapper = await setup(state);
process.kill(wrapper.child().pid, 'SIGTERM');
expect(await wrapper.out('exitCode')).toBe('0');
expect(await wrapper.out('signal')).toBe('null');
expect(await wrapper.childExitCode()).toBe(0);
});
it.slow()('should close the browser on SIGHUP', async state => {
const result = await testSignal(state, child => process.kill(child.pid, 'SIGHUP'));
expect(result.exitCode).toBe(0);
expect(result.browserExitCode).toBe('0');
expect(result.browserSignal).toBe('null');
const wrapper = await setup(state);
process.kill(wrapper.child().pid, 'SIGHUP');
expect(await wrapper.out('exitCode')).toBe('0');
expect(await wrapper.out('signal')).toBe('null');
expect(await wrapper.childExitCode()).toBe(0);
});
it.slow()('should kill the browser on double SIGINT', async state => {
const result = await testSignal(state, child => {
process.kill(child.pid, 'SIGINT');
process.kill(child.pid, 'SIGINT');
});
expect(result.exitCode).toBe(130);
// TODO: ideally, we would expect the SIGKILL on the browser from
// force kill, but that's racy with sending two signals.
const wrapper = await setup(state, { stallOnClose: true });
process.kill(wrapper.child().pid, 'SIGINT');
await wrapper.out('stalled');
process.kill(wrapper.child().pid, 'SIGINT');
expect(await wrapper.out('exitCode')).toBe('null');
expect(await wrapper.out('signal')).toBe('SIGKILL');
expect(await wrapper.childExitCode()).toBe(130);
});
// TODO: flaky - https://app.circleci.com/pipelines/github/microsoft/playwright/582/workflows/b49033ce-fe20-4029-b665-13fb331f842e/jobs/579
it.slow().fail(FFOX && LINUX)('should kill the browser on SIGINT + SIGTERM', async state => {
const result = await testSignal(state, child => {
process.kill(child.pid, 'SIGINT');
process.kill(child.pid, 'SIGTERM');
});
expect(result.exitCode).toBe(130);
// TODO: ideally, we would expect the SIGKILL on the browser from
// force kill, but that's racy with sending two signals.
it.slow()('should kill the browser on SIGINT + SIGTERM', async state => {
const wrapper = await setup(state, { stallOnClose: true });
process.kill(wrapper.child().pid, 'SIGINT');
await wrapper.out('stalled');
process.kill(wrapper.child().pid, 'SIGTERM');
expect(await wrapper.out('exitCode')).toBe('null');
expect(await wrapper.out('signal')).toBe('SIGKILL');
expect(await wrapper.childExitCode()).toBe(0);
});
// TODO: flaky!
// - firefox: https://github.com/microsoft/playwright/pull/1911/checks?check_run_id=607148951
// - chromium: https://travis-ci.com/github/microsoft/playwright/builds/161356178
it.slow().fail((FFOX || CHROMIUM) && LINUX)('should kill the browser on SIGTERM + SIGINT', async state => {
const result = await testSignal(state, child => {
process.kill(child.pid, 'SIGTERM');
process.kill(child.pid, 'SIGINT');
});
expect(result.exitCode).toBe(130);
// TODO: ideally, we would expect the SIGKILL on the browser from
// force kill, but that's racy with sending two signals.
it.slow()('should kill the browser on SIGTERM + SIGINT', async state => {
const wrapper = await setup(state, { stallOnClose: true });
process.kill(wrapper.child().pid, 'SIGTERM');
await wrapper.out('stalled');
process.kill(wrapper.child().pid, 'SIGINT');
expect(await wrapper.out('exitCode')).toBe('null');
expect(await wrapper.out('signal')).toBe('SIGKILL');
expect(await wrapper.childExitCode()).toBe(130);
});
});
});

19
test/fixtures/closeme.js vendored
View File

@ -1,11 +1,16 @@
(async() => {
const [, , playwrightIndex, browserType, options, exitOnClose] = process.argv;
const browserServer = await require(playwrightIndex)[browserType].launchServer(JSON.parse(options));
const { playwrightFile, browserTypeName, launchOptions, stallOnClose } = JSON.parse(process.argv[2]);
if (stallOnClose) {
launchOptions.__testHookGracefullyClose = () => {
console.log(`(stalled=>true)`);
return new Promise(() => {});
};
}
const browserServer = await require(playwrightFile)[browserTypeName].launchServer(launchOptions);
browserServer.on('close', (exitCode, signal) => {
console.log(`browserClose:${exitCode}:${signal}:browserClose`);
if (exitOnClose)
process.exit(0);
console.log(`(exitCode=>${exitCode})`);
console.log(`(signal=>${signal})`);
});
console.log(`browserPid:${browserServer.process().pid}:browserPid`);
console.log(`browserWS:${browserServer.wsEndpoint()}:browserWS`);
console.log(`(pid=>${browserServer.process().pid})`);
console.log(`(wsEndpoint=>${browserServer.wsEndpoint()})`);
})();