From 63b88c4b75c0cfc1b5f182e3e28412b56ce821d4 Mon Sep 17 00:00:00 2001
From: Massimiliano Culpo <massimiliano.culpo@gmail.com>
Date: Mon, 17 Jul 2023 10:36:29 +0200
Subject: Minimal cleanup of a few tests in `test/packaging.py` (#38880)
* Minimal cleanup of a few tests in packaging.py
* Use f-strings
---
lib/spack/spack/test/packaging.py | 287 +++++++++++++++++---------------------
1 file changed, 125 insertions(+), 162 deletions(-)
diff --git a/lib/spack/spack/test/packaging.py b/lib/spack/spack/test/packaging.py
index 43193b578c..b9bc51ac42 100644
--- a/lib/spack/spack/test/packaging.py
+++ b/lib/spack/spack/test/packaging.py
@@ -8,15 +8,15 @@ This test checks the binary packaging infrastructure
"""
import argparse
import os
+import pathlib
import platform
import shutil
-import stat
import sys
from collections import OrderedDict
import pytest
-from llnl.util.filesystem import mkdirp
+from llnl.util import filesystem as fs
from llnl.util.symlink import symlink
import spack.binary_distribution as bindist
@@ -43,144 +43,110 @@ from spack.spec import Spec
pytestmark = pytest.mark.skipif(sys.platform == "win32", reason="does not run on windows")
-def fake_fetchify(url, pkg):
- """Fake the URL for a package so it downloads from a file."""
- fetcher = FetchStrategyComposite()
- fetcher.append(URLFetchStrategy(url))
- pkg.fetcher = fetcher
-
-
@pytest.mark.usefixtures("install_mockery", "mock_gnupghome")
-def test_buildcache(mock_archive, tmpdir):
- # tweak patchelf to only do a download
- pspec = Spec("patchelf").concretized()
- pkg = pspec.package
- fake_fetchify(pkg.fetcher, pkg)
- mkdirp(os.path.join(pkg.prefix, "bin"))
- patchelfscr = os.path.join(pkg.prefix, "bin", "patchelf")
- f = open(patchelfscr, "w")
- body = """#!/bin/bash
-echo $PATH"""
- f.write(body)
- f.close()
- st = os.stat(patchelfscr)
- os.chmod(patchelfscr, st.st_mode | stat.S_IEXEC)
-
- # Install the test package
- spec = Spec("trivial-install-test-package")
- spec.concretize()
- assert spec.concrete
- pkg = spec.package
- fake_fetchify(mock_archive.url, pkg)
- pkg.do_install()
+def test_buildcache(mock_archive, tmp_path, monkeypatch, mutable_config):
+ # Install a test package
+ spec = Spec("trivial-install-test-package").concretized()
+ fetcher = FetchStrategyComposite()
+ fetcher.append(URLFetchStrategy(mock_archive.url))
+ monkeypatch.setattr(spec.package, "fetcher", fetcher)
+ spec.package.do_install()
pkghash = "/" + str(spec.dag_hash(7))
# Put some non-relocatable file in there
- filename = os.path.join(spec.prefix, "dummy.txt")
- with open(filename, "w") as script:
- script.write(spec.prefix)
+ dummy_txt = pathlib.Path(spec.prefix) / "dummy.txt"
+ dummy_txt.write_text(spec.prefix)
# Create an absolute symlink
linkname = os.path.join(spec.prefix, "link_to_dummy.txt")
- symlink(filename, linkname)
+ symlink(dummy_txt, linkname)
- # Create the build cache and
- # put it directly into the mirror
- mirror_path = os.path.join(str(tmpdir), "test-mirror")
+ # Create the build cache and put it directly into the mirror
+ mirror_path = str(tmp_path / "test-mirror")
spack.mirror.create(mirror_path, specs=[])
# register mirror with spack config
mirrors = {"spack-mirror-test": url_util.path_to_file_url(mirror_path)}
spack.config.set("mirrors", mirrors)
- stage = spack.stage.Stage(mirrors["spack-mirror-test"], name="build_cache", keep=True)
- stage.create()
-
- # setup argument parser
- parser = argparse.ArgumentParser()
- buildcache.setup_parser(parser)
+ with spack.stage.Stage(mirrors["spack-mirror-test"], name="build_cache", keep=True):
+ parser = argparse.ArgumentParser()
+ buildcache.setup_parser(parser)
+
+ create_args = ["create", "-a", "-f", mirror_path, pkghash]
+ # Create a private key to sign package with if gpg2 available
+ spack.util.gpg.create(
+ name="test key 1",
+ expires="0",
+ email="spack@googlegroups.com",
+ comment="Spack test key",
+ )
- create_args = ["create", "-a", "-f", mirror_path, pkghash]
- # Create a private key to sign package with if gpg2 available
- spack.util.gpg.create(
- name="test key 1", expires="0", email="spack@googlegroups.com", comment="Spack test key"
- )
+ create_args.insert(create_args.index("-a"), "--rebuild-index")
- create_args.insert(create_args.index("-a"), "--rebuild-index")
+ args = parser.parse_args(create_args)
+ buildcache.buildcache(parser, args)
+ # trigger overwrite warning
+ buildcache.buildcache(parser, args)
- args = parser.parse_args(create_args)
- buildcache.buildcache(parser, args)
- # trigger overwrite warning
- buildcache.buildcache(parser, args)
+ # Uninstall the package
+ spec.package.do_uninstall(force=True)
- # Uninstall the package
- pkg.do_uninstall(force=True)
+ install_args = ["install", "-f", pkghash]
+ args = parser.parse_args(install_args)
+ # Test install
+ buildcache.buildcache(parser, args)
- install_args = ["install", "-f", pkghash]
- args = parser.parse_args(install_args)
- # Test install
- buildcache.buildcache(parser, args)
+ files = os.listdir(spec.prefix)
- files = os.listdir(spec.prefix)
+ assert "link_to_dummy.txt" in files
+ assert "dummy.txt" in files
- assert "link_to_dummy.txt" in files
- assert "dummy.txt" in files
+ # Validate the relocation information
+ buildinfo = bindist.read_buildinfo_file(spec.prefix)
+ assert buildinfo["relocate_textfiles"] == ["dummy.txt"]
+ assert buildinfo["relocate_links"] == ["link_to_dummy.txt"]
- # Validate the relocation information
- buildinfo = bindist.read_buildinfo_file(spec.prefix)
- assert buildinfo["relocate_textfiles"] == ["dummy.txt"]
- assert buildinfo["relocate_links"] == ["link_to_dummy.txt"]
+ args = parser.parse_args(["keys"])
+ buildcache.buildcache(parser, args)
- args = parser.parse_args(["keys"])
- buildcache.buildcache(parser, args)
+ args = parser.parse_args(["list"])
+ buildcache.buildcache(parser, args)
- args = parser.parse_args(["list"])
- buildcache.buildcache(parser, args)
+ args = parser.parse_args(["list"])
+ buildcache.buildcache(parser, args)
- args = parser.parse_args(["list"])
- buildcache.buildcache(parser, args)
+ args = parser.parse_args(["list", "trivial"])
+ buildcache.buildcache(parser, args)
- args = parser.parse_args(["list", "trivial"])
- buildcache.buildcache(parser, args)
+ # Copy a key to the mirror to have something to download
+ shutil.copyfile(mock_gpg_keys_path + "/external.key", mirror_path + "/external.key")
- # Copy a key to the mirror to have something to download
- shutil.copyfile(mock_gpg_keys_path + "/external.key", mirror_path + "/external.key")
+ args = parser.parse_args(["keys"])
+ buildcache.buildcache(parser, args)
- args = parser.parse_args(["keys"])
- buildcache.buildcache(parser, args)
+ args = parser.parse_args(["keys", "-f"])
+ buildcache.buildcache(parser, args)
- args = parser.parse_args(["keys", "-f"])
- buildcache.buildcache(parser, args)
+ args = parser.parse_args(["keys", "-i", "-t"])
+ buildcache.buildcache(parser, args)
- args = parser.parse_args(["keys", "-i", "-t"])
- buildcache.buildcache(parser, args)
- # unregister mirror with spack config
- mirrors = {}
- spack.config.set("mirrors", mirrors)
- shutil.rmtree(mirror_path)
- stage.destroy()
+def test_relocate_text(tmp_path):
+ """Tests that a text file containing the original directory of an installation, can be
+ relocated to a target directory.
+ """
+ original_dir = "/home/spack/opt/spack"
+ relocation_dir = "/opt/rh/devtoolset/"
+ dummy_txt = tmp_path / "dummy.txt"
+ dummy_txt.write_text(original_dir)
+ ensure_binary_is_relocatable(os.path.realpath(dummy_txt))
+ relocate_text([str(dummy_txt)], {original_dir: relocation_dir})
+ text = dummy_txt.read_text()
-@pytest.mark.usefixtures("install_mockery")
-def test_relocate_text(tmpdir):
- spec = Spec("trivial-install-test-package")
- spec.concretize()
- with tmpdir.as_cwd():
- # Validate the text path replacement
- old_dir = "/home/spack/opt/spack"
- filename = "dummy.txt"
- with open(filename, "w") as script:
- script.write(old_dir)
- script.close()
- filenames = [filename]
- new_dir = "/opt/rh/devtoolset/"
- # Singleton dict doesn't matter if Ordered
- relocate_text(filenames, {old_dir: new_dir})
- with open(filename, "r") as script:
- for line in script:
- assert new_dir in line
- ensure_binary_is_relocatable(os.path.realpath(filename))
+ assert relocation_dir in text
+ assert original_dir not in text
def test_relocate_links(tmpdir):
@@ -229,7 +195,6 @@ def test_needs_relocation():
assert not needs_binary_relocation("text", "x-")
assert needs_text_relocation("text", "x-")
assert not needs_text_relocation("symbolic link to", "x-")
-
assert needs_binary_relocation("application", "x-mach-binary")
@@ -242,41 +207,41 @@ def test_replace_paths(tmpdir):
hash_d = "hukkosc7ahff7o65h6cdhvcoxm57d4bw"
hash_loco = "zy4oigsc4eovn5yhr2lk4aukwzoespob"
- prefix2hash = dict()
+ prefix2hash = {}
- old_spack_dir = os.path.join("%s" % tmpdir, "Users", "developer", "spack")
- mkdirp(old_spack_dir)
+ old_spack_dir = os.path.join(f"{tmpdir}", "Users", "developer", "spack")
+ fs.mkdirp(old_spack_dir)
- oldprefix_a = os.path.join("%s" % old_spack_dir, "pkgA-%s" % hash_a)
- oldlibdir_a = os.path.join("%s" % oldprefix_a, "lib")
- mkdirp(oldlibdir_a)
+ oldprefix_a = os.path.join(f"{old_spack_dir}", f"pkgA-{hash_a}")
+ oldlibdir_a = os.path.join(f"{oldprefix_a}", "lib")
+ fs.mkdirp(oldlibdir_a)
prefix2hash[str(oldprefix_a)] = hash_a
- oldprefix_b = os.path.join("%s" % old_spack_dir, "pkgB-%s" % hash_b)
- oldlibdir_b = os.path.join("%s" % oldprefix_b, "lib")
- mkdirp(oldlibdir_b)
+ oldprefix_b = os.path.join(f"{old_spack_dir}", f"pkgB-{hash_b}")
+ oldlibdir_b = os.path.join(f"{oldprefix_b}", "lib")
+ fs.mkdirp(oldlibdir_b)
prefix2hash[str(oldprefix_b)] = hash_b
- oldprefix_c = os.path.join("%s" % old_spack_dir, "pkgC-%s" % hash_c)
- oldlibdir_c = os.path.join("%s" % oldprefix_c, "lib")
- oldlibdir_cc = os.path.join("%s" % oldlibdir_c, "C")
- mkdirp(oldlibdir_c)
+ oldprefix_c = os.path.join(f"{old_spack_dir}", f"pkgC-{hash_c}")
+ oldlibdir_c = os.path.join(f"{oldprefix_c}", "lib")
+ oldlibdir_cc = os.path.join(f"{oldlibdir_c}", "C")
+ fs.mkdirp(oldlibdir_c)
prefix2hash[str(oldprefix_c)] = hash_c
- oldprefix_d = os.path.join("%s" % old_spack_dir, "pkgD-%s" % hash_d)
- oldlibdir_d = os.path.join("%s" % oldprefix_d, "lib")
- mkdirp(oldlibdir_d)
+ oldprefix_d = os.path.join(f"{old_spack_dir}", f"pkgD-{hash_d}")
+ oldlibdir_d = os.path.join(f"{oldprefix_d}", "lib")
+ fs.mkdirp(oldlibdir_d)
prefix2hash[str(oldprefix_d)] = hash_d
- oldprefix_local = os.path.join("%s" % tmpdir, "usr", "local")
- oldlibdir_local = os.path.join("%s" % oldprefix_local, "lib")
- mkdirp(oldlibdir_local)
+ oldprefix_local = os.path.join(f"{tmpdir}", "usr", "local")
+ oldlibdir_local = os.path.join(f"{oldprefix_local}", "lib")
+ fs.mkdirp(oldlibdir_local)
prefix2hash[str(oldprefix_local)] = hash_loco
- libfile_a = "libA.%s" % suffix
- libfile_b = "libB.%s" % suffix
- libfile_c = "libC.%s" % suffix
- libfile_d = "libD.%s" % suffix
- libfile_loco = "libloco.%s" % suffix
+ libfile_a = f"libA.{suffix}"
+ libfile_b = f"libB.{suffix}"
+ libfile_c = f"libC.{suffix}"
+ libfile_d = f"libD.{suffix}"
+ libfile_loco = f"libloco.{suffix}"
old_libnames = [
os.path.join(oldlibdir_a, libfile_a),
os.path.join(oldlibdir_b, libfile_b),
@@ -291,33 +256,33 @@ def test_replace_paths(tmpdir):
hash2prefix = dict()
- new_spack_dir = os.path.join("%s" % tmpdir, "Users", "Shared", "spack")
- mkdirp(new_spack_dir)
+ new_spack_dir = os.path.join(f"{tmpdir}", "Users", "Shared", "spack")
+ fs.mkdirp(new_spack_dir)
- prefix_a = os.path.join(new_spack_dir, "pkgA-%s" % hash_a)
+ prefix_a = os.path.join(new_spack_dir, f"pkgA-{hash_a}")
libdir_a = os.path.join(prefix_a, "lib")
- mkdirp(libdir_a)
+ fs.mkdirp(libdir_a)
hash2prefix[hash_a] = str(prefix_a)
- prefix_b = os.path.join(new_spack_dir, "pkgB-%s" % hash_b)
+ prefix_b = os.path.join(new_spack_dir, f"pkgB-{hash_b}")
libdir_b = os.path.join(prefix_b, "lib")
- mkdirp(libdir_b)
+ fs.mkdirp(libdir_b)
hash2prefix[hash_b] = str(prefix_b)
- prefix_c = os.path.join(new_spack_dir, "pkgC-%s" % hash_c)
+ prefix_c = os.path.join(new_spack_dir, f"pkgC-{hash_c}")
libdir_c = os.path.join(prefix_c, "lib")
libdir_cc = os.path.join(libdir_c, "C")
- mkdirp(libdir_cc)
+ fs.mkdirp(libdir_cc)
hash2prefix[hash_c] = str(prefix_c)
- prefix_d = os.path.join(new_spack_dir, "pkgD-%s" % hash_d)
+ prefix_d = os.path.join(new_spack_dir, f"pkgD-{hash_d}")
libdir_d = os.path.join(prefix_d, "lib")
- mkdirp(libdir_d)
+ fs.mkdirp(libdir_d)
hash2prefix[hash_d] = str(prefix_d)
- prefix_local = os.path.join("%s" % tmpdir, "usr", "local")
+ prefix_local = os.path.join(f"{tmpdir}", "usr", "local")
libdir_local = os.path.join(prefix_local, "lib")
- mkdirp(libdir_local)
+ fs.mkdirp(libdir_local)
hash2prefix[hash_loco] = str(prefix_local)
new_libnames = [
@@ -386,10 +351,10 @@ def test_replace_paths(tmpdir):
out_dict = macho_find_paths(
[oldlibdir_a, oldlibdir_b, oldlibdir_c, oldlibdir_cc, oldlibdir_local],
[
- "@rpath/%s" % libfile_a,
- "@rpath/%s" % libfile_b,
- "@rpath/%s" % libfile_c,
- "@rpath/%s" % libfile_loco,
+ f"@rpath/{libfile_a}",
+ f"@rpath/{libfile_b}",
+ f"@rpath/{libfile_c}",
+ f"@rpath/{libfile_loco}",
],
None,
old_spack_dir,
@@ -397,10 +362,10 @@ def test_replace_paths(tmpdir):
)
assert out_dict == {
- "@rpath/%s" % libfile_a: "@rpath/%s" % libfile_a,
- "@rpath/%s" % libfile_b: "@rpath/%s" % libfile_b,
- "@rpath/%s" % libfile_c: "@rpath/%s" % libfile_c,
- "@rpath/%s" % libfile_loco: "@rpath/%s" % libfile_loco,
+ f"@rpath/{libfile_a}": f"@rpath/{libfile_a}",
+ f"@rpath/{libfile_b}": f"@rpath/{libfile_b}",
+ f"@rpath/{libfile_c}": f"@rpath/{libfile_c}",
+ f"@rpath/{libfile_loco}": f"@rpath/{libfile_loco}",
oldlibdir_a: libdir_a,
oldlibdir_b: libdir_b,
oldlibdir_c: libdir_c,
@@ -410,15 +375,15 @@ def test_replace_paths(tmpdir):
out_dict = macho_find_paths(
[oldlibdir_a, oldlibdir_b, oldlibdir_d, oldlibdir_local],
- ["@rpath/%s" % libfile_a, "@rpath/%s" % libfile_b, "@rpath/%s" % libfile_loco],
+ [f"@rpath/{libfile_a}", f"@rpath/{libfile_b}", f"@rpath/{libfile_loco}"],
None,
old_spack_dir,
prefix2prefix,
)
assert out_dict == {
- "@rpath/%s" % libfile_a: "@rpath/%s" % libfile_a,
- "@rpath/%s" % libfile_b: "@rpath/%s" % libfile_b,
- "@rpath/%s" % libfile_loco: "@rpath/%s" % libfile_loco,
+ f"@rpath/{libfile_a}": f"@rpath/{libfile_a}",
+ f"@rpath/{libfile_b}": f"@rpath/{libfile_b}",
+ f"@rpath/{libfile_loco}": f"@rpath/{libfile_loco}",
oldlibdir_a: libdir_a,
oldlibdir_b: libdir_b,
oldlibdir_d: libdir_d,
@@ -550,18 +515,16 @@ def test_manual_download(
@property
def _instr(pkg):
- return "Download instructions for {0}".format(pkg.spec.name)
+ return f"Download instructions for {pkg.spec.name}"
spec = default_mock_concretization("a")
- pkg = spec.package
-
- pkg.manual_download = manual
+ spec.package.manual_download = manual
if instr:
monkeypatch.setattr(spack.package_base.PackageBase, "download_instr", _instr)
- expected = pkg.download_instr if manual else "All fetchers failed"
+ expected = spec.package.download_instr if manual else "All fetchers failed"
with pytest.raises(spack.util.web.FetchError, match=expected):
- pkg.do_fetch()
+ spec.package.do_fetch()
@pytest.fixture()
--
cgit v1.2.3-70-g09d2