diff --git a/validations_libs/cli/base.py b/validations_libs/cli/base.py index 2ef60da2..cf821466 100644 --- a/validations_libs/cli/base.py +++ b/validations_libs/cli/base.py @@ -22,14 +22,9 @@ from cliff.command import Command from cliff.lister import Lister from cliff.show import ShowOne -from validations_libs import constants +from validations_libs.cli import constants as cli_constants from validations_libs import utils - -# Handle backward compatibility for Cliff 2.16.0 in stable/train: -if hasattr(_argparse, 'SmartHelpFormatter'): - from cliff._argparse import SmartHelpFormatter -else: - from cliff.command import _SmartHelpFormatter as SmartHelpFormatter +from validations_libs.cli.common import ValidationHelpFormatter class Base: @@ -71,7 +66,7 @@ class BaseCommand(Command): description=self.get_description(), epilog=self.get_epilog(), prog=prog_name, - formatter_class=SmartHelpFormatter, + formatter_class=ValidationHelpFormatter, conflict_handler='resolve', ) for hook in self._hooks: @@ -81,8 +76,7 @@ class BaseCommand(Command): '--config', dest='config', default=utils.find_config_file(), - help=("Config file path for Validation.") - ) + help=cli_constants.CONF_FILE_DESC) return parser @@ -98,7 +92,7 @@ class BaseLister(Lister): description=self.get_description(), epilog=self.get_epilog(), prog=prog_name, - formatter_class=SmartHelpFormatter, + formatter_class=ValidationHelpFormatter, conflict_handler='resolve', ) @@ -109,8 +103,7 @@ class BaseLister(Lister): '--config', dest='config', default=utils.find_config_file(), - help=("Config file path for Validation.") - ) + help=cli_constants.CONF_FILE_DESC) return vf_parser @@ -126,7 +119,6 @@ class BaseShow(ShowOne): '--config', dest='config', default=utils.find_config_file(), - help=("Config file path for Validation.") - ) + help=cli_constants.CONF_FILE_DESC) return parser diff --git a/validations_libs/cli/common.py b/validations_libs/cli/common.py index a9314b9c..038732c8 100644 --- a/validations_libs/cli/common.py +++ b/validations_libs/cli/common.py @@ -14,6 +14,8 @@ # License for the specific language governing permissions and limitations # under the License. +from argparse import ArgumentDefaultsHelpFormatter +from cliff import _argparse import json import logging from prettytable import PrettyTable @@ -29,8 +31,27 @@ try: except ImportError: JUNIT_XML_FOUND = False -from validations_libs import utils as v_utils from validations_libs.cli import colors +from validations_libs import utils + +# Handle backward compatibility for Cliff 2.16.0 in stable/train: +if hasattr(_argparse, 'SmartHelpFormatter'): + from cliff._argparse import SmartHelpFormatter +else: + from cliff.command import _SmartHelpFormatter as SmartHelpFormatter + + +class ValidationHelpFormatter(ArgumentDefaultsHelpFormatter, SmartHelpFormatter): + """Composite CLI help formatter, providing both default argument values, + and correct new line treatment. + """ + + def _get_help_string(self, action): + default_value = action.default + if isinstance(default_value, list) or isinstance(default_value, str): + if len(default_value) > 0: + return super()._get_help_string(action) + return super(ArgumentDefaultsHelpFormatter, self)._get_help_string(action) def print_dict(data): diff --git a/validations_libs/cli/constants.py b/validations_libs/cli/constants.py new file mode 100644 index 00000000..74552066 --- /dev/null +++ b/validations_libs/cli/constants.py @@ -0,0 +1,29 @@ +# Copyright 2021 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +"""Constants for the VF CLI. +Constains larger, more frequently used and redundant CLI help strings. +""" + +CONF_FILE_DESC = "Config file path for Validation Framework.\n" +LOG_PATH_DESC = "Path where the log files and artifacts are located.\n" +PLAY_PATH_DESC = "Path where validation playbooks are located.\n" +VAL_GROUP_DESC = ("List specific group of validations, " + "if more than one group is required " + "separate the group names with commas.\n") +VAL_CAT_DESC = ("List specific category of validations, " + "if more than one category is required " + "separate the category names with commas.\n") +VAL_PROD_DESC = ("List specific product of validations, " + "if more than one product is required " + "separate the product names with commas.\n") diff --git a/validations_libs/cli/history.py b/validations_libs/cli/history.py index 0d01becc..c4eb0b33 100644 --- a/validations_libs/cli/history.py +++ b/validations_libs/cli/history.py @@ -17,6 +17,7 @@ import json from validations_libs import constants +from validations_libs.cli import constants as cli_constants from validations_libs.validation_actions import ValidationActions from validations_libs.validation_logs import ValidationLogs from validations_libs.cli.base import BaseCommand, BaseLister @@ -43,8 +44,7 @@ class ListHistory(BaseLister): 'The default display limit is set to 15.\n')) parser.add_argument('--validation-log-dir', dest='validation_log_dir', default=constants.VALIDATIONS_LOG_BASEDIR, - help=("Path where the validation log files " - "is located.")) + help=cli_constants.LOG_PATH_DESC) return parser def take_action(self, parsed_args): @@ -81,12 +81,11 @@ class GetHistory(BaseCommand): parser.add_argument('--full', action='store_true', - help='Show Full Details for the run') + help='Show full details of the validation run') parser.add_argument('--validation-log-dir', dest='validation_log_dir', default=constants.VALIDATIONS_LOG_BASEDIR, - help=("Path where the validation log files " - "is located.")) + help=cli_constants.LOG_PATH_DESC) return parser def take_action(self, parsed_args): diff --git a/validations_libs/cli/lister.py b/validations_libs/cli/lister.py index 6cbeb26b..745b454c 100644 --- a/validations_libs/cli/lister.py +++ b/validations_libs/cli/lister.py @@ -18,6 +18,7 @@ from validations_libs.validation_actions import ValidationActions from validations_libs import constants from validations_libs.cli.base import BaseLister from validations_libs.cli.parseractions import CommaListAction +from validations_libs.cli import constants as cli_constants class ValidationList(BaseLister): @@ -30,27 +31,20 @@ class ValidationList(BaseLister): metavar='[,,...]', action=CommaListAction, default=[], - help=("List specific group of validations, " - "if more than one group is required " - "separate the group names with commas.")) + help=cli_constants.VAL_GROUP_DESC) parser.add_argument('--category', metavar='[,,...]', action=CommaListAction, default=[], - help=("List specific category of validations, " - "if more than one category is required " - "separate the category names with commas.")) + help=cli_constants.VAL_CAT_DESC) parser.add_argument('--product', metavar='[,,...]', action=CommaListAction, default=[], - help=("List specific product of validations, " - "if more than one product is required " - "separate the product names with commas.")) + help=cli_constants.VAL_PROD_DESC) parser.add_argument('--validation-dir', dest='validation_dir', default=constants.ANSIBLE_VALIDATION_DIR, - help=("Path where the validation playbooks " - "are located.")) + help=cli_constants.PLAY_PATH_DESC) return parser def take_action(self, parsed_args): diff --git a/validations_libs/cli/run.py b/validations_libs/cli/run.py index e5c7760a..d1d1a2e5 100644 --- a/validations_libs/cli/run.py +++ b/validations_libs/cli/run.py @@ -18,6 +18,7 @@ import getpass import sys from validations_libs import constants +from validations_libs.cli import constants as cli_constants from validations_libs.validation_actions import ValidationActions from validations_libs.cli import common from validations_libs.cli.base import BaseCommand @@ -37,44 +38,41 @@ class Run(BaseCommand): required=False, help=( "A string that identifies a single node or comma-separated " - "list of nodes to be upgraded in parallel in this upgrade " - "run invocation.")) + "list of nodes to be validated in this run invocation.\n")) parser.add_argument( '--ssh-user', dest='ssh_user', default=getpass.getuser(), - help=("SSH user name for the Ansible ssh connection.")) + help=("SSH user name for the Ansible ssh connection.\n")) parser.add_argument('--validation-dir', dest='validation_dir', default=constants.ANSIBLE_VALIDATION_DIR, - help=("Path where the validation playbooks " - "is located.")) + help=cli_constants.PLAY_PATH_DESC) parser.add_argument('--ansible-base-dir', dest='ansible_base_dir', default=constants.DEFAULT_VALIDATIONS_BASEDIR, help=("Path where the ansible roles, library " - "and plugins are located.")) + "and plugins are located.\n")) parser.add_argument( '--validation-log-dir', dest='validation_log_dir', default=constants.VALIDATIONS_LOG_BASEDIR, - help=( - "Path where the log files and artifacts will be located. ")) + help=cli_constants.LOG_PATH_DESC) parser.add_argument('--inventory', '-i', type=str, default="localhost", - help="Path of the Ansible inventory.") + help="Path of the Ansible inventory.\n") parser.add_argument('--output-log', dest='output_log', default=None, - help=("Path where the run result will be stored.")) + help=("Path where the run result will be stored.\n")) parser.add_argument('--junitxml', dest='junitxml', default=None, help=("Path where the run result in JUnitXML " - "format will be stored.")) + "format will be stored.\n")) parser.add_argument( '--python-interpreter', @@ -83,7 +81,7 @@ class Run(BaseCommand): default="{}".format( sys.executable if sys.executable else "/usr/bin/python" ), - help=("Python interpreter for Ansible execution.")) + help=("Python interpreter for Ansible execution.\n")) parser.add_argument( '--extra-env-vars', @@ -95,7 +93,7 @@ class Run(BaseCommand): "to provide to your Ansible execution " "as KEY=VALUE pairs. Note that if you pass the same " "KEY multiple times, the last given VALUE for that same KEY " - "will override the other(s)")) + "will override the other(s).\n")) parser.add_argument('--skiplist', dest='skip_list', default=None, @@ -114,7 +112,7 @@ class Run(BaseCommand): "Add Ansible extra variables to the validation(s) execution " "as KEY=VALUE pair(s). Note that if you pass the same " "KEY multiple times, the last given VALUE for that same KEY " - "will override the other(s)")) + "will override the other(s).\n")) extra_vars_group.add_argument( '--extra-vars-file', @@ -123,7 +121,7 @@ class Run(BaseCommand): default=None, help=( "Absolute or relative Path to a JSON/YAML file containing extra variable(s) " - "to pass to one or multiple validation(s) execution.")) + "to pass to one or multiple validation(s) execution.\n")) ex_group = parser.add_mutually_exclusive_group(required=True) ex_group.add_argument( @@ -134,16 +132,16 @@ class Run(BaseCommand): default=[], help=("Run specific validations, " "if more than one validation is required " - "separate the names with commas.")) + "separate the names with commas.\n")) ex_group.add_argument( '--group', '-g', metavar='[,,...]', action=CommaListAction, default=[], - help=("Run specific group validations, " + help=("Run specific validations by group, " "if more than one group is required " - "separate the group names with commas.")) + "separate the group names with commas.\n")) ex_group.add_argument( '--category', @@ -152,7 +150,7 @@ class Run(BaseCommand): default=[], help=("Run specific validations by category, " "if more than one category is required " - "separate the category names with commas.")) + "separate the category names with commas.\n")) ex_group.add_argument( '--product', @@ -161,7 +159,7 @@ class Run(BaseCommand): default=[], help=("Run specific validations by product, " "if more than one product is required " - "separate the product names with commas.")) + "separate the product names with commas.\n")) return parser def take_action(self, parsed_args): diff --git a/validations_libs/cli/show.py b/validations_libs/cli/show.py index 26c22f72..d3119e82 100644 --- a/validations_libs/cli/show.py +++ b/validations_libs/cli/show.py @@ -18,6 +18,7 @@ from validations_libs.validation_actions import ValidationActions from validations_libs import constants from validations_libs.cli.parseractions import CommaListAction from validations_libs.cli.base import BaseShow, BaseLister +from validations_libs.cli import constants as cli_constants class Show(BaseShow): @@ -28,8 +29,7 @@ class Show(BaseShow): parser = super(Show, self).get_parser(parser) parser.add_argument('--validation-dir', dest='validation_dir', default=constants.ANSIBLE_VALIDATION_DIR, - help=("Path where the validation playbooks " - "are located.")) + help=cli_constants.PLAY_PATH_DESC) parser.add_argument('validation_name', metavar="", type=str, @@ -61,8 +61,7 @@ class ShowGroup(BaseLister): parser.add_argument('--validation-dir', dest='validation_dir', default=constants.ANSIBLE_VALIDATION_DIR, - help=("Path where the validation playbooks " - "are located.")) + help=cli_constants.PLAY_PATH_DESC) return parser def take_action(self, parsed_args): @@ -89,8 +88,7 @@ class ShowParameter(BaseShow): parser.add_argument('--validation-dir', dest='validation_dir', default=constants.ANSIBLE_VALIDATION_DIR, - help=("Path where the validation playbooks " - "are located.")) + help=cli_constants.PLAY_PATH_DESC) ex_group = parser.add_mutually_exclusive_group(required=False) ex_group.add_argument( @@ -101,36 +99,28 @@ class ShowParameter(BaseShow): default=[], help=("List specific validations, " "if more than one validation is required " - "separate the names with commas.") - ) + "separate the names with commas.")) ex_group.add_argument( '--group', '-g', metavar='[,,...]', action=CommaListAction, default=[], - help=("List specific group validations, " - "if more than one group is required " - "separate the group names with commas.") - ) + help=cli_constants.VAL_GROUP_DESC) ex_group.add_argument( '--category', metavar='[,,...]', action=CommaListAction, default=[], - help=("List specific validations by category, " - "if more than one category is required " - "separate the category names with commas.")) + help=cli_constants.VAL_CAT_DESC) ex_group.add_argument( '--product', metavar='[,,...]', action=CommaListAction, default=[], - help=("List specific validations by product, " - "if more than one product is required " - "separate the product names with commas.")) + help=cli_constants.VAL_PROD_DESC) parser.add_argument( '--download', @@ -139,8 +129,7 @@ class ShowParameter(BaseShow): help=("Create a json or a yaml file " "containing all the variables " "available for the validations: " - "/tmp/myvars") - ) + "/tmp/myvars")) parser.add_argument( '--format-output', diff --git a/validations_libs/tests/cli/test_common.py b/validations_libs/tests/cli/test_common.py index f36debf7..5e1e712c 100644 --- a/validations_libs/tests/cli/test_common.py +++ b/validations_libs/tests/cli/test_common.py @@ -14,19 +14,20 @@ # from unittest import TestCase import yaml +import sys +import importlib +from validations_libs.cli import common try: from unittest import mock except ImportError: import mock -from validations_libs.cli import common - class TestCommon(TestCase): def setUp(self): - super(TestCommon, self).setUp() + return super().setUp() def test_read_cli_data_file_with_example_file(self): example_data = {'check-cpu': {'hosts': 'undercloud', @@ -45,3 +46,22 @@ class TestCommon(TestCase): @mock.patch('yaml.safe_load', side_effect=yaml.YAMLError) def test_read_cli_data_file_yaml_error(self, mock_yaml): self.assertRaises(RuntimeError, common.read_cli_data_file, 'foo') + + @mock.patch('cliff._argparse', spec={}) + def test_argparse_conditional_false(self, mock_argparse): + """Test if the imporst are properly resolved based + on presence of the `SmartHelpFormatter` in the namespace + of the cliff._argparse. + If the attribute isn't in the namespace, and it shouldn't be + because the object is mocked to behave as a dictionary, + the next statement after conditional will trigger `ImportError`. + Because relevant class in the cliff module is mocked with side effect. + """ + sys.modules[ + 'cliff.command.SmartHelpFormatter'] = mock.MagicMock( + side_effect=ImportError) + + self.assertRaises( + ImportError, + importlib.reload, + common)