Propagate --test= for environments (#22040)

* Propagate --test= for environments

* Improve help comment for spack concretize --test flag

* Add tests for --test with environments

lib/spack/spack/cmd/concretize.py

@@ -14,11 +14,25 @@ def setup_parser(subparser):
     subparser.add_argument(
         '-f', '--force', action='store_true',
         help="Re-concretize even if already concretized.")
+    subparser.add_argument(
+        '--test', default=None,
+        choices=['root', 'all'],
+        help="""Concretize with test dependencies. When 'root' is chosen, test
+dependencies are only added for the environment's root specs. When 'all' is
+chosen, test dependencies are enabled for all packages in the environment.""")
 
 
 def concretize(parser, args):
     env = ev.get_env(args, 'concretize', required=True)
+
+    if args.test == 'all':
+        tests = True
+    elif args.test == 'root':
+        tests = [spec.name for spec in env.user_specs]
+    else:
+        tests = False
+
     with env.write_transaction():
-        concretized_specs = env.concretize(force=args.force)
+        concretized_specs = env.concretize(force=args.force, tests=tests)
         ev.display_specs(concretized_specs)
         env.write()

lib/spack/spack/cmd/install.py

@@ -241,14 +241,28 @@ def install(parser, args, **kwargs):
     if args.log_file:
         reporter.filename = args.log_file
 
+    if args.run_tests:
+        tty.warn("Deprecated option: --run-tests: use --test=all instead")
+
+    def get_tests(specs):
+        if args.test == 'all' or args.run_tests:
+            return True
+        elif args.test == 'root':
+            return [spec.name for spec in specs]
+        else:
+            return False
+
     if not args.spec and not args.specfiles:
         # if there are no args but an active environment
         # then install the packages from it.
         env = ev.get_env(args, 'install')
         if env:
+            tests = get_tests(env.user_specs)
+            kwargs['tests'] = tests
+
             if not args.only_concrete:
                 with env.write_transaction():
-                    concretized_specs = env.concretize()
+                    concretized_specs = env.concretize(tests=tests)
                     ev.display_specs(concretized_specs)
 
                     # save view regeneration for later, so that we only do it
@@ -295,16 +309,9 @@ def install(parser, args, **kwargs):
     # that will be passed to the package installer
     update_kwargs_from_args(args, kwargs)
 
-    if args.run_tests:
-        tty.warn("Deprecated option: --run-tests: use --test=all instead")
-
     # 1. Abstract specs from cli
     abstract_specs = spack.cmd.parse_specs(args.spec)
-    tests = False
+    tests = get_tests(abstract_specs)
-    if args.test == 'all' or args.run_tests:
-        tests = True
-    elif args.test == 'root':
-        tests = [spec.name for spec in abstract_specs]
     kwargs['tests'] = tests
 
     try:

lib/spack/spack/concretize.py

@@ -714,10 +714,12 @@ def _compiler_concretization_failure(compiler_spec, arch):
         raise UnavailableCompilerVersionError(compiler_spec, arch)
 
 
-def concretize_specs_together(*abstract_specs):
+def concretize_specs_together(*abstract_specs, **kwargs):
     """Given a number of specs as input, tries to concretize them together.
 
     Args:
+        tests (bool or list or set): False to run no tests, True to test
+            all packages, or a list of package names to run tests for some
         *abstract_specs: abstract specs to be concretized, given either
             as Specs or strings
 
@@ -757,7 +759,7 @@ def make_concretization_repository(abstract_specs):
     with spack.repo.additional_repository(concretization_repository):
         # Spec from a helper package that depends on all the abstract_specs
         concretization_root = spack.spec.Spec('concretizationroot')
-        concretization_root.concretize()
+        concretization_root.concretize(tests=kwargs.get('tests', False))
         # Retrieve the direct dependencies
         concrete_specs = [
             concretization_root[spec.name].copy() for spec in abstract_specs

lib/spack/spack/environment.py

@@ -1064,7 +1064,7 @@ def undevelop(self, spec):
             return True
         return False
 
-    def concretize(self, force=False):
+    def concretize(self, force=False, tests=False):
         """Concretize user_specs in this environment.
 
         Only concretizes specs that haven't been concretized yet unless
@@ -1076,6 +1076,8 @@ def concretize(self, force=False):
         Arguments:
             force (bool): re-concretize ALL specs, even those that were
                already concretized
+            tests (bool or list or set): False to run no tests, True to test
+                all packages, or a list of package names to run tests for some
 
         Returns:
             List of specs that have been concretized. Each entry is a tuple of
@@ -1089,14 +1091,14 @@ def concretize(self, force=False):
 
         # Pick the right concretization strategy
         if self.concretization == 'together':
-            return self._concretize_together()
+            return self._concretize_together(tests=tests)
         if self.concretization == 'separately':
-            return self._concretize_separately()
+            return self._concretize_separately(tests=tests)
 
         msg = 'concretization strategy not implemented [{0}]'
         raise SpackEnvironmentError(msg.format(self.concretization))
 
-    def _concretize_together(self):
+    def _concretize_together(self, tests=False):
         """Concretization strategy that concretizes all the specs
         in the same DAG.
         """
@@ -1129,14 +1131,13 @@ def _concretize_together(self):
         self.specs_by_hash = {}
 
         concrete_specs = spack.concretize.concretize_specs_together(
-            *self.user_specs
+            *self.user_specs, tests=tests)
-        )
         concretized_specs = [x for x in zip(self.user_specs, concrete_specs)]
         for abstract, concrete in concretized_specs:
             self._add_concrete_spec(abstract, concrete)
         return concretized_specs
 
-    def _concretize_separately(self):
+    def _concretize_separately(self, tests=False):
         """Concretization strategy that concretizes separately one
         user spec after the other.
         """
