From 057eac9daf4c53227f49cba69415e8b984c13331 Mon Sep 17 00:00:00 2001 From: Gitii Date: Sun, 26 Dec 2021 15:59:06 +0100 Subject: [PATCH] Fix ManagedCommand. (#1) * Fix ManagedCommand. Refactor Process management. Add more tests * Add more documentation. --- .../IntegrationsTests/ManagedCommandTests.cs | 21 ++++ .../UnitTests/ManagedCommandTests.cs | 106 ++++++++++++++++++ Community.Wsl.Sdk/Strategies/Api/IProcess.cs | 55 +++++++++ .../Strategies/Api/IProcessManager.cs | 21 ++++ .../Strategies/Api/Win32Process.cs | 47 ++++++++ .../Strategies/Api/Win32ProcessManager.cs | 17 +++ .../Strategies/Command/CommandStreams.cs | 23 +++- .../Strategies/Command/IStreamReader.cs | 19 +++- .../Strategies/Command/ManagedCommand.cs | 31 +++-- .../Strategies/Command/StreamDataReader.cs | 13 ++- .../Strategies/Command/StreamNullReader.cs | 23 +++- .../Strategies/Command/StreamStringReader.cs | 12 +- 12 files changed, 370 insertions(+), 18 deletions(-) create mode 100644 Community.Wsl.Sdk.Tests/UnitTests/ManagedCommandTests.cs create mode 100644 Community.Wsl.Sdk/Strategies/Api/IProcess.cs create mode 100644 Community.Wsl.Sdk/Strategies/Api/IProcessManager.cs create mode 100644 Community.Wsl.Sdk/Strategies/Api/Win32Process.cs create mode 100644 Community.Wsl.Sdk/Strategies/Api/Win32ProcessManager.cs diff --git a/Community.Wsl.Sdk.Tests/IntegrationsTests/ManagedCommandTests.cs b/Community.Wsl.Sdk.Tests/IntegrationsTests/ManagedCommandTests.cs index d9d074d..f29f4bf 100644 --- a/Community.Wsl.Sdk.Tests/IntegrationsTests/ManagedCommandTests.cs +++ b/Community.Wsl.Sdk.Tests/IntegrationsTests/ManagedCommandTests.cs @@ -1,4 +1,5 @@ using System; +using System.Threading.Tasks; using Community.Wsl.Sdk.Strategies.Api; using Community.Wsl.Sdk.Strategies.Command; using Community.Wsl.Sdk.Strategies.NativeMethods; @@ -88,4 +89,24 @@ public void Test_expect_stdout_to_equal_stdin() result.Stderr.Should().BeEmpty(); result.StderrData.Should().BeNull(); } + + [Test] + public async Task Test_async_wait() + { + var cmd = new ManagedCommand( + _distroName, + "echo", + new string[] { "-n", "test" }, + new CommandExecutionOptions() { StdoutDataProcessingMode = DataProcessingMode.String } + ); + + cmd.Start(); + var result = await cmd.WaitAndGetResultsAsync(); + + result.Stdout.Should().BeEquivalentTo("test"); + result.StdoutData.Should().BeNull(); + + result.Stderr.Should().BeNull(); + result.StderrData.Should().BeNull(); + } } diff --git a/Community.Wsl.Sdk.Tests/UnitTests/ManagedCommandTests.cs b/Community.Wsl.Sdk.Tests/UnitTests/ManagedCommandTests.cs new file mode 100644 index 0000000..03d6f90 --- /dev/null +++ b/Community.Wsl.Sdk.Tests/UnitTests/ManagedCommandTests.cs @@ -0,0 +1,106 @@ +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.IO; +using System.Linq; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using Castle.Core.Resource; +using Community.Wsl.Sdk.Strategies.Api; +using Community.Wsl.Sdk.Strategies.Command; +using FakeItEasy; +using FluentAssertions; +using NUnit.Framework; + +namespace Community.Wsl.Sdk.Tests.UnitTests +{ + public class ManagedCommandTests + { + public ManagedCommand CreateCommand( + string distroName, + string command, + string[] arguments, + CommandExecutionOptions options, + bool asRoot = false, + bool shellExecute = false + ) + { + var io = A.Fake(); + var env = A.Fake(); + var pm = A.Fake(); + + return new ManagedCommand( + distroName, + command, + arguments, + options, + asRoot, + shellExecute, + env, + io, + pm + ); + } + + [Test] + public void Test_Constructor() + { + var cmd = CreateCommand("dn", "", Array.Empty(), new CommandExecutionOptions()); + + cmd.HasExited.Should().BeFalse(); + cmd.HasWaited.Should().BeFalse(); + cmd.IsDisposed.Should().BeFalse(); + cmd.IsStarted.Should().BeFalse(); + } + + [Test] + public void Test_Start() + { + var distroName = "distro"; + var command = "command"; + var arguments = new string[] { "args" }; + var options = new CommandExecutionOptions(); + + var io = A.Fake(); + var env = A.Fake(); + var pm = A.Fake(); + + var p = A.Fake(); + + ProcessStartInfo actualStartInfo = new ProcessStartInfo(); + A.CallTo(() => pm.Start(A._)) + .Invokes((psi) => actualStartInfo = psi.GetArgument(0)!) + .Returns(p); + + var cmd = new ManagedCommand( + distroName, + command, + arguments, + options, + false, + false, + env, + io, + pm + ); + + var results = cmd.Start(); + + results.StandardOutput.Should().Be(StreamReader.Null); + results.StandardError.Should().Be(StreamReader.Null); + results.StandardInput.Should().Be(StreamWriter.Null); + + A.CallTo(() => pm.Start(A._)).MustHaveHappened(); + + actualStartInfo.ArgumentList + .Should() + .BeEquivalentTo("-d", distroName, "--exec", command, arguments[0]); + actualStartInfo.RedirectStandardOutput.Should().BeFalse(); + actualStartInfo.RedirectStandardError.Should().BeFalse(); + actualStartInfo.RedirectStandardInput.Should().BeFalse(); + + actualStartInfo.CreateNoWindow.Should().BeTrue(); + } + } +} diff --git a/Community.Wsl.Sdk/Strategies/Api/IProcess.cs b/Community.Wsl.Sdk/Strategies/Api/IProcess.cs new file mode 100644 index 0000000..96c4527 --- /dev/null +++ b/Community.Wsl.Sdk/Strategies/Api/IProcess.cs @@ -0,0 +1,55 @@ +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.IO; +using System.Text; + +namespace Community.Wsl.Sdk.Strategies.Api +{ + /// + /// Wrapper for a . + /// + /// + public interface IProcess : IDisposable + { + /// + /// + /// + public bool HasExited { get; } + + /// + /// + /// + public StreamReader StandardOutput { get; } + + /// + /// + /// + public StreamReader StandardError { get; } + + /// + /// + /// + public StreamWriter StandardInput { get; } + + /// + /// + /// + public bool EnableRaisingEvents { get; set; } + + /// + /// + /// + public event EventHandler Exited; + + /// + /// + /// + public int ExitCode { get; } + + /// + /// + /// + public void WaitForExit(); + } +} diff --git a/Community.Wsl.Sdk/Strategies/Api/IProcessManager.cs b/Community.Wsl.Sdk/Strategies/Api/IProcessManager.cs new file mode 100644 index 0000000..3133ad7 --- /dev/null +++ b/Community.Wsl.Sdk/Strategies/Api/IProcessManager.cs @@ -0,0 +1,21 @@ +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Text; + +namespace Community.Wsl.Sdk.Strategies.Api +{ + /// + /// Wrapper for . + /// Returns an wrapped in a . + /// + public interface IProcessManager + { + /// + /// + /// + /// + /// + public abstract IProcess? Start(ProcessStartInfo startInfo); + } +} diff --git a/Community.Wsl.Sdk/Strategies/Api/Win32Process.cs b/Community.Wsl.Sdk/Strategies/Api/Win32Process.cs new file mode 100644 index 0000000..70d21c4 --- /dev/null +++ b/Community.Wsl.Sdk/Strategies/Api/Win32Process.cs @@ -0,0 +1,47 @@ +using System; +using System.Diagnostics; +using System.IO; + +namespace Community.Wsl.Sdk.Strategies.Api; + +internal class Win32Process : IProcess +{ + private readonly Process _process; + + public Win32Process(Process process) + { + _process = process; + } + + public void Dispose() + { + _process.Dispose(); + } + + public bool HasExited => _process.HasExited; + + public StreamReader StandardOutput => _process.StandardOutput; + + public StreamReader StandardError => _process.StandardError; + + public StreamWriter StandardInput => _process.StandardInput; + + public bool EnableRaisingEvents + { + get => _process.EnableRaisingEvents; + set => _process.EnableRaisingEvents = value; + } + + public event EventHandler? Exited + { + add => _process.Exited += value; + remove => _process.Exited -= value; + } + + public int ExitCode => _process.ExitCode; + + public void WaitForExit() + { + _process.WaitForExit(); + } +} diff --git a/Community.Wsl.Sdk/Strategies/Api/Win32ProcessManager.cs b/Community.Wsl.Sdk/Strategies/Api/Win32ProcessManager.cs new file mode 100644 index 0000000..38d22f5 --- /dev/null +++ b/Community.Wsl.Sdk/Strategies/Api/Win32ProcessManager.cs @@ -0,0 +1,17 @@ +using System.Diagnostics; + +namespace Community.Wsl.Sdk.Strategies.Api; + +internal class Win32ProcessManager : IProcessManager +{ + public IProcess? Start(ProcessStartInfo startInfo) + { + var process = Process.Start(startInfo); + if (process == null) + { + return null; + } + + return new Win32Process(process); + } +} \ No newline at end of file diff --git a/Community.Wsl.Sdk/Strategies/Command/CommandStreams.cs b/Community.Wsl.Sdk/Strategies/Command/CommandStreams.cs index f222da5..538686d 100644 --- a/Community.Wsl.Sdk/Strategies/Command/CommandStreams.cs +++ b/Community.Wsl.Sdk/Strategies/Command/CommandStreams.cs @@ -2,9 +2,26 @@ namespace Community.Wsl.Sdk.Strategies.Command; +/// +/// Encapsulates the io stream of a . +/// public class CommandStreams { - public StreamWriter? StandardInput { get; init; } - public StreamReader? StandardOutput { get; init; } - public StreamReader? StandardError { get; init; } + /// + /// The standard input stream. + /// Will be if equals . + /// + public StreamWriter StandardInput { get; init; } = null!; + + /// + /// The standard output stream. + /// Will be if equals . + /// + public StreamReader StandardOutput { get; init; } = null!; + + /// + /// The standard error stream. + /// Will be if equals . + /// + public StreamReader StandardError { get; init; } = null!; } diff --git a/Community.Wsl.Sdk/Strategies/Command/IStreamReader.cs b/Community.Wsl.Sdk/Strategies/Command/IStreamReader.cs index 596fb5e..6d7c16e 100644 --- a/Community.Wsl.Sdk/Strategies/Command/IStreamReader.cs +++ b/Community.Wsl.Sdk/Strategies/Command/IStreamReader.cs @@ -1,12 +1,29 @@ -namespace Community.Wsl.Sdk.Strategies.Command; +using System.Threading.Tasks; + +namespace Community.Wsl.Sdk.Strategies.Command; internal interface IStreamReader { + /// + /// Starts copying the streamed data from the stream to an internal buffer. + /// This is done in a separate thread. + /// void Fetch(); + + /// + /// Copies the contents of the internal buffer to . + /// + /// + /// void CopyResultTo(ref CommandResult result, bool isStdOut); /// /// Blocks the current thread until fetching has been finished. /// void Wait(); + + /// + /// Returns a task which completes when the fetching has been finished. + /// + Task WaitAsync(); } \ No newline at end of file diff --git a/Community.Wsl.Sdk/Strategies/Command/ManagedCommand.cs b/Community.Wsl.Sdk/Strategies/Command/ManagedCommand.cs index 0551fd5..2f37623 100644 --- a/Community.Wsl.Sdk/Strategies/Command/ManagedCommand.cs +++ b/Community.Wsl.Sdk/Strategies/Command/ManagedCommand.cs @@ -2,13 +2,14 @@ using System.Diagnostics; using System.IO; using System.Threading.Tasks; +using Community.Wsl.Sdk.Strategies.Api; namespace Community.Wsl.Sdk.Strategies.Command; public class ManagedCommand : ICommand { private ProcessStartInfo? _startInfo; - private Process? _process; + private IProcess? _process; private bool _isStarted = false; private bool _hasWaited = false; private bool _isDisposed = false; @@ -22,15 +23,26 @@ public class ManagedCommand : ICommand private IStreamReader _stdoutReader; private IStreamReader _stderrReader; + private IEnvironment _environment; + private IIo _io; + private IProcessManager _processManager; + public ManagedCommand( string distroName, string command, string[] arguments, CommandExecutionOptions options, bool asRoot = false, - bool shellExecute = false + bool shellExecute = false, + IEnvironment? environment = null, + IIo? io = null, + IProcessManager? processManager = null ) { + _environment = environment ?? new Win32Environment(); + _io = io ?? new Win32IO(); + _processManager = processManager ?? new Win32ProcessManager(); + _options = options; _asRoot = asRoot; _shellExecute = shellExecute; @@ -70,8 +82,8 @@ public CommandStreams Start() _isStarted = true; - var wslPath = Path.Combine( - Environment.GetFolderPath(Environment.SpecialFolder.System), + var wslPath = _io.Combine( + _environment.GetFolderPath(Environment.SpecialFolder.System), "wsl.exe" ); @@ -124,7 +136,8 @@ public CommandStreams Start() _startInfo.StandardInputEncoding = _options.StdinEncoding ?? Console.InputEncoding; } - var process = Process.Start(_startInfo) ?? throw new Exception("Cannot start wsl process."); + var process = + _processManager.Start(_startInfo) ?? throw new Exception("Cannot start wsl process."); _process = process; @@ -227,14 +240,17 @@ public async Task WaitAndGetResultsAsync() await WaitForExit(_process!); - if (_options.FailOnNegativeExitCode && _process.ExitCode != 0) + if (_options.FailOnNegativeExitCode && _process!.ExitCode != 0) { throw new Exception($"Process exit code is non-zero: {_process.ExitCode}"); } var result = new CommandResult(); + await _stdoutReader.WaitAsync(); _stdoutReader.CopyResultTo(ref result, true); + + await _stderrReader.WaitAsync(); _stderrReader.CopyResultTo(ref result, false); return result; @@ -252,7 +268,7 @@ public Task StartAndGetResultsAsync() return WaitAndGetResultsAsync(); } - private Task WaitForExit(Process process) + private Task WaitForExit(IProcess process) { if (process.HasExited) { @@ -260,6 +276,7 @@ private Task WaitForExit(Process process) } TaskCompletionSource tcs = new TaskCompletionSource(); + process.EnableRaisingEvents = true; process.Exited += HasExited; if (process.HasExited) diff --git a/Community.Wsl.Sdk/Strategies/Command/StreamDataReader.cs b/Community.Wsl.Sdk/Strategies/Command/StreamDataReader.cs index c0e91a6..da4aa62 100644 --- a/Community.Wsl.Sdk/Strategies/Command/StreamDataReader.cs +++ b/Community.Wsl.Sdk/Strategies/Command/StreamDataReader.cs @@ -1,6 +1,7 @@ using System; using System.IO; using System.Threading; +using System.Threading.Tasks; namespace Community.Wsl.Sdk.Strategies.Command; @@ -9,6 +10,7 @@ internal class StreamDataReader : IStreamReader private StreamReader _reader; private Thread? _thread; private byte[]? _data; + private TaskCompletionSource? _completionSource; public StreamDataReader(StreamReader reader) { @@ -20,6 +22,7 @@ public StreamDataReader(StreamReader reader) private void Finished(byte[] data) { _data = data; + _completionSource?.SetResult(data); } public void Fetch() @@ -29,6 +32,8 @@ public void Fetch() throw new ArgumentException("Already started fetching!"); } + _completionSource = new TaskCompletionSource(); + _thread = new Thread( () => { @@ -37,7 +42,8 @@ public void Fetch() reader.BaseStream.CopyTo(stream); - Finished(stream.ToArray()); + var data = stream.ToArray(); + Finished(data); } ); @@ -70,4 +76,9 @@ public void Wait() { _thread?.Join(); } + + public async Task WaitAsync() + { + await (_completionSource?.Task ?? Task.FromResult(Array.Empty())); + } } \ No newline at end of file diff --git a/Community.Wsl.Sdk/Strategies/Command/StreamNullReader.cs b/Community.Wsl.Sdk/Strategies/Command/StreamNullReader.cs index 53623e7..c48a9ef 100644 --- a/Community.Wsl.Sdk/Strategies/Command/StreamNullReader.cs +++ b/Community.Wsl.Sdk/Strategies/Command/StreamNullReader.cs @@ -1,10 +1,23 @@ -namespace Community.Wsl.Sdk.Strategies.Command; +using System.Threading.Tasks; + +namespace Community.Wsl.Sdk.Strategies.Command; internal class StreamNullReader : IStreamReader { - public void Fetch() { } + public void Fetch() + { + } + + public void CopyResultTo(ref CommandResult result, bool isStdOut) + { + } - public void CopyResultTo(ref CommandResult result, bool isStdOut) { } + public void Wait() + { + } - public void Wait() { } -} + public Task WaitAsync() + { + return Task.CompletedTask; + } +} \ No newline at end of file diff --git a/Community.Wsl.Sdk/Strategies/Command/StreamStringReader.cs b/Community.Wsl.Sdk/Strategies/Command/StreamStringReader.cs index 71aa348..b29e5b5 100644 --- a/Community.Wsl.Sdk/Strategies/Command/StreamStringReader.cs +++ b/Community.Wsl.Sdk/Strategies/Command/StreamStringReader.cs @@ -1,6 +1,7 @@ using System; using System.IO; using System.Threading; +using System.Threading.Tasks; namespace Community.Wsl.Sdk.Strategies.Command { @@ -9,6 +10,7 @@ internal class StreamStringReader : IStreamReader private StreamReader _reader; private Thread? _thread; private string? _data; + private TaskCompletionSource? _completionSource; public StreamStringReader(StreamReader reader) { @@ -20,6 +22,7 @@ public StreamStringReader(StreamReader reader) private void Finished(string data) { _data = data; + _completionSource?.SetResult(data); } public void Fetch() @@ -29,6 +32,8 @@ public void Fetch() throw new ArgumentException("Already started fetching!"); } + _completionSource = new TaskCompletionSource(); + _thread = new Thread( () => { @@ -66,5 +71,10 @@ public void Wait() { _thread?.Join(); } + + public async Task WaitAsync() + { + await (_completionSource?.Task ?? Task.FromResult(String.Empty)); + } } -} +} \ No newline at end of file