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>

15 files changed, 224 insertions, 475 deletions

diff --git a/lib/spack/docs/build_systems/custompackage.rst b/lib/spack/docs/build_systems/custompackage.rst
index 35475fc8ec..cf5862d6a0 100644
--- a/lib/spack/docs/build_systems/custompackage.rst
+++ b/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
-
-       if spec.satisfies("+cpanm"):
-           with working_dir(join_path("cpanm", "cpanm")):
-               perl = spec["perl"].command
-               perl("Makefile.PL")
-               make()
-               make("install")
+        spec = self.spec
+        maker = make
+        cpan_dir = join_path("cpanm", "cpanm")
+        if sys.platform == "win32":
+            maker = nmake
+            cpan_dir = join_path(self.stage.source_path, cpan_dir)
+            cpan_dir = windows_sfn(cpan_dir)
+        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):
-       make("test")
+   def build_test(self):
+        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
diff --git a/lib/spack/docs/build_systems/sconspackage.rst b/lib/spack/docs/build_systems/sconspackage.rst
index 8200d0998d..5e4b888c1c 100644
--- a/lib/spack/docs/build_systems/sconspackage.rst
+++ b/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")
 
 
diff --git a/lib/spack/spack/audit.py b/lib/spack/spack/audit.py
index 6b441c6965..3ccdd5b9cd 100644
--- a/lib/spack/spack/audit.py
+++ b/lib/spack/spack/audit.py
@@ -39,6 +39,7 @@ import ast
 import collections
 import collections.abc
 import glob
+import inspect
 import io
 import itertools
 import os
@@ -50,6 +51,7 @@ from typing import Iterable, List, Set, Tuple
 from urllib.request import urlopen
 
 import llnl.util.lang
+from llnl.string import plural
 
 import spack.builder
 import spack.config
@@ -386,6 +388,14 @@ package_attributes = AuditClass(
 )
 
 
+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 @@ package_https_directives = AuditClass(
 )
 
 
