From 8d0e0d5c77b74b964021f3d7f7930c7ae5a786f2 Mon Sep 17 00:00:00 2001 From: Harmen Stoppels Date: Mon, 27 Nov 2023 16:37:31 +0100 Subject: [PATCH] tests: fix more cases of env variables (#41226) --- lib/spack/spack/test/cmd/gpg.py | 4 +- lib/spack/spack/test/cmd/install.py | 11 +- lib/spack/spack/test/make_executable.py | 161 +++++++++++------------- 3 files changed, 83 insertions(+), 93 deletions(-) diff --git a/lib/spack/spack/test/cmd/gpg.py b/lib/spack/spack/test/cmd/gpg.py index 78a2a9ece9..08749022ca 100644 --- a/lib/spack/spack/test/cmd/gpg.py +++ b/lib/spack/spack/test/cmd/gpg.py @@ -43,7 +43,7 @@ def test_find_gpg(cmd_name, version, tmpdir, mock_gnupghome, monkeypatch): f.write(TEMPLATE.format(version=version)) fs.set_executable(fname) - monkeypatch.setitem(os.environ, "PATH", str(tmpdir)) + monkeypatch.setenv("PATH", str(tmpdir)) if version == "undetectable" or version.endswith("1.3.4"): with pytest.raises(spack.util.gpg.SpackGPGError): spack.util.gpg.init(force=True) @@ -54,7 +54,7 @@ def test_find_gpg(cmd_name, version, tmpdir, mock_gnupghome, monkeypatch): def test_no_gpg_in_path(tmpdir, mock_gnupghome, monkeypatch, mutable_config): - monkeypatch.setitem(os.environ, "PATH", str(tmpdir)) + monkeypatch.setenv("PATH", str(tmpdir)) bootstrap("disable") with pytest.raises(RuntimeError): spack.util.gpg.init(force=True) diff --git a/lib/spack/spack/test/cmd/install.py b/lib/spack/spack/test/cmd/install.py index ef9d19d778..8eb3bfedb8 100644 --- a/lib/spack/spack/test/cmd/install.py +++ b/lib/spack/spack/test/cmd/install.py @@ -904,13 +904,12 @@ def test_install_help_cdash(): @pytest.mark.disable_clean_stage_check -def test_cdash_auth_token(tmpdir, mock_fetch, install_mockery, capfd): +def test_cdash_auth_token(tmpdir, mock_fetch, install_mockery, monkeypatch, capfd): # capfd interferes with Spack's capturing - with tmpdir.as_cwd(): - with capfd.disabled(): - os.environ["SPACK_CDASH_AUTH_TOKEN"] = "asdf" - out = install("-v", "--log-file=cdash_reports", "--log-format=cdash", "a") - assert "Using CDash auth token from environment" in out + with tmpdir.as_cwd(), capfd.disabled(): + monkeypatch.setenv("SPACK_CDASH_AUTH_TOKEN", "asdf") + out = install("-v", "--log-file=cdash_reports", "--log-format=cdash", "a") + assert "Using CDash auth token from environment" in out @pytest.mark.not_on_windows("Windows log_output logs phase header out of order") diff --git a/lib/spack/spack/test/make_executable.py b/lib/spack/spack/test/make_executable.py index b333ae58fe..5716ca5a48 100644 --- a/lib/spack/spack/test/make_executable.py +++ b/lib/spack/spack/test/make_executable.py @@ -9,10 +9,7 @@ This just tests whether the right args are getting passed to make. """ import os -import shutil import sys -import tempfile -import unittest import pytest @@ -20,110 +17,104 @@ from spack.util.environment import path_put_first pytestmark = pytest.mark.skipif( - sys.platform == "win32", - reason="MakeExecutable \ - not supported on Windows", + sys.platform == "win32", reason="MakeExecutable not supported on Windows" ) -class MakeExecutableTest(unittest.TestCase): - def setUp(self): - self.tmpdir = tempfile.mkdtemp() +@pytest.fixture(autouse=True) +def make_executable(tmp_path, working_env): + make_exe = tmp_path / "make" + with open(make_exe, "w") as f: + f.write("#!/bin/sh\n") + f.write('echo "$@"') + os.chmod(make_exe, 0o700) - make_exe = os.path.join(self.tmpdir, "make") - with open(make_exe, "w") as f: - f.write("#!/bin/sh\n") - f.write('echo "$@"') - os.chmod(make_exe, 0o700) + path_put_first("PATH", [tmp_path]) - path_put_first("PATH", [self.tmpdir]) - def tearDown(self): - shutil.rmtree(self.tmpdir) +def test_make_normal(): + make = MakeExecutable("make", 8) + assert make(output=str).strip() == "-j8" + assert make("install", output=str).strip() == "-j8 install" - def test_make_normal(self): - make = MakeExecutable("make", 8) - self.assertEqual(make(output=str).strip(), "-j8") - self.assertEqual(make("install", output=str).strip(), "-j8 install") - def test_make_explicit(self): - make = MakeExecutable("make", 8) - self.assertEqual(make(parallel=True, output=str).strip(), "-j8") - self.assertEqual(make("install", parallel=True, output=str).strip(), "-j8 install") +def test_make_explicit(): + make = MakeExecutable("make", 8) + assert make(parallel=True, output=str).strip() == "-j8" + assert make("install", parallel=True, output=str).strip() == "-j8 install" - def test_make_one_job(self): - make = MakeExecutable("make", 1) - self.assertEqual(make(output=str).strip(), "-j1") - self.assertEqual(make("install", output=str).strip(), "-j1 install") - def test_make_parallel_false(self): - make = MakeExecutable("make", 8) - self.assertEqual(make(parallel=False, output=str).strip(), "-j1") - self.assertEqual(make("install", parallel=False, output=str).strip(), "-j1 install") +def test_make_one_job(): + make = MakeExecutable("make", 1) + assert make(output=str).strip() == "-j1" + assert make("install", output=str).strip() == "-j1 install" - def test_make_parallel_disabled(self): - make = MakeExecutable("make", 8) - os.environ["SPACK_NO_PARALLEL_MAKE"] = "true" - self.assertEqual(make(output=str).strip(), "-j1") - self.assertEqual(make("install", output=str).strip(), "-j1 install") +def test_make_parallel_false(): + make = MakeExecutable("make", 8) + assert make(parallel=False, output=str).strip() == "-j1" + assert make("install", parallel=False, output=str).strip() == "-j1 install" - os.environ["SPACK_NO_PARALLEL_MAKE"] = "1" - self.assertEqual(make(output=str).strip(), "-j1") - self.assertEqual(make("install", output=str).strip(), "-j1 install") - # These don't disable (false and random string) - os.environ["SPACK_NO_PARALLEL_MAKE"] = "false" - self.assertEqual(make(output=str).strip(), "-j8") - self.assertEqual(make("install", output=str).strip(), "-j8 install") +def test_make_parallel_disabled(monkeypatch): + make = MakeExecutable("make", 8) - os.environ["SPACK_NO_PARALLEL_MAKE"] = "foobar" - self.assertEqual(make(output=str).strip(), "-j8") - self.assertEqual(make("install", output=str).strip(), "-j8 install") + monkeypatch.setenv("SPACK_NO_PARALLEL_MAKE", "true") + assert make(output=str).strip() == "-j1" + assert make("install", output=str).strip() == "-j1 install" - del os.environ["SPACK_NO_PARALLEL_MAKE"] + monkeypatch.setenv("SPACK_NO_PARALLEL_MAKE", "1") + assert make(output=str).strip() == "-j1" + assert make("install", output=str).strip() == "-j1 install" - def test_make_parallel_precedence(self): - make = MakeExecutable("make", 8) + # These don't disable (false and random string) + monkeypatch.setenv("SPACK_NO_PARALLEL_MAKE", "false") + assert make(output=str).strip() == "-j8" + assert make("install", output=str).strip() == "-j8 install" - # These should work - os.environ["SPACK_NO_PARALLEL_MAKE"] = "true" - self.assertEqual(make(parallel=True, output=str).strip(), "-j1") - self.assertEqual(make("install", parallel=True, output=str).strip(), "-j1 install") + monkeypatch.setenv("SPACK_NO_PARALLEL_MAKE", "foobar") + assert make(output=str).strip() == "-j8" + assert make("install", output=str).strip() == "-j8 install" - os.environ["SPACK_NO_PARALLEL_MAKE"] = "1" - self.assertEqual(make(parallel=True, output=str).strip(), "-j1") - self.assertEqual(make("install", parallel=True, output=str).strip(), "-j1 install") - # These don't disable (false and random string) - os.environ["SPACK_NO_PARALLEL_MAKE"] = "false" - self.assertEqual(make(parallel=True, output=str).strip(), "-j8") - self.assertEqual(make("install", parallel=True, output=str).strip(), "-j8 install") +def test_make_parallel_precedence(monkeypatch): + make = MakeExecutable("make", 8) - os.environ["SPACK_NO_PARALLEL_MAKE"] = "foobar" - self.assertEqual(make(parallel=True, output=str).strip(), "-j8") - self.assertEqual(make("install", parallel=True, output=str).strip(), "-j8 install") + # These should work + monkeypatch.setenv("SPACK_NO_PARALLEL_MAKE", "true") + assert make(parallel=True, output=str).strip() == "-j1" + assert make("install", parallel=True, output=str).strip() == "-j1 install" - del os.environ["SPACK_NO_PARALLEL_MAKE"] + monkeypatch.setenv("SPACK_NO_PARALLEL_MAKE", "1") + assert make(parallel=True, output=str).strip() == "-j1" + assert make("install", parallel=True, output=str).strip() == "-j1 install" - def test_make_jobs_env(self): - make = MakeExecutable("make", 8) - dump_env = {} - self.assertEqual( - make(output=str, jobs_env="MAKE_PARALLELISM", _dump_env=dump_env).strip(), "-j8" - ) - self.assertEqual(dump_env["MAKE_PARALLELISM"], "8") + # These don't disable (false and random string) + monkeypatch.setenv("SPACK_NO_PARALLEL_MAKE", "false") + assert make(parallel=True, output=str).strip() == "-j8" + assert make("install", parallel=True, output=str).strip() == "-j8 install" - def test_make_jobserver(self): - make = MakeExecutable("make", 8) - os.environ["MAKEFLAGS"] = "--jobserver-auth=X,Y" - self.assertEqual(make(output=str).strip(), "") - self.assertEqual(make(parallel=False, output=str).strip(), "-j1") - del os.environ["MAKEFLAGS"] + monkeypatch.setenv("SPACK_NO_PARALLEL_MAKE", "foobar") + assert make(parallel=True, output=str).strip() == "-j8" + assert make("install", parallel=True, output=str).strip() == "-j8 install" - def test_make_jobserver_not_supported(self): - make = MakeExecutable("make", 8, supports_jobserver=False) - os.environ["MAKEFLAGS"] = "--jobserver-auth=X,Y" - # Currently fallback on default job count, Maybe it should force -j1 ? - self.assertEqual(make(output=str).strip(), "-j8") - del os.environ["MAKEFLAGS"] + +def test_make_jobs_env(): + make = MakeExecutable("make", 8) + dump_env = {} + assert make(output=str, jobs_env="MAKE_PARALLELISM", _dump_env=dump_env).strip() == "-j8" + assert dump_env["MAKE_PARALLELISM"] == "8" + + +def test_make_jobserver(monkeypatch): + make = MakeExecutable("make", 8) + monkeypatch.setenv("MAKEFLAGS", "--jobserver-auth=X,Y") + assert make(output=str).strip() == "" + assert make(parallel=False, output=str).strip() == "-j1" + + +def test_make_jobserver_not_supported(monkeypatch): + make = MakeExecutable("make", 8, supports_jobserver=False) + monkeypatch.setenv("MAKEFLAGS", "--jobserver-auth=X,Y") + # Currently fallback on default job count, Maybe it should force -j1 ? + assert make(output=str).strip() == "-j8"