From 4f1d9d6095e5d59ebec5563e628c74efebc150e9 Mon Sep 17 00:00:00 2001 From: Harmen Stoppels Date: Mon, 15 Mar 2021 21:34:18 +0100 Subject: 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 | 16 +++++++++- lib/spack/spack/cmd/install.py | 25 ++++++++++------ lib/spack/spack/concretize.py | 6 ++-- lib/spack/spack/environment.py | 27 +++++++++-------- lib/spack/spack/test/cmd/concretize.py | 55 ++++++++++++++++++++++++++++++++++ lib/spack/spack/test/cmd/install.py | 20 +++++++++++++ lib/spack/spack/test/concretize.py | 3 +- 7 files changed, 125 insertions(+), 27 deletions(-) create mode 100644 lib/spack/spack/test/cmd/concretize.py (limited to 'lib') diff --git a/lib/spack/spack/cmd/concretize.py b/lib/spack/spack/cmd/concretize.py index b94511569b..e6a8ecf6f5 100644 --- a/lib/spack/spack/cmd/concretize.py +++ b/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() diff --git a/lib/spack/spack/cmd/install.py b/lib/spack/spack/cmd/install.py index 5e572f82f6..613c4b2422 100644 --- a/lib/spack/spack/cmd/install.py +++ b/lib/spack/spack/cmd/install.py @@ -241,14 +241,28 @@ environment variables: 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 @@ environment variables: # 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 - if args.test == 'all' or args.run_tests: - tests = True - elif args.test == 'root': - tests = [spec.name for spec in abstract_specs] + tests = get_tests(abstract_specs) kwargs['tests'] = tests try: diff --git a/lib/spack/spack/concretize.py b/lib/spack/spack/concretize.py index f5c33e5974..9228c66181 100644 --- a/lib/spack/spack/concretize.py +++ b/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 concretize_specs_together(*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 diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py index 8f930d67c8..fe1eb67459 100644 --- a/lib/spack/spack/environment.py +++ b/lib/spack/spack/environment.py @@ -1064,7 +1064,7 @@ class Environment(object): 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 @@ class Environment(object): 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 @@ class Environment(object): # 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 @@ class Environment(object): 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 @@ class Environment(object): 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 @@ class Environment(object): 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 @@ class Environment(object): ) 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 display_specs(concretized_specs): 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 diff --git a/lib/spack/spack/test/cmd/concretize.py b/lib/spack/spack/test/cmd/concretize.py new file mode 100644 index 0000000000..ec8dbdf910 --- /dev/null +++ b/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') diff --git a/lib/spack/spack/test/cmd/install.py b/lib/spack/spack/test/cmd/install.py index f770bc4184..ba11c68987 100644 --- a/lib/spack/spack/test/cmd/install.py +++ b/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) diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index f6898e3e03..f53330fc7a 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -660,8 +660,7 @@ class TestConcretize(object): 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( -- cgit v1.2.3-70-g09d2