diff options
6 files changed, 90 insertions, 117 deletions
diff --git a/lib/spack/spack/test/cmd/extensions.py b/lib/spack/spack/test/cmd/extensions.py index 14747d0439..88f073519c 100644 --- a/lib/spack/spack/test/cmd/extensions.py +++ b/lib/spack/spack/test/cmd/extensions.py @@ -39,13 +39,11 @@ def test_extensions(mock_packages, python_database, config, capsys): installed = extensions("-s", "installed", "python") activated = extensions("-s", "activated", "python") assert "==> python@2.7.11" in output - assert "==> 3 extensions" in output - assert "flake8" in output + assert "==> 2 extensions" in output assert "py-extension1" in output assert "py-extension2" in output - assert "==> 3 extensions" in packages - assert "flake8" in packages + assert "==> 2 extensions" in packages assert "py-extension1" in packages assert "py-extension2" in packages assert "installed" not in packages diff --git a/lib/spack/spack/test/cmd/resource.py b/lib/spack/spack/test/cmd/resource.py index 64a79fd601..c6dd537693 100644 --- a/lib/spack/spack/test/cmd/resource.py +++ b/lib/spack/spack/test/cmd/resource.py @@ -18,7 +18,6 @@ mock_hashes = [ 'c45c1564f70def3fc1a6e22139f62cb21cd190cc3a7dbe6f4120fa59ce33dcb8', '24eceabef5fe8f575ff4b438313dc3e7b30f6a2d1c78841fbbe3b9293a589277', '689b8f9b32cb1d2f9271d29ea3fca2e1de5df665e121fca14e1364b711450deb', - 'ebe27f9930b99ebd8761ed2db3ea365142d0bafd78317efb4baadf62c7bf94d0', '208fcfb50e5a965d5757d151b675ca4af4ce2dfd56401721b6168fae60ab798f', 'bf07a7fbb825fc0aae7bf4a1177b2b31fcf8a3feeaf7092761e18c859ee52a9c', '7d865e959b2466918c9863afca942d0fb89d7c9ac0c99bafc3749504ded97730', diff --git a/lib/spack/spack/test/cmd/style.py b/lib/spack/spack/test/cmd/style.py index 2b8330aa59..35e03b0387 100644 --- a/lib/spack/spack/test/cmd/style.py +++ b/lib/spack/spack/test/cmd/style.py @@ -19,29 +19,29 @@ from spack.cmd.style import changed_files from spack.util.executable import which #: directory with sample style files -style_data = os.path.join(spack.paths.test_path, 'data', 'style') +style_data = os.path.join(spack.paths.test_path, "data", "style") style = spack.main.SpackCommand("style") def has_develop_branch(): - git = which('git') + git = which("git") if not git: return False - git("show-ref", "--verify", "--quiet", - "refs/heads/develop", fail_on_error=False) + git("show-ref", "--verify", "--quiet", "refs/heads/develop", fail_on_error=False) return git.returncode == 0 # spack style requires git to run -- skip the tests if it's not there -pytestmark = pytest.mark.skipif(not has_develop_branch(), - reason='requires git with develop branch') +pytestmark = pytest.mark.skipif( + not has_develop_branch(), reason="requires git with develop branch" +) # The style tools have requirements to use newer Python versions. We simplify by # requiring Python 3.6 or higher to run spack style. skip_old_python = pytest.mark.skipif( - sys.version_info < (3, 6), reason='requires Python 3.6 or higher' + sys.version_info < (3, 6), reason="requires Python 3.6 or higher" ) @@ -74,11 +74,13 @@ def flake8_package_with_errors(scope="function"): try: shutil.copy(filename, tmp) package = FileFilter(filename) - package.filter("state = 'unmodified'", "state = 'modified'", string=True) + + # this is a black error (quote style and spacing before/after operator) + package.filter('state = "unmodified"', "state = 'modified'", string=True) + + # this is an isort error (orderign) and a flake8 error (unused import) package.filter( - "from spack.package import *", - "from spack.package import *\nimport os", - string=True + "from spack.package import *", "from spack.package import *\nimport os", string=True ) yield filename finally: @@ -88,10 +90,7 @@ def flake8_package_with_errors(scope="function"): 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, os.path.normpath(path)) - for path in changed_files() - ] + files = [os.path.join(spack.paths.prefix, os.path.normpath(path)) for path in changed_files()] # There will likely be other files that have changed # when these tests are run @@ -108,11 +107,11 @@ def test_changed_files_from_git_rev_base(tmpdir, capfd): git("config", "user.email", "test@user.com") git("commit", "--allow-empty", "-m", "initial commit") - tmpdir.ensure('bin/spack') - assert changed_files(base="HEAD") == ['bin/spack'] - assert changed_files(base="main") == ['bin/spack'] + tmpdir.ensure("bin/spack") + assert changed_files(base="HEAD") == ["bin/spack"] + assert changed_files(base="main") == ["bin/spack"] - git("add", 'bin/spack') + git("add", "bin/spack") git("commit", "-m", "v1") assert changed_files(base="HEAD") == [] assert changed_files(base="HEAD~") == ["bin/spack"] @@ -138,10 +137,12 @@ def test_changed_no_base(tmpdir, capfd): 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, os.path.normpath(path)) - for path in changed_files(all_files=True) - ]) + files = set( + [ + os.path.join(spack.paths.prefix, os.path.normpath(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 @@ -180,12 +181,8 @@ def test_bad_root(tmpdir): def test_style_is_package(tmpdir): """Ensure the is_package() function works.""" - assert spack.cmd.style.is_package( - "var/spack/repos/builtin/packages/hdf5/package.py" - ) - assert spack.cmd.style.is_package( - "var/spack/repos/builtin/packages/zlib/package.py" - ) + assert spack.cmd.style.is_package("var/spack/repos/builtin/packages/hdf5/package.py") + assert spack.cmd.style.is_package("var/spack/repos/builtin/packages/zlib/package.py") assert not spack.cmd.style.is_package("lib/spack/spack/spec.py") assert not spack.cmd.style.is_package("lib/spack/external/pytest.py") @@ -241,10 +238,10 @@ def test_fix_style(external_style_root): assert not filecmp.cmp(broken_py, fixed_py) output = style( - "--root", str(tmpdir), - "--no-mypy", # mypy doesn't fix, so skip it + "--root", + str(tmpdir), + "--no-mypy", # mypy doesn't fix, so skip it "--no-flake8", # flake8 doesn't fix, so skip it - "--black", "--fix", ) print(output) @@ -262,10 +259,7 @@ def test_external_root(external_style_root): # make sure tools are finding issues with external root, # not the real one. - output = style( - "--root-relative", "--black", "--root", str(tmpdir), - fail_on_error=False - ) + output = style("--root-relative", "--root", str(tmpdir), fail_on_error=False) # make sure it failed assert style.returncode != 0 @@ -330,7 +324,7 @@ def test_style_with_errors(flake8_package_with_errors): @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) + output = style(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 diff --git a/share/spack/qa/flake8_formatter.py b/share/spack/qa/flake8_formatter.py index 523ee7b4d0..93cb4cf3fb 100644 --- a/share/spack/qa/flake8_formatter.py +++ b/share/spack/qa/flake8_formatter.py @@ -21,24 +21,6 @@ pattern_exemptions = { r"^from spack.package import \*$", r"^from spack.package_defs import \*$", ], - # Exempt lines with urls and descriptions from overlong line errors. - "E501": [ - r"^\s*homepage\s*=", - r"^\s*url\s*=", - r"^\s*git\s*=", - r"^\s*svn\s*=", - r"^\s*hg\s*=", - r"^\s*pypi\s*=", - r"^\s*list_url\s*=", - r"^\s*version\(", - r"^\s*variant\(", - r"^\s*provides\(", - r"^\s*extends\(", - r"^\s*depends_on\(", - r"^\s*conflicts\(", - r"^\s*resource\(", - r"^\s*patch\(", - ], # Exempt '@when' decorated functions from redefinition errors. "F811": [ r"^\s*@when\(.*\)", @@ -47,7 +29,7 @@ pattern_exemptions = { # exemptions applied to all files. r".py$": { "E501": [ - r"(https?|ftp|file)\:", # URLs + r"(ssh|https?|ftp|file)\:", # URLs r'([\'"])[0-9a-fA-F]{32,}\1', # long hex checksums ] }, @@ -58,10 +40,7 @@ pattern_exemptions = { pattern_exemptions = dict( ( re.compile(file_pattern), - dict( - (code, [re.compile(p) for p in patterns]) - for code, patterns in error_dict.items() - ), + dict((code, [re.compile(p) for p in patterns]) for code, patterns in error_dict.items()), ) for file_pattern, error_dict in pattern_exemptions.items() ) @@ -105,9 +84,7 @@ class SpackFormatter(Pylint): # get list of patterns for this error code pats = self.spack_errors.get(error.code, None) # if any pattern matches, skip line - if pats is not None and any( - (pat.search(error.physical_line) for pat in pats) - ): + if pats is not None and any((pat.search(error.physical_line) for pat in pats)): return # Special F811 handling @@ -117,16 +94,10 @@ class SpackFormatter(Pylint): # https://gitlab.com/pycqa/flake8/issues/583 # we can only determine if F811 should be ignored given the previous # line, so get the previous line and check it - if ( - self.spack_errors.get("F811", False) - and error.code == "F811" - and error.line_number > 1 - ): + if self.spack_errors.get("F811", False) and error.code == "F811" and error.line_number > 1: if self.file_lines is None: if self.filename in {"stdin", "-", "(none)", None}: - self.file_lines = pycodestyle.stdin_get_value().splitlines( - True - ) + self.file_lines = pycodestyle.stdin_get_value().splitlines(True) else: self.file_lines = pycodestyle.readlines(self.filename) for pat in self.spack_errors["F811"]: diff --git a/var/spack/repos/builtin.mock/packages/flake8/hyper-specific-patch-that-fixes-some-random-bug-that-probably-only-affects-one-user.patch b/var/spack/repos/builtin.mock/packages/flake8/hyper-specific-patch-that-fixes-some-random-bug-that-probably-only-affects-one-user.patch deleted file mode 100644 index b8c9065e4b..0000000000 --- a/var/spack/repos/builtin.mock/packages/flake8/hyper-specific-patch-that-fixes-some-random-bug-that-probably-only-affects-one-user.patch +++ /dev/null @@ -1 +0,0 @@ -Fake patch diff --git a/var/spack/repos/builtin.mock/packages/flake8/package.py b/var/spack/repos/builtin.mock/packages/flake8/package.py index ec9454cfec..d8ce33859f 100644 --- a/var/spack/repos/builtin.mock/packages/flake8/package.py +++ b/var/spack/repos/builtin.mock/packages/flake8/package.py @@ -7,62 +7,74 @@ from spack.package import * class Flake8(Package): - """Package containing as many PEP 8 violations as possible. - All of these violations are exceptions that we allow in - package.py files.""" + """Package containing as many acceptable ``PEP8`` violations as possible. - # Used to tell whether or not the package has been modified - state = 'unmodified' - - # Make sure pre-existing noqa is not interfered with - blatant_violation = 'line-that-has-absolutely-no-execuse-for-being-over-79-characters' # noqa - blatant_violation = 'line-that-has-absolutely-no-execuse-for-being-over-79-characters' # noqa: E501 - - # Keywords exempt from line-length checks - homepage = '#####################################################################' - url = '#####################################################################' - git = '#####################################################################' - svn = '#####################################################################' - hg = '#####################################################################' - list_url = '#####################################################################' - - # URL strings exempt from line-length checks - # http://######################################################################## - # https://####################################################################### - # ftp://######################################################################### - # file://######################################################################## - - # Directives exempt from line-length checks - version('2.0', '0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef') - version('1.0', '0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef') + All of these violations are exceptions that we allow in ``package.py`` files, and + Spack is more lenient than ``flake8`` is for things like URLs and long SHA sums. - variant('super-awesome-feature', default=True, description='Enable super awesome feature') - variant('somewhat-awesome-feature', default=False, description='Enable somewhat awesome feature') + See ``share/spack/qa/flake8_formatter.py`` for specifics of how we handle ``flake8`` + exemptions. - provides('somevirt', when='@2.0+super-awesome-feature+somewhat-awesome-feature') + """ - extends('python', ignore='bin/(why|does|every|package|that|depends|on|numpy|need|to|copy|f2py3?)') - - depends_on('boost+atomic+chrono+date_time~debug+filesystem~graph~icu+iostreams+locale+log+math~mpi+multithreaded+program_options~python+random+regex+serialization+shared+signals~singlethreaded+system~taggedlayout+test+thread+timer+wave') - - conflicts('+super-awesome-feature', when='%intel@16:17+somewhat-awesome-feature') - - resource(name='Deez-Nuts', destination='White-House', placement='President', when='@2020', url='www.elect-deez-nuts.com') + # Used to tell whether or not the package has been modified + state = "unmodified" - patch('hyper-specific-patch-that-fixes-some-random-bug-that-probably-only-affects-one-user.patch', when='%gcc@3.2.2:3.2.3') + # Make sure pre-existing noqa is not interfered with + # note that black can sometimes fix shorter assignment statements by sticking them in + # parens and adding line breaks, e.g.: + # + # foo = ( + # "too-long-string" + # ) + # + # but the one below can't even be fixed that way -- you have to add noqa, or break + # it up inside parens yourself. + blatant_violation = "line-that-has-absolutely-no-execuse-for-being-over-99-characters-and-that-black-cannot-fix-with-parens" # noqa: E501 + + # All URL strings are exempt from line-length checks. + # + # flake8 normally would complain about these, but the fix it wants (a multi-line + # string) is ugbly, and we're more lenient since there are many places where Spack + # wants URLs in strings. + hg = "https://example.com/this-is-a-really-long-url/that-goes-over-99-characters/that-flake8-will-not-ignore-by-default" + list_url = "https://example.com/this-is-a-really-long-url/that-goes-over-99-characters/that-flake8-will-not-ignore-by-default" + git = "ssh://example.com/this-is-a-really-long-url/that-goes-over-99-characters/that-flake8-will-not-ignore-by-default" + + # directives with URLs are exempt as well + version( + "1.0", + url="https://example.com/this-is-a-really-long-url/that-goes-over-99-characters/that-flake8-will-not-ignore-by-default", + ) + + # + # Also test URL comments (though flake8 will ignore these by default anyway) + # + # http://example.com/this-is-a-really-long-url/that-goes-over-99-characters/that-flake8-will-ignore-by-default + # https://example.com/this-is-a-really-long-url/that-goes-over-99-characters/that-flake8-will-ignore-by-default + # ftp://example.com/this-is-a-really-long-url/that-goes-over-99-characters/that-flake8-will-ignore-by-default + # ssh://example.com/this-is-a-really-long-url/that-goes-over-99-characters/that-flake8-will-ignore-by-default + # file://example.com/this-is-a-really-long-url/that-goes-over-99-characters/that-flake8-will-ignore-by-default + + # Strings and comments with really long checksums require no noqa annotation. + sha512sum = "abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890" + # the sha512sum is "abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890" def install(self, spec, prefix): # Make sure lines with '# noqa' work as expected. Don't just # remove them entirely. This will mess up the indentation of # the following lines. - if 'really-long-if-statement' != 'that-goes-over-the-line-length-limit-and-requires-noqa': # noqa + if ( + "really-long-if-statement" + != "this-string-is-so-long-that-it-is-over-the-line-limit-and-black-will-not-split-it-so-it-requires-noqa" # noqa: E501 + ): pass # sanity_check_prefix requires something in the install directory mkdirp(prefix.bin) # '@when' decorated functions are exempt from redefinition errors - @when('@2.0') + @when("@2.0") def install(self, spec, prefix): # sanity_check_prefix requires something in the install directory mkdirp(prefix.bin) |