Stand-alone testing: remove deprecated methods (#45752)

* Stand-alone testing: remove deprecated methods
* audit: replace deprecated test method checks for test callbacks (AcriusWinter)
* install_test: replace deprecated with new test method name starts (AcriusWinter)
* package_base: removed deprecated test methods (AcriusWinter)
* test/package_class: remove deprecated test methods (AcriusWinter)
* test/reporters: remove deprecated test methods (AcriusWinter)
* reporters/extract: remove deprecated test-related methods (AcriusWinter)
* py-test-callback: rename test in callbacks and output (AcriusWinter)
* reporters/extract: remove deprecated expected failure output capture (AcriusWinter)
* stand-alone test cleanup: f-string, remove deprecation warning, etc. (AcriusWinter)
* stand-alone tests: f-string fix and remove unused imports (AcriusWinter)
* package_base: finish removing the rest of deprecated run_test method (AcriusWinter)
* py-test-callback: remove stand-alone test method (AcriusWinter)
* package_base: remove now unused imports (AcriusWinter)
* Python: test_imports replaces test (tldahlgren)
* mptensor, trivial-smoke-test: replace deprecated cache_extra_test_sources method (tldahlgren)
* trivial-smoke-test: replace deprecated install_test_root method (tldahlgren)
* docs: update perl and scons package's test methods (tldahlgren)
* builder: remove deprecated test() method (tldahlgren)
* Update legion and mfem test data for stand-alone tests (tldahlgren)
* py-test-callback: restore the test method
* Audit/stand-alone testing: Fix and added stand-alone test audit checks
   - fix audit failure message for build-time test callback check
   - remove empty test method check during stand-alone testing
   - change build-time test callback check to package_properties
   - add test method docstring audit check and mock fail-test-audit-docstring pkg
   - add test method implementation audit check and mock fail-test-audit-impl pkg
   - add new mock packages to test_package_audits and test_test_list_all
* audits: loosen docstring content constraints
* Add missing test method docstring
* py-test-callback: resolve builder issues
* Audits: Add checks for use of deprecated stand-alone test methods; improve output for other test-related checks
* Fix style issues
* test_test_list_all: add new fail-test-audit-deprecated package

---------

Signed-off-by: Tamara Dahlgren <dahlgren1@llnl.gov>
Co-authored-by: Tamara Dahlgren <35777542+tldahlgren@users.noreply.github.com>

lib/spack/docs/build_systems/custompackage.rst

@@ -130,14 +130,19 @@ before or after a particular phase. For example, in ``perl``, we see:
 
    @run_after("install")
    def install_cpanm(self):
-       spec = self.spec
+        spec = self.spec
-
+        maker = make
-       if spec.satisfies("+cpanm"):
+        cpan_dir = join_path("cpanm", "cpanm")
-           with working_dir(join_path("cpanm", "cpanm")):
+        if sys.platform == "win32":
-               perl = spec["perl"].command
+            maker = nmake
-               perl("Makefile.PL")
+            cpan_dir = join_path(self.stage.source_path, cpan_dir)
-               make()
+            cpan_dir = windows_sfn(cpan_dir)
-               make("install")
+        if "+cpanm" in spec:
+            with working_dir(cpan_dir):
+                perl = spec["perl"].command
+                perl("Makefile.PL")
+                maker()
+                maker("install")
 
 This extra step automatically installs ``cpanm`` in addition to the
 base Perl installation.
@@ -176,8 +181,14 @@ In the ``perl`` package, we can see:
 
    @run_after("build")
    @on_package_attributes(run_tests=True)
-   def test(self):
+   def build_test(self):
-       make("test")
+        if sys.platform == "win32":
+            win32_dir = os.path.join(self.stage.source_path, "win32")
+            win32_dir = windows_sfn(win32_dir)
+            with working_dir(win32_dir):
+                nmake("test", ignore_quotes=True)
+        else:
+            make("test")
 
 As you can guess, this runs ``make test`` *after* building the package,
 if and only if testing is requested. Again, this is not specific to

lib/spack/docs/build_systems/sconspackage.rst

@@ -49,14 +49,14 @@ following phases:
 #. ``install`` - install the package
 
 Package developers often add unit tests that can be invoked with
-``scons test`` or ``scons check``. Spack provides a ``test`` method
+``scons test`` or ``scons check``. Spack provides a ``build_test`` method
 to handle this. Since we don't know which one the package developer
-chose, the ``test`` method does nothing by default, but can be easily
+chose, the ``build_test`` method does nothing by default, but can be easily
 overridden like so:
 
 .. code-block:: python
 
-   def test(self):
+   def build_test(self):
        scons("check")
 
 

lib/spack/spack/audit.py

@@ -39,6 +39,7 @@ def _search_duplicate_compilers(error_cls):
 import collections
 import collections.abc
 import glob
+import inspect
 import io
 import itertools
 import os
@@ -50,6 +51,7 @@ def _search_duplicate_compilers(error_cls):
 from urllib.request import urlopen
 
 import llnl.util.lang
+from llnl.string import plural
 
 import spack.builder
 import spack.config
@@ -386,6 +388,14 @@ def _make_config_error(config_data, summary, error_cls):
 )
 
 
+package_deprecated_attributes = AuditClass(
+    group="packages",
+    tag="PKG-DEPRECATED-ATTRIBUTES",
+    description="Sanity checks to preclude use of deprecated package attributes",
+    kwargs=("pkgs",),
+)
+
+
 package_properties = AuditClass(
     group="packages",
     tag="PKG-PROPERTIES",
@@ -404,22 +414,23 @@ def _make_config_error(config_data, summary, error_cls):
 )
 
 
-@package_directives
+@package_properties
 def _check_build_test_callbacks(pkgs, error_cls):
-    """Ensure stand-alone test method is not included in build-time callbacks"""
+    """Ensure stand-alone test methods are not included in build-time callbacks.
+
+    Test methods are for checking the installed software as stand-alone tests.
+    They could also be called during the post-install phase of a build.
+    """
     errors = []
     for pkg_name in pkgs:
         pkg_cls = spack.repo.PATH.get_pkg_class(pkg_name)
         test_callbacks = getattr(pkg_cls, "build_time_test_callbacks", None)
 
-        # TODO (post-34236): "test*"->"test_*" once remove deprecated methods
+        has_test_method = test_callbacks and any([m.startswith("test_") for m in test_callbacks])
-        # TODO (post-34236): "test"->"test_" once remove deprecated methods
-        has_test_method = test_callbacks and any([m.startswith("test") for m in test_callbacks])
         if has_test_method:
