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>

14 files changed, 127 insertions, 27 deletions

diff --git a/var/spack/repos/builtin.mock/packages/fail-test-audit-deprecated/package.py b/var/spack/repos/builtin.mock/packages/fail-test-audit-deprecated/package.py
new file mode 100644
index 0000000000..7c4181b7b7
--- /dev/null
+++ b/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"])
diff --git a/var/spack/repos/builtin.mock/packages/fail-test-audit-docstring/package.py b/var/spack/repos/builtin.mock/packages/fail-test-audit-docstring/package.py
new file mode 100644
index 0000000000..3f0da784ae
--- /dev/null
+++ b/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")
diff --git a/var/spack/repos/builtin.mock/packages/fail-test-audit-impl/package.py b/var/spack/repos/builtin.mock/packages/fail-test-audit-impl/package.py
new file mode 100644
index 0000000000..886f426706
--- /dev/null
+++ b/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
diff --git a/var/spack/repos/builtin.mock/packages/fail-test-audit/package.py b/var/spack/repos/builtin.mock/packages/fail-test-audit/package.py
index 54b63ed79b..dd94c37427 100644
--- a/var/spack/repos/builtin.mock/packages/fail-test-audit/package.py
+++ b/var/spack/repos/builtin.mock/packages/fail-test-audit/package.py
@@ -6,16 +6,17 @@ from spack.package import *
 
 
 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"
-    url = "http://www.example.com/a-1.0.tar.gz"
+    homepage = "http://github.com/dummy/fail-test-audit"
+    url = "https://github.com/dummy/fail-test-audit/archive/v1.0.tar.gz"
 
-    version("1.0", md5="0123456789abcdef0123456789abcdef")
-    version("2.0", md5="abcdef0123456789abcdef0123456789")
+    version("2.0", sha256="c3e5e9fdd5004dcb542feda5ee4f0ff0744628baf8ed2dd5d66f8ca1197cb1a1")
+    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"""
-        print("test-build-callbacks")
+        """test build time test callbacks failure"""
+        print("test_build_callbacks")
diff --git a/var/spack/repos/builtin.mock/packages/py-test-callback/package.py b/var/spack/repos/builtin.mock/packages/py-test-callback/package.py
index 46a16a465f..b152490895 100644
--- a/var/spack/repos/builtin.mock/packages/py-test-callback/package.py
+++ b/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
-    install_time_test_callbacks = ["test"]
+    #: This attribute is used in UI queries that need to know the build
+    #: 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(self):
-        super().test()
-
+    def test_callback(self):
         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()
diff --git a/var/spack/repos/builtin.mock/packages/trivial-smoke-test/package.py b/var/spack/repos/builtin.mock/packages/trivial-smoke-test/package.py
index 9b877c5ef4..a4c3af3002 100644
--- a/var/spack/repos/builtin.mock/packages/trivial-smoke-test/package.py
+++ b/var/spack/repos/builtin.mock/packages/trivial-smoke-test/package.py
@@ -21,9 +21,9 @@ class TrivialSmokeTest(Package):
 
     @run_before("install")
     def create_extra_test_source(self):
-        mkdirp(self.install_test_root)
-        touch(join_path(self.install_test_root, self.test_source_filename))
+        mkdirp(install_test_root(self))
+        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])
diff --git a/var/spack/repos/builtin/packages/binutils/package.py b/var/spack/repos/builtin/packages/binutils/package.py
index 10f4752f10..18bece43ac 100644
--- a/var/spack/repos/builtin/packages/binutils/package.py
+++ b/var/spack/repos/builtin/packages/binutils/package.py
@@ -224,6 +224,7 @@ class Binutils(AutotoolsPackage, GNUMirrorPackage):
         return (iflags, None, flags)
 
     def test_binaries(self):
+        """check versions reported by binaries"""
         binaries = [
             "ar",
             "c++filt",
diff --git a/var/spack/repos/builtin/packages/cpmd/package.py b/var/spack/repos/builtin/packages/cpmd/package.py
index 8bc10da0c5..42fb0a49f2 100644
--- a/var/spack/repos/builtin/packages/cpmd/package.py
+++ b/var/spack/repos/builtin/packages/cpmd/package.py
@@ -78,6 +78,7 @@ class Cpmd(MakefilePackage):
         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 = []
diff --git a/var/spack/repos/builtin/packages/hip/package.py b/var/spack/repos/builtin/packages/hip/package.py
index 548fc6d5dc..483b6d43bd 100644
--- a/var/spack/repos/builtin/packages/hip/package.py
+++ b/var/spack/repos/builtin/packages/hip/package.py
@@ -620,7 +620,7 @@ class Hip(CMakePackage):
             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:"):
diff --git a/var/spack/repos/builtin/packages/mptensor/package.py b/var/spack/repos/builtin/packages/mptensor/package.py
index 5558ca6242..b3b3e97203 100644
--- a/var/spack/repos/builtin/packages/mptensor/package.py
+++ b/var/spack/repos/builtin/packages/mptensor/package.py
@@ -76,7 +76,7 @@ class Mptensor(CMakePackage):
             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")
diff --git a/var/spack/repos/builtin/packages/openmpi/package.py b/var/spack/repos/builtin/packages/openmpi/package.py
index b7c57c282c..8c509efda1 100644
--- a/var/spack/repos/builtin/packages/openmpi/package.py
+++ b/var/spack/repos/builtin/packages/openmpi/package.py
@@ -1315,7 +1315,7 @@ class Openmpi(AutotoolsPackage, CudaPackage):
         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):
diff --git a/var/spack/repos/builtin/packages/phist/package.py b/var/spack/repos/builtin/packages/phist/package.py
index 34d704e3ce..1021abc280 100644
--- a/var/spack/repos/builtin/packages/phist/package.py
+++ b/var/spack/repos/builtin/packages/phist/package.py
@@ -325,6 +325,7 @@ class Phist(CMakePackage):
     @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"),
diff --git a/var/spack/repos/builtin/packages/py-genshi/package.py b/var/spack/repos/builtin/packages/py-genshi/package.py
index fba512eca0..d9112609a5 100644
--- a/var/spack/repos/builtin/packages/py-genshi/package.py
+++ b/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")
diff --git a/var/spack/repos/builtin/packages/qscintilla/package.py b/var/spack/repos/builtin/packages/qscintilla/package.py
index 53e8e407f6..b61c9d5295 100644
--- a/var/spack/repos/builtin/packages/qscintilla/package.py
+++ b/var/spack/repos/builtin/packages/qscintilla/package.py
@@ -137,11 +137,12 @@ class Qscintilla(QMakePackage):
             make("install", "-C", "build/")
 
     def test_python_import(self):
-        if "+python" in self.spec:
-            python = self.spec["python"].command
-            if "^py-pyqt5" in self.spec:
-                python("-c", "import PyQt5.Qsci")
-            if "^py-pyqt6" in self.spec:
-                python("-c", "import PyQt6.Qsci")
-        else:
-            print("qscintilla ins't built with python, skipping import test")
+        """check Qsci import"""
+        if self.spec.satisfies("~python"):
+            raise SkipTest("Package must be installed with +python")
+
+        python = self.spec["python"].command
+        if "^py-pyqt5" in self.spec:
+            python("-c", "import PyQt5.Qsci")
+        if "^py-pyqt6" in self.spec:
+            python("-c", "import PyQt6.Qsci")