From 74266ea789bfb691597e272aa0a11ebf1e24dcce Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Wed, 22 Jan 2020 23:04:16 +0100 Subject: tests: removed code duplication (#14596) - [x] Factored to a common place the fixture `testing_gpg_directory`, renamed it as `mock_gnupghome` - [x] Removed altogether the function `has_gnupg2` For `has_gnupg2`, since we were not trying to parse the version from the output of: ```console $ gpg2 --version ``` this is effectively equivalent to check if `spack.util.gpg.GPG.gpg()` was found. If we need to ensure version is `^2.X` it's probably better to do it in `spack.util.gpg.GPG.gpg()` than in a separate function. --- lib/spack/spack/binary_distribution.py | 41 +++++++++++++++------------------- lib/spack/spack/test/ci.py | 11 +-------- lib/spack/spack/test/cmd/ci.py | 13 ++--------- lib/spack/spack/test/cmd/gpg.py | 25 +++++---------------- lib/spack/spack/test/conftest.py | 8 +++++++ lib/spack/spack/test/packaging.py | 22 +++--------------- 6 files changed, 37 insertions(+), 83 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/binary_distribution.py b/lib/spack/spack/binary_distribution.py index 37190a549c..acfe6f2cea 100644 --- a/lib/spack/spack/binary_distribution.py +++ b/lib/spack/spack/binary_distribution.py @@ -23,7 +23,7 @@ from llnl.util.filesystem import mkdirp, install_tree import spack.cmd import spack.config as config import spack.fetch_strategy as fs -import spack.util.gpg as gpg_util +import spack.util.gpg import spack.relocate as relocate import spack.util.spack_yaml as syaml import spack.mirror @@ -33,7 +33,6 @@ import spack.util.web as web_util from spack.spec import Spec from spack.stage import Stage from spack.util.gpg import Gpg -from spack.util.executable import ProcessError _build_cache_relative_path = 'build_cache' @@ -110,14 +109,6 @@ class NewLayoutException(spack.error.SpackError): pass -def has_gnupg2(): - try: - gpg_util.Gpg.gpg()('--version', output=os.devnull) - return True - except ProcessError: - return False - - def build_cache_relative_path(): return _build_cache_relative_path @@ -243,27 +234,31 @@ def checksum_tarball(file): def sign_tarball(key, force, specfile_path): # Sign the packages if keys available - if not has_gnupg2(): + if spack.util.gpg.Gpg.gpg() is None: raise NoGpgException( "gpg2 is not available in $PATH .\n" "Use spack install gnupg and spack load gnupg.") - else: - if key is None: - keys = Gpg.signing_keys() - if len(keys) == 1: - key = keys[0] - if len(keys) > 1: - raise PickKeyException(str(keys)) - if len(keys) == 0: - msg = "No default key available for signing.\n" - msg += "Use spack gpg init and spack gpg create" - msg += " to create a default key." - raise NoKeyException(msg) + + if key is None: + keys = Gpg.signing_keys() + if len(keys) == 1: + key = keys[0] + + if len(keys) > 1: + raise PickKeyException(str(keys)) + + if len(keys) == 0: + msg = "No default key available for signing.\n" + msg += "Use spack gpg init and spack gpg create" + msg += " to create a default key." + raise NoKeyException(msg) + if os.path.exists('%s.asc' % specfile_path): if force: os.remove('%s.asc' % specfile_path) else: raise NoOverwriteException('%s.asc' % specfile_path) + Gpg.sign(key, specfile_path, '%s.asc' % specfile_path) diff --git a/lib/spack/spack/test/ci.py b/lib/spack/spack/test/ci.py index fd8fd7d79d..3143a938c2 100644 --- a/lib/spack/spack/test/ci.py +++ b/lib/spack/spack/test/ci.py @@ -12,18 +12,9 @@ import spack.main as spack_main import spack.config as cfg import spack.paths as spack_paths import spack.spec as spec -import spack.util.gpg as gpg_util import spack.util.web as web_util -@pytest.fixture(scope='function') -def testing_gpg_directory(tmpdir): - old_gpg_path = gpg_util.GNUPGHOME - gpg_util.GNUPGHOME = str(tmpdir.join('gpg')) - yield - gpg_util.GNUPGHOME = old_gpg_path - - @pytest.fixture def tmp_scope(): """Creates a temporary configuration scope""" @@ -50,7 +41,7 @@ def test_urlencode_string(): assert(s_enc == 'Spack+Test+Project') -def test_import_signing_key(testing_gpg_directory): +def test_import_signing_key(mock_gnupghome): signing_key_dir = spack_paths.mock_gpg_keys_path signing_key_path = os.path.join(signing_key_dir, 'package-signing-key') with open(signing_key_path) as fd: diff --git a/lib/spack/spack/test/cmd/ci.py b/lib/spack/spack/test/cmd/ci.py index c914f565fc..49663b3409 100644 --- a/lib/spack/spack/test/cmd/ci.py +++ b/lib/spack/spack/test/cmd/ci.py @@ -14,7 +14,6 @@ import spack.ci as ci import spack.config import spack.environment as ev import spack.hash_types as ht -import spack.util.gpg as gpg_util from spack.main import SpackCommand import spack.paths as spack_paths import spack.repo as repo @@ -33,14 +32,6 @@ buildcache_cmd = SpackCommand('buildcache') git = exe.which('git', required=True) -@pytest.fixture(scope='function') -def testing_gpg_directory(tmpdir): - old_gpg_path = gpg_util.GNUPGHOME - gpg_util.GNUPGHOME = str(tmpdir.join('gpg')) - yield - gpg_util.GNUPGHOME = old_gpg_path - - @pytest.fixture() def env_deactivate(): yield @@ -393,7 +384,7 @@ spack: def test_ci_rebuild_basic(tmpdir, mutable_mock_env_path, env_deactivate, install_mockery, mock_packages, - testing_gpg_directory): + mock_gnupghome): working_dir = tmpdir.join('working_dir') mirror_dir = working_dir.join('mirror') @@ -505,7 +496,7 @@ def test_ci_pushyaml(tmpdir): @pytest.mark.disable_clean_stage_check def test_push_mirror_contents(tmpdir, mutable_mock_env_path, env_deactivate, install_mockery, mock_packages, mock_fetch, - mock_stage, testing_gpg_directory): + mock_stage, mock_gnupghome): working_dir = tmpdir.join('working_dir') mirror_dir = working_dir.join('mirror') diff --git a/lib/spack/spack/test/cmd/gpg.py b/lib/spack/spack/test/cmd/gpg.py index 2078705b7c..2b63fbdb29 100644 --- a/lib/spack/spack/test/cmd/gpg.py +++ b/lib/spack/spack/test/cmd/gpg.py @@ -7,37 +7,22 @@ import os import pytest +import spack.util.gpg + from spack.paths import mock_gpg_data_path, mock_gpg_keys_path -import spack.util.gpg as gpg_util from spack.main import SpackCommand from spack.util.executable import ProcessError -@pytest.fixture(scope='function') -def testing_gpg_directory(tmpdir): - old_gpg_path = gpg_util.GNUPGHOME - gpg_util.GNUPGHOME = str(tmpdir.join('gpg')) - yield - gpg_util.GNUPGHOME = old_gpg_path - - @pytest.fixture(scope='function') def gpg(): return SpackCommand('gpg') -def has_gnupg2(): - try: - gpg_util.Gpg.gpg()('--version', output=os.devnull) - return True - except Exception: - return False - - @pytest.mark.maybeslow -@pytest.mark.skipif(not has_gnupg2(), +@pytest.mark.skipif(not spack.util.gpg.Gpg.gpg(), reason='These tests require gnupg2') -def test_gpg(gpg, tmpdir, testing_gpg_directory): +def test_gpg(gpg, tmpdir, mock_gnupghome): # Verify a file with an empty keyring. with pytest.raises(ProcessError): gpg('verify', os.path.join(mock_gpg_data_path, 'content.txt')) @@ -77,7 +62,7 @@ def test_gpg(gpg, tmpdir, testing_gpg_directory): '--export', str(keypath), 'Spack testing 1', 'spack@googlegroups.com') - keyfp = gpg_util.Gpg.signing_keys()[0] + keyfp = spack.util.gpg.Gpg.signing_keys()[0] # List the keys. # TODO: Test the output here. diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py index 7fb99a4147..b40dd92f99 100644 --- a/lib/spack/spack/test/conftest.py +++ b/lib/spack/spack/test/conftest.py @@ -33,6 +33,8 @@ import spack.platforms.test import spack.repo import spack.stage import spack.util.executable +import spack.util.gpg + from spack.util.pattern import Bunch from spack.dependency import Dependency from spack.package import PackageBase @@ -670,6 +672,12 @@ def module_configuration(monkeypatch, request): ) return _impl + +@pytest.fixture() +def mock_gnupghome(tmpdir, monkeypatch): + monkeypatch.setattr(spack.util.gpg, 'GNUPGHOME', str(tmpdir.join('gpg'))) + + ########## # Fake archives and repositories ########## diff --git a/lib/spack/spack/test/packaging.py b/lib/spack/spack/test/packaging.py index b06c2ac16e..fa601196c3 100644 --- a/lib/spack/spack/test/packaging.py +++ b/lib/spack/spack/test/packaging.py @@ -19,10 +19,10 @@ import spack.repo import spack.store import spack.binary_distribution as bindist import spack.cmd.buildcache as buildcache +import spack.util.gpg from spack.spec import Spec from spack.paths import mock_gpg_keys_path from spack.fetch_strategy import URLFetchStrategy, FetchStrategyComposite -from spack.util.executable import ProcessError from spack.relocate import needs_binary_relocation, needs_text_relocation from spack.relocate import strings_contains_installroot from spack.relocate import get_patchelf, relocate_text, relocate_links @@ -31,22 +31,6 @@ from spack.relocate import macho_replace_paths, macho_make_paths_relative from spack.relocate import modify_macho_object, macho_get_paths -@pytest.fixture(scope='function') -def testing_gpg_directory(tmpdir): - old_gpg_path = spack.util.gpg.GNUPGHOME - spack.util.gpg.GNUPGHOME = str(tmpdir.join('gpg')) - yield - spack.util.gpg.GNUPGHOME = old_gpg_path - - -def has_gnupg2(): - try: - spack.util.gpg.Gpg.gpg()('--version', output=os.devnull) - return True - except ProcessError: - return False - - def fake_fetchify(url, pkg): """Fake the URL for a package so it downloads from a file.""" fetcher = FetchStrategyComposite() @@ -54,7 +38,7 @@ def fake_fetchify(url, pkg): pkg.fetcher = fetcher -@pytest.mark.usefixtures('install_mockery', 'testing_gpg_directory') +@pytest.mark.usefixtures('install_mockery', 'mock_gnupghome') def test_buildcache(mock_archive, tmpdir): # tweak patchelf to only do a download spec = Spec("patchelf") @@ -107,7 +91,7 @@ echo $PATH""" buildcache.setup_parser(parser) # Create a private key to sign package with if gpg2 available - if has_gnupg2(): + if spack.util.gpg.Gpg.gpg(): spack.util.gpg.Gpg.create(name='test key 1', expires='0', email='spack@googlegroups.com', comment='Spack test key') -- cgit v1.2.3-60-g2f50