From e715901cb2b5d7392f40c9a8ffb8eaf8eb075497 Mon Sep 17 00:00:00 2001 From: Chris Green Date: Fri, 18 Nov 2022 15:22:51 -0600 Subject: PackageBase should not define builder legacy attributes (#33942) * Add a regression test for 33928 * PackageBase should not set `(build|install)_time_test_callbacks` * Fix audits by preserving the current semantic Co-authored-by: Massimiliano Culpo --- lib/spack/spack/audit.py | 2 +- lib/spack/spack/cmd/info.py | 4 ++-- lib/spack/spack/package_base.py | 8 -------- lib/spack/spack/test/builder.py | 14 ++++++++++++++ .../builder.test/packages/old-style-autotools/package.py | 6 ++++++ 5 files changed, 23 insertions(+), 11 deletions(-) diff --git a/lib/spack/spack/audit.py b/lib/spack/spack/audit.py index 4bc85dac46..677c1a47c4 100644 --- a/lib/spack/spack/audit.py +++ b/lib/spack/spack/audit.py @@ -287,7 +287,7 @@ def _check_build_test_callbacks(pkgs, error_cls): errors = [] for pkg_name in pkgs: pkg_cls = spack.repo.path.get_pkg_class(pkg_name) - test_callbacks = pkg_cls.build_time_test_callbacks + test_callbacks = getattr(pkg_cls, "build_time_test_callbacks", None) if test_callbacks and "test" in test_callbacks: msg = '{0} package contains "test" method in ' "build_time_test_callbacks" diff --git a/lib/spack/spack/cmd/info.py b/lib/spack/spack/cmd/info.py index 5076ff9261..a36a547644 100644 --- a/lib/spack/spack/cmd/info.py +++ b/lib/spack/spack/cmd/info.py @@ -241,8 +241,8 @@ def print_tests(pkg): # So the presence of a callback in Spack does not necessarily correspond # to the actual presence of built-time tests for a package. for callbacks, phase in [ - (pkg.build_time_test_callbacks, "Build"), - (pkg.install_time_test_callbacks, "Install"), + (getattr(pkg, "build_time_test_callbacks", None), "Build"), + (getattr(pkg, "install_time_test_callbacks", None), "Install"), ]: color.cprint("") color.cprint(section_title("Available {0} Phase Test Methods:".format(phase))) diff --git a/lib/spack/spack/package_base.py b/lib/spack/spack/package_base.py index 81fbe86ee0..be0721e667 100644 --- a/lib/spack/spack/package_base.py +++ b/lib/spack/spack/package_base.py @@ -528,10 +528,6 @@ class PackageBase(WindowsRPath, PackageViewMixin, metaclass=PackageMeta): # These are default values for instance variables. # - #: A list or set of build time test functions to be called when tests - #: are executed or 'None' if there are no such test functions. - build_time_test_callbacks = None # type: Optional[List[str]] - #: By default, packages are not virtual #: Virtual packages override this attribute virtual = False @@ -540,10 +536,6 @@ class PackageBase(WindowsRPath, PackageViewMixin, metaclass=PackageMeta): #: those that do not can be used to install a set of other Spack packages. has_code = True - #: A list or set of install time test functions to be called when tests - #: are executed or 'None' if there are no such test functions. - install_time_test_callbacks = None # type: Optional[List[str]] - #: By default we build in parallel. Subclasses can override this. parallel = True diff --git a/lib/spack/spack/test/builder.py b/lib/spack/spack/test/builder.py index bda72c8b49..efba6aacf1 100644 --- a/lib/spack/spack/test/builder.py +++ b/lib/spack/spack/test/builder.py @@ -121,3 +121,17 @@ def test_old_style_compatibility_with_super(spec_str, method_name, expected): builder = spack.builder.create(s.package) value = getattr(builder, method_name)() assert value == expected + + +@pytest.mark.regression("33928") +@pytest.mark.usefixtures("builder_test_repository", "config", "working_env") +@pytest.mark.disable_clean_stage_check +def test_build_time_tests_are_executed_from_default_builder(): + s = spack.spec.Spec("old-style-autotools").concretized() + builder = spack.builder.create(s.package) + builder.pkg.run_tests = True + for phase_fn in builder: + phase_fn.execute() + + assert os.environ.get("CHECK_CALLED") == "1", "Build time tests not executed" + assert os.environ.get("INSTALLCHECK_CALLED") == "1", "Install time tests not executed" diff --git a/var/spack/repos/builder.test/packages/old-style-autotools/package.py b/var/spack/repos/builder.test/packages/old-style-autotools/package.py index 56213d7158..5335519984 100644 --- a/var/spack/repos/builder.test/packages/old-style-autotools/package.py +++ b/var/spack/repos/builder.test/packages/old-style-autotools/package.py @@ -48,3 +48,9 @@ class OldStyleAutotools(AutotoolsPackage): @run_after("autoreconf", when="@2.0") def after_autoreconf_2(self): os.environ["AFTER_AUTORECONF_2_CALLED"] = "1" + + def check(self): + os.environ["CHECK_CALLED"] = "1" + + def installcheck(self): + os.environ["INSTALLCHECK_CALLED"] = "1" -- cgit v1.2.3-70-g09d2