Merge lp:~abentley/juju-ci-tools/more-py3-2 into lp:juju-ci-tools

[Aaron Bentley]

Improve Python3 compatibility of jujupy.

This branch improves the Python3 compatibility of jujupy, along the same lines as the previous branch.

[Curtis Hovey (sinzui)]

Thank you.

review: Approve (code)

=== modified file 'jujupy/client.py'
--- jujupy/client.py	2017-03-10 19:57:45 +0000
+++ jujupy/client.py	2017-03-10 19:57:46 +0000
@@ -1914,7 +1914,8 @@
 
     def get_env_option(self, option):
         """Return the value of the environment's configured option."""
-        return self.get_juju_output('model-config', option)
+        return self.get_juju_output(
+            'model-config', option).decode(getpreferredencoding())
 
     def set_env_option(self, option, value):
         """Set the value of the option in the environment."""
@@ -2474,7 +2475,8 @@
             log.info(e.output)
             raise
         log.info(output)
-        backup_file_pattern = re.compile('(juju-backup-[0-9-]+\.(t|tar.)gz)')
+        backup_file_pattern = re.compile(
+            '(juju-backup-[0-9-]+\.(t|tar.)gz)'.encode('ascii'))
         match = backup_file_pattern.search(output)
         if match is None:
             raise Exception("The backup file was not found in output: %s" %
@@ -2482,7 +2484,7 @@
         backup_file_name = match.group(1)
         backup_file_path = os.path.abspath(backup_file_name)
         log.info("State-Server backup at %s", backup_file_path)
-        return backup_file_path
+        return backup_file_path.decode(getpreferredencoding())
 
     def restore_backup(self, backup_file):
         self.juju(

=== modified file 'jujupy/fake.py'
--- jujupy/fake.py	2017-03-10 19:57:45 +0000
+++ jujupy/fake.py	2017-03-10 19:57:46 +0000
@@ -1050,7 +1050,7 @@
 
 
 def get_user_register_token(username):
-    return b64encode(sha512(username).digest())
+    return b64encode(sha512(username.encode('utf-8')).digest()).decode('ascii')
 
 
 class FakeBackend2_1(FakeBackend):

=== modified file 'jujupy/tests/test_client.py'
--- jujupy/tests/test_client.py	2017-03-10 19:57:45 +0000
+++ jujupy/tests/test_client.py	2017-03-10 19:57:46 +0000
@@ -337,9 +337,9 @@
         self.assertEqual(300, conditions.timeout)
 
     def test_iter_blocking_state(self):
-        mock_ab = Mock()
+        mock_ab = Mock(timeout=0)
         mock_ab.iter_blocking_state.return_value = [('a', 'b')]
-        mock_cd = Mock()
+        mock_cd = Mock(timeout=0)
         mock_cd.iter_blocking_state.return_value = [('c', 'd')]
         conditions = ConditionList([mock_ab, mock_cd])
         self.assertEqual([('a', 'b'), ('c', 'd')],
@@ -425,14 +425,15 @@
         self.assertEqual(client.env.juju_home, 'baz')
 
     def test_get_version(self):
-        value = ' 5.6 \n'
+        value = ' 5.6 \n'.encode('ascii')
         with patch('subprocess.check_output', return_value=value) as vsn:
             version = ModelClient.get_version()
         self.assertEqual('5.6', version)
         vsn.assert_called_with(('juju', '--version'))
 
     def test_get_version_path(self):
-        with patch('subprocess.check_output', return_value=' 4.3') as vsn:
+        with patch('subprocess.check_output',
+                   return_value=' 4.3'.encode('ascii')) as vsn:
             ModelClient.get_version('foo/bar/baz')
         vsn.assert_called_once_with(('foo/bar/baz', '--version'))
 
@@ -982,7 +983,7 @@
         fake_popen = FakePopen('asdf', None, 0)
         with patch('subprocess.Popen', return_value=fake_popen) as mock:
             result = client.get_juju_output('bar')
-        self.assertEqual('asdf', result)
+        self.assertEqual('asdf'.encode('ascii'), result)
         self.assertEqual((('juju', '--show-log', 'bar', '-m', 'foo:foo'),),
                          mock.call_args[0])
 
@@ -992,7 +993,7 @@
         client = ModelClient(env, None, 'juju')
         with patch('subprocess.Popen', return_value=fake_popen) as mock:
             result = client.get_juju_output('bar', 'baz', '--qux')
-        self.assertEqual('asdf', result)
+        self.assertEqual('asdf'.encode('ascii'), result)
         self.assertEqual((('juju', '--show-log', 'bar', '-m', 'foo:foo', 'baz',
                            '--qux'),), mock.call_args[0])
 
@@ -1003,7 +1004,7 @@
         with self.assertRaises(subprocess.CalledProcessError) as exc:
             with patch('subprocess.Popen', return_value=fake_popen):
                 client.get_juju_output('bar')
-        self.assertEqual(exc.exception.stderr, 'Hello!')
+        self.assertEqual(exc.exception.stderr, 'Hello!'.encode('ascii'))
 
     def test_get_juju_output_merge_stderr(self):
         env = JujuData('foo')
@@ -1011,7 +1012,7 @@
         client = ModelClient(env, None, 'juju')
         with patch('subprocess.Popen', return_value=fake_popen) as mock_popen:
             result = client.get_juju_output('bar', merge_stderr=True)
-        self.assertEqual(result, 'Err on out')
+        self.assertEqual(result, 'Err on out'.encode('ascii'))
         mock_popen.assert_called_once_with(
             ('juju', '--show-log', 'bar', '-m', 'foo:foo'),
             stdin=subprocess.PIPE, stderr=subprocess.STDOUT,
@@ -1062,7 +1063,7 @@
                 - a
                 - b
                 - c
-                """)
+                """).encode('ascii')
         env = JujuData('foo')
         client = ModelClient(env, None, None)
         with patch.object(client, 'get_juju_output',
@@ -1080,7 +1081,7 @@
 
         def get_juju_output(command, *args, **kwargs):
             if client.attempt == 1:
-                return '"hello"'
+                return '"hello"'.encode('ascii')
             client.attempt += 1
             raise subprocess.CalledProcessError(1, command)
 
@@ -1370,7 +1371,8 @@
         with patch.object(client, 'get_juju_output', return_value=status_txt):
             result = list(client.status_until(-1))
         self.assertEqual(
-            [r.status for r in result], [Status.from_text(status_txt).status])
+            [r.status for r in result],
+            [Status.from_text(status_txt.decode('ascii')).status])
 
     def test_status_until_timeout(self):
         client = ModelClient(
@@ -1710,7 +1712,8 @@
                         'Timed out waiting for agents to start in local'):
                     client.wait_for_started(0)
             self.assertEqual(writes, ['pending: 0', '\n'])
-        self.assertEqual(self.log_stream.getvalue(), 'ERROR %s\n' % value)
+        self.assertEqual(
+            self.log_stream.getvalue(), 'ERROR %s\n' % value.decode('ascii'))
 
     def test_wait_for_subordinate_units(self):
         value = dedent("""\
@@ -1732,7 +1735,7 @@
                         agent-state: started
                       sub3/0:
                         agent-state: started
-        """)
+        """).encode('ascii')
         client = ModelClient(JujuData('local'), None, None)
         now = datetime.now() + timedelta(days=1)
         with patch('utility.until_timeout.now', return_value=now):
@@ -1770,7 +1773,7 @@
                       sub3/0:
                         agent-status:
                           current: idle
-        """)
+        """).encode('ascii')
         client = ModelClient(JujuData('local'), None, None)
         now = datetime.now() + timedelta(days=1)
         with patch('utility.until_timeout.now', return_value=now):
@@ -1801,7 +1804,7 @@
                     subordinates:
                       sub/1:
                         agent-state: started
-        """)
+        """).encode('ascii')
         client = ModelClient(JujuData('local'), None, None)
         now = datetime.now() + timedelta(days=1)
         with patch('utility.until_timeout.now', return_value=now):
@@ -1828,7 +1831,7 @@
                     subordinates:
                       sub1:
                         agent-state: started
-        """)
+        """).encode('ascii')
         client = ModelClient(JujuData('local'), None, None)
         now = datetime.now() + timedelta(days=1)
         with patch('utility.until_timeout.now', return_value=now):
@@ -1849,7 +1852,7 @@
                 units:
                   jenkins/0:
                     agent-state: started
-        """)
+        """).encode('ascii')
         client = ModelClient(JujuData('local'), None, None)
         now = datetime.now() + timedelta(days=1)
         with patch('utility.until_timeout.now', return_value=now):
@@ -2281,7 +2284,7 @@
         return client.get_controller_client()
 
     def test_wait_for_ha(self):
-        value = yaml.safe_dump(self.make_ha_status())
+        value = yaml.safe_dump(self.make_ha_status()).encode('ascii')
         client = self.make_controller_client()
         with patch.object(client, 'get_juju_output',
                           return_value=value) as gjo_mock:
@@ -2339,7 +2342,7 @@
                 '1': {'agent-state-info': 'error: foo'},
             },
             'services': {},
-        })
+        }).encode('ascii')
         client = self.make_controller_client()
         with patch('jujupy.client.until_timeout', autospec=True,
                    return_value=[2, 1]):
@@ -2371,7 +2374,7 @@
                     }
                 }
             }
-        })
+        }).encode('ascii')
         client = ModelClient(JujuData('local'), None, None)
         with patch.object(client, 'get_juju_output', return_value=value):
             client.wait_for_deploy_started()
@@ -2476,7 +2479,7 @@
             'machines': {
                 '0': {'agent-state': 'started'},
             },
-        })
+        }).encode('ascii')
         client = ModelClient(JujuData('local'), None, None)
         with patch.object(client, 'get_juju_output', return_value=value):
             client.wait_for(WaitMachineNotPresent('1'), quiet=True)
@@ -4735,7 +4738,7 @@
 
     def test_from_text(self):
         text = TestModelClient.make_status_yaml(
-            'agent-state', 'pending', 'horsefeathers')
+            'agent-state', 'pending', 'horsefeathers').decode('ascii')
         status = Status.from_text(text)
         self.assertEqual(status.status_text, text)
         self.assertEqual(status.status, {
@@ -5939,7 +5942,11 @@
         status = Status.from_text(self.machine_0_ipv6)
         fake_client = Mock(spec=['status_until'])
         fake_client.status_until.return_value = [status]
+        socket_error = socket.error
         with patch('jujupy.utility.socket', wraps=socket) as wrapped_socket:
+            # Must not convert socket.error into a Mock, because Mocks don't
+            # descend from BaseException
+            wrapped_socket.error = socket_error
             del wrapped_socket.inet_pton
             host = get_machine_dns_name(fake_client, '0')
         self.assertEqual(host, "2001:db8::3")

=== modified file 'jujupy/tests/test_version_client.py'
--- jujupy/tests/test_version_client.py	2017-03-10 19:57:45 +0000
+++ jujupy/tests/test_version_client.py	2017-03-10 19:57:46 +0000
@@ -657,7 +657,7 @@
         fake_popen = FakePopen('asdf', None, 0)
         with patch('subprocess.Popen', return_value=fake_popen) as mock:
             result = client.get_juju_output('bar')
-        self.assertEqual('asdf', result)
+        self.assertEqual('asdf', result.encode('ascii'))
         self.assertEqual((('juju', '--show-log', 'bar', '-e', 'foo'),),
                          mock.call_args[0])
 
@@ -667,7 +667,7 @@
         client = EnvJujuClient1X(env, None, 'juju')
         with patch('subprocess.Popen', return_value=fake_popen) as mock:
             result = client.get_juju_output('bar', 'baz', '--qux')
-        self.assertEqual('asdf', result)
+        self.assertEqual('asdf'.encode('ascii'), result)
         self.assertEqual((('juju', '--show-log', 'bar', '-e', 'foo', 'baz',
                            '--qux'),), mock.call_args[0])
 
@@ -678,8 +678,8 @@
         with self.assertRaises(subprocess.CalledProcessError) as exc:
             with patch('subprocess.Popen', return_value=fake_popen):
                 client.get_juju_output('bar')
-        self.assertEqual(exc.exception.output, 'Hello')
-        self.assertEqual(exc.exception.stderr, 'Error!')
+        self.assertEqual(exc.exception.output, 'Hello'.encode('ascii'))
+        self.assertEqual(exc.exception.stderr, 'Error!'.encode('ascii'))
 
     def test_get_juju_output_full_cmd(self):
         env = SimpleEnvironment('foo')
@@ -1046,7 +1046,7 @@
                     subordinates:
                       sub/1:
                         agent-state: started
-        """)
+        """).encode('ascii')
         client = EnvJujuClient1X(SimpleEnvironment('local'), None, None)
         now = datetime.now() + timedelta(days=1)
         with patch('utility.until_timeout.now', return_value=now):
@@ -1181,7 +1181,7 @@
                 '2': {'state-server-member-status': 'has-vote'},
             },
             'services': {},
-        })
+        }).encode('ascii')
         client = EnvJujuClient1X(SimpleEnvironment('local'), None, None)
         with patch.object(client, 'get_juju_output', return_value=value):
             client.wait_for_ha()
@@ -1194,7 +1194,7 @@
                 '2': {'state-server-member-status': 'no-vote'},
             },
             'services': {},
-        })
+        }).encode('ascii')
         client = EnvJujuClient1X(SimpleEnvironment('local'), None, None)
         with patch.object(client, 'get_juju_output', return_value=value):
             writes = []
@@ -1240,7 +1240,7 @@
                     }
                 }
             }
-        })
+        }).encode('ascii')
         client = EnvJujuClient1X(SimpleEnvironment('local'), None, None)
         with patch.object(client, 'get_juju_output', return_value=value):
             client.wait_for_deploy_started()

=== modified file 'jujupy/utility.py'
--- jujupy/utility.py	2017-02-22 18:05:47 +0000
+++ jujupy/utility.py	2017-03-10 19:57:46 +0000
@@ -168,9 +168,9 @@
 
 @contextmanager
 def temp_yaml_file(yaml_dict, encoding="utf-8"):
-    temp_file = NamedTemporaryFile(suffix='.yaml', delete=False)
+    temp_file_cxt = NamedTemporaryFile(suffix='.yaml', delete=False)
     try:
-        with temp_file:
+        with temp_file_cxt as temp_file:
             yaml.safe_dump(yaml_dict, temp_file, encoding=encoding)
         yield temp_file.name
     finally:

=== modified file 'jujupy/version_client.py'
--- jujupy/version_client.py	2017-03-01 19:02:24 +0000
+++ jujupy/version_client.py	2017-03-10 19:57:46 +0000
@@ -1,5 +1,6 @@
 from contextlib import contextmanager
 import json
+from locale import getpreferredencoding
 import logging
 import os
 import re
@@ -370,7 +371,8 @@
 
     def get_env_option(self, option):
         """Return the value of the environment's configured option."""
