Added cancelation feature #7638
Conversation
|
Should you have a test? It can just sleep for 30 sec and if not canceled by then, Fail the test. |
|
|
||
| private void HandleServerNodeBuildCancel(ServerNodeBuildCancel command) | ||
| { | ||
| BuildManager.DefaultBuildManager.CancelAllSubmissions(); |
There was a problem hiding this comment.
I wonder, why can't we reuse here the cancellation from the MSBuildApp.Execute() function, by throwing the cancellation event manually? Also, how the things about the server after the current cancellation implementation works? Does server send the ServerNodeBuildResult package with the relevant information? A unit test would be helpful here, as we would see the intended output after the cancellation as well as check that the build was finalized.
There was a problem hiding this comment.
We discussed with Roman what we can reuse from the cancellation code in XMake (Console_CancelKeyPress). It contains some handling for interactive console, but server node works in non-interactive console.
There was a problem hiding this comment.
I planned to add test when Nathan's PR with server node tests is merged. We can merge his PR first and I will add test for cancelation.
AR-May
left a comment
AR-May
left a comment
There was a problem hiding this comment.
Feel free to merge it to the feature branch, and implement the test in other PR, as discussed!
I would be able to work on client side cancellation as soon as this PR is merged.
PR to the feature branch. It enables cancellation of the running build.