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

[Aaron Bentley]

Improve Python 3 compatibility.

This branch improves jujupy's compatibility with Python 3.

The bulk of this is due to string handling differences: We had been using string literals as expected values, where the actual value was a binary string. The simplest thing to do seems to be encoding the string to ascii.

The location of StringIO changed, so its imports had to be updated.

Python 3 dicts cannot be sorted, so in a few places I changed it to sort by application name, not sorting application data itself.

Similarly, in Python 3 the return value of dict.keys() is not a list, and is never equal to a list. So dict.keys() had to become list(dict.keys()).

Also, since iterator.next() is iterator.__next__ on Python 3, next(iterator) is used instead.

Python 2's TestCase.assertItemsEqual is replaced in Python 3 with TestCase.assertCountEqual. I forced the old name to work again for tests.TestCase.

jujupy.tests.test_client was importing make_resource_list from tests.test_assess_resources. This is a dependency inversion. It was also causing winrm to be imported, which failed. Rather than chase that down, I fixed the dependency inversion by having test_assess_resources import from test_client.

(I have discovered there are more dependency inversions, especially with juju-ci-tools/tests/__init__.py, but that will have to wait.)

[Curtis Hovey (sinzui)]

Thank you very much.

review: Approve (code)

=== modified file '.bzrignore'
--- .bzrignore	2017-02-24 18:06:58 +0000
+++ .bzrignore	2017-03-10 16:30:01 +0000
@@ -4,3 +4,4 @@
 /logs
 /repository
 ./.coverage
+__pycache__

=== modified file 'jujupy/client.py'
--- jujupy/client.py	2017-03-10 15:44:39 +0000
+++ jujupy/client.py	2017-03-10 16:30:01 +0000
@@ -987,7 +987,7 @@
 
     def get_unit(self, unit_name):
         """Return metadata about a unit."""
-        for service in sorted(self.get_applications().values()):
+        for name, service in sorted(self.get_applications().items()):
             if unit_name in service.get('units', {}):
                 return service['units'][unit_name]
         raise KeyError(unit_name)
@@ -996,8 +996,8 @@
         """Return subordinate metadata for a service_name."""
         services = self.get_applications()
         if service_name in services:
-            for unit in sorted(services[service_name].get(
-                    'units', {}).values()):
+            for name, unit in sorted(services[service_name].get(
+                    'units', {}).items()):
                 for sub_name, sub in unit.get('subordinates', {}).items():
                     yield sub_name, sub
 
@@ -2366,7 +2366,7 @@
                 if status is None:
                     continue
                 states.setdefault(status, []).append(machine)
-            if states.keys() == [desired_state]:
+            if list(states.keys()) == [desired_state]:
                 if len(states.get(desired_state, [])) >= 3:
                     return None
             return states

=== modified file 'jujupy/fake.py'
--- jujupy/fake.py	2017-03-02 17:14:13 +0000
+++ jujupy/fake.py	2017-03-10 16:30:01 +0000
@@ -390,7 +390,7 @@
         if type(pattern) is not list:
             pattern = [pattern]
         try:
-            prompt = self.prompts.next()
+            prompt = next(self.prompts)
         except StopIteration:
             if pexpect.EOF not in pattern:
                 raise

=== modified file 'jujupy/tests/test_client.py'
--- jujupy/tests/test_client.py	2017-03-09 22:57:32 +0000
+++ jujupy/tests/test_client.py	2017-03-10 16:30:01 +0000
@@ -9,7 +9,10 @@
 import logging
 import os
 import socket
-import StringIO
+try:
+    from StringIO import StringIO
+except ImportError:
+    from io import StringIO
 import subprocess
 import sys
 from textwrap import dedent
@@ -99,7 +102,6 @@
     observable_temp_file,
     TestCase,
     )
-from tests.test_assess_resources import make_resource_list
 from jujupy.utility import (
     JujuResourceTimeout,
     scoped_environ,
@@ -301,7 +303,7 @@
                                soft_deadline=None)
         with patch('subprocess.Popen') as mock_popen:
             mock_popen.return_value.communicate.return_value = (
-                'ctrl1:user1/model1\n', '')
+                'ctrl1:user1/model1\n'.encode('ascii'), '')
             mock_popen.return_value.returncode = 0
             result = backend.get_active_model('/foo/bar')
         self.assertEqual(('ctrl1', 'user1', 'model1'), result)
@@ -367,6 +369,21 @@
             not_present.do_raise('', None)
 
 
+def make_resource_list(service_app_id='applicationId'):
+    return {'resources': [{
+        'expected': {
+            'origin': 'upload', 'used': True, 'description': 'foo resource.',
+            'username': 'admin', 'resourceid': 'dummy-resource/foo',
+            'name': 'foo', service_app_id: 'dummy-resource', 'size': 27,
+            'fingerprint': '1234', 'type': 'file', 'path': 'foo.txt'},
+        'unit': {
+            'origin': 'upload', 'username': 'admin', 'used': True,
+            'name': 'foo', 'resourceid': 'dummy-resource/foo',
+            service_app_id: 'dummy-resource', 'fingerprint': '1234',
+            'path': 'foo.txt', 'size': 27, 'type': 'file',
+            'description': 'foo resource.'}}]}
+
+
 class TestModelClient(ClientTest):
 
     def test_get_full_path(self):
