-
-
Notifications
You must be signed in to change notification settings - Fork 740
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
Debug support #858
Merged
Merged
Debug support #858
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
using Cake.Core; | ||
using Cake.Core.Diagnostics; | ||
using Cake.Core.Scripting; | ||
using Cake.Commands; | ||
using Cake.Diagnostics; | ||
using Cake.Scripting; | ||
using NSubstitute; | ||
|
||
namespace Cake.Tests.Fixtures | ||
{ | ||
internal sealed class DebugCommandFixture | ||
{ | ||
public IScriptRunner ScriptRunner { get; set; } | ||
public ICakeLog Log { get; set; } | ||
public IDebugger Debugger { get; set; } | ||
public CakeOptions Options { get; set; } | ||
|
||
public DebugCommandFixture() | ||
{ | ||
ScriptRunner = Substitute.For<IScriptRunner>(); | ||
Log = Substitute.For<ICakeLog>(); | ||
Debugger = Substitute.For<IDebugger>(); | ||
Options = new CakeOptions | ||
{ | ||
Script = "build.cake" | ||
}; | ||
} | ||
|
||
public DebugCommand CreateCommand() | ||
{ | ||
var context = Substitute.For<ICakeContext>(); | ||
var engine = Substitute.For<ICakeEngine>(); | ||
var printer = Substitute.For<ICakeReportPrinter>(); | ||
|
||
context.Log.Returns(Log); | ||
|
||
var host = new BuildScriptHost(engine, context, printer, Log); | ||
|
||
return new DebugCommand(ScriptRunner, Debugger, host); | ||
} | ||
|
||
public bool Execute() | ||
{ | ||
return CreateCommand().Execute(Options); | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
using Cake.Core.Diagnostics; | ||
using Cake.Scripting; | ||
using Cake.Tests.Fixtures; | ||
using NSubstitute; | ||
using Xunit; | ||
|
||
namespace Cake.Tests.Unit.Scripting | ||
{ | ||
public sealed class DebugCommandTests | ||
{ | ||
public sealed class TheExecuteMethod | ||
{ | ||
[Fact] | ||
public void Should_Throw_If_Options_Is_Null() | ||
{ | ||
// Given | ||
var fixture = new DebugCommandFixture(); | ||
fixture.Options = null; | ||
|
||
// When | ||
var result = Record.Exception(() => fixture.Execute()); | ||
|
||
// Then | ||
Assert.IsArgumentNullException(result, "options"); | ||
} | ||
|
||
[Fact] | ||
public void Should_Proxy_Call_To_ScriptRunner() | ||
{ | ||
// Given | ||
var fixture = new DebugCommandFixture(); | ||
var runner = fixture.ScriptRunner; | ||
var options = fixture.Options; | ||
|
||
// When | ||
fixture.Execute(); | ||
|
||
// Then | ||
runner.Received(1).Run(Arg.Any<BuildScriptHost>(), options.Script, options.Arguments); | ||
} | ||
|
||
[Fact] | ||
public void Should_Report_Correct_Process_Id() | ||
{ | ||
// Given | ||
var fixture = new DebugCommandFixture(); | ||
var debugger = fixture.Debugger; | ||
var log = fixture.Log; | ||
var pid = 1234567; | ||
|
||
debugger.GetProcessId().Returns(pid); | ||
|
||
// When | ||
fixture.Execute(); | ||
|
||
// Then | ||
log.Received(1).Information(Verbosity.Quiet, Arg.Any<string>(), pid); | ||
} | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
using System; | ||
using System.Threading; | ||
using Cake.Core.Diagnostics; | ||
using Cake.Core.Scripting; | ||
using Cake.Diagnostics; | ||
using Cake.Scripting; | ||
|
||
namespace Cake.Commands | ||
{ | ||
/// <summary> | ||
/// A command that builds and debugs a build script. | ||
/// </summary> | ||
internal sealed class DebugCommand : ICommand | ||
{ | ||
private readonly IScriptRunner _scriptRunner; | ||
private readonly IDebugger _debugger; | ||
private readonly ICakeLog _log; | ||
private readonly BuildScriptHost _host; | ||
|
||
// Delegate factory used by Autofac. | ||
public delegate DebugCommand Factory(); | ||
|
||
public DebugCommand(IScriptRunner scriptRunner, IDebugger debugger, BuildScriptHost host) | ||
{ | ||
_scriptRunner = scriptRunner; | ||
_debugger = debugger; | ||
_host = host; | ||
_log = _host.Context.Log; | ||
} | ||
|
||
public bool Execute(CakeOptions options) | ||
{ | ||
if (options == null) | ||
{ | ||
throw new ArgumentNullException("options"); | ||
} | ||
|
||
var message = "Attach debugger to process {0} to continue"; | ||
var pid = _debugger.GetProcessId(); | ||
|
||
_log.Debug("Performing debug..."); | ||
_log.Information(Verbosity.Quiet, message, pid); | ||
|
||
_debugger.WaitForAttach(Timeout.InfiniteTimeSpan); | ||
|
||
_log.Debug("Debugger attached"); | ||
|
||
_scriptRunner.Run(_host, options.Script, options.Arguments); | ||
return true; | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Currently it will crash if break point isn't an task as the debugger isn't attached until tasks are executed.

I.e. if I start the script with a
System.Diagnostics.Debugger.Break();
at root level.Did a quick test by adding this before the
_scriptRunner.Run(_host, options.Script, options.Arguments);
This will launch the visual studio debug selector:

Then break at the launch

F5 will take you to the "root level" Break and all works from there

@cake-build/team & @mholo65 what do you think perhaps makes sense to move DebugScriptHost::RunTarget to DebugCommand::Execute?
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.
@devlead Yep, that's probably only solution if one wants to debug prior to RunTarget, which one most probably wants 😄
By doing this change, a separate DebugScriptHost shouldn't be needed anymore, it's fine with the default BuildScriptHost.
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.
@mholo65 yes makes total sense, go with that. Other than this looks epic 👍
Ping us once you adjusted, rebased and squashed into 1 commit and I'll review for merging.
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 think we should wait with launching the debugger until later. Waiting for attachment is probably OK for this PR and then we can expand on this. What do you think?
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.
@patriksvensson yes agree, but we should do wait in debug command execute and not RunTarget to fully enable debugging otherwise non task based code can't be debugged. i.e. arguments/global variables/etc.
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.
@patriksvensson @devlead yep, a --debug-launch switch could be useful and should be added in another PR... I'll open up an issue to track that.
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.
@devlead I agree. It should be done in command execute 😄
@mholo65 Perfect!
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.
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.
🍰