-            msg = '{0} package contains "test*" method(s) in ' "build_time_test_callbacks"
+            msg = f"Package {pkg_name} includes stand-alone test methods in build-time checks."
-            instr = 'Remove all methods whose names start with "test" from: [{0}]'.format(
+            callbacks = ", ".join(test_callbacks)
-                ", ".join(test_callbacks)
+            instr = f"Remove the following from 'build_time_test_callbacks': {callbacks}"
-            )
             errors.append(error_cls(msg.format(pkg_name), [instr]))
 
     return errors
@@ -517,6 +528,46 @@ def _search_for_reserved_attributes_names_in_packages(pkgs, error_cls):
     return errors
 
 
+@package_deprecated_attributes
+def _search_for_deprecated_package_methods(pkgs, error_cls):
+    """Ensure the package doesn't define or use deprecated methods"""
+    DEPRECATED_METHOD = (("test", "a name starting with 'test_'"),)
+    DEPRECATED_USE = (
+        ("self.cache_extra_test_sources(", "cache_extra_test_sources(self, ..)"),
+        ("self.install_test_root(", "install_test_root(self, ..)"),
+        ("self.run_test(", "test_part(self, ..)"),
+    )
+    errors = []
+    for pkg_name in pkgs:
+        pkg_cls = spack.repo.PATH.get_pkg_class(pkg_name)
+        methods = inspect.getmembers(pkg_cls, predicate=lambda x: inspect.isfunction(x))
+        method_errors = collections.defaultdict(list)
+        for name, function in methods:
+            for deprecated_name, alternate in DEPRECATED_METHOD:
+                if name == deprecated_name:
+                    msg = f"Rename '{deprecated_name}' method to {alternate} instead."
+                    method_errors[name].append(msg)
+
+            source = inspect.getsource(function)
+            for deprecated_name, alternate in DEPRECATED_USE:
+                if deprecated_name in source:
+                    msg = f"Change '{deprecated_name}' to '{alternate}' in '{name}' method."
+                    method_errors[name].append(msg)
+
+        num_methods = len(method_errors)
+        if num_methods > 0:
+            methods = plural(num_methods, "method", show_n=False)
+            error_msg = (
+                f"Package '{pkg_name}' implements or uses unsupported deprecated {methods}."
+            )
+            instr = [f"Make changes to '{pkg_cls.__module__}':"]
+            for name in sorted(method_errors):
+                instr.extend([f"    {msg}" for msg in method_errors[name]])
+            errors.append(error_cls(error_msg, instr))
+
+    return errors
+
+
 @package_properties
 def _ensure_all_package_names_are_lowercase(pkgs, error_cls):
     """Ensure package names are lowercase and consistent"""
@@ -771,6 +822,89 @@ def _uses_deprecated_globals(pkgs, error_cls):
     return errors
 
 
+@package_properties
+def _ensure_test_docstring(pkgs, error_cls):
+    """Ensure stand-alone test methods have a docstring.
+
+    The docstring of a test method is implicitly used as the description of
+    the corresponding test part during test results reporting.
+    """
+    doc_regex = r'\s+("""[^"]+""")'
+
+    errors = []
+    for pkg_name in pkgs:
+        pkg_cls = spack.repo.PATH.get_pkg_class(pkg_name)
+        methods = inspect.getmembers(pkg_cls, predicate=lambda x: inspect.isfunction(x))
+        method_names = []
+        for name, test_fn in methods:
+            if not name.startswith("test_"):
+                continue
+
+            # Ensure the test method has a docstring
+            source = inspect.getsource(test_fn)
+            match = re.search(doc_regex, source)
+            if match is None or len(match.group(0).replace('"', "").strip()) == 0:
+                method_names.append(name)
+
+        num_methods = len(method_names)
+        if num_methods > 0:
+            methods = plural(num_methods, "method", show_n=False)
+            docstrings = plural(num_methods, "docstring", show_n=False)
+            msg = f"Package {pkg_name} has test {methods} with empty or missing {docstrings}."
+            names = ", ".join(method_names)
+            instr = [
+                "Docstrings are used as descriptions in test outputs.",
+                f"Add a concise summary to the following {methods} in '{pkg_cls.__module__}':",
+                f"{names}",
+            ]
+            errors.append(error_cls(msg, instr))
+
+    return errors
+
+
+@package_properties
+def _ensure_test_implemented(pkgs, error_cls):
+    """Ensure stand-alone test methods are implemented.
+
+    The test method is also required to be non-empty.
+    """
+
+    def skip(line):
+        ln = line.strip()
+        return ln.startswith("#") or "pass" in ln
+
+    doc_regex = r'\s+("""[^"]+""")'
+
+    errors = []
+    for pkg_name in pkgs:
+        pkg_cls = spack.repo.PATH.get_pkg_class(pkg_name)
+        methods = inspect.getmembers(pkg_cls, predicate=lambda x: inspect.isfunction(x))
+        method_names = []
+        for name, test_fn in methods:
+            if not name.startswith("test_"):
+                continue
+
+            source = inspect.getsource(test_fn)
+
+            # Attempt to ensure the test method is implemented.
+            impl = re.sub(doc_regex, r"", source).splitlines()[1:]
+            lines = [ln.strip() for ln in impl if not skip(ln)]
+            if not lines:
+                method_names.append(name)
+
+        num_methods = len(method_names)
+        if num_methods > 0:
+            methods = plural(num_methods, "method", show_n=False)
+            msg = f"Package {pkg_name} has empty or missing test {methods}."
+            names = ", ".join(method_names)
+            instr = [
+                f"Implement or remove the following {methods} from '{pkg_cls.__module__}': {names}"
+            ]
+            errors.append(error_cls(msg, instr))
+
+    return errors
+
+
 @package_https_directives
 def _linting_package_file(pkgs, error_cls):
     """Check for correctness of links"""

lib/spack/spack/build_systems/python.py

@@ -339,7 +339,7 @@ class PythonPackage(PythonExtension):
     legacy_buildsystem = "python_pip"
 
     #: Callback names for install-time test
-    install_time_test_callbacks = ["test"]
+    install_time_test_callbacks = ["test_imports"]
 
     build_system("python_pip")
 
@@ -429,7 +429,7 @@ class PythonPipBuilder(BaseBuilder):
     phases = ("install",)
 
     #: Names associated with package methods in the old build-system format
