Test package detection in a systematic way (#18175)

This PR adds a new audit sub-command to check that detection of relevant packages
is performed correctly in a few scenarios mocking real use-cases. The data for each 
package being tested is in a YAML file called detection_test.yaml alongside the 
corresponding package.py file.

This is to allow encoding detection tests for compilers and other widely used tools, 
in preparation for compilers as dependencies.

16 files changed, 591 insertions, 24 deletions

diff --git a/.github/workflows/audit.yaml b/.github/workflows/audit.yaml
index ca9c02e62d..3d8e1f10ae 100644
--- a/.github/workflows/audit.yaml
+++ b/.github/workflows/audit.yaml
@@ -34,6 +34,7 @@ jobs:
       run: |
           . share/spack/setup-env.sh
           coverage run $(which spack) audit packages
+          coverage run $(which spack) audit externals        
           coverage combine
           coverage xml
     - name: Package audits (without coverage)
@@ -41,6 +42,7 @@ jobs:
       run: |
           . share/spack/setup-env.sh
           $(which spack) audit packages
+          $(which spack) audit externals
     - uses: codecov/codecov-action@eaaf4bedf32dbdc6b720b63067d99c4d77d6047d # @v2.1.0
       if: ${{ inputs.with_coverage == 'true' }}
       with:
diff --git a/lib/spack/docs/packaging_guide.rst b/lib/spack/docs/packaging_guide.rst
index f433996ec7..d25009532a 100644
--- a/lib/spack/docs/packaging_guide.rst
+++ b/lib/spack/docs/packaging_guide.rst
@@ -6196,7 +6196,100 @@ follows:
                "foo-package@{0}".format(version_str)
            )
 
-.. _package-lifecycle:
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+Add detection tests to packages
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+To ensure that software is detected correctly for multiple configurations
+and on different systems users can write a ``detection_test.yaml`` file and
+put it in the package directory alongside the ``package.py`` file.
+This YAML file contains enough information for Spack to mock an environment
+and try to check if the detection logic yields the results that are expected.
+
+As a general rule, attributes at the top-level of ``detection_test.yaml``
+represent search mechanisms and they each map to a list of tests that should confirm
+the validity of the package's detection logic.
+
+The detection tests can be run with the following command:
+
+.. code-block:: console
+
+   $ spack audit externals
+
+Errors that have been detected are reported to screen.
+
+""""""""""""""""""""""""""
+Tests for PATH inspections
+""""""""""""""""""""""""""
+
+Detection tests insisting on ``PATH`` inspections are listed under
+the ``paths`` attribute:
+
+.. code-block:: yaml
+
+   paths:
+   - layout:
+     - executables:
+       - "bin/clang-3.9"
+       - "bin/clang++-3.9"
+       script: |
+         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 Field
+   * - ``layout``
+     - Specifies the filesystem tree used for the test
+     - List of objects
+     - Yes
+   * - ``layout:[0]:executables``
+     - Relative paths for the mock executables to be created
+     - List of strings
+     - Yes
+   * - ``layout:[0]:script``
+     - 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
+
+"""""""""""""""""""""""""""""""
+Reuse tests from other packages
+"""""""""""""""""""""""""""""""
+
+When using a custom repository, it is possible to customize a package that already exists in ``builtin``
+and reuse its external tests. To do so, just write a ``detection_tests.yaml`` alongside the customized
+``package.py`` with an ``includes`` attribute. For instance the ``detection_tests.yaml`` for
+``myrepo.llvm`` might look like:
+
+.. code-block:: yaml
+
+   includes:
+   - "builtin.llvm"
+
+This YAML file instructs Spack to run the detection tests defined in ``builtin.llvm`` in addition to
+those locally defined in the file.
 
 -----------------------------
 Style guidelines for packages
diff --git a/lib/spack/spack/audit.py b/lib/spack/spack/audit.py
index c3a028a72b..176c45487f 100644
--- a/lib/spack/spack/audit.py
+++ b/lib/spack/spack/audit.py
@@ -38,10 +38,13 @@ as input.
 import ast
 import collections
 import collections.abc
+import glob
 import inspect
 import itertools
+import pathlib
 import pickle
 import re
