Refactor ipautil.run

The ipautil.run function now returns an object with returncode and
output are accessible as attributes.

The stdout and stderr of all commands are logged (unless skip_output is given).

The stdout/stderr contents must be explicitly requested with a keyword
argument, otherwise they are None.
This is because in Python 3, the output needs to be decoded, and that can
fail if it's not decodable (human-readable) text.

The raw (bytes) output is always available from the result object,
as is "leniently" decoded output suitable for logging.

All calls are changed to reflect this.

A use of Popen in cainstance is changed to ipautil.run.

Reviewed-By: Jan Cholasta <jcholast@redhat.com>

2 files changed, 70 insertions, 8 deletions

diff --git a/ipatests/test_ipapython/test_ipautil.py b/ipatests/test_ipapython/test_ipautil.py
index d574bd809..f91b730c5 100644
--- a/ipatests/test_ipapython/test_ipautil.py
+++ b/ipatests/test_ipapython/test_ipautil.py
@@ -1,3 +1,5 @@
+# encoding: utf-8
+
 # Authors:
 #   Jan Cholasta <jcholast@redhat.com>
 #
@@ -417,3 +419,62 @@ class TestTimeParser(object):
         nose.tools.assert_equal(-30, time.tzinfo.minoffset)
         offset = time.tzinfo.utcoffset(time.tzinfo.dst())
         nose.tools.assert_equal(((24 - 9) * 60 * 60) - (30 * 60), offset.seconds)
+
+
+def test_run():
+    result = ipautil.run(['echo', 'foo\x02bar'],
+                         capture_output=True,
+                         capture_error=True)
+    assert result.returncode == 0
+    assert result.output == 'foo\x02bar\n'
+    assert result.raw_output == b'foo\x02bar\n'
+    assert result.error_output == ''
+    assert result.raw_error_output == b''
+
+
+def test_run_no_capture_output():
+    result = ipautil.run(['echo', 'foo\x02bar'])
+    assert result.returncode == 0
+    assert result.output is None
+    assert result.raw_output == b'foo\x02bar\n'
+    assert result.error_output is None
+    assert result.raw_error_output == b''
+
+
+def test_run_bytes():
+    result = ipautil.run(['echo', b'\x01\x02'], capture_output=True)
+    assert result.returncode == 0
+    assert result.output == b'\x01\x02\n'
+
+
+def test_run_decode():
+    result = ipautil.run(['echo', u'á'.encode('utf-8')],
+                         encoding='utf-8', capture_output=True)
+    assert result.returncode == 0
+    if six.PY3:
+        assert result.output == 'á\n'
+    else:
+        assert result.output == 'á\n'.encode('utf-8')
+
+
+def test_run_decode_bad():
+    if six.PY3:
+        with pytest.raises(UnicodeDecodeError):
+            ipautil.run(['echo', b'\xa0\xa1'],
+                        capture_output=True,
+                        encoding='utf-8')
+    else:
+        result = ipautil.run(['echo', '\xa0\xa1'],
+                             capture_output=True,
+                             encoding='utf-8')
+        assert result.returncode == 0
+        assert result.output == '\xa0\xa1\n'
+
+
+def test_backcompat():
+    result = out, err, rc = ipautil.run(['echo', 'foo\x02bar'],
+                                        capture_output=True,
+                                        capture_error=True)
+    assert rc is result.returncode
+    assert out is result.output
+    assert err is result.error_output
diff --git a/ipatests/test_ipapython/test_keyring.py b/ipatests/test_ipapython/test_keyring.py
index 84b9f0ffa..02fd29e8c 100644
--- a/ipatests/test_ipapython/test_keyring.py
+++ b/ipatests/test_ipapython/test_keyring.py
@@ -28,8 +28,8 @@ import pytest
 pytestmark = pytest.mark.tier0
 
 TEST_KEY = 'ipa_test'
-TEST_VALUE = 'abc123'
-UPDATE_VALUE = '123abc'
+TEST_VALUE = b'abc123'
+UPDATE_VALUE = b'123abc'
 
 SIZE_256 = 'abcdefgh' * 32
 SIZE_512 = 'abcdefgh' * 64
@@ -94,9 +94,10 @@ class test_keyring(object):
 
         # Now update it 10 times
         for i in range(10):
-            kernel_keyring.update_key(TEST_KEY, 'test %d' %  i)
+            value = ('test %d' % i).encode('ascii')
+            kernel_keyring.update_key(TEST_KEY, value)
             result = kernel_keyring.read_key(TEST_KEY)
-            assert(result == 'test %d' % i)
+            assert(result == value)
 
         kernel_keyring.del_key(TEST_KEY)
 
@@ -134,9 +135,9 @@ class test_keyring(object):
         """
         Test 512-bytes of data
         """
-        kernel_keyring.add_key(TEST_KEY, SIZE_512)
+        kernel_keyring.add_key(TEST_KEY, SIZE_512.encode('ascii'))
         result = kernel_keyring.read_key(TEST_KEY)
-        assert(result == SIZE_512)
+        assert(result == SIZE_512.encode('ascii'))
 
         kernel_keyring.del_key(TEST_KEY)
 
@@ -144,8 +145,8 @@ class test_keyring(object):
         """
         Test 1k bytes of data
         """
-        kernel_keyring.add_key(TEST_KEY, SIZE_1024)
+        kernel_keyring.add_key(TEST_KEY, SIZE_1024.encode('ascii'))
         result = kernel_keyring.read_key(TEST_KEY)
-        assert(result == SIZE_1024)
+        assert(result == SIZE_1024.encode('ascii'))
 
         kernel_keyring.del_key(TEST_KEY)