Test package detection in a systematic way

lib/spack/docs/developer_guide.rst

@@ -349,9 +349,119 @@ make sure to update Spack's `Bash tab completion script
 Unit tests
 ----------
 
-------------
-Unit testing
-------------
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+Add detection tests for packages
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+To ensure that a software is detected correctly for multiple configurations
+and on different systems developers can define a dictionary-variable called
+``detection_tests`` in the same ``package.py`` file where the package recipe
+resides. The data in this variable is used by a parametrized unit test:
+
+.. code-block:: console
+
+   % spack unit-test lib/spack/spack/test/cmd/external.py::test_package_detection
+   =========================================================================== test session starts ============================================================================
+   platform darwin -- Python 3.7.3, pytest-3.2.5, py-1.4.34, pluggy-0.4.0
+   rootdir: /Users/culpo/PycharmProjects/spack, inifile: pytest.ini
+   collected 3 items
+
+   lib/spack/spack/test/cmd/external.py ...
+
+   ======================================================================== slowest 20 test durations =========================================================================
+   2.03s call     lib/spack/spack/test/cmd/external.py::test_package_detection[gcc]
+   1.90s call     lib/spack/spack/test/cmd/external.py::test_package_detection[llvm]
+   0.86s call     lib/spack/spack/test/cmd/external.py::test_package_detection[intel]
+   0.23s setup    lib/spack/spack/test/cmd/external.py::test_package_detection[gcc]
+   0.00s setup    lib/spack/spack/test/cmd/external.py::test_package_detection[llvm]
+   0.00s setup    lib/spack/spack/test/cmd/external.py::test_package_detection[intel]
+   0.00s teardown lib/spack/spack/test/cmd/external.py::test_package_detection[intel]
+   0.00s teardown lib/spack/spack/test/cmd/external.py::test_package_detection[gcc]
+   0.00s teardown lib/spack/spack/test/cmd/external.py::test_package_detection[llvm]
+   ========================================================================= 3 passed in 5.05 seconds =========================================================================
+
+that mock an environment and try to check if the detection logic yields theresults that are expected.
+
+As a general rule, attributes at the top-level of ``detection_tests``
+represent search mechanisms and they all map to a list of tests that should confirm
+the validity of each package detection logic.
+
+""""""""""""""""""""""""""
+Tests for PATH inspections
+""""""""""""""""""""""""""
+
+Detection tests insisting on ``PATH`` inspections are listed under
+the ``paths`` attribute:
+
+.. code-block:: python
+
+   detection_tests = {
+       'paths': [
+           {
+               'layout': [
+                   {
+                       'subdir': ['bin'], 'name': 'clang-3.9',
+                       'output': """
+   echo "clang version 3.9.1-19ubuntu1 (tags/RELEASE_391/rc2)"
+   echo "Target: x86_64-pc-linux-gnu"
+   echo "Thread model: posix"
+   echo "InstalledDir: /usr/bin"
+   """
+                   },
+                   {
+                       'subdir': ['bin'], 'name': 'clang++-3.9',
+                       'output': """
+   echo "clang version 3.9.1-19ubuntu1 (tags/RELEASE_391/rc2)"
+   echo "Target: x86_64-pc-linux-gnu"
+   echo "Thread model: posix"
+   echo "InstalledDir: /usr/bin"
+   """,
+                   }
+               ],
+               'results': [
+                   {'spec': 'llvm@3.9.1 +clang~lld~lldb'}
+               ]
+           },
+       ]
+   }
+
+Each test is performed by first creating a temporary directory structure as
+specified in the corresponding ``layout`` and by then running
+package detection and checking that the outcome matches the expected
+``results``. The exact details on how to specify both the ``layout`` and the
+``results`` are reported in the table below:
+
+.. list-table:: Test based on PATH inspections
+   :header-rows: 1
+
+   * - Option Name
+     - Description
+     - Allowed Values
+     - Required
+   * - ``layout``
+     - Specifies the filesystem tree used for the test
+     - List of objects
+     - Yes
+   * - ``layout:[0]:subdir``
+     - Subdirectory for this executable
+     - List of strings
+     - Yes
+   * - ``layout:[0]:name``
+     - Name of the executable
+     - A valid filename
+     - Yes
+   * - ``layout:[0]:output``
+     - Mock logic for the executable
+     - Any valid shell script
+     - Yes
+   * - ``results``
+     - List of expected results
+     - List of objects (empty if no result is expected)
+     - Yes
+   * - ``results:[0]:spec``
+     - A spec that is expected from detection
+     - Any valid spec
+     - Yes
 
 ------------------
 Developer commands

