Testing-Builder refactor: move phase_tests to builder.py

lib/spack/spack/build_systems/_checks.py

@@ -112,7 +112,7 @@ def execute_build_time_tests(builder: spack.builder.Builder):
     if not builder.pkg.run_tests or not builder.build_time_test_callbacks:
         return
 
-    builder.pkg.tester.phase_tests(builder, "build", builder.build_time_test_callbacks)
+    builder.phase_tests("build", builder.build_time_test_callbacks)
 
 
 def execute_install_time_tests(builder: spack.builder.Builder):
@@ -125,7 +125,7 @@ def execute_install_time_tests(builder: spack.builder.Builder):
     if not builder.pkg.run_tests or not builder.install_time_test_callbacks:
         return
 
-    builder.pkg.tester.phase_tests(builder, "install", builder.install_time_test_callbacks)
+    builder.phase_tests("install", builder.install_time_test_callbacks)
 
 
 class BuilderWithDefaults(spack.builder.Builder):

lib/spack/spack/builder.py

@@ -8,6 +8,10 @@
 import functools
 from typing import Dict, List, Optional, Tuple, Type
 
+import llnl.util.tty as tty
+import llnl.util.tty.log as log
+
+import spack.config
 import spack.error
 import spack.multimethod
 import spack.package_base
@@ -481,7 +485,7 @@ def __str__(self):
 
 class Builder(BaseBuilder, collections.abc.Sequence):
     """A builder is a class that, given a package object (i.e. associated with concrete spec),
-    knows how to install it.
+    knows how to install it and perform install-time checks.
 
     The builder behaves like a sequence, and when iterated over return the "phases" of the
     installation in the correct order.
@@ -518,3 +522,54 @@ def __getitem__(self, idx):
 
     def __len__(self):
         return len(self.phases)
+
+    def phase_tests(self, phase_name: str, method_names: List[str]):
+        """Execute the package's phase-time tests.
+
+        This process uses the same test setup and logging used for
+        stand-alone tests for consistency.
+
+        Args:
+            phase_name: the name of the build-time phase (e.g., ``build``, ``install``)
+            method_names: phase-specific callback method names
+        """
+        verbose = tty.is_verbose()
+        fail_fast = spack.config.get("config:fail_fast", False)
+
+        tester = self.pkg.tester
+        testsuite = self.pkg.test_suite
+        with tester.test_logger(verbose=verbose, externals=False) as logger:
+            # Report running each of the methods in the build log
+            log.print_message(logger, f"Running {phase_name}-time tests", verbose)
+            testsuite.current_test_spec = self.pkg.spec
+            testsuite.current_base_spec = self.pkg.spec
+
+            have_tests = any(name.startswith("test_") for name in method_names)
+            if have_tests:
+                spack.install_test.copy_test_files(self.pkg, self.pkg.spec)
+
+            for name in method_names:
+                try:
+                    # Prefer the method in the package over the builder's.
+                    # We need this primarily to pick up arbitrarily named test
+                    # methods but also some build-time checks.
+                    fn = getattr(self.pkg, name, getattr(self, name))
+
+                    msg = f"RUN-TESTS: {phase_name}-time tests [{name}]"
+                    log.print_message(logger, msg, verbose)
+
+                    fn()
+
+                except AttributeError as e:
+                    msg = f"RUN-TESTS: method not implemented [{name}]"
+                    log.print_message(logger, msg, verbose)
+
+                    tester.add_failure(e, msg)
+                    if fail_fast:
+                        break
+
+            if have_tests:
+                log.print_message(logger, "Completed testing", verbose)
+
+            # Raise exception if any failures encountered
+            tester.handle_failures()

lib/spack/spack/install_test.py

@@ -335,51 +335,12 @@ def status(self, name: str, status: "TestStatus", msg: Optional[str] = None):
         self.test_parts[part_name] = status
         self.counts[status] += 1
 
-    def phase_tests(self, builder, phase_name: str, method_names: List[str]):
+    def handle_failures(self):
-        """Execute the builder's package phase-time tests.
+        """Raise exception if any failures were collected during testing
 
-        Args:
+        Raises:
-            builder: builder for package being tested
+            TestFailure: test failures were collected
-            phase_name: the name of the build-time phase (e.g., ``build``, ``install``)
-            method_names: phase-specific callback method names
         """
-        verbose = tty.is_verbose()
-        fail_fast = spack.config.get("config:fail_fast", False)
-
-        with self.test_logger(verbose=verbose, externals=False) as logger:
-            # Report running each of the methods in the build log
-            log.print_message(logger, f"Running {phase_name}-time tests", verbose)
-            builder.pkg.test_suite.current_test_spec = builder.pkg.spec
-            builder.pkg.test_suite.current_base_spec = builder.pkg.spec
-
-            have_tests = any(name.startswith("test_") for name in method_names)
-            if have_tests:
-                copy_test_files(builder.pkg, builder.pkg.spec)
-
-            for name in method_names:
-                try:
-                    # Prefer the method in the package over the builder's.
-                    # We need this primarily to pick up arbitrarily named test
-                    # methods but also some build-time checks.
-                    fn = getattr(builder.pkg, name, getattr(builder, name))
-
-                    msg = f"RUN-TESTS: {phase_name}-time tests [{name}]"
-                    log.print_message(logger, msg, verbose)
-
-                    fn()
-
-                except AttributeError as e:
-                    msg = f"RUN-TESTS: method not implemented [{name}]"
-                    log.print_message(logger, msg, verbose)
-
-                    self.add_failure(e, msg)
-                    if fail_fast:
-                        break
-
-            if have_tests:
-                log.print_message(logger, "Completed testing", verbose)
-
-            # Raise any collected failures here
         if self.test_failures:
             raise TestFailure(self.test_failures)
 
@@ -683,10 +644,9 @@ def process_test_parts(pkg: Pb, test_specs: List[spack.spec.Spec], verbose: bool
                 ):
                     test_fn(pkg)
 
-        # If fail-fast was on, we error out above
+        # If fail-fast was on, we errored out above
-        # If we collect errors, raise them in batch here
+        # If we collected errors, raise them in batch here
-        if tester.test_failures:
+        tester.handle_failures()
-            raise TestFailure(tester.test_failures)
 
     finally:
         if tester.ran_tests():