pylint: Fix pylint warnings reported in pylint 2.0.0.

Pylint 2.0.0 was recently released and complains more about
logging-not-lazy than it used to.  I've fixed those warnings, here.

The changes in rh_subscription are more extensive.  pylint may be
complaining incorrectly there, but the tests were not correctly un-doing
all of their mock/patching.  This cleans those up and makes pylint happy.

1 files changed, 92 insertions, 93 deletions

diff --git a/tests/unittests/test_rh_subscription.py b/tests/unittests/test_rh_subscription.py
index 22718108..4cd27eed 100644
--- a/tests/unittests/test_rh_subscription.py
+++ b/tests/unittests/test_rh_subscription.py
@@ -8,10 +8,16 @@ import logging
 from cloudinit.config import cc_rh_subscription
 from cloudinit import util
 
-from cloudinit.tests.helpers import TestCase, mock
+from cloudinit.tests.helpers import CiTestCase, mock
 
+SUBMGR = cc_rh_subscription.SubscriptionManager
+SUB_MAN_CLI = 'cloudinit.config.cc_rh_subscription._sub_man_cli'
+
+
+@mock.patch(SUB_MAN_CLI)
+class GoodTests(CiTestCase):
+    with_logs = True
 
-class GoodTests(TestCase):
     def setUp(self):
         super(GoodTests, self).setUp()
         self.name = "cc_rh_subscription"
@@ -19,7 +25,6 @@ class GoodTests(TestCase):
         self.log = logging.getLogger("good_tests")
         self.args = []
         self.handle = cc_rh_subscription.handle
-        self.SM = cc_rh_subscription.SubscriptionManager
 
         self.config = {'rh_subscription':
                        {'username': 'scooby@do.com',
@@ -35,55 +40,47 @@ class GoodTests(TestCase):
                              'disable-repo': ['repo4', 'repo5']
                              }}
 
-    def test_already_registered(self):
+    def test_already_registered(self, m_sman_cli):
         '''
         Emulates a system that is already registered. Ensure it gets
         a non-ProcessExecution error from is_registered()
         '''
-        with mock.patch.object(cc_rh_subscription.SubscriptionManager,
-                               '_sub_man_cli') as mockobj:
-            self.SM.log_success = mock.MagicMock()
-            self.handle(self.name, self.config, self.cloud_init,
-                        self.log, self.args)
-            self.assertEqual(self.SM.log_success.call_count, 1)
-            self.assertEqual(mockobj.call_count, 1)
-
-    def test_simple_registration(self):
+        self.handle(self.name, self.config, self.cloud_init,
+                    self.log, self.args)
+        self.assertEqual(m_sman_cli.call_count, 1)
+        self.assertIn('System is already registered', self.logs.getvalue())
+
+    def test_simple_registration(self, m_sman_cli):
         '''
         Simple registration with username and password
         '''
-        self.SM.log_success = mock.MagicMock()
         reg = "The system has been registered with ID:" \
               " 12345678-abde-abcde-1234-1234567890abc"
-        self.SM._sub_man_cli = mock.MagicMock(
-            side_effect=[util.ProcessExecutionError, (reg, 'bar')])
+        m_sman_cli.side_effect = [util.ProcessExecutionError, (reg, 'bar')]
         self.handle(self.name, self.config, self.cloud_init,
                     self.log, self.args)
-        self.assertIn(mock.call(['identity']),
-                      self.SM._sub_man_cli.call_args_list)
+        self.assertIn(mock.call(['identity']), m_sman_cli.call_args_list)
         self.assertIn(mock.call(['register', '--username=scooby@do.com',
                                  '--password=scooby-snacks'],
                                 logstring_val=True),
-                      self.SM._sub_man_cli.call_args_list)
-
-        self.assertEqual(self.SM.log_success.call_count, 1)
-        self.assertEqual(self.SM._sub_man_cli.call_count, 2)
+                      m_sman_cli.call_args_list)
+        self.assertIn('rh_subscription plugin completed successfully',
+                      self.logs.getvalue())
+        self.assertEqual(m_sman_cli.call_count, 2)
 
     @mock.patch.object(cc_rh_subscription.SubscriptionManager, "_getRepos")
-    @mock.patch.object(cc_rh_subscription.SubscriptionManager, "_sub_man_cli")
-    def test_update_repos_disable_with_none(self, m_sub_man_cli, m_get_repos):
+    def test_update_repos_disable_with_none(self, m_get_repos, m_sman_cli):
         cfg = copy.deepcopy(self.config)
         m_get_repos.return_value = ([], ['repo1'])
