Refactor unit-tests in test/architecture.py (#25848)

Modifications:
- Export platforms from spack.platforms directly, so that client modules don't have to import submodules
- Use only plain imports in test/architecture.py
- Parametrized test in test/architecture.py and put most of the setup/teardown in fixtures

5 files changed, 101 insertions, 121 deletions

diff --git a/lib/spack/spack/bootstrap.py b/lib/spack/spack/bootstrap.py
index 34e5151b78..efcb8a7e19 100644
--- a/lib/spack/spack/bootstrap.py
+++ b/lib/spack/spack/bootstrap.py
@@ -30,6 +30,7 @@ import spack.environment
 import spack.main
 import spack.modules
 import spack.paths
+import spack.platforms
 import spack.repo
 import spack.spec
 import spack.store
@@ -178,9 +179,6 @@ class _BuildcacheBootstrapper(object):
         self.url = conf['info']['url']
 
     def try_import(self, module, abstract_spec_str):
-        # This import is local since it is needed only on Cray
-        import spack.platforms.linux
-
         if _try_import_from_store(module, abstract_spec_str):
             return True
 
@@ -192,7 +190,7 @@ class _BuildcacheBootstrapper(object):
         # On Cray we want to use Linux binaries if available from mirrors
         bincache_platform = spack.architecture.real_platform()
         if str(bincache_platform) == 'cray':
-            bincache_platform = spack.platforms.linux.Linux()
+            bincache_platform = spack.platforms.Linux()
             with spack.architecture.use_platform(bincache_platform):
                 abstract_spec = spack.spec.Spec(
                     abstract_spec_str + ' ^' + spec_for_current_python()
diff --git a/lib/spack/spack/platforms/__init__.py b/lib/spack/spack/platforms/__init__.py
index 103eae6134..f401ad0e11 100644
--- a/lib/spack/spack/platforms/__init__.py
+++ b/lib/spack/spack/platforms/__init__.py
@@ -2,3 +2,14 @@
 # Spack Project Developers. See the top-level COPYRIGHT file for details.
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
+from .cray import Cray
+from .darwin import Darwin
+from .linux import Linux
+from .test import Test
+
+__all__ = [
+    'Cray',
+    'Darwin',
+    'Linux',
+    'Test'
+]
diff --git a/lib/spack/spack/test/architecture.py b/lib/spack/spack/test/architecture.py
index 12e70cff47..bc12e3f857 100644
--- a/lib/spack/spack/test/architecture.py
+++ b/lib/spack/spack/test/architecture.py
@@ -2,141 +2,112 @@
 # Spack Project Developers. See the top-level COPYRIGHT file for details.
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
-
-""" 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
+import platform
 
 import pytest
 
 import spack.architecture
 import spack.concretize
-from spack.platforms.cray import Cray
-from spack.platforms.darwin import Darwin
-from spack.platforms.linux import Linux
-from spack.spec import Spec
+import spack.platforms
+import spack.spec
 
 
-def test_dict_functions_for_architecture():
+@pytest.fixture
+def sample_arch():
+    """Sample test architecture"""
     arch = spack.architecture.Arch()
     arch.platform = spack.architecture.platform()
     arch.os = arch.platform.operating_system('default_os')
     arch.target = arch.platform.target('default_target')
-
-    new_arch = spack.architecture.Arch.from_dict(arch.to_dict())
-
-    assert arch == new_arch
-    assert isinstance(arch, spack.architecture.Arch)
-    assert isinstance(arch.platform, spack.architecture.Platform)
-    assert isinstance(arch.os, spack.architecture.OperatingSystem)
-    assert isinstance(arch.target, spack.architecture.Target)
-    assert isinstance(new_arch, spack.architecture.Arch)
-    assert isinstance(new_arch.platform, spack.architecture.Platform)
-    assert isinstance(new_arch.os, spack.architecture.OperatingSystem)
-    assert isinstance(new_arch.target, spack.architecture.Target)
+    return arch
 
 
-def test_platform():
-    output_platform_class = spack.architecture.real_platform()
+@pytest.fixture
+def current_host_platform():
+    """Return the platform of the current host as detected by the
+    'platform' stdlib package.
+    """
     if os.path.exists('/opt/cray/pe'):
-        my_platform_class = Cray()
-    elif 'Linux' in py_platform.system():
-        my_platform_class = Linux()
-    elif 'Darwin' in py_platform.system():
-        my_platform_class = Darwin()
-
-    assert str(output_platform_class) == str(my_platform_class)
+        current_platform = spack.platforms.Cray()
+    elif 'Linux' in platform.system():
+        current_platform = spack.platforms.Linux()
+    elif 'Darwin' in platform.system():
+        current_platform = spack.platforms.Darwin()
+    return current_platform
 
 
-def test_boolness():
-    # Make sure architecture reports that it's False when nothing's set.
-    arch = spack.architecture.Arch()
-    assert not arch
+# Valid keywords for os=xxx or target=xxx
+valid_keywords = ["fe", "be", "frontend", "backend"]
 
-    # Dummy architecture parts
-    plat = spack.architecture.platform()
-    plat_os = plat.operating_system('default_os')
-    plat_target = plat.target('default_target')
 
-    # Make sure architecture reports that it's True when anything is set.
-    arch = spack.architecture.Arch()
-    arch.platform = plat
-    assert arch
+@pytest.fixture(
+    params=([x for x in spack.platforms.Test().targets]
+            + valid_keywords + ['default_target'])
+)
+def target_str(request):
+    """All the possible strings that can be used for targets"""
+    return str(request.param)
 
-    arch = spack.architecture.Arch()
-    arch.os = plat_os
-    assert arch
 
-    arch = spack.architecture.Arch()
-    arch.target = plat_target
-    assert arch
+@pytest.fixture(
+    params=([x for x in spack.platforms.Test().operating_sys]
+            + valid_keywords + ['default_os'])
+)
+def os_str(request):
+    """All the possible strings that can be used for operating systems"""
+    return str(request.param)
 
 
-def test_user_front_end_input(config):
-    """Test when user inputs just frontend that both the frontend target
-    and frontend operating system match
-    """
-    platform = spack.architecture.platform()
-    frontend_os = str(platform.operating_system('frontend'))
-    frontend_target = platform.target('frontend')
+def test_dict_round_trip(sample_arch):
+    """Check that a round trip through dict return an equivalent architecture"""
+    sample_arch_copy = spack.architecture.Arch.from_dict(sample_arch.to_dict())
 
-    frontend_spec = Spec('libelf os=frontend target=frontend')
-    frontend_spec.concretize()
+    assert sample_arch == sample_arch_copy
+    for current_arch in (sample_arch, sample_arch_copy):
+        assert isinstance(current_arch, spack.architecture.Arch)
+        assert isinstance(current_arch.platform, spack.architecture.Platform)
+        assert isinstance(current_arch.os, spack.architecture.OperatingSystem)
+        assert isinstance(current_arch.target, spack.architecture.Target)
 
-    assert frontend_os == frontend_spec.architecture.os
-    assert frontend_target == frontend_spec.architecture.target
 
+def test_platform(current_host_platform):
+    """Check that current host detection return the correct platform"""
+    detected_platform = spack.architecture.real_platform()
+    assert str(detected_platform) == str(current_host_platform)
 
-def test_user_back_end_input(config):
-    """Test when user inputs backend that both the backend target and
-    backend operating system match
-    """
-    platform = spack.architecture.platform()
-    backend_os = str(platform.operating_system("backend"))
-    backend_target = platform.target("backend")
-
-    backend_spec = Spec("libelf os=backend target=backend")
-    backend_spec.concretize()
-
-    assert backend_os == backend_spec.architecture.os
-    assert backend_target == backend_spec.architecture.target
-
-
-def test_user_defaults(config):
-    platform = spack.architecture.platform()
-    default_os = str(platform.operating_system("default_os"))
-    default_target = platform.target("default_target")
-
-    default_spec = Spec("libelf")  # default is no args
-    default_spec.concretize()
-
-    assert default_os == default_spec.architecture.os
-    assert default_target == default_spec.architecture.target
 
+@pytest.mark.parametrize('attribute_name,expected', [
+    # Make sure architecture reports that it's False when nothing's set.
+    (None, False),
+    # ... and it reports True otherwise
+    ('platform', True),
+    ('os', True),
+    ('target', True)
+])
+def test_boolness(sample_arch, attribute_name, expected):
+    """Boolean checks on an architecture object"""
+    arch = spack.architecture.Arch()
+    if attribute_name:
+        setattr(arch, attribute_name, getattr(sample_arch, attribute_name))
 
-def test_user_input_combination(config):
-    valid_keywords = ["fe", "be", "frontend", "backend"]
+    assert bool(arch) is expected
 
-    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)
+def test_user_input_combination(config, target_str, os_str):
+    """Test for all the valid user input combinations that both the target and
+    the operating system match.
+    """
+    platform = spack.platforms.Test()
+    spec_str = "libelf"
+    if os_str != "default_os":
+        spec_str += " os={0}".format(os_str)
+    if target_str != "default_target":
+        spec_str += " target={0}".format(target_str)
+    spec = spack.spec.Spec(spec_str).concretized()
 
-    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)
+    assert spec.architecture.os == str(platform.operating_system(os_str))
+    assert spec.architecture.target == platform.target(target_str)
 
 
 def test_operating_system_conversion_to_dict():
@@ -234,16 +205,15 @@ def test_satisfy_strict_constraint_when_not_concrete(
 
 
 @pytest.mark.parametrize('root_target_range,dep_target_range,result', [
-    (('x86_64:nocona', 'x86_64:core2', 'nocona')),  # pref not in intersection
-    (('x86_64:core2', 'x86_64:nocona', 'nocona')),
-    (('x86_64:haswell', 'x86_64:mic_knl', 'core2')),  # pref in intersection
-    (('ivybridge', 'nocona:skylake', 'ivybridge')),  # one side concrete
-    (('haswell:icelake', 'broadwell', 'broadwell')),
+    ('x86_64:nocona', 'x86_64:core2', 'nocona'),  # pref not in intersection
+    ('x86_64:core2', 'x86_64:nocona', 'nocona'),
+    ('x86_64:haswell', 'x86_64:mic_knl', 'core2'),  # pref in intersection
+    ('ivybridge', 'nocona:skylake', 'ivybridge'),  # one side concrete
+    ('haswell:icelake', 'broadwell', 'broadwell'),
     # multiple ranges in lists with multiple overlaps
-    (('x86_64:nocona,haswell:broadwell', 'nocona:haswell,skylake:',
-      'nocona')),
+    ('x86_64:nocona,haswell:broadwell', 'nocona:haswell,skylake:', 'nocona'),
     # lists with concrete targets, lists compared to ranges
-    (('x86_64,haswell', 'core2:broadwell', 'haswell'))
+    ('x86_64,haswell', 'core2:broadwell', 'haswell')
 ])
 @pytest.mark.usefixtures('mock_packages', 'config')
 def test_concretize_target_ranges(
@@ -251,8 +221,9 @@ def test_concretize_target_ranges(
 ):
     # use foobar=bar to make the problem simpler for the old concretizer
     # the new concretizer should not need that help
-    spec = Spec('a %%gcc@10 foobar=bar target=%s ^b target=%s' %
+    spec_str = ('a %%gcc@10 foobar=bar target=%s ^b target=%s' %
                 (root_target_range, dep_target_range))
+    spec = spack.spec.Spec(spec_str)
     with spack.concretize.disable_compiler_existence_check():
         spec.concretize()
 
diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py
index 0becb4d55b..b86b18392a 100644
--- a/lib/spack/spack/test/concretize.py
+++ b/lib/spack/spack/test/concretize.py
@@ -15,7 +15,7 @@ import spack.architecture
 import spack.compilers
 import spack.concretize
 import spack.error
-import spack.platforms.test
+import spack.platforms
 import spack.repo
 from spack.concretize import find_spec
 from spack.spec import Spec
@@ -101,8 +101,8 @@ def current_host(request, monkeypatch):
     # this function is memoized, so clear its state for testing
     spack.architecture.get_platform.cache.clear()
 
-    monkeypatch.setattr(spack.platforms.test.Test, 'default', cpu)
-    monkeypatch.setattr(spack.platforms.test.Test, 'front_end', cpu)
+    monkeypatch.setattr(spack.platforms.Test, 'default', cpu)
+    monkeypatch.setattr(spack.platforms.Test, 'front_end', cpu)
     if not is_preference:
         monkeypatch.setattr(archspec.cpu, 'host', lambda: target)
         yield target
diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py
index f68dafa393..9e44547ce1 100644
--- a/lib/spack/spack/test/conftest.py
+++ b/lib/spack/spack/test/conftest.py
@@ -35,7 +35,7 @@ import spack.environment as ev
 import spack.package
 import spack.package_prefs
 import spack.paths
-import spack.platforms.test
+import spack.platforms
 import spack.repo
 import spack.stage
 import spack.store
@@ -345,7 +345,7 @@ def _skip_if_missing_executables(request):
 
 @pytest.fixture(scope='session')
 def test_platform():
-    return spack.platforms.test.Test()
+    return spack.platforms.Test()
 
 
 @pytest.fixture(autouse=True, scope='session')