-    legacy_methods = ("test",)
+    legacy_methods = ("test_imports",)
 
     #: Same as legacy_methods, but the signature is different
     legacy_long_methods = ("install_options", "global_options", "config_settings")
@@ -438,7 +438,7 @@ class PythonPipBuilder(BaseBuilder):
     legacy_attributes = ("archive_files", "build_directory", "install_time_test_callbacks")
 
     #: Callback names for install-time test
-    install_time_test_callbacks = ["test"]
+    install_time_test_callbacks = ["test_imports"]
 
     @staticmethod
     def std_args(cls) -> List[str]:

lib/spack/spack/builder.py

@@ -521,10 +521,6 @@ def stage(self):
     def prefix(self):
         return self.pkg.prefix
 
-    def test(self):
-        # Defer tests to virtual and concrete packages
-        pass
-
     def setup_build_environment(self, env):
         """Sets up the build environment for a package.
 

lib/spack/spack/install_test.py

@@ -372,8 +372,7 @@ def phase_tests(
             builder.pkg.test_suite.current_test_spec = builder.pkg.spec
             builder.pkg.test_suite.current_base_spec = builder.pkg.spec
 
-            # TODO (post-34236): "test"->"test_" once remove deprecated methods
+            have_tests = any(name.startswith("test_") for name in method_names)
-            have_tests = any(name.startswith("test") for name in method_names)
             if have_tests:
                 copy_test_files(builder.pkg, builder.pkg.spec)
 
@@ -477,16 +476,9 @@ def write_tested_status(self):
 def test_part(pkg: Pb, test_name: str, purpose: str, work_dir: str = ".", verbose: bool = False):
     wdir = "." if work_dir is None else work_dir
     tester = pkg.tester
-    # TODO (post-34236): "test"->"test_" once remove deprecated methods
     assert test_name and test_name.startswith(
-        "test"
+        "test_"
-    ), f"Test name must start with 'test' but {test_name} was provided"
+    ), f"Test name must start with 'test_' but {test_name} was provided"
-
-    if test_name == "test":
-        tty.warn(
-            "{}: the 'test' method is deprecated. Convert stand-alone "
-            "test(s) to methods with names starting 'test_'.".format(pkg.name)
-        )
 
     title = "test: {}: {}".format(test_name, purpose or "unspecified purpose")
     with fs.working_dir(wdir, create=True):
@@ -646,28 +638,11 @@ def test_functions(
             except spack.repo.UnknownPackageError:
                 tty.debug(f"{vname}: virtual does not appear to have a package file")
 
-    # TODO (post-34236): Remove if removing empty test method check
-    def skip(line):
-        # This should match the lines in the deprecated test() method
-        ln = line.strip()
-        return ln.startswith("#") or ("warn" in ln and "deprecated" in ln)
-
-    doc_regex = r'\s+("""[\w\s\(\)\-\,\;\:]+""")'
     tests = []
     for clss in classes:
         methods = inspect.getmembers(clss, predicate=lambda x: inspect.isfunction(x))
         for name, test_fn in methods:
-            # TODO (post-34236): "test"->"test_" once remove deprecated methods
+            if not name.startswith("test_"):
-            if not name.startswith("test"):
-                continue
-
-            # TODO (post-34236): Could remove empty method check once remove
-            # TODO (post-34236): deprecated methods though some use cases,
-            # TODO (post-34236): such as checking packages have actual, non-
-            # TODO (post-34236): empty tests, may want this check to remain.
-            source = re.sub(doc_regex, r"", inspect.getsource(test_fn)).splitlines()[1:]
-            lines = [ln.strip() for ln in source if not skip(ln)]
-            if not lines:
                 continue
 
             tests.append((clss.__name__, test_fn))  # type: ignore[union-attr]

lib/spack/spack/package_base.py

@@ -55,17 +55,9 @@
 import spack.util.web
 from spack.error import InstallError, NoURLError, PackageError
 from spack.filesystem_view import YamlFilesystemView
-from spack.install_test import (
+from spack.install_test import PackageTest, TestSuite
-    PackageTest,
-    TestFailure,
-    TestStatus,
-    TestSuite,
-    cache_extra_test_sources,
-    install_test_root,
-)
 from spack.solver.version_order import concretization_version_order
 from spack.stage import DevelopStage, ResourceStage, Stage, StageComposite, compute_stage_name
-from spack.util.executable import ProcessError, which
 from spack.util.package_hash import package_hash
 from spack.version import GitVersion, StandardVersion
 
@@ -1355,18 +1347,6 @@ def install_configure_args_path(self):
         """Return the configure args file path on successful installation."""
         return os.path.join(self.metadata_dir, _spack_configure_argsfile)
 
-    # TODO (post-34236): Update tests and all packages that use this as a
-    # TODO (post-34236): package method to the function already available
-    # TODO (post-34236): to packages. Once done, remove this property.
-    @property
-    def install_test_root(self):
-        """Return the install test root directory."""
-        tty.warn(
-            "The 'pkg.install_test_root' property is deprecated with removal "
-            "expected v0.23. Use 'install_test_root(pkg)' instead."
-        )
-        return install_test_root(self)
-
     def archive_install_test_log(self):
         """Archive the install-phase test log, if present."""
         if getattr(self, "tester", None):
@@ -1959,31 +1939,6 @@ def _resource_stage(self, resource):
         resource_stage_folder = "-".join(pieces)
         return resource_stage_folder
 
-    # TODO (post-34236): Update tests and all packages that use this as a
-    # TODO (post-34236): package method to the routine made available to
-    # TODO (post-34236): packages. Once done, remove this method.
-    def cache_extra_test_sources(self, srcs):
-        """Copy relative source paths to the corresponding install test subdir
-
-        This method is intended as an optional install test setup helper for
-        grabbing source files/directories during the installation process and
-        copying them to the installation test subdirectory for subsequent use
-        during install testing.
-
-        Args:
-            srcs (str or list): relative path for files and or
-                subdirectories located in the staged source path that are to
-                be copied to the corresponding location(s) under the install
-                testing directory.
-        """
-        msg = (
-            "'pkg.cache_extra_test_sources(srcs) is deprecated with removal "
-            "expected in v0.23. Use 'cache_extra_test_sources(pkg, srcs)' "
-            "instead."
-        )
-        warnings.warn(msg)
-        cache_extra_test_sources(self, srcs)
-
     def do_test(self, dirty=False, externals=False):
         if self.test_requires_compiler:
             compilers = spack.compilers.compilers_for_spec(
@@ -2007,178 +1962,6 @@ def do_test(self, dirty=False, externals=False):
 
         self.tester.stand_alone_tests(kwargs)
 
-    # TODO (post-34236): Remove this deprecated method when eliminate test,
-    # TODO (post-34236): run_test, etc.
-    @property
-    def _test_deprecated_warning(self):
-        alt = f"Use any name starting with 'test_' instead in {self.spec.name}."
-        return f"The 'test' method is deprecated. {alt}"
-
-    # TODO (post-34236): Remove this deprecated method when eliminate test,
-    # TODO (post-34236): run_test, etc.
-    def test(self):
-        # Defer tests to virtual and concrete packages
-        warnings.warn(self._test_deprecated_warning)
-
-    # TODO (post-34236): Remove this deprecated method when eliminate test,
-    # TODO (post-34236): run_test, etc.
-    def run_test(
-        self,
-        exe,
-        options=[],
-        expected=[],
-        status=0,
-        installed=False,
-        purpose=None,
-        skip_missing=False,
-        work_dir=None,
-    ):
-        """Run the test and confirm the expected results are obtained
-
-        Log any failures and continue, they will be re-raised later
-
-        Args:
-            exe (str): the name of the executable
-            options (str or list): list of options to pass to the runner
-            expected (str or list): list of expected output strings.
-                Each string is a regex expected to match part of the output.
-            status (int or list): possible passing status values
-                with 0 meaning the test is expected to succeed
-            installed (bool): if ``True``, the executable must be in the
-                install prefix
-            purpose (str): message to display before running test
-            skip_missing (bool): skip the test if the executable is not
-                in the install prefix bin directory or the provided work_dir
-            work_dir (str or None): path to the smoke test directory
-        """
-
-        def test_title(purpose, test_name):
-            if not purpose:
-                return f"test: {test_name}: execute {test_name}"
-
-            match = re.search(r"test: ([^:]*): (.*)", purpose)
-            if match:
-                # The test title has all the expected parts
-                return purpose
-
-            match = re.search(r"test: (.*)", purpose)
-            if match:
-                reason = match.group(1)
-                return f"test: {test_name}: {reason}"
-
-            return f"test: {test_name}: {purpose}"
-
-        base_exe = os.path.basename(exe)
-        alternate = f"Use 'test_part' instead for {self.spec.name} to process {base_exe}."
-        warnings.warn(f"The 'run_test' method is deprecated. {alternate}")
-
-        extra = re.compile(r"[\s,\- ]")
-        details = (
-            [extra.sub("", options)]
-            if isinstance(options, str)
-            else [extra.sub("", os.path.basename(opt)) for opt in options]
-        )
-        details = "_".join([""] + details) if details else ""
-        test_name = f"test_{base_exe}{details}"
-        tty.info(test_title(purpose, test_name), format="g")
-
-        wdir = "." if work_dir is None else work_dir
-        with fsys.working_dir(wdir, create=True):
-            try:
-                runner = which(exe)
-                if runner is None and skip_missing:
-                    self.tester.status(test_name, TestStatus.SKIPPED, f"{exe} is missing")
-                    return
-                assert runner is not None, f"Failed to find executable '{exe}'"
-
-                self._run_test_helper(runner, options, expected, status, installed, purpose)
-                self.tester.status(test_name, TestStatus.PASSED, None)
-                return True
-            except (AssertionError, BaseException) as e:
-                # print a summary of the error to the log file
-                # so that cdash and junit reporters know about it
-                exc_type, _, tb = sys.exc_info()
-
-                self.tester.status(test_name, TestStatus.FAILED, str(e))
-
-                import traceback
-
-                # remove the current call frame to exclude the extract_stack
-                # call from the error
-                stack = traceback.extract_stack()[:-1]
-
-                # Package files have a line added at import time, so we re-read
-                # the file to make line numbers match. We have to subtract two
-                # from the line number because the original line number is
-                # inflated once by the import statement and the lines are
-                # displaced one by the import statement.
-                for i, entry in enumerate(stack):
-                    filename, lineno, function, text = entry
-                    if spack.repo.is_package_file(filename):
-                        with open(filename, "r") as f:
-                            lines = f.readlines()
-                        new_lineno = lineno - 2
-                        text = lines[new_lineno]
-                        stack[i] = (filename, new_lineno, function, text)
-
-                # Format the stack to print and print it
-                out = traceback.format_list(stack)
-                for line in out:
-                    print(line.rstrip("\n"))
-
-                if exc_type is spack.util.executable.ProcessError:
-                    out = io.StringIO()
-                    spack.build_environment.write_log_summary(
-                        out, "test", self.tester.test_log_file, last=1
-                    )
-                    m = out.getvalue()
-                else:
-                    # We're below the package context, so get context from
-                    # stack instead of from traceback.
-                    # The traceback is truncated here, so we can't use it to
-                    # traverse the stack.
-                    context = spack.build_environment.get_package_context(tb)
-                    m = "\n".join(context) if context else ""
-
-                exc = e  # e is deleted after this block
-
-                # If we fail fast, raise another error
-                if spack.config.get("config:fail_fast", False):
-                    raise TestFailure([(exc, m)])
-                else:
-                    self.tester.add_failure(exc, m)
-                return False
-
-    # TODO (post-34236): Remove this deprecated method when eliminate test,
-    # TODO (post-34236): run_test, etc.
-    def _run_test_helper(self, runner, options, expected, status, installed, purpose):
-        status = [status] if isinstance(status, int) else status
-        expected = [expected] if isinstance(expected, str) else expected
-        options = [options] if isinstance(options, str) else options
-
-        if installed:
-            msg = f"Executable '{runner.name}' expected in prefix, "
-            msg += f"found in {runner.path} instead"
-            assert runner.path.startswith(self.spec.prefix), msg
-
-        tty.msg(f"Expecting return code in {status}")
-
-        try:
-            output = runner(*options, output=str.split, error=str.split)
-
-            assert 0 in status, f"Expected {runner.name} execution to fail"
-        except ProcessError as err:
-            output = str(err)
-            match = re.search(r"exited with status ([0-9]+)", output)
-            if not (match and int(match.group(1)) in status):
-                raise
-
-        for check in expected:
-            cmd = " ".join([runner.name] + options)
-            msg = f"Expected '{check}' to match output of `{cmd}`"
-            msg += f"\n\nOutput: {output}"
-            assert re.search(check, output), msg
-
     def unit_test_check(self):
         """Hook for unit tests to assert things about package internals.
 

