From d81ffef38d75313fb7c790e70911a1fd577036da Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: May 09 2018 07:43:22 +0000 Subject: Write mock config correctly when run in Py 3 Mock config is a plain text file, so it is unnecessary to write in binary mode. When run in Python 3, original code fails due to it writes a unicode string in binary mode without encode in advance. Tests are updated accordingly. Now, file operations like open and write are not mocked, which can catch any potential issues happening during those calls. Signed-off-by: Chenxiong Qi --- diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py index 4aa47da..664b727 100644 --- a/pyrpkg/__init__.py +++ b/pyrpkg/__init__.py @@ -16,6 +16,7 @@ import fnmatch import getpass import git import glob +import io import koji import logging import os @@ -2317,8 +2318,11 @@ class Commands(object): % error) config_file = os.path.join(config_dir, '%s.cfg' % root) + if not isinstance(config_content, six.text_type): + config_content = config_content.decode('utf-8') try: - open(config_file, 'wb').write(config_content) + with io.open(config_file, 'w', encoding='utf-8') as f: + f.write(config_content) except IOError as error: self._cleanup_tmp_dir(my_config_dir) raise rpkgError('Could not write config file: %s' % error) diff --git a/tests/test_commands.py b/tests/test_commands.py index e2ad992..c849cdb 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -1,14 +1,13 @@ # -*- coding: utf-8 -*- import errno +import io import os import shutil import six import subprocess import tempfile -from contextlib import contextmanager - import git import rpm from mock import call @@ -774,6 +773,7 @@ class TestConfigMockConfigDir(CommandTestCase): super(TestConfigMockConfigDir, self).setUp() self.cmd = self.make_commands() + self.temp_config_dir = tempfile.mkdtemp() self.fake_root = 'fedora-26-x86_64' self.mock_config_patcher = patch('pyrpkg.Commands.mock_config', @@ -781,58 +781,52 @@ class TestConfigMockConfigDir(CommandTestCase): self.mock_mock_config = self.mock_config_patcher.start() self.mkdtemp_patcher = patch('tempfile.mkdtemp', - return_value='/tmp/mockconfig/dir') + return_value=self.temp_config_dir) self.mock_mkdtemp = self.mkdtemp_patcher.start() def tearDown(self): + shutil.rmtree(self.temp_config_dir) self.mkdtemp_patcher.stop() self.mock_config_patcher.stop() super(TestConfigMockConfigDir, self).tearDown() - @contextmanager - def assert_file_op(self, filename, mode, write_data=None): - """Assert file object operation""" - with patch.object(six.moves.builtins, 'open', mock_open()) as mock: - yield - mock.assert_called_once_with(filename, mode) - if write_data is not None: - mock.return_value.write.assert_called_once_with(write_data) - def test_config_in_created_config_dir(self): - config_file = '{0}.cfg'.format( - os.path.join(self.mock_mkdtemp.return_value, self.fake_root)) + config_dir = self.cmd._config_dir_basic(root=self.fake_root) - with self.assert_file_op( - config_file, 'wb', - write_data=self.mock_mock_config.return_value): - config_dir = self.cmd._config_dir_basic(root=self.fake_root) - self.assertEqual(self.mock_mkdtemp.return_value, config_dir) + self.assertEqual(self.temp_config_dir, config_dir) - def test_config_in_specified_config_dir(self): - fake_config_dir = '/path/to/fake/config-dir' config_file = '{0}.cfg'.format( - os.path.join(fake_config_dir, self.fake_root)) + os.path.join(self.temp_config_dir, self.fake_root)) + with io.open(config_file, 'r', encoding='utf-8') as f: + self.assertEqual(self.mock_mock_config.return_value, + f.read().strip()) - with self.assert_file_op( - config_file, 'wb', - write_data=self.mock_mock_config.return_value): - config_dir = self.cmd._config_dir_basic(config_dir=fake_config_dir, - root=self.fake_root) - self.assertEqual(fake_config_dir, config_dir) + def test_config_in_specified_config_dir(self): + config_dir = self.cmd._config_dir_basic( + config_dir=self.temp_config_dir, + root=self.fake_root) + + self.assertEqual(self.temp_config_dir, config_dir) + + config_file = '{0}.cfg'.format( + os.path.join(self.temp_config_dir, self.fake_root)) + with io.open(config_file, 'r', encoding='utf-8') as f: + self.assertEqual(self.mock_mock_config.return_value, + f.read().strip()) @patch('pyrpkg.Commands.mockconfig', new_callable=PropertyMock) def test_config_using_root_guessed_from_branch(self, mockconfig): mockconfig.return_value = 'f26-candidate-i686' - config_file = '{0}.cfg'.format( - os.path.join(self.mock_mkdtemp.return_value, - mockconfig.return_value)) - with self.assert_file_op( - config_file, 'wb', - write_data=self.mock_mock_config.return_value): - config_dir = self.cmd._config_dir_basic() - self.assertEqual(self.mock_mkdtemp.return_value, - config_dir) + config_dir = self.cmd._config_dir_basic() + + self.assertEqual(self.temp_config_dir, config_dir) + + config_file = '{0}.cfg'.format( + os.path.join(self.temp_config_dir, mockconfig.return_value)) + with io.open(config_file, 'r', encoding='utf-8') as f: + self.assertEqual(self.mock_mock_config.return_value, + f.read().strip()) def test_fail_if_error_occurs_while_getting_mock_config(self): self.mock_mock_config.side_effect = rpkgError @@ -850,8 +844,8 @@ class TestConfigMockConfigDir(CommandTestCase): mock.assert_called_once_with(None) def test_fail_if_error_occurs_while_writing_cfg_file(self): - with patch.object(six.moves.builtins, 'open', mock_open()) as m: - m.return_value.write.side_effect = IOError + with patch.object(io, 'open') as m: + m.return_value.__enter__.return_value.write.side_effect = IOError with patch('pyrpkg.Commands._cleanup_tmp_dir') as mock: self.assertRaises(rpkgError,