-@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
-        # TODO (post-34236): "test"->"test_" once remove deprecated methods
-        has_test_method = test_callbacks and any([m.startswith("test") for m in test_callbacks])
+        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"
-            instr = 'Remove all methods whose names start with "test" from: [{0}]'.format(
-                ", ".join(test_callbacks)
-            )
+            msg = f"Package {pkg_name} includes stand-alone test methods in build-time checks."
+            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"""
diff --git a/lib/spack/spack/build_systems/python.py b/lib/spack/spack/build_systems/python.py
index e589ac94ef..b951ec2a97 100644
--- a/lib/spack/spack/build_systems/python.py
+++ b/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]:
diff --git a/lib/spack/spack/builder.py b/lib/spack/spack/builder.py
index eaeb82b17c..098f583d53 100644
--- a/lib/spack/spack/builder.py
+++ b/lib/spack/spack/builder.py
@@ -521,10 +521,6 @@ class Builder(collections.abc.Sequence, metaclass=BuilderMeta):
     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.
 
diff --git a/lib/spack/spack/install_test.py b/lib/spack/spack/install_test.py
index 9bd171f017..f21f4f8cde 100644
--- a/lib/spack/spack/install_test.py
+++ b/lib/spack/spack/install_test.py
@@ -372,8 +372,7 @@ class PackageTest:
             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 @@ class PackageTest:
 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"
-    ), 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)
-        )
+        "test_"
+    ), f"Test name must start with 'test_' but {test_name} was provided"
 
     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"):
-                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:
+            if not name.startswith("test_"):
                 continue
 
             tests.append((clss.__name__, test_fn))  # type: ignore[union-attr]
diff --git a/lib/spack/spack/package_base.py b/lib/spack/spack/package_base.py
index 414020faa4..f0458e47cc 100644
--- a/lib/spack/spack/package_base.py
+++ b/lib/spack/spack/package_base.py
@@ -55,17 +55,9 @@ import spack.util.path
 import spack.util.web
 from spack.error import InstallError, NoURLError, PackageError
 from spack.filesystem_view import YamlFilesystemView
-from spack.install_test import (
-    PackageTest,
-    TestFailure,
-    TestStatus,
-    TestSuite,
-    cache_extra_test_sources,
-    install_test_root,
-)
+from spack.install_test import PackageTest, TestSuite
 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 @@ class PackageBase(WindowsRPath, PackageViewMixin, RedistributionMixin, metaclass
         """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 @@ class PackageBase(WindowsRPath, PackageViewMixin, RedistributionMixin, metaclass
         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 @@ class PackageBase(WindowsRPath, PackageViewMixin, RedistributionMixin, metaclass
 
         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.
 
diff --git a/lib/spack/spack/reporters/extract.py b/lib/spack/spack/reporters/extract.py
index 5554d89f0a..b222fdbad9 100644
--- a/lib/spack/spack/reporters/extract.py
+++ b/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
-            # TODO (post-34236): remove deprecated test methods.
-            if stat == "passed" and expected_failure(part["desc"]):
-                part["completed"] = "Expected to fail"
-            elif part["completed"] == "Unknown":
+            if 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
diff --git a/lib/spack/spack/test/audit.py b/lib/spack/spack/test/audit.py
index 0d2ca594f1..18994fa88b 100644
--- a/lib/spack/spack/test/audit.py
+++ b/lib/spack/spack/test/audit.py
@@ -27,8 +27,15 @@ import spack.config
         (["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
-        (["fail-test-audit"], ["PKG-DIRECTIVES", "PKG-PROPERTIES"]),
+        # This package has a stand-alone test method in build-time callbacks
+        (["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:
diff --git a/lib/spack/spack/test/cmd/test.py b/lib/spack/spack/test/cmd/test.py
index 3b3785a9ae..866d579aa7 100644
--- a/lib/spack/spack/test/cmd/test.py
+++ b/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",
diff --git a/lib/spack/spack/test/data/unparse/legion.txt b/lib/spack/spack/test/data/unparse/legion.txt
index d6b37e2c9a..788375d497 100644
--- a/lib/spack/spack/test/data/unparse/legion.txt
+++ b/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):
-        """Run stand alone test: local_function_tasks"""
+    def test_run_local_function_tasks(self):
+        """Build and run external application example"""
 
-        test_dir = join_path(self.test_suite.current_test_cache_dir,
-                             'examples', 'local_function_tasks')
+        test_dir = join_path(
+            self.test_suite.current_test_cache_dir, "examples", "local_function_tasks"
+        )
 
         if not os.path.exists(test_dir):
-            print('Skipping local_function_tasks test')
-            return
+            raise SkipTest(f"{test_dir} must exist")
 
-        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),
-                      '-DCMAKE_CXX_COMPILER={0}'.format(self.compiler.cxx),
-                      '-DLegion_DIR={0}'.format(join_path(self.prefix,
-                                                          'share',
-                                                          'Legion',
-                                                          'cmake'))]
+        with working_dir(test_dir):
+            cmake = self.spec["cmake"].command
+            cmake(*cmake_args)
 
-        self.run_test('cmake',
-                      options=cmake_args,
-                      purpose='test: generate makefile for {0} example'.format(exe),
-                      work_dir=test_dir)
+            make = which("make")
+            make()
 
-        self.run_test('make',
-                      purpose='test: build {0} example'.format(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()
+            exe = which("local_function_tasks")
+            exe()
diff --git a/lib/spack/spack/test/data/unparse/mfem.txt b/lib/spack/spack/test/data/unparse/mfem.txt
index 20dea06035..bdcb2e2615 100644
--- a/lib/spack/spack/test/data/unparse/mfem.txt
+++ b/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):
-        test_dir = join_path(
-            self.test_suite.current_test_cache_dir,
-            self.examples_src_dir
-        )
-
+    def test_ex10(self):
+        """build and run ex10(p)"""
         # 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'
-        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(
-            './{0}'.format(test_exe),
-            [
-                '--mesh',
-                '../{0}/beam-quad.mesh'.format(self.examples_data_dir)
-            ],
-            [],
-            installed=False,
-            purpose='test: running {0}'.format(test_exe),
-            skip_missing=False,
-            work_dir=test_dir,
-        )
+        test_dir = join_path(self.test_suite.current_test_cache_dir, self.examples_src_dir)
+
+        mesh = join_path("..", self.examples_data_dir, "beam-quad.mesh")
+        test_exe = "ex10p" if ("+mpi" in self.spec) else "ex10"
+
+        with working_dir(test_dir):
+            make = which("make")
+            make(f"CONFIG_MK={self.config_mk}", test_exe, "parallel=False")
+
+            ex10 = which(test_exe)
+            ex10("--mesh", mesh)
 
     # this patch is only needed for mfem 4.1, where a few
     # released files include byte order marks
diff --git a/lib/spack/spack/test/package_class.py b/lib/spack/spack/test/package_class.py
index 70955e9638..76ace98049 100644
--- a/lib/spack/spack/test/package_class.py
+++ b/lib/spack/spack/test/package_class.py
@@ -286,56 +286,3 @@ def test_package_test_no_compilers(mock_packages, monkeypatch, capfd):
     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")
diff --git a/lib/spack/spack/test/reporters.py b/lib/spack/spack/test/reporters.py
index 72fb8ddd52..55ab019a8a 100644
--- a/lib/spack/spack/test/reporters.py
+++ b/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:
diff --git a/lib/spack/spack/test/util/package_hash.py b/lib/spack/spack/test/util/package_hash.py
index 5a0251b2d0..cfadae40be 100644
--- a/lib/spack/spack/test/util/package_hash.py
+++ b/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@4.0.0", "lbhr43gm5zdye2yhqznucxb4sg6vhryl"),
-        ("mfem@4.1.0", "vjdjdgjt6nyo7ited2seki5epggw5gza"),
+        ("mfem", "whwftpqbjvzncmb52oz6izkanbha2uji"),
+        ("mfem@4.0.0", "whwftpqbjvzncmb52oz6izkanbha2uji"),
+        ("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"),
     ],