From 2a5f46d8d33ea42ec506544d7727843073398b11 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Tue, 2 Feb 2021 09:57:09 +0100 Subject: Added a context manager to swap architectures This solves a few FIXMEs in conftest.py, where we were manipulating globals and seeing side effects prior to registering fixtures. This commit solves the FIXMEs, but introduces a performance regression on tests that may need to be investigated (cherry picked from commit 4558dc06e21e01ab07a43737b8cb99d1d69abb5d) --- lib/spack/spack/architecture.py | 58 ++++++++++++++++++++++++++++++++-- lib/spack/spack/test/architecture.py | 35 ++++++++++++-------- lib/spack/spack/test/cmd/extensions.py | 2 +- lib/spack/spack/test/cmd/undevelop.py | 4 +-- lib/spack/spack/test/conftest.py | 25 ++++++--------- 5 files changed, 89 insertions(+), 35 deletions(-) diff --git a/lib/spack/spack/architecture.py b/lib/spack/spack/architecture.py index c0930fe2d0..ece9040a7d 100644 --- a/lib/spack/spack/architecture.py +++ b/lib/spack/spack/architecture.py @@ -56,6 +56,7 @@ set. The user can set the front-end and back-end operating setting by the class attributes front_os and back_os. The operating system as described earlier, will be responsible for compiler detection. """ +import contextlib import functools import inspect import warnings @@ -67,6 +68,8 @@ import llnl.util.tty as tty from llnl.util.lang import memoized, list_modules, key_ordering import spack.compiler +import spack.compilers +import spack.config import spack.paths import spack.error as serr import spack.util.executable @@ -491,7 +494,7 @@ def arch_for_spec(arch_spec): @memoized -def all_platforms(): +def _all_platforms(): classes = [] mod_path = spack.paths.platform_path parent_module = "spack.platforms" @@ -512,7 +515,7 @@ def all_platforms(): @memoized -def platform(): +def _platform(): """Detects the platform for this machine. Gather a list of all available subclasses of platforms. @@ -521,7 +524,7 @@ def platform(): a file path (/opt/cray...) """ # Try to create a Platform object using the config file FIRST - platform_list = all_platforms() + platform_list = _all_platforms() platform_list.sort(key=lambda a: a.priority) for platform_cls in platform_list: @@ -529,6 +532,19 @@ def platform(): return platform_cls() +#: The "real" platform of the host running Spack. This should not be changed +#: by any method and is here as a convenient way to refer to the host platform. +real_platform = _platform + +#: The current platform used by Spack. May be swapped by the use_platform +#: context manager. +platform = _platform + +#: The list of all platform classes. May be swapped by the use_platform +#: context manager. +all_platforms = _all_platforms + + @memoized def default_arch(): """Default ``Arch`` object for this machine. @@ -563,3 +579,39 @@ def compatible_sys_types(): arch = Arch(platform(), 'default_os', target) compatible_archs.append(str(arch)) return compatible_archs + + +class _PickleableCallable(object): + """Class used to pickle a callable that may substitute either + _platform or _all_platforms. Lambda or nested functions are + not pickleable. + """ + def __init__(self, return_value): + self.return_value = return_value + + def __call__(self): + return self.return_value + + +@contextlib.contextmanager +def use_platform(new_platform): + global platform, all_platforms + + msg = '"{0}" must be an instance of Platform' + assert isinstance(new_platform, Platform), msg.format(new_platform) + + original_platform_fn, original_all_platforms_fn = platform, all_platforms + platform = _PickleableCallable(new_platform) + all_platforms = _PickleableCallable([type(new_platform)]) + + # Clear configuration and compiler caches + spack.config.config.clear_caches() + spack.compilers._cache_config_files = [] + + yield new_platform + + platform, all_platforms = original_platform_fn, original_all_platforms_fn + + # Clear configuration and compiler caches + spack.config.config.clear_caches() + spack.compilers._cache_config_files = [] diff --git a/lib/spack/spack/test/architecture.py b/lib/spack/spack/test/architecture.py index 66c87240a1..47e1e25fe8 100644 --- a/lib/spack/spack/test/architecture.py +++ b/lib/spack/spack/test/architecture.py @@ -6,6 +6,7 @@ """ Test checks if the architecture class is created correctly and also that the functions are looking for the correct architecture name """ +import itertools import os import platform as py_platform @@ -116,20 +117,26 @@ def test_user_defaults(config): assert default_target == default_spec.architecture.target -@pytest.mark.parametrize('operating_system', [ - x for x in spack.architecture.platform().operating_sys -] + ["fe", "be", "frontend", "backend"]) -@pytest.mark.parametrize('target', [ - x for x in spack.architecture.platform().targets -] + ["fe", "be", "frontend", "backend"]) -def test_user_input_combination(config, operating_system, target): - platform = spack.architecture.platform() - spec = Spec("libelf os=%s target=%s" % (operating_system, target)) - spec.concretize() - assert spec.architecture.os == str( - platform.operating_system(operating_system) - ) - assert spec.architecture.target == platform.target(target) +def test_user_input_combination(config): + valid_keywords = ["fe", "be", "frontend", "backend"] + + possible_targets = ([x for x in spack.architecture.platform().targets] + + valid_keywords) + + possible_os = ([x for x in spack.architecture.platform().operating_sys] + + valid_keywords) + + for target, operating_system in itertools.product( + possible_targets, possible_os + ): + platform = spack.architecture.platform() + spec_str = "libelf os={0} target={1}".format(operating_system, target) + spec = Spec(spec_str) + spec.concretize() + assert spec.architecture.os == str( + platform.operating_system(operating_system) + ) + assert spec.architecture.target == platform.target(target) def test_operating_system_conversion_to_dict(): diff --git a/lib/spack/spack/test/cmd/extensions.py b/lib/spack/spack/test/cmd/extensions.py index 7fc56593eb..ad993244ae 100644 --- a/lib/spack/spack/test/cmd/extensions.py +++ b/lib/spack/spack/test/cmd/extensions.py @@ -27,7 +27,7 @@ def python_database(mock_packages, mutable_database): @pytest.mark.db -def test_extensions(mock_packages, python_database, capsys): +def test_extensions(mock_packages, python_database, config, capsys): ext2 = Spec("py-extension2").concretized() def check_output(ni, na): diff --git a/lib/spack/spack/test/cmd/undevelop.py b/lib/spack/spack/test/cmd/undevelop.py index d44330fe76..493bb99228 100644 --- a/lib/spack/spack/test/cmd/undevelop.py +++ b/lib/spack/spack/test/cmd/undevelop.py @@ -12,7 +12,7 @@ env = SpackCommand('env') concretize = SpackCommand('concretize') -def test_undevelop(tmpdir, mock_packages, mutable_mock_env_path): +def test_undevelop(tmpdir, config, mock_packages, mutable_mock_env_path): # setup environment envdir = tmpdir.mkdir('env') with envdir.as_cwd(): @@ -39,7 +39,7 @@ env: assert not after.satisfies('dev_path=*') -def test_undevelop_nonexistent(tmpdir, mock_packages, mutable_mock_env_path): +def test_undevelop_nonexistent(tmpdir, config, mock_packages, mutable_mock_env_path): # setup environment envdir = tmpdir.mkdir('env') with envdir.as_cwd(): diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py index 18a7924c9e..297ae0d4f2 100644 --- a/lib/spack/spack/test/conftest.py +++ b/lib/spack/spack/test/conftest.py @@ -1,4 +1,4 @@ -# Copyright 2013-2020 Lawrence Livermore National Security, LLC and other +# Copyright 2013-2021 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) @@ -315,24 +315,18 @@ def _skip_if_missing_executables(request): pytest.skip(msg.format(', '.join(missing_execs))) -# FIXME: The lines below should better be added to a fixture with -# FIXME: session-scope. Anyhow doing it is not easy, as it seems -# FIXME: there's some weird interaction with compilers during concretization. -spack.architecture.real_platform = spack.architecture.platform - - +@pytest.fixture(scope='session') def test_platform(): return spack.platforms.test.Test() -spack.architecture.platform = test_platform - - -# FIXME: Since we change the architecture above, we have to (re)initialize -# FIXME: the config singleton. If it gets initialized too early with the -# FIXME: actual architecture, tests will fail. -spack.config.config = spack.config._config() - +@pytest.fixture(autouse=True, scope='session') +def _use_test_platform(test_platform): + # This is the only context manager used at session scope (see note + # below for more insight) since we want to use the test platform as + # a default during tests. + with spack.architecture.use_platform(test_platform): + yield # # Note on context managers used by fixtures @@ -356,6 +350,7 @@ spack.config.config = spack.config._config() # *USE*, or things can get really confusing. # + # # Test-specific fixtures # -- cgit v1.2.3-70-g09d2