Validation help improvement
New formatter prints text with proper line breaks as well as the argument defaults. Larger, and more frequently used, help strings were moved to the validations_libs.cli.constants module. Test was added for the conditional import of the cliff help formatter. Signed-off-by: Jiri Podivin <jpodivin@redhat.com> Change-Id: I26648b495e075fb0e6f9741693eca2feed3fc140
This commit is contained in:
parent
794fb1a0a3
commit
3de8f4ddc9
@ -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
|
||||
|
@ -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):
|
||||
|
29
validations_libs/cli/constants.py
Normal file
29
validations_libs/cli/constants.py
Normal file
@ -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")
|
@ -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):
|
||||
|
@ -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='<group_id>[,<group_id>,...]',
|
||||
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='<category_id>[,<category_id>,...]',
|
||||
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='<product_id>[,<product_id>,...]',
|
||||
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):
|
||||
|
@ -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='<group_id>[,<group_id>,...]',
|
||||
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):
|
||||
|
@ -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="<validation>",
|
||||
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='<group_id>[,<group_id>,...]',
|
||||
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='<category_id>[,<category_id>,...]',
|
||||
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='<product_id>[,<product_id>,...]',
|
||||
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',
|
||||
|
@ -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)
|
||||
|
Loading…
Reference in New Issue
Block a user