summaryrefslogtreecommitdiff
path: root/lib
diff options
context:
space:
mode:
Diffstat (limited to 'lib')
-rw-r--r--lib/spack/spack/cmd/style.py88
-rw-r--r--lib/spack/spack/test/cmd/style.py38
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