Fix pylint warnings/errors in cli

cli_testcase and cli_tier have been refactored to avoid duplicating
code. Then functest/cli and funtest/unit/cli can be added to the list
of modules rated 10/10.

Change-Id: Iec90e806397248a10f39080ec554e3f0a6eda7c1
Signed-off-by: Cédric Ollivier <cedric.ollivier@orange.com>

10 files changed, 111 insertions, 142 deletions

diff --git a/functest/cli/cli_base.py b/functest/cli/cli_base.py
index 5890e0a3..1a057e1b 100644
--- a/functest/cli/cli_base.py
+++ b/functest/cli/cli_base.py
@@ -5,12 +5,14 @@
 # are made available under the terms of the Apache License, Version 2.0
 # which accompanies this distribution, and is available at
 # http://www.apache.org/licenses/LICENSE-2.0
-#
 
-import click
+# pylint: disable=missing-docstring
+
 import logging.config
 import pkg_resources
 
+import click
+
 from functest.cli.commands.cli_env import CliEnv
 from functest.cli.commands.cli_os import CliOpenStack
 from functest.cli.commands.cli_testcase import CliTestcase
@@ -28,62 +30,62 @@ def cli():
     logging.captureWarnings(True)
 
 
-_env = CliEnv()
-_openstack = CliOpenStack()
-_testcase = CliTestcase()
-_tier = CliTier()
+ENV = CliEnv()
+OPENSTACK = CliOpenStack()
+TESTCASE = CliTestcase()
+TIER = CliTier()
 
 
 @cli.group()
 @click.pass_context
-def env(ctx):
+def env(ctx):  # pylint: disable=unused-argument
     pass
 
 
 @cli.group()
 @click.pass_context
-def openstack(ctx):
+def openstack(ctx):  # pylint: disable=unused-argument
     pass
 
 
 @cli.group()
 @click.pass_context
-def testcase(ctx):
+def testcase(ctx):  # pylint: disable=unused-argument
     pass
 
 
 @cli.group()
 @click.pass_context
-def tier(ctx):
+def tier(ctx):  # pylint: disable=unused-argument
     pass
 
 
 @openstack.command('check', help="Checks connectivity and status "
                    "to the OpenStack deployment.")
 def os_check():
-    _openstack.check()
+    OPENSTACK.check()
 
 
 @openstack.command('show-credentials',
                    help="Prints the OpenStack credentials.")
 def os_show_credentials():
-    _openstack.show_credentials()
+    OPENSTACK.show_credentials()
 
 
 @env.command('show', help="Shows information about the current environment.")
 def env_show():
-    _env.show()
+    ENV.show()
 
 
 @testcase.command('list', help="Lists the available testcases.")
 def testcase_list():
-    _testcase.list()
+    TESTCASE.list()
 
 
 @testcase.command('show', help="Shows information about a test case.")
 @click.argument('testname', type=click.STRING, required=True)
 def testcase_show(testname):
-    _testcase.show(testname)
+    TESTCASE.show(testname)
 
 
 @testcase.command('run', help="Executes a test case.")
@@ -95,24 +97,24 @@ def testcase_show(testname):
               help='Push results to the results DataBase. Only CI Pods'
               'have rights to do that.')
 def testcase_run(testname, noclean, report):
-    _testcase.run(testname, noclean, report)
+    TESTCASE.run(testname, noclean, report)
 
 
 @tier.command('list', help="Lists the available tiers.")
 def tier_list():
-    _tier.list()
+    TIER.list()
 
 
 @tier.command('show', help="Shows information about a tier.")
 @click.argument('tiername', type=click.STRING, required=True)
 def tier_show(tiername):
-    _tier.show(tiername)
+    TIER.show(tiername)
 
 
 @tier.command('get-tests', help="Prints the tests in a tier.")
 @click.argument('tiername', type=click.STRING, required=True)
 def tier_gettests(tiername):
-    _tier.gettests(tiername)
+    TIER.gettests(tiername)
 
 
 @tier.command('run', help="Executes all the tests within a tier.")
@@ -124,4 +126,4 @@ def tier_gettests(tiername):
               help='Push results to the results DataBase. Only CI Pods'
               'have rights to do that.')
 def tier_run(tiername, noclean, report):
