From 4c5920fb349606c63c2fdc2f5aaf39f2afbef5f6 Mon Sep 17 00:00:00 2001
From: Iristyle <Iristyle@github>
Date: Mon, 6 Nov 2017 14:57:26 -0800
Subject: [PATCH] (maint) Change winrm specs to write to stdout

 - The current WinRM specs use the Write-Output cmdlet. The
   Write-Output cmdlet is designed for writing objects to the
   PowerShell pipeline, *not* for writing stdout to a host.

   PowerShell has a few different methods of writing more directly to
   stdout, the most popular of which is Write-Host. Write-Host is
   typically frowned upon by the PowerShell community, but because it
   is not appropriate for use where pipelines may be involved:

   http://www.jsnover.com/blog/2013/12/07/write-host-considered-harmful/

   When the goal is to emit text to be consumed by other tooling that
   doesn't integrate with the PowerShell pipeline, Write-Host becomes
   an important tool, and likely the case the tests are interested in
   (independent ordered text-based writes to stdout).

   Such counterpoints are provided at:

   https://psitem.com/2015/01/16/is-write-host-really-harmful/

   The other alternatives to calling Write-Host are Out-Host,
   [Console]::WriteLine and the lesser known $Host.UI.WriteLine

 - While Write-Output can generate output for both stdout *and* for
   the PowerShell pipeline, there is also a problem with using it if
   output ordering is important.

   In PowerShell 5, changes were introduced to its behavior, so that
   implicit table formatting receives a 300ms delay. This often
   results in out-of-order output when Write-Output is used, and may
   especially be the case when pipeline writes with Write-Output are
   mixed with direct writes to stdout with Write-Host

   https://github.com/PowerShell/PowerShell/issues/4594
   https://windowsserver.uservoice.com/forums/301869-powershell/suggestions/14925213-bug-console-output-appears-out-of-order

 - Modify 2 tests so that Write-Host emits a single string with
   multiple embeds, rather than calling Write-Output with multiple
   string objects

 - Introduce additional test demonstrating ways of generating stdout
   in PowerShell - namely:

   Write-Output
      explicitly sent to Format-Table to avoid ordering issue
   Write-Host
   Out-Host
   $Host.UI.WriteLine
   [Console]::WriteLine
   Write-Information
---
 spec/bolt/node/winrm_spec.rb | 46 +++++++++++++++++++++++++++++-------
 1 file changed, 38 insertions(+), 8 deletions(-)

diff --git a/spec/bolt/node/winrm_spec.rb b/spec/bolt/node/winrm_spec.rb
index 1610ff68f2..9a811888a0 100644
--- a/spec/bolt/node/winrm_spec.rb
+++ b/spec/bolt/node/winrm_spec.rb
@@ -18,7 +18,7 @@
   let(:echo_script) { <<PS }
 foreach ($i in $args)
 {
-    Write-Output $i
+    Write-Host $i
 }
 PS
 
@@ -203,19 +203,50 @@ def stub_winrm_to_raise(klass, message)
   end
 
   it "can run a task remotely", winrm: true do
-    contents = 'Write-Output "$env:PT_message_one" ${env:PT_message two}'
+    contents = 'Write-Host "$env:PT_message_one ${env:PT_message two}"'
     arguments = { :message_one => 'task is running',
                   :"message two" => 'task has run' }
     with_tempfile_containing('task-test-winrm', contents) do |file|
       expect(winrm._run_task(file.path, 'environment', arguments).value)
-        .to eq("task is running\r\ntask has run\r\n")
+        .to eq("task is running task has run\r\n")
+    end
+  end
+
+  it "will collect on stdout using valid output mechanisms", winrm: true do
+    contents = <<-PS
+    # explicit Format-Table for PS5+ overrides implicit Format-Table which
+    # includes a 300ms delay waiting for more pipeline output
+    # https://github.com/PowerShell/PowerShell/issues/4594
+    Write-Output "message 1" | Format-Table
+
+    Write-Host "message 2"
+    "message 3" | Out-Host
+    $Host.UI.WriteLine("message 4")
+    [Console]::WriteLine("message 5")
+
+    # preference variable must be set to show Information messages
+    $InformationPreference = 'Continue'
+    Write-Information "message 6"
+    PS
+
+    with_tempfile_containing('stdout-test-winrm', contents) do |file|
+      expect(
+        winrm._run_script(file.path, []).value
+      ).to eq([
+        "message 1\r\n",
+        "message 2\r\n",
+        "message 3\r\n",
+        "message 4\r\n",
+        "message 5\r\n",
+        "message 6\r\n"
+      ].join(''))
     end
   end
 
   it "can run a task passing input on stdin", winrm: true do
     contents = <<PS
 $line = [Console]::In.ReadLine()
-Write-Output $line
+Write-Host $line
 PS
     arguments = { message_one: 'Hello from task', message_two: 'Goodbye' }
     with_tempfile_containing('tasks-test-stdin-winrm', contents) do |file|
@@ -226,17 +257,16 @@ def stub_winrm_to_raise(klass, message)
 
   it "can run a task passing input on stdin and environment", winrm: true do
     contents = <<PS
-Write-Output "$env:PT_message_one" "$env:PT_message_two"
+Write-Host "$env:PT_message_one $env:PT_message_two"
 $line = [Console]::In.ReadLine()
-Write-Output $line
+Write-Host $line
 PS
     arguments = { message_one: 'Hello from task', message_two: 'Goodbye' }
     with_tempfile_containing('tasks-test-both-winrm', contents) do |file|
       expect(
         winrm._run_task(file.path, 'both', arguments).value
       ).to eq([
-        "Hello from task\r\n",
-        "Goodbye\r\n",
+        "Hello from task Goodbye\r\n",
         "{\"message_one\":\"Hello from task\",\"message_two\":\"Goodbye\"}\r\n"
       ].join(''))
     end