zhangyiss/spack
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Make GHA tests parallel by using xdist (#32361)

Browse Source 
* Add two no-op jobs named "all-prechecks" and "all"

These are a suggestion from @tgamblin, they are stable named markers we
can use from gitlab and possibly for required checks to make CI more
resilient to refactors changing the names of specific checks.

* Enable parallel testing using xdist for unit testing in CI

* Normalize tmp paths to deal with macos

* add -u flag compatibility to spack python

As of now, it is accepted and ignored.  The usage with xdist, where it
is invoked specifically by `python -u spack python` which is then passed
`-u` by xdist is the entire reason for doing this.  It should never be
used without explicitly passing -u to the executing python interpreter.

* use spack python in xdist to support python 2

When running on python2, spack has many import cycles unless started
through main.  To allow that, this uses `spack python` as the
interpreter, leveraging the `-u` support so xdist doesn't error out when
it unconditionally requests unbuffered binary IO.

* Use shutil.move to account for tmpdir being in a separate filesystem sometimes
This commit is contained in:
Tom Scogland
2022-09-07 11:12:57 -07:00
committed by GitHub
parent 8e5ccddc13
commit 762ba27036
25 changed files with 221 additions and 123 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
lib/spack/llnl/util/tty/pty.py
View File

@@ -228,8 +228,8 @@ def __init__(self, controller_function, minion_function):
self.minion_function = minion_function
# these can be optionally set to change defaults
self.controller_timeout = 1
self.sleep_time = 0
self.controller_timeout = 3
self.sleep_time = 0.1
def start(self, **kwargs):
"""Start the controller and minion processes.

6
lib/spack/spack/cmd/python.py
View File

@@ -30,6 +30,12 @@ def setup_parser(subparser):
help="print the Python version number and exit",
)
subparser.add_argument("-c", dest="python_command", help="command to execute")
subparser.add_argument(
"-u",
dest="unbuffered",
action="store_true",
help="for compatibility with xdist, do not use without adding -u to the interpreter",
)
subparser.add_argument(
"-i",
dest="python_interpreter",

50
lib/spack/spack/main.py
View File

@@ -18,6 +18,7 @@
import pstats
import re
import signal
import subprocess as sp
import sys
import traceback
import warnings
@@ -623,15 +624,19 @@ class SpackCommand(object):
their output.
"""
def __init__(self, command_name):
def __init__(self, command_name, subprocess=False):
"""Create a new SpackCommand that invokes ``command_name`` when called.
Args:
command_name (str): name of the command to invoke
subprocess (bool): whether to fork a subprocess or not. Currently not supported on
Windows, where it is always False.
"""
self.parser = make_argument_parser()
self.command = self.parser.add_command(command_name)
self.command_name = command_name
# TODO: figure out how to support this on windows
self.subprocess = subprocess if sys.platform != "win32" else False
def __call__(self, *argv, **kwargs):
"""Invoke this SpackCommand.
@@ -656,25 +661,36 @@ def __call__(self, *argv, **kwargs):
self.error = None
prepend = kwargs["global_args"] if "global_args" in kwargs else []
args, unknown = self.parser.parse_known_args(prepend + [self.command_name] + list(argv))
fail_on_error = kwargs.get("fail_on_error", True)
out = StringIO()
try:
with log_output(out):
self.returncode = _invoke_command(self.command, self.parser, args, unknown)
if self.subprocess:
p = sp.Popen(
[spack.paths.spack_script, self.command_name] + prepend + list(argv),
stdout=sp.PIPE,
stderr=sp.STDOUT,
)
out, self.returncode = p.communicate()
out = out.decode()
else:
args, unknown = self.parser.parse_known_args(
prepend + [self.command_name] + list(argv)
)
except SystemExit as e:
self.returncode = e.code
out = StringIO()
try:
with log_output(out):
self.returncode = _invoke_command(self.command, self.parser, args, unknown)
except BaseException as e:
tty.debug(e)
self.error = e
if fail_on_error:
self._log_command_output(out)
raise
except SystemExit as e:
self.returncode = e.code
except BaseException as e:
tty.debug(e)
self.error = e
if fail_on_error:
self._log_command_output(out)
raise
out = out.getvalue()
if fail_on_error and self.returncode not in (None, 0):
self._log_command_output(out)
@@ -683,7 +699,7 @@ def __call__(self, *argv, **kwargs):
% (self.returncode, self.command_name, ", ".join("'%s'" % a for a in argv))
)
return out.getvalue()
return out
def _log_command_output(self, out):
if tty.is_verbose():

1
lib/spack/spack/test/ci.py
View File

@@ -484,6 +484,7 @@ def test_get_spec_filter_list(mutable_mock_env_path, config, mutable_mock_repo):
assert affected_pkg_names == expected_affected_pkg_names
@pytest.mark.maybeslow
@pytest.mark.regression("29947")
def test_affected_specs_on_first_concretization(mutable_mock_env_path, config):
e = ev.create("first_concretization")

4
lib/spack/spack/test/cmd/checksum.py
View File

@@ -50,12 +50,14 @@ def test_checksum(arguments, expected, mock_packages, mock_stage):
@pytest.mark.skipif(sys.platform == "win32", reason="Not supported on Windows (yet)")
def test_checksum_interactive(mock_packages, mock_fetch, mock_stage, monkeypatch):
# TODO: mock_fetch doesn't actually work with stage, working around with ignoring
# fail_on_error for now
def _get_number(*args, **kwargs):
return 1
monkeypatch.setattr(tty, "get_number", _get_number)
output = spack_checksum("preferred-test")
output = spack_checksum("preferred-test", fail_on_error=False)
assert "version of preferred-test" in output
assert "version(" in output

4
lib/spack/spack/test/cmd/ci.py
View File

@@ -2192,10 +2192,8 @@ def fake_download_and_extract_artifacts(url, work_dir):
)
def test_ci_help(subcmd, capsys):
"""Make sure `spack ci` --help describes the (sub)command help."""
with pytest.raises(SystemExit):
ci_cmd(subcmd, "--help")
out = spack.main.SpackCommand("ci", subprocess=True)(subcmd, "--help")
out = str(capsys.readouterr())
usage = "usage: spack ci {0}{1}[".format(subcmd, " " if subcmd else "")
assert usage in out

12
lib/spack/spack/test/cmd/commands.py
View File

@@ -16,7 +16,7 @@
import spack.paths
from spack.cmd.commands import _positional_to_subroutine
commands = spack.main.SpackCommand("commands")
commands = spack.main.SpackCommand("commands", subprocess=True)
parser = spack.main.make_argument_parser()
spack.main.add_all_commands(parser)
@@ -104,17 +104,18 @@ def test_rst_with_input_files(tmpdir):
def test_rst_with_header(tmpdir):
local_commands = spack.main.SpackCommand("commands")
fake_header = "this is a header!\n\n"
filename = tmpdir.join("header.txt")
with filename.open("w") as f:
f.write(fake_header)
out = commands("--format=rst", "--header", str(filename))
out = local_commands("--format=rst", "--header", str(filename))
assert out.startswith(fake_header)
with pytest.raises(spack.main.SpackCommandError):
commands("--format=rst", "--header", "asdfjhkf")
local_commands("--format=rst", "--header", "asdfjhkf")
def test_rst_update(tmpdir):
@@ -207,13 +208,14 @@ def test_update_completion_arg(tmpdir, monkeypatch):
monkeypatch.setattr(spack.cmd.commands, "update_completion_args", mock_args)
local_commands = spack.main.SpackCommand("commands")
# ensure things fail if --update-completion isn't specified alone
with pytest.raises(spack.main.SpackCommandError):
commands("--update-completion", "-a")
local_commands("--update-completion", "-a")
# ensure arg is restored
assert "--update-completion" not in mock_bashfile.read()
commands("--update-completion")
local_commands("--update-completion")
assert "--update-completion" in mock_bashfile.read()

2
lib/spack/spack/test/cmd/env.py
View File

@@ -869,7 +869,7 @@ def test_env_with_included_config_var_path(packages_file):
config_real_path = substitute_path_variables(config_var_path)
fs.mkdirp(os.path.dirname(config_real_path))
fs.rename(packages_file.strpath, config_real_path)
shutil.move(packages_file.strpath, config_real_path)
assert os.path.exists(config_real_path)
with e:

6
lib/spack/spack/test/cmd/help.py
View File

@@ -11,7 +11,7 @@
@pytest.mark.xfail
def test_reuse_after_help():
"""Test `spack help` can be called twice with the same SpackCommand."""
help_cmd = SpackCommand("help")
help_cmd = SpackCommand("help", subprocess=True)
help_cmd()
# This second invocation will somehow fail because the parser no
@@ -30,14 +30,14 @@ def test_reuse_after_help():
def test_help():
"""Sanity check the help command to make sure it works."""
help_cmd = SpackCommand("help")
help_cmd = SpackCommand("help", subprocess=True)
out = help_cmd()
assert "These are common spack commands:" in out
def test_help_all():
"""Test the spack help --all flag"""
help_cmd = SpackCommand("help")
help_cmd = SpackCommand("help", subprocess=True)
out = help_cmd("--all")
assert "Complete list of spack commands:" in out

33
lib/spack/spack/test/cmd/is_git_repo.py
View File

@@ -10,7 +10,7 @@
import pytest
from llnl.util.filesystem import mkdirp
from llnl.util.filesystem import mkdirp, working_dir
import spack
from spack.util.executable import which
@@ -40,28 +40,29 @@ def check_git_version():
@pytest.fixture(scope="function")
def git_tmp_worktree(tmpdir):
def git_tmp_worktree(tmpdir, mock_git_version_info):
"""Create new worktree in a temporary folder and monkeypatch
spack.paths.prefix to point to it.
"""
# TODO: This is fragile and should be high priority for
# follow up fixes. 27021
# Path length is occasionally too long on Windows
# the following reduces the path length to acceptable levels
if sys.platform == "win32":
long_pth = str(tmpdir).split(os.path.sep)
tmp_worktree = os.path.sep.join(long_pth[:-1])
else:
tmp_worktree = str(tmpdir)
worktree_root = os.path.sep.join([tmp_worktree, "wrktree"])
with working_dir(mock_git_version_info[0]):
# TODO: This is fragile and should be high priority for
# follow up fixes. 27021
# Path length is occasionally too long on Windows
# the following reduces the path length to acceptable levels
if sys.platform == "win32":
long_pth = str(tmpdir).split(os.path.sep)
tmp_worktree = os.path.sep.join(long_pth[:-1])
else:
tmp_worktree = str(tmpdir)
worktree_root = os.path.sep.join([tmp_worktree, "wrktree"])
mkdirp(worktree_root)
mkdirp(worktree_root)
git("worktree", "add", "--detach", worktree_root, "HEAD")
git("worktree", "add", "--detach", worktree_root, "HEAD")
yield worktree_root
yield worktree_root
git("worktree", "remove", "--force", worktree_root)
git("worktree", "remove", "--force", worktree_root)
def test_is_git_repo_in_worktree(git_tmp_worktree):

4
lib/spack/spack/test/cmd/module.py
View File

@@ -210,7 +210,7 @@ def test_setdefault_command(mutable_database, mutable_config):
for k in preferred, other_spec:
assert os.path.exists(writers[k].layout.filename)
assert os.path.exists(link_name) and os.path.islink(link_name)
assert os.path.realpath(link_name) == writers[other_spec].layout.filename
assert os.path.realpath(link_name) == os.path.realpath(writers[other_spec].layout.filename)
# Reset the default to be the preferred spec
module("lmod", "setdefault", preferred)
@@ -219,4 +219,4 @@ def test_setdefault_command(mutable_database, mutable_config):
for k in preferred, other_spec:
assert os.path.exists(writers[k].layout.filename)
assert os.path.exists(link_name) and os.path.islink(link_name)
assert os.path.realpath(link_name) == writers[preferred].layout.filename
assert os.path.realpath(link_name) == os.path.realpath(writers[preferred].layout.filename)

50
lib/spack/spack/test/cmd/style.py
View File

@@ -46,22 +46,22 @@ def has_develop_branch():
@pytest.fixture(scope="function")
def flake8_package():
def flake8_package(tmpdir):
"""Style only checks files that have been modified. This fixture makes a small
change to the ``flake8`` mock package, yields the filename, then undoes the
change on cleanup.
"""
repo = spack.repo.Repo(spack.paths.mock_packages_path)
filename = repo.filename_for_package_name("flake8")
tmp = filename + ".tmp"
rel_path = os.path.dirname(os.path.relpath(filename, spack.paths.prefix))
tmp = tmpdir / rel_path / "flake8-ci-package.py"
tmp.ensure()
tmp = str(tmp)
try:
shutil.copy(filename, tmp)
package = FileFilter(filename)
package.filter("state = 'unmodified'", "state = 'modified'", string=True)
yield filename
finally:
shutil.move(tmp, filename)
shutil.copy(filename, tmp)
package = FileFilter(tmp)
package.filter("state = 'unmodified'", "state = 'modified'", string=True)
yield tmp
@pytest.fixture
@@ -71,20 +71,17 @@ def flake8_package_with_errors(scope="function"):
filename = repo.filename_for_package_name("flake8")
tmp = filename + ".tmp"
try:
shutil.copy(filename, tmp)
package = FileFilter(filename)
shutil.copy(filename, tmp)
package = FileFilter(tmp)
# this is a black error (quote style and spacing before/after operator)
package.filter('state = "unmodified"', "state = 'modified'", string=True)
# this is a black error (quote style and spacing before/after operator)
package.filter('state = "unmodified"', "state = 'modified'", string=True)
# this is an isort error (orderign) and a flake8 error (unused import)
package.filter(
"from spack.package import *", "from spack.package import *\nimport os", string=True
)
yield filename
finally:
shutil.move(tmp, filename)
# this is an isort error (orderign) and a flake8 error (unused import)
package.filter(
"from spack.package import *", "from spack.package import *\nimport os", string=True
)
yield tmp
def test_changed_files_from_git_rev_base(tmpdir, capfd):
@@ -125,7 +122,7 @@ def test_changed_no_base(tmpdir, capfd):
assert "This repository does not have a 'foobar'" in err
def test_changed_files_all_files(flake8_package):
def test_changed_files_all_files():
# it's hard to guarantee "all files", so do some sanity checks.
files = set(
[
@@ -139,13 +136,18 @@ def test_changed_files_all_files(flake8_package):
# a builtin package
zlib = spack.repo.path.get_pkg_class("zlib")
assert zlib.module.__file__ in files
zlib_file = zlib.module.__file__
if zlib_file.endswith("pyc"):
zlib_file = zlib_file[:-1]
assert zlib_file in files
# a core spack file
assert os.path.join(spack.paths.module_path, "spec.py") in files
# a mock package
assert flake8_package in files
repo = spack.repo.Repo(spack.paths.mock_packages_path)
filename = repo.filename_for_package_name("flake8")
assert filename in files
# this test
assert __file__ in files

11
lib/spack/spack/test/cmd_extensions.py
View File

@@ -228,14 +228,17 @@ def test_missing_command():
],
ids=["no_stem", "vacuous", "leading_hyphen", "basic_good", "trailing_slash", "hyphenated"],
)
def test_extension_naming(extension_path, expected_exception, config):
def test_extension_naming(tmpdir, extension_path, expected_exception, config):
"""Ensure that we are correctly validating configured extension paths
for conformity with the rules: the basename should match
``spack-<name>``; <name> may have embedded hyphens but not begin with one.
"""
with spack.config.override("config:extensions", [extension_path]):
with pytest.raises(expected_exception):
spack.cmd.get_module("no-such-command")
# NOTE: if the directory is a valid extension directory name the "vacuous" test will
# fail because it resolves to current working directory
with tmpdir.as_cwd():
with spack.config.override("config:extensions", [extension_path]):
with pytest.raises(expected_exception):
spack.cmd.get_module("no-such-command")
def test_missing_command_function(extension_creator, capsys):

6
lib/spack/spack/util/debug.py
View File

@@ -10,6 +10,7 @@
"""
import code
import io
import os
import pdb
import signal
@@ -53,7 +54,10 @@ class and use as a drop in for Pdb, although the syntax here is slightly differe
the run of Spack.install, or any where else Spack spawns a child process.
"""
_original_stdin_fd = sys.stdin.fileno()
try:
_original_stdin_fd = sys.stdin.fileno()
except io.UnsupportedOperation:
_original_stdin_fd = None
_original_stdin = None
def __init__(self, stdout_fd=None, stderr_fd=None):