-    _tier.run(tiername, noclean, report)
+    TIER.run(tiername, noclean, report)
diff --git a/functest/cli/commands/cli_env.py b/functest/cli/commands/cli_env.py
index c41f8f34..c1b66da4 100644
--- a/functest/cli/commands/cli_env.py
+++ b/functest/cli/commands/cli_env.py
@@ -5,7 +5,8 @@
 # are made available under the terms of the Apache License, Version 2.0
 # which accompanies this distribution, and is available at
 # http://www.apache.org/licenses/LICENSE-2.0
-#
+
+# pylint: disable=missing-docstring
 
 import click
 import prettytable
@@ -15,12 +16,10 @@ from functest.utils.constants import CONST
 import six
 
 
-class Env(object):
+class Env(object):  # pylint: disable=too-few-public-methods
 
-    def __init__(self):
-        pass
-
-    def show(self):
+    @staticmethod
+    def show():
         def _get_value(attr, default='Unknown'):
             return attr if attr else default
 
@@ -44,13 +43,11 @@ class Env(object):
         return env_info
 
 
-class CliEnv(Env):
-
-    def __init__(self):
-        super(CliEnv, self).__init__()
+class CliEnv(object):  # pylint: disable=too-few-public-methods
 
-    def show(self):
-        env_info = super(CliEnv, self).show()
+    @staticmethod
+    def show():
+        env_info = Env.show()
         msg = prettytable.PrettyTable(
             header_style='upper', padding_width=5,
             field_names=['Functest Environment', 'value'])
diff --git a/functest/cli/commands/cli_os.py b/functest/cli/commands/cli_os.py
index 9057da84..a543e845 100644
--- a/functest/cli/commands/cli_os.py
+++ b/functest/cli/commands/cli_os.py
@@ -5,13 +5,13 @@
 # are made available under the terms of the Apache License, Version 2.0
 # which accompanies this distribution, and is available at
 # http://www.apache.org/licenses/LICENSE-2.0
-#
 
+# pylint: disable=missing-docstring
 
 import os
 
 import click
-from six.moves.urllib.parse import urlparse
+from six.moves import urllib
 
 from functest.ci import check_deployment
 from functest.utils.constants import CONST
@@ -25,8 +25,8 @@ class OpenStack(object):
         self.endpoint_port = None
         self.openstack_creds = CONST.__getattribute__('openstack_creds')
         if self.os_auth_url:
-            self.endpoint_ip = urlparse(self.os_auth_url).hostname
-            self.endpoint_port = urlparse(self.os_auth_url).port
+            self.endpoint_ip = urllib.parse.urlparse(self.os_auth_url).hostname
+            self.endpoint_port = urllib.parse.urlparse(self.os_auth_url).port
 
     def ping_endpoint(self):
         if self.os_auth_url is None:
@@ -55,12 +55,9 @@ class OpenStack(object):
 
 class CliOpenStack(OpenStack):
 
-    def __init__(self):
-        super(CliOpenStack, self).__init__()
-
     @staticmethod
     def show_credentials():
         dic_credentials = OpenStack.show_credentials()
         for key, value in dic_credentials.items():
-                if key.startswith('OS_'):
-                    click.echo("{}={}".format(key, value))
+            if key.startswith('OS_'):
+                click.echo("{}={}".format(key, value))
diff --git a/functest/cli/commands/cli_testcase.py b/functest/cli/commands/cli_testcase.py
index a424a05b..a8ead5f5 100644
--- a/functest/cli/commands/cli_testcase.py
+++ b/functest/cli/commands/cli_testcase.py
@@ -5,26 +5,16 @@
 # are made available under the terms of the Apache License, Version 2.0
 # which accompanies this distribution, and is available at
 # http://www.apache.org/licenses/LICENSE-2.0
-#
-
-""" global variables """
 
-import pkg_resources
+# pylint: disable=missing-docstring
 
 import click
 
-import functest.ci.tier_builder as tb
-from functest.utils.constants import CONST
-import functest.utils.functest_utils as ft_utils
-
+from functest.cli.commands import cli_tier
+from functest.utils import functest_utils
 
-class Testcase(object):
 
-    def __init__(self):
-        self.tiers = tb.TierBuilder(
-            CONST.__getattribute__('INSTALLER_TYPE'),
-            CONST.__getattribute__('DEPLOY_SCENARIO'),
-            pkg_resources.resource_filename('functest', 'ci/testcases.yaml'))
+class Testcase(cli_tier.Tier):
 
     def list(self):
         summary = ""