+import warnings
 from urllib.request import urlopen
 
 import llnl.util.lang
@@ -798,3 +801,76 @@ def _analyze_variants_in_directive(pkg, constraint, directive, error_cls):
             errors.append(err)
 
     return errors
+
+
+#: Sanity checks on package directives
+external_detection = AuditClass(
+    group="externals",
+    tag="PKG-EXTERNALS",
+    description="Sanity checks for external software detection",
+    kwargs=("pkgs",),
+)
+
+
+def packages_with_detection_tests():
+    """Return the list of packages with a corresponding detection_test.yaml file."""
+    import spack.config
+    import spack.util.path
+
+    to_be_tested = []
+    for current_repo in spack.repo.PATH.repos:
+        namespace = current_repo.namespace
+        packages_dir = pathlib.PurePath(current_repo.packages_path)
+        pattern = packages_dir / "**" / "detection_test.yaml"
+        pkgs_with_tests = [
+            f"{namespace}.{str(pathlib.PurePath(x).parent.name)}" for x in glob.glob(str(pattern))
+        ]
+        to_be_tested.extend(pkgs_with_tests)
+
+    return to_be_tested
+
+
+@external_detection
+def _test_detection_by_executable(pkgs, error_cls):
+    """Test drive external detection for packages"""
+    import spack.detection
+
+    errors = []
+
+    # Filter the packages and retain only the ones with detection tests
+    pkgs_with_tests = packages_with_detection_tests()
+    selected_pkgs = []
+    for current_package in pkgs_with_tests:
+        _, unqualified_name = spack.repo.partition_package_name(current_package)
+        # Check for both unqualified name and qualified name
+        if unqualified_name in pkgs or current_package in pkgs:
+            selected_pkgs.append(current_package)
+    selected_pkgs.sort()
+
+    if not selected_pkgs:
+        summary = "No detection test to run"
+        details = [f'  "{p}" has no detection test' for p in pkgs]
+        warnings.warn("\n".join([summary] + details))
+        return errors
+
+    for pkg_name in selected_pkgs:
+        for idx, test_runner in enumerate(
+            spack.detection.detection_tests(pkg_name, spack.repo.PATH)
+        ):
+            specs = test_runner.execute()
+            expected_specs = test_runner.expected_specs
+
+            not_detected = set(expected_specs) - set(specs)
+            if not_detected:
+                summary = pkg_name + ": cannot detect some specs"
+                details = [f'"{s}" was not detected [test_id={idx}]' for s in sorted(not_detected)]
+                errors.append(error_cls(summary=summary, details=details))
+
+            not_expected = set(specs) - set(expected_specs)
+            if not_expected:
+                summary = pkg_name + ": detected unexpected specs"
+                msg = '"{0}" was detected, but was not expected [test_id={1}]'
+                details = [msg.format(s, idx) for s in sorted(not_expected)]
+                errors.append(error_cls(summary=summary, details=details))
+
+    return errors
diff --git a/lib/spack/spack/cmd/audit.py b/lib/spack/spack/cmd/audit.py
index cd56dfadd9..86eea9f7bc 100644
--- a/lib/spack/spack/cmd/audit.py
+++ b/lib/spack/spack/cmd/audit.py
@@ -3,6 +3,7 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 import llnl.util.tty as tty
+import llnl.util.tty.colify
 import llnl.util.tty.color as cl
 
 import spack.audit
@@ -20,6 +21,15 @@ def setup_parser(subparser):
     # Audit configuration files
     sp.add_parser("configs", help="audit configuration files")
 
