Fix bug with truncating newline (#950)

Fix bug with truncating newline

This was followup promised in #943
Remaining concern
Of course I love .rstrip for the simplicity of it, but there may be a case where this method receives multiple empty lines, and it is expected to keep them. Like, it gets foo\n\n\n, and it is expected to write foo\n\n. This whole thing is frustrating and confusing. This scenario is probably unlikely to happen because verbose events only write 1 line at a time anyway. There might still be a case I'm missing that's important, and in that case, this could be adjusted so that only 1 trailing newline is removed from stdout_chunk, instead of all newline characters. I expect that would be less performant, so if this satisfies the current needs I'd keep it.

Reviewed-by: Alexander Sowitzki <dev@eqrx.net>
Reviewed-by: David Shrewsbury <None>
Reviewed-by: None <None>

ansible_runner/utils/__init__.py

@@ -410,7 +410,7 @@ def _emit_event(self, buffered_stdout, next_event_data=None):
                 event_data['uuid'] = str(uuid.uuid4())
             self._counter += 1
             event_data['counter'] = self._counter
-            event_data['stdout'] = stdout_chunk[:-2] if len(stdout_chunk) > 2 else ""
+            event_data['stdout'] = stdout_chunk.rstrip('\n\r')
             n_lines = stdout_chunk.count('\n')
             event_data['start_line'] = self._start_line
             event_data['end_line'] = self._start_line + n_lines

test/unit/test_runner.py

@@ -151,7 +151,7 @@ def test_status_callback_interface(rc, mocker):
 def test_stdout_file_write(rc, runner_mode):
     if runner_mode == 'pexpect':
         pytest.skip('Writing to stdout can be flaky, probably due to some pexpect bug')
-    rc.command = ['echo', 'hello_world_marker ']  # workaround bug in stdout handl wrapper
+    rc.command = ['echo', 'hello_world_marker']
     rc.runner_mode = runner_mode
     runner = Runner(config=rc)
     status, exitcode = runner.run()
@@ -168,7 +168,7 @@ def test_stdout_file_write(rc, runner_mode):
 
 @pytest.mark.parametrize('runner_mode', ['pexpect', 'subprocess'])
 def test_stdout_file_no_write(rc, runner_mode):
-    rc.command = ['echo', 'hello_world_marker ']  # workaround bug in stdout handl wrapper
+    rc.command = ['echo', 'hello_world_marker']
     rc.runner_mode = runner_mode
     rc.suppress_output_file = True
     runner = Runner(config=rc)
@@ -177,3 +177,14 @@ def test_stdout_file_no_write(rc, runner_mode):
     for filename in ('stdout', 'stderr'):
         stdout_path = Path(rc.artifact_dir) / filename
         assert not stdout_path.exists()
+
+
+@pytest.mark.parametrize('runner_mode', ['pexpect', 'subprocess'])
+def test_multiline_blank_write(rc, runner_mode):
+    rc.command = ['echo', 'hello_world_marker\n\n\n']
+    rc.runner_mode = runner_mode
+    runner = Runner(config=rc)
+    status, exitcode = runner.run()
+    assert status == 'successful'
+    stdout_path = Path(rc.artifact_dir) / 'stdout'
+    assert stdout_path.read_text() == 'hello_world_marker\n\n\n\n'  # one extra newline okay