From 57c2e191dbabcf0500e9264ceaf4ea2855dd4296 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Sat, 21 Oct 2017 09:00:18 -0700 Subject: [PATCH 1/2] don't wrap stderr as ErrorRecord updated tests --- .../engine/MshCommandRuntime.cs | 19 ++++++++++++++ .../engine/NativeCommandProcessor.cs | 5 +--- test/powershell/Host/ConsoleHost.Tests.ps1 | 25 ++++++------------- .../NativeExecution/NativeStreams.Tests.ps1 | 2 +- test/tools/TestExe/TestExe.cs | 14 +++++++++++ 5 files changed, 42 insertions(+), 23 deletions(-) diff --git a/src/System.Management.Automation/engine/MshCommandRuntime.cs b/src/System.Management.Automation/engine/MshCommandRuntime.cs index be5ce9d3089..f8b4216acea 100644 --- a/src/System.Management.Automation/engine/MshCommandRuntime.cs +++ b/src/System.Management.Automation/engine/MshCommandRuntime.cs @@ -2579,6 +2579,25 @@ internal void _WriteObjectSkipAllowCheck(object sendToPipeline) this.OutputPipe.Add(sendToPipeline); } + internal void _WriteErrorSkipAllowCheck(object sendToPipeline) + { + ThrowIfStopping(); + + if (AutomationNull.Value == sendToPipeline) + return; + + sendToPipeline = LanguagePrimitives.AsPSObjectOrNull(sendToPipeline); + + if (ErrorMergeTo != MergeDataStream.None) + { + Dbg.Assert(ErrorMergeTo == MergeDataStream.Output, "Only merging to success output is supported."); + this.OutputPipe.Add(sendToPipeline); + } + else + { + this.ErrorOutputPipe.Add(sendToPipeline); + } + } // NOTICE-2004/06/08-JonN 959638 // Use this variant to skip the ThrowIfWriteNotPermitted check diff --git a/src/System.Management.Automation/engine/NativeCommandProcessor.cs b/src/System.Management.Automation/engine/NativeCommandProcessor.cs index af785632519..f4ac40dfd3e 100644 --- a/src/System.Management.Automation/engine/NativeCommandProcessor.cs +++ b/src/System.Management.Automation/engine/NativeCommandProcessor.cs @@ -1032,10 +1032,7 @@ private void ProcessOutputRecord(ProcessOutputObject outputValue) if (outputValue.Stream == MinishellStream.Error) { - ErrorRecord record = outputValue.Data as ErrorRecord; - Dbg.Assert(record != null, "ProcessReader should ensure that data is ErrorRecord"); - record.SetInvocationInfo(this.Command.MyInvocation); - this.commandRuntime._WriteErrorSkipAllowCheck(record, isNativeError: true); + this.commandRuntime._WriteErrorSkipAllowCheck(outputValue.Data); } else if (outputValue.Stream == MinishellStream.Output) { diff --git a/test/powershell/Host/ConsoleHost.Tests.ps1 b/test/powershell/Host/ConsoleHost.Tests.ps1 index 44f7c177963..eccb58b9d2b 100644 --- a/test/powershell/Host/ConsoleHost.Tests.ps1 +++ b/test/powershell/Host/ConsoleHost.Tests.ps1 @@ -102,24 +102,13 @@ Describe "ConsoleHost unit tests" -tags "Feature" { } } - It "Verify Validate Dollar Error Populated should throw exception" { - $origEA = $ErrorActionPreference - $ErrorActionPreference = "Stop" - try - { - $a = 1,2,3 - $a | & $powershell -noprofile -command { wgwg-wrwrhqwrhrh35h3h3} - Throw "Test execution should not reach here!" - } - catch - { - $_.ToString() | Should Match "wgwg-wrwrhqwrhrh35h3h3" - $_.FullyQualifiedErrorId | Should Be "CommandNotFoundException" - } - finally - { - $ErrorActionPreference = $origEA - } + It "Verify Validate Dollar Error Populated should be in stderr" { + $stderrFile = "$testdrive\stderr.txt" + & $powershell -noprofile -command { wgwg-wrwrhqwrhrh35h3h3} 2> $stderrFile + $stderrFile | Should Exist + $stderr = Get-Content $stderrFile -Raw + $stderr | Should Match "wgwg-wrwrhqwrhrh35h3h3" + $stderr | Should Match "CommandNotFoundException" } It "Verify Validate Output Format As Text Explicitly Child Single Shell should works" { diff --git a/test/powershell/Language/Scripting/NativeExecution/NativeStreams.Tests.ps1 b/test/powershell/Language/Scripting/NativeExecution/NativeStreams.Tests.ps1 index 7ff0bde45a5..4a7e6e2a68e 100644 --- a/test/powershell/Language/Scripting/NativeExecution/NativeStreams.Tests.ps1 +++ b/test/powershell/Language/Scripting/NativeExecution/NativeStreams.Tests.ps1 @@ -18,7 +18,7 @@ Describe "Native streams behavior with PowerShell" -Tags 'CI' { # this check should be the first one, because $error is a global shared variable It 'should not add records to $error variable' { # we are keeping existing Windows PS v5.1 behavior for $error variable - $error.Count | Should Be 9 + $error.Count | Should Be 0 } It 'uses ErrorRecord object to return stderr output' { diff --git a/test/tools/TestExe/TestExe.cs b/test/tools/TestExe/TestExe.cs index bd2ae5219f5..210ccee88cd 100644 --- a/test/tools/TestExe/TestExe.cs +++ b/test/tools/TestExe/TestExe.cs @@ -18,6 +18,9 @@ static void Main(string[] args) case "-createchildprocess": CreateChildProcess(args); break; + case "-echostderr": + EchoStderr(args); + break; default: Console.WriteLine("Unknown test {0}", args[0]); break; @@ -40,6 +43,17 @@ static void EchoArgs(string[] args) } } + // + // Echos back to stderr the arguments passed in + // + static void EchoStderr(string[] args) + { + for (int i = 1; i < args.Length; i++) + { + Console.Error.WriteLine("{0}", args[i]); + } + } + // // First argument is the number of child processes to create which are instances of itself // Processes automatically exit after 100 seconds From 9a0c663e6b81ca7d12095baf08f2aa57a2e24855 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Tue, 24 Oct 2017 16:29:24 +0800 Subject: [PATCH 2/2] address PaulHigin's comments --- .../engine/MshCommandRuntime.cs | 7 +++++++ .../engine/NativeCommandProcessor.cs | 1 + test/powershell/Host/ConsoleHost.Tests.ps1 | 8 +++++++- 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/MshCommandRuntime.cs b/src/System.Management.Automation/engine/MshCommandRuntime.cs index f8b4216acea..f747125c470 100644 --- a/src/System.Management.Automation/engine/MshCommandRuntime.cs +++ b/src/System.Management.Automation/engine/MshCommandRuntime.cs @@ -2579,6 +2579,13 @@ internal void _WriteObjectSkipAllowCheck(object sendToPipeline) this.OutputPipe.Add(sendToPipeline); } + // Use this variant to write stderr as string and skip the ThrowIfWriteNotPermitted check + /// + /// The pipeline has already been terminated, or was terminated + /// during the execution of this method. + /// The Cmdlet should generally just allow PipelineStoppedException + /// to percolate up to the caller of ProcessRecord etc. + /// internal void _WriteErrorSkipAllowCheck(object sendToPipeline) { ThrowIfStopping(); diff --git a/src/System.Management.Automation/engine/NativeCommandProcessor.cs b/src/System.Management.Automation/engine/NativeCommandProcessor.cs index f4ac40dfd3e..3d5d42552c2 100644 --- a/src/System.Management.Automation/engine/NativeCommandProcessor.cs +++ b/src/System.Management.Automation/engine/NativeCommandProcessor.cs @@ -1032,6 +1032,7 @@ private void ProcessOutputRecord(ProcessOutputObject outputValue) if (outputValue.Stream == MinishellStream.Error) { + // write stderr as string instead of as ErrorRecord this.commandRuntime._WriteErrorSkipAllowCheck(outputValue.Data); } else if (outputValue.Stream == MinishellStream.Output) diff --git a/test/powershell/Host/ConsoleHost.Tests.ps1 b/test/powershell/Host/ConsoleHost.Tests.ps1 index eccb58b9d2b..ae2b368c95f 100644 --- a/test/powershell/Host/ConsoleHost.Tests.ps1 +++ b/test/powershell/Host/ConsoleHost.Tests.ps1 @@ -104,13 +104,19 @@ Describe "ConsoleHost unit tests" -tags "Feature" { It "Verify Validate Dollar Error Populated should be in stderr" { $stderrFile = "$testdrive\stderr.txt" - & $powershell -noprofile -command { wgwg-wrwrhqwrhrh35h3h3} 2> $stderrFile + & $powershell -noprofile -command { wgwg-wrwrhqwrhrh35h3h3 } 2> $stderrFile $stderrFile | Should Exist $stderr = Get-Content $stderrFile -Raw $stderr | Should Match "wgwg-wrwrhqwrhrh35h3h3" $stderr | Should Match "CommandNotFoundException" } + It "Verify Validate Dollar Error Populated should be in stderr redirected to stdout" { + $err = & $powershell -noprofile -command { wgwg-wrwrhqwrhrh35h3h3 } 2>&1 + $err.Exception.Message | Should Match "wgwg-wrwrhqwrhrh35h3h3" + $err.FullyQualifiedErrorId | Should BeExactly "CommandNotFoundException" + } + It "Verify Validate Output Format As Text Explicitly Child Single Shell should works" { { $a="blahblah"