@@ -1147,7 +1164,7 @@
                 units:
                   jenkins/0:
                     {0}: {2}
-        """.format(key, machine_value, unit_value))
+        """.format(key, machine_value, unit_value)).encode('ascii')
 
     def test_deploy_non_joyent(self):
         env = ModelClient(
@@ -2278,7 +2295,8 @@
             client.wait_for_ha()
 
     def test_wait_for_ha_no_has_vote(self):
-        value = yaml.safe_dump(self.make_ha_status(voting='no-vote'))
+        value = yaml.safe_dump(
+            self.make_ha_status(voting='no-vote')).encode('ascii')
         client = self.make_controller_client()
         with patch.object(client, 'get_juju_output', return_value=value):
             writes = []
@@ -2469,7 +2487,7 @@
                 '0': {'agent-state': 'started'},
                 '1': {'agent-state': 'started'},
             },
-        })
+        }).encode('ascii')
         client = ModelClient(JujuData('local'), None, None)
         with patch.object(client, 'get_juju_output', return_value=value), \
             patch('jujupy.client.until_timeout',
@@ -5697,7 +5715,7 @@
 class TestGroupReporter(TestCase):
 
     def test_single(self):
-        sio = StringIO.StringIO()
+        sio = StringIO()
         reporter = GroupReporter(sio, "done")
         self.assertEqual(sio.getvalue(), "")
         reporter.update({"working": ["1"]})
@@ -5706,7 +5724,7 @@
         self.assertEqual(sio.getvalue(), "working: 1\n")
 
     def test_single_ticks(self):
-        sio = StringIO.StringIO()
+        sio = StringIO()
         reporter = GroupReporter(sio, "done")
         reporter.update({"working": ["1"]})
         self.assertEqual(sio.getvalue(), "working: 1")
@@ -5718,7 +5736,7 @@
         self.assertEqual(sio.getvalue(), "working: 1 ..\n")
 
     def test_multiple_values(self):
-        sio = StringIO.StringIO()
+        sio = StringIO()
         reporter = GroupReporter(sio, "done")
         reporter.update({"working": ["1", "2"]})
         self.assertEqual(sio.getvalue(), "working: 1, 2")
@@ -5728,7 +5746,7 @@
         self.assertEqual(sio.getvalue(), "working: 1, 2\nworking: 1\n")
 
     def test_multiple_groups(self):
-        sio = StringIO.StringIO()
+        sio = StringIO()
         reporter = GroupReporter(sio, "done")
         reporter.update({"working": ["1", "2"], "starting": ["3"]})
         first = "starting: 3 | working: 1, 2"
@@ -5740,7 +5758,7 @@
         self.assertEqual(sio.getvalue(), "\n".join([first, second, ""]))
 
     def test_finish(self):
-        sio = StringIO.StringIO()
+        sio = StringIO()
         reporter = GroupReporter(sio, "done")
         self.assertEqual(sio.getvalue(), "")
         reporter.update({"working": ["1"]})
@@ -5749,14 +5767,14 @@
         self.assertEqual(sio.getvalue(), "working: 1\n")
 
     def test_finish_unchanged(self):
-        sio = StringIO.StringIO()
+        sio = StringIO()
         reporter = GroupReporter(sio, "done")
         self.assertEqual(sio.getvalue(), "")
         reporter.finish()
         self.assertEqual(sio.getvalue(), "")
 
     def test_wrap_to_width(self):
-        sio = StringIO.StringIO()
+        sio = StringIO()
         reporter = GroupReporter(sio, "done")
         self.assertEqual(sio.getvalue(), "")
         for _ in range(150):
@@ -5769,7 +5787,7 @@
 """)
 
     def test_wrap_to_width_exact(self):
-        sio = StringIO.StringIO()
+        sio = StringIO()
         reporter = GroupReporter(sio, "done")
         reporter.wrap_width = 12
         self.assertEqual(sio.getvalue(), "")
@@ -5788,7 +5806,7 @@
         self.assertEqual(sio.getvalue(), changes[-1] + "\n")
 
     def test_wrap_to_width_overflow(self):
-        sio = StringIO.StringIO()
+        sio = StringIO()
         reporter = GroupReporter(sio, "done")
         reporter.wrap_width = 8
         self.assertEqual(sio.getvalue(), "")
@@ -5806,7 +5824,7 @@
         self.assertEqual(sio.getvalue(), changes[-1] + "\n")
 
     def test_wrap_to_width_multiple_groups(self):
-        sio = StringIO.StringIO()
+        sio = StringIO()
         reporter = GroupReporter(sio, "done")
         reporter.wrap_width = 16
         self.assertEqual(sio.getvalue(), "")

=== modified file 'jujupy/tests/test_version_client.py'
--- jujupy/tests/test_version_client.py	2017-03-01 19:02:24 +0000
+++ jujupy/tests/test_version_client.py	2017-03-10 16:30:01 +0000
@@ -804,7 +804,7 @@
                 units:
                   jenkins/0:
                     {0}: {2}
-        """.format(key, machine_value, unit_value))
+        """.format(key, machine_value, unit_value)).encode('ascii')
 
     def test_deploy_non_joyent(self):
         env = EnvJujuClient1X(
@@ -992,7 +992,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("""\
@@ -1014,7 +1015,7 @@
                         agent-state: started
                       sub3/0:
                         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):
