From ccaaebf6e5ec9fe797e467229195b9544d8ffa2b Mon Sep 17 00:00:00 2001 From: Bryan Thornbury Date: Thu, 4 Feb 2016 18:18:08 -0800 Subject: [PATCH] Stream Forwarding changes to not wait on buffer full before writing. Instead input streams will be read character by character as they Console.Write or Console.WriteLine. Upon finding a newline character, the line will be printed to the parent process's console. --- src/Microsoft.DotNet.Cli.Utils/Command.cs | 16 +- src/Microsoft.DotNet.Cli.Utils/Reporter.cs | 6 +- .../StreamForwarder.cs | 118 ++++------ src/dotnet/commands/dotnet-restore/Program.cs | 1 - .../Commands/TestCommand.cs | 8 +- .../StreamForwarderTests.cs | 207 ++++++++++++------ .../OutputStandardOutputAndError.xproj | 20 ++ .../OutputStandardOutputAndError/Program.cs | 19 ++ .../OutputStandardOutputAndError/project.json | 14 ++ 9 files changed, 256 insertions(+), 153 deletions(-) create mode 100644 test/TestProjects/OutputStandardOutputAndError/OutputStandardOutputAndError.xproj create mode 100644 test/TestProjects/OutputStandardOutputAndError/Program.cs create mode 100644 test/TestProjects/OutputStandardOutputAndError/project.json diff --git a/src/Microsoft.DotNet.Cli.Utils/Command.cs b/src/Microsoft.DotNet.Cli.Utils/Command.cs index e8a0c7d60..6118deaa1 100644 --- a/src/Microsoft.DotNet.Cli.Utils/Command.cs +++ b/src/Microsoft.DotNet.Cli.Utils/Command.cs @@ -111,8 +111,8 @@ namespace Microsoft.DotNet.Cli.Utils return new CommandResult( this._process.StartInfo, exitCode, - _stdOut.GetCapturedOutput(), - _stdErr.GetCapturedOutput()); + _stdOut.CapturedOutput, + _stdErr.CapturedOutput); } public Command WorkingDirectory(string projectDirectory) @@ -148,11 +148,11 @@ namespace Microsoft.DotNet.Cli.Utils { if (to == null) { - _stdOut.ForwardTo(write: Reporter.Output.Write, writeLine: Reporter.Output.WriteLine); + _stdOut.ForwardTo(writeLine: Reporter.Output.WriteLine); } else { - _stdOut.ForwardTo(write: to.Write, writeLine: to.WriteLine); + _stdOut.ForwardTo(writeLine: to.WriteLine); } } return this; @@ -165,11 +165,11 @@ namespace Microsoft.DotNet.Cli.Utils { if (to == null) { - _stdErr.ForwardTo(write: Reporter.Error.Write, writeLine: Reporter.Error.WriteLine); + _stdErr.ForwardTo(writeLine: Reporter.Error.WriteLine); } else { - _stdErr.ForwardTo(write: to.Write, writeLine: to.WriteLine); + _stdErr.ForwardTo(writeLine: to.WriteLine); } } return this; @@ -178,14 +178,14 @@ namespace Microsoft.DotNet.Cli.Utils public Command OnOutputLine(Action handler) { ThrowIfRunning(); - _stdOut.ForwardTo(write: null, writeLine: handler); + _stdOut.ForwardTo(writeLine: handler); return this; } public Command OnErrorLine(Action handler) { ThrowIfRunning(); - _stdErr.ForwardTo(write: null, writeLine: handler); + _stdErr.ForwardTo(writeLine: handler); return this; } diff --git a/src/Microsoft.DotNet.Cli.Utils/Reporter.cs b/src/Microsoft.DotNet.Cli.Utils/Reporter.cs index aca05a270..e60ac3c50 100644 --- a/src/Microsoft.DotNet.Cli.Utils/Reporter.cs +++ b/src/Microsoft.DotNet.Cli.Utils/Reporter.cs @@ -9,7 +9,7 @@ namespace Microsoft.DotNet.Cli.Utils // Stupid-simple console manager public class Reporter { - private static readonly Reporter Null = new Reporter(console: null); + private static readonly Reporter NullReporter = new Reporter(console: null); private static object _lock = new object(); private readonly AnsiConsole _console; @@ -20,8 +20,8 @@ namespace Microsoft.DotNet.Cli.Utils } public static Reporter Output { get; } = Create(AnsiConsole.GetOutput); - public static Reporter Error { get; } = Create(AnsiConsole.GetOutput); - public static Reporter Verbose { get; } = CommandContext.IsVerbose() ? Create(AnsiConsole.GetOutput) : Null; + public static Reporter Error { get; } = Create(AnsiConsole.GetError); + public static Reporter Verbose { get; } = CommandContext.IsVerbose() ? Create(AnsiConsole.GetOutput) : NullReporter; public static Reporter Create(Func getter) { diff --git a/src/Microsoft.DotNet.Cli.Utils/StreamForwarder.cs b/src/Microsoft.DotNet.Cli.Utils/StreamForwarder.cs index 1d8537209..4927426d1 100644 --- a/src/Microsoft.DotNet.Cli.Utils/StreamForwarder.cs +++ b/src/Microsoft.DotNet.Cli.Utils/StreamForwarder.cs @@ -7,45 +7,45 @@ namespace Microsoft.DotNet.Cli.Utils { public sealed class StreamForwarder { - private const int DefaultBufferSize = 256; - - private readonly int _bufferSize; private StringBuilder _builder; private StringWriter _capture; private Action _write; private Action _writeLine; - public StreamForwarder(int bufferSize = DefaultBufferSize) + public string CapturedOutput { - _bufferSize = bufferSize; + get + { + return _capture?.GetStringBuilder()?.ToString(); + } } - public void Capture() + public StreamForwarder Capture() { if (_capture != null) { throw new InvalidOperationException("Already capturing stream!"); } _capture = new StringWriter(); + + return this; } - public string GetCapturedOutput() - { - return _capture?.GetStringBuilder()?.ToString(); - } - - public void ForwardTo(Action write, Action writeLine) + public StreamForwarder ForwardTo(Action writeLine) { if (writeLine == null) { throw new ArgumentNullException(nameof(writeLine)); } + if (_writeLine != null) { - throw new InvalidOperationException("Already handling stream!"); + throw new InvalidOperationException("WriteLine forwarder set previously"); } - _write = write; + _writeLine = writeLine; + + return this; } public Thread BeginRead(TextReader reader) @@ -57,91 +57,59 @@ namespace Microsoft.DotNet.Cli.Utils public void Read(TextReader reader) { + var bufferSize = 1; + + int readCharacterCount; + char currentCharacter; + + var buffer = new char[bufferSize]; _builder = new StringBuilder(); - var buffer = new char[_bufferSize]; - int n; - while ((n = reader.Read(buffer, 0, _bufferSize)) > 0) - { - _builder.Append(buffer, 0, n); - WriteBlocks(); - } - WriteRemainder(); - } - private void WriteBlocks() - { - int n = _builder.Length; - if (n == 0) + // Using Read with buffer size 1 to prevent looping endlessly + // like we would when using Read() with no buffer + while ((readCharacterCount = reader.Read(buffer, 0, bufferSize)) > 0) { - return; - } + currentCharacter = buffer[0]; - int offset = 0; - bool sawReturn = false; - for (int i = 0; i < n; i++) - { - char c = _builder[i]; - switch (c) + // Flush per line + if (currentCharacter == '\n') { - case '\r': - sawReturn = true; - continue; - case '\n': - WriteLine(_builder.ToString(offset, i - offset - (sawReturn ? 1 : 0))); - offset = i + 1; - break; + WriteBuilder(); + } + else + { + // Ignore \r + if (currentCharacter != '\r') + { + _builder.Append(currentCharacter); + } } - sawReturn = false; } - // If the buffer contains no line breaks and _write is - // supported, send the buffer content. - if (!sawReturn && - (offset == 0) && - ((_write != null) || (_writeLine == null))) - { - WriteRemainder(); - } - else - { - _builder.Remove(0, offset); - } + // Flush anything else when the stream is closed + // Which should only happen if someone used console.Write + WriteBuilder(); } - private void WriteRemainder() + private void WriteBuilder() { if (_builder.Length == 0) { return; } - Write(_builder.ToString()); + + WriteLine(_builder.ToString()); _builder.Clear(); } private void WriteLine(string str) - { + { if (_capture != null) { _capture.WriteLine(str); } - // If _write is supported, so is _writeLine. - if (_writeLine != null) - { - _writeLine(str); - } - } - private void Write(string str) - { - if (_capture != null) - { - _capture.Write(str); - } - if (_write != null) - { - _write(str); - } - else if (_writeLine != null) + if (_writeLine != null) { _writeLine(str); } diff --git a/src/dotnet/commands/dotnet-restore/Program.cs b/src/dotnet/commands/dotnet-restore/Program.cs index 5a3a3ce42..2c85ea577 100644 --- a/src/dotnet/commands/dotnet-restore/Program.cs +++ b/src/dotnet/commands/dotnet-restore/Program.cs @@ -21,7 +21,6 @@ namespace Microsoft.DotNet.Tools.Restore { private static readonly string DefaultRid = PlatformServices.Default.Runtime.GetLegacyRestoreRuntimeIdentifier(); - public static int Run(string[] args) { DebugHelper.HandleDebugSwitch(ref args); diff --git a/test/Microsoft.DotNet.Tools.Tests.Utilities/Commands/TestCommand.cs b/test/Microsoft.DotNet.Tools.Tests.Utilities/Commands/TestCommand.cs index 0b319ed6f..f1672a62a 100644 --- a/test/Microsoft.DotNet.Tools.Tests.Utilities/Commands/TestCommand.cs +++ b/test/Microsoft.DotNet.Tools.Tests.Utilities/Commands/TestCommand.cs @@ -32,8 +32,8 @@ namespace Microsoft.DotNet.Tools.Test.Utilities var stdOut = new StreamForwarder(); var stdErr = new StreamForwarder(); - stdOut.ForwardTo(write: Reporter.Output.Write, writeLine: Reporter.Output.WriteLine); - stdErr.ForwardTo(write: Reporter.Error.Write, writeLine: Reporter.Output.WriteLine); + stdOut.ForwardTo(writeLine: Reporter.Output.WriteLine); + stdErr.ForwardTo(writeLine: Reporter.Output.WriteLine); return RunProcess(commandPath, args, stdOut, stdErr); } @@ -82,8 +82,8 @@ namespace Microsoft.DotNet.Tools.Test.Utilities var result = new CommandResult( process.StartInfo, process.ExitCode, - stdOut.GetCapturedOutput(), - stdErr.GetCapturedOutput()); + stdOut.CapturedOutput, + stdErr.CapturedOutput); return result; } diff --git a/test/StreamForwarderTests/StreamForwarderTests.cs b/test/StreamForwarderTests/StreamForwarderTests.cs index 226ab4a51..0e1eb0081 100644 --- a/test/StreamForwarderTests/StreamForwarderTests.cs +++ b/test/StreamForwarderTests/StreamForwarderTests.cs @@ -10,109 +10,192 @@ using Xunit; using Microsoft.DotNet.Cli.Utils; using Microsoft.DotNet.ProjectModel; using Microsoft.DotNet.Tools.Test.Utilities; +using Microsoft.Extensions.PlatformAbstractions; +using System.Threading; namespace StreamForwarderTests { - public class StreamForwarderTests + public class StreamForwarderTests : TestBase { + private static readonly string s_rid = PlatformServices.Default.Runtime.GetLegacyRestoreRuntimeIdentifier(); + private static readonly string s_testProjectRoot = Path.Combine(AppContext.BaseDirectory, "TestProjects"); + + private TempDirectory _root; + public static void Main() { Console.WriteLine("Dummy Entrypoint"); } - [Fact] - public void Unbuffered() + public StreamForwarderTests() { - Forward(4, true, ""); - Forward(4, true, "123", "123"); - Forward(4, true, "1234", "1234"); - Forward(3, true, "123456789", "123", "456", "789"); - Forward(4, true, "\r\n", "\n"); - Forward(4, true, "\r\n34", "\n", "34"); - Forward(4, true, "1\r\n4", "1\n", "4"); - Forward(4, true, "12\r\n", "12\n"); - Forward(4, true, "123\r\n", "123\n"); - Forward(4, true, "1234\r\n", "1234", "\n"); - Forward(3, true, "\r\n3456\r\n9", "\n", "3456", "\n", "9"); - Forward(4, true, "\n", "\n"); - Forward(4, true, "\n234", "\n", "234"); - Forward(4, true, "1\n34", "1\n", "34"); - Forward(4, true, "12\n4", "12\n", "4"); - Forward(4, true, "123\n", "123\n"); - Forward(4, true, "1234\n", "1234", "\n"); - Forward(3, true, "\n23456\n89", "\n", "23456", "\n", "89"); + _root = Temp.CreateDirectory(); } - [Fact] - public void LineBuffered() + public static IEnumerable ForwardingTheoryVariations { - Forward(4, false, ""); - Forward(4, false, "123", "123\n"); - Forward(4, false, "1234", "1234\n"); - Forward(3, false, "123456789", "123456789\n"); - Forward(4, false, "\r\n", "\n"); - Forward(4, false, "\r\n34", "\n", "34\n"); - Forward(4, false, "1\r\n4", "1\n", "4\n"); - Forward(4, false, "12\r\n", "12\n"); - Forward(4, false, "123\r\n", "123\n"); - Forward(4, false, "1234\r\n", "1234\n"); - Forward(3, false, "\r\n3456\r\n9", "\n", "3456\n", "9\n"); - Forward(4, false, "\n", "\n"); - Forward(4, false, "\n234", "\n", "234\n"); - Forward(4, false, "1\n34", "1\n", "34\n"); - Forward(4, false, "12\n4", "12\n", "4\n"); - Forward(4, false, "123\n", "123\n"); - Forward(4, false, "1234\n", "1234\n"); - Forward(3, false, "\n23456\n89", "\n", "23456\n", "89\n"); + get + { + return new[] + { + new object[] { "123", new string[]{"123"} }, + new object[] { "123\n", new string[] {"123"} }, + new object[] { "123\r\n", new string[] {"123"} }, + new object[] { "1234\n5678", new string[] {"1234", "5678"} }, + new object[] { "1234\r\n5678", new string[] {"1234", "5678"} }, + new object[] { "1234\n5678\n", new string[] {"1234", "5678"} }, + new object[] { "1234\r\n5678\r\n", new string[] {"1234", "5678"} }, + new object[] { "1234\n5678\nabcdefghijklmnopqrstuvwxyz", new string[] {"1234", "5678", "abcdefghijklmnopqrstuvwxyz"} }, + new object[] { "1234\r\n5678\r\nabcdefghijklmnopqrstuvwxyz", new string[] {"1234", "5678", "abcdefghijklmnopqrstuvwxyz"} }, + new object[] { "1234\n5678\nabcdefghijklmnopqrstuvwxyz\n", new string[] {"1234", "5678", "abcdefghijklmnopqrstuvwxyz"} }, + new object[] { "1234\r\n5678\r\nabcdefghijklmnopqrstuvwxyz\r\n", new string[] {"1234", "5678", "abcdefghijklmnopqrstuvwxyz"} } + }; + } } - private static void Forward(int bufferSize, bool unbuffered, string str, params string[] expectedWrites) + [Theory] + [InlineData("123")] + [InlineData("123\n")] + public void TestNoForwardingNoCapture(string inputStr) + { + TestCapturingAndForwardingHelper(ForwardOptions.None, inputStr, null, new string[0]); + } + + [Theory] + [MemberData("ForwardingTheoryVariations")] + public void TestForwardingOnly(string inputStr, string[] expectedWrites) { - var expectedCaptured = str.Replace("\r", "").Replace("\n", Environment.NewLine); + for(int i = 0; i < expectedWrites.Length; ++i) + { + expectedWrites[i] += Environment.NewLine; + } + + TestCapturingAndForwardingHelper(ForwardOptions.WriteLine, inputStr, null, expectedWrites); + } - // No forwarding. - Forward(bufferSize, ForwardOptions.None, str, null, new string[0]); + [Theory] + [MemberData("ForwardingTheoryVariations")] + public void TestCaptureOnly(string inputStr, string[] expectedWrites) + { + for(int i = 0; i < expectedWrites.Length; ++i) + { + expectedWrites[i] += Environment.NewLine; + } - // Capture only. - Forward(bufferSize, ForwardOptions.Capture, str, expectedCaptured, new string[0]); + var expectedCaptured = string.Join("", expectedWrites); + + TestCapturingAndForwardingHelper(ForwardOptions.Capture, inputStr, expectedCaptured, new string[0]); + } - var writeOptions = unbuffered ? - ForwardOptions.Write | ForwardOptions.WriteLine : - ForwardOptions.WriteLine; + [Theory] + [MemberData("ForwardingTheoryVariations")] + public void TestCaptureAndForwardingTogether(string inputStr, string[] expectedWrites) + { + for(int i = 0; i < expectedWrites.Length; ++i) + { + expectedWrites[i] += Environment.NewLine; + } - // Forward. - Forward(bufferSize, writeOptions, str, null, expectedWrites); + var expectedCaptured = string.Join("", expectedWrites); - // Forward and capture. - Forward(bufferSize, writeOptions | ForwardOptions.Capture, str, expectedCaptured, expectedWrites); + TestCapturingAndForwardingHelper(ForwardOptions.WriteLine | ForwardOptions.Capture, inputStr, expectedCaptured, expectedWrites); } private enum ForwardOptions { None = 0x0, Capture = 0x1, - Write = 0x02, - WriteLine = 0x04, + WriteLine = 0x02, } - private static void Forward(int bufferSize, ForwardOptions options, string str, string expectedCaptured, string[] expectedWrites) + private void TestCapturingAndForwardingHelper(ForwardOptions options, string str, string expectedCaptured, string[] expectedWrites) { - var forwarder = new StreamForwarder(bufferSize); + var forwarder = new StreamForwarder(); var writes = new List(); + if ((options & ForwardOptions.WriteLine) != 0) { - forwarder.ForwardTo( - write: (options & ForwardOptions.Write) == 0 ? (Action)null : writes.Add, - writeLine: s => writes.Add(s + "\n")); + forwarder.ForwardTo(writeLine: s => writes.Add(s + Environment.NewLine)); } if ((options & ForwardOptions.Capture) != 0) { forwarder.Capture(); } + forwarder.Read(new StringReader(str)); Assert.Equal(expectedWrites, writes); - var captured = forwarder.GetCapturedOutput(); + + var captured = forwarder.CapturedOutput; Assert.Equal(expectedCaptured, captured); } + + [Fact] + public void TestAsyncOrdering() + { + var expectedOutputLines = new string[] + { + "** Standard Out 1 **", + "** Standard Error 1 **", + "** Standard Out 2 **", + "** Standard Error 2 **" + }; + + var expectedOutput = string.Join(Environment.NewLine, expectedOutputLines) + Environment.NewLine; + + var testProjectDllPath = SetupTestProject(); + + var testReporter = new TestReporter(); + + var testCommand = Command.Create(testProjectDllPath, new string[0]) + .OnOutputLine(testReporter.WriteLine) + .OnErrorLine(testReporter.WriteLine); + + testCommand.Execute(); + + var resultString = testReporter.InternalStringWriter.GetStringBuilder().ToString(); + Console.WriteLine(expectedOutput); + Console.WriteLine(resultString); + + Assert.Equal(expectedOutput, resultString); + } + + private string SetupTestProject() + { + var sourceTestProjectPath = Path.Combine(s_testProjectRoot, "OutputStandardOutputAndError"); + + var binTestProjectPath = _root.CopyDirectory(sourceTestProjectPath).Path; + + var buildCommand = new BuildCommand(Path.Combine(binTestProjectPath, "project.json")); + buildCommand.Execute(); + + var buildOutputExe = "OutputStandardOutputAndError" + Constants.ExeSuffix; + var buildOutputPath = Path.Combine(binTestProjectPath, "bin/Debug/dnxcore50", buildOutputExe); + + return buildOutputPath; + } + + private class TestReporter + { + private static object _lock = new object(); + private StringWriter _stringWriter; + + public StringWriter InternalStringWriter + { + get { return _stringWriter; } + } + + public TestReporter() + { + _stringWriter = new StringWriter(); + } + + public void WriteLine(string message) + { + lock(_lock) + { + _stringWriter.WriteLine(message); + } + } + } } } diff --git a/test/TestProjects/OutputStandardOutputAndError/OutputStandardOutputAndError.xproj b/test/TestProjects/OutputStandardOutputAndError/OutputStandardOutputAndError.xproj new file mode 100644 index 000000000..ee6ccb69d --- /dev/null +++ b/test/TestProjects/OutputStandardOutputAndError/OutputStandardOutputAndError.xproj @@ -0,0 +1,20 @@ + + + + 14.0 + $(MSBuildExtensionsPath32)\Microsoft\VisualStudio\v$(VisualStudioVersion) + + + + + 58808bbc-371e-47d6-a3d0-4902145eda4e + OutputStandardOutputAndError + ..\..\artifacts\obj\$(MSBuildProjectName) + ..\..\artifacts\bin\$(MSBuildProjectName)\ + + + + 2.0 + + + diff --git a/test/TestProjects/OutputStandardOutputAndError/Program.cs b/test/TestProjects/OutputStandardOutputAndError/Program.cs new file mode 100644 index 000000000..07c3e376d --- /dev/null +++ b/test/TestProjects/OutputStandardOutputAndError/Program.cs @@ -0,0 +1,19 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Diagnostics; + +namespace TestApp +{ + public class Program + { + public static void Main(string[] args) + { + Console.Out.WriteLine("** Standard Out 1 **"); + Console.Error.WriteLine("** Standard Error 1 **"); + Console.Out.WriteLine("** Standard Out 2 **"); + Console.Error.WriteLine("** Standard Error 2 **"); + } + } +} diff --git a/test/TestProjects/OutputStandardOutputAndError/project.json b/test/TestProjects/OutputStandardOutputAndError/project.json new file mode 100644 index 000000000..f9b804dda --- /dev/null +++ b/test/TestProjects/OutputStandardOutputAndError/project.json @@ -0,0 +1,14 @@ +{ + "version": "1.0.0-*", + "compilationOptions": { + "emitEntryPoint": true + }, + + "dependencies": { + "NETStandard.Library": "1.0.0-rc2-23728" + }, + + "frameworks": { + "dnxcore50": { } + } +}