-
Notifications
You must be signed in to change notification settings - Fork 675
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
Implement client side of test debugging #5965
Conversation
}; | ||
|
||
const result = await vscode.debug.startDebugging(undefined, debugConfiguration, undefined); | ||
return { |
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.
Does something inside the target process block waiting for Debugger.IsAttached
to return true? If not, you likely need a listener to know when the attach is complete (and we might need a debugger change to raise a new event).
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.
Yeah - the server side blocks and the tests won't start running until it gets this response from the client.
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.
Right, that isn't good enough -- you are going to return before the real attach is complete
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.
Ah the vscode API returns before the attach is complete? That is interesting. So on the server side I could wait for Debugger.IsAttached
? Is there any client side API that I could use to know when the real attach is done, or do I have to do it on the server?
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.
Hmm this is hard because I'm not actually attaching to the server process, but the vs test process. So I can't check Debugger.IsAttached
in the server, that check has to be done in the test host, which I don't think we can do easily.
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.
you likely need a listener to know when the attach is complete (and we might need a debugger change to raise a new event).
Do you have any more details on how to do this?
Also wondering if any of the other vscode debug events might be able to let me know whenthe debugger is actually attached.
/**
* An {@link Event} which fires when the {@link debug.activeDebugSession active debug session}
* has changed. *Note* that the event also fires when the active debug session changes
* to `undefined`.
*/
export const onDidChangeActiveDebugSession: Event<DebugSession | undefined>;
/**
* An {@link Event} which fires when a new {@link DebugSession debug session} has been started.
*/
export const onDidStartDebugSession: Event<DebugSession>;
/**
* An {@link Event} which fires when a custom DAP event is received from the {@link DebugSession debug session}.
*/
export const onDidReceiveDebugSessionCustomEvent: Event<DebugSessionCustomEvent>;
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.
If it isn't easy for the test host to call Debugger.IsAttached
I think what we should do is have the debugger raise a custom AttachComplete
event that we should be able to receive through onDidReceiveDebugSessionCustomEvent
. If that sounds good to you, I can open an issue.
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.
Yeah I think having the debugger raise a complete event sounds perfect. That would then work no matter what process we're trying to attach to.
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 opened #5968. I would suggest completing this PR when you are ready and I can wire things up when we insert a debugger with the fix.
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.
Perfect thanks! It will likely take a day or two for this PR to go in as the server side changes also need to be reviewed and merged before this goes in.
Client side part of test debugging
Part of #5719