From bbfe9b09d2b1ac7bfe286311fef83d36c6125c96 Mon Sep 17 00:00:00 2001 From: =?utf8?q?C=C3=A9dric=20Ollivier?= Date: Tue, 23 Jan 2018 11:19:41 +0100 Subject: [PATCH] Fix pylint warnings/errors in cli MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 --- functest/cli/cli_base.py | 42 +++++++++-------- functest/cli/commands/cli_env.py | 21 ++++----- functest/cli/commands/cli_os.py | 15 +++--- functest/cli/commands/cli_testcase.py | 47 ++++++------------- functest/cli/commands/cli_tier.py | 53 ++++++++++------------ functest/tests/unit/cli/commands/test_cli_env.py | 23 +++++----- functest/tests/unit/cli/commands/test_cli_os.py | 17 +++---- .../tests/unit/cli/commands/test_cli_testcase.py | 10 ++-- functest/tests/unit/cli/commands/test_cli_tier.py | 17 ++++--- functest/tests/unit/cli/test_cli_base.py | 8 ++-- tox.ini | 2 + 11 files changed, 113 insertions(+), 142 deletions(-) diff --git a/functest/cli/cli_base.py b/functest/cli/cli_base.py index 5890e0a35..1a057e1bf 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 c41f8f340..c1b66da41 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 9057da84b..a543e8457 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 a424a05b2..a8ead5f53 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 104cf10b5..7aa3f714e 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 b5c653770..e5c409bf4 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 e4854cd6f..26956e08b 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 9ed7d8293..30e55fac0 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 49c3c7a3e..f81ad31d0 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 5ac1fda9b..876e8aa15 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: diff --git a/tox.ini b/tox.ini index 1def975c5..0f0d50a14 100644 --- a/tox.ini +++ b/tox.ini @@ -31,10 +31,12 @@ whitelist_externals = bash modules = functest.api functest.ci + functest.cli functest.core functest.energy functest.opnfv_tests.sdn.odl functest.tests.unit.ci + functest.tests.unit.cli functest.tests.unit.core functest.tests.unit.energy functest.tests.unit.odl -- 2.16.6