From 64774f30155f701fb1961e11e38ea3af6d8513b3 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Fri, 16 Aug 2024 10:11:05 +0200 Subject: Skip test_foreground_background + other minor cleanups The test_foreground_background unit test has been marked xfail for a while, meaning: - Nobody looks at the results of the test - It still runs every time That test happens to hang frequently on some Apple M1 I have access to, so here I mark it as skip. Also went through other xfailing and skipped tests, and applied minor changes. --- lib/spack/spack/test/cmd/commands.py | 17 ----------------- lib/spack/spack/test/cmd/external.py | 2 +- lib/spack/spack/test/cmd/help.py | 16 ---------------- lib/spack/spack/test/cmd/style.py | 26 ++++++++++++++++---------- lib/spack/spack/test/container/cli.py | 2 +- lib/spack/spack/test/installer.py | 2 +- lib/spack/spack/test/llnl/util/filesystem.py | 2 +- lib/spack/spack/test/llnl/util/tty/log.py | 4 ++-- lib/spack/spack/test/make_executable.py | 5 +---- lib/spack/spack/test/spack_yaml.py | 4 +--- 10 files changed, 24 insertions(+), 56 deletions(-) diff --git a/lib/spack/spack/test/cmd/commands.py b/lib/spack/spack/test/cmd/commands.py index 68d28f5684..b7cc59e115 100644 --- a/lib/spack/spack/test/cmd/commands.py +++ b/lib/spack/spack/test/cmd/commands.py @@ -6,7 +6,6 @@ import filecmp import os import shutil -import subprocess import pytest @@ -156,22 +155,6 @@ def test_update_with_header(tmpdir): commands("--update", str(update_file), "--header", str(filename)) -@pytest.mark.xfail -def test_no_pipe_error(): - """Make sure we don't see any pipe errors when piping output.""" - - proc = subprocess.Popen( - ["spack", "commands", "--format=rst"], stdout=subprocess.PIPE, stderr=subprocess.PIPE - ) - - # Call close() on stdout to cause a broken pipe - proc.stdout.close() - proc.wait() - stderr = proc.stderr.read().decode("utf-8") - - assert "Broken pipe" not in stderr - - def test_bash_completion(): """Test the bash completion writer.""" out1 = commands("--format=bash") diff --git a/lib/spack/spack/test/cmd/external.py b/lib/spack/spack/test/cmd/external.py index d911f8adb5..86e6f41389 100644 --- a/lib/spack/spack/test/cmd/external.py +++ b/lib/spack/spack/test/cmd/external.py @@ -100,7 +100,7 @@ external = SpackCommand("external") # TODO: this test should be made to work, but in the meantime it is # causing intermittent (spurious) CI failures on all PRs -@pytest.mark.skipif(sys.platform == "win32", reason="Test fails intermittently on Windows") +@pytest.mark.not_on_windows("Test fails intermittently on Windows") def test_find_external_cmd_not_buildable(mutable_config, working_env, mock_executable): """When the user invokes 'spack external find --not-buildable', the config for any package where Spack finds an external version should be marked as diff --git a/lib/spack/spack/test/cmd/help.py b/lib/spack/spack/test/cmd/help.py index 688675be68..91303dbb6e 100644 --- a/lib/spack/spack/test/cmd/help.py +++ b/lib/spack/spack/test/cmd/help.py @@ -2,29 +2,13 @@ # Spack Project Developers. See the top-level COPYRIGHT file for details. # # SPDX-License-Identifier: (Apache-2.0 OR MIT) - -import pytest - 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", subprocess=True) help_cmd() - - # This second invocation will somehow fail because the parser no - # longer works after add_all_commands() is called in - # SpackArgumentParser.format_help_sections(). - # - # TODO: figure out why this doesn't work properly and change this - # test to use a single SpackCommand. - # - # It seems that parse_known_args() finds "too few arguments" the - # second time through b/c add_all_commands() ends up leaving extra - # positionals in the parser. But this used to work before we loaded - # commands lazily. help_cmd() diff --git a/lib/spack/spack/test/cmd/style.py b/lib/spack/spack/test/cmd/style.py index 5605181604..7444c970c1 100644 --- a/lib/spack/spack/test/cmd/style.py +++ b/lib/spack/spack/test/cmd/style.py @@ -24,6 +24,12 @@ style_data = os.path.join(spack.paths.test_path, "data", "style") style = spack.main.SpackCommand("style") +ISORT = which("isort") +BLACK = which("black") +FLAKE8 = which("flake8") +MYPY = which("mypy") + + @pytest.fixture(autouse=True) def has_develop_branch(git): """spack style requires git and a develop branch to run -- skip if we're missing either.""" @@ -190,8 +196,8 @@ def external_style_root(git, flake8_package_with_errors, tmpdir): yield tmpdir, py_file -@pytest.mark.skipif(not which("isort"), reason="isort is not installed.") -@pytest.mark.skipif(not which("black"), reason="black is not installed.") +@pytest.mark.skipif(not ISORT, reason="isort is not installed.") +@pytest.mark.skipif(not BLACK, reason="black is not installed.") def test_fix_style(external_style_root): """Make sure spack style --fix works.""" tmpdir, py_file = external_style_root @@ -209,10 +215,10 @@ def test_fix_style(external_style_root): assert filecmp.cmp(broken_py, fixed_py) -@pytest.mark.skipif(not which("flake8"), reason="flake8 is not installed.") -@pytest.mark.skipif(not which("isort"), reason="isort is not installed.") -@pytest.mark.skipif(not which("mypy"), reason="mypy is not installed.") -@pytest.mark.skipif(not which("black"), reason="black is not installed.") +@pytest.mark.skipif(not FLAKE8, reason="flake8 is not installed.") +@pytest.mark.skipif(not ISORT, reason="isort is not installed.") +@pytest.mark.skipif(not MYPY, reason="mypy is not installed.") +@pytest.mark.skipif(not BLACK, reason="black is not installed.") def test_external_root(external_style_root, capfd): """Ensure we can run in a separate root directory w/o configuration files.""" tmpdir, py_file = external_style_root @@ -238,7 +244,7 @@ def test_external_root(external_style_root, capfd): assert "lib/spack/spack/dummy.py:7: [F401] 'os' imported but unused" in output -@pytest.mark.skipif(not which("flake8"), reason="flake8 is not installed.") +@pytest.mark.skipif(not FLAKE8, reason="flake8 is not installed.") def test_style(flake8_package, tmpdir): root_relative = os.path.relpath(flake8_package, spack.paths.prefix) @@ -264,7 +270,7 @@ def test_style(flake8_package, tmpdir): assert "spack style checks were clean" in output -@pytest.mark.skipif(not which("flake8"), reason="flake8 is not installed.") +@pytest.mark.skipif(not FLAKE8, reason="flake8 is not installed.") def test_style_with_errors(flake8_package_with_errors): root_relative = os.path.relpath(flake8_package_with_errors, spack.paths.prefix) output = style( @@ -275,8 +281,8 @@ def test_style_with_errors(flake8_package_with_errors): assert "spack style found errors" in output -@pytest.mark.skipif(not which("black"), reason="black is not installed.") -@pytest.mark.skipif(not which("flake8"), reason="flake8 is not installed.") +@pytest.mark.skipif(not BLACK, reason="black is not installed.") +@pytest.mark.skipif(not FLAKE8, reason="flake8 is not installed.") def test_style_with_black(flake8_package_with_errors): output = style("--tool", "black,flake8", flake8_package_with_errors, fail_on_error=False) assert "black found errors" in output diff --git a/lib/spack/spack/test/container/cli.py b/lib/spack/spack/test/container/cli.py index 3cb4ed05a8..fa57eac723 100644 --- a/lib/spack/spack/test/container/cli.py +++ b/lib/spack/spack/test/container/cli.py @@ -27,7 +27,7 @@ def test_listing_possible_os(): assert expected_os in output -@pytest.mark.skipif(str(spack.platforms.host()) == "windows", reason="test unsupported on Windows") +@pytest.mark.not_on_windows("test unsupported on Windows") @pytest.mark.maybeslow @pytest.mark.requires_executables("git") def test_bootstrap_phase(minimal_configuration, config_dumper, capsys): diff --git a/lib/spack/spack/test/installer.py b/lib/spack/spack/test/installer.py index 1ca3982dd6..1af1fb1d90 100644 --- a/lib/spack/spack/test/installer.py +++ b/lib/spack/spack/test/installer.py @@ -582,7 +582,7 @@ def test_clear_failures_success(tmpdir): assert os.path.isfile(failures.locker.lock_path) -@pytest.mark.xfail(sys.platform == "win32", reason="chmod does not prevent removal on Win") +@pytest.mark.not_on_windows("chmod does not prevent removal on Win") def test_clear_failures_errs(tmpdir, capsys): """Test the clear_failures exception paths.""" failures = spack.database.FailureTracker(str(tmpdir), default_timeout=0.1) diff --git a/lib/spack/spack/test/llnl/util/filesystem.py b/lib/spack/spack/test/llnl/util/filesystem.py index d9e34d5009..7803ecaf8c 100644 --- a/lib/spack/spack/test/llnl/util/filesystem.py +++ b/lib/spack/spack/test/llnl/util/filesystem.py @@ -274,7 +274,7 @@ class TestInstallTree: assert not os.path.islink("dest/2") check_added_exe_permissions("source/2", "dest/2") - @pytest.mark.skipif(sys.platform == "win32", reason="Broken symlinks not allowed on Windows") + @pytest.mark.not_on_windows("Broken symlinks not allowed on Windows") def test_allow_broken_symlinks(self, stage): """Test installing with a broken symlink.""" with fs.working_dir(str(stage)): diff --git a/lib/spack/spack/test/llnl/util/tty/log.py b/lib/spack/spack/test/llnl/util/tty/log.py index 3ff3ee995c..ea03fd2689 100644 --- a/lib/spack/spack/test/llnl/util/tty/log.py +++ b/lib/spack/spack/test/llnl/util/tty/log.py @@ -348,7 +348,7 @@ def no_termios(): (mock_shell_tstp_tstp_cont_cont, no_termios), ], ) -@pytest.mark.xfail(reason="Fails almost consistently when run with coverage and xdist") +@pytest.mark.skip(reason="Fails almost consistently when run with coverage and xdist") def test_foreground_background(test_fn, termios_on_or_off, tmpdir): """Functional tests for foregrounding and backgrounding a logged process. @@ -465,7 +465,7 @@ def mock_shell_v_v_no_termios(proc, ctl, **kwargs): "test_fn,termios_on_or_off", [(mock_shell_v_v, lang.nullcontext), (mock_shell_v_v_no_termios, no_termios)], ) -@pytest.mark.xfail(reason="Fails almost consistently when run with coverage and xdist") +@pytest.mark.skip(reason="Fails almost consistently when run with coverage and xdist") def test_foreground_background_output(test_fn, capfd, termios_on_or_off, tmpdir): """Tests hitting 'v' toggles output, and that force_echo works.""" if sys.version_info >= (3, 8) and sys.platform == "darwin" and termios_on_or_off == no_termios: diff --git a/lib/spack/spack/test/make_executable.py b/lib/spack/spack/test/make_executable.py index 0dd0109e90..3090f7be5b 100644 --- a/lib/spack/spack/test/make_executable.py +++ b/lib/spack/spack/test/make_executable.py @@ -9,16 +9,13 @@ Tests for Spack's built-in parallel make support. This just tests whether the right args are getting passed to make. """ import os -import sys import pytest from spack.build_environment import MakeExecutable from spack.util.environment import path_put_first -pytestmark = pytest.mark.skipif( - sys.platform == "win32", reason="MakeExecutable not supported on Windows" -) +pytestmark = pytest.mark.not_on_windows("MakeExecutable not supported on Windows") @pytest.fixture(autouse=True) diff --git a/lib/spack/spack/test/spack_yaml.py b/lib/spack/spack/test/spack_yaml.py index 4f3c035724..738d5f91b8 100644 --- a/lib/spack/spack/test/spack_yaml.py +++ b/lib/spack/spack/test/spack_yaml.py @@ -2,10 +2,8 @@ # Spack Project Developers. See the top-level COPYRIGHT file for details. # # SPDX-License-Identifier: (Apache-2.0 OR MIT) - """Test Spack's custom YAML format.""" import io -import sys import pytest @@ -126,7 +124,7 @@ def test_yaml_aliases(): ), ], ) -@pytest.mark.xfail(sys.platform == "win32", reason="fails on Windows") +@pytest.mark.not_on_windows(reason="fails on Windows") def test_round_trip_configuration(initial_content, expected_final_content, tmp_path): """Test that configuration can be loaded and dumped without too many changes""" file = tmp_path / "test.yaml" -- cgit v1.2.3-70-g09d2