+    # Audit package recipes
+    external_parser = sp.add_parser("externals", help="check external detection in packages")
+    external_parser.add_argument(
+        "--list",
+        action="store_true",
+        dest="list_externals",
+        help="if passed, list which packages have detection tests",
+    )
+
     # Https and other linting
     https_parser = sp.add_parser("packages-https", help="check https in packages")
     https_parser.add_argument(
@@ -29,7 +39,7 @@ def setup_parser(subparser):
     # Audit package recipes
     pkg_parser = sp.add_parser("packages", help="audit package recipes")
 
-    for group in [pkg_parser, https_parser]:
+    for group in [pkg_parser, https_parser, external_parser]:
         group.add_argument(
             "name",
             metavar="PKG",
@@ -62,6 +72,18 @@ def packages_https(parser, args):
     _process_reports(reports)
 
 
+def externals(parser, args):
+    if args.list_externals:
+        msg = "@*{The following packages have detection tests:}"
+        tty.msg(cl.colorize(msg))
+        llnl.util.tty.colify.colify(spack.audit.packages_with_detection_tests(), indent=2)
+        return
+
+    pkgs = args.name or spack.repo.PATH.all_package_names()
+    reports = spack.audit.run_group(args.subcommand, pkgs=pkgs)
+    _process_reports(reports)
+
+
 def list(parser, args):
     for subcommand, check_tags in spack.audit.GROUPS.items():
         print(cl.colorize("@*b{" + subcommand + "}:"))
@@ -78,6 +100,7 @@ def list(parser, args):
 def audit(parser, args):
     subcommands = {
         "configs": configs,
+        "externals": externals,
         "packages": packages,
         "packages-https": packages_https,
         "list": list,
diff --git a/lib/spack/spack/cmd/external.py b/lib/spack/spack/cmd/external.py
index bf29787db9..081ec80394 100644
--- a/lib/spack/spack/cmd/external.py
+++ b/lib/spack/spack/cmd/external.py
@@ -5,6 +5,7 @@
 import argparse
 import errno
 import os
+import re
 import sys
 from typing import List, Optional
 
@@ -156,11 +157,20 @@ def packages_to_search_for(
 ):
     result = []
     for current_tag in tags:
-        result.extend(spack.repo.PATH.packages_with_tags(current_tag))
+        result.extend(spack.repo.PATH.packages_with_tags(current_tag, full=True))
+
     if names:
-        result = [x for x in result if x in names]
+        # Match both fully qualified and unqualified
+        parts = [rf"(^{x}$|[.]{x}$)" for x in names]
+        select_re = re.compile("|".join(parts))
+        result = [x for x in result if select_re.search(x)]
+
     if exclude:
-        result = [x for x in result if x not in exclude]
+        # Match both fully qualified and unqualified
+        parts = [rf"(^{x}$|[.]{x}$)" for x in exclude]
+        select_re = re.compile("|".join(parts))
+        result = [x for x in result if not select_re.search(x)]
+
     return result
 
 
diff --git a/lib/spack/spack/detection/__init__.py b/lib/spack/spack/detection/__init__.py
index 73ae34ce63..7c54fb9d49 100644
--- a/lib/spack/spack/detection/__init__.py
+++ b/lib/spack/spack/detection/__init__.py
@@ -4,6 +4,7 @@
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 from .common import DetectedPackage, executable_prefix, update_configuration
 from .path import by_path, executables_in_path
+from .test import detection_tests
 
 __all__ = [
     "DetectedPackage",
@@ -11,4 +12,5 @@ __all__ = [
     "executables_in_path",
     "executable_prefix",
     "update_configuration",
+    "detection_tests",
 ]
diff --git a/lib/spack/spack/detection/path.py b/lib/spack/spack/detection/path.py
index 4a085aacd0..4de703ac97 100644
--- a/lib/spack/spack/detection/path.py
+++ b/lib/spack/spack/detection/path.py
@@ -2,7 +2,7 @@
 # Spack Project Developers. See the top-level COPYRIGHT file for details.
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
-"""Detection of software installed in the system based on paths inspections
+"""Detection of software installed in the system, based on paths inspections
 and running executables.
 """
 import collections
@@ -322,12 +322,14 @@ def by_path(
     path_hints: Optional[List[str]] = None,
     max_workers: Optional[int] = None,
 ) -> Dict[str, List[DetectedPackage]]:
-    """Return the list of packages that have been detected on the system,
-    searching by path.
+    """Return the list of packages that have been detected on the system, keyed by
+    unqualified package name.
 
     Args:
-        packages_to_search: list of package classes to be detected
+        packages_to_search: list of packages to be detected. Each package can be either unqualified
+            of fully qualified
         path_hints: initial list of paths to be searched
+        max_workers: maximum number of workers to search for packages in parallel
     """
     # TODO: Packages should be able to define both .libraries and .executables in the future
     # TODO: determine_spec_details should get all relevant libraries and executables in one call
@@ -355,7 +357,8 @@ def by_path(
                 try:
                     detected = future.result(timeout=DETECTION_TIMEOUT)
                     if detected:
-                        result[pkg_name].extend(detected)
+                        _, unqualified_name = spack.repo.partition_package_name(pkg_name)
+                        result[unqualified_name].extend(detected)
                 except Exception:
                     llnl.util.tty.debug(
                         f"[EXTERNAL DETECTION] Skipping {pkg_name}: timeout reached"
diff --git a/lib/spack/spack/detection/test.py b/lib/spack/spack/detection/test.py
new file mode 100644
index 0000000000..f33040f292
--- /dev/null
+++ b/lib/spack/spack/detection/test.py
@@ -0,0 +1,187 @@
+# Copyright 2013-2023 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)
+"""Create and run mock e2e tests for package detection."""
+import collections
+import contextlib
+import pathlib
+import tempfile
+from typing import Any, Deque, Dict, Generator, List, NamedTuple, Tuple
+
+import jinja2
+
+from llnl.util import filesystem
+
+import spack.repo
+import spack.spec
+from spack.util import spack_yaml
+
+from .path import by_path
+
+
+class MockExecutables(NamedTuple):
+    """Mock executables to be used in detection tests"""
+
+    #: Relative paths for mock executables to be created
+    executables: List[str]
+    #: Shell script for the mock executable
+    script: str
+
+
+class ExpectedTestResult(NamedTuple):
+    """Data structure to model assertions on detection tests"""
+
+    #: Spec to be detected
+    spec: str
+
+
+class DetectionTest(NamedTuple):
+    """Data structure to construct detection tests by PATH inspection.
+
+    Packages may have a YAML file containing the description of one or more detection tests
+    to be performed. Each test creates a few mock executable scripts in a temporary folder,
+    and checks that detection by PATH gives the expected results.
+    """
+
+    pkg_name: str
+    layout: List[MockExecutables]
+    results: List[ExpectedTestResult]
+
+
+class Runner:
+    """Runs an external detection test"""
+
+    def __init__(self, *, test: DetectionTest, repository: spack.repo.RepoPath) -> None:
+        self.test = test
+        self.repository = repository
+        self.tmpdir = tempfile.TemporaryDirectory()
+
+    def execute(self) -> List[spack.spec.Spec]:
+        """Executes a test and returns the specs that have been detected.
+
+        This function sets-up a test in a temporary directory, according to the prescriptions
+        in the test layout, then performs a detection by executables and returns the specs that
+        have been detected.
+        """
+        with self._mock_layout() as path_hints:
+            entries = by_path([self.test.pkg_name], path_hints=path_hints)
+            _, unqualified_name = spack.repo.partition_package_name(self.test.pkg_name)
+            specs = set(x.spec for x in entries[unqualified_name])
+        return list(specs)
+
+    @contextlib.contextmanager
+    def _mock_layout(self) -> Generator[List[str], None, None]:
+        hints = set()
+        try:
+            for entry in self.test.layout:
+                exes = self._create_executable_scripts(entry)
+
+                for mock_executable in exes:
+                    hints.add(str(mock_executable.parent))
+
+            yield list(hints)
+        finally:
+            self.tmpdir.cleanup()
+
+    def _create_executable_scripts(self, mock_executables: MockExecutables) -> List[pathlib.Path]:
+        relative_paths = mock_executables.executables
+        script = mock_executables.script
+        script_template = jinja2.Template("#!/bin/bash\n{{ script }}\n")
+        result = []
+        for mock_exe_path in relative_paths:
+            rel_path = pathlib.Path(mock_exe_path)
+            abs_path = pathlib.Path(self.tmpdir.name) / rel_path
+            abs_path.parent.mkdir(parents=True, exist_ok=True)
+            abs_path.write_text(script_template.render(script=script))
+            filesystem.set_executable(abs_path)
+            result.append(abs_path)
+        return result
+
+    @property
+    def expected_specs(self) -> List[spack.spec.Spec]:
+        return [spack.spec.Spec(r.spec) for r in self.test.results]
+
+
+def detection_tests(pkg_name: str, repository: spack.repo.RepoPath) -> List[Runner]:
+    """Returns a list of test runners for a given package.
+
+    Currently, detection tests are specified in a YAML file, called ``detection_test.yaml``,
+    alongside the ``package.py`` file.
+
+    This function reads that file to create a bunch of ``Runner`` objects.
+
+    Args:
+        pkg_name: name of the package to test
+        repository: repository where the package lives
+    """
+    result = []
+    detection_tests_content = read_detection_tests(pkg_name, repository)
+
+    tests_by_path = detection_tests_content.get("paths", [])
+    for single_test_data in tests_by_path:
+        mock_executables = []
+        for layout in single_test_data["layout"]:
+            mock_executables.append(
+                MockExecutables(executables=layout["executables"], script=layout["script"])
+            )
+        expected_results = []
+        for assertion in single_test_data["results"]:
+            expected_results.append(ExpectedTestResult(spec=assertion["spec"]))
+
+        current_test = DetectionTest(
+            pkg_name=pkg_name, layout=mock_executables, results=expected_results
+        )
+        result.append(Runner(test=current_test, repository=repository))
+
+    return result
+
+
+def read_detection_tests(pkg_name: str, repository: spack.repo.RepoPath) -> Dict[str, Any]:
+    """Returns the normalized content of the detection_tests.yaml associated with the package
+    passed in input.
+
+    The content is merged with that of any package that is transitively included using the
+    "includes" attribute.
+
+    Args:
+        pkg_name: name of the package to test
+        repository: repository in which to search for packages
+    """
+    content_stack, seen = [], set()
+    included_packages: Deque[str] = collections.deque()
+
+    root_detection_yaml, result = _detection_tests_yaml(pkg_name, repository)
+    included_packages.extend(result.get("includes", []))
+    seen |= set(result.get("includes", []))
+
+    while included_packages:
+        current_package = included_packages.popleft()
+        try:
+            current_detection_yaml, content = _detection_tests_yaml(current_package, repository)
+        except FileNotFoundError as e:
+            msg = (
+                f"cannot read the detection tests from the '{current_package}' package, "
+                f"included by {root_detection_yaml}"
+            )
+            raise FileNotFoundError(msg + f"\n\n\t{e}\n")
+
+        content_stack.append((current_package, content))
+        included_packages.extend(x for x in content.get("includes", []) if x not in seen)
+        seen |= set(content.get("includes", []))
+
+    result.setdefault("paths", [])
+    for pkg_name, content in content_stack:
+        result["paths"].extend(content.get("paths", []))
+
+    return result
+
+
+def _detection_tests_yaml(
+    pkg_name: str, repository: spack.repo.RepoPath
+) -> Tuple[pathlib.Path, Dict[str, Any]]:
+    pkg_dir = pathlib.Path(repository.filename_for_package_name(pkg_name)).parent
+    detection_tests_yaml = pkg_dir / "detection_test.yaml"
+    with open(str(detection_tests_yaml)) as f:
+        content = spack_yaml.load(f)
+    return detection_tests_yaml, content
diff --git a/lib/spack/spack/repo.py b/lib/spack/spack/repo.py
index 4391d9a9a2..a89b5dd407 100644
--- a/lib/spack/spack/repo.py
+++ b/lib/spack/spack/repo.py
@@ -24,7 +24,7 @@ import sys
 import traceback
 import types
 import uuid
-from typing import Any, Dict, List, Union
+from typing import Any, Dict, List, Tuple, Union
 
 import llnl.path
 import llnl.util.filesystem as fs
@@ -745,10 +745,18 @@ class RepoPath:
         for name in self.all_package_names():
             yield self.package_path(name)
 
-    def packages_with_tags(self, *tags):
+    def packages_with_tags(self, *tags, full=False):
+        """Returns a list of packages matching any of the tags in input.
+
+        Args:
+            full: if True the package names in the output are fully-qualified
+        """
         r = set()
         for repo in self.repos:
-            r |= set(repo.packages_with_tags(*tags))
+            current = repo.packages_with_tags(*tags)
+            if full:
+                current = [f"{repo.namespace}.{x}" for x in current]
+            r |= set(current)
         return sorted(r)
 
     def all_package_classes(self):
@@ -1124,7 +1132,8 @@ class Repo:
     def dirname_for_package_name(self, pkg_name):
         """Get the directory name for a particular package.  This is the
         directory that contains its package.py file."""
-        return os.path.join(self.packages_path, pkg_name)
+        _, unqualified_name = self.partition_package_name(pkg_name)
+        return os.path.join(self.packages_path, unqualified_name)
 
     def filename_for_package_name(self, pkg_name):
         """Get the filename for the module we should load for a particular
@@ -1222,15 +1231,10 @@ class Repo:
         package. Then extracts the package class from the module
         according to Spack's naming convention.
         """
-        namespace, _, pkg_name = pkg_name.rpartition(".")
-        if namespace and (namespace != self.namespace):
-            raise InvalidNamespaceError(
-                "Invalid namespace for %s repo: %s" % (self.namespace, namespace)
-            )
-
+        namespace, pkg_name = self.partition_package_name(pkg_name)
         class_name = nm.mod_to_class(pkg_name)
+        fullname = f"{self.full_namespace}.{pkg_name}"
 
-        fullname = "{0}.{1}".format(self.full_namespace, pkg_name)
         try:
             module = importlib.import_module(fullname)
         except ImportError:
@@ -1241,7 +1245,7 @@ class Repo:
 
         cls = getattr(module, class_name)
         if not inspect.isclass(cls):
-            tty.die("%s.%s is not a class" % (pkg_name, class_name))
+            tty.die(f"{pkg_name}.{class_name} is not a class")
 
         new_cfg_settings = (
             spack.config.get("packages").get(pkg_name, {}).get("package_attributes", {})
@@ -1280,6 +1284,15 @@ class Repo:
 
         return cls
 
+    def partition_package_name(self, pkg_name: str) -> Tuple[str, str]:
+        namespace, pkg_name = partition_package_name(pkg_name)
+        if namespace and (namespace != self.namespace):
+            raise InvalidNamespaceError(
+                f"Invalid namespace for the '{self.namespace}' repo: {namespace}"
+            )
+
+        return namespace, pkg_name
+
     def __str__(self):
         return "[Repo '%s' at '%s']" % (self.namespace, self.root)
 
@@ -1293,6 +1306,20 @@ class Repo:
 RepoType = Union[Repo, RepoPath]
 
 
+def partition_package_name(pkg_name: str) -> Tuple[str, str]:
+    """Given a package name that might be fully-qualified, returns the namespace part,
+    if present and the unqualified package name.
+
+    If the package name is unqualified, the namespace is an empty string.
+
+    Args:
+        pkg_name: a package name, either unqualified like "llvl", or
+            fully-qualified, like "builtin.llvm"
+    """
+    namespace, _, pkg_name = pkg_name.rpartition(".")
+    return namespace, pkg_name
+
+
 def create_repo(root, namespace=None, subdir=packages_dir_name):
     """Create a new repository in root with the specified namespace.
 
diff --git a/lib/spack/spack/test/cmd/external.py b/lib/spack/spack/test/cmd/external.py
index 7d54057b46..0b4eca124d 100644
--- a/lib/spack/spack/test/cmd/external.py
+++ b/lib/spack/spack/test/cmd/external.py
@@ -120,8 +120,9 @@ def test_find_external_cmd_not_buildable(mutable_config, working_env, mock_execu
     "names,tags,exclude,expected",
     [
         # find --all
-        (None, ["detectable"], [], ["find-externals1"]),
+        (None, ["detectable"], [], ["builtin.mock.find-externals1"]),
         # find --all --exclude find-externals1
+        (None, ["detectable"], ["builtin.mock.find-externals1"], []),
         (None, ["detectable"], ["find-externals1"], []),
         # find cmake (and cmake is not detectable)
         (["cmake"], ["detectable"], [], []),
diff --git a/lib/spack/spack/test/repo.py b/lib/spack/spack/test/repo.py
index 7314beebb5..eb6b123916 100644
--- a/lib/spack/spack/test/repo.py
+++ b/lib/spack/spack/test/repo.py
@@ -181,3 +181,15 @@ def test_repository_construction_doesnt_use_globals(nullify_globals, repo_paths,
     repo_path = spack.repo.RepoPath(*repo_paths)
     assert len(repo_path.repos) == len(namespaces)
     assert [x.namespace for x in repo_path.repos] == namespaces
+
+
+@pytest.mark.parametrize("method_name", ["dirname_for_package_name", "filename_for_package_name"])
+def test_path_computation_with_names(method_name, mock_repo_path):
+    """Tests that repositories can compute the correct paths when using both fully qualified
+    names and unqualified names.
+    """
+    repo_path = spack.repo.RepoPath(mock_repo_path)
+    method = getattr(repo_path, method_name)
+    unqualified = method("mpileaks")
+    qualified = method("builtin.mock.mpileaks")
+    assert qualified == unqualified
diff --git a/share/spack/spack-completion.bash b/share/spack/spack-completion.bash
index 1983b960d5..b9521b8f0c 100755
--- a/share/spack/spack-completion.bash
+++ b/share/spack/spack-completion.bash
@@ -423,7 +423,7 @@ _spack_audit() {
     then
         SPACK_COMPREPLY="-h --help"
     else
-        SPACK_COMPREPLY="configs packages-https packages list"
+        SPACK_COMPREPLY="configs externals packages-https packages list"
     fi
 }
 
@@ -431,6 +431,15 @@ _spack_audit_configs() {
     SPACK_COMPREPLY="-h --help"
 }
 
+_spack_audit_externals() {
+    if $list_options
+    then
+        SPACK_COMPREPLY="-h --help --list"
+    else
+        SPACK_COMPREPLY=""
+    fi
+}
+
 _spack_audit_packages_https() {
     if $list_options
     then
diff --git a/share/spack/spack-completion.fish b/share/spack/spack-completion.fish
index c5da416817..f4ac310ada 100755
--- a/share/spack/spack-completion.fish
+++ b/share/spack/spack-completion.fish
@@ -508,6 +508,7 @@ complete -c spack -n '__fish_spack_using_command arch' -s b -l backend -d 'print
 # spack audit
 set -g __fish_spack_optspecs_spack_audit h/help
 complete -c spack -n '__fish_spack_using_command_pos 0 audit' -f -a configs -d 'audit configuration files'
+complete -c spack -n '__fish_spack_using_command_pos 0 audit' -f -a externals -d 'check external detection in packages'
 complete -c spack -n '__fish_spack_using_command_pos 0 audit' -f -a packages-https -d 'check https in packages'
 complete -c spack -n '__fish_spack_using_command_pos 0 audit' -f -a packages -d 'audit package recipes'
 complete -c spack -n '__fish_spack_using_command_pos 0 audit' -f -a list -d 'list available checks and exits'
@@ -519,6 +520,14 @@ set -g __fish_spack_optspecs_spack_audit_configs h/help
 complete -c spack -n '__fish_spack_using_command audit configs' -s h -l help -f -a help
 complete -c spack -n '__fish_spack_using_command audit configs' -s h -l help -d 'show this help message and exit'
 
+# spack audit externals
+set -g __fish_spack_optspecs_spack_audit_externals h/help list
+complete -c spack -n '__fish_spack_using_command_pos_remainder 0 audit externals' -f -a '(__fish_spack_packages)'
+complete -c spack -n '__fish_spack_using_command audit externals' -s h -l help -f -a help
+complete -c spack -n '__fish_spack_using_command audit externals' -s h -l help -d 'show this help message and exit'
+complete -c spack -n '__fish_spack_using_command audit externals' -l list -f -a list_externals
+complete -c spack -n '__fish_spack_using_command audit externals' -l list -d 'if passed, list which packages have detection tests'
+
 # spack audit packages-https
 set -g __fish_spack_optspecs_spack_audit_packages_https h/help all
 complete -c spack -n '__fish_spack_using_command_pos_remainder 0 audit packages-https' -f -a '(__fish_spack_packages)'
diff --git a/var/spack/repos/builtin/packages/gcc/detection_test.yaml b/var/spack/repos/builtin/packages/gcc/detection_test.yaml
new file mode 100644
index 0000000000..0930f82d93
--- /dev/null
+++ b/var/spack/repos/builtin/packages/gcc/detection_test.yaml
@@ -0,0 +1,38 @@
+paths:
+  # Ubuntu 18.04, system compilers without Fortran
+  - layout:
+      - executables:
+        - "bin/gcc"
+        - "bin/g++"
+        script: "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:
+      - executables:
+        - "bin/gcc-5"
+        - "bin/g++-5"
+        - "bin/gfortran-5"
+        script: |
+          if [[ "$1" == "-dumpversion" ]] ; then
+            echo "5.5.0"
+          else
+            echo "gcc-5: fatal error: no input files"
+            echo "compilation terminated."
+            exit 1
+          fi
+    results:
+      - spec: "gcc@5.5.0 languages=c,c++,fortran"
+  # Multiple compilers present at the same time
+  - layout:
+      - executables:
+        - "bin/x86_64-linux-gnu-gcc-6"
+        script: 'echo 6.5.0'
+      - executables:
+        - "bin/x86_64-linux-gnu-gcc-10"
+        - "bin/x86_64-linux-gnu-g++-10"
+        script: "echo 10.1.0"
+    results:
+      - spec: "gcc@6.5.0 languages=c"
+      - spec: "gcc@10.1.0 languages=c,c++"
diff --git a/var/spack/repos/builtin/packages/intel/detection_test.yaml b/var/spack/repos/builtin/packages/intel/detection_test.yaml
new file mode 100644
index 0000000000..076bfeaaba
--- /dev/null
+++ b/var/spack/repos/builtin/packages/intel/detection_test.yaml
@@ -0,0 +1,19 @@
+paths:
+  - layout:
+      - executables:
+        - "bin/intel64/icc"
+        script: |
+          echo "icc (ICC) 18.0.5 20180823"
+          echo "Copyright (C) 1985-2018 Intel Corporation.  All rights reserved."
+      - executables:
+        - "bin/intel64/icpc"
+        script: |
+          echo "icpc (ICC) 18.0.5 20180823"
+          echo "Copyright (C) 1985-2018 Intel Corporation.  All rights reserved."
+      - executables:
+        - "bin/intel64/ifort"
+        script: |
+          echo "ifort (IFORT) 18.0.5 20180823"
+          echo "Copyright (C) 1985-2018 Intel Corporation.  All rights reserved."
+    results:
+      - spec: 'intel@18.0.5'
diff --git a/var/spack/repos/builtin/packages/llvm/detection_test.yaml b/var/spack/repos/builtin/packages/llvm/detection_test.yaml
new file mode 100644
index 0000000000..48e9d6751a
--- /dev/null
+++ b/var/spack/repos/builtin/packages/llvm/detection_test.yaml
@@ -0,0 +1,56 @@
+paths:
+  - layout:
+      - executables:
+        - "bin/clang-3.9"
+        script: |
+          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"
+      - executables:
+        - "bin/clang++-3.9"
+        script: |
+          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'
+  # Multiple LLVM packages in the same prefix
+  - layout:
+      - executables:
+        - "bin/clang-8"
+        - "bin/clang++-8"
+        script: |
+          echo "clang version 8.0.0-3~ubuntu18.04.2 (tags/RELEASE_800/final)"
+          echo "Target: x86_64-pc-linux-gnu"
+          echo "Thread model: posix"
+          echo "InstalledDir: /usr/bin"
+      - executables:
+        - "bin/ld.lld-8"
+        script: 'echo "LLD 8.0.0 (compatible with GNU linkers)"'
+      - executables:
+        - "bin/lldb"
+        script: 'echo "lldb version 8.0.0"'
+      - executables:
+        - "bin/clang-3.9"
+        - "bin/clang++-3.9"
+        script: |
+          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@8.0.0+clang+lld+lldb'
+      - spec: 'llvm@3.9.1+clang~lld~lldb'
+  # Apple Clang should not be detected
+  - layout:
+    - executables:
+      - "bin/clang"
+      - "bin/clang++"
+      script: |
+        echo "Apple clang version 11.0.0 (clang-1100.0.33.8)"
+        echo "Target: x86_64-apple-darwin19.5.0"
+        echo "Thread model: posix"
+        echo "InstalledDir: /Library/Developer/CommandLineTools/usr/bin"
+    results: []