@@ -33,37 +23,28 @@ class Testcase(object):
                 summary += (" %s\n" % test.get_name())
         return summary
 
-    def show(self, testname):
-        description = self.tiers.get_test(testname)
+    def show(self, name):
+        description = self.tiers.get_test(name)
         return description
 
     @staticmethod
-    def run(testname, noclean=False, report=False):
-
-        flags = ""
-        if noclean:
-            flags += "-n "
-        if report:
-            flags += "-r "
-
-        tests = testname.split(",")
+    def run(name, noclean=False, report=False):
+        tests = name.split(",")
         for test in tests:
-            cmd = "run_tests {}-t {}".format(flags, test)
-            ft_utils.execute_command(cmd)
+            cmd = "run_tests {}-t {}".format(
+                Testcase.get_flags(noclean, report), test)
+            functest_utils.execute_command(cmd)
 
 
 class CliTestcase(Testcase):
 
-    def __init__(self):
-        super(CliTestcase, self).__init__()
-
     def list(self):
         click.echo(super(CliTestcase, self).list())
 
-    def show(self, testname):
-        testcase_show = super(CliTestcase, self).show(testname)
+    def show(self, name):
+        testcase_show = super(CliTestcase, self).show(name)
         if testcase_show:
             click.echo(testcase_show)
         else:
             click.echo("The test case '%s' does not exist or is not supported."
-                       % testname)
+                       % name)
diff --git a/functest/cli/commands/cli_tier.py b/functest/cli/commands/cli_tier.py
index 104cf10b..7aa3f714 100644
--- a/functest/cli/commands/cli_tier.py
+++ b/functest/cli/commands/cli_tier.py
@@ -5,23 +5,22 @@
 # are made available under the terms of the Apache License, Version 2.0
 # which accompanies this distribution, and is available at
 # http://www.apache.org/licenses/LICENSE-2.0
-#
 
-""" global variables """
+# pylint: disable=missing-docstring
 
 import pkg_resources
 
 import click
 
-import functest.ci.tier_builder as tb
+from functest.ci import tier_builder
 from functest.utils.constants import CONST
-import functest.utils.functest_utils as ft_utils
+from functest.utils import functest_utils
 
 
 class Tier(object):
 
     def __init__(self):
-        self.tiers = tb.TierBuilder(
+        self.tiers = tier_builder.TierBuilder(
             CONST.__getattribute__('INSTALLER_TYPE'),
             CONST.__getattribute__('DEPLOY_SCENARIO'),
             pkg_resources.resource_filename('functest', 'ci/testcases.yaml'))
@@ -35,56 +34,54 @@ class Tier(object):
                            tier.get_test_names()))
         return summary
 
-    def show(self, tiername):
-        tier = self.tiers.get_tier(tiername)
+    def show(self, name):
+        tier = self.tiers.get_tier(name)
         if tier is None:
             return None
-        else:
-            tier_info = self.tiers.get_tier(tiername)
-            return tier_info
+        tier_info = self.tiers.get_tier(name)
+        return tier_info
 
-    def gettests(self, tiername):
-        tier = self.tiers.get_tier(tiername)
+    def gettests(self, name):
+        tier = self.tiers.get_tier(name)
         if tier is None:
             return None
-        else:
-            tests = tier.get_test_names()
-            return tests
+        tests = tier.get_test_names()
+        return tests
 
     @staticmethod
-    def run(tiername, noclean=False, report=False):
+    def get_flags(noclean=False, report=False):
         flags = ""
         if noclean:
             flags += "-n "
         if report:
             flags += "-r "
+        return flags
 
-        cmd = "run_tests {}-t {}".format(flags, tiername)
-        ft_utils.execute_command(cmd)
+    @staticmethod
+    def run(name, noclean=False, report=False):
+        cmd = "run_tests {}-t {}".format(Tier.get_flags(noclean, report), name)
+        functest_utils.execute_command(cmd)
 
 
 class CliTier(Tier):
 
-    def __init__(self):
-        super(CliTier, self).__init__()
-
     def list(self):
         click.echo(super(CliTier, self).list())
 
