tests: fix more cases of env variables (#41226)

3 files changed, 105 insertions, 115 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 @@ Tests for Spack's built-in parallel make support.
 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.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",
+    sys.platform == "win32", reason="MakeExecutable not supported on Windows"
 )
 
 
-class MakeExecutableTest(unittest.TestCase):
-    def setUp(self):
-        self.tmpdir = tempfile.mkdtemp()
-
-        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", [self.tmpdir])
-
-    def tearDown(self):
-        shutil.rmtree(self.tmpdir)
-
-    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_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_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")
-
-        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")
-
-        os.environ["SPACK_NO_PARALLEL_MAKE"] = "foobar"
-        self.assertEqual(make(output=str).strip(), "-j8")
-        self.assertEqual(make("install", output=str).strip(), "-j8 install")
-
-        del os.environ["SPACK_NO_PARALLEL_MAKE"]
-
-    def test_make_parallel_precedence(self):
-        make = MakeExecutable("make", 8)
-
-        # 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")
-
-        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")
-
-        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")
-
-        del os.environ["SPACK_NO_PARALLEL_MAKE"]
-
-    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")
-
-    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"]
-
-    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"]
+@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)
+
+    path_put_first("PATH", [tmp_path])
+
+
+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_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():
+    make = MakeExecutable("make", 1)
+    assert make(output=str).strip() == "-j1"
+    assert 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"
+
+
+def test_make_parallel_disabled(monkeypatch):
+    make = MakeExecutable("make", 8)
+
+    monkeypatch.setenv("SPACK_NO_PARALLEL_MAKE", "true")
+    assert make(output=str).strip() == "-j1"
+    assert make("install", output=str).strip() == "-j1 install"
+
+    monkeypatch.setenv("SPACK_NO_PARALLEL_MAKE", "1")
+    assert make(output=str).strip() == "-j1"
+    assert make("install", output=str).strip() == "-j1 install"
+
+    # 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"
+
+    monkeypatch.setenv("SPACK_NO_PARALLEL_MAKE", "foobar")
+    assert make(output=str).strip() == "-j8"
+    assert make("install", output=str).strip() == "-j8 install"
+
+
+def test_make_parallel_precedence(monkeypatch):
+    make = MakeExecutable("make", 8)
+
+    # 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"
+
+    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"
+
+    # 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"
+
+    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_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"