tests: Spec tests shouldn't fetch remote git repositories. (#43656)

Currently, some of the tests in `spec_format` and `spec_semantics` fetch
the actual zlib repository when run, because they call `str()` on specs
like `zlib@foo/bar`, which at least currently requires a remote git clone
to resolve.

This doesn't change the behavior of git versions, but it uses our mock git
repo infrastructure and clones the `git-test` package instead of the *real*
URL from the mock `zlib` package.

This should speed up tests.  We could probably refactor more so that the git
tests *all* use such a fixture, but the `checks` field that unfortunately
tightly couples the mock git repository and the `git_fetch` tests complicates
this. We could also consider *not* making `str()` resolve git versions, but
I did not dig into that here.

- [x] add a mock_git_test_package fixture that sets up a mock git repo *and*
      monkeypatches the `git-test` package (like our git test packages do)
- [x] use fixture in `test_spec_format_path`
- [x] use fixture in `test_spec_format_path_posix`
- [x] use fixture in `test_spec_format_path_windows`
- [x] use fixture in `test_parse_single_spec`
This commit is contained in:
Todd Gamblin 2024-04-15 10:20:23 -06:00 committed by GitHub
parent 8033455d5f
commit f9f97bf22b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 46 additions and 24 deletions

View File

@ -55,6 +55,7 @@
import spack.util.gpg import spack.util.gpg
import spack.util.spack_yaml as syaml import spack.util.spack_yaml as syaml
import spack.util.url as url_util import spack.util.url as url_util
import spack.version
from spack.fetch_strategy import URLFetchStrategy from spack.fetch_strategy import URLFetchStrategy
from spack.util.pattern import Bunch from spack.util.pattern import Bunch
@ -1440,6 +1441,15 @@ def mock_git_repository(git, tmpdir_factory):
yield t yield t
@pytest.fixture(scope="function")
def mock_git_test_package(mock_git_repository, mutable_mock_repo, monkeypatch):
# install a fake git version in the package class
pkg_class = spack.repo.PATH.get_pkg_class("git-test")
monkeypatch.delattr(pkg_class, "git")
monkeypatch.setitem(pkg_class.versions, spack.version.Version("git"), mock_git_repository.url)
return pkg_class
@pytest.fixture(scope="session") @pytest.fixture(scope="session")
def mock_hg_repository(tmpdir_factory): def mock_hg_repository(tmpdir_factory):
"""Creates a very simple hg repository with two commits.""" """Creates a very simple hg repository with two commits."""

View File