lib/spack/spack/reporters/extract.py

@@ -2,7 +2,6 @@
 # Spack Project Developers. See the top-level COPYRIGHT file for details.
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
-import os
 import re
 import xml.sax.saxutils
 from datetime import datetime
@@ -42,17 +41,6 @@ def elapsed(current, previous):
     return diff.total_seconds()
 
 
-# TODO (post-34236): Should remove with deprecated test methods since don't
-# TODO (post-34236): have an XFAIL mechanism with the new test_part() approach.
-def expected_failure(line):
-    if not line:
-        return False
-
-    match = returns_regexp.search(line)
-    xfail = "0" not in match.group(1) if match else False
-    return xfail
-
-
 def new_part():
     return {
         "command": None,
@@ -66,14 +54,6 @@ def new_part():
     }
 
 
-# TODO (post-34236): Remove this when remove deprecated methods
-def part_name(source):
-    elements = []
-    for e in source.replace("'", "").split(" "):
-        elements.append(os.path.basename(e) if os.sep in e else e)
-    return "_".join(elements)
-
-
 def process_part_end(part, curr_time, last_time):
     if part:
         if not part["elapsed"]:
@@ -81,11 +61,7 @@ def process_part_end(part, curr_time, last_time):
 
         stat = part["status"]
         if stat in completed:
