Skip to content

Allow Command to be subclassed by application code #288

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

Allow Command to be subclassed by application code #288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6d16216
Fix StandardInput, StandardOutput, and StandardError properties on Pr…
DaRosenberg Apr 22, 2025
36a3259
Change CreateStartInfo() method on Command from private to protected …
DaRosenberg Apr 22, 2025
01a5199
Add forgotten virtual keyword
DaRosenberg Apr 22, 2025
7d75a92
Fix indentation
DaRosenberg Apr 22, 2025
09ee619
Shift command configuration immutability to a contained record struct
DaRosenberg Apr 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 24 additions & 33 deletions CliWrap.Tests/ConfigurationSpecs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ public void I_can_configure_the_target_file()
var modified = original.WithTargetFile("bar");

// Assert
original.Should().BeEquivalentTo(modified, o => o.Excluding(c => c.TargetFilePath));
original.TargetFilePath.Should().NotBe(modified.TargetFilePath);
modified.Should().BeSameAs(original);
modified.TargetFilePath.Should().Be("bar");
}

Expand All @@ -53,8 +52,7 @@ public void I_can_configure_the_command_line_arguments()
var modified = original.WithArguments("qqq ppp");

// Assert
original.Should().BeEquivalentTo(modified, o => o.Excluding(c => c.Arguments));
original.Arguments.Should().NotBe(modified.Arguments);
modified.Should().BeSameAs(original);
modified.Arguments.Should().Be("qqq ppp");
}

Expand All @@ -68,8 +66,7 @@ public void I_can_configure_the_command_line_arguments_by_passing_an_array()
var modified = original.WithArguments(["-a", "foo bar"]);

// Assert
original.Should().BeEquivalentTo(modified, o => o.Excluding(c => c.Arguments));
original.Arguments.Should().NotBe(modified.Arguments);
modified.Should().BeSameAs(original);
modified.Arguments.Should().Be("-a \"foo bar\"");
}

Expand All @@ -90,8 +87,7 @@ public void I_can_configure_the_command_line_arguments_using_a_builder()
);

// Assert
original.Should().BeEquivalentTo(modified, o => o.Excluding(c => c.Arguments));
original.Arguments.Should().NotBe(modified.Arguments);
modified.Should().BeSameAs(original);
modified
.Arguments.Should()
.Be("-a \"foo bar\" \"\\\"foo\\\\bar\\\"\" 3.14 foo bar -5 89.13");
Expand All @@ -107,8 +103,7 @@ public void I_can_configure_the_working_directory()
var modified = original.WithWorkingDirectory("new");

// Assert
original.Should().BeEquivalentTo(modified, o => o.Excluding(c => c.WorkingDirPath));
original.WorkingDirPath.Should().NotBe(modified.WorkingDirPath);
modified.Should().BeSameAs(original);
modified.WorkingDirPath.Should().Be("new");
}

Expand All @@ -124,8 +119,7 @@ public void I_can_configure_the_resource_policy()
);

// Assert
original.Should().BeEquivalentTo(modified, o => o.Excluding(c => c.ResourcePolicy));
original.ResourcePolicy.Should().NotBe(modified.ResourcePolicy);
modified.Should().BeSameAs(original);
modified
.ResourcePolicy.Should()
.BeEquivalentTo(new ResourcePolicy(ProcessPriorityClass.High, 0x1, 1024, 2048));
Expand All @@ -146,8 +140,7 @@ public void I_can_configure_the_resource_policy_using_a_builder()
);

// Assert
original.Should().BeEquivalentTo(modified, o => o.Excluding(c => c.ResourcePolicy));
original.ResourcePolicy.Should().NotBe(modified.ResourcePolicy);
modified.Should().BeSameAs(original);
modified
.ResourcePolicy.Should()
.BeEquivalentTo(new ResourcePolicy(ProcessPriorityClass.High, 0x1, 1024, 2048));
Expand All @@ -165,8 +158,7 @@ public void I_can_configure_the_user_credentials()
);

// Assert
original.Should().BeEquivalentTo(modified, o => o.Excluding(c => c.Credentials));
original.Credentials.Should().NotBe(modified.Credentials);
modified.Should().BeSameAs(original);
modified
.Credentials.Should()
.BeEquivalentTo(new Credentials("domain", "username", "password", true));
Expand All @@ -184,8 +176,7 @@ public void I_can_configure_the_user_credentials_using_a_builder()
);

