-
Notifications
You must be signed in to change notification settings - Fork 74
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
Conversation
src/extension.ts
Outdated
testRun.token.onCancellationRequested(() => debugEnd.cancel()); | ||
|
||
let mainDebugRun: vscodeTypes.DebugSession | undefined; | ||
this._vscode.debug.onDidStartDebugSession(session => { mainDebugRun ??= session; }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's check the session.name
to be our own.
src/extension.ts
Outdated
@@ -534,7 +534,19 @@ export class Extension implements RunHooks { | |||
}; | |||
|
|||
if (mode === 'debug') { | |||
await model.debugTests(items, testListener, testRun.token); | |||
const debugEnd = new this._vscode.CancellationTokenSource(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I now think we should move this code to the place that calls vscode.debug.startDebugging
, instead of having it here.
Second attempt at #525, which got reverted.
The problem was that we were stopping everything as soon as the first debug session stops. It turns out that there's more than one debug session though! The child process that's used to evaluate
playwright.config.ts
gets its own child debug session. Once it successfully closes, theonDidTerminateDebugSession
handler is executed. In the past attempt, this closed debugging.The fix is to only call
debugEnd.cancel()
if the main debug session is closed.Closes microsoft/playwright#32387