diff options
Diffstat (limited to 'lib')
| -rw-r--r-- | lib/spack/spack/cmd/style.py | 88 | ||||
| -rw-r--r-- | lib/spack/spack/test/cmd/style.py | 38 |
2 files changed, 68 insertions, 58 deletions
diff --git a/lib/spack/spack/cmd/style.py b/lib/spack/spack/cmd/style.py index 3d24228a54..dc4426c8b9 100644 --- a/lib/spack/spack/cmd/style.py +++ b/lib/spack/spack/cmd/style.py @@ -46,7 +46,8 @@ exclude_directories = [ #: Order in which tools should be run. flake8 is last so that it can #: double-check the results of other tools (if, e.g., --fix was provided) -#: The list maps an executable name to a spack spec needed to install it. +#: The list maps an executable name to a method to ensure the tool is +#: bootstrapped or present in the environment. tool_order = [ ("isort", spack.bootstrap.ensure_isort_in_path_or_raise), ("mypy", spack.bootstrap.ensure_mypy_in_path_or_raise), @@ -54,6 +55,9 @@ tool_order = [ ("flake8", spack.bootstrap.ensure_flake8_in_path_or_raise), ] +#: list of just the tool names -- for argparse +tool_names = [k for k, _ in tool_order] + #: tools we run in spack style tools = {} @@ -180,35 +184,27 @@ def setup_parser(subparser): help="format automatically if possible (e.g., with isort, black)", ) subparser.add_argument( - "--no-isort", - dest="isort", - action="store_false", - help="do not run isort (default: run isort if available)", - ) - subparser.add_argument( - "--no-flake8", - dest="flake8", - action="store_false", - help="do not run flake8 (default: run flake8 or fail)", - ) - subparser.add_argument( - "--no-mypy", - dest="mypy", - action="store_false", - help="do not run mypy (default: run mypy if available)", - ) - subparser.add_argument( - "--no-black", - dest="black", - action="store_false", - help="run black if available (default: run black if available)", - ) - subparser.add_argument( "--root", action="store", default=None, help="style check a different spack instance", ) + + tool_group = subparser.add_mutually_exclusive_group() + tool_group.add_argument( + "-t", + "--tool", + action="append", + help="specify which tools to run (default: %s)" % ",".join(tool_names), + ) + tool_group.add_argument( + "-s", + "--skip", + metavar="TOOL", + action="append", + help="specify tools to skip (choose from %s)" % ",".join(tool_names), + ) + subparser.add_argument("files", nargs=argparse.REMAINDER, help="specific files to check") @@ -233,8 +229,8 @@ def rewrite_and_print_output( print(line) -def print_style_header(file_list, args): - tools = [tool for tool, _ in tool_order if getattr(args, tool)] +def print_style_header(file_list, args, selected): + tools = [tool for tool in tool_names if tool in selected] tty.msg("Running style checks on spack", "selected: " + ", ".join(tools)) # translate modified paths to cwd_relative if needed @@ -394,6 +390,15 @@ def run_black(black_cmd, file_list, args): return returncode +def validate_toolset(arg_value): + """Validate --tool and --skip arguments (sets of optionally comma-separated tools).""" + tools = set(",".join(arg_value).split(",")) # allow args like 'isort,flake8' + for tool in tools: + if tool not in tool_names: + tty.die("Invaild tool: '%s'" % tool, "Choose from: %s" % ", ".join(tool_names)) + return tools + + def style(parser, args): # ensure python version is new enough if sys.version_info < (3, 6): @@ -421,26 +426,33 @@ def style(parser, args): file_list = [prefix_relative(p) for p in file_list] + # process --tool and --skip arguments + selected = set(tool_names) + if args.tool is not None: + selected = validate_toolset(args.tool) + if args.skip is not None: + selected -= validate_toolset(args.skip) + + if not selected: + tty.msg("Nothing to run.") + return + return_code = 0 with working_dir(args.root): if not file_list: file_list = changed_files(args.base, args.untracked, args.all) - print_style_header(file_list, args) + print_style_header(file_list, args, selected) + + tools_to_run = [(tool, fn) for tool, fn in tool_order if tool in selected] commands = {} with spack.bootstrap.ensure_bootstrap_configuration(): - for tool_name, bootstrap_fn in tool_order: - # Skip the tool if it was not requested - if not getattr(args, tool_name): - continue - + # bootstrap everything first to get commands + for tool_name, bootstrap_fn in tools_to_run: commands[tool_name] = bootstrap_fn() - for tool_name, bootstrap_fn in tool_order: - # Skip the tool if it was not requested - if not getattr(args, tool_name): - continue - + # run tools once bootstrapping is done + for tool_name, bootstrap_fn in tools_to_run: run_function, required = tools[tool_name] print_tool_header(tool_name) return_code |= run_function(commands[tool_name], file_list, args) diff --git a/lib/spack/spack/test/cmd/style.py b/lib/spack/spack/test/cmd/style.py index 35e03b0387..7a5926ae29 100644 --- a/lib/spack/spack/test/cmd/style.py +++ b/lib/spack/spack/test/cmd/style.py @@ -224,8 +224,8 @@ def external_style_root(flake8_package_with_errors, tmpdir): @skip_old_python +@pytest.mark.skipif(not which("isort"), reason="isort is not installed.") @pytest.mark.skipif(not which("black"), reason="black is not installed.") -@pytest.mark.skipif(not which("flake8"), reason="flake8 is not installed.") def test_fix_style(external_style_root): """Make sure spack style --fix works.""" tmpdir, py_file = external_style_root @@ -237,14 +237,9 @@ def test_fix_style(external_style_root): shutil.copy(broken_dummy, broken_py) assert not filecmp.cmp(broken_py, fixed_py) - output = style( - "--root", - str(tmpdir), - "--no-mypy", # mypy doesn't fix, so skip it - "--no-flake8", # flake8 doesn't fix, so skip it - "--fix", - ) - print(output) + # black and isort are the tools that actually fix things + style("--root", str(tmpdir), "--tool", "isort,black", "--fix") + assert filecmp.cmp(broken_py, fixed_py) @@ -288,24 +283,19 @@ def test_style(flake8_package, tmpdir): with tmpdir.as_cwd(): relative = os.path.relpath(flake8_package) - # no args - output = style(fail_on_error=False) - assert relative in output - assert "spack style checks were clean" in output - # one specific arg - output = style(flake8_package, fail_on_error=False) + output = style("--tool", "flake8", flake8_package, fail_on_error=False) assert relative in output assert "spack style checks were clean" in output # specific file that isn't changed - output = style(__file__, fail_on_error=False) + output = style("--tool", "flake8", __file__, fail_on_error=False) assert relative not in output assert __file__ in output assert "spack style checks were clean" in output # root-relative paths - output = style("--root-relative", flake8_package) + output = style("--tool", "flake8", "--root-relative", flake8_package) assert root_relative in output assert "spack style checks were clean" in output @@ -314,17 +304,25 @@ def test_style(flake8_package, tmpdir): @pytest.mark.skipif(not which("flake8"), reason="flake8 is not installed.") def test_style_with_errors(flake8_package_with_errors): root_relative = os.path.relpath(flake8_package_with_errors, spack.paths.prefix) - output = style("--root-relative", flake8_package_with_errors, fail_on_error=False) + output = style( + "--tool", "flake8", "--root-relative", flake8_package_with_errors, fail_on_error=False + ) assert root_relative in output assert style.returncode != 0 assert "spack style found errors" in output @skip_old_python -@pytest.mark.skipif(not which("flake8"), reason="flake8 is not installed.") @pytest.mark.skipif(not which("black"), reason="black is not installed.") +@pytest.mark.skipif(not which("flake8"), reason="flake8 is not installed.") def test_style_with_black(flake8_package_with_errors): - output = style(flake8_package_with_errors, fail_on_error=False) + output = style("--tool", "black,flake8", flake8_package_with_errors, fail_on_error=False) assert "black found errors" in output assert style.returncode != 0 assert "spack style found errors" in output + + +@skip_old_python +def test_skip_tools(): + output = style("--skip", "isort,mypy,black,flake8") + assert "Nothing to run" in output |