From d81ffef38d75313fb7c790e70911a1fd577036da Mon Sep 17 00:00:00 2001
From: Chenxiong Qi <cqi@redhat.com>
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 <cqi@redhat.com>

---

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,