-            # TODO (post-34236): remove the expected failure mapping when
+            if part["completed"] == "Unknown":
-            # TODO (post-34236): remove deprecated test methods.
-            if stat == "passed" and expected_failure(part["desc"]):
-                part["completed"] = "Expected to fail"
-            elif part["completed"] == "Unknown":
                 part["completed"] = completed[stat]
         elif stat is None or stat == "unknown":
             part["status"] = "passed"
@@ -153,14 +129,6 @@ def extract_test_parts(default_name, outputs):
                 if msg.startswith("Installing"):
                     continue
 
-                # TODO (post-34236): Remove this check when remove run_test(),
-                # TODO (post-34236): etc. since no longer supporting expected
-                # TODO (post-34236): failures.
-                if msg.startswith("Expecting return code"):
-                    if part:
-                        part["desc"] += f"; {msg}"
-                    continue
-
                 # Terminate without further parsing if no more test messages
                 if "Completed testing" in msg:
                     # Process last lingering part IF it didn't generate status

lib/spack/spack/test/audit.py

@@ -27,8 +27,15 @@
         (["invalid-gitlab-patch-url"], ["PKG-DIRECTIVES", "PKG-PROPERTIES"]),
         # This package has invalid GitLab patch URLs
         (["invalid-selfhosted-gitlab-patch-url"], ["PKG-DIRECTIVES", "PKG-PROPERTIES"]),
-        # This package has a stand-alone 'test*' method in build-time callbacks
+        # This package has a stand-alone test method in build-time callbacks
-        (["fail-test-audit"], ["PKG-DIRECTIVES", "PKG-PROPERTIES"]),
+        (["fail-test-audit"], ["PKG-PROPERTIES"]),
+        # This package implements and uses several deprecated stand-alone
+        # test methods
+        (["fail-test-audit-deprecated"], ["PKG-DEPRECATED-ATTRIBUTES"]),
+        # This package has stand-alone test methods without non-trivial docstrings
+        (["fail-test-audit-docstring"], ["PKG-PROPERTIES"]),
+        # This package has a stand-alone test method without an implementation
+        (["fail-test-audit-impl"], ["PKG-PROPERTIES"]),
         # This package has no issues
         (["mpileaks"], None),
         # This package has a conflict with a trigger which cannot constrain the constraint
@@ -41,7 +48,7 @@ def test_package_audits(packages, expected_errors, mock_packages):
 
     # Check that errors were reported only for the expected failure
     actual_errors = [check for check, errors in reports if errors]
-    msg = [str(e) for _, errors in reports for e in errors]
+    msg = "\n".join([str(e) for _, errors in reports for e in errors])
     if expected_errors:
         assert expected_errors == actual_errors, msg
     else:

lib/spack/spack/test/cmd/test.py

