Conversation
salmanmkc
left a comment
salmanmkc
left a comment
There was a problem hiding this comment.
couple tests that'd be nice to have:
- cancellation during
WaitForCommandAsync(not just pre-cancelled) - exception in
OnJobCompletedAsyncduring FinalizeJob
| try | ||
| { | ||
| _dapDebugger = HostContext.GetService<IDapDebugger>(); | ||
| await _dapDebugger.StartAsync(jobContext); |
There was a problem hiding this comment.
should we pass in the step level context instead of the jobContext?
There was a problem hiding this comment.
The debugger needs jobs cancellation, Global.Debugger and variable provider from the job context, plus I think it needs a context that outlives the setup step? This isn't new btw, it just moved into the step.
There was a problem hiding this comment.
the Global object is shared across all context (step + job).
you were using jobcontext since the code in jobrunner.cs doesn't have a step context.
There was a problem hiding this comment.
But also we hook up to jobcontext cancelation in the debugger, if we used the step context wouldn't that be completed when Setup job completes?
This adds the DAP server setup to the Setup job step, to both help with messaging for users and keep the step as in progress to communicate we're waiting on a debugger to connect.
We also add a final pause during the Complete job step to allow one final inspection after all steps have completed.
https://github.com/github/c2c-actions/issues/9828