// Assert
original.Should().BeEquivalentTo(modified, o => o.Excluding(c => c.Credentials));
original.Credentials.Should().NotBe(modified.Credentials);
modified.Should().BeSameAs(original);
modified
.Credentials.Should()
.BeEquivalentTo(new Credentials("domain", "username", "password", true));
Expand All @@ -203,8 +194,7 @@ public void I_can_configure_the_environment_variables()
);

// Assert
original.Should().BeEquivalentTo(modified, o => o.Excluding(c => c.EnvironmentVariables));
original.EnvironmentVariables.Should().NotBeEquivalentTo(modified.EnvironmentVariables);
modified.Should().BeSameAs(original);
modified
.EnvironmentVariables.Should()
.BeEquivalentTo(
Expand All @@ -226,8 +216,7 @@ public void I_can_configure_the_environment_variables_using_a_builder()
);

// Assert
original.Should().BeEquivalentTo(modified, o => o.Excluding(c => c.EnvironmentVariables));
original.EnvironmentVariables.Should().NotBeEquivalentTo(modified.EnvironmentVariables);
modified.Should().BeSameAs(original);
modified
.EnvironmentVariables.Should()
.BeEquivalentTo(
Expand All @@ -251,8 +240,7 @@ public void I_can_configure_the_result_validation_strategy()
var modified = original.WithValidation(CommandResultValidation.None);

// Assert
original.Should().BeEquivalentTo(modified, o => o.Excluding(c => c.Validation));
original.Validation.Should().NotBe(modified.Validation);
modified.Should().BeSameAs(original);
modified.Validation.Should().Be(CommandResultValidation.None);
}

Expand All @@ -263,11 +251,12 @@ public void I_can_configure_the_stdin_pipe()
var original = Cli.Wrap("foo").WithStandardInputPipe(PipeSource.Null);

// Act
var modified = original.WithStandardInputPipe(PipeSource.FromString("new"));
var modifiedPipeSource = PipeSource.FromString("new");
var modified = original.WithStandardInputPipe(modifiedPipeSource);

// Assert
original.Should().BeEquivalentTo(modified, o => o.Excluding(c => c.StandardInputPipe));
original.StandardInputPipe.Should().NotBeSameAs(modified.StandardInputPipe);
modified.Should().BeSameAs(original);
modified.StandardInputPipe.Should().BeSameAs(modifiedPipeSource);
}

[Fact]
Expand All @@ -277,11 +266,12 @@ public void I_can_configure_the_stdout_pipe()
var original = Cli.Wrap("foo").WithStandardOutputPipe(PipeTarget.Null);

// Act
var modified = original.WithStandardOutputPipe(PipeTarget.ToStream(Stream.Null));
var modifiedPipeTarget = PipeTarget.ToStream(Stream.Null);
var modified = original.WithStandardOutputPipe(modifiedPipeTarget);

// Assert
original.Should().BeEquivalentTo(modified, o => o.Excluding(c => c.StandardOutputPipe));
original.StandardOutputPipe.Should().NotBeSameAs(modified.StandardOutputPipe);
modified.Should().BeSameAs(original);
modified.StandardOutputPipe.Should().BeSameAs(modifiedPipeTarget);
}

[Fact]
Expand All @@ -291,10 +281,11 @@ public void I_can_configure_the_stderr_pipe()
var original = Cli.Wrap("foo").WithStandardErrorPipe(PipeTarget.Null);

// Act
var modified = original.WithStandardErrorPipe(PipeTarget.ToStream(Stream.Null));
var modifiedPipeTarget = PipeTarget.ToStream(Stream.Null);
var modified = original.WithStandardErrorPipe(modifiedPipeTarget);

// Assert
original.Should().BeEquivalentTo(modified, o => o.Excluding(c => c.StandardErrorPipe));
original.StandardErrorPipe.Should().NotBeSameAs(modified.StandardErrorPipe);
modified.Should().BeSameAs(original);
modified.StandardErrorPipe.Should().BeSameAs(modifiedPipeTarget);
}
}
4 changes: 2 additions & 2 deletions CliWrap.Tests/ValidationSpecs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public async Task I_can_try_to_execute_a_command_and_get_an_error_if_it_returns_
);

ex.ExitCode.Should().Be(1);
ex.Command.Should().BeEquivalentTo(cmd);
ex.Command.Should().BeEquivalentTo(cmd.Configuration);

