From 9a604bf61537b3c81201a32de1e8861a1207e839 Mon Sep 17 00:00:00 2001 From: Tamara Dahlgren <35777542+tldahlgren@users.noreply.github.com> Date: Fri, 15 Nov 2024 17:26:26 -0800 Subject: [PATCH] Add set_current_specs to TestSuite and PackageTest These changes allow removing references to test_suite from builder.py, deferring them to PackageTest, which ensures the test_suite is set for the builder's phase tests. Also added comments to explain the need for test-related attributes to the builder and package_base. --- lib/spack/spack/builder.py | 9 ++++++--- lib/spack/spack/install_test.py | 29 ++++++++++++++++++++--------- lib/spack/spack/package_base.py | 2 +- 3 files changed, 27 insertions(+), 13 deletions(-) diff --git a/lib/spack/spack/builder.py b/lib/spack/spack/builder.py index 82030831ef0..30cf3102019 100644 --- a/lib/spack/spack/builder.py +++ b/lib/spack/spack/builder.py @@ -126,11 +126,16 @@ def __init__(self, wrapped_pkg_object, root_builder): new_cls_name, bases, { + # boolean to indicate whether install-time tests are run "run_tests": property(lambda x: x.wrapped_package_object.run_tests), + # boolean to indicate whether the package's stand-alone tests + # require a compiler "test_requires_compiler": property( lambda x: x.wrapped_package_object.test_requires_compiler ), + # TestSuite instance the spec is a part of "test_suite": property(lambda x: x.wrapped_package_object.test_suite), + # PackageTest instance to manage the spec's testing "tester": property(lambda x: x.wrapped_package_object.tester), }, ) @@ -537,12 +542,10 @@ def phase_tests(self, phase_name: str, method_names: List[str]): 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 + tester.set_current_specs(self.pkg.spec, self.pkg.spec) have_tests = any(name.startswith("test_") for name in method_names) if have_tests: diff --git a/lib/spack/spack/install_test.py b/lib/spack/spack/install_test.py index 1b1d6796483..3c91f12f200 100644 --- a/lib/spack/spack/install_test.py +++ b/lib/spack/spack/install_test.py @@ -252,15 +252,16 @@ def __init__(self, pkg: Pb): self.test_log_file: str self.pkg_id: str - if pkg.test_suite: + if self.pkg.test_suite is not None: # Running stand-alone tests - self.test_log_file = pkg.test_suite.log_file_for_spec(pkg.spec) - self.tested_file = pkg.test_suite.tested_file_for_spec(pkg.spec) - self.pkg_id = pkg.test_suite.test_pkg_id(pkg.spec) + suite = self.pkg.test_suite + self.test_log_file = suite.log_file_for_spec(pkg.spec) # type: ignore[union-attr] + self.tested_file = suite.tested_file_for_spec(pkg.spec) # type: ignore[union-attr] + self.pkg_id = suite.test_pkg_id(pkg.spec) # type: ignore[union-attr] else: # Running phase-time tests for a single package whose results are # retained in the package's stage directory. - pkg.test_suite = TestSuite([pkg.spec]) + self.pkg.test_suite = TestSuite([pkg.spec]) self.test_log_file = fs.join_path(pkg.stage.path, spack_install_test_log) self.pkg_id = pkg.spec.format("{name}-{version}-{hash:7}") @@ -314,6 +315,13 @@ def add_failure(self, exception: Exception, msg: str): """Add the failure details to the current list.""" self.test_failures.append((exception, msg)) + def set_current_specs(self, base_spec: spack.spec.Spec, test_spec: spack.spec.Spec): + # Ignore union-attr check for test_suite since the constructor of this + # class ensures it is always not None. + test_suite = self.pkg.test_suite + test_suite.current_base_spec = base_spec # type: ignore[union-attr] + test_suite.current_test_spec = test_spec # type: ignore[union-attr] + def status(self, name: str, status: "TestStatus", msg: Optional[str] = None): """Track and print the test status for the test part name.""" part_name = f"{self.pkg.__class__.__name__}::{name}" @@ -826,9 +834,14 @@ def content_hash(self): self._hash = b32_hash return self._hash + # TODO/TLD: Finish this def _run_test(self, pkg, dirty, externals): pkg.do_test(dirty=dirty, externals=externals) + def set_current_specs(self, base_spec: spack.spec.Spec, test_spec: spack.spec.Spec): + self.current_base_spec = base_spec + self.current_test_spec = test_spec + def __call__(self, *args, **kwargs): self.write_reproducibility_data() @@ -848,8 +861,7 @@ def __call__(self, *args, **kwargs): # Set up the test suite to know which test is running spec.package.test_suite = self - self.current_base_spec = spec - self.current_test_spec = spec + self.set_current_specs(spec, spec) # setup per-test directory in the stage dir test_dir = self.test_dir_for_spec(spec) @@ -893,8 +905,7 @@ def __call__(self, *args, **kwargs): finally: spec.package.test_suite = None - self.current_test_spec = None - self.current_base_spec = None + self.set_current_specs(None, None) write_test_summary(self.counts) diff --git a/lib/spack/spack/package_base.py b/lib/spack/spack/package_base.py index 6ebc948ca46..be82e4758d0 100644 --- a/lib/spack/spack/package_base.py +++ b/lib/spack/spack/package_base.py @@ -712,7 +712,7 @@ class PackageBase(WindowsRPath, PackageViewMixin, metaclass=PackageMeta): #: are available to build a custom test code. test_requires_compiler: bool = False - #: TestSuite instance used to manage stand-alone tests for 1+ specs. + #: The spec's TestSuite instance, which is used to manage its testing. test_suite: Optional[Any] = None def __init__(self, spec):