-    def show(self, tiername):
-        tier_info = super(CliTier, self).show(tiername)
+    def show(self, name):
+        tier_info = super(CliTier, self).show(name)
         if tier_info:
             click.echo(tier_info)
         else:
             tier_names = self.tiers.get_tier_names()
             click.echo("The tier with name '%s' does not exist. "
-                       "Available tiers are:\n  %s\n" % (tiername, tier_names))
+                       "Available tiers are:\n  %s\n" % (name, tier_names))
 
-    def gettests(self, tiername):
-        tests = super(CliTier, self).gettests(tiername)
+    def gettests(self, name):
+        tests = super(CliTier, self).gettests(name)
         if tests:
-            click.echo("Test cases in tier '%s':\n %s\n" % (tiername, tests))
+            click.echo("Test cases in tier '%s':\n %s\n" % (name, tests))
         else:
             tier_names = self.tiers.get_tier_names()
             click.echo("The tier with name '%s' does not exist. "
-                       "Available tiers are:\n  %s\n" % (tiername, tier_names))
+                       "Available tiers are:\n  %s\n" % (name, tier_names))
diff --git a/functest/tests/unit/cli/commands/test_cli_env.py b/functest/tests/unit/cli/commands/test_cli_env.py
index b5c65377..e5c409bf 100644
--- a/functest/tests/unit/cli/commands/test_cli_env.py
+++ b/functest/tests/unit/cli/commands/test_cli_env.py
@@ -17,15 +17,13 @@ from functest.cli.commands import cli_env
 from functest.utils.constants import CONST
 
 
-class RegexMatch(object):
+class RegexMatch(object):  # pylint: disable=too-few-public-methods
     def __init__(self, msg):
         self.msg = msg
 
     def __eq__(self, other):
         match = re.search(self.msg, other)
-        if match:
-            return True
-        return False
+        return match is not None
 
 
 class CliEnvTesting(unittest.TestCase):
@@ -34,34 +32,35 @@ class CliEnvTesting(unittest.TestCase):
         self.cli_environ = cli_env.CliEnv()
 
     def _test_show_missing_env_var(self, var, *args):
+        # pylint: disable=unused-argument
         if var == 'INSTALLER_TYPE':
             CONST.__setattr__('INSTALLER_TYPE', None)
-            reg_string = "|  INSTALLER: Unknown, \S+\s*|"
+            reg_string = r"|  INSTALLER: Unknown, \S+\s*|"
         elif var == 'INSTALLER_IP':
             CONST.__setattr__('INSTALLER_IP', None)
-            reg_string = "|  INSTALLER: \S+, Unknown\s*|"
+            reg_string = r"|  INSTALLER: \S+, Unknown\s*|"
         elif var == 'SCENARIO':
             CONST.__setattr__('DEPLOY_SCENARIO', None)
-            reg_string = "|   SCENARIO: Unknown\s*|"
+            reg_string = r"|   SCENARIO: Unknown\s*|"
         elif var == 'NODE':
             CONST.__setattr__('NODE_NAME', None)
-            reg_string = "|        POD: Unknown\s*|"
+            reg_string = r"|        POD: Unknown\s*|"
         elif var == 'BUILD_TAG':
             CONST.__setattr__('BUILD_TAG', None)
-            reg_string = "|  BUILD TAG: None|"
+            reg_string = r"|  BUILD TAG: None|"
         elif var == 'DEBUG':
             CONST.__setattr__('CI_DEBUG', None)
-            reg_string = "| DEBUG FLAG: false\s*|"
+            reg_string = r"| DEBUG FLAG: false\s*|"
 
         with mock.patch('functest.cli.commands.cli_env.click.echo') \
                 as mock_click_echo:
             self.cli_environ.show()
             mock_click_echo.assert_called_with(RegexMatch(reg_string))
 
-    def test_show_missing_ci_installer_type(self, *args):
+    def test_show_ci_installer_type_ko(self, *args):
         self._test_show_missing_env_var('INSTALLER_TYPE', *args)
 
-    def test_show_missing_ci_installer_ip(self, *args):
+    def test_show_ci_installer_ip_ko(self, *args):
         self._test_show_missing_env_var('INSTALLER_IP', *args)
 
     def test_show_missing_ci_scenario(self, *args):
