diff options
| -rw-r--r-- | .stestr.conf | 4 | ||||
| -rw-r--r-- | .testr.conf | 4 | ||||
| -rw-r--r-- | blacklist-py3.txt | 3 | ||||
| -rw-r--r-- | tox.ini | 13 | ||||
| -rw-r--r-- | trove/common/crypto_utils.py | 5 | ||||
| -rw-r--r-- | trove/common/stream_codecs.py | 48 | ||||
| -rw-r--r-- | trove/guestagent/common/operating_system.py | 64 | ||||
| -rw-r--r-- | trove/tests/unittests/guestagent/test_operating_system.py | 171 |
8 files changed, 242 insertions, 70 deletions
diff --git a/.stestr.conf b/.stestr.conf index f80c78cc..6d13b420 100644 --- a/.stestr.conf +++ b/.stestr.conf @@ -1,3 +1,3 @@ [DEFAULT] -test_path=${OS_TEST_PATH:-./trove/tests} -top_dir=./
\ No newline at end of file +test_path=${OS_TEST_PATH:-./trove/tests/unittests} +top_dir=./ diff --git a/.testr.conf b/.testr.conf deleted file mode 100644 index 9a613868..00000000 --- a/.testr.conf +++ /dev/null @@ -1,4 +0,0 @@ -[DEFAULT] -test_command=${PYTHON:-python} -m subunit.run discover ./trove/tests/unittests $LISTOPT $IDOPTION -test_id_option=--load-list $IDFILE -test_list_option=--list diff --git a/blacklist-py3.txt b/blacklist-py3.txt deleted file mode 100644 index c49759ed..00000000 --- a/blacklist-py3.txt +++ /dev/null @@ -1,3 +0,0 @@ -# Use a blacklist of tests known to fail on Python 3, until -# all unit tests will pass on Python 3. -guestagent.test_operating_system @@ -32,20 +32,21 @@ commands = flake8 doc8 {posargs} +[stestr-base] +commands = stestr run --serial '{posargs}' + stestr slowest + [testenv:py27] commands = {[testenv]commands} - ostestr {posargs} --slowest --serial - -[py3base] -commands = ostestr --slowest --blacklist-file=blacklist-py3.txt --serial --regex '.*' + {[stestr-base]commands} [testenv:py34] commands = {[testenv]commands} - {[py3base]commands} + {[stestr-base]commands} [testenv:py35] commands = {[testenv]commands} - {[py3base]commands} + {[stestr-base]commands} [testenv:apiexamples] commands = {envpython} generate_examples.py diff --git a/trove/common/crypto_utils.py b/trove/common/crypto_utils.py index 9e3d5613..2c61b3f8 100644 --- a/trove/common/crypto_utils.py +++ b/trove/common/crypto_utils.py @@ -31,8 +31,9 @@ IV_BIT_COUNT = 16 def encode_data(data): - if isinstance(data, six.text_type): - data = data.encode('utf-8') + # NOTE(zhaochao) No need to encoding string object any more, + # as Base64Codec is now using oslo_serialization.base64 which + # could take care of this. return stream_codecs.Base64Codec().serialize(data) diff --git a/trove/common/stream_codecs.py b/trove/common/stream_codecs.py index e6bb41b5..752dbb9a 100644 --- a/trove/common/stream_codecs.py +++ b/trove/common/stream_codecs.py @@ -15,7 +15,6 @@ import abc import ast -import base64 import csv import json import re @@ -26,6 +25,8 @@ from six.moves import configparser import xmltodict import yaml +from oslo_serialization import base64 + from trove.common import utils as trove_utils @@ -324,9 +325,14 @@ class PropertiesCodec(StreamCodec): key = row[0].strip() # Ignore comment lines. if not key.strip().startswith(self._comment_markers): - items = self._string_converter.to_objects( + # NOTE(zhaochao): a list object is expected for + # trove_utils.unpack_singleton, however in python3 + # map objects won't be treated as lists, so we + # convert the result of StringConverter.to_objects + # to a list explicitly. + items = list(self._string_converter.to_objects( [v if v else None for v in - map(self._strip_comments, row[1:])]) + map(self._strip_comments, row[1:])])) current = data_dict.get(key) if current is not None: current.append(trove_utils.unpack_singleton(items) @@ -360,9 +366,14 @@ class PropertiesCodec(StreamCodec): header, self._string_converter.to_strings(items))) else: # This is a single-row property with only one argument. + # Note(zhaochao): csv.writerows expects a list object before + # python 3.5, but map objects won't be treated as lists in + # python 3, so we explicitly convert the result of + # StringConverter.to_strings to a list here to support py34 + # unittests. rows.append( - self._string_converter.to_strings( - self._to_list(header, items))) + list(self._string_converter.to_strings( + self._to_list(header, items)))) return rows @@ -434,7 +445,14 @@ class KeyValueCodec(StreamCodec): return self._line_terminator.join(lines) def deserialize(self, stream): - lines = stream.split(self._line_terminator) + # Note(zhaochao): In Python 3, when files are opened in text mode, + # newlines will be translated to '\n' by default, so we just split + # the stream by '\n'. + if sys.version_info[0] >= 3: + lines = stream.split('\n') + else: + lines = stream.split(self._line_terminator) + result = {} for line in lines: line = line.lstrip().rstrip() @@ -500,22 +518,14 @@ class Base64Codec(StreamCodec): binary data before writing to a file as well. """ - def serialize(self, data): + # NOTE(zhaochao): migrate to oslo_serialization.base64 to serialize(return + # a text object) and deserialize(return a bytes object) data. - try: - # py27str - if we've got text data, this should encode it - # py27aa/py34aa - if we've got a bytearray, this should work too - encoded = str(base64.b64encode(data).decode('utf-8')) - except TypeError: - # py34str - convert to bytes first, then we can encode - data_bytes = bytes([ord(item) for item in data]) - encoded = base64.b64encode(data_bytes).decode('utf-8') - return encoded + def serialize(self, data): + return base64.encode_as_text(data) def deserialize(self, stream): - - # py27 & py34 seem to understand bytearray the same - return bytearray([item for item in base64.b64decode(stream)]) + return base64.decode_as_bytes(stream) class XmlCodec(StreamCodec): diff --git a/trove/guestagent/common/operating_system.py b/trove/guestagent/common/operating_system.py index 282d8857..a210fdf5 100644 --- a/trove/guestagent/common/operating_system.py +++ b/trove/guestagent/common/operating_system.py @@ -56,13 +56,18 @@ def read_file(path, codec=IdentityCodec(), as_root=False, decode=True): :raises: :class:`UnprocessableEntity` if codec not given. """ if path and exists(path, is_directory=False, as_root=as_root): + if decode: + open_flag = 'r' + convert_func = codec.deserialize + else: + open_flag = 'rb' + convert_func = codec.serialize + if as_root: - return _read_file_as_root(path, codec, decode=decode) + return _read_file_as_root(path, open_flag, convert_func) - with open(path, 'r') as fp: - if decode: - return codec.deserialize(fp.read()) - return codec.serialize(fp.read()) + with open(path, open_flag) as fp: + return convert_func(fp.read()) raise exception.UnprocessableEntity(_("File does not exist: %s") % path) @@ -97,24 +102,22 @@ def exists(path, is_directory=False, as_root=False): return found -def _read_file_as_root(path, codec, decode=True): +def _read_file_as_root(path, open_flag, convert_func): """Read a file as root. :param path Path to the written file. :type path string - :param codec: A codec used to transform the data. - :type codec: StreamCodec + :param open_flag: The flag for opening a file + :type open_flag: string - :param decode: Should the codec decode the data. - :type decode: boolean + :param convert_func: The function for converting data. + :type convert_func: callable """ - with tempfile.NamedTemporaryFile() as fp: + with tempfile.NamedTemporaryFile(open_flag) as fp: copy(path, fp.name, force=True, dereference=True, as_root=True) chmod(fp.name, FileMode.ADD_READ_ALL(), as_root=True) - if decode: - return codec.deserialize(fp.read()) - return codec.serialize(fp.read()) + return convert_func(fp.read()) def write_file(path, data, codec=IdentityCodec(), as_root=False, encode=True): @@ -141,20 +144,24 @@ def write_file(path, data, codec=IdentityCodec(), as_root=False, encode=True): :raises: :class:`UnprocessableEntity` if path not given. """ if path: + if encode: + open_flag = 'w' + convert_func = codec.serialize + else: + open_flag = 'wb' + convert_func = codec.deserialize + if as_root: - _write_file_as_root(path, data, codec, encode=encode) + _write_file_as_root(path, data, open_flag, convert_func) else: - with open(path, 'w') as fp: - if encode: - fp.write(codec.serialize(data)) - else: - fp.write(codec.deserialize(data)) + with open(path, open_flag) as fp: + fp.write(convert_func(data)) fp.flush() else: raise exception.UnprocessableEntity(_("Invalid path: %s") % path) -def _write_file_as_root(path, data, codec, encode=True): +def _write_file_as_root(path, data, open_flag, convert_func): """Write a file as root. Overwrite any existing contents. :param path Path to the written file. @@ -163,19 +170,16 @@ def _write_file_as_root(path, data, codec, encode=True): :param data: An object representing the file contents. :type data: StreamCodec - :param codec: A codec used to transform the data. - :type codec: StreamCodec + :param open_flag: The flag for opening a file + :type open_flag: string - :param encode: Should the codec encode the data. - :type encode: boolean + :param convert_func: The function for converting data. + :type convert_func: callable """ # The files gets removed automatically once the managing object goes # out of scope. - with tempfile.NamedTemporaryFile('w', delete=False) as fp: - if encode: - fp.write(codec.serialize(data)) - else: - fp.write(codec.deserialize(data)) + with tempfile.NamedTemporaryFile(open_flag, delete=False) as fp: + fp.write(convert_func(data)) fp.flush() fp.close() # Release the resource before proceeding. copy(fp.name, path, force=True, as_root=True) diff --git a/trove/tests/unittests/guestagent/test_operating_system.py b/trove/tests/unittests/guestagent/test_operating_system.py index 9f7969b8..adb66fa6 100644 --- a/trove/tests/unittests/guestagent/test_operating_system.py +++ b/trove/tests/unittests/guestagent/test_operating_system.py @@ -18,7 +18,7 @@ import re import stat import tempfile -from mock import call, patch +from mock import call, patch, mock_open from oslo_concurrency.processutils import UnknownArgumentError import six from testtools import ExpectedException @@ -38,8 +38,12 @@ class TestOperatingSystem(trove_testtools.TestCase): def test_base64_codec(self): data = "Line 1\nLine 2\n" - self._test_file_codec(data, Base64Codec()) + # Base64Codec.deserialize returns bytes instead of string. + self._test_file_codec(data, Base64Codec(), + expected_data=data.encode('utf-8')) + # when encoding is reversed for Base64Codec, reading from files + # will call Base64Codec.serialize which returns string. data = "TGluZSAxCkxpbmUgMgo=" self._test_file_codec(data, Base64Codec(), reverse_encoding=True) @@ -192,7 +196,7 @@ class TestOperatingSystem(trove_testtools.TestCase): @patch.object(operating_system, 'copy') def test_write_file_as_root(self, copy_mock): target_file = tempfile.NamedTemporaryFile() - temp_file = tempfile.NamedTemporaryFile() + temp_file = tempfile.NamedTemporaryFile('w') with patch('tempfile.NamedTemporaryFile', return_value=temp_file): operating_system.write_file( @@ -205,13 +209,172 @@ class TestOperatingSystem(trove_testtools.TestCase): side_effect=Exception("Error while executing 'copy'.")) def test_write_file_as_root_with_error(self, copy_mock): target_file = tempfile.NamedTemporaryFile() - temp_file = tempfile.NamedTemporaryFile() + temp_file = tempfile.NamedTemporaryFile('w') with patch('tempfile.NamedTemporaryFile', return_value=temp_file): with ExpectedException(Exception, "Error while executing 'copy'."): operating_system.write_file(target_file.name, "Lorem Ipsum", as_root=True) self.assertFalse(os.path.exists(temp_file.name)) + @patch.object(operating_system, 'exists', return_value=True) + @patch.object(operating_system, 'copy') + @patch.object(operating_system, 'chmod') + @patch.object(IdentityCodec, 'deserialize') + @patch.object(IdentityCodec, 'serialize') + @patch.object(operating_system, 'open', + mock_open(read_data='MockingRead')) + def test_read_file_with_flags_and_conv_func(self, mock_serialize, + mock_deserialize, + mock_chmod, mock_copy, + *args): + test_path = '/path/of/file' + test_data = 'MockingRead' + # use getattr to avoid pylint 'no-member' warning + mock_file = getattr(operating_system, 'open') + + # simple read + operating_system.read_file(test_path) + mock_file.assert_called_once_with(test_path, 'r') + mock_file().read.assert_called_once() + mock_deserialize.called_once_with(test_data) + mock_file.reset_mock() + mock_deserialize.reset_mock() + + # read with decode=False + operating_system.read_file(test_path, decode=False) + mock_file.assert_called_once_with(test_path, 'rb') + mock_file().read.assert_called_once() + mock_serialize.called_once_with(test_data) + mock_file.reset_mock() + mock_serialize.reset_mock() + + # checking _read_file_as_root arguments + with patch.object(operating_system, + '_read_file_as_root') as mock_read_file_as_root: + # simple read as root, + operating_system.read_file(test_path, as_root=True) + mock_read_file_as_root.assert_called_once_with( + test_path, 'r', mock_deserialize) + mock_deserialize.assert_not_called() + mock_read_file_as_root.reset_mock() + + # read as root with decode=False, + operating_system.read_file(test_path, as_root=True, decode=False) + mock_read_file_as_root.assert_called_once_with( + test_path, 'rb', mock_serialize) + mock_serialize.assert_not_called() + + # simple read as root + temp_file = tempfile.NamedTemporaryFile('r') + with patch.object(tempfile, 'NamedTemporaryFile', + return_value=temp_file) as mock_temp_file: + operating_system.read_file(test_path, as_root=True) + mock_temp_file.assert_called_once_with('r') + mock_copy.called_once_with(test_path, temp_file.name, + force=True, dereference=True, + as_root=True) + mock_chmod.called_once_with(temp_file.name, + FileMode.ADD_READ_ALL(), + as_root=True) + mock_deserialize.assert_called_once_with('') + self.assertFalse(os.path.exists(temp_file.name)) + mock_copy.reset_mock() + mock_chmod.reset_mock() + mock_deserialize.reset_mock() + + # read as root with decode=False + temp_file = tempfile.NamedTemporaryFile('rb') + with patch.object(tempfile, 'NamedTemporaryFile', + return_value=temp_file) as mock_temp_file: + operating_system.read_file(test_path, as_root=True, + decode=False) + mock_temp_file.assert_called_once_with('rb') + mock_copy.called_once_with(test_path, temp_file.name, + force=True, dereference=True, + as_root=True) + mock_chmod.called_once_with(temp_file.name, + FileMode.ADD_READ_ALL(), + as_root=True) + mock_serialize.assert_called_once_with(b'') + self.assertFalse(os.path.exists(temp_file.name)) + + @patch.object(operating_system, 'copy') + @patch.object(operating_system, 'chmod') + @patch.object(IdentityCodec, 'deserialize', + return_value=b'DeseiralizedData') + @patch.object(IdentityCodec, 'serialize', + return_value='SerializedData') + @patch.object(operating_system, 'open', mock_open()) + def test_write_file_with_flags_and_conv_func(self, mock_serialize, + mock_deserialize, + mock_chmod, mock_copy): + test_path = '/path/of/file' + test_data = 'MockingWrite' + test_serialize = 'SerializedData' + test_deserialize = b'DeseiralizedData' + mock_file = getattr(operating_system, 'open') + + # simple write + operating_system.write_file(test_path, test_data) + mock_file.assert_called_once_with(test_path, 'w') + mock_serialize.called_once_with(test_data) + mock_file().write.assert_called_once_with(test_serialize) + mock_file().flush.assert_called_once() + mock_file.reset_mock() + mock_serialize.reset_mock() + + # write with encode=False + operating_system.write_file(test_path, test_data, encode=False) + mock_file.assert_called_once_with(test_path, 'wb') + mock_deserialize.called_once_with(test_data) + mock_file().write.assert_called_once_with(test_deserialize) + mock_file().flush.assert_called_once() + mock_file.reset_mock() + mock_deserialize.reset_mock() + + # checking _write_file_as_root arguments + with patch.object(operating_system, + '_write_file_as_root') as mock_write_file_as_root: + # simple write as root, + operating_system.write_file(test_path, test_data, as_root=True) + mock_write_file_as_root.assert_called_once_with( + test_path, test_data, 'w', mock_serialize) + mock_serialize.assert_not_called() + mock_write_file_as_root.reset_mock() + + # read as root with encode=False, + operating_system.write_file(test_path, test_data, + as_root=True, encode=False) + mock_write_file_as_root.assert_called_once_with( + test_path, test_data, 'wb', mock_deserialize) + mock_deserialize.assert_not_called() + + # simple write as root + temp_file = tempfile.NamedTemporaryFile('w') + with patch.object(tempfile, 'NamedTemporaryFile', + return_value=temp_file) as mock_temp_file: + operating_system.write_file(test_path, test_data, as_root=True) + mock_temp_file.assert_called_once_with('w', delete=False) + mock_serialize.assert_called_once_with(test_data) + mock_copy.called_once_with(temp_file.name, test_path, + force=True, as_root=True) + self.assertFalse(os.path.exists(temp_file.name)) + mock_copy.reset_mock() + mock_chmod.reset_mock() + mock_serialize.reset_mock() + + # write as root with decode=False + temp_file = tempfile.NamedTemporaryFile('wb') + with patch.object(tempfile, 'NamedTemporaryFile', + return_value=temp_file) as mock_temp_file: + operating_system.write_file(test_path, test_data, + as_root=True, encode=False) + mock_temp_file.assert_called_once_with('wb', delete=False) + mock_deserialize.assert_called_once_with(test_data) + mock_copy.called_once_with(temp_file.name, test_path, + force=True, as_root=True) + self.assertFalse(os.path.exists(temp_file.name)) + def test_start_service(self): self._assert_service_call(operating_system.start_service, 'cmd_start') |