@@ -1072,7 +1073,7 @@
                     subordinates:
                       sub1:
                         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):
@@ -1093,7 +1094,7 @@
                 units:
                   jenkins/0:
                     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):
@@ -1319,7 +1320,7 @@
             'machines': {
                 '0': {'agent-state': 'started'},
             },
-        })
+        }).encode('utf-8')
         client = EnvJujuClient1X(SimpleEnvironment('local'), None, None)
         with patch.object(client, 'get_juju_output', return_value=value):
             client.wait_for(WaitMachineNotPresent('1'), quiet=True)
@@ -1330,7 +1331,7 @@
                 '0': {'agent-state': 'started'},
                 '1': {'agent-state': 'started'},
             },
-        })
+        }).encode('ascii')
         client = EnvJujuClient1X(SimpleEnvironment('local'), None, None)
         with patch.object(client, 'get_juju_output', return_value=value), \
             patch('jujupy.client.until_timeout',
@@ -1865,7 +1866,7 @@
     def test_action_fetch_timeout(self):
         client = EnvJujuClient1X(SimpleEnvironment(None, {'type': 'local'}),
                                  '1.23-series-arch', None)
-        ret = "status: pending\nfoo: bar"
+        ret = "status: pending\nfoo: bar".encode('ascii')
         with patch.object(EnvJujuClient1X,
                           'get_juju_output', return_value=ret):
             with self.assertRaisesRegexp(Exception,
@@ -2268,7 +2269,7 @@
 
     def test_iter_status_data(self):
         iterator = self.run_iter_status()
-        self.assertEqual(iterator.next().status,
+        self.assertEqual(next(iterator).status,
                          dict(current='started', message='all good',
                               version='1.25.1'))
 

=== modified file 'tests/__init__.py'
--- tests/__init__.py	2017-02-22 18:02:19 +0000
+++ tests/__init__.py	2017-03-10 16:30:01 +0000
@@ -82,6 +82,10 @@
         return context.__enter__()
 
 
+if getattr(TestCase, 'assertItemsEqual', None) is None:
+    TestCase.assertItemsEqual = TestCase.assertCountEqual
+
+
 class FakeHomeTestCase(TestCase):
     """FakeHomeTestCase creates an isolated home dir for Juju to use."""
 

=== modified file 'tests/test_assess_resources.py'
--- tests/test_assess_resources.py	2017-02-25 06:09:40 +0000
+++ tests/test_assess_resources.py	2017-03-10 16:30:01 +0000
@@ -1,7 +1,10 @@
 import logging
 from argparse import Namespace
 from mock import Mock, patch, call
-import StringIO
+try:
+    from StringIO import StringIO
+except ImportError:
+    from io import StringIO
 
 from assess_resources import (
     assess_resources,
@@ -11,6 +14,7 @@
     main,
     verify_status,
 )
+from jujupy.tests.test_client import make_resource_list
 from tests import (
     parse_error,
     TestCase,
@@ -33,7 +37,7 @@
         self.assertEqual(False, args.debug)
 
     def test_help(self):
-        fake_stdout = StringIO.StringIO()
+        fake_stdout = StringIO()
         with parse_error(self) as fake_stderr:
             with patch("sys.stdout", fake_stdout):
                 parse_args(["--help"])
@@ -225,20 +229,5 @@
         deadline=None,)
 
 
-def make_resource_list(service_app_id='applicationId'):
-    return {'resources': [{
-        'expected': {
-            'origin': 'upload', 'used': True, 'description': 'foo resource.',
-            'username': 'admin', 'resourceid': 'dummy-resource/foo',
-            'name': 'foo', service_app_id: 'dummy-resource', 'size': 27,
-            'fingerprint': '1234', 'type': 'file', 'path': 'foo.txt'},
-        'unit': {
-            'origin': 'upload', 'username': 'admin', 'used': True,
-            'name': 'foo', 'resourceid': 'dummy-resource/foo',
-            service_app_id: 'dummy-resource', 'fingerprint': '1234',
-            'path': 'foo.txt', 'size': 27, 'type': 'file',
-            'description': 'foo resource.'}}]}
-
-
 class FakeFile:
     name = '/tmp/fake'