diff --git a/functest/tests/unit/cli/commands/test_cli_os.py b/functest/tests/unit/cli/commands/test_cli_os.py
index e4854cd6..26956e08 100644
--- a/functest/tests/unit/cli/commands/test_cli_os.py
+++ b/functest/tests/unit/cli/commands/test_cli_os.py
@@ -37,27 +37,24 @@ class CliOpenStackTesting(unittest.TestCase):
 
     @mock.patch('functest.cli.commands.cli_os.exit', side_effect=Exception)
     @mock.patch('functest.cli.commands.cli_os.click.echo')
-    def test_ping_endpoint_missing_auth_url(self, mock_click_echo,
-                                            mock_exit):
+    def test_ping_endpoint_auth_url_ko(self, mock_click_echo, mock_exit):
         with self.assertRaises(Exception):
             self.cli_os.os_auth_url = None
             self.cli_os.ping_endpoint()
-            mock_click_echo.assert_called_once_with("Source the OpenStack "
-                                                    "credentials first '. "
-                                                    "$creds'")
+        mock_click_echo.assert_called_once_with(
+            "Source the OpenStack credentials first '. $creds'")
+        mock_exit.assert_called_once_with(0)
 
     @mock.patch('functest.cli.commands.cli_os.exit')
     @mock.patch('functest.cli.commands.cli_os.click.echo')
-    def test_ping_endpoint_os_system_fails(self, mock_click_echo,
-                                           mock_exit):
+    def test_ping_endpoint_system_fails(self, mock_click_echo, mock_exit):
         self.cli_os.os_auth_url = self.os_auth_url
         self.cli_os.endpoint_ip = self.endpoint_ip
         with mock.patch('functest.cli.commands.cli_os.os.system',
                         return_value=1):
             self.cli_os.ping_endpoint()
-            mock_click_echo.assert_called_once_with("Cannot talk to the "
-                                                    "endpoint %s\n" %
-                                                    self.endpoint_ip)
+            mock_click_echo.assert_called_once_with(
+                "Cannot talk to the endpoint %s\n" % self.endpoint_ip)
             mock_exit.assert_called_once_with(0)
 
     def test_check(self):
diff --git a/functest/tests/unit/cli/commands/test_cli_testcase.py b/functest/tests/unit/cli/commands/test_cli_testcase.py
index 9ed7d829..30e55fac 100644
--- a/functest/tests/unit/cli/commands/test_cli_testcase.py
+++ b/functest/tests/unit/cli/commands/test_cli_testcase.py
@@ -19,28 +19,28 @@ class CliTestCasesTesting(unittest.TestCase):
 
     def setUp(self):
         self.testname = 'testname'
-        with mock.patch('functest.cli.commands.cli_testcase.tb'):
+        with mock.patch('functest.ci.tier_builder'):
             self.cli_tests = cli_testcase.CliTestcase()
 
-    @mock.patch('functest.cli.commands.cli_testcase.ft_utils.execute_command')
+    @mock.patch('functest.utils.functest_utils.execute_command')
     def test_run_default(self, mock_ft_utils):
         cmd = "run_tests -n -r -t {}".format(self.testname)
         self.cli_tests.run(self.testname, noclean=True, report=True)
         mock_ft_utils.assert_called_with(cmd)
 
-    @mock.patch('functest.cli.commands.cli_testcase.ft_utils.execute_command')
+    @mock.patch('functest.utils.functest_utils.execute_command')
     def test_run_noclean_missing_report(self, mock_ft_utils):
         cmd = "run_tests -n -t {}".format(self.testname)
         self.cli_tests.run(self.testname, noclean=True, report=False)
         mock_ft_utils.assert_called_with(cmd)
 
-    @mock.patch('functest.cli.commands.cli_testcase.ft_utils.execute_command')
+    @mock.patch('functest.utils.functest_utils.execute_command')
     def test_run_report_missing_noclean(self, mock_ft_utils):
         cmd = "run_tests -r -t {}".format(self.testname)
         self.cli_tests.run(self.testname, noclean=False, report=True)
         mock_ft_utils.assert_called_with(cmd)
 
-    @mock.patch('functest.cli.commands.cli_testcase.ft_utils.execute_command')
+    @mock.patch('functest.utils.functest_utils.execute_command')
     def test_run_missing_noclean_report(self, mock_ft_utils):
         cmd = "run_tests -t {}".format(self.testname)
         self.cli_tests.run(self.testname, noclean=False, report=False)