@ -1096,22 +1096,22 @@ def test_unsatisfiable_virtual_deps_bindings(self, spec_str):
@pytest.mark.parametrize( @pytest.mark.parametrize(
"spec_str,format_str,expected", "spec_str,format_str,expected",
[ [
("zlib@git.foo/bar", "{name}-{version}", str(pathlib.Path("zlib-git.foo_bar"))), ("git-test@git.foo/bar", "{name}-{version}", str(pathlib.Path("git-test-git.foo_bar"))),
("zlib@git.foo/bar", "{name}-{version}-{/hash}", None), ("git-test@git.foo/bar", "{name}-{version}-{/hash}", None),
("zlib@git.foo/bar", "{name}/{version}", str(pathlib.Path("zlib", "git.foo_bar"))), ("git-test@git.foo/bar", "{name}/{version}", str(pathlib.Path("git-test", "git.foo_bar"))),
( (
"zlib@{0}=1.0%gcc".format("a" * 40), "git-test@{0}=1.0%gcc".format("a" * 40),
"{name}/{version}/{compiler}", "{name}/{version}/{compiler}",
str(pathlib.Path("zlib", "{0}_1.0".format("a" * 40), "gcc")), str(pathlib.Path("git-test", "{0}_1.0".format("a" * 40), "gcc")),
), ),
( (
"zlib@git.foo/bar=1.0%gcc", "git-test@git.foo/bar=1.0%gcc",
"{name}/{version}/{compiler}", "{name}/{version}/{compiler}",
str(pathlib.Path("zlib", "git.foo_bar_1.0", "gcc")), str(pathlib.Path("git-test", "git.foo_bar_1.0", "gcc")),
), ),
], ],
) )
def test_spec_format_path(spec_str, format_str, expected): def test_spec_format_path(spec_str, format_str, expected, mock_git_test_package):
_check_spec_format_path(spec_str, format_str, expected) _check_spec_format_path(spec_str, format_str, expected)
@ -1129,45 +1129,57 @@ def _check_spec_format_path(spec_str, format_str, expected, path_ctor=None):
"spec_str,format_str,expected", "spec_str,format_str,expected",
[ [
( (
"zlib@git.foo/bar", "git-test@git.foo/bar",
r"C:\\installroot\{name}\{version}", r"C:\\installroot\{name}\{version}",
r"C:\installroot\zlib\git.foo_bar", r"C:\installroot\git-test\git.foo_bar",
), ),
( (
"zlib@git.foo/bar", "git-test@git.foo/bar",
r"\\hostname\sharename\{name}\{version}", r"\\hostname\sharename\{name}\{version}",
r"\\hostname\sharename\zlib\git.foo_bar", r"\\hostname\sharename\git-test\git.foo_bar",
), ),
# Windows doesn't attribute any significance to a leading # Windows doesn't attribute any significance to a leading
# "/" so it is discarded # "/" so it is discarded
("zlib@git.foo/bar", r"/installroot/{name}/{version}", r"installroot\zlib\git.foo_bar"), (
"git-test@git.foo/bar",
r"/installroot/{name}/{version}",
r"installroot\git-test\git.foo_bar",
),
], ],
) )
def test_spec_format_path_windows(spec_str, format_str, expected): def test_spec_format_path_windows(spec_str, format_str, expected, mock_git_test_package):
_check_spec_format_path(spec_str, format_str, expected, path_ctor=pathlib.PureWindowsPath) _check_spec_format_path(spec_str, format_str, expected, path_ctor=pathlib.PureWindowsPath)
@pytest.mark.parametrize( @pytest.mark.parametrize(
"spec_str,format_str,expected", "spec_str,format_str,expected",
[ [
("zlib@git.foo/bar", r"/installroot/{name}/{version}", "/installroot/zlib/git.foo_bar"), (
("zlib@git.foo/bar", r"//installroot/{name}/{version}", "//installroot/zlib/git.foo_bar"), "git-test@git.foo/bar",
r"/installroot/{name}/{version}",
"/installroot/git-test/git.foo_bar",
),
(
"git-test@git.foo/bar",
r"//installroot/{name}/{version}",
"//installroot/git-test/git.foo_bar",
),
# This is likely unintentional on Linux: Firstly, "\" is not a # This is likely unintentional on Linux: Firstly, "\" is not a
# path separator for POSIX, so this is treated as a single path # path separator for POSIX, so this is treated as a single path
# component (containing literal "\" characters); secondly, # component (containing literal "\" characters); secondly,
# Spec.format treats "\" as an escape character, so is # Spec.format treats "\" as an escape character, so is
# discarded (unless directly following another "\") # discarded (unless directly following another "\")
( (
"zlib@git.foo/bar", "git-test@git.foo/bar",
r"C:\\installroot\package-{name}-{version}", r"C:\\installroot\package-{name}-{version}",
r"C__installrootpackage-zlib-git.foo_bar", r"C__installrootpackage-git-test-git.foo_bar",
), ),
# "\" is not a POSIX separator, and Spec.format treats "\{" as a literal # "\" is not a POSIX separator, and Spec.format treats "\{" as a literal
# "{", which means that the resulting format string is invalid # "{", which means that the resulting format string is invalid
("zlib@git.foo/bar", r"package\{name}\{version}", None), ("git-test@git.foo/bar", r"package\{name}\{version}", None),
], ],
) )
def test_spec_format_path_posix(spec_str, format_str, expected): def test_spec_format_path_posix(spec_str, format_str, expected, mock_git_test_package):
_check_spec_format_path(spec_str, format_str, expected, path_ctor=pathlib.PurePosixPath) _check_spec_format_path(spec_str, format_str, expected, path_ctor=pathlib.PurePosixPath)

View File

@ -551,12 +551,12 @@ def _specfile_for(spec_str, filename):
"^[deptypes=build,link] zlib", "^[deptypes=build,link] zlib",
), ),
( (
"zlib@git.foo/bar", "git-test@git.foo/bar",
[ [
Token(TokenType.UNQUALIFIED_PACKAGE_NAME, "zlib"), Token(TokenType.UNQUALIFIED_PACKAGE_NAME, "git-test"),
Token(TokenType.GIT_VERSION, "@git.foo/bar"), Token(TokenType.GIT_VERSION, "@git.foo/bar"),
], ],
"zlib@git.foo/bar", "git-test@git.foo/bar",
), ),
# Variant propagation # Variant propagation
( (
@ -585,7 +585,7 @@ def _specfile_for(spec_str, filename):
), ),
], ],
) )
def test_parse_single_spec(spec_str, tokens, expected_roundtrip): def test_parse_single_spec(spec_str, tokens, expected_roundtrip, mock_git_test_package):
parser = SpecParser(spec_str) parser = SpecParser(spec_str)
assert tokens == parser.tokens() assert tokens == parser.tokens()
assert expected_roundtrip == str(parser.next_spec()) assert expected_roundtrip == str(parser.next_spec())