testOutput.WriteLine(ex.ToString());
}
Expand All @@ -39,7 +39,7 @@ public async Task I_can_try_to_execute_a_command_with_buffering_and_get_a_detail

ex.Message.Should().Contain("Exit code set to 1"); // expected stderr
ex.ExitCode.Should().Be(1);
ex.Command.Should().BeEquivalentTo(cmd);
ex.Command.Should().BeEquivalentTo(cmd.Configuration);

testOutput.WriteLine(ex.ToString());
}
Expand Down
80 changes: 44 additions & 36 deletions CliWrap/Command.Execution.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,13 @@ public partial class Command
// BAT or CMD file, even if it's on the PATH. If the extension is specified, it will work in both cases.
private string GetOptimallyQualifiedTargetFilePath()
{
var configuration = _configuration; // Snapshot to avoid race conditions

// Currently, we only need this workaround for script files on Windows, so short-circuit
// if we are on a different platform.
if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
return TargetFilePath;
return configuration.TargetFilePath;
}

// Don't do anything for fully qualified paths or paths that already have an extension specified.
Expand All @@ -33,11 +35,11 @@ private string GetOptimallyQualifiedTargetFilePath()
// strings like 'c:foo.txt' (which is relative to the current directory on drive C), but it's good
// enough for our purposes and the alternative is only available on .NET Standard 2.1+.
if (
Path.IsPathRooted(TargetFilePath)
|| !string.IsNullOrWhiteSpace(Path.GetExtension(TargetFilePath))
Path.IsPathRooted(configuration.TargetFilePath)
|| !string.IsNullOrWhiteSpace(Path.GetExtension(configuration.TargetFilePath))
)
{
return TargetFilePath;
return configuration.TargetFilePath;
}

static IEnumerable<string> GetProbeDirectoryPaths()
Expand Down Expand Up @@ -68,19 +70,22 @@ static IEnumerable<string> GetProbeDirectoryPaths()
return (
from probeDirPath in GetProbeDirectoryPaths()
where Directory.Exists(probeDirPath)
select Path.Combine(probeDirPath, TargetFilePath) into baseFilePath
select Path.Combine(probeDirPath, configuration.TargetFilePath) into baseFilePath
from extension in new[] { "exe", "cmd", "bat" }
select Path.ChangeExtension(baseFilePath, extension)
).FirstOrDefault(File.Exists) ?? TargetFilePath;
).FirstOrDefault(File.Exists) ?? configuration.TargetFilePath;
}