diff --git a/functest/tests/unit/cli/commands/test_cli_tier.py b/functest/tests/unit/cli/commands/test_cli_tier.py
index 49c3c7a3..f81ad31d 100644
--- a/functest/tests/unit/cli/commands/test_cli_tier.py
+++ b/functest/tests/unit/cli/commands/test_cli_tier.py
@@ -20,7 +20,7 @@ class CliTierTesting(unittest.TestCase):
     def setUp(self):
         self.tiername = 'tiername'
         self.testnames = 'testnames'
-        with mock.patch('functest.cli.commands.cli_tier.tb'):
+        with mock.patch('functest.ci.tier_builder'):
             self.cli_tier = cli_tier.CliTier()
 
     @mock.patch('functest.cli.commands.cli_tier.click.echo')
@@ -58,10 +58,9 @@ class CliTierTesting(unittest.TestCase):
         with mock.patch.object(self.cli_tier.tiers, 'get_tier',
                                return_value=mock_obj):
             self.cli_tier.gettests(self.tiername)
-            mock_click_echo.assert_called_with("Test cases in tier "
-                                               "'%s':\n %s\n" % (self.tiername,
-                                                                 self.testnames
-                                                                 ))
+            mock_click_echo.assert_called_with(
+                "Test cases in tier '%s':\n %s\n" % (
+                    self.tiername, self.testnames))
 
     @mock.patch('functest.cli.commands.cli_tier.click.echo')
     def test_gettests_missing_tier(self, mock_click_echo):
@@ -75,25 +74,25 @@ class CliTierTesting(unittest.TestCase):
                                                ":\n  %s\n" % (self.tiername,
                                                               'tiernames'))
 
-    @mock.patch('functest.cli.commands.cli_tier.ft_utils.execute_command')
+    @mock.patch('functest.utils.functest_utils.execute_command')
     def test_run_default(self, mock_ft_utils):
         cmd = "run_tests -n -r -t {}".format(self.tiername)
         self.cli_tier.run(self.tiername, noclean=True, report=True)
         mock_ft_utils.assert_called_with(cmd)
 
-    @mock.patch('functest.cli.commands.cli_tier.ft_utils.execute_command')
+    @mock.patch('functest.utils.functest_utils.execute_command')
     def test_run_report_missing_noclean(self, mock_ft_utils):
         cmd = "run_tests -r -t {}".format(self.tiername)
         self.cli_tier.run(self.tiername, noclean=False, report=True)
         mock_ft_utils.assert_called_with(cmd)
 
-    @mock.patch('functest.cli.commands.cli_tier.ft_utils.execute_command')
+    @mock.patch('functest.utils.functest_utils.execute_command')
     def test_run_noclean_missing_report(self, mock_ft_utils):
         cmd = "run_tests -n -t {}".format(self.tiername)
         self.cli_tier.run(self.tiername, noclean=True, report=False)
         mock_ft_utils.assert_called_with(cmd)
 
-    @mock.patch('functest.cli.commands.cli_tier.ft_utils.execute_command')
+    @mock.patch('functest.utils.functest_utils.execute_command')
     def test_run_missing_noclean_report(self, mock_ft_utils):
         cmd = "run_tests -t {}".format(self.tiername)
         self.cli_tier.run(self.tiername, noclean=False, report=False)
diff --git a/functest/tests/unit/cli/test_cli_base.py b/functest/tests/unit/cli/test_cli_base.py
index 5ac1fda9..876e8aa1 100644
--- a/functest/tests/unit/cli/test_cli_base.py
+++ b/functest/tests/unit/cli/test_cli_base.py
@@ -26,10 +26,10 @@ class CliBaseTesting(unittest.TestCase):
 
     def setUp(self):
         self.runner = CliRunner()
-        self._openstack = cli_base._openstack
-        self._env = cli_base._env
-        self._testcase = cli_base._testcase
-        self._tier = cli_base._tier
+        self._openstack = cli_base.OPENSTACK
+        self._env = cli_base.ENV
+        self._testcase = cli_base.TESTCASE
+        self._tier = cli_base.TIER
 
     def test_os_check(self):
         with mock.patch.object(self._openstack, 'check') as mock_method: