diff --git a/.flake8 b/.flake8 index be946cee0b0..f62e177f7ac 100644 --- a/.flake8 +++ b/.flake8 @@ -2,14 +2,20 @@ max-line-length = 120 max-complexity = 10 ignore = - E501, # line too long, it is covered by pylint - E722, # bare except, bad practice, to be removed in the future - F401, # imported but unused, too many violations, to be removed in the future - F811, # redefinition of unused, to be removed in the future - C901 # code flow is too complex, too many violations, to be removed in the future - W503 # line break before binary operator effect on readability is subjective - W504 # line break after binary operator effect on readability is subjective - + # line too long, it is covered by pylint + E501 + # bare except, bad practice, to be removed in the future + E722 + # imported but unused, too many violations, to be removed in the future + F401 + # redefinition of unused, to be removed in the future + F811 + # code flow is too complex, too many violations, to be removed in the future + C901 + # line break before binary operator effect on readability is subjective + W503 + # line break after binary operator effect on readability is subjective + W504 exclude = */vendored_sdks docs diff --git a/azure-pipelines.yml b/azure-pipelines.yml index a6bb432806e..2b7b9b4e099 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -67,20 +67,6 @@ jobs: source ./env/bin/activate azdev verify license -- job: StaticAnalysis - displayName: "Static Analysis" - pool: - name: 'pool-ubuntu-2004' - steps: - - task: UsePythonVersion@0 - displayName: 'Use Python 3.6' - inputs: - versionSpec: 3.6 - - bash: pip install wheel==0.30.0 pylint==1.9.5 flake8==3.5.0 requests - displayName: 'Install wheel, pylint, flake8, requests' - - bash: python scripts/ci/source_code_static_analysis.py - displayName: "Static Analysis" - - job: IndexVerify displayName: "Verify Extensions Index" pool: @@ -131,9 +117,10 @@ jobs: ADO_PULL_REQUEST_LATEST_COMMIT: HEAD ADO_PULL_REQUEST_TARGET_BRANCH: $(System.PullRequest.TargetBranch) -- job: LintModifiedExtensions - displayName: "CLI Linter on Modified Extensions" +- job: AzdevStyleModifiedExtensions + displayName: "azdev style on Modified Extensions" condition: and(succeeded(), eq(variables['Build.Reason'], 'PullRequest')) + continueOnError: true pool: name: 'pool-ubuntu-2004' steps: @@ -149,8 +136,33 @@ jobs: # overwrite the default AZURE_EXTENSION_DIR set by ADO AZURE_EXTENSION_DIR=~/.azure/cliextensions az --version - AZURE_EXTENSION_DIR=~/.azure/cliextensions python scripts/ci/verify_linter.py - displayName: "CLI Linter on Modified Extension" + AZURE_EXTENSION_DIR=~/.azure/cliextensions python scripts/ci/azdev_linter_style.py --type style + displayName: "azdev style on Modified Extensions" + env: + ADO_PULL_REQUEST_LATEST_COMMIT: HEAD + ADO_PULL_REQUEST_TARGET_BRANCH: $(System.PullRequest.TargetBranch) + +- job: AzdevLinterModifiedExtensions + displayName: "azdev linter on Modified Extensions" + condition: and(succeeded(), eq(variables['Build.Reason'], 'PullRequest')) + pool: + name: 'pool-ubuntu-2004' + steps: + - task: UsePythonVersion@0 + displayName: 'Use Python 3.11' + inputs: + versionSpec: 3.11 + - template: .azure-pipelines/templates/azdev_setup.yml + - bash: | + #!/usr/bin/env bash + set -ev + source ./env/bin/activate + # overwrite the default AZURE_EXTENSION_DIR set by ADO + AZURE_EXTENSION_DIR=~/.azure/cliextensions az --version + + # TODO: remove --type linter once all extensions are fixed + AZURE_EXTENSION_DIR=~/.azure/cliextensions python scripts/ci/azdev_linter_style.py --type linter + displayName: "azdev linter on Modified Extensions" env: ADO_PULL_REQUEST_LATEST_COMMIT: HEAD ADO_PULL_REQUEST_TARGET_BRANCH: $(System.PullRequest.TargetBranch) diff --git a/pylintrc b/pylintrc index dbe731101ce..6c4a8f4c7ec 100644 --- a/pylintrc +++ b/pylintrc @@ -1,27 +1,58 @@ [MASTER] ignore=tests,generated,vendored_sdks,privates +ignore-patterns=test.*,azure_devops_build.* +ignore-paths=.*/aaz reports=no [MESSAGES CONTROL] # For all codes, run 'pylint --list-msgs' or go to 'https://pylint.readthedocs.io/en/latest/reference_guide/features.html' +# cyclic-import: because of https://github.com/PyCQA/pylint/issues/850 +# import-outside-toplevel: Lazy import to improve performance # locally-disabled: Warning locally suppressed using disable-msg -# cyclic-import: Because of https://github.com/PyCQA/pylint/issues/850 # too-many-arguments: Due to the nature of the CLI many commands have large arguments set which reflect in large arguments set in corresponding methods. -# import-outside-toplevel: Lazy import to improve performance +# consider-using-f-string: str.format is still a Python standard +# unspecified-encoding: Allow using the system encoding, instead of forcing utf-8 to avoid opening errors disable= - missing-docstring, - locally-disabled, - fixme, + arguments-out-of-order, + bad-option-value, + consider-using-with, cyclic-import, - too-many-arguments, - invalid-name, duplicate-code, + fixme, import-outside-toplevel, - too-many-lines - -[TYPECHECK] -# For Azure CLI extensions, we ignore some import errors as they'll be available in the environment of the CLI -ignored-modules=azure,azure.cli,azure.cli.core,azure.cli.core.commands,knack,msrestazure,argcomplete,azure.cli.testsdk,isodate,OpenSSL + inconsistent-return-statements, + invalid-name, + invalid-str-returned, + locally-disabled, + missing-docstring, + missing-kwoa, + no-member, + no-value-for-parameter, + raise-missing-from, + subprocess-run-check, + super-with-arguments, + too-many-arguments, + too-many-function-args, + too-many-lines, + using-constant-test, + wrong-import-order, + consider-using-f-string, + unspecified-encoding, + use-maxsplit-arg, + arguments-renamed, + consider-using-in, + use-dict-literal, + consider-using-dict-items, + consider-using-enumerate, + redundant-u-string-prefix, + raising-bad-type, + unused-private-member, + used-before-assignment, + # These rules were added in Pylint >= 2.12, disable them to avoid making retroactive change + missing-timeout, + superfluous-parens, + implicit-str-concat, + unnecessary-dunder-call, [FORMAT] max-line-length=120 @@ -35,24 +66,8 @@ init-import=yes max-locals=25 # Maximum number of branch for function / method body max-branches=20 +# Temporally disable this check as inline disable is not working, see https://github.com/pylint-dev/pylint/issues/8806 +max-statements=500 [SIMILARITIES] min-similarity-lines=10 - -[BASIC] -# Naming hints based on PEP 8 (https://www.python.org/dev/peps/pep-0008/#naming-conventions). -# Consider these guidelines and not hard rules. Read PEP 8 for more details. - -# The invalid-name checker must be **enabled** for these hints to be used. -include-naming-hint=yes - -module-name-hint=lowercase (keep short; underscores are discouraged) -const-name-hint=UPPER_CASE_WITH_UNDERSCORES -class-name-hint=CapitalizedWords -class-attribute-name-hint=lower_case_with_underscores -attr-name-hint=lower_case_with_underscores -method-name-hint=lower_case_with_underscores -function-name-hint=lower_case_with_underscores -argument-name-hint=lower_case_with_underscores -variable-name-hint=lower_case_with_underscores -inlinevar-name-hint=lower_case_with_underscores (short is OK) diff --git a/scripts/ci/verify_linter.py b/scripts/ci/azdev_linter_style.py similarity index 74% rename from scripts/ci/verify_linter.py rename to scripts/ci/azdev_linter_style.py index 7fee6294307..23a06eb7d67 100644 --- a/scripts/ci/verify_linter.py +++ b/scripts/ci/azdev_linter_style.py @@ -4,22 +4,27 @@ # -------------------------------------------------------------------------------------------- """ -This script is used to verify linter on extensions. +This script is used to run azdev linter and azdev style on extensions. It's only working on ADO by default. If want to run locally, please update the target branch/commit to find diff in function find_modified_files_against_master_branch() """ - -import os import json -from subprocess import check_output, check_call -from pkg_resources import parse_version +import logging +import os +from subprocess import check_call, check_output import service_name +from pkg_resources import parse_version +logger = logging.getLogger(__name__) +logger.setLevel(logging.DEBUG) +ch = logging.StreamHandler() +ch.setLevel(logging.DEBUG) +logger.addHandler(ch) def separator_line(): - print('-' * 100) + logger.info('-' * 100) class ModifiedFilesNotAllowedError(Exception): @@ -50,7 +55,7 @@ def __init__(self, extension_name): @staticmethod def _cmd(cmd): - print(cmd) + logger.info(cmd) check_call(cmd, shell=True) def add_from_url(self, url): @@ -66,7 +71,7 @@ def __init__(self, extension_name): @staticmethod def _cmd(cmd): - print(cmd) + logger.info(cmd) check_call(cmd, shell=True) def add_from_code(self): @@ -78,6 +83,9 @@ def remove(self): def linter(self): self._cmd('azdev linter --include-whl-extensions {}'.format(self.extension_name)) + def style(self): + self._cmd('azdev style {}'.format(self.extension_name)) + def build(self): pass @@ -91,17 +99,17 @@ def find_modified_files_against_master_branch(): ado_pr_target_branch = 'origin/' + os.environ.get('ADO_PULL_REQUEST_TARGET_BRANCH') separator_line() - print('pull request target branch:', ado_pr_target_branch) + logger.info('pull request target branch: %s', ado_pr_target_branch) cmd = 'git --no-pager diff --name-only --diff-filter=ACMRT {} -- src/'.format(ado_pr_target_branch) files = check_output(cmd.split()).decode('utf-8').split('\n') files = [f for f in files if len(f) > 0] if files: - print('modified files:') + logger.info('modified files:') separator_line() for f in files: - print(f) + logger.info(f) return files @@ -131,10 +139,10 @@ def contain_extension_code(files): return False -def linter_on_external_extension(index_json): +def azdev_on_external_extension(index_json, azdev_type): """ Check if the modified metadata items in index.json refer to the extension in repo. - If not, az extension linter on wheel. Otherwise skip it. + If not, az extension check on wheel. Otherwise skip it. """ public_extensions = json.loads(check_output('az extension list-available -d', shell=True)) @@ -160,15 +168,20 @@ def linter_on_external_extension(index_json): az_extension.add_from_url(latest_entry['downloadUrl']) azdev_extension = AzdevExtensionHelper(name) - azdev_extension.linter() - - print('Checking service name for external extensions') + if azdev_type in ['all', 'linter']: + azdev_extension.linter() + # TODO: + # azdev style support external extension + # azdev test support external extension + # azdev_extension.style() + + logger.info('Checking service name for external extensions') service_name.check() az_extension.remove() -def linter_on_internal_extension(modified_files): +def azdev_on_internal_extension(modified_files, azdev_type): extension_names = set() for f in modified_files: @@ -178,22 +191,34 @@ def linter_on_internal_extension(modified_files): if not extension_names: separator_line() - print('no extension source code modified, no extension needs to be linter') + logger.info('no extension source code modified, no extension needs to be checked') for name in extension_names: separator_line() azdev_extension = AzdevExtensionHelper(name) azdev_extension.add_from_code() - azdev_extension.linter() + if azdev_type in ['all', 'linter']: + azdev_extension.linter() + if azdev_type in ['all', 'style']: + azdev_extension.style() - print('Checking service name for internal extensions') + logger.info('Checking service name for internal extensions') service_name.check() azdev_extension.remove() def main(): + import argparse + parser = argparse.ArgumentParser(description='azdev linter and azdev style on modified extensions') + parser.add_argument('--type', + type=str, + help='Control whether azdev linter, azdev style, azdev test needs to be run. ' + 'Supported values: linter, style, test, all, all is the default.', default='all') + args = parser.parse_args() + azdev_type = args.type + logger.info('azdev type: %s', azdev_type) modified_files = find_modified_files_against_master_branch() if len(modified_files) == 1 and contain_index_json(modified_files): @@ -201,8 +226,8 @@ def main(): # This scenarios is for modify index.json only. # If the modified metadata items refer to the extension code exits in this repo, PR is be created via Pipeline. # If the modified metadata items refer to the extension code doesn't exist, PR is created from Service Team. - # We try to verify linter on it. - linter_on_external_extension(modified_files[0]) + # We try to run azdev linter and azdev style on it. + azdev_on_external_extension(modified_files[0], azdev_type) else: # modified files contain more than one file @@ -211,10 +236,10 @@ def main(): if contain_index_json(modified_files): raise ModifiedFilesNotAllowedError() - linter_on_internal_extension(modified_files) + azdev_on_internal_extension(modified_files, azdev_type) else: separator_line() - print('no extension source code modified, no extension needs to be linter') + logger.info('no extension source code modified, no extension needs to be checked') separator_line() @@ -222,5 +247,5 @@ def main(): try: main() except ModifiedFilesNotAllowedError as e: - print(e) + logger.error(e) exit(1) diff --git a/scripts/ci/source_code_static_analysis.py b/scripts/ci/source_code_static_analysis.py deleted file mode 100644 index 2247e551b9f..00000000000 --- a/scripts/ci/source_code_static_analysis.py +++ /dev/null @@ -1,148 +0,0 @@ -# -------------------------------------------------------------------------------------------- -# Copyright (c) Microsoft Corporation. All rights reserved. -# Licensed under the MIT License. See License.txt in the project root for license information. -# -------------------------------------------------------------------------------------------- - -from __future__ import print_function - -import os -import sys -import multiprocessing - -from pylint import lint -from flake8.main import application - -from util import get_repo_root - -from verify_codeowners import main as _verify_codeowners - - -SDK_FILE_MARKER = r"# Code generated by Microsoft (R) AutoRest Code Generator." - - -# build list of sdk files -def _get_sdk_module_list(root_dir): - dir_paths = [os.path.join(root_dir, path) for path in os.listdir(root_dir)] - sdk_files = [] - - for path in dir_paths: - if os.path.isdir(path): - sdk_files.extend(_get_sdk_module_list(path)) - elif _is_sdk_file(path): - sdk_files.append(os.path.dirname(path)) - - return sdk_files - - -# check if the current file is a python sdk file -def _is_sdk_file(file_path): - # don't bother opening non-python files. e.g pyc files. - if not file_path.endswith(".py"): - return False - with open(file_path, encoding='utf-8') as f: - for line in f: - if SDK_FILE_MARKER in line: - return True - return False - - -# build list of modules that start with prefix "azext_" in the src -def _get_azext_module_paths(root_dir): - src_root = os.path.join(root_dir, "src") - src_paths = [os.path.join(src_root, path) for path in os.listdir(src_root)] - ext_modules = [] - for ext_root in src_paths: - if os.path.isdir(ext_root): - paths = [path for path in os.listdir(ext_root)] - ext_modules.extend([os.path.join(ext_root, path) for path in paths if path.startswith("azext_")]) - return ext_modules - - -# build list of python ci scripts -def _get_ci_py_file_paths(directory): - return [os.path.join(directory, path) for path in os.listdir(directory) if path.endswith(".py")] - - -def _run_pylint(module_paths, ignored_modules=None, rcfile=None, cpu_count=1): - pylint_opts = [] - - if ignored_modules: - pylint_opts.append("--ignore={}".format(ignored_modules)) - if rcfile: - pylint_opts.append("--rcfile={}".format(rcfile)) - - pylint_opts.append("--jobs={}".format(cpu_count)) - pylint_opts.extend(module_paths) - - try: - lint.Run(pylint_opts) - except SystemExit as se: - # 0: everything is fine - # 1: Fatal message issued - # 2: Error message issued - # 4: Warning message issued - # 8: Refactor message issued - # 16: Convention message issued - # 32: Usage error - if se.code != 0: - sys.exit(se.code) - - -def _run_flake8(module_paths, config_file=None): - flake8_opts = ["--statistics"] - - if config_file: - flake8_opts.append("--append-config={}".format(config_file)) - - flake8_opts.extend(module_paths) - - app = application.Application() - app.run(flake8_opts) - try: - app.exit() - except SystemExit: - pass - - if app.result_count > 0 or app.catastrophic_failure: - sys.exit(app.result_count or 1) - - -def main(): - cpu_count = multiprocessing.cpu_count() - - root_dir = get_repo_root() - sdk_modules = _get_sdk_module_list(root_dir) - sdk_modules.append("vendored_sdks") - module_paths = _get_azext_module_paths(root_dir) - - scripts_dir = os.path.join(root_dir, "scripts") - ci_files = _get_ci_py_file_paths(os.path.join(scripts_dir, "ci")) - - rc_file = os.path.join(root_dir, "pylintrc") - config_file = os.path.join(root_dir, ".flake8") - - print("\nRunning pylint on extensions...") - _run_pylint(module_paths, ",".join(sdk_modules), rc_file, cpu_count) - print("Pylint OK.\n") - - print("Running flake8 on extensions...") - _run_flake8(module_paths, config_file) - print("Flake8 OK.\n") - - print("Running pylint on CI scripts...") - _run_pylint(ci_files, rcfile=rc_file, cpu_count=cpu_count) - print("Pylint OK.\n") - - print("Running flake8 on CI scripts...") - _run_flake8(ci_files, config_file=config_file) - print("Pylint OK.\n") - - print("Other Static checks...") - - _verify_codeowners() - - print("All static checks successful!") - - -if __name__ == "__main__": - main() diff --git a/scripts/ci/test_source.py b/scripts/ci/test_source.py index 53aca2ae0a3..94ddf5bafd7 100755 --- a/scripts/ci/test_source.py +++ b/scripts/ci/test_source.py @@ -65,14 +65,22 @@ def run_command(cmd, check_return_code=False, cwd=None): def test_extension(): - for tname, ext_path in ALL_TESTS: + for pkg_name, ext_path in ALL_TESTS: ext_name = ext_path.split('/')[-1] logger.info(f'installing extension: {ext_name}') cmd = ['azdev', 'extension', 'add', ext_name] run_command(cmd, check_return_code=True) - # python -m azdev test --no-exitfirst --discover --verbose azext_containerapp - test_args = [sys.executable, '-m', 'azdev', 'test', '--no-exitfirst', '--discover', '--verbose', tname] + + # Use azext_$ext_name, a unique long name for testing, to avoid the following error when the main module and extension name have the same name: + # 'containerapp' exists in both 'azext_containerapp' and 'containerapp'. Resolve using `azext_containerapp.containerapp` or `containerapp.containerapp` + # 'containerapp' not found. If newly added, re-run with --discover + # No tests selected to run. + # ---------------------------------------------------------------------- + # For the recommended azdev test example, please refer to: `azdev test --help` + # `python -m azdev test --no-exitfirst --discover --verbose azext_containerapp` + test_args = [sys.executable, '-m', 'azdev', 'test', '--no-exitfirst', '--discover', '--verbose', pkg_name] logger.warning(f'test_args: {test_args}') + run_command(test_args, check_return_code=True) logger.info(f'uninstalling extension: {ext_name}') cmd = ['azdev', 'extension', 'remove', ext_name]