lib/spack/spack/cmd/external.py

@@ -213,12 +213,16 @@ def _convert_to_iterable(single_val_or_multiple):
 
 
 def _determine_base_dir(prefix):
-    # Given a prefix where an executable is found, assuming that prefix ends
-    # with /bin/, strip off the 'bin' directory to get a Spack-compatible
-    # prefix
+    # Given a prefix where an executable is found, assuming that prefix
+    # contains /bin/, strip off the 'bin' directory and all subsequent
+    # directories to get a Spack-compatible prefix
     assert os.path.isdir(prefix)
-    if os.path.basename(prefix) == 'bin':
-        return os.path.dirname(prefix)
+
+    components = prefix.split(os.sep)
+    if 'bin' not in components:
+        return None
+    idx = components.index('bin')
+    return os.sep.join(components[:idx])
 
 
 def _get_predefined_externals():

lib/spack/spack/test/cmd/external.py

@@ -4,10 +4,12 @@
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 import pytest
 
+import contextlib
 import os
 import os.path
 
 import spack
+import spack.paths
 from spack.spec import Spec
 from spack.cmd.external import ExternalPackageEntry
 from spack.main import SpackCommand
@@ -267,3 +269,67 @@ def test_use_tags_for_detection(
     assert 'The following specs have been' in output
     assert 'cmake' in output
     assert 'openssl' not in output
+
+
+def candidate_packages():
+    """Return the list of packages with a corresponding
+    detection_test.yaml file.
+    """
+    # Directory where we store all the data to setup unit tests for detection.
+    data_dir = os.path.join(spack.paths.test_path, 'data', 'detection')
+    to_be_tested = [os.path.basename(x).replace('.yaml', '')
+                    for x in os.listdir(data_dir)]
+    return to_be_tested
+
+
+@pytest.mark.detection
+@pytest.mark.parametrize('package_name', [
+    'gcc', 'intel', 'llvm'
+])
+def test_package_detection(mock_executable, package_name):
+    def detection_tests_for(pkg):
+        module = spack.repo.path.repo_for_pkg(pkg)._get_pkg_module(pkg)
+        return module.detection_tests
+
+    @contextlib.contextmanager
+    def setup_test_layout(layout):
+        exes_by_path, to_be_removed = {}, []
+        for binary in layout:
+            exe = mock_executable(
+                binary['name'], binary['output'], subdir=binary['subdir']
+            )
+            to_be_removed.append(exe)
+            exes_by_path[str(exe)] = os.path.basename(str(exe))
+
+        yield exes_by_path
+
+        for exe in to_be_removed:
+            os.unlink(exe)
+
+    # Retrieve detection test data for this package and cycle over each
+    # of the scenarios that are encoded
+    detection_tests = detection_tests_for(package_name)
+    if 'paths' not in detection_tests:
+        msg = 'Package "{0}" has no detection tests based on PATH'
+        pytest.skip(msg.format(package_name))
+
+    for test in detection_tests['paths']:
+        # Setup the mock layout for detection. The context manager will
+        # remove mock files when it's finished.
+        with setup_test_layout(test['layout']) as abs_path_to_exe:
+            entries = spack.cmd.external._get_external_packages(
+                [spack.repo.get(package_name)], abs_path_to_exe
+            )
+            specs = set(x.spec for x in entries[package_name])
+            results = test['results']
+            # If no result was expected, check that nothing was detected
+            if not results:
+                msg = 'No spec was expected [detected={0}]'
+                assert not specs, msg.format(sorted(specs))
+                continue
+
+            # If we expected results check that all of the expected
+            # specs were detected.
+            for result in results:
+                spec, msg = result['spec'], 'Not able to detect "{0}"'
+                assert spack.spec.Spec(spec) in specs, msg.format(str(spec))

var/spack/repos/builtin/packages/gcc/package.py

@@ -649,3 +649,56 @@ def setup_run_environment(self, env):
                 env.set(lang.upper(), abspath)
                 # Stop searching filename/regex combos for this language
                 break
+
+
+# Data used to test external package detection
+detection_tests = {
+    'paths': [
+        # Ubuntu 18.04, system compilers without Fortran
+        {
+            'layout': [
+                {'subdir': ['bin'], 'name': 'gcc', 'output': 'echo 7.5.0'},
+                {'subdir': ['bin'], 'name': 'g++', 'output': 'echo 7.5.0'}
+            ],
+            'results': [{'spec': 'gcc@7.5.0 languages=c,c++'}]
+        },
+        # Mock a version < 7 of GCC that requires -dumpversion and
+        # errors with -dumpfullversion
+        {
+            'layout': [
+                {
+                    'subdir': ['bin'], 'name': 'gcc-5',
+                    'output': """
+if [[ "$1" == "-dumpversion" ]] ; then
+  echo "5.5.0"
+else
+  echo "gcc-5: fatal error: no input files"
+  echo "compilation terminated."
+  exit 1
+fi
+"""
+                },
+                {'subdir': ['bin'], 'name': 'g++-5', 'output': 'echo 5.5.0'},
+                {'subdir': ['bin'], 'name': 'gfortran-5', 'output': 'echo 5.5.0'}
+            ],
+            'results': [
+                {'spec': 'gcc@5.5.0 languages=c,c++,fortran'}
+            ]
+        },
+        # Multiple compilers present at the same time
+        {
+            'layout': [
+                {'subdir': ['bin'], 'name': 'x86_64-linux-gnu-gcc-6',
+                 'output': 'echo 6.5.0'},
+                {'subdir': ['bin'], 'name': 'x86_64-linux-gnu-gcc-10',
+                 'output': 'echo 10.1.0'},
+                {'subdir': ['bin'], 'name': 'x86_64-linux-gnu-g++-10',
+                 'output': 'echo 10.1.0'}
+            ],
+            'results': [
+                {'spec': 'gcc@6.5.0 languages=c'},
+                {'spec': 'gcc@10.1.0 languages=c,c++'}
+            ]
+        }
+    ]
+}

var/spack/repos/builtin/packages/intel/package.py

@@ -123,3 +123,38 @@ def fortran(self):
 
     # Since the current package is a subset of 'intel-parallel-studio',
     # all remaining Spack actions are handled in the package class.
+
+
+# Data used to test external package detection
+detection_tests = {
+    'paths': [
+        {
+            'layout': [
+                {
+                    'subdir': ['bin', 'intel64'], 'name': 'icc',
+                    'output': """
+echo "icc (ICC) 18.0.5 20180823"
+echo "Copyright (C) 1985-2018 Intel Corporation.  All rights reserved."
+"""
+                },
+                {
+                    'subdir': ['bin', 'intel64'], 'name': 'icpc',
+                    'output': """
+echo "icpc (ICC) 18.0.5 20180823"
+echo "Copyright (C) 1985-2018 Intel Corporation.  All rights reserved."
+"""
+                },
+                {
+                    'subdir': ['bin', 'intel64'], 'name': 'ifort',
+                    'output': """
+echo "ifort (IFORT) 18.0.5 20180823"
+echo "Copyright (C) 1985-2018 Intel Corporation.  All rights reserved."
+"""
+                }
+            ],
+            'results': [
+                {'spec': 'intel@18.0.5'}
+            ]
+        }
+    ]
+}