private ProcessStartInfo CreateStartInfo()
/// <summary>
/// Creates and configures a new <see cref="ProcessStartInfo" /> instance for executing the command.
/// </summary>
protected virtual ProcessStartInfo CreateStartInfo(ICommandConfiguration configuration)
{
var startInfo = new ProcessStartInfo
{
FileName = GetOptimallyQualifiedTargetFilePath(),
Arguments = Arguments,
WorkingDirectory = WorkingDirPath,
Arguments = configuration.Arguments,
WorkingDirectory = configuration.WorkingDirPath,
RedirectStandardInput = true,
RedirectStandardOutput = true,
RedirectStandardError = true,
Expand All @@ -98,17 +103,17 @@ private ProcessStartInfo CreateStartInfo()
{
// Disable CA1416 because we're handling an exception that is thrown by the property setters
#pragma warning disable CA1416
if (Credentials.Domain is not null)
startInfo.Domain = Credentials.Domain;
if (configuration.Credentials.Domain is not null)
startInfo.Domain = configuration.Credentials.Domain;

if (Credentials.UserName is not null)
startInfo.UserName = Credentials.UserName;
if (configuration.Credentials.UserName is not null)
startInfo.UserName = configuration.Credentials.UserName;

if (Credentials.Password is not null)
startInfo.Password = Credentials.Password.ToSecureString();
if (configuration.Credentials.Password is not null)
startInfo.Password = configuration.Credentials.Password.ToSecureString();

if (Credentials.LoadUserProfile)
startInfo.LoadUserProfile = Credentials.LoadUserProfile;
if (configuration.Credentials.LoadUserProfile)
startInfo.LoadUserProfile = configuration.Credentials.LoadUserProfile;
#pragma warning restore CA1416
}
catch (NotSupportedException ex)
Expand All @@ -121,7 +126,7 @@ private ProcessStartInfo CreateStartInfo()
}

// Set environment variables
foreach (var (key, value) in EnvironmentVariables)
foreach (var (key, value) in configuration.EnvironmentVariables)
{
if (value is not null)
{
Expand All @@ -148,8 +153,8 @@ private async Task PipeStandardInputAsync(
{
try
{
await StandardInputPipe
.CopyToAsync(process.StandardInput, cancellationToken)
await _configuration
.StandardInputPipe.CopyToAsync(process.StandardInput, cancellationToken)
// Some streams do not support cancellation, so we add a fallback that
// drops the task and returns early.
// This is important with stdin because the process might finish before
Expand All @@ -174,8 +179,8 @@ private async Task PipeStandardOutputAsync(
{
await using (process.StandardOutput.ToAsyncDisposable())
{
await StandardOutputPipe
.CopyFromAsync(process.StandardOutput, cancellationToken)
await _configuration
.StandardOutputPipe.CopyFromAsync(process.StandardOutput, cancellationToken)
.ConfigureAwait(false);
}
}
Expand All @@ -187,8 +192,8 @@ private async Task PipeStandardErrorAsync(
{
await using (process.StandardError.ToAsyncDisposable())
{
await StandardErrorPipe
.CopyFromAsync(process.StandardError, cancellationToken)
await _configuration
.StandardErrorPipe.CopyFromAsync(process.StandardError, cancellationToken)
.ConfigureAwait(false);
}
}
Expand All @@ -199,6 +204,8 @@ private async Task<CommandResult> ExecuteAsync(
CancellationToken gracefulCancellationToken = default
)
{
var configuration = _configuration; // Snapshot to avoid race conditions

using var _ = process;

// Additional cancellation to ensure we don't wait forever for the process to terminate
Expand Down Expand Up @@ -273,16 +280,16 @@ private async Task<CommandResult> ExecuteAsync(
);

// Validate the exit code if required
if (process.ExitCode != 0 && Validation.IsZeroExitCodeValidationEnabled())
if (process.ExitCode != 0 && configuration.Validation.IsZeroExitCodeValidationEnabled())
{
throw new CommandExecutionException(
this,
configuration,
process.ExitCode,
$"""
Command execution failed because the underlying process ({process.Name}#{process.Id}) returned a non-zero exit code ({process.ExitCode}).

Command:
{TargetFilePath} {Arguments}
{configuration.TargetFilePath} {configuration.Arguments}

You can suppress this validation by calling `{nameof(WithValidation)}({nameof(
CommandResultValidation
Expand All @@ -306,7 +313,8 @@ public CommandTask<CommandResult> ExecuteAsync(
CancellationToken gracefulCancellationToken
)
{
var process = new ProcessEx(CreateStartInfo());
var configuration = _configuration; // Snapshot to avoid race conditions
var process = new ProcessEx(CreateStartInfo(configuration));

// This method may fail, and we want to propagate the exceptions immediately instead
// of wrapping them in a task, so it needs to be executed in a synchronous context.
Expand All @@ -317,17 +325,17 @@ CancellationToken gracefulCancellationToken
{
// Disable CA1416 because we're handling an exception that is thrown by the property setters
#pragma warning disable CA1416
if (ResourcePolicy.Priority is not null)
p.PriorityClass = ResourcePolicy.Priority.Value;
if (configuration.ResourcePolicy.Priority is not null)
p.PriorityClass = configuration.ResourcePolicy.Priority.Value;

if (ResourcePolicy.Affinity is not null)
p.ProcessorAffinity = ResourcePolicy.Affinity.Value;
if (configuration.ResourcePolicy.Affinity is not null)
p.ProcessorAffinity = configuration.ResourcePolicy.Affinity.Value;

if (ResourcePolicy.MinWorkingSet is not null)
p.MinWorkingSet = ResourcePolicy.MinWorkingSet.Value;
if (configuration.ResourcePolicy.MinWorkingSet is not null)
p.MinWorkingSet = configuration.ResourcePolicy.MinWorkingSet.Value;

if (ResourcePolicy.MaxWorkingSet is not null)
p.MaxWorkingSet = ResourcePolicy.MaxWorkingSet.Value;
if (configuration.ResourcePolicy.MaxWorkingSet is not null)
p.MaxWorkingSet = configuration.ResourcePolicy.MaxWorkingSet.Value;
#pragma warning restore CA1416
}
catch (NotSupportedException ex)
Expand Down
Loading