Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: stop global setup when debugger is terminated attempt #2 #537

Merged
merged 6 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,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
17 changes: 17 additions & 0 deletions src/playwrightTestCLI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,23 @@ export class PlaywrightTestCLI {
const reporterServer = new ReporterServer(this._vscode);
const testOptions = await this._options.runHooks.onWillRunTests(this._model.config, true);
try {
const debugEnd = new this._vscode.CancellationTokenSource();
token.onCancellationRequested(() => debugEnd.cancel());

let mainDebugRun: vscodeTypes.DebugSession | undefined;
this._vscode.debug.onDidStartDebugSession(session => {
if (session.name === debugSessionName)
mainDebugRun ??= session;
});
this._vscode.debug.onDidTerminateDebugSession(session => {
// child processes have their own debug sessions,
// but we only want to stop debugging if the user cancels the main session
if (session.id === mainDebugRun?.id)
debugEnd.cancel();
});

token = debugEnd.token;

await this._vscode.debug.startDebugging(undefined, {
type: 'pwa-node',
name: debugSessionName,
Expand Down
51 changes: 36 additions & 15 deletions src/playwrightTestServer.ts
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,8 +224,25 @@ 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 {
const debugEnd = new this._vscode.CancellationTokenSource();
token.onCancellationRequested(() => debugEnd.cancel());

let mainDebugRun: vscodeTypes.DebugSession | undefined;
this._vscode.debug.onDidStartDebugSession(session => {
if (session.name === debugSessionName)
mainDebugRun ??= session;
});
this._vscode.debug.onDidTerminateDebugSession(session => {
// child processes have their own debug sessions,
// but we only want to stop debugging if the user cancels the main session
if (session.id === mainDebugRun?.id)
debugEnd.cancel();
});

token = debugEnd.token;

await this._vscode.debug.startDebugging(undefined, {
type: 'pwa-node',
name: debugSessionName,
Expand Down Expand Up @@ -255,11 +278,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 +292,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
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
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
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