#13 drop extra newline from stdin processing
Closed 6 years ago by pviktori. Opened 6 years ago by spoore.
spoore/python-pytest-multihost master  into  master

file modified
+2 -2
@@ -249,7 +249,7 @@ 

              command.stdin.write(encode(self.command_prelude))

  

          if stdin_text:

-             command.stdin.write(b"echo -e ")

+             command.stdin.write(b"echo -en ")

              command.stdin.write(_echo_quote(encode(stdin_text)))

              command.stdin.write(b" | ")

  
@@ -273,7 +273,7 @@ 

  

  

  def _echo_quote(bytestring):

-     """Encode a bytestring for use with bash & "echo -e"

+     """Encode a bytestring for use with bash & "echo -en"

      """

      bytestring = bytestring.replace(b"\\", br"\\")

      bytestring = bytestring.replace(b"\0", br"\x00")

@@ -246,7 +246,7 @@ 

              raiseonerr=False,

          )

          print(tee.stderr_text)

-         assert tee.stdout_text == stdin_text + '\n'

+         assert tee.stdout_text == stdin_text

          with open(test_file_path, "r") as f:

              assert f.read() == tee.stdout_text

  
@@ -262,10 +262,20 @@ 

              stdin_text=stdin_bytes,

              raiseonerr=False,

          )

-         assert tee.stdout_bytes == stdin_bytes + b'\n'

+         assert tee.stdout_bytes == stdin_bytes

          with open(test_file_path, "rb") as f:

              assert f.read() == tee.stdout_bytes

  

+     def test_piping_input(self, multihost, tmpdir):

+         host = multihost.host

+         b64 = host.run_command(['base64'], stdin_text='test')

+         assert b64.stdout_text == 'dGVzdA==' + '\n'

+ 

+     def test_piping_input_with_newline(self, multihost, tmpdir):

+         host = multihost.host

+         b64 = host.run_command(['base64'], stdin_text='test\n')

+         assert b64.stdout_text == 'dGVzdAo=' + '\n'

+ 

      def test_background_explicit_wait(self, multihost, tmpdir):

          host = multihost.host

  
@@ -278,7 +288,7 @@ 

          host.run_command('cat > ' + pipe_filename, stdin_text='expected value')

  

          cat.wait()

-         assert cat.stdout_text == 'expected value\n'

+         assert cat.stdout_text == 'expected value'

          assert cat.returncode == 0

  

      def test_background_context(self, multihost, tmpdir):
@@ -293,7 +303,7 @@ 

              host.run_command('cat > ' + pipe_filename,

                               stdin_text='expected value')

  

-         assert cat.stdout_text == 'expected value\n'

+         assert cat.stdout_text == 'expected value'

          assert cat.returncode == 0

  

  

https://pagure.io/python-pytest-multihost/issue/12

  • Modified run_command to add -n to echo
  • added test_piping_input
  • dropped newline from assert for:
  • test_escaping
  • test_escaping_binary
  • test_background_explicit_wait
  • test_background_context

echo -e used by stdin_text processing is appending an extra
newline character to stdin. For some commands this causes
problems.

$ echo -e test|base64
dGVzdAo=

vs:

$ echo -en test|base64
dGVzdA==

and:

$ echo -en test> test_file
$ cat test_file|base64
dGVzdA==

So if stdin is a string that doesn't already contain a newline,
we shouldn't append it as extra to the end of what we pass to
the command.

Signed-off-by: Scott Poore spoore@redhat.com

@slaykovsky @pviktori

Can you guys take a look at this? Will this break anything or can we merge this in?

Thanks

Technically speaking it is a breaking change. I'll bump major version. I'll get to it today or Monday.

Do we know if it will directly break something where @slaykovsky needed the encoding fixes and stdin changes?

No, but I think this is a genuine fix. If it breaks something, I would – with apologies to the affected people – rather see the calling code change.

ACK, merged. Thank you!

Pull-Request has been closed by pviktori

6 years ago

Released in 3.0, including packages for Fedora rawhide and F28.
Please test, and let me know if something is missing.