Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cmd example change #490

Draft
wants to merge 2 commits into
base: dev
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions azdev/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ def operation_group(name):
g.command('meta-export', 'export_command_meta')
g.command('meta-diff', 'cmp_command_meta')
g.command('tree-export', 'export_command_tree')
g.command('cmd-example-diff', 'meta_command_example_diff')

with CommandGroup(self, 'cli', operation_group('pypi')) as g:
g.command('check-versions', 'verify_versions')
Expand Down
9 changes: 8 additions & 1 deletion azdev/help.py
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@
short-summary: Diff the command meta between provided meta files.
examples:
- name: Diff the command meta change from fileA to fileB
text: azdev statistics meta-diff --base-meta-file fileA --diff-meta-file fileB --only-break
text: azdev command-change meta-diff --base-meta-file fileA --diff-meta-file fileB --only-break
"""

helps['command-change tree-export'] = """
Expand All @@ -253,6 +253,13 @@
text: azdev command-change tree-export CLI --output-file command_tree.json
"""

helps['command-change cmd-example-diff'] = """
short-summary: Check the command example number between provided meta files.
examples:
- name: Check the command example number from fileA to fileB
text: azdev command-change cmd-example-diff --base-meta-file fileA --diff-meta-file fileB --only-break
"""

helps['perf'] = """
short-summary: Commands to test CLI performance.
"""
Expand Down
18 changes: 17 additions & 1 deletion azdev/operations/command_change/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@
# pylint: disable=no-else-return, too-many-nested-blocks, too-many-locals, too-many-branches

import time
import os
import json

from knack.log import get_logger
import azure_cli_diff_tool
from azdev.utilities import display, require_azure_cli, heading, get_path_table, filter_by_git_diff, \
calc_selected_mod_names
from .custom import DiffExportFormat, get_commands_meta, STORED_DEPRECATION_KEY
from .custom import DiffExportFormat, get_commands_meta, STORED_DEPRECATION_KEY, command_example_diff
from .util import export_commands_meta, dump_command_tree, add_to_command_tree
from ..statistics import _create_invoker_and_load_cmds, _get_command_source, \
_command_codegen_info # pylint: disable=protected-access
Expand Down Expand Up @@ -178,3 +180,17 @@ def export_command_tree(modules, output_file=None):
add_to_command_tree(command_tree, command_name, module_source)

dump_command_tree(command_tree, output_file)


def meta_command_example_diff(base_meta_file, diff_meta_file):
if not os.path.exists(base_meta_file) and not os.path.exists(diff_meta_file):
raise Exception("base and/or diff meta file needed")
command_tree_before = {}
command_tree_after = {}
if os.path.exists(base_meta_file):
with open(base_meta_file, "r") as g:
command_tree_before = json.load(g)
if os.path.exists(diff_meta_file):
with open(diff_meta_file, "r") as g:
command_tree_after = json.load(g)
return command_example_diff(command_tree_before, command_tree_after)
64 changes: 63 additions & 1 deletion azdev/operations/command_change/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from enum import Enum

from knack.log import get_logger
from .util import get_command_tree
from .util import get_command_tree, search_cmd_obj, ChangeType

logger = get_logger(__name__)

Expand Down Expand Up @@ -241,3 +241,65 @@ def get_commands_meta(command_group_table, commands_info, with_help, with_exampl
command_group_info["commands"][command_name] = command_meta
break
return commands_meta


def command_example_diff(base_meta=None, diff_meta=None):
diff_cmds = []
find_command_remove(base_meta, diff_meta, diff_cmds)
check_command_add(diff_meta, diff_cmds)
add_cmd_example = []
for cmd_name, diff_type in diff_cmds:
if diff_type == ChangeType.REMOVE:
cmd_obj = search_cmd_obj(cmd_name, base_meta)
add_cmd_example.append({
"cmd": cmd_name,
"change_type": diff_type,
"param_count": len(cmd_obj["parameters"]) if "parameters" in cmd_obj else 0,
"example_count": len(cmd_obj["examples"]) if "examples" in cmd_obj else 0
})
continue
diff_cmd_obj = search_cmd_obj(cmd_name, diff_meta)
add_cmd_example.append({
"cmd": cmd_name,
"change_type": diff_type,
"param_count": len(diff_cmd_obj["parameters"]) if "parameters" in diff_cmd_obj else 0,
"example_count": len(diff_cmd_obj["examples"]) if "examples" in diff_cmd_obj else 0
})
return add_cmd_example


def find_command_remove(base_meta, diff_meta, diff_cmds):
if "sub_groups" in base_meta:
for sub_group, group_item in base_meta["sub_groups"].items():
find_command_remove(group_item, diff_meta, diff_cmds)
if "commands" in base_meta:
generate_command_remove(base_meta["commands"], diff_meta, diff_cmds)


def generate_command_remove(base_cmd_items, diff_meta, diff_cmds):
for cmd_name, cmd_obj in base_cmd_items.items():
if cmd_obj.get("checked", None):
continue
cmd_obj["checked"] = True
diff_cmd_obj = search_cmd_obj(cmd_name, diff_meta)
if diff_cmd_obj is None:
# cmd lost
diff_cmds.append((cmd_name, ChangeType.REMOVE))
continue
diff_cmd_obj["checked"] = True


def check_command_add(diff_meta, diff_cmds):
if "sub_groups" in diff_meta:
for sub_group, group_item in diff_meta["sub_groups"].items():
check_command_add(group_item, diff_cmds)
if "commands" in diff_meta:
generate_command_add(diff_meta["commands"], diff_cmds)


def generate_command_add(diff_cmd_items, diff_cmds):
for cmd_name, cmd_obj in diff_cmd_items.items():
if cmd_obj.get("checked", None):
continue
diff_cmds.append((cmd_name, ChangeType.ADD))
cmd_obj["checked"] = True
21 changes: 21 additions & 0 deletions azdev/operations/command_change/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,27 @@ def get_command_tree(command_name):
return ret


def search_cmd_obj(cmd_name, search_meta):
command_tree = get_command_tree(cmd_name)
meta_search = search_meta
while True:
if "is_group" not in command_tree:
break
if command_tree["is_group"]:
group_name = command_tree["group_name"]
if group_name not in meta_search["sub_groups"]:
return None
meta_search = meta_search["sub_groups"][group_name]
command_tree = command_tree["sub_info"]
else:
cmd_name = command_tree["cmd_name"]
if cmd_name not in meta_search["commands"]:
return None
meta_search = meta_search["commands"][cmd_name]
break
return meta_search


def export_commands_meta(commands_meta, meta_output_path=None):
options = jsbeautifier.default_options()
options.indent_size = 4
Expand Down
8 changes: 6 additions & 2 deletions azdev/operations/linter/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
# pylint:disable=too-many-locals, too-many-statements, too-many-branches
def run_linter(modules=None, rule_types=None, rules=None, ci_exclusions=None,
git_source=None, git_target=None, git_repo=None, include_whl_extensions=False,
min_severity=None, save_global_exclusion=False):
min_severity=None, save_global_exclusion=False, base_meta_path=None, diff_meta_path=None):

require_azure_cli()

Expand Down Expand Up @@ -155,7 +155,10 @@ def run_linter(modules=None, rule_types=None, rules=None, ci_exclusions=None,
update_global_exclusion=update_global_exclusion,
git_source=git_source,
git_target=git_target,
git_repo=git_repo)
git_repo=git_repo,
base_meta_path=base_meta_path,
diff_meta_path=diff_meta_path
)

subheading('Results')
logger.info('Running linter: %i commands, %i help entries',
Expand All @@ -166,6 +169,7 @@ def run_linter(modules=None, rule_types=None, rules=None, ci_exclusions=None,
run_command_groups=not rule_types or 'command_groups' in rule_types,
run_help_files_entries=not rule_types or 'help_entries' in rule_types,
run_command_test_coverage=not rule_types or 'command_test_coverage' in rule_types,
run_extra_cli_linter=not rule_types or "extra_cli_linter" in rule_types,
)
display(os.linesep + 'Run custom pylint rules.')
exit_code += pylint_rules(selected_modules)
Expand Down
36 changes: 30 additions & 6 deletions azdev/operations/linter/linter.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@
search_argument_context,
search_command,
search_command_group)
from azdev.operations.command_change import meta_command_example_diff
from azdev.utilities import diff_branches_detail, diff_branch_file_patch
from azdev.utilities.path import get_cli_repo_path, get_ext_repo_paths
from .util import share_element, exclude_commands, LinterError
from .util import share_element, exclude_commands, LinterError, add_cmd_example_justification

PACKAGE_NAME = 'azdev.operations.linter'
_logger = get_logger(__name__)
Expand All @@ -47,7 +48,7 @@ def get_ordered_members():

class Linter: # pylint: disable=too-many-public-methods, too-many-instance-attributes
def __init__(self, command_loader=None, help_file_entries=None, loaded_help=None, git_source=None, git_target=None,
git_repo=None, exclusions=None):
git_repo=None, exclusions=None, base_meta_path=None, diff_meta_path=None):
self._all_yaml_help = help_file_entries
self._loaded_help = loaded_help
self._command_loader = command_loader
Expand All @@ -63,6 +64,8 @@ def __init__(self, command_loader=None, help_file_entries=None, loaded_help=None
self.git_target = git_target
self.git_repo = git_repo
self.exclusions = exclusions
self.base_meta_path = base_meta_path
self.diff_meta_path = diff_meta_path
self.diffed_lines = set()
self._get_diffed_patches()

Expand Down Expand Up @@ -371,6 +374,21 @@ def _run_parameter_test_coverage(parameters, all_tested_command):
'Or add the parameter with missing_parameter_test_coverage rule in linter_exclusions.yml'])
return exec_state, violations

def check_examples_from_added_command(self):
if not self.diff_meta_path:
return None
added_cmd_example_counts = []
for root, _, files in os.walk(self.diff_meta_path):
for file_name in files:
diff_metadata_file = os.path.join(root, file_name)
base_metadata_file = os.path.join(self.base_meta_path, file_name)
if not os.path.exists(diff_metadata_file):
continue
added_cmd_example_counts += meta_command_example_diff(base_metadata_file, diff_metadata_file)
added_cmd_example_counts = list(map(add_cmd_example_justification, added_cmd_example_counts))
filtered_cmd_example = list(filter(lambda x: not x or not x["valid"], added_cmd_example_counts))
return filtered_cmd_example

def _get_diffed_patches(self):
if not self.git_source or not self.git_target or not self.git_repo:
return
Expand All @@ -387,17 +405,20 @@ def _get_diffed_patches(self):

# pylint: disable=too-many-instance-attributes
class LinterManager:
_RULE_TYPES = {'help_file_entries', 'command_groups', 'commands', 'params', 'command_test_coverage'}
_RULE_TYPES = {'help_file_entries', 'command_groups', 'commands', 'params', 'command_test_coverage',
'extra_cli_linter'}

def __init__(self, command_loader=None, help_file_entries=None, loaded_help=None, exclusions=None,
rule_inclusions=None, use_ci_exclusions=None, min_severity=None, update_global_exclusion=None,
git_source=None, git_target=None, git_repo=None):
git_source=None, git_target=None, git_repo=None, base_meta_path=None, diff_meta_path=None):
# default to running only rules of the highest severity
self.min_severity = min_severity or LinterSeverity.get_ordered_members()[-1]
self._exclusions = exclusions or {}
self.linter = Linter(command_loader=command_loader, help_file_entries=help_file_entries,
loaded_help=loaded_help, git_source=git_source, git_target=git_target, git_repo=git_repo,
exclusions=self._exclusions)
exclusions=self._exclusions,
base_meta_path=base_meta_path,
diff_meta_path=diff_meta_path)
self._rules = {rule_type: {} for rule_type in LinterManager._RULE_TYPES} # initialize empty rules
self._ci_exclusions = {}
self._rule_inclusions = rule_inclusions
Expand Down Expand Up @@ -440,7 +461,7 @@ def exit_code(self):
return self._exit_code

def run(self, run_params=None, run_commands=None, run_command_groups=None,
run_help_files_entries=None, run_command_test_coverage=None):
run_help_files_entries=None, run_command_test_coverage=None, run_extra_cli_linter=None):
paths = import_module('{}.rules'.format(PACKAGE_NAME)).__path__

if paths:
Expand Down Expand Up @@ -478,6 +499,9 @@ def run(self, run_params=None, run_commands=None, run_command_groups=None,
if run_command_test_coverage and self._rules.get('command_test_coverage'):
self._run_rules('command_test_coverage')

if run_extra_cli_linter and self._rules.get("extra_cli_linter"):
self._run_rules("extra_cli_linter")

if not self.exit_code:
print(os.linesep + 'No violations found for linter rules.')

Expand Down
23 changes: 22 additions & 1 deletion azdev/operations/linter/rule_decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
# Licensed under the MIT License. See License.txt in the project root for
# license information.
# -----------------------------------------------------------------------------

# pylint: disable=line-too-long
from knack.util import CLIError
from .linter import RuleError, LinterSeverity

Expand Down Expand Up @@ -86,6 +86,27 @@ def wrapper():
return add_to_linter


class ExtraCliLinterRule(BaseRule):

def __call__(self, func):
def add_to_linter(linter_manager):
def wrapper():
linter = linter_manager.linter
try:
func(linter)
except RuleError as ex:
linter_manager.mark_rule_failure(self.severity)
yield (_create_violation_msg(ex, 'Repo: {}, Src Branch: {}, Target Branch: {}, base meta path: {}, diff meta path: {} \n',
linter.git_repo, linter.git_source, linter.git_target, linter.base_meta_path, linter.diff_meta_path),
(linter.base_meta_path, linter.diff_meta_path),
func.__name__)

linter_manager.add_rule('extra_cli_linter', func.__name__, wrapper, self.severity)

add_to_linter.linter_rule = True
return add_to_linter


def _get_decorator(func, rule_group, print_format, severity):
def add_to_linter(linter_manager):
def wrapper():
Expand Down
23 changes: 23 additions & 0 deletions azdev/operations/linter/rules/extra_cli_linter_rules.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# -----------------------------------------------------------------------------
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License. See License.txt in the project root for
# license information.
# -----------------------------------------------------------------------------

from ..rule_decorators import ExtraCliLinterRule
from ..linter import RuleError, LinterSeverity

CMD_EXAMPLE_VIOLATE_MESSAGE = " cmd: `{}` should have as least {} examples, while {} detected"


@ExtraCliLinterRule(LinterSeverity.HIGH)
def missing_examples_from_added_command(linter):
filtered_cmd_example = linter.check_examples_from_added_command()

def format_cmd_example_violation_message(cmd_obj):
return CMD_EXAMPLE_VIOLATE_MESSAGE.format(cmd_obj["cmd"], cmd_obj["min_example_count"],
cmd_obj["example_count"])
if filtered_cmd_example:
violation_msg = "Please check following cmd example violation messages: \n"
violation_msg += "\n".join(list(map(format_cmd_example_violation_message, filtered_cmd_example)))
raise RuleError(violation_msg + "\n")
23 changes: 23 additions & 0 deletions azdev/operations/linter/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

from knack.log import get_logger

from azdev.operations.command_change.util import ChangeType
from azdev.utilities import get_name_index
from azdev.operations.constant import ALLOWED_HTML_TAG

Expand Down Expand Up @@ -155,3 +156,25 @@ def has_broken_site_links(help_message, filtered_lines=None):
if filtered_lines:
invalid_urls = [s for s in invalid_urls if any(s in diff_line for diff_line in filtered_lines)]
return invalid_urls


def is_substring_in_target(string_list, target):
return any(s in target for s in string_list)


# pylint: disable=line-too-long
def add_cmd_example_justification(cmd_example_count):
if "cmd" not in cmd_example_count or "param_count" not in cmd_example_count or\
"example_count" not in cmd_example_count:
return None
if cmd_example_count["change_type"] != ChangeType.ADD:
return None
cmd_name_arr = cmd_example_count["cmd"].split()
if is_substring_in_target(["add", "create", "update"], cmd_name_arr[-1]):
cmd_example_count["min_example_count"] = max(2, cmd_example_count["param_count"] / 5)
elif is_substring_in_target(["show", "list", "delete"], cmd_name_arr[-1]):
cmd_example_count["min_example_count"] = 1
else:
cmd_example_count["min_example_count"] = 1
cmd_example_count["valid"] = True if cmd_example_count["min_example_count"] <= cmd_example_count["example_count"] else False
return cmd_example_count
Loading
Loading