-        return self.get_juju_output('get-env', option)
+        return self.get_juju_output(
+            'get-env', option).decode(getpreferredencoding())
 
     def set_env_option(self, option, value):
         """Set the value of the option in the environment."""

=== modified file 'tests/__init__.py'
--- tests/__init__.py	2017-03-10 19:57:45 +0000
+++ tests/__init__.py	2017-03-10 19:57:46 +0000
@@ -173,8 +173,8 @@
     """Create an artifical version of the Popen class."""
 
     def __init__(self, out, err, returncode):
-        self._out = out
-        self._err = err
+        self._out = out if out is None else out.encode('ascii')
+        self._err = err if err is None else err.encode('ascii')
         self._code = returncode
 
     def communicate(self):
@@ -191,10 +191,16 @@
     temporary_file = NamedTemporaryFile(delete=False)
     try:
         with temporary_file as temp_file:
+
+            @contextmanager
+            def nt():
+                # This is used to prevent NamedTemporaryFile.close from being
+                # called.
+                yield temporary_file
+
             with patch('jujupy.utility.NamedTemporaryFile',
-                       return_value=temp_file):
-                with patch.object(temp_file, '__exit__'):
-                    yield temp_file
+                       return_value=nt()):
+                yield temp_file
     finally:
         # File may have already been deleted, e.g. by temp_yaml_file.
         with utility.skip_on_missing_file():