Skip to content

Commit

Permalink
fix: stop global setup when debugger is terminated (#525)
Browse files Browse the repository at this point in the history 
* fix: stop global setup when debugger is terminated

* don't try stopping ended test

* handle cancellationtoken in _runGlobalHooksInServer

* pass two more tokens

* add test

* listen for abort in wireTestListener

* wait for testEnd earlier
  • Loading branch information
@Skn0tt
Skn0tt authored Sep 10, 2024
1 parent 35b435c commit 4328be9
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 25 deletions.
7 changes: 5 additions & 2 deletions src/extension.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ export class Extension implements RunHooks {
const testRun = this._testController.createTestRun(request);
const testListener = this._errorReportingListener(testRun);
try {
const status = (await this._models.selectedModel()?.runGlobalHooks(type, testListener)) || 'failed';
const status = (await this._models.selectedModel()?.runGlobalHooks(type, testListener, testRun.token)) || 'failed';
return status;
} finally {
testRun.end();
Expand Down Expand Up @@ -533,7 +533,10 @@ export class Extension implements RunHooks {
};

if (mode === 'debug') {
await model.debugTests(items, testListener, testRun.token);
const debugEnd = new this._vscode.CancellationTokenSource();
testRun.token.onCancellationRequested(() => debugEnd.cancel());
this._vscode.debug.onDidTerminateDebugSession(() => debugEnd.cancel());
await model.debugTests(items, testListener, debugEnd.token);
} else {
// Force trace viewer update to surface check version errors.
await this._models.selectedModel()?.updateTraceViewer(mode === 'run')?.willRunTests();
Expand Down
34 changes: 19 additions & 15 deletions src/playwrightTestServer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,14 @@ export class PlaywrightTestServer {
teleReceiver.dispatch(message);
}

async runGlobalHooks(type: 'setup' | 'teardown', testListener: reporterTypes.ReporterV2): Promise<'passed' | 'failed' | 'interrupted' | 'timedout'> {
async runGlobalHooks(type: 'setup' | 'teardown', testListener: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken): Promise<'passed' | 'failed' | 'interrupted' | 'timedout'> {
const testServer = await this._testServer();
if (!testServer)
return 'failed';
return await this._runGlobalHooksInServer(testServer, type, testListener);
return await this._runGlobalHooksInServer(testServer, type, testListener, token);
}

private async _runGlobalHooksInServer(testServer: TestServerConnection, type: 'setup' | 'teardown', testListener: reporterTypes.ReporterV2): Promise<'passed' | 'failed' | 'interrupted' | 'timedout'> {
private async _runGlobalHooksInServer(testServer: TestServerConnection, type: 'setup' | 'teardown', testListener: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken): Promise<'passed' | 'failed' | 'interrupted' | 'timedout'> {
const teleReceiver = new TeleReporterReceiver(testListener, {
mergeProjects: true,
mergeTestCases: true,
Expand All @@ -130,12 +130,18 @@ export class PlaywrightTestServer {
try {
if (type === 'setup') {
testListener.onStdOut?.('\x1b[2mRunning global setup if any\u2026\x1b[0m\n');
const { report, status } = await testServer.runGlobalSetup({});
const { report, status } = await Promise.race([
testServer.runGlobalSetup({}),
new Promise<{ status: 'interrupted', report: [] }>(f => token.onCancellationRequested(() => f({ status: 'interrupted', report: [] }))),
]);
for (const message of report)
teleReceiver.dispatch(message);
return status;
}
const { report, status } = await testServer.runGlobalTeardown({});
const { report, status } = await Promise.race([
testServer.runGlobalTeardown({}),
new Promise<{ status: 'interrupted', report: [] }>(f => token.onCancellationRequested(() => f({ status: 'interrupted', report: [] }))),
]);
for (const message of report)
teleReceiver.dispatch(message);
return status;
Expand Down Expand Up @@ -218,7 +224,7 @@ export class PlaywrightTestServer {
const testDirs = this._model.enabledProjects().map(project => project.project.testDir);

let debugTestServer: TestServerConnection | undefined;
let disposable: vscodeTypes.Disposable | undefined;
const disposables: vscodeTypes.Disposable[] = [];
try {
await this._vscode.debug.startDebugging(undefined, {
type: 'pwa-node',
Expand Down Expand Up @@ -255,11 +261,9 @@ export class PlaywrightTestServer {
if (!locations && !testIds)
return;

const result = await this._runGlobalHooksInServer(debugTestServer, 'setup', reporter);
const result = await this._runGlobalHooksInServer(debugTestServer, 'setup', reporter, token);
if (result !== 'passed')
return;
if (token?.isCancellationRequested)
return;

// Locations are regular expressions.
const locationPatterns = locations ? locations.map(escapeRegex) : undefined;
Expand All @@ -271,21 +275,21 @@ export class PlaywrightTestServer {
};
debugTestServer.runTests(options);

token.onCancellationRequested(() => {
disposables.push(token.onCancellationRequested(() => {
debugTestServer!.stopTestsNoReply({});
});
disposable = debugTestServer.onStdio(params => {
}));
disposables.push(debugTestServer.onStdio(params => {
if (params.type === 'stdout')
reporter.onStdOut?.(unwrapString(params));
if (params.type === 'stderr')
reporter.onStdErr?.(unwrapString(params));
});
}));
const testEndPromise = this._wireTestServer(debugTestServer, reporter, token);
await testEndPromise;
} finally {
disposable?.dispose();
disposables.forEach(disposable => disposable.dispose());
if (!token.isCancellationRequested && debugTestServer && !debugTestServer.isClosed())
await this._runGlobalHooksInServer(debugTestServer, 'teardown', reporter);
await this._runGlobalHooksInServer(debugTestServer, 'teardown', reporter, token);
debugTestServer?.close();
await this._options.runHooks.onDidRunTests(true);
}
Expand Down
17 changes: 14 additions & 3 deletions src/reporterServer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,15 @@ export class ReporterServer {
return wsEndpoint;
}

private _close() {
this._wsServer?.close();
}

async wireTestListener(listener: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken) {
let timeout: NodeJS.Timeout | undefined;
const transport = await this._waitForTransport();
const transport = await this._waitForTransport(token);
if (transport === 'cancellationRequested')
return;

const killTestProcess = () => {
if (!transport.isClosed()) {
Expand Down Expand Up @@ -105,8 +111,13 @@ export class ReporterServer {
clearTimeout(timeout);
}

private async _waitForTransport(): Promise<ConnectionTransport> {
const socket = await this._clientSocketPromise;
private async _waitForTransport(token: vscodeTypes.CancellationToken): Promise<ConnectionTransport | 'cancellationRequested'> {
const socket = await Promise.race([
this._clientSocketPromise,
new Promise<'cancellationRequested'>(f => token.onCancellationRequested(() => { this._close(); f('cancellationRequested'); }))
]);
if (socket === 'cancellationRequested')
return 'cancellationRequested';

const transport: ConnectionTransport = {
send: function(message): void {
Expand Down
10 changes: 5 additions & 5 deletions src/testModel.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -470,21 +470,21 @@ export class TestModel extends DisposableBase {
return false;
}

async runGlobalHooks(type: 'setup' | 'teardown', testListener: reporterTypes.ReporterV2): Promise<reporterTypes.FullResult['status']> {
async runGlobalHooks(type: 'setup' | 'teardown', testListener: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken): Promise<reporterTypes.FullResult['status']> {
if (!this.canRunGlobalHooks(type))
return 'passed';
if (type === 'setup') {
if (!this.canRunGlobalHooks('setup'))
return 'passed';
const status = await this._playwrightTest.runGlobalHooks('setup', testListener);
const status = await this._playwrightTest.runGlobalHooks('setup', testListener, token);
if (status === 'passed')
this._ranGlobalSetup = true;
return status;
}

if (!this._ranGlobalSetup)
return 'passed';
const status = await this._playwrightTest.runGlobalHooks('teardown', testListener);
const status = await this._playwrightTest.runGlobalHooks('teardown', testListener, token);
this._ranGlobalSetup = false;
return status;
}
Expand Down Expand Up @@ -524,7 +524,7 @@ export class TestModel extends DisposableBase {
// Run global setup with the first test.
let globalSetupResult: reporterTypes.FullResult['status'] = 'passed';
if (this.canRunGlobalHooks('setup'))
globalSetupResult = await this.runGlobalHooks('setup', reporter);
globalSetupResult = await this.runGlobalHooks('setup', reporter, token);
if (globalSetupResult !== 'passed')
return;

Expand Down Expand Up @@ -568,7 +568,7 @@ export class TestModel extends DisposableBase {
return;

// Underlying debugTest implementation will run the global setup.
await this.runGlobalHooks('teardown', reporter);
await this.runGlobalHooks('teardown', reporter, token);
if (token?.isCancellationRequested)
return;

Expand Down
42 changes: 42 additions & 0 deletions tests/debug-tests.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,48 @@ test('should end test run when stopping the debugging', async ({ activate }, tes
testRun.token.source.cancel();
});

test('should end test run when stopping the debugging during config parsing', async ({ activate }) => {
const { vscode, testController } = await activate({
'package.json': JSON.stringify({ type: 'module' }),
'playwright.config.js': `
// Simulate breakpoint via stalling.
async function stall() {
console.log('READY TO BREAK');
await new Promise(() => {});
}
process.env.STALL_CONFIG === '1' && await stall();
export default { testDir: 'tests' }
`,
'tests/test.spec.ts': `
import { test } from '@playwright/test';
test('should fail', async () => {});
`,
}, { env: { STALL_CONFIG: '0' } });

await testController.expandTestItems(/test.spec/);
const testItems = testController.findTestItems(/fail/);

const configuration = vscode.workspace.getConfiguration('playwright');
configuration.update('env', { STALL_CONFIG: '1' });

const profile = testController.debugProfile();
const testRunPromise = new Promise<TestRun>(f => testController.onDidCreateTestRun(f));
profile.run(testItems);
const testRun = await testRunPromise;
const endPromise = new Promise(f => testRun.onDidEnd(f));
await expect.poll(() => vscode.debug.output).toContain('READY TO BREAK');
vscode.debug.stopDebugging();
await endPromise;

expect(testRun.renderLog({ messages: true })).toBe(`
tests > test.spec.ts > should fail [2:0]
enqueued
`);

testRun.token.source.cancel();
});

test('should pass all args as string[] when debugging', async ({ activate }) => {
test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/30829' });
const { vscode, testController } = await activate({
Expand Down

0 comments on commit 4328be9

Please sign in to comment.