From 8e8700330660cba8cf0b93fe47d8066fac0d88a1 Mon Sep 17 00:00:00 2001 From: Harold Zeng Date: Sun, 28 Jun 2020 17:18:09 +0800 Subject: [PATCH 1/5] hornor flake and pylint settings of CLI and Extensions repo --- azdev/operations/style.py | 108 +++++++++++++++++++++++++++----------- 1 file changed, 77 insertions(+), 31 deletions(-) diff --git a/azdev/operations/style.py b/azdev/operations/style.py index 592630ded..dda67fc7a 100644 --- a/azdev/operations/style.py +++ b/azdev/operations/style.py @@ -14,7 +14,7 @@ from azdev.utilities import ( display, heading, subheading, py_cmd, get_path_table, EXTENSION_PREFIX, - get_azdev_config_dir, require_azure_cli, filter_by_git_diff) + get_azdev_config, get_azdev_config_dir, require_azure_cli, filter_by_git_diff) logger = get_logger(__name__) @@ -125,62 +125,108 @@ def apply_result(item): final_result.result += item.result else: final_result.result = item.result + apply_result(cli_result) apply_result(ext_result) return final_result def _run_pylint(modules): - def get_core_module_paths(modules): core_paths = [] - for p in modules['core'].values(): + for p in modules["core"].values(): _, tail = os.path.split(p) - for x in str(tail).split('-'): + for x in str(tail).split("-"): p = os.path.join(p, x) core_paths.append(p) return core_paths - cli_paths = get_core_module_paths(modules) + list(modules['mod'].values()) + cli_paths = get_core_module_paths(modules) + list(modules["mod"].values()) ext_paths = [] - for path in list(modules['ext'].values()): - glob_pattern = os.path.normcase(os.path.join('{}*'.format(EXTENSION_PREFIX))) + for path in list(modules["ext"].values()): + glob_pattern = os.path.normcase(os.path.join("{}*".format(EXTENSION_PREFIX))) ext_paths.append(glob(os.path.join(path, glob_pattern))[0]) def run(paths, rcfile, desc): if not paths: return None - config_path = os.path.join(get_azdev_config_dir(), 'config_files', rcfile) - logger.info('Using rcfile file: %s', config_path) - logger.info('Running on %s: %s', desc, ' '.join(paths)) - command = 'pylint {} --ignore vendored_sdks,privates --rcfile={} -j {}'.format( - ' '.join(paths), - config_path, - multiprocessing.cpu_count()) - return py_cmd(command, message='Running pylint on {}...'.format(desc)) - - cli_result = run(cli_paths, 'cli_pylintrc', 'modules') - ext_result = run(ext_paths, 'ext_pylintrc', 'extensions') + logger.warning("Using rcfile file: %s", rcfile) + logger.warning("Running on %s: %s", desc, " ".join(paths)) + command = "pylint {} --ignore vendored_sdks,privates --rcfile={} -j {}".format( + " ".join(paths), rcfile, multiprocessing.cpu_count() + ) + return py_cmd(command, message="Running pylint on {}...".format(desc)) + + cli_pylintrc, ext_pylintrc = _config_file_path("pylint") + + cli_result = run(cli_paths, cli_pylintrc, "modules") + ext_result = run(ext_paths, ext_pylintrc, "extensions") return _combine_command_result(cli_result, ext_result) def _run_pep8(modules): - cli_paths = list(modules['core'].values()) + list(modules['mod'].values()) - ext_paths = list(modules['ext'].values()) + cli_paths = list(modules["core"].values()) + list(modules["mod"].values()) + ext_paths = list(modules["ext"].values()) - def run(paths, config_file, desc): + def run(paths, rcfile, desc): if not paths: return None - config_path = os.path.join(get_azdev_config_dir(), 'config_files', config_file) - logger.info('Using config file: %s', config_path) - logger.info('Running on %s: %s', desc, ' '.join(paths)) - command = 'flake8 --statistics --append-config={} {}'.format( - config_path, - ' '.join(paths)) - return py_cmd(command, message='Running flake8 on {}...'.format(desc)) - - cli_result = run(cli_paths, 'cli.flake8', 'modules') - ext_result = run(ext_paths, 'ext.flake8', 'extensions') + logger.warning("Using config file: %s", rcfile) + logger.warning("Running on %s: %s", desc, " ".join(paths)) + command = "flake8 --statistics --append-config={} {}".format( + rcfile, " ".join(paths) + ) + return py_cmd(command, message="Running flake8 on {}...".format(desc)) + + cli_config, ext_config = _config_file_path("flake8") + + cli_result = run(cli_paths, cli_config, "modules") + ext_result = run(ext_paths, ext_config, "extensions") return _combine_command_result(cli_result, ext_result) + + +def _config_file_path(style_type="pylint"): + cli_repo_path = get_azdev_config().get("cli", "repo_path") + + ext_repo_path = list( + filter( + lambda x: "azure-cli-extension" in x, + get_azdev_config().get("ext", "repo_paths").split(), + ) + )[0] + + if style_type not in ["pylint", "flake8"]: + raise ValueError("style_tyle value allows only: pylint, flake8.") + + config_file_mapping = { + "pylint": "pylintrc", + "flake8": ".flake8", + } + default_config_file_mapping = { + "cli_pylintrc": "cli_pylintrc", + "ext_pylintrc": "ext_pylintrc", + "cli.flake8": "cli.flake8", + "ext.flake8": "ext.flake8", + } + + if cli_repo_path: + cli_config_path = os.path.join(cli_repo_path, config_file_mapping[style_type]) + else: + cli_config_path = os.path.join( + get_azdev_config_dir(), + "config_files", + default_config_file_mapping["cli_pylintrc"], + ) + + if ext_repo_path: + ext_config_path = os.path.join(ext_repo_path, config_file_mapping[style_type]) + else: + ext_config_path = os.path.join( + get_azdev_config_dir(), + "config_files", + default_config_file_mapping["ext_pylintrc"], + ) + + return cli_config_path, ext_config_path From 3cb58ebb022f0469faf731182bfc6cfa1fa51901 Mon Sep 17 00:00:00 2001 From: Harold Zeng Date: Sun, 28 Jun 2020 17:30:00 +0800 Subject: [PATCH 2/5] fix default value bug --- azdev/operations/style.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/azdev/operations/style.py b/azdev/operations/style.py index dda67fc7a..2a23ec261 100644 --- a/azdev/operations/style.py +++ b/azdev/operations/style.py @@ -205,10 +205,14 @@ def _config_file_path(style_type="pylint"): "flake8": ".flake8", } default_config_file_mapping = { - "cli_pylintrc": "cli_pylintrc", - "ext_pylintrc": "ext_pylintrc", - "cli.flake8": "cli.flake8", - "ext.flake8": "ext.flake8", + "cli": { + "pylint": "cli_pylintrc", + "flake8": "cli.flake8" + }, + "ext": { + "pylint": "ext_pylintrc", + "flake8": "ext.flake8" + } } if cli_repo_path: @@ -217,7 +221,7 @@ def _config_file_path(style_type="pylint"): cli_config_path = os.path.join( get_azdev_config_dir(), "config_files", - default_config_file_mapping["cli_pylintrc"], + default_config_file_mapping["cli"][style_type], ) if ext_repo_path: @@ -226,7 +230,7 @@ def _config_file_path(style_type="pylint"): ext_config_path = os.path.join( get_azdev_config_dir(), "config_files", - default_config_file_mapping["ext_pylintrc"], + default_config_file_mapping["ext"][style_type], ) return cli_config_path, ext_config_path From eda22e601ffc87c0986e030f0f9c3a66a257eea2 Mon Sep 17 00:00:00 2001 From: Harold Zeng Date: Sun, 28 Jun 2020 17:51:42 +0800 Subject: [PATCH 3/5] fix bug that azure-cli-extension not setup --- azdev/operations/style.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/azdev/operations/style.py b/azdev/operations/style.py index 2a23ec261..6574583e4 100644 --- a/azdev/operations/style.py +++ b/azdev/operations/style.py @@ -190,12 +190,14 @@ def run(paths, rcfile, desc): def _config_file_path(style_type="pylint"): cli_repo_path = get_azdev_config().get("cli", "repo_path") - ext_repo_path = list( - filter( - lambda x: "azure-cli-extension" in x, - get_azdev_config().get("ext", "repo_paths").split(), - ) - )[0] + ext_repo_path = filter( + lambda x: "azure-cli-extension" in x, + get_azdev_config().get("ext", "repo_paths").split(), + ) + try: + ext_repo_path = next(ext_repo_path) + except StopIteration: + ext_repo_path = [] if style_type not in ["pylint", "flake8"]: raise ValueError("style_tyle value allows only: pylint, flake8.") From bf21687cb5f574d684fa7d00e448ad0da653b5d3 Mon Sep 17 00:00:00 2001 From: Harold Zeng Date: Mon, 13 Jul 2020 13:57:37 +0800 Subject: [PATCH 4/5] make warning message to debug --- azdev/operations/style.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/azdev/operations/style.py b/azdev/operations/style.py index 6574583e4..4c5b2a9be 100644 --- a/azdev/operations/style.py +++ b/azdev/operations/style.py @@ -151,8 +151,8 @@ def get_core_module_paths(modules): def run(paths, rcfile, desc): if not paths: return None - logger.warning("Using rcfile file: %s", rcfile) - logger.warning("Running on %s: %s", desc, " ".join(paths)) + logger.debug("Using rcfile file: %s", rcfile) + logger.debug("Running on %s: %s", desc, "\n".join(paths)) command = "pylint {} --ignore vendored_sdks,privates --rcfile={} -j {}".format( " ".join(paths), rcfile, multiprocessing.cpu_count() ) @@ -173,8 +173,8 @@ def _run_pep8(modules): def run(paths, rcfile, desc): if not paths: return None - logger.warning("Using config file: %s", rcfile) - logger.warning("Running on %s: %s", desc, " ".join(paths)) + logger.debug("Using config file: %s", rcfile) + logger.debug("Running on %s:\n%s", desc, "\n".join(paths)) command = "flake8 --statistics --append-config={} {}".format( rcfile, " ".join(paths) ) From 32a2260e5a4c9063efff1ad51d4352e8297d3b98 Mon Sep 17 00:00:00 2001 From: Harold Zeng Date: Thu, 16 Jul 2020 15:19:53 +0800 Subject: [PATCH 5/5] Add TestConfigFilePath --- azdev/operations/tests/__init__.py | 5 ++ azdev/operations/tests/test_style.py | 97 ++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+) create mode 100644 azdev/operations/tests/__init__.py create mode 100644 azdev/operations/tests/test_style.py diff --git a/azdev/operations/tests/__init__.py b/azdev/operations/tests/__init__.py new file mode 100644 index 000000000..99c0f28cd --- /dev/null +++ b/azdev/operations/tests/__init__.py @@ -0,0 +1,5 @@ +# ----------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for +# license information. +# ----------------------------------------------------------------------------- diff --git a/azdev/operations/tests/test_style.py b/azdev/operations/tests/test_style.py new file mode 100644 index 000000000..701f472cb --- /dev/null +++ b/azdev/operations/tests/test_style.py @@ -0,0 +1,97 @@ +# ----------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for +# license information. +# ----------------------------------------------------------------------------- + +import configparser +import unittest +from unittest import mock + +from azdev.operations.style import _config_file_path + + +class TestConfigFilePath(unittest.TestCase): + def test_unsupported_code_style_checker(self): + with self.assertRaises(ValueError): + _config_file_path(style_type="unknown") + + def test_pylint_config_without_setup(self): + mocked_config = configparser.ConfigParser() + mocked_config.add_section("cli") + mocked_config.set("cli", "repo_path", "") + mocked_config.add_section("ext") + mocked_config.set("ext", "repo_paths", "") + + with mock.patch("azdev.operations.style.get_azdev_config", return_value=mocked_config): + r = _config_file_path(style_type="pylint") + self.assertTrue(r[0].endswith("/config_files/cli_pylintrc")) + self.assertTrue(r[1].endswith("/config_files/ext_pylintrc")) + + def test_pylint_config_with_partially_setup(self): + cli_repo_path = "~/Azure/azure-cli" + mocked_config = configparser.ConfigParser() + mocked_config.add_section("cli") + mocked_config.set("cli", "repo_path", cli_repo_path) + mocked_config.add_section("ext") + mocked_config.set("ext", "repo_paths", "") + + with mock.patch("azdev.operations.style.get_azdev_config", return_value=mocked_config): + r = _config_file_path(style_type="pylint") + self.assertEqual(r[0], cli_repo_path + "/pylintrc") + self.assertTrue(r[1].endswith("/config_files/ext_pylintrc")) + + def test_pylint_config_with_all_setup(self): + cli_repo_path = "~/Azure/azure-cli" + ext_repo_path = "~/Azure/azure-cli-extensions" + mocked_config = configparser.ConfigParser() + mocked_config.add_section("cli") + mocked_config.set("cli", "repo_path", cli_repo_path) + mocked_config.add_section("ext") + mocked_config.set("ext", "repo_paths", ext_repo_path) + + with mock.patch("azdev.operations.style.get_azdev_config", return_value=mocked_config): + r = _config_file_path() + self.assertEqual(r[0], cli_repo_path + "/pylintrc") + self.assertTrue(r[1], "/pylintrc") + + def test_flake8_config_wihtout_setup(self): + mocked_config = configparser.ConfigParser() + mocked_config.add_section("cli") + mocked_config.set("cli", "repo_path", "") + mocked_config.add_section("ext") + mocked_config.set("ext", "repo_paths", "") + + with mock.patch("azdev.operations.style.get_azdev_config", return_value=mocked_config): + r = _config_file_path(style_type="flake8") + self.assertTrue(r[0].endswith("/config_files/cli.flake8")) + self.assertTrue(r[1].endswith("/config_files/ext.flake8")) + + def test_flake8_config_with_partially_setup(self): + ext_repo_path = "~/Azure/azure-cli-extensions" + + mocked_config = configparser.ConfigParser() + mocked_config.add_section("cli") + mocked_config.set("cli", "repo_path", "") + mocked_config.add_section("ext") + mocked_config.set("ext", "repo_paths", ext_repo_path) + + with mock.patch("azdev.operations.style.get_azdev_config", return_value=mocked_config): + r = _config_file_path(style_type="flake8") + self.assertTrue(r[0].endswith("/config_files/cli.flake8")) + self.assertTrue(r[1].endswith(ext_repo_path + "/.flake8")) + + def test_flake9_config_with_all_setup(self): + cli_repo_path = "~/Azure/azure-cli" + ext_repo_path = "~/Azure/azure-cli-extensions" + + mocked_config = configparser.ConfigParser() + mocked_config.add_section("cli") + mocked_config.set("cli", "repo_path", cli_repo_path) + mocked_config.add_section("ext") + mocked_config.set("ext", "repo_paths", ext_repo_path) + + with mock.patch("azdev.operations.style.get_azdev_config", return_value=mocked_config): + r = _config_file_path(style_type="flake8") + self.assertTrue(r[0].endswith(cli_repo_path + "/.flake8")) + self.assertTrue(r[1].endswith(ext_repo_path + "/.flake8"))