-        m_sub_man_cli.return_value = (b'', b'')
         cfg['rh_subscription'].update(
             {'enable-repo': ['repo1'], 'disable-repo': None})
         mysm = cc_rh_subscription.SubscriptionManager(cfg)
         self.assertEqual(True, mysm.update_repos())
         m_get_repos.assert_called_with()
-        self.assertEqual(m_sub_man_cli.call_args_list,
+        self.assertEqual(m_sman_cli.call_args_list,
                          [mock.call(['repos', '--enable=repo1'])])
 
-    def test_full_registration(self):
+    def test_full_registration(self, m_sman_cli):
         '''
         Registration with auto-attach, service-level, adding pools,
         and enabling and disabling yum repos
@@ -93,26 +90,28 @@ class GoodTests(TestCase):
         call_lists.append(['repos', '--disable=repo5', '--enable=repo2',
                            '--enable=repo3'])
         call_lists.append(['attach', '--auto', '--servicelevel=self-support'])
-        self.SM.log_success = mock.MagicMock()
         reg = "The system has been registered with ID:" \
               " 12345678-abde-abcde-1234-1234567890abc"
-        self.SM._sub_man_cli = mock.MagicMock(
-            side_effect=[util.ProcessExecutionError, (reg, 'bar'),
-                         ('Service level set to: self-support', ''),
-                         ('pool1\npool3\n', ''), ('pool2\n', ''), ('', ''),
-                         ('Repo ID: repo1\nRepo ID: repo5\n', ''),
-                         ('Repo ID: repo2\nRepo ID: repo3\nRepo ID: '
-                          'repo4', ''),
-                         ('', '')])
+        m_sman_cli.side_effect = [
+            util.ProcessExecutionError,
+            (reg, 'bar'),
+            ('Service level set to: self-support', ''),
+            ('pool1\npool3\n', ''), ('pool2\n', ''), ('', ''),
+            ('Repo ID: repo1\nRepo ID: repo5\n', ''),
+            ('Repo ID: repo2\nRepo ID: repo3\nRepo ID: repo4', ''),
+            ('', '')]
         self.handle(self.name, self.config_full, self.cloud_init,
                     self.log, self.args)
+        self.assertEqual(m_sman_cli.call_count, 9)
         for call in call_lists:
-            self.assertIn(mock.call(call), self.SM._sub_man_cli.call_args_list)
-        self.assertEqual(self.SM.log_success.call_count, 1)
-        self.assertEqual(self.SM._sub_man_cli.call_count, 9)
+            self.assertIn(mock.call(call), m_sman_cli.call_args_list)
+        self.assertIn("rh_subscription plugin completed successfully",
+                      self.logs.getvalue())
 
 
-class TestBadInput(TestCase):
+@mock.patch(SUB_MAN_CLI)
+class TestBadInput(CiTestCase):
+    with_logs = True
     name = "cc_rh_subscription"
     cloud_init = None
     log = logging.getLogger("bad_tests")
@@ -155,81 +154,81 @@ class TestBadInput(TestCase):
         super(TestBadInput, self).setUp()
         self.handle = cc_rh_subscription.handle
 
-    def test_no_password(self):
-        '''
-        Attempt to register without the password key/value
-        '''
-        self.SM._sub_man_cli = mock.MagicMock(
-            side_effect=[util.ProcessExecutionError, (self.reg, 'bar')])
+    def assert_logged_warnings(self, warnings):
+        logs = self.logs.getvalue()
+        missing = [w for w in warnings if "WARNING: " + w not in logs]
+        self.assertEqual([], missing, "Missing expected warnings.")
+
+    def test_no_password(self, m_sman_cli):
+        '''Attempt to register without the password key/value.'''
+        m_sman_cli.side_effect = [util.ProcessExecutionError,
+                                  (self.reg, 'bar')]
         self.handle(self.name, self.config_no_password, self.cloud_init,
                     self.log, self.args)
-        self.assertEqual(self.SM._sub_man_cli.call_count, 0)
+        self.assertEqual(m_sman_cli.call_count, 0)
 
-    def test_no_org(self):
-        '''
-        Attempt to register without the org key/value
-        '''
-        self.input_is_missing_data(self.config_no_key)
-
-    def test_service_level_without_auto(self):
-        '''
-        Attempt to register using service-level without the auto-attach key
-        '''
-        self.SM.log_warn = mock.MagicMock()
-        self.SM._sub_man_cli = mock.MagicMock(
-            side_effect=[util.ProcessExecutionError, (self.reg, 'bar')])
+    def test_no_org(self, m_sman_cli):
+        '''Attempt to register without the org key/value.'''
+        m_sman_cli.side_effect = [util.ProcessExecutionError]
+        self.handle(self.name, self.config_no_key, self.cloud_init,
+                    self.log, self.args)
+        m_sman_cli.assert_called_with(['identity'])
+        self.assertEqual(m_sman_cli.call_count, 1)
+        self.assert_logged_warnings((
+            'Unable to register system due to incomplete information.',
+            'Use either activationkey and org *or* userid and password',
+            'Registration failed or did not run completely',
+            'rh_subscription plugin did not complete successfully'))
+
+    def test_service_level_without_auto(self, m_sman_cli):
+        '''Attempt to register using service-level without auto-attach key.'''
+        m_sman_cli.side_effect = [util.ProcessExecutionError,
+                                  (self.reg, 'bar')]
         self.handle(self.name, self.config_service, self.cloud_init,
                     self.log, self.args)
-        self.assertEqual(self.SM._sub_man_cli.call_count, 1)
-        self.assertEqual(self.SM.log_warn.call_count, 2)
+        self.assertEqual(m_sman_cli.call_count, 1)
+        self.assert_logged_warnings((
+            'The service-level key must be used in conjunction with ',
+            'rh_subscription plugin did not complete successfully'))
 
-    def test_pool_not_a_list(self):
+    def test_pool_not_a_list(self, m_sman_cli):
         '''
         Register with pools that are not in the format of a list
         '''
-        self.SM.log_warn = mock.MagicMock()
-        self.SM._sub_man_cli = mock.MagicMock(
-            side_effect=[util.ProcessExecutionError, (self.reg, 'bar')])
+        m_sman_cli.side_effect = [util.ProcessExecutionError,
+                                  (self.reg, 'bar')]
         self.handle(self.name, self.config_badpool, self.cloud_init,
                     self.log, self.args)
-        self.assertEqual(self.SM._sub_man_cli.call_count, 2)
-        self.assertEqual(self.SM.log_warn.call_count, 2)
+        self.assertEqual(m_sman_cli.call_count, 2)
+        self.assert_logged_warnings((
+            'Pools must in the format of a list',
+            'rh_subscription plugin did not complete successfully'))
 
-    def test_repo_not_a_list(self):
+    def test_repo_not_a_list(self, m_sman_cli):
         '''
         Register with repos that are not in the format of a list
         '''
-        self.SM.log_warn = mock.MagicMock()
-        self.SM._sub_man_cli = mock.MagicMock(
-            side_effect=[util.ProcessExecutionError, (self.reg, 'bar')])
+        m_sman_cli.side_effect = [util.ProcessExecutionError,
+                                  (self.reg, 'bar')]
         self.handle(self.name, self.config_badrepo, self.cloud_init,
                     self.log, self.args)
-        self.assertEqual(self.SM.log_warn.call_count, 3)
-        self.assertEqual(self.SM._sub_man_cli.call_count, 2)
+        self.assertEqual(m_sman_cli.call_count, 2)
+        self.assert_logged_warnings((
+            'Repo IDs must in the format of a list.',
+            'Unable to add or remove repos',
+            'rh_subscription plugin did not complete successfully'))
 
-    def test_bad_key_value(self):
+    def test_bad_key_value(self, m_sman_cli):
         '''
         Attempt to register with a key that we don't know
         '''
-        self.SM.log_warn = mock.MagicMock()
-        self.SM._sub_man_cli = mock.MagicMock(
-            side_effect=[util.ProcessExecutionError, (self.reg, 'bar')])
+        m_sman_cli.side_effect = [util.ProcessExecutionError,
+                                  (self.reg, 'bar')]
         self.handle(self.name, self.config_badkey, self.cloud_init,
                     self.log, self.args)
-        self.assertEqual(self.SM.log_warn.call_count, 2)
-        self.assertEqual(self.SM._sub_man_cli.call_count, 1)
-
-    def input_is_missing_data(self, config):
-        '''
-        Helper def for tests that having missing information
-        '''
-        self.SM.log_warn = mock.MagicMock()
-        self.SM._sub_man_cli = mock.MagicMock(
-            side_effect=[util.ProcessExecutionError])
-        self.handle(self.name, config, self.cloud_init,
-                    self.log, self.args)
-        self.SM._sub_man_cli.assert_called_with(['identity'])
-        self.assertEqual(self.SM.log_warn.call_count, 4)
-        self.assertEqual(self.SM._sub_man_cli.call_count, 1)
+        self.assertEqual(m_sman_cli.call_count, 1)
+        self.assert_logged_warnings((
+            'fookey is not a valid key for rh_subscription. Valid keys are:',
+            'rh_subscription plugin did not complete successfully'))
 
 # vi: ts=4 expandtab