From 762ba27036e717aeceeb1b43f3090f0e9a049869 Mon Sep 17 00:00:00 2001 From: Tom Scogland Date: Wed, 7 Sep 2022 11:12:57 -0700 Subject: Make GHA tests parallel by using xdist (#32361) * 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 --- .flake8 | 1 + .github/workflows/audit.yaml | 44 +++++++++++++++++++++++++++++ .github/workflows/bootstrap.yml | 2 +- .github/workflows/build-containers.yml | 2 +- .github/workflows/ci.yaml | 25 +++++++++++++++-- .github/workflows/unit_tests.yaml | 33 ++++++++++++++++------ .github/workflows/valid-style.yml | 34 ++++------------------ .github/workflows/windows_python.yml | 2 +- bin/spack-tmpconfig | 1 - lib/spack/llnl/util/tty/pty.py | 4 +-- lib/spack/spack/cmd/python.py | 6 ++++ lib/spack/spack/main.py | 50 ++++++++++++++++++++++----------- lib/spack/spack/test/ci.py | 1 + lib/spack/spack/test/cmd/checksum.py | 4 ++- lib/spack/spack/test/cmd/ci.py | 4 +-- lib/spack/spack/test/cmd/commands.py | 12 ++++---- lib/spack/spack/test/cmd/env.py | 2 +- lib/spack/spack/test/cmd/help.py | 6 ++-- lib/spack/spack/test/cmd/is_git_repo.py | 33 +++++++++++----------- lib/spack/spack/test/cmd/module.py | 4 +-- lib/spack/spack/test/cmd/style.py | 50 +++++++++++++++++---------------- lib/spack/spack/test/cmd_extensions.py | 11 +++++--- lib/spack/spack/util/debug.py | 6 +++- share/spack/qa/run-unit-tests | 5 ++++ share/spack/spack-completion.bash | 2 +- 25 files changed, 221 insertions(+), 123 deletions(-) create mode 100644 .github/workflows/audit.yaml diff --git a/.flake8 b/.flake8 index 226e694894..ddd6b6c2fd 100644 --- a/.flake8 +++ b/.flake8 @@ -29,6 +29,7 @@ max-line-length = 99 # per-file-ignores = var/spack/repos/*/package.py:F403,F405,F821 + *-ci-package.py:F403,F405,F821 # exclude things we usually do not want linting for. # These still get linted when passed explicitly, as when spack flake8 passes diff --git a/.github/workflows/audit.yaml b/.github/workflows/audit.yaml new file mode 100644 index 0000000000..2b8c989518 --- /dev/null +++ b/.github/workflows/audit.yaml @@ -0,0 +1,44 @@ +name: audit + +on: + workflow_call: + inputs: + with_coverage: + required: true + type: string + python_version: + required: true + type: string + +concurrency: + group: audit-${{inputs.python_version}}-${{github.ref}}-${{github.event.pull_request.number || github.run_number}} + cancel-in-progress: true + +jobs: + # Run audits on all the packages in the built-in repository + package-audits: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@2541b1294d2704b0964813337f33b291d3f8596b # @v2 + - uses: actions/setup-python@b55428b1882923874294fa556849718a1d7f2ca5 # @v2 + with: + python-version: ${{inputs.python_version}} + - name: Install Python packages + run: | + pip install --upgrade pip six setuptools pytest codecov 'coverage[toml]<=6.2' + - name: Package audits (with coverage) + if: ${{ inputs.with_coverage == 'true' }} + run: | + . share/spack/setup-env.sh + coverage run $(which spack) audit packages + coverage combine + coverage xml + - name: Package audits (without coverage) + if: ${{ inputs.with_coverage == 'false' }} + run: | + . share/spack/setup-env.sh + $(which spack) audit packages + - uses: codecov/codecov-action@81cd2dc8148241f03f5839d295e000b8f761e378 # @v2.1.0 + if: ${{ inputs.with_coverage == 'true' }} + with: + flags: unittests,linux,audits diff --git a/.github/workflows/bootstrap.yml b/.github/workflows/bootstrap.yml index 138a9c7382..e649de2fb4 100644 --- a/.github/workflows/bootstrap.yml +++ b/.github/workflows/bootstrap.yml @@ -9,7 +9,7 @@ on: - cron: '16 2 * * *' concurrency: - group: bootstrap-${{ github.workflow }}-${{ github.event.pull_request.number || github.run_number }} + group: bootstrap-${{github.ref}}-${{github.event.pull_request.number || github.run_number}} cancel-in-progress: true jobs: diff --git a/.github/workflows/build-containers.yml b/.github/workflows/build-containers.yml index f849747995..30765cfc86 100644 --- a/.github/workflows/build-containers.yml +++ b/.github/workflows/build-containers.yml @@ -20,7 +20,7 @@ on: types: [published] concurrency: - group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_number }} + group: build_containers-${{github.ref}}-${{github.event.pull_request.number || github.run_number}} cancel-in-progress: true jobs: diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 62f2be080f..f4943789d6 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -11,7 +11,7 @@ on: - releases/** concurrency: - group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_number }} + group: ci-${{github.ref}}-${{github.event.pull_request.number || github.run_number}} cancel-in-progress: true jobs: @@ -20,6 +20,18 @@ jobs: uses: ./.github/workflows/valid-style.yml with: with_coverage: ${{ needs.changes.outputs.with_coverage }} + audit-ancient-python: + uses: ./.github/workflows/audit.yaml + needs: [ changes ] + with: + with_coverage: ${{ needs.changes.outputs.with_coverage }} + python_version: 2.7 + all-prechecks: + needs: [ prechecks ] + runs-on: ubuntu-latest + steps: + - name: Success + run: "true" # Check which files have been updated by the PR changes: runs-on: ubuntu-latest @@ -53,6 +65,7 @@ jobs: - 'share/spack/**' - '.github/workflows/bootstrap.yml' core: + - '!lib/spack/docs/**' - './!(var/**)/**' packages: - 'var/**' @@ -79,7 +92,7 @@ jobs: needs: [ prechecks, changes ] uses: ./.github/workflows/bootstrap.yml unit-tests: - if: ${{ github.repository == 'spack/spack' }} + if: ${{ github.repository == 'spack/spack' && needs.changes.outputs.core == 'true' }} needs: [ prechecks, changes ] uses: ./.github/workflows/unit_tests.yaml with: @@ -87,7 +100,13 @@ jobs: packages: ${{ needs.changes.outputs.packages }} with_coverage: ${{ needs.changes.outputs.with_coverage }} windows: - if: ${{ github.repository == 'spack/spack' }} + if: ${{ github.repository == 'spack/spack' && needs.changes.outputs.core == 'true' }} needs: [ prechecks ] uses: ./.github/workflows/windows_python.yml + all: + needs: [ windows, unit-tests, bootstrap, audit-ancient-python ] + runs-on: ubuntu-latest + steps: + - name: Success + run: "true" diff --git a/.github/workflows/unit_tests.yaml b/.github/workflows/unit_tests.yaml index 3856f1039f..6b1411c61b 100644 --- a/.github/workflows/unit_tests.yaml +++ b/.github/workflows/unit_tests.yaml @@ -1,6 +1,7 @@ name: unit tests on: + workflow_dispatch: workflow_call: inputs: core: @@ -14,7 +15,7 @@ on: type: string concurrency: - group: unit_tests-${{ github.workflow }}-${{ github.event.pull_request.number || github.run_number }} + group: unit_tests-${{github.ref}}-${{github.event.pull_request.number || github.run_number}} cancel-in-progress: true jobs: @@ -25,11 +26,26 @@ jobs: matrix: python-version: ['2.7', '3.6', '3.7', '3.8', '3.9', '3.10'] concretizer: ['clingo'] + on_develop: + - ${{ github.ref == 'refs/heads/develop' }} include: - python-version: 2.7 concretizer: original - - python-version: 3.9 + on_develop: false + - python-version: '3.10' concretizer: original + on_develop: false + exclude: + - python-version: '3.7' + concretizer: 'clingo' + on_develop: false + - python-version: '3.8' + concretizer: 'clingo' + on_develop: false + - python-version: '3.9' + concretizer: 'clingo' + on_develop: false + steps: - uses: actions/checkout@2541b1294d2704b0964813337f33b291d3f8596b # @v2 with: @@ -46,7 +62,7 @@ jobs: patchelf cmake bison libbison-dev kcov - name: Install Python packages run: | - pip install --upgrade pip six setuptools pytest codecov "coverage[toml]<=6.2" + pip install --upgrade pip six setuptools pytest codecov "coverage[toml]<=6.2" pytest-xdist # ensure style checks are not skipped in unit tests for python >= 3.6 # note that true/false (i.e., 1/0) are opposite in conditions in python and bash if python -c 'import sys; sys.exit(not sys.version_info >= (3, 6))'; then @@ -108,7 +124,7 @@ jobs: sudo apt-get install -y coreutils kcov csh zsh tcsh fish dash bash - name: Install Python packages run: | - pip install --upgrade pip six setuptools pytest codecov coverage[toml]==6.2 + pip install --upgrade pip six setuptools pytest codecov coverage[toml]==6.2 pytest-xdist - name: Setup git configuration run: | # Need this for the git tests to succeed. @@ -174,7 +190,7 @@ jobs: patchelf kcov - name: Install Python packages run: | - pip install --upgrade pip six setuptools pytest codecov coverage[toml]==6.2 clingo + pip install --upgrade pip six setuptools pytest codecov coverage[toml]==6.2 clingo pytest-xdist - name: Setup git configuration run: | # Need this for the git tests to succeed. @@ -216,7 +232,7 @@ jobs: - name: Install Python packages run: | pip install --upgrade pip six setuptools - pip install --upgrade pytest codecov coverage[toml]==6.2 + pip install --upgrade pytest codecov coverage[toml]==6.2 pytest-xdist - name: Setup Homebrew packages run: | brew install dash fish gcc gnupg2 kcov @@ -229,9 +245,10 @@ jobs: . share/spack/setup-env.sh $(which spack) bootstrap untrust spack-install $(which spack) solve zlib + common_args=(--dist loadfile --tx '4*popen//python=./bin/spack-tmpconfig python -u ./bin/spack python' -x) if [ "${{ inputs.with_coverage }}" == "true" ] then - coverage run $(which spack) unit-test -x + coverage run $(which spack) unit-test "${common_args[@]}" coverage combine coverage xml # Delete the symlink going from ./lib/spack/docs/_spack_root back to @@ -239,7 +256,7 @@ jobs: rm lib/spack/docs/_spack_root else echo "ONLY PACKAGE RECIPES CHANGED [skipping coverage]" - $(which spack) unit-test -x -m "not maybeslow" -k "test_all_virtual_packages_have_default_providers" + $(which spack) unit-test "${common_args[@]}" -m "not maybeslow" -k "test_all_virtual_packages_have_default_providers" fi - uses: codecov/codecov-action@81cd2dc8148241f03f5839d295e000b8f761e378 # @v2.1.0 if: ${{ inputs.with_coverage == 'true' }} diff --git a/.github/workflows/valid-style.yml b/.github/workflows/valid-style.yml index 73c8dc76f1..2519fd77a0 100644 --- a/.github/workflows/valid-style.yml +++ b/.github/workflows/valid-style.yml @@ -8,7 +8,7 @@ on: type: string concurrency: - group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_number }} + group: style-${{github.ref}}-${{github.event.pull_request.number || github.run_number}} cancel-in-progress: true @@ -53,30 +53,8 @@ jobs: - name: Run style tests run: | share/spack/qa/run-style-tests - # Run audits on all the packages in the built-in repository - package-audits: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@2541b1294d2704b0964813337f33b291d3f8596b # @v2 - - uses: actions/setup-python@b55428b1882923874294fa556849718a1d7f2ca5 # @v2 - with: - python-version: '3.10' - - name: Install Python packages - run: | - pip install --upgrade pip six setuptools pytest codecov coverage[toml]==6.2 - - name: Package audits (with coverage) - if: ${{ inputs.with_coverage == 'true' }} - run: | - . share/spack/setup-env.sh - coverage run $(which spack) audit packages - coverage combine - coverage xml - - name: Package audits (without coverage) - if: ${{ inputs.with_coverage == 'false' }} - run: | - . share/spack/setup-env.sh - $(which spack) audit packages - - uses: codecov/codecov-action@81cd2dc8148241f03f5839d295e000b8f761e378 # @v2.1.0 - if: ${{ inputs.with_coverage == 'true' }} - with: - flags: unittests,linux,audits + audit: + uses: ./.github/workflows/audit.yaml + with: + with_coverage: ${{ inputs.with_coverage }} + python_version: '3.10' diff --git a/.github/workflows/windows_python.yml b/.github/workflows/windows_python.yml index 5bd009df44..82e48370ac 100644 --- a/.github/workflows/windows_python.yml +++ b/.github/workflows/windows_python.yml @@ -4,7 +4,7 @@ on: workflow_call: concurrency: - group: windows-${{ github.workflow }}-${{ github.event.pull_request.number || github.run_number }} + group: windows-${{github.ref}}-${{github.event.pull_request.number || github.run_number}} cancel-in-progress: true defaults: diff --git a/bin/spack-tmpconfig b/bin/spack-tmpconfig index b9cca15cc0..a3d053d340 100755 --- a/bin/spack-tmpconfig +++ b/bin/spack-tmpconfig @@ -7,7 +7,6 @@ export TMPDIR="${XDG_RUNTIME_DIR}" export TMP_DIR="$(mktemp -d -t spack-test-XXXXX)" clean_up() { [[ -n "$TMPCONFIG_DEBUG" ]] && printf "cleaning up: $TMP_DIR\n" - [[ -n "$TMPCONFIG_DEBUG" ]] && tree "$TMP_DIR" rm -rf "$TMP_DIR" } trap clean_up EXIT diff --git a/lib/spack/llnl/util/tty/pty.py b/lib/spack/llnl/util/tty/pty.py index 8d7213a533..df743a6055 100644 --- a/lib/spack/llnl/util/tty/pty.py +++ b/lib/spack/llnl/util/tty/pty.py @@ -228,8 +228,8 @@ class PseudoShell(object): 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. diff --git a/lib/spack/spack/cmd/python.py b/lib/spack/spack/cmd/python.py index 028184b817..5df8b30dde 100644 --- a/lib/spack/spack/cmd/python.py +++ b/lib/spack/spack/cmd/python.py @@ -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", diff --git a/lib/spack/spack/main.py b/lib/spack/spack/main.py index f69b1e0088..78a0d47519 100644 --- a/lib/spack/spack/main.py +++ b/lib/spack/spack/main.py @@ -18,6 +18,7 @@ import os.path 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 @@ class SpackCommand(object): 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) + ) + + out = StringIO() + try: + with log_output(out): + self.returncode = _invoke_command(self.command, self.parser, args, unknown) - except SystemExit as e: - self.returncode = e.code + 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 + 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 @@ class SpackCommand(object): % (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(): diff --git a/lib/spack/spack/test/ci.py b/lib/spack/spack/test/ci.py index f42518f7fb..2d8e65e8a5 100644 --- a/lib/spack/spack/test/ci.py +++ b/lib/spack/spack/test/ci.py @@ -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") diff --git a/lib/spack/spack/test/cmd/checksum.py b/lib/spack/spack/test/cmd/checksum.py index b0fffbea08..a84b0eb8b7 100644 --- a/lib/spack/spack/test/cmd/checksum.py +++ b/lib/spack/spack/test/cmd/checksum.py @@ -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 diff --git a/lib/spack/spack/test/cmd/ci.py b/lib/spack/spack/test/cmd/ci.py index 0ac352c1df..e7e7569fe9 100644 --- a/lib/spack/spack/test/cmd/ci.py +++ b/lib/spack/spack/test/cmd/ci.py @@ -2192,10 +2192,8 @@ spack: ) 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 diff --git a/lib/spack/spack/test/cmd/commands.py b/lib/spack/spack/test/cmd/commands.py index 23794a6bb0..d972f86d7f 100644 --- a/lib/spack/spack/test/cmd/commands.py +++ b/lib/spack/spack/test/cmd/commands.py @@ -16,7 +16,7 @@ import spack.main 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 @@ _cmd-spack-install: 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() diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index ceefd3323b..f564ca3c80 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -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: diff --git a/lib/spack/spack/test/cmd/help.py b/lib/spack/spack/test/cmd/help.py index ed7f872275..99f1ac7ed3 100644 --- a/lib/spack/spack/test/cmd/help.py +++ b/lib/spack/spack/test/cmd/help.py @@ -11,7 +11,7 @@ from spack.main import SpackCommand @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 diff --git a/lib/spack/spack/test/cmd/is_git_repo.py b/lib/spack/spack/test/cmd/is_git_repo.py index e094476a2e..27fb15cd5a 100644 --- a/lib/spack/spack/test/cmd/is_git_repo.py +++ b/lib/spack/spack/test/cmd/is_git_repo.py @@ -10,7 +10,7 @@ import sys 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 @@ pytestmark = pytest.mark.skipif( @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): diff --git a/lib/spack/spack/test/cmd/module.py b/lib/spack/spack/test/cmd/module.py index 1af511b85c..5ec99b0c64 100644 --- a/lib/spack/spack/test/cmd/module.py +++ b/lib/spack/spack/test/cmd/module.py @@ -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) diff --git a/lib/spack/spack/test/cmd/style.py b/lib/spack/spack/test/cmd/style.py index dec795ca99..179973af52 100644 --- a/lib/spack/spack/test/cmd/style.py +++ b/lib/spack/spack/test/cmd/style.py @@ -46,22 +46,22 @@ skip_old_python = pytest.mark.skipif( @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 diff --git a/lib/spack/spack/test/cmd_extensions.py b/lib/spack/spack/test/cmd_extensions.py index e68ac434fc..6947725afa 100644 --- a/lib/spack/spack/test/cmd_extensions.py +++ b/lib/spack/spack/test/cmd_extensions.py @@ -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-``; 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): diff --git a/lib/spack/spack/util/debug.py b/lib/spack/spack/util/debug.py index eb3b59a4fc..bdeee2df6c 100644 --- a/lib/spack/spack/util/debug.py +++ b/lib/spack/spack/util/debug.py @@ -10,6 +10,7 @@ a stack trace and drops the user into an interpreter. """ import code +import io import os import pdb import signal @@ -53,7 +54,10 @@ class ForkablePdb(pdb.Pdb): 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): diff --git a/share/spack/qa/run-unit-tests b/share/spack/qa/run-unit-tests index 6222cbccd9..62cca273bf 100755 --- a/share/spack/qa/run-unit-tests +++ b/share/spack/qa/run-unit-tests @@ -54,6 +54,11 @@ elif [[ "$SPACK_TEST_SOLVER" == "original" ]]; then export PYTEST_ADDOPTS='-m "not maybeslow"' fi +# Check if xdist is available +if python -m pytest --trace-config 2>&1 | grep xdist; then + export PYTEST_ADDOPTS="$PYTEST_ADDOPTS --dist loadfile --tx '${SPACK_TEST_PARALLEL:=3}*popen//python=./bin/spack-tmpconfig python -u ./bin/spack python'" +fi + $coverage_run $(which spack) unit-test -x --verbose bash "$QA_DIR/test-env-cfg.sh" diff --git a/share/spack/spack-completion.bash b/share/spack/spack-completion.bash index bde48d8e97..c91b888f09 100755 --- a/share/spack/spack-completion.bash +++ b/share/spack/spack-completion.bash @@ -1568,7 +1568,7 @@ _spack_pydoc() { _spack_python() { if $list_options then - SPACK_COMPREPLY="-h --help -V --version -c -i -m --path" + SPACK_COMPREPLY="-h --help -V --version -c -u -i -m --path" else SPACK_COMPREPLY="" fi -- cgit v1.2.3-60-g2f50