summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTodd Gamblin <tgamblin@llnl.gov>2021-07-06 02:43:12 -0700
committerTodd Gamblin <tgamblin@llnl.gov>2021-07-07 17:27:31 -0700
commit0dd04ffbfb97bad2dfe0530a23ab5fa8af89a26b (patch)
tree9aeb76bb08bbf2a2a852c430fcf6389a8ca80885
parent24a4d81097c2a930b34d55e25400d14b74ec4112 (diff)
downloadspack-0dd04ffbfb97bad2dfe0530a23ab5fa8af89a26b.tar.gz
spack-0dd04ffbfb97bad2dfe0530a23ab5fa8af89a26b.tar.bz2
spack-0dd04ffbfb97bad2dfe0530a23ab5fa8af89a26b.tar.xz
spack-0dd04ffbfb97bad2dfe0530a23ab5fa8af89a26b.zip
style: get close to full coverage of `spack style`
Previous tests of `spack style` didn't really run the tools -- they just ensure that the commands worked enough to get coverage. This adds several real tests and ensures that we hit the corner cases in `spack style`. This also tests sucess as well as failure cases.
-rw-r--r--lib/spack/spack/cmd/style.py48
-rw-r--r--lib/spack/spack/test/cmd/flake8.py77
-rw-r--r--lib/spack/spack/test/cmd/style.py152
3 files changed, 182 insertions, 95 deletions
diff --git a/lib/spack/spack/cmd/style.py b/lib/spack/spack/cmd/style.py
index dec8cbad4e..0eceb4dc2d 100644
--- a/lib/spack/spack/cmd/style.py
+++ b/lib/spack/spack/cmd/style.py
@@ -38,6 +38,9 @@ def grouper(iterable, n, fillvalue=None):
yield filter(None, group)
+#: directory where spack style started, for relativizing paths
+initial_working_dir = None
+
#: List of directories to exclude from checks.
exclude_directories = [spack.paths.external_path]
@@ -183,34 +186,39 @@ def setup_parser(subparser):
)
+def cwd_relative(path):
+ """Translate prefix-relative path to current working directory-relative."""
+ return os.path.relpath(os.path.join(spack.paths.prefix, path), initial_working_dir)
+
+
def rewrite_and_print_output(
output, args, re_obj=re.compile(r"^(.+):([0-9]+):"), replacement=r"{0}:{1}:"
):
"""rewrite ouput with <file>:<line>: format to respect path args"""
- if args.root_relative or re_obj is None:
- # print results relative to repo root.
- for line in output.split("\n"):
- print(" " + output)
- else:
- # print results relative to current working directory
- def cwd_relative(path):
- return replacement.format(
- os.path.relpath(
- os.path.join(spack.paths.prefix, path.group(1)), os.getcwd()
- ),
- *list(path.groups()[1:])
- )
+ # print results relative to current working directory
+ def translate(match):
+ return replacement.format(
+ cwd_relative(match.group(1)), *list(match.groups()[1:])
+ )
- for line in output.split("\n"):
- if not line:
- continue
- print(" " + re_obj.sub(cwd_relative, line))
+ for line in output.split("\n"):
+ if not line:
+ continue
+ if not args.root_relative and re_obj:
+ line = re_obj.sub(translate, line)
+ print(" " + line)
def print_style_header(file_list, args):
tools = [tool for tool in tool_order if getattr(args, tool)]
tty.msg("Running style checks on spack:", "selected: " + ", ".join(tools))
- tty.msg("Modified files:", *[filename.strip() for filename in file_list])
+
+ # translate modified paths to cwd_relative if needed
+ paths = [filename.strip() for filename in file_list]
+ if not args.root_relative:
+ paths = [cwd_relative(filename) for filename in paths]
+
+ tty.msg("Modified files:", *paths)
sys.stdout.flush()
@@ -308,6 +316,10 @@ def run_black(black_cmd, file_list, args):
def style(parser, args):
+ # save initial working directory for relativizing paths later
+ global initial_working_dir
+ initial_working_dir = os.getcwd()
+
file_list = args.files
if file_list:
diff --git a/lib/spack/spack/test/cmd/flake8.py b/lib/spack/spack/test/cmd/flake8.py
deleted file mode 100644
index c1de26a278..0000000000
--- a/lib/spack/spack/test/cmd/flake8.py
+++ /dev/null
@@ -1,77 +0,0 @@
-# Copyright 2013-2021 Lawrence Livermore National Security, LLC and other
-# Spack Project Developers. See the top-level COPYRIGHT file for details.
-#
-# SPDX-License-Identifier: (Apache-2.0 OR MIT)
-
-import argparse
-import os
-import pytest
-import sys
-
-from llnl.util.filesystem import FileFilter
-
-import spack.paths
-from spack.cmd.style import style, setup_parser, changed_files
-from spack.repo import Repo
-from spack.util.executable import which
-
-
-@pytest.fixture(scope="module")
-def parser():
- """Returns the parser for the ``flake8`` command"""
- parser = argparse.ArgumentParser()
- setup_parser(parser)
- return parser
-
-
-@pytest.fixture(scope="module")
-def flake8_package():
- """Flake8 only checks files that have been modified.
- This fixture makes a small change to the ``flake8``
- mock package, yields the filename, then undoes the
- change on cleanup.
- """
- repo = Repo(spack.paths.mock_packages_path)
- filename = repo.filename_for_package_name("flake8")
- package = FileFilter(filename)
-
- # Make the change
- package.filter("state = 'unmodified'", "state = 'modified'", string=True)
-
- yield filename
-
- # Undo the change
- package.filter("state = 'modified'", "state = 'unmodified'", string=True)
-
-
-def test_changed_files(parser, flake8_package):
- args = parser.parse_args([])
-
- # changed_files returns file paths relative to the root
- # directory of Spack. Convert to absolute file paths.
- files = changed_files(args)
- files = [os.path.join(spack.paths.prefix, path) for path in files]
-
- # There will likely be other files that have changed
- # when these tests are run
- assert flake8_package in files
-
-
-# As of flake8 3.0.0, Python 2.6 and 3.3 are no longer supported
-# http://flake8.pycqa.org/en/latest/release-notes/3.0.0.html
-@pytest.mark.skipif(
- sys.version_info[:2] <= (2, 6) or (3, 0) <= sys.version_info[:2] <= (3, 3),
- reason="flake8 no longer supports Python 2.6 or 3.3 and older",
-)
-@pytest.mark.skipif(not which("flake8"), reason="flake8 is not installed.")
-def test_flake8(parser, flake8_package):
- # Only test the flake8_package that we modified
- # Otherwise, the unit tests would fail every time
- # the flake8 tests fail
- args = parser.parse_args(["--no-mypy", flake8_package])
- style(parser, args)
- # Get even more coverage
- args = parser.parse_args(
- ["--no-mypy", "--output", "--root-relative", flake8_package]
- )
- style(parser, args)
diff --git a/lib/spack/spack/test/cmd/style.py b/lib/spack/spack/test/cmd/style.py
new file mode 100644
index 0000000000..ceaae5f694
--- /dev/null
+++ b/lib/spack/spack/test/cmd/style.py
@@ -0,0 +1,152 @@
+# Copyright 2013-2021 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+import os
+import shutil
+import sys
+
+import pytest
+
+from llnl.util.filesystem import FileFilter
+
+import spack.main
+import spack.paths
+import spack.repo
+from spack.cmd.style import changed_files
+from spack.util.executable import which
+
+style = spack.main.SpackCommand("style")
+
+
+@pytest.fixture(scope="function")
+def flake8_package():
+ """Style only checks files that have been modified. This fixture makes a small
+ change to the ``flake8`` mock package, yields the filename, then undoes the
+ change on cleanup.
+ """
+ repo = spack.repo.Repo(spack.paths.mock_packages_path)
+ filename = repo.filename_for_package_name("flake8")
+ tmp = filename + ".tmp"
+
+ try:
+ shutil.copy(filename, tmp)
+ package = FileFilter(filename)
+ package.filter("state = 'unmodified'", "state = 'modified'", string=True)
+ yield filename
+ finally:
+ shutil.move(tmp, filename)
+
+
+@pytest.fixture
+def flake8_package_with_errors(scope="function"):
+ """A flake8 package with errors."""
+ repo = spack.repo.Repo(spack.paths.mock_packages_path)
+ filename = repo.filename_for_package_name("flake8")
+ tmp = filename + ".tmp"
+
+ try:
+ shutil.copy(filename, tmp)
+ package = FileFilter(filename)
+ package.filter("state = 'unmodified'", "state = 'modified'", string=True)
+ yield filename
+ finally:
+ shutil.move(tmp, filename)
+
+
+def test_changed_files(flake8_package):
+ # changed_files returns file paths relative to the root
+ # directory of Spack. Convert to absolute file paths.
+ files = [os.path.join(spack.paths.prefix, path) for path in changed_files()]
+
+ # There will likely be other files that have changed
+ # when these tests are run
+ assert flake8_package in files
+
+
+def test_changed_files_all_files(flake8_package):
+ # it's hard to guarantee "all files", so do some sanity checks.
+ files = set([
+ os.path.join(spack.paths.prefix, path)
+ for path in changed_files(all_files=True)
+ ])
+
+ # spack has a lot of files -- check that we're in the right ballpark
+ assert len(files) > 6000
+
+ # a builtin package
+ zlib = spack.repo.path.get_pkg_class("zlib")
+ assert zlib.module.__file__ in files
+
+ # a core spack file
+ assert os.path.join(spack.paths.module_path, "spec.py") in files
+
+ # a mock package
+ assert flake8_package in files
+
+ # this test
+ assert __file__ in files
+
+ # ensure externals are excluded
+ assert not any(f.startswith(spack.paths.external_path) for f in files)
+
+
+# As of flake8 3.0.0, Python 2.6 and 3.3 are no longer supported
+# http://flake8.pycqa.org/en/latest/release-notes/3.0.0.html
+skip_old_python = pytest.mark.skipif(
+ sys.version_info[:2] <= (2, 6) or (3, 0) <= sys.version_info[:2] <= (3, 3),
+ reason="flake8 no longer supports Python 2.6 or 3.3 and older",
+)
+
+
+@skip_old_python
+@pytest.mark.skipif(not which("flake8"), reason="flake8 is not installed.")
+def test_style(flake8_package, tmpdir):
+ root_relative = os.path.relpath(flake8_package, spack.paths.prefix)
+
+ # use a working directory to test cwd-relative paths, as tests run in
+ # the spack prefix by default
+ with tmpdir.as_cwd():
+ relative = os.path.relpath(flake8_package)
+
+ # no args
+ output = style()
+ assert relative in output
+ assert "spack style checks were clean" in output
+
+ # one specific arg
+ output = style(flake8_package)
+ assert relative in output
+ assert "spack style checks were clean" in output
+
+ # specific file that isn't changed
+ output = style(__file__)
+ 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)
+ assert root_relative in output
+ assert "spack style checks were clean" in output
+
+
+@skip_old_python
+@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)
+ assert root_relative in output
+ assert style.returncode == 1
+ 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.")
+def test_style_with_black(flake8_package_with_errors):
+ output = style("--black", flake8_package_with_errors, fail_on_error=False)
+ assert "black found errors" in output
+ assert style.returncode == 1
+ assert "spack style found errors" in output