From 1ed564a1e623812c3902d8c592e00e5a3367bcb3 Mon Sep 17 00:00:00 2001 From: Peter Scheibel Date: Thu, 7 May 2020 14:01:58 -0700 Subject: Testing: fix unintended interactions between tests, part 2 (#16429) This fixes some errors with setting up test configuration. These errors do not cause current Spack tests to fail but do create red herring issues elsewhere (see #15666). Fixing these errors leads to more errors in tests that depended on the original misconfigured state, so those are also addressed here. This is an update to #16003 which accounts for some unit tests with conflicting config/mutable_config fixtures. These conflicts were not exposed until the mutable_config fixture was fixed. Details are included below. The change which builds on #16003 is prefixed with "(new)". * For tests that use the real Spack package repository, the config needs to avoid using MPI providers that are not intended to be installed by Spack. Without this, it is possible that Spack tests which concretize the MPI virtual will end up trying to use an implementation that it shouldn't (e.g. one that is always provided externally). See #15666 for an example. * The mutable_config test fixture was not initializing the scope roots to the right directories (so the resulting config was empty). * The current_host fixture in the concretize.py tests was using the config fixture rather than mutable_config, and was polluting the config cache for other tests. * One test in concretize.py was clearing a nonexistent cache (PackagePrefs._packages_config_cache). This reference has been removed. * The test 'test_preferred_compilers' was was depending on cross test config pollution to succeed. The initial spec before concretization has been updated to updated to be explicit about the desired result. * (new) For tests that use install_mockery and mutable_config, replace install_mockery with a separate install_mockery_mutable_config fixture that is exactly the same as install_mockery but uses the mutable_config fixture to avoid conflicts. --- lib/spack/spack/cmd/compiler.py | 14 +++++++++++++- lib/spack/spack/test/cmd/install.py | 8 ++++---- lib/spack/spack/test/concretize.py | 8 ++++---- lib/spack/spack/test/concretize_preferences.py | 2 +- lib/spack/spack/test/conftest.py | 24 ++++++++++++++++++++++-- lib/spack/spack/test/data/config/packages.yaml | 3 +++ 6 files changed, 47 insertions(+), 12 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/cmd/compiler.py b/lib/spack/spack/cmd/compiler.py index 625466f8c7..c8f52af04d 100644 --- a/lib/spack/spack/cmd/compiler.py +++ b/lib/spack/spack/cmd/compiler.py @@ -159,7 +159,19 @@ def compiler_list(args): tty.msg("Available compilers") index = index_by(spack.compilers.all_compilers(scope=args.scope), lambda c: (c.spec.name, c.operating_system, c.target)) - ordered_sections = sorted(index.items(), key=lambda item: item[0]) + + # For a container, take each element which does not evaluate to false and + # convert it to a string. For elements which evaluate to False (e.g. None) + # convert them to '' (in which case it still evaluates to False but is a + # string type). Tuples produced by this are guaranteed to be comparable in + # Python 3 + convert_str = ( + lambda tuple_container: + tuple(str(x) if x else '' for x in tuple_container)) + + index_str_keys = list( + (convert_str(x), y) for x, y in index.items()) + ordered_sections = sorted(index_str_keys, key=lambda item: item[0]) for i, (key, compilers) in enumerate(ordered_sections): if i >= 1: print() diff --git a/lib/spack/spack/test/cmd/install.py b/lib/spack/spack/test/cmd/install.py index dd52cea406..4a5db55c01 100644 --- a/lib/spack/spack/test/cmd/install.py +++ b/lib/spack/spack/test/cmd/install.py @@ -722,8 +722,8 @@ def test_cdash_auth_token(tmpdir, install_mockery, capfd): def test_compiler_bootstrap( - install_mockery, mock_packages, mock_fetch, mock_archive, - mutable_config, monkeypatch): + install_mockery_mutable_config, mock_packages, mock_fetch, + mock_archive, mutable_config, monkeypatch): monkeypatch.setattr(spack.concretize.Concretizer, 'check_for_compiler_existence', False) spack.config.set('config:install_missing_compilers', True) @@ -735,8 +735,8 @@ def test_compiler_bootstrap( @pytest.mark.regression('16221') def test_compiler_bootstrap_already_installed( - install_mockery, mock_packages, mock_fetch, mock_archive, - mutable_config, monkeypatch): + install_mockery_mutable_config, mock_packages, mock_fetch, + mock_archive, mutable_config, monkeypatch): monkeypatch.setattr(spack.concretize.Concretizer, 'check_for_compiler_existence', False) spack.config.set('config:install_missing_compilers', True) diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index 232dfaeb4b..ec594e4088 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -12,7 +12,6 @@ import spack.repo from spack.concretize import find_spec, NoValidVersionError from spack.error import SpecError -from spack.package_prefs import PackagePrefs from spack.spec import Spec, CompilerSpec, ConflictsInSpecError from spack.version import ver from spack.util.mock_package import MockPackageMultiRepo @@ -103,8 +102,6 @@ def current_host(request, monkeypatch): monkeypatch.setattr(spack.platforms.test.Test, 'default', cpu) yield target else: - # There's a cache that needs to be cleared for unit tests - PackagePrefs._packages_config_cache = None with spack.config.override('packages:all', {'target': [cpu]}): yield target @@ -112,7 +109,10 @@ def current_host(request, monkeypatch): spack.architecture.get_platform.cache.clear() -@pytest.mark.usefixtures('config', 'mock_packages') +# This must use the mutable_config fixture because the test +# adjusting_default_target_based_on_compiler uses the current_host fixture, +# which changes the config. +@pytest.mark.usefixtures('mutable_config', 'mock_packages') class TestConcretize(object): def test_concretize(self, spec): check_concretize(spec) diff --git a/lib/spack/spack/test/concretize_preferences.py b/lib/spack/spack/test/concretize_preferences.py index 922d5a11d8..ca4df6700c 100644 --- a/lib/spack/spack/test/concretize_preferences.py +++ b/lib/spack/spack/test/concretize_preferences.py @@ -100,7 +100,7 @@ class TestConcretizePreferences(object): # Try the last available compiler compiler = str(compiler_list[-1]) update_packages('mpileaks', 'compiler', [compiler]) - spec = concretize('mpileaks') + spec = concretize('mpileaks os=redhat6 target=x86') assert spec.compiler == spack.spec.CompilerSpec(compiler) def test_preferred_target(self, mutable_mock_repo): diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py index 736c5a5563..ae8fa80ab7 100644 --- a/lib/spack/spack/test/conftest.py +++ b/lib/spack/spack/test/conftest.py @@ -423,13 +423,13 @@ def config(mock_configuration): @pytest.fixture(scope='function') -def mutable_config(tmpdir_factory, configuration_dir, monkeypatch): +def mutable_config(tmpdir_factory, configuration_dir): """Like config, but tests can modify the configuration.""" mutable_dir = tmpdir_factory.mktemp('mutable_config').join('tmp') configuration_dir.copy(mutable_dir) cfg = spack.config.Configuration( - *[spack.config.ConfigScope(name, str(mutable_dir)) + *[spack.config.ConfigScope(name, str(mutable_dir.join(name))) for name in ['site', 'system', 'user']]) with use_configuration(cfg): @@ -602,6 +602,26 @@ def install_mockery(tmpdir, config, mock_packages, monkeypatch): spack.store.store = real_store +@pytest.fixture(scope='function') +def install_mockery_mutable_config( + tmpdir, mutable_config, mock_packages, monkeypatch): + """Hooks a fake install directory, DB, and stage directory into Spack. + + This is specifically for tests which want to use 'install_mockery' but + also need to modify configuration (and hence would want to use + 'mutable config'): 'install_mockery' does not support this. + """ + real_store = spack.store.store + spack.store.store = spack.store.Store(str(tmpdir.join('opt'))) + + # We use a fake package, so temporarily disable checksumming + with spack.config.override('config:checksum', False): + yield + + tmpdir.join('opt').remove() + spack.store.store = real_store + + @pytest.fixture() def mock_fetch(mock_archive): """Fake the URL for a package so it downloads from a file.""" diff --git a/lib/spack/spack/test/data/config/packages.yaml b/lib/spack/spack/test/data/config/packages.yaml index c7256ddb33..63e63e525d 100644 --- a/lib/spack/spack/test/data/config/packages.yaml +++ b/lib/spack/spack/test/data/config/packages.yaml @@ -1,4 +1,7 @@ packages: + all: + providers: + mpi: [openmpi, mpich] externaltool: buildable: False paths: -- cgit v1.2.3-70-g09d2