@@ -1159,12 +1160,12 @@ def _concretize_separately(self):
         for uspec, uspec_constraints in zip(
                 self.user_specs, self.user_specs.specs_as_constraints):
             if uspec not in old_concretized_user_specs:
-                concrete = _concretize_from_constraints(uspec_constraints)
+                concrete = _concretize_from_constraints(uspec_constraints, tests=tests)
                 self._add_concrete_spec(uspec, concrete)
                 concretized_specs.append((uspec, concrete))
         return concretized_specs
 
-    def concretize_and_add(self, user_spec, concrete_spec=None):
+    def concretize_and_add(self, user_spec, concrete_spec=None, tests=False):
         """Concretize and add a single spec to the environment.
 
         Concretize the provided ``user_spec`` and add it along with the
@@ -1187,7 +1188,7 @@ def concretize_and_add(self, user_spec, concrete_spec=None):
         spec = Spec(user_spec)
 
         if self.add(spec):
-            concrete = concrete_spec or spec.concretized()
+            concrete = concrete_spec or spec.concretized(tests=tests)
             self._add_concrete_spec(spec, concrete)
         else:
             # spec might be in the user_specs, but not installed.
@@ -1197,7 +1198,7 @@ def concretize_and_add(self, user_spec, concrete_spec=None):
             )
             concrete = self.specs_by_hash.get(spec.build_hash())
             if not concrete:
-                concrete = spec.concretized()
+                concrete = spec.concretized(tests=tests)
                 self._add_concrete_spec(spec, concrete)
 
         return concrete
@@ -1904,7 +1905,7 @@ def _tree_to_display(spec):
         print('')
 
 
-def _concretize_from_constraints(spec_constraints):
+def _concretize_from_constraints(spec_constraints, tests=False):
     # Accept only valid constraints from list and concretize spec
     # Get the named spec even if out of order
     root_spec = [s for s in spec_constraints if s.name]
@@ -1923,7 +1924,7 @@ def _concretize_from_constraints(spec_constraints):
             if c not in invalid_constraints:
                 s.constrain(c)
         try:
-            return s.concretized()
+            return s.concretized(tests=tests)
         except spack.spec.InvalidDependencyError as e:
             invalid_deps_string = ['^' + d for d in e.invalid_deps]
             invalid_deps = [c for c in spec_constraints

lib/spack/spack/test/cmd/concretize.py

@@ -0,0 +1,55 @@
+# 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)
+
+
+import pytest
+import spack.environment as ev
+from spack.main import SpackCommand
+
+
+# everything here uses the mock_env_path
+pytestmark = pytest.mark.usefixtures(
+    'mutable_mock_env_path', 'config', 'mutable_mock_repo')
+
+env        = SpackCommand('env')
+add        = SpackCommand('add')
+concretize = SpackCommand('concretize')
+
+
+@pytest.mark.parametrize('concretization', ['separately', 'together'])
+def test_concretize_all_test_dependencies(concretization):
+    """Check all test dependencies are concretized."""
+    env('create', 'test')
+
+    with ev.read('test') as e:
+        e.concretization = concretization
+        add('depb')
+        concretize('--test', 'all')
+        assert e.matching_spec('test-dependency')
+
+
+@pytest.mark.parametrize('concretization', ['separately', 'together'])
+def test_concretize_root_test_dependencies_not_recursive(concretization):
+    """Check that test dependencies are not concretized recursively."""
+    env('create', 'test')
+
+    with ev.read('test') as e:
+        e.concretization = concretization
+        add('depb')
+        concretize('--test', 'root')
+        assert e.matching_spec('test-dependency') is None
+
+
+@pytest.mark.parametrize('concretization', ['separately', 'together'])
+def test_concretize_root_test_dependencies_are_concretized(concretization):
+    """Check that root test dependencies are concretized."""
+    env('create', 'test')
+
+    with ev.read('test') as e:
+        e.concretization = concretization
+        add('a')
+        add('b')
+        concretize('--test', 'root')
+        assert e.matching_spec('test-dependency')

lib/spack/spack/test/cmd/install.py

@@ -885,3 +885,23 @@ def test_cache_install_full_hash_match(
 
     uninstall('-y', s.name)
     mirror('rm', 'test-mirror')
+
+
+def test_install_env_with_tests_all(tmpdir, mock_packages, mock_fetch,
+                                    install_mockery, mutable_mock_env_path):
+    env('create', 'test')
+    with ev.read('test'):
+        test_dep = Spec('test-dependency').concretized()
+        add('depb')
+        install('--test', 'all')
+        assert os.path.exists(test_dep.prefix)
+
+
+def test_install_env_with_tests_root(tmpdir, mock_packages, mock_fetch,
+                                     install_mockery, mutable_mock_env_path):
+    env('create', 'test')
+    with ev.read('test'):
+        test_dep = Spec('test-dependency').concretized()
+        add('depb')
+        install('--test', 'root')
+        assert not os.path.exists(test_dep.prefix)

lib/spack/spack/test/concretize.py

@@ -660,8 +660,7 @@ def test_simultaneous_concretization_of_specs(self, abstract_specs):
 
         abstract_specs = [Spec(x) for x in abstract_specs]
         concrete_specs = spack.concretize.concretize_specs_together(
-            *abstract_specs
+            *abstract_specs)
-        )
 
         # Check there's only one configuration of each package in the DAG
         names = set(

share/spack/spack-completion.bash

@@ -579,7 +579,7 @@ _spack_compilers() {
 }
 
 _spack_concretize() {
-    SPACK_COMPREPLY="-h --help -f --force"
+    SPACK_COMPREPLY="-h --help -f --force --test"
 }
 
 _spack_config() {