openstack
/
validations-libs
Code Issues Proposed changes

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:
Jiri Podivin 2021-09-08 09:14:26 +02:00
parent b37015ee1f
commit 524a48d7b9
8 changed files with 119 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
validations_libs/cli/base.py
View File

@ -21,14 +21,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 con
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:
@ -79,7 +74,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:
@ -89,8 +84,7 @@ class BaseCommand(Command):
'--config',
dest='config',
default=utils.find_config_file(),
help=("Config file path for Validation.")
)
help=con.CONF_FILE_DESC)
return parser
@ -106,7 +100,7 @@ class BaseLister(Lister):
description=self.get_description(),
epilog=self.get_epilog(),
prog=prog_name,
formatter_class=SmartHelpFormatter,
formatter_class=ValidationHelpFormatter,
conflict_handler='resolve',
)
@ -117,8 +111,7 @@ class BaseLister(Lister):
'--config',
dest='config',
default=utils.find_config_file(),
help=("Config file path for Validation.")
)
help=con.CONF_FILE_DESC)
return vf_parser
@ -134,7 +127,6 @@ class BaseShow(ShowOne):
'--config',
dest='config',
default=utils.find_config_file(),
help=("Config file path for Validation.")
)
help=con.CONF_FILE_DESC)
return parser

23
validations_libs/cli/common.py
View File

@ -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(action.default) > 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
View 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")

10
validations_libs/cli/history.py
View File

@ -17,6 +17,7 @@
import json
from validations_libs import constants
from validations_libs.cli import constants as con
from validations_libs.validation_actions import ValidationActions
from validations_libs.validation_logs import ValidationLogs
from validations_libs.cli.base import BaseCommand, BaseLister
@ -43,13 +44,11 @@ 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=con.LOG_PATH_DESC)
# Merge config and CLI args:
return self.base.set_argument_parser(parser)
def take_action(self, parsed_args):
validation_log_dir = parsed_args.validation_log_dir
history_limit = parsed_args.history_limit
if history_limit < 1:
@ -80,12 +79,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=con.LOG_PATH_DESC)
# Merge config and CLI args:
return self.base.set_argument_parser(parser)

16
validations_libs/cli/lister.py
View File

@ -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 con
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=con.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=con.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=con.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=con.PLAY_PATH_DESC)
# Merge config and CLI args:
return self.base.set_argument_parser(parser)

38
validations_libs/cli/run.py
View File

@ -18,6 +18,7 @@ import getpass
import sys
from validations_libs import constants
from validations_libs.cli import constants as con
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=con.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=con.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"))
if self.app:
# Merge config and CLI args:

32
validations_libs/cli/show.py
View File

@ -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 con
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=con.PLAY_PATH_DESC)
parser.add_argument('validation_name',
metavar="<validation>",
type=str,
@ -59,8 +59,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=con.PLAY_PATH_DESC)
# Merge config and CLI args:
return self.base.set_argument_parser(parser)
@ -85,8 +84,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=con.PLAY_PATH_DESC)
ex_group = parser.add_mutually_exclusive_group(required=False)
ex_group.add_argument(
@ -97,36 +95,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=con.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=con.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=con.VAL_PROD_DESC)
parser.add_argument(
'--download',
@ -135,8 +125,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',
@ -145,8 +134,7 @@ class ShowParameter(BaseShow):
default='json',
choices=['json', 'yaml'],
help=("Print representation of the validation. "
"The choices of the output format is json,yaml. ")
)
"The choices of the output format is json,yaml. "))
# Merge config and CLI args:
return self.base.set_argument_parser(parser)

28
validations_libs/tests/cli/test_common.py
View File

@ -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()
def setUp(self) -> None:
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)