From 8991cc4632eec4bae68cf04cb26af610aa2e5181 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Thu, 26 Nov 2020 08:55:17 +0100 Subject: concretizer: allow a bool to be passed as argument for tests dependencies (#20082) refers #20079 Added docstrings to 'concretize' and 'concretized' to document the format for tests. Added tests for the activation of test dependencies. --- lib/spack/spack/solver/asp.py | 9 +++++-- lib/spack/spack/spec.py | 25 ++++++++++++++----- lib/spack/spack/test/concretize.py | 28 ++++++++++++++++++++++ var/spack/repos/builtin.mock/packages/a/package.py | 1 + var/spack/repos/builtin.mock/packages/b/package.py | 2 ++ .../packages/test-dependency/package.py | 12 ++++++++++ 6 files changed, 69 insertions(+), 8 deletions(-) create mode 100644 var/spack/repos/builtin.mock/packages/test-dependency/package.py diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index 5ef362a22d..a7c69ae8e3 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -916,8 +916,13 @@ class SpackSolverSetup(object): named_cond.name = named_cond.name or pkg.name for t in sorted(dep.type): - # Skip test dependencies if they're not requested - if t == 'test' and (not tests or pkg.name not in tests): + # Skip test dependencies if they're not requested at all + if t == 'test' and not tests: + continue + + # ... or if they are requested only for certain packages + if t == 'test' and (not isinstance(tests, bool) + and pkg.name not in tests): continue if cond == spack.spec.Spec(): diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 743f84c3c8..ac4ce87331 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -2463,8 +2463,14 @@ class Spec(object): self._dup(concretized) self._mark_concrete() - #: choose your concretizer here. def concretize(self, tests=False): + """Concretize the current spec. + + Args: + tests (bool or list): if False disregard 'test' dependencies, + if a list of names activate them for the packages in the list, + if True activate 'test' dependencies for all packages. + """ if spack.config.get('config:concretizer') == "clingo": self._new_concretize(tests) else: @@ -2482,12 +2488,19 @@ class Spec(object): s._normal = value s._concrete = value - def concretized(self): - """This is a non-destructive version of concretize(). First clones, - then returns a concrete version of this package without modifying - this package. """ + def concretized(self, tests=False): + """This is a non-destructive version of concretize(). + + First clones, then returns a concrete version of this package + without modifying this package. + + Args: + tests (bool or list): if False disregard 'test' dependencies, + if a list of names activate them for the packages in the list, + if True activate 'test' dependencies for all packages. + """ clone = self.copy(caches=False) - clone.concretize() + clone.concretize(tests=tests) return clone def flat_dependencies(self, **kwargs): diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index b150d60426..8640c91a9e 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -895,3 +895,31 @@ class TestConcretize(object): assert 'v1-provider' in s assert s['v1'].name == 'v1-provider' assert s['v2'].name == 'conditional-provider' + + @pytest.mark.regression('20079') + @pytest.mark.parametrize('spec_str,tests_arg,with_dep,without_dep', [ + # Check that True is treated correctly and attaches test deps + # to all nodes in the DAG + ('a', True, ['a'], []), + ('a foobar=bar', True, ['a', 'b'], []), + # Check that a list of names activates the dependency only for + # packages in that list + ('a foobar=bar', ['a'], ['a'], ['b']), + ('a foobar=bar', ['b'], ['b'], ['a']), + # Check that False disregard test dependencies + ('a foobar=bar', False, [], ['a', 'b']), + ]) + def test_activating_test_dependencies( + self, spec_str, tests_arg, with_dep, without_dep + ): + s = Spec(spec_str).concretized(tests=tests_arg) + + for pkg_name in with_dep: + msg = "Cannot find test dependency in package '{0}'" + node = s[pkg_name] + assert node.dependencies(deptype='test'), msg.format(pkg_name) + + for pkg_name in without_dep: + msg = "Test dependency in package '{0}' is unexpected" + node = s[pkg_name] + assert not node.dependencies(deptype='test'), msg.format(pkg_name) diff --git a/var/spack/repos/builtin.mock/packages/a/package.py b/var/spack/repos/builtin.mock/packages/a/package.py index 04e69dcd91..a603940b8b 100644 --- a/var/spack/repos/builtin.mock/packages/a/package.py +++ b/var/spack/repos/builtin.mock/packages/a/package.py @@ -31,6 +31,7 @@ class A(AutotoolsPackage): variant('bvv', default=True, description='The good old BV variant') depends_on('b', when='foobar=bar') + depends_on('test-dependency', type='test') parallel = False diff --git a/var/spack/repos/builtin.mock/packages/b/package.py b/var/spack/repos/builtin.mock/packages/b/package.py index 0dd6556e82..8cf5674219 100644 --- a/var/spack/repos/builtin.mock/packages/b/package.py +++ b/var/spack/repos/builtin.mock/packages/b/package.py @@ -13,3 +13,5 @@ class B(Package): url = "http://www.example.com/b-1.0.tar.gz" version('1.0', '0123456789abcdef0123456789abcdef') + + depends_on('test-dependency', type='test') diff --git a/var/spack/repos/builtin.mock/packages/test-dependency/package.py b/var/spack/repos/builtin.mock/packages/test-dependency/package.py new file mode 100644 index 0000000000..70302debd8 --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/test-dependency/package.py @@ -0,0 +1,12 @@ +# Copyright 2013-2020 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) +class TestDependency(Package): + """Represent a dependency that is pulled-in to allow testing other + packages. + """ + homepage = "http://www.example.com" + url = "http://www.example.com/tdep-1.0.tar.gz" + + version('1.0', '0123456789abcdef0123456789abcdef') -- cgit v1.2.3-60-g2f50