@@ -194,6 +194,9 @@ def test_test_list_all(mock_packages):
     assert set(pkgs) == set(
         [
             "fail-test-audit",
+            "fail-test-audit-deprecated",
+            "fail-test-audit-docstring",
+            "fail-test-audit-impl",
             "mpich",
             "perl-extension",
             "printing-package",

lib/spack/spack/test/data/unparse/legion.txt

@@ -357,37 +357,28 @@ class Legion(CMakePackage):
         install test subdirectory for use during `spack test run`."""
         cache_extra_test_sources(self, [join_path('examples', 'local_function_tasks')])
 
-    def run_local_function_tasks_test(self):
+    def test_run_local_function_tasks(self):
-        """Run stand alone test: local_function_tasks"""
+        """Build and run external application example"""
 
-        test_dir = join_path(self.test_suite.current_test_cache_dir,
+        test_dir = join_path(
-                             'examples', 'local_function_tasks')
+            self.test_suite.current_test_cache_dir, "examples", "local_function_tasks"
+        )
 
         if not os.path.exists(test_dir):
-            print('Skipping local_function_tasks test')
+            raise SkipTest(f"{test_dir} must exist")
-            return
 
-        exe = 'local_function_tasks'
+        cmake_args = [
+            f"-DCMAKE_C_COMPILER={self.compiler.cc}",
+            f"-DCMAKE_CXX_COMPILER={self.compiler.cxx}",
+            f"-DLegion_DIR={join_path(self.prefix, 'share', 'Legion', 'cmake')}",
+        ]
 
-        cmake_args = ['-DCMAKE_C_COMPILER={0}'.format(self.compiler.cc),
+        with working_dir(test_dir):
-                      '-DCMAKE_CXX_COMPILER={0}'.format(self.compiler.cxx),
+            cmake = self.spec["cmake"].command
-                      '-DLegion_DIR={0}'.format(join_path(self.prefix,
+            cmake(*cmake_args)
-                                                          'share',
-                                                          'Legion',
-                                                          'cmake'))]
 
-        self.run_test('cmake',
+            make = which("make")
-                      options=cmake_args,
+            make()
-                      purpose='test: generate makefile for {0} example'.format(exe),
-                      work_dir=test_dir)
 
-        self.run_test('make',
+            exe = which("local_function_tasks")
-                      purpose='test: build {0} example'.format(exe),
+            exe()
-                      work_dir=test_dir)
-
-        self.run_test(exe,
-                      purpose='test: run {0} example'.format(exe),
-                      work_dir=test_dir)
-
-    def test(self):
-        self.run_local_function_tasks_test()

lib/spack/spack/test/data/unparse/mfem.txt

@@ -816,40 +816,22 @@ class Mfem(Package, CudaPackage, ROCmPackage):
         install test subdirectory for use during `spack test run`."""
         cache_extra_test_sources(self, [self.examples_src_dir, self.examples_data_dir])
 
-    def test(self):
+    def test_ex10(self):
-        test_dir = join_path(
+        """build and run ex10(p)"""
-            self.test_suite.current_test_cache_dir,
-            self.examples_src_dir
-        )
-
         # MFEM has many examples to serve as a suitable smoke check. ex10
         # was chosen arbitrarily among the examples that work both with
         # MPI and without it
-        test_exe = 'ex10p' if ('+mpi' in self.spec) else 'ex10'
+        test_dir = join_path(self.test_suite.current_test_cache_dir, self.examples_src_dir)
-        self.run_test(
-            'make',
-            [
-                'CONFIG_MK={0}/share/mfem/config.mk'.format(self.prefix),
-                test_exe,
-                'parallel=False'
-            ],
-            purpose='test: building {0}'.format(test_exe),
-            skip_missing=False,
-            work_dir=test_dir,
-        )
 
-        self.run_test(
+        mesh = join_path("..", self.examples_data_dir, "beam-quad.mesh")
-            './{0}'.format(test_exe),
+        test_exe = "ex10p" if ("+mpi" in self.spec) else "ex10"
-            [
+
-                '--mesh',
+        with working_dir(test_dir):
-                '../{0}/beam-quad.mesh'.format(self.examples_data_dir)
+            make = which("make")
-            ],
+            make(f"CONFIG_MK={self.config_mk}", test_exe, "parallel=False")
-            [],
+
-            installed=False,
+            ex10 = which(test_exe)
-            purpose='test: running {0}'.format(test_exe),
+            ex10("--mesh", mesh)
-            skip_missing=False,
-            work_dir=test_dir,
-        )
 
     # this patch is only needed for mfem 4.1, where a few
     # released files include byte order marks

lib/spack/spack/test/package_class.py

@@ -286,56 +286,3 @@ def compilers(compiler, arch_spec):
     error = capfd.readouterr()[1]
     assert "Skipping tests for package" in error
     assert "test requires missing compiler" in error
-
-
-# TODO (post-34236): Remove when remove deprecated run_test(), etc.
-@pytest.mark.not_on_windows("echo not available on Windows")
-@pytest.mark.parametrize(
-    "msg,installed,purpose,expected",
-    [
-        ("do-nothing", False, "test: echo", "do-nothing"),
-        ("not installed", True, "test: echo not installed", "expected in prefix"),
-    ],
-)
-def test_package_run_test_install(
-    install_mockery, mock_fetch, capfd, msg, installed, purpose, expected
-):
-    """Confirm expected outputs from run_test for installed/not installed exe."""
-    s = spack.spec.Spec("trivial-smoke-test").concretized()
-    pkg = s.package
-
-    pkg.run_test(
-        "echo", msg, expected=[expected], installed=installed, purpose=purpose, work_dir="."
-    )
-    output = capfd.readouterr()[0]
-    assert expected in output
-
-
-# TODO (post-34236): Remove when remove deprecated run_test(), etc.
-@pytest.mark.parametrize(
-    "skip,failures,status",
-    [
-        (True, 0, str(spack.install_test.TestStatus.SKIPPED)),
-        (False, 1, str(spack.install_test.TestStatus.FAILED)),
-    ],
-)
-def test_package_run_test_missing(install_mockery, mock_fetch, capfd, skip, failures, status):
-    """Confirm expected results from run_test for missing exe when skip or not."""
-    s = spack.spec.Spec("trivial-smoke-test").concretized()
-    pkg = s.package
-
-    pkg.run_test("no-possible-program", skip_missing=skip)
-    output = capfd.readouterr()[0]
-    assert len(pkg.tester.test_failures) == failures
-    assert status in output
-
-
-# TODO (post-34236): Remove when remove deprecated run_test(), etc.
-def test_package_run_test_fail_fast(install_mockery, mock_fetch):
-    """Confirm expected exception when run_test with fail_fast enabled."""
-    s = spack.spec.Spec("trivial-smoke-test").concretized()
-    pkg = s.package
-
-    with spack.config.override("config:fail_fast", True):
-        with pytest.raises(spack.install_test.TestFailure, match="Failed to find executable"):
-            pkg.run_test("no-possible-program")

lib/spack/spack/test/reporters.py

@@ -120,26 +120,6 @@ def test_reporters_extract_missing_desc():
     assert parts[2]["command"] == "exe1 1; exe2 2"
 
 
-# TODO (post-34236): Remove this test when removing deprecated run_test(), etc.
-def test_reporters_extract_xfail():
-    fake_bin = fs.join_path(fake_install_prefix, "bin", "fake-app")
-    outputs = """
-==> Testing package fake-1.0-abcdefg
-==> [2022-02-15-18:44:21.250165] test: test_fake: Checking fake imports
-==> [2022-02-15-18:44:21.250175] Expecting return code in [3]
-==> [2022-02-15-18:44:21.250200] '{0}'
-{1}
-""".format(
-        fake_bin, str(TestStatus.PASSED)
-    ).splitlines()
-
-    parts = spack.reporters.extract.extract_test_parts("fake", outputs)
-
-    assert len(parts) == 1
-    parts[0]["command"] == fake_bin
-    parts[0]["completed"] == "Expected to fail"
-
-
 @pytest.mark.parametrize("state", [("not installed"), ("external")])
 def test_reporters_extract_skipped(state):
     expected = "Skipped {0} package".format(state)
@@ -156,34 +136,6 @@ def test_reporters_extract_skipped(state):
     parts[0]["completed"] == expected
 
 
-# TODO (post-34236): Remove this test when removing deprecated run_test(), etc.
-def test_reporters_skip():
-    # This test ticks 3 boxes:
-    # 1) covers an as yet uncovered skip messages
-    # 2) covers debug timestamps
-    # 3) unrecognized output
-    fake_bin = fs.join_path(fake_install_prefix, "bin", "fake")
-    unknown_message = "missing timestamp"
-    outputs = """
-==> Testing package fake-1.0-abcdefg
-==> [2022-02-15-18:44:21.250165, 123456] Detected the following modules: fake1
-==> {0}
-==> [2022-02-15-18:44:21.250175, 123456] test: test_fake: running fake program
-==> [2022-02-15-18:44:21.250200, 123456] '{1}'
-INVALID
-Results for test suite abcdefghijklmn
-""".format(
-        unknown_message, fake_bin
-    ).splitlines()
-
-    parts = spack.reporters.extract.extract_test_parts("fake", outputs)
-
-    assert len(parts) == 1
-    assert fake_bin in parts[0]["command"]
-    assert parts[0]["loglines"] == ["INVALID"]
-    assert parts[0]["elapsed"] == 0.0
-
-
 def test_reporters_skip_new():
     outputs = """
 ==> [2023-04-06-15:55:13.094025] test: test_skip:

lib/spack/spack/test/util/package_hash.py

@@ -338,15 +338,15 @@ def test_remove_complex_package_logic_filtered():
         ("grads", "rrlmwml3f2frdnqavmro3ias66h5b2ce"),
         ("llvm", "nufffum5dabmaf4l5tpfcblnbfjknvd3"),
         # has @when("@4.1.0") and raw unicode literals
-        ("mfem", "lbhr43gm5zdye2yhqznucxb4sg6vhryl"),
+        ("mfem", "whwftpqbjvzncmb52oz6izkanbha2uji"),
-        ("mfem@4.0.0", "lbhr43gm5zdye2yhqznucxb4sg6vhryl"),
+        ("mfem@4.0.0", "whwftpqbjvzncmb52oz6izkanbha2uji"),
-        ("mfem@4.1.0", "vjdjdgjt6nyo7ited2seki5epggw5gza"),
+        ("mfem@4.1.0", "bpi7of3xelo7fr3ta2lm6bmiruijnxcg"),
         # has @when("@1.5.0:")
         ("py-torch", "qs7djgqn7dy7r3ps4g7hv2pjvjk4qkhd"),
         ("py-torch@1.0", "qs7djgqn7dy7r3ps4g7hv2pjvjk4qkhd"),
         ("py-torch@1.6", "p4ine4hc6f2ik2f2wyuwieslqbozll5w"),
         # has a print with multiple arguments
-        ("legion", "efpfd2c4pzhsbyc3o7plqcmtwm6b57yh"),
+        ("legion", "bq2etsik5l6pbryxmbhfhzynci56ruy4"),
         # has nested `with when()` blocks and loops
         ("trilinos", "vqrgscjrla4hi7bllink7v6v6dwxgc2p"),
     ],

var/spack/repos/builtin.mock/packages/fail-test-audit-deprecated/package.py

@@ -0,0 +1,32 @@
+# Copyright 2013-2024 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)
+from spack.package import *
+
+
+class FailTestAuditDeprecated(MakefilePackage):
+    """Simple package attempting to implement and use deprecated stand-alone test methods."""
+
+    homepage = "http://github.com/dummy/fail-test-audit-deprecated"
+    url = "https://github.com/dummy/fail-test-audit-deprecated/archive/v1.0.tar.gz"
+
+    version("2.0", sha256="c3e5e9fdd5004dcb542feda5ee4f0ff0744628baf8ed2dd5d66f8ca1197cb1a1")
+    version("1.0", sha256="abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234")
+
+    @run_after("install")
+    def copy_test_files(self):
+        """test that uses the deprecated install_test_root method"""
+        self.cache_extra_test_sources(".")
+
+    def test(self):
+        """this is a deprecated reserved method for stand-alone testing"""
+        pass
+
+    def test_use_install_test_root(self):
+        """use the deprecated install_test_root method"""
+        print(f"install test root = {self.install_test_root()}")
+
+    def test_run_test(self):
+        """use the deprecated run_test method"""
+        self.run_test("which", ["make"])

var/spack/repos/builtin.mock/packages/fail-test-audit-docstring/package.py

@@ -0,0 +1,24 @@
+# Copyright 2013-2024 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)
+from spack.package import *
+
+
+class FailTestAuditDocstring(MakefilePackage):
+    """Simple package with a stand-alone test that is missing its docstring."""
+
+    homepage = "http://github.com/dummy/fail-test-audit-docstring"
+    url = "https://github.com/dummy/fail-test-audit-docstring/archive/v1.0.tar.gz"
+
+    version("2.0", sha256="c3e5e9fdd5004dcb542feda5ee4f0ff0744628baf8ed2dd5d66f8ca1197cb1a1")
+    version("1.0", sha256="abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234")
+
+    # The required docstring is missing.
+    def test_missing_docstring(self):
+        print("Ran test_missing_docstring")
+
+    # The required docstring is effectively empty.
+    def test_empty_docstring(self):
+        """ """
+        print("Ran test_empty_docstring")

var/spack/repos/builtin.mock/packages/fail-test-audit-impl/package.py

@@ -0,0 +1,21 @@
+# Copyright 2013-2024 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)
+from spack.package import *
+
+
+class FailTestAuditImpl(MakefilePackage):
+    """Simple package that is missing the stand-alone test implementation."""
+
+    homepage = "http://github.com/dummy/fail-test-audit-impl"
+    url = "https://github.com/dummy/fail-test-audit-impl/archive/v1.0.tar.gz"
+
+    version("2.0", sha256="c3e5e9fdd5004dcb542feda5ee4f0ff0744628baf8ed2dd5d66f8ca1197cb1a1")
+    version("1.0", sha256="abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234")
+
+    # The test method has not been implemented.
+    def test_no_impl(self):
+        """test sans implementation"""
+        # this comment should not matter
+        pass

var/spack/repos/builtin.mock/packages/fail-test-audit/package.py

@@ -6,16 +6,17 @@
 
 
 class FailTestAudit(MakefilePackage):
-    """Simple package with one optional dependency"""
+    """Simple package attempting to re-use stand-alone test method as a build check."""
 
-    homepage = "http://www.example.com"
+    homepage = "http://github.com/dummy/fail-test-audit"
-    url = "http://www.example.com/a-1.0.tar.gz"
+    url = "https://github.com/dummy/fail-test-audit/archive/v1.0.tar.gz"
 
-    version("1.0", md5="0123456789abcdef0123456789abcdef")
+    version("2.0", sha256="c3e5e9fdd5004dcb542feda5ee4f0ff0744628baf8ed2dd5d66f8ca1197cb1a1")
-    version("2.0", md5="abcdef0123456789abcdef0123456789")
+    version("1.0", sha256="abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234abcd1234")
 
+    # Stand-alone test methods cannot be included in build_time_test_callbacks
     build_time_test_callbacks = ["test_build_callbacks"]
 
     def test_build_callbacks(self):
-        """test build time test callbacks"""
+        """test build time test callbacks failure"""
-        print("test-build-callbacks")
+        print("test_build_callbacks")

var/spack/repos/builtin.mock/packages/py-test-callback/package.py

@@ -4,6 +4,7 @@
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
 import spack.pkg.builtin.mock.python as mp
+from spack.build_systems._checks import BaseBuilder, execute_install_time_tests
 from spack.package import *
 
 
@@ -13,8 +14,11 @@ class PyTestCallback(mp.Python):
     homepage = "http://www.example.com"
     url = "http://www.example.com/test-callback-1.0.tar.gz"
 
-    # TODO (post-34236): "test" -> "test_callback" once remove "test" support
+    #: This attribute is used in UI queries that need to know the build
-    install_time_test_callbacks = ["test"]
+    #: system base class
+    build_system_class = "PyTestCallback"
+
+    build_system("testcallback")
 
     version("1.0", "00000000000000000000000000000110")
     version("2.0", "00000000000000000000000000000120")
@@ -22,8 +26,21 @@ class PyTestCallback(mp.Python):
     def install(self, spec, prefix):
         mkdirp(prefix.bin)
 
-    # TODO (post-34236): "test" -> "test_callback" once remove "test" support
+    def test_callback(self):
-    def test(self):
-        super().test()
-
         print("PyTestCallback test")
+
+
+@spack.builder.builder("testcallback")
+class MyBuilder(BaseBuilder):
+    phases = ("install",)
+
+    #: Callback names for install-time test
+    install_time_test_callbacks = ["test_callback"]
+
+    def install(self, pkg, spec, prefix):
+        pkg.install(spec, prefix)
+
+    spack.builder.run_after("install")(execute_install_time_tests)
+
+    def test_callback(self):
+        self.pkg.test_callback()

var/spack/repos/builtin.mock/packages/trivial-smoke-test/package.py

@@ -21,9 +21,9 @@ def install(self, spec, prefix):
 
     @run_before("install")
     def create_extra_test_source(self):
-        mkdirp(self.install_test_root)
+        mkdirp(install_test_root(self))
-        touch(join_path(self.install_test_root, self.test_source_filename))
+        touch(join_path(install_test_root(self), self.test_source_filename))
 
     @run_after("install")
     def copy_test_sources(self):
-        self.cache_extra_test_sources([self.test_source_filename])
+        cache_extra_test_sources(self, [self.test_source_filename])

var/spack/repos/builtin/packages/binutils/package.py

@@ -224,6 +224,7 @@ def flag_handler(self, name, flags):
         return (iflags, None, flags)
 
     def test_binaries(self):
+        """check versions reported by binaries"""
         binaries = [
             "ar",
             "c++filt",

var/spack/repos/builtin/packages/cpmd/package.py

@@ -78,6 +78,7 @@ def install(self, spec, prefix):
         install_tree(".", prefix)
 
     def test_cpmd(self):
+        """check cpmd.x outputs"""
         test_dir = self.test_suite.current_test_data_dir
         test_file = join_path(test_dir, "1-h2o-pbc-geoopt.inp")
         opts = []

var/spack/repos/builtin/packages/hip/package.py

@@ -620,7 +620,7 @@ def cache_test_sources(self):
             cache_extra_test_sources(self, [self.test_src_dir])
 
     def test_samples(self):
-        # configure, build and run all hip samples
+        """build and run all hip samples"""
         if self.spec.satisfies("@5.1:5.5"):
             test_dir = join_path(self.test_suite.current_test_cache_dir, self.test_src_dir_old)
         elif self.spec.satisfies("@5.6:"):

var/spack/repos/builtin/packages/mptensor/package.py

@@ -76,7 +76,7 @@ def setup_build_tests(self):
             print("Skipping copy of stand-alone test files: requires +mpi build")
             return
 
-        self.cache_extra_test_sources(".")
+        cache_extra_test_sources(self, ".")
 
         # Clean cached makefiles now so only done once
         print("Converting cached Makefile for stand-alone test use")

var/spack/repos/builtin/packages/openmpi/package.py

@@ -1315,7 +1315,7 @@ def test_mpirun(self):
         self.run_installed_binary("mpirun", options, [f"openmpi-{self.spec.version}"])
 
     def test_opmpi_info(self):
-        """test installed mpirun"""
+        """test installed ompi_info"""
         self.run_installed_binary("ompi_info", [], [f"Ident string: {self.spec.version}", "MCA"])
 
     def test_version(self):

var/spack/repos/builtin/packages/phist/package.py

@@ -325,6 +325,7 @@ def check(self):
     @run_after("install")
     @on_package_attributes(run_tests=True)
     def test_install(self):
+        """run 'make test_install'"""
         # The build script of test_install expects the sources to be copied here:
         install_tree(
             join_path(self.stage.source_path, "exampleProjects"),

var/spack/repos/builtin/packages/py-genshi/package.py

@@ -21,4 +21,5 @@ class PyGenshi(PythonPackage):
     depends_on("py-six", type=("build", "run", "test"))
 
     def test_testsuite(self):
+        """run unittest suite"""
         python("-m", "unittest", "-v", "genshi.tests.suite")

var/spack/repos/builtin/packages/qscintilla/package.py

@@ -137,11 +137,12 @@ def make_qsci_python(self):
             make("install", "-C", "build/")
 
     def test_python_import(self):
-        if "+python" in self.spec:
+        """check Qsci import"""
-            python = self.spec["python"].command
+        if self.spec.satisfies("~python"):
-            if "^py-pyqt5" in self.spec:
+            raise SkipTest("Package must be installed with +python")
-                python("-c", "import PyQt5.Qsci")
+
-            if "^py-pyqt6" in self.spec:
+        python = self.spec["python"].command
-                python("-c", "import PyQt6.Qsci")
+        if "^py-pyqt5" in self.spec:
-        else:
+            python("-c", "import PyQt5.Qsci")
-            print("qscintilla ins't built with python, skipping import test")
+        if "^py-pyqt6" in self.spec:
+            python("-c", "import PyQt6.Qsci")