summaryrefslogtreecommitdiff
path: root/lib
diff options
context:
space:
mode:
authorMassimiliano Culpo <massimiliano.culpo@gmail.com>2022-09-01 08:21:12 +0200
committerGitHub <noreply@github.com>2022-09-01 08:21:12 +0200
commit0df0b9a505d3731088a462c7fafadf6e162b1cc6 (patch)
tree8cfacff56958d2b57a9ea305f9021ab69d24915e /lib
parentc4647a9c1f14fdb81d6837cbc614047fcfe3932c (diff)
downloadspack-0df0b9a505d3731088a462c7fafadf6e162b1cc6.tar.gz
spack-0df0b9a505d3731088a462c7fafadf6e162b1cc6.tar.bz2
spack-0df0b9a505d3731088a462c7fafadf6e162b1cc6.tar.xz
spack-0df0b9a505d3731088a462c7fafadf6e162b1cc6.zip
Port package sanity unit tests to audits (#32405)
Diffstat (limited to 'lib')
-rw-r--r--lib/spack/spack/audit.py180
-rw-r--r--lib/spack/spack/test/audit.py23
-rw-r--r--lib/spack/spack/test/package_sanity.py300
-rw-r--r--lib/spack/spack/test/repo.py20
4 files changed, 213 insertions, 310 deletions
diff --git a/lib/spack/spack/audit.py b/lib/spack/spack/audit.py
index 8957320e48..ee974a19b1 100644
--- a/lib/spack/spack/audit.py
+++ b/lib/spack/spack/audit.py
@@ -35,9 +35,11 @@ Calls to each of these functions are triggered by the ``run`` method of
the decorator object, that will forward the keyword arguments passed
as input.
"""
+import ast
import collections
import inspect
import itertools
+import pickle
import re
from six.moves.urllib.request import urlopen
@@ -49,6 +51,7 @@ import spack.config
import spack.patch
import spack.repo
import spack.spec
+import spack.util.crypto
import spack.variant
#: Map an audit tag to a list of callables implementing checks
@@ -261,6 +264,14 @@ package_attributes = AuditClass(
)
+package_properties = AuditClass(
+ group="packages",
+ tag="PKG-PROPERTIES",
+ description="Sanity checks on properties a package should maintain",
+ kwargs=("pkgs",),
+)
+
+
#: Sanity checks on linting
# This can take some time, so it's run separately from packages
package_https_directives = AuditClass(
@@ -353,6 +364,145 @@ def _search_for_reserved_attributes_names_in_packages(pkgs, error_cls):
return errors
+@package_properties
+def _ensure_all_package_names_are_lowercase(pkgs, error_cls):
+ """Ensure package names are lowercase and consistent"""
+ badname_regex, errors = re.compile(r"[_A-Z]"), []
+ for pkg_name in pkgs:
+ if badname_regex.search(pkg_name):
+ error_msg = "Package name '{}' is either lowercase or conatine '_'".format(pkg_name)
+ errors.append(error_cls(error_msg, []))
+ return errors
+
+
+@package_properties
+def _ensure_packages_are_pickeleable(pkgs, error_cls):
+ """Ensure that package objects are pickleable"""
+ errors = []
+ for pkg_name in pkgs:
+ pkg_cls = spack.repo.path.get_pkg_class(pkg_name)
+ pkg = pkg_cls(spack.spec.Spec(pkg_name))
+ try:
+ pickle.dumps(pkg)
+ except Exception as e:
+ error_msg = "Package '{}' failed to pickle".format(pkg_name)
+ details = ["{}".format(str(e))]
+ errors.append(error_cls(error_msg, details))
+ return errors
+
+
+@package_properties
+def _ensure_packages_are_unparseable(pkgs, error_cls):
+ """Ensure that all packages can unparse and that unparsed code is valid Python"""
+ import spack.util.package_hash as ph
+
+ errors = []
+ for pkg_name in pkgs:
+ try:
+ source = ph.canonical_source(pkg_name, filter_multimethods=False)
+ except Exception as e:
+ error_msg = "Package '{}' failed to unparse".format(pkg_name)
+ details = ["{}".format(str(e))]
+ errors.append(error_cls(error_msg, details))
+ continue
+
+ try:
+ compile(source, "internal", "exec", ast.PyCF_ONLY_AST)
+ except Exception as e:
+ error_msg = "The unparsed package '{}' failed to compile".format(pkg_name)
+ details = ["{}".format(str(e))]
+ errors.append(error_cls(error_msg, details))
+
+ return errors
+
+
+@package_properties
+def _ensure_all_versions_can_produce_a_fetcher(pkgs, error_cls):
+ """Ensure all versions in a package can produce a fetcher"""
+ errors = []
+ for pkg_name in pkgs:
+ pkg_cls = spack.repo.path.get_pkg_class(pkg_name)
+ pkg = pkg_cls(spack.spec.Spec(pkg_name))
+ try:
+ spack.fetch_strategy.check_pkg_attributes(pkg)
+ for version in pkg.versions:
+ assert spack.fetch_strategy.for_package_version(pkg, version)
+ except Exception as e:
+ error_msg = "The package '{}' cannot produce a fetcher for some of its versions"
+ details = ["{}".format(str(e))]
+ errors.append(error_cls(error_msg.format(pkg_name), details))
+ return errors
+
+
+@package_properties
+def _ensure_docstring_and_no_fixme(pkgs, error_cls):
+ """Ensure the package has a docstring and no fixmes"""
+ errors = []
+ fixme_regexes = [
+ re.compile(r"remove this boilerplate"),
+ re.compile(r"FIXME: Put"),
+ re.compile(r"FIXME: Add"),
+ re.compile(r"example.com"),
+ ]
+ for pkg_name in pkgs:
+ details = []
+ filename = spack.repo.path.filename_for_package_name(pkg_name)
+ with open(filename, "r") as package_file:
+ for i, line in enumerate(package_file):
+ pattern = next((r for r in fixme_regexes if r.search(line)), None)
+ if pattern:
+ details.append(
+ "%s:%d: boilerplate needs to be removed: %s" % (filename, i, line.strip())
+ )
+ if details:
+ error_msg = "Package '{}' contains boilerplate that need to be removed"
+ errors.append(error_cls(error_msg.format(pkg_name), details))
+
+ pkg_cls = spack.repo.path.get_pkg_class(pkg_name)
+ if not pkg_cls.__doc__:
+ error_msg = "Package '{}' miss a docstring"
+ errors.append(error_cls(error_msg.format(pkg_name), []))
+
+ return errors
+
+
+@package_properties
+def _ensure_all_packages_use_sha256_checksums(pkgs, error_cls):
+ """Ensure no packages use md5 checksums"""
+ errors = []
+ for pkg_name in pkgs:
+ pkg_cls = spack.repo.path.get_pkg_class(pkg_name)
+ if pkg_cls.manual_download:
+ continue
+
+ pkg = pkg_cls(spack.spec.Spec(pkg_name))
+
+ def invalid_sha256_digest(fetcher):
+ if getattr(fetcher, "digest", None):
+ h = spack.util.crypto.hash_algo_for_digest(fetcher.digest)
+ if h != "sha256":
+ return h, True
+ return None, False
+
+ error_msg = "Package '{}' does not use sha256 checksum".format(pkg_name)
+ details = []
+ for v, args in pkg.versions.items():
+ fetcher = spack.fetch_strategy.for_package_version(pkg, v)
+ digest, is_bad = invalid_sha256_digest(fetcher)
+ if is_bad:
+ details.append("{}@{} uses {}".format(pkg_name, v, digest))
+
+ for _, resources in pkg.resources.items():
+ for resource in resources:
+ digest, is_bad = invalid_sha256_digest(resource.fetcher)
+ if is_bad:
+ details.append("Resource in '{}' uses {}".format(pkg_name, digest))
+ if details:
+ errors.append(error_cls(error_msg, details))
+
+ return errors
+
+
@package_https_directives
def _linting_package_file(pkgs, error_cls):
"""Check for correctness of links"""
@@ -491,6 +641,36 @@ def _unknown_variants_in_dependencies(pkgs, error_cls):
@package_directives
+def _ensure_variant_defaults_are_parsable(pkgs, error_cls):
+ """Ensures that variant defaults are present and parsable from cli"""
+ errors = []
+ for pkg_name in pkgs:
+ pkg_cls = spack.repo.path.get_pkg_class(pkg_name)
+ for variant_name, entry in pkg_cls.variants.items():
+ variant, _ = entry
+ default_is_parsable = (
+ # Permitting a default that is an instance on 'int' permits
+ # to have foo=false or foo=0. Other falsish values are
+ # not allowed, since they can't be parsed from cli ('foo=')
+ isinstance(variant.default, int)
+ or variant.default
+ )
+ if not default_is_parsable:
+ error_msg = "Variant '{}' of package '{}' has a bad default value"
+ errors.append(error_cls(error_msg.format(variant_name, pkg_name), []))
+ continue
+
+ vspec = variant.make_default()
+ try:
+ variant.validate_or_raise(vspec, pkg_cls=pkg_cls)
+ except spack.variant.InvalidVariantValueError:
+ error_msg = "The variant '{}' default value in package '{}' cannot be validated"
+ errors.append(error_cls(error_msg.format(variant_name, pkg_name), []))
+
+ return errors
+
+
+@package_directives
def _version_constraints_are_satisfiable_by_some_version_in_repo(pkgs, error_cls):
"""Report if version constraints used in directives are not satisfiable"""
errors = []
diff --git a/lib/spack/spack/test/audit.py b/lib/spack/spack/test/audit.py
index 890a8dcaf8..eded1a92f2 100644
--- a/lib/spack/spack/test/audit.py
+++ b/lib/spack/spack/test/audit.py
@@ -9,18 +9,20 @@ import spack.config
@pytest.mark.parametrize(
- "packages,expected_error",
+ # PKG-PROPERTIES are ubiquitous in mock packages, since they don't use sha256
+ # and they don't change the example.com URL very often.
+ "packages,expected_errors",
[
# A non existing variant is used in a conflict directive
- (["wrong-variant-in-conflicts"], "PKG-DIRECTIVES"),
+ (["wrong-variant-in-conflicts"], ["PKG-DIRECTIVES", "PKG-PROPERTIES"]),
# The package declares a non-existing dependency
- (["missing-dependency"], "PKG-DIRECTIVES"),
+ (["missing-dependency"], ["PKG-DIRECTIVES", "PKG-PROPERTIES"]),
# The package use a non existing variant in a depends_on directive
- (["wrong-variant-in-depends-on"], "PKG-DIRECTIVES"),
+ (["wrong-variant-in-depends-on"], ["PKG-DIRECTIVES", "PKG-PROPERTIES"]),
# This package has a GitHub patch URL without full_index=1
- (["invalid-github-patch-url"], "PKG-DIRECTIVES"),
+ (["invalid-github-patch-url"], ["PKG-DIRECTIVES", "PKG-PROPERTIES"]),
# This package has a stand-alone 'test' method in build-time callbacks
- (["test-build-callbacks"], "PKG-DIRECTIVES"),
+ (["test-build-callbacks"], ["PKG-DIRECTIVES", "PKG-PROPERTIES"]),
# This package has no issues
(["mpileaks"], None),
# This package has a conflict with a trigger which cannot constrain the constraint
@@ -28,15 +30,16 @@ import spack.config
(["unconstrainable-conflict"], None),
],
)
-def test_package_audits(packages, expected_error, mock_packages):
+def test_package_audits(packages, expected_errors, mock_packages):
reports = spack.audit.run_group("packages", pkgs=packages)
# Check that errors were reported only for the expected failure
actual_errors = [check for check, errors in reports if errors]
- if expected_error:
- assert [expected_error] == actual_errors
+ msg = [str(e) for _, errors in reports for e in errors]
+ if expected_errors:
+ assert expected_errors == actual_errors, msg
else:
- assert not actual_errors
+ assert not actual_errors, msg
# Data used in the test below to audit the double definition of a compiler
diff --git a/lib/spack/spack/test/package_sanity.py b/lib/spack/spack/test/package_sanity.py
deleted file mode 100644
index ba98ccf2a9..0000000000
--- a/lib/spack/spack/test/package_sanity.py
+++ /dev/null
@@ -1,300 +0,0 @@
-# Copyright 2013-2022 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)
-"""This test does sanity checks on Spack's builtin package database."""
-import ast
-import os.path
-import pickle
-import re
-
-import pytest
-
-import llnl.util.tty as tty
-
-# A few functions from this module are used to
-# do sanity checks only on packagess modified by a PR
-import spack.cmd.style as style
-import spack.fetch_strategy
-import spack.package_base
-import spack.paths
-import spack.repo
-import spack.spec
-import spack.util.crypto as crypto
-import spack.util.executable as executable
-import spack.util.package_hash as ph
-import spack.variant
-
-
-def check_repo():
- """Get all packages in the builtin repo to make sure they work."""
- for name in spack.repo.all_package_names():
- spack.repo.path.get_pkg_class(name)
-
-
-@pytest.mark.maybeslow
-def test_get_all_packages():
- """Get all packages once and make sure that works."""
- check_repo()
-
-
-def test_packages_are_pickleable():
- failed_to_pickle = list()
- for name in spack.repo.all_package_names():
- pkg_cls = spack.repo.path.get_pkg_class(name)
- pkg = pkg_cls(spack.spec.Spec(name))
- try:
- pickle.dumps(pkg)
- except Exception:
- # If there are any failures, keep track of all packages that aren't
- # pickle-able and re-run the pickling later on to recreate the
- # error
- failed_to_pickle.append(name)
-
- if failed_to_pickle:
- tty.msg("The following packages failed to pickle: " + ", ".join(failed_to_pickle))
-
- for name in failed_to_pickle:
- pkg_cls = spack.repo.path.get_pkg_class(name)
- pkg = pkg_cls(spack.spec.Spec(name))
- pickle.dumps(pkg)
-
-
-def test_packages_are_unparseable():
- """Ensure that all packages can unparse and that unparsed code is valid Python."""
- failed_to_unparse = []
- failed_to_compile = []
-
- for name in spack.repo.all_package_names():
- try:
- source = ph.canonical_source(name, filter_multimethods=False)
- except Exception:
- failed_to_unparse.append(name)
-
- try:
- compile(source, "internal", "exec", ast.PyCF_ONLY_AST)
- except Exception:
- failed_to_compile.append(name)
-
- if failed_to_unparse:
- tty.msg("The following packages failed to unparse: " + ", ".join(failed_to_unparse))
- assert False
-
- if failed_to_compile:
- tty.msg(
- "The following unparsed packages failed to compile: " + ", ".join(failed_to_compile)
- )
- assert False
-
-
-def test_repo_getpkg_names_and_classes():
- """Ensure that all_packages/names/classes are consistent."""
- names = spack.repo.path.all_package_names()
- print(names)
- classes = spack.repo.path.all_package_classes()
- print(list(classes))
-
- for name, cls in zip(names, classes):
- assert cls.name == name
-
-
-def test_get_all_mock_packages():
- """Get the mock packages once each too."""
- db = spack.repo.RepoPath(spack.paths.mock_packages_path)
- with spack.repo.use_repositories(db):
- check_repo()
-
-
-def test_all_versions_are_lowercase():
- """Spack package names must be lowercase, and use `-` instead of `_`."""
- errors = []
- for name in spack.repo.all_package_names():
- if re.search(r"[_A-Z]", name):
- errors.append(name)
-
- assert len(errors) == 0
-
-
-def test_all_virtual_packages_have_default_providers():
- """All virtual packages must have a default provider explicitly set."""
- defaults = spack.config.get("packages", scope="defaults")
- default_providers = defaults["all"]["providers"]
- providers = spack.repo.path.provider_index.providers
- default_providers_filename = spack.config.config.scopes["defaults"].get_section_filename(
- "packages"
- )
- for provider in providers:
- assert provider in default_providers, (
- "all providers must have a default in %s" % default_providers_filename
- )
-
-
-def test_package_version_consistency():
- """Make sure all versions on builtin packages produce a fetcher."""
- for name in spack.repo.all_package_names():
- pkg_cls = spack.repo.path.get_pkg_class(name)
- pkg = pkg_cls(spack.spec.Spec(name))
- spack.fetch_strategy.check_pkg_attributes(pkg)
- for version in pkg.versions:
- assert spack.fetch_strategy.for_package_version(pkg, version)
-
-
-def test_no_fixme():
- """Packages should not contain any boilerplate such as
- FIXME or example.com."""
- errors = []
- fixme_regexes = [
- r"remove this boilerplate",
- r"FIXME: Put",
- r"FIXME: Add",
- r"example.com",
- ]
- for name in spack.repo.all_package_names():
- filename = spack.repo.path.filename_for_package_name(name)
- with open(filename, "r") as package_file:
- for i, line in enumerate(package_file):
- pattern = next((r for r in fixme_regexes if re.search(r, line)), None)
- if pattern:
- errors.append(
- "%s:%d: boilerplate needs to be removed: %s" % (filename, i, line.strip())
- )
- assert [] == errors
-
-
-def test_docstring():
- """Ensure that every package has a docstring."""
- for name in spack.repo.all_package_names():
- pkg_cls = spack.repo.path.get_pkg_class(name)
- assert pkg_cls.__doc__
-
-
-def test_all_packages_use_sha256_checksums():
- """Make sure that no packages use md5 checksums."""
-
- errors = []
- for name in spack.repo.all_package_names():
- pkg_cls = spack.repo.path.get_pkg_class(name)
- pkg = pkg_cls(spack.spec.Spec(name))
-
- # for now, don't enforce on packages that require manual downloads
- # TODO: eventually fix these, too.
- if pkg.manual_download:
- continue
-
- def invalid_sha256_digest(fetcher):
- if getattr(fetcher, "digest", None):
- h = crypto.hash_algo_for_digest(fetcher.digest)
- if h != "sha256":
- return h
-
- for v, args in pkg.versions.items():
- fetcher = spack.fetch_strategy.for_package_version(pkg, v)
- bad_digest = invalid_sha256_digest(fetcher)
- if bad_digest:
- errors.append(
- "All packages must use sha256 checksums. %s@%s uses %s."
- % (name, v, bad_digest)
- )
-
- for _, resources in pkg.resources.items():
- for resource in resources:
- bad_digest = invalid_sha256_digest(resource.fetcher)
- if bad_digest:
- errors.append(
- "All packages must use sha256 checksums."
- "Resource in %s uses %s." % (name, bad_digest)
- )
-
- assert [] == errors
-
-
-def test_api_for_build_and_run_environment():
- """Ensure that every package uses the correct API to set build and
- run environment, and not the old one.
- """
- failing = []
- for pkg_cls in spack.repo.path.all_package_classes():
- add_to_list = hasattr(pkg_cls, "setup_environment") or hasattr(
- pkg_cls, "setup_dependent_environment"
- )
- if add_to_list:
- failing.append(pkg_cls)
-
- msg = (
- "there are {0} packages using the old API to set build "
- "and run environment [{1}], for further information see "
- "https://github.com/spack/spack/pull/11115"
- )
- assert not failing, msg.format(len(failing), ",".join(x.name for x in failing))
-
-
-@pytest.mark.skipif(not executable.which("git"), reason="requires git to be installed")
-def test_prs_update_old_api():
- """Ensures that every package modified in a PR doesn't contain
- deprecated calls to any method.
- """
- ref = os.getenv("GITHUB_BASE_REF")
- if not ref:
- pytest.skip("No base ref found")
-
- changed_package_files = [x for x in style.changed_files(base=ref) if style.is_package(x)]
- failing = []
- for file in changed_package_files:
- if "builtin.mock" not in file: # don't restrict packages for tests
- name = os.path.basename(os.path.dirname(file))
- pkg_cls = spack.repo.path.get_pkg_class(name)
- pkg = pkg_cls(spack.spec.Spec(name))
-
- failed = hasattr(pkg, "setup_environment") or hasattr(
- pkg, "setup_dependent_environment"
- )
- if failed:
- failing.append(name)
-
- msg = (
- "there are {0} packages using the old API to set build "
- "and run environment [{1}], for further information see "
- "https://github.com/spack/spack/pull/11115"
- )
- assert not failing, msg.format(len(failing), ",".join(failing))
-
-
-def test_all_dependencies_exist():
- """Make sure no packages have nonexisting dependencies."""
- missing = {}
- pkgs = [pkg for pkg in spack.repo.path.all_package_names()]
- spack.package_base.possible_dependencies(*pkgs, transitive=True, missing=missing)
-
- lines = ["%s: [%s]" % (name, ", ".join(deps)) for name, deps in missing.items()]
- assert not missing, "These packages have missing dependencies:\n" + ("\n".join(lines))
-
-
-def test_variant_defaults_are_parsable_from_cli():
- """Ensures that variant defaults are parsable from cli."""
- failing = []
- for pkg_cls in spack.repo.path.all_package_classes():
- for variant_name, entry in pkg_cls.variants.items():
- variant, _ = entry
- default_is_parsable = (
- # Permitting a default that is an instance on 'int' permits
- # to have foo=false or foo=0. Other falsish values are
- # not allowed, since they can't be parsed from cli ('foo=')
- isinstance(variant.default, int)
- or variant.default
- )
- if not default_is_parsable:
- failing.append((pkg_cls.name, variant_name))
- assert not failing
-
-
-def test_variant_defaults_listed_explicitly_in_values():
- failing = []
- for pkg_cls in spack.repo.path.all_package_classes():
- for variant_name, entry in pkg_cls.variants.items():
- variant, _ = entry
- vspec = variant.make_default()
- try:
- variant.validate_or_raise(vspec, pkg_cls=pkg_cls)
- except spack.variant.InvalidVariantValueError:
- failing.append((pkg_cls.name, variant.name))
- assert not failing
diff --git a/lib/spack/spack/test/repo.py b/lib/spack/spack/test/repo.py
index 1ab0a7dd5c..cb376d9430 100644
--- a/lib/spack/spack/test/repo.py
+++ b/lib/spack/spack/test/repo.py
@@ -121,3 +121,23 @@ def test_relative_import_spack_packages_as_python_modules(mock_packages):
assert isinstance(Mpileaks, spack.package_base.PackageMeta)
assert issubclass(Mpileaks, spack.package_base.Package)
+
+
+def test_all_virtual_packages_have_default_providers():
+ """All virtual packages must have a default provider explicitly set."""
+ defaults = spack.config.get("packages", scope="defaults")
+ default_providers = defaults["all"]["providers"]
+ providers = spack.repo.path.provider_index.providers
+ default_providers_filename = spack.config.config.scopes["defaults"].get_section_filename(
+ "packages"
+ )
+ for provider in providers:
+ assert provider in default_providers, (
+ "all providers must have a default in %s" % default_providers_filename
+ )
+
+
+def test_get_all_mock_packages(mock_packages):
+ """Get the mock packages once each too."""
+ for name in mock_packages.all_package_names():
+ mock_packages.get_pkg_class(name)