ci: clean up env between tests with working_dir (#29807)

2 files changed, 70 insertions, 98 deletions

diff --git a/lib/spack/spack/test/ci.py b/lib/spack/spack/test/ci.py
index cd87c2dc60..9d653be35c 100644
--- a/lib/spack/spack/test/ci.py
+++ b/lib/spack/spack/test/ci.py
@@ -264,8 +264,10 @@ def test_read_write_cdash_ids(config, tmp_scope, tmpdir, mock_packages):
     assert(str(read_cdashid) == orig_cdashid)
 
 
-def test_download_and_extract_artifacts(tmpdir, monkeypatch):
-    os.environ['GITLAB_PRIVATE_TOKEN'] = 'faketoken'
+def test_download_and_extract_artifacts(tmpdir, monkeypatch, working_env):
+    os.environ.update({
+        'GITLAB_PRIVATE_TOKEN': 'faketoken',
+    })
 
     url = 'https://www.nosuchurlexists.itsfake/artifacts.zip'
     working_dir = os.path.join(tmpdir.strpath, 'repro')
diff --git a/lib/spack/spack/test/cmd/ci.py b/lib/spack/spack/test/cmd/ci.py
index 41761a7e81..ce6359fcf8 100644
--- a/lib/spack/spack/test/cmd/ci.py
+++ b/lib/spack/spack/test/cmd/ci.py
@@ -46,18 +46,8 @@ pytestmark = [pytest.mark.skipif(sys.platform == "win32",
 
 
 @pytest.fixture()
-def project_dir_env():
-    def _set_project_dir(path):
-        os.environ['CI_PROJECT_DIR'] = path
-
-    yield _set_project_dir
-
-    if 'CI_PROJECT_DIR' in os.environ:
-        os.environ.pop('CI_PROJECT_DIR')
-
-
-def set_env_var(key, val):
-    os.environ[key] = val
+def ci_base_environment(working_env, tmpdir):
+    os.environ['CI_PROJECT_DIR'] = tmpdir.strpath
 
 
 def test_specs_staging(config):
@@ -120,11 +110,10 @@ and then 'd', 'b', and 'a' to be put in the next three stages, respectively.
 
 
 def test_ci_generate_with_env(tmpdir, mutable_mock_env_path,
-                              install_mockery, mock_packages, project_dir_env,
-                              mock_binary_index):
+                              install_mockery, mock_packages,
+                              ci_base_environment, mock_binary_index):
     """Make sure we can get a .gitlab-ci.yml from an environment file
        which has the gitlab-ci, cdash, and mirrors sections."""
-    project_dir_env(tmpdir.strpath)
     mirror_url = 'https://my.fake.mirror'
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
@@ -217,10 +206,9 @@ def _validate_needs_graph(yaml_contents, needs_graph, artifacts):
 
 def test_ci_generate_bootstrap_gcc(tmpdir, mutable_mock_env_path,
                                    install_mockery,
-                                   mock_packages, project_dir_env):
+                                   mock_packages, ci_base_environment):
     """Test that we can bootstrap a compiler and use it as the
     compiler for a spec in the environment"""
-    project_dir_env(tmpdir.strpath)
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
         f.write("""\
@@ -281,10 +269,9 @@ def test_ci_generate_bootstrap_artifacts_buildcache(tmpdir,
                                                     mutable_mock_env_path,
                                                     install_mockery,
                                                     mock_packages,
-                                                    project_dir_env):
+                                                    ci_base_environment):
     """Test that we can bootstrap a compiler when artifacts buildcache
     is turned on"""
-    project_dir_env(tmpdir.strpath)
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
         f.write("""\
@@ -346,10 +333,9 @@ spack:
 
 def test_ci_generate_with_env_missing_section(tmpdir, mutable_mock_env_path,
                                               install_mockery,
-                                              mock_packages, project_dir_env,
+                                              mock_packages, ci_base_environment,
                                               mock_binary_index):
     """Make sure we get a reasonable message if we omit gitlab-ci section"""
-    project_dir_env(tmpdir.strpath)
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
         f.write("""\
@@ -372,10 +358,12 @@ spack:
 
 def test_ci_generate_with_cdash_token(tmpdir, mutable_mock_env_path,
                                       install_mockery,
-                                      mock_packages, project_dir_env,
+                                      mock_packages, ci_base_environment,
                                       mock_binary_index):
     """Make sure we it doesn't break if we configure cdash"""
-    project_dir_env(tmpdir.strpath)
+    os.environ.update({
+        'SPACK_CDASH_AUTH_TOKEN': 'notreallyatokenbutshouldnotmatter',
+    })
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
         f.write("""\
@@ -404,13 +392,8 @@ spack:
         env_cmd('create', 'test', './spack.yaml')
 
         with ev.read('test'):
-            fake_token = 'notreallyatokenbutshouldnotmatter'
-            os.environ['SPACK_CDASH_AUTH_TOKEN'] = fake_token
             copy_to_file = str(tmpdir.join('backup-ci.yml'))
-            try:
-                output = ci_cmd('generate', '--copy-to', copy_to_file, output=str)
-            finally:
-                del os.environ['SPACK_CDASH_AUTH_TOKEN']
+            output = ci_cmd('generate', '--copy-to', copy_to_file, output=str)
             # That fake token should still have resulted in being unable to
             # register build group with cdash, but the workload should
             # still have been generated.
@@ -429,9 +412,8 @@ spack:
 def test_ci_generate_with_custom_scripts(tmpdir, mutable_mock_env_path,
                                          install_mockery,
                                          mock_packages, monkeypatch,
-                                         project_dir_env, mock_binary_index):
+                                         ci_base_environment, mock_binary_index):
     """Test use of user-provided scripts"""
-    project_dir_env(tmpdir.strpath)
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
         f.write("""\
@@ -519,9 +501,8 @@ spack:
 
 def test_ci_generate_pkg_with_deps(tmpdir, mutable_mock_env_path,
                                    install_mockery,
-                                   mock_packages, project_dir_env):
+                                   mock_packages, ci_base_environment):
     """Test pipeline generation for a package w/ dependencies"""
-    project_dir_env(tmpdir.strpath)
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
         f.write("""\
@@ -574,11 +555,14 @@ spack:
 def test_ci_generate_for_pr_pipeline(tmpdir, mutable_mock_env_path,
                                      install_mockery,
                                      mock_packages, monkeypatch,
-                                     project_dir_env):
+                                     ci_base_environment):
     """Test that PR pipelines do not include a final stage job for
     rebuilding the mirror index, even if that job is specifically
     configured"""
-    project_dir_env(tmpdir.strpath)
+    os.environ.update({
+        'SPACK_PIPELINE_TYPE': 'spack_pull_request',
+        'SPACK_PR_BRANCH': 'fake-test-branch',
+    })
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
         f.write("""\
@@ -611,17 +595,11 @@ spack:
         outputfile = str(tmpdir.join('.gitlab-ci.yml'))
 
         with ev.read('test'):
-            os.environ['SPACK_PIPELINE_TYPE'] = 'spack_pull_request'
-            os.environ['SPACK_PR_BRANCH'] = 'fake-test-branch'
             monkeypatch.setattr(
                 ci, 'SPACK_PR_MIRRORS_ROOT_URL', r"file:///fake/mirror")
             monkeypatch.setattr(
                 ci, 'SPACK_SHARED_PR_MIRROR_URL', r"file:///fake/mirror_two")
-            try:
-                ci_cmd('generate', '--output-file', outputfile)
-            finally:
-                del os.environ['SPACK_PIPELINE_TYPE']
-                del os.environ['SPACK_PR_BRANCH']
+            ci_cmd('generate', '--output-file', outputfile)
 
         with open(outputfile) as f:
             contents = f.read()
@@ -638,9 +616,8 @@ spack:
 def test_ci_generate_with_external_pkg(tmpdir, mutable_mock_env_path,
                                        install_mockery,
                                        mock_packages, monkeypatch,
-                                       project_dir_env):
+                                       ci_base_environment):
     """Make sure we do not generate jobs for external pkgs"""
-    project_dir_env(tmpdir.strpath)
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
         f.write("""\
@@ -682,9 +659,8 @@ spack:
 @pytest.mark.xfail(reason='fails intermittently and covered by gitlab ci')
 def test_ci_rebuild(tmpdir, mutable_mock_env_path,
                     install_mockery, mock_packages, monkeypatch,
-                    mock_gnupghome, mock_fetch, project_dir_env,
+                    mock_gnupghome, mock_fetch, ci_base_environment,
                     mock_binary_index):
-    project_dir_env(tmpdir.strpath)
     working_dir = tmpdir.join('working_dir')
 
     log_dir = os.path.join(working_dir.strpath, 'logs')
@@ -778,23 +754,25 @@ spack:
         env_cmd('activate', '--without-view', '--sh', '-d', '.')
 
         # Create environment variables as gitlab would do it
-        set_env_var('SPACK_ARTIFACTS_ROOT', working_dir.strpath)
-        set_env_var('SPACK_JOB_LOG_DIR', log_dir)
-        set_env_var('SPACK_JOB_REPRO_DIR', repro_dir)
-        set_env_var('SPACK_LOCAL_MIRROR_DIR', mirror_dir.strpath)
-        set_env_var('SPACK_CONCRETE_ENV_DIR', env_dir.strpath)
-        set_env_var('CI_PIPELINE_ID', '7192')
-        set_env_var('SPACK_SIGNING_KEY', signing_key)
-        set_env_var('SPACK_ROOT_SPEC', root_spec_build_hash)
-        set_env_var('SPACK_JOB_SPEC_DAG_HASH', job_spec_dag_hash)
-        set_env_var('SPACK_JOB_SPEC_PKG_NAME', 'archive-files')
-        set_env_var('SPACK_COMPILER_ACTION', 'NONE')
-        set_env_var('SPACK_CDASH_BUILD_NAME', '(specs) archive-files')
-        set_env_var('SPACK_RELATED_BUILDS_CDASH', '')
-        set_env_var('SPACK_REMOTE_MIRROR_URL', mirror_url)
-        set_env_var('SPACK_PIPELINE_TYPE', 'spack_protected_branch')
-        set_env_var('CI_JOB_URL', ci_job_url)
-        set_env_var('CI_PIPELINE_URL', ci_pipeline_url)
+        os.environ.update({
+            'SPACK_ARTIFACTS_ROOT': working_dir.strpath,
+            'SPACK_JOB_LOG_DIR': log_dir,
+            'SPACK_JOB_REPRO_DIR': repro_dir,
+            'SPACK_LOCAL_MIRROR_DIR': mirror_dir.strpath,
+            'SPACK_CONCRETE_ENV_DIR': env_dir.strpath,
+            'CI_PIPELINE_ID': '7192',
+            'SPACK_SIGNING_KEY': signing_key,
+            'SPACK_ROOT_SPEC': root_spec_build_hash,
+            'SPACK_JOB_SPEC_DAG_HASH': job_spec_dag_hash,
+            'SPACK_JOB_SPEC_PKG_NAME': 'archive-files',
+            'SPACK_COMPILER_ACTION': 'NONE',
+            'SPACK_CDASH_BUILD_NAME': '(specs) archive-files',
+            'SPACK_RELATED_BUILDS_CDASH': '',
+            'SPACK_REMOTE_MIRROR_URL': mirror_url,
+            'SPACK_PIPELINE_TYPE': 'spack_protected_branch',
+            'CI_JOB_URL': ci_job_url,
+            'CI_PIPELINE_URL': ci_pipeline_url,
+        })
 
         ci_cmd('rebuild', fail_on_error=False)
 
@@ -841,8 +819,7 @@ spack:
 
 def test_ci_nothing_to_rebuild(tmpdir, mutable_mock_env_path,
                                install_mockery, mock_packages, monkeypatch,
-                               mock_fetch, project_dir_env, mock_binary_index):
-    project_dir_env(tmpdir.strpath)
+                               mock_fetch, ci_base_environment, mock_binary_index):
     working_dir = tmpdir.join('working_dir')
 
     mirror_dir = working_dir.join('mirror')
@@ -888,16 +865,18 @@ spack:
                     job_spec_dag_hash = s.dag_hash()
 
             # Create environment variables as gitlab would do it
-            set_env_var('SPACK_ARTIFACTS_ROOT', working_dir.strpath)
-            set_env_var('SPACK_JOB_LOG_DIR', 'log_dir')
-            set_env_var('SPACK_JOB_REPRO_DIR', 'repro_dir')
-            set_env_var('SPACK_LOCAL_MIRROR_DIR', mirror_dir.strpath)
-            set_env_var('SPACK_CONCRETE_ENV_DIR', tmpdir.strpath)
-            set_env_var('SPACK_ROOT_SPEC', root_spec_build_hash)
-            set_env_var('SPACK_JOB_SPEC_DAG_HASH', job_spec_dag_hash)
-            set_env_var('SPACK_JOB_SPEC_PKG_NAME', 'archive-files')
-            set_env_var('SPACK_COMPILER_ACTION', 'NONE')
-            set_env_var('SPACK_REMOTE_MIRROR_URL', mirror_url)
+            os.environ.update({
+                'SPACK_ARTIFACTS_ROOT': working_dir.strpath,
+                'SPACK_JOB_LOG_DIR': 'log_dir',
+                'SPACK_JOB_REPRO_DIR': 'repro_dir',
+                'SPACK_LOCAL_MIRROR_DIR': mirror_dir.strpath,
+                'SPACK_CONCRETE_ENV_DIR': tmpdir.strpath,
+                'SPACK_ROOT_SPEC': root_spec_build_hash,
+                'SPACK_JOB_SPEC_DAG_HASH': job_spec_dag_hash,
+                'SPACK_JOB_SPEC_PKG_NAME': 'archive-files',
+                'SPACK_COMPILER_ACTION': 'NONE',
+                'SPACK_REMOTE_MIRROR_URL': mirror_url,
+            })
 
             def fake_dl_method(spec, *args, **kwargs):
                 print('fake download buildcache {0}'.format(spec.name))
@@ -917,8 +896,7 @@ spack:
 def test_push_mirror_contents(tmpdir, mutable_mock_env_path,
                               install_mockery_mutable_config, mock_packages,
                               mock_fetch, mock_stage, mock_gnupghome,
-                              project_dir_env):
-    project_dir_env(tmpdir.strpath)
+                              ci_base_environment):
     working_dir = tmpdir.join('working_dir')
 
     mirror_dir = working_dir.join('mirror')
@@ -1083,12 +1061,11 @@ def test_push_mirror_contents_exceptions(monkeypatch, capsys):
 def test_ci_generate_override_runner_attrs(tmpdir, mutable_mock_env_path,
                                            install_mockery,
                                            mock_packages, monkeypatch,
-                                           project_dir_env):
+                                           ci_base_environment):
     """Test that we get the behavior we want with respect to the provision
        of runner attributes like tags, variables, and scripts, both when we
        inherit them from the top level, as well as when we override one or
        more at the runner level"""
-    project_dir_env(tmpdir.strpath)
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
         f.write("""\
@@ -1228,9 +1205,8 @@ spack:
 def test_ci_generate_with_workarounds(tmpdir, mutable_mock_env_path,
                                       install_mockery,
                                       mock_packages, monkeypatch,
-                                      project_dir_env):
+                                      ci_base_environment):
     """Make sure the post-processing cli workarounds do what they should"""
-    project_dir_env(tmpdir.strpath)
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
         f.write("""\
@@ -1331,14 +1307,13 @@ spack:
 def test_ci_generate_bootstrap_prune_dag(
         install_mockery_mutable_config, mock_packages, mock_fetch,
         mock_archive, mutable_config, monkeypatch, tmpdir,
-        mutable_mock_env_path, project_dir_env):
+        mutable_mock_env_path, ci_base_environment):
     """Test compiler bootstrapping with DAG pruning.  Specifically, make
        sure that if we detect the bootstrapped compiler needs to be rebuilt,
        we ensure the spec we want to build with that compiler is scheduled
        for rebuild as well."""
 
     # Create a temp mirror directory for buildcache usage
-    project_dir_env(tmpdir.strpath)
     mirror_dir = tmpdir.join('mirror_dir')
     mirror_url = 'file://{0}'.format(mirror_dir.strpath)
 
@@ -1464,10 +1439,12 @@ spack:
 
 def test_ci_generate_prune_untouched(tmpdir, mutable_mock_env_path,
                                      install_mockery, mock_packages,
-                                     project_dir_env, monkeypatch):
+                                     ci_base_environment, monkeypatch):
     """Test pipeline generation with pruning works to eliminate
        specs that were not affected by a change"""
-    project_dir_env(tmpdir.strpath)
+    os.environ.update({
+        'SPACK_PRUNE_UNTOUCHED': 'TRUE',  # enables pruning of untouched specs
+    })
     mirror_url = 'https://my.fake.mirror'
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
@@ -1495,12 +1472,9 @@ spack:
             return ['libdwarf']
 
         with ev.read('test'):
-            # This kind of pruning is enabled with the following env var
-            os.environ['SPACK_PRUNE_UNTOUCHED'] = 'TRUE'
             monkeypatch.setattr(
                 ci, 'compute_affected_packages', fake_compute_affected)
             ci_cmd('generate', '--output-file', outputfile)
-            os.environ.pop('SPACK_PRUNE_UNTOUCHED')
 
         with open(outputfile) as f:
             contents = f.read()
@@ -1514,10 +1488,9 @@ spack:
 
 def test_ci_subcommands_without_mirror(tmpdir, mutable_mock_env_path,
                                        mock_packages,
-                                       install_mockery, project_dir_env,
+                                       install_mockery, ci_base_environment,
                                        mock_binary_index):
     """Make sure we catch if there is not a mirror and report an error"""
-    project_dir_env(tmpdir.strpath)
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
         f.write("""\
@@ -1594,9 +1567,8 @@ def test_ensure_only_one_temporary_storage():
 def test_ci_generate_temp_storage_url(tmpdir, mutable_mock_env_path,
                                       install_mockery,
                                       mock_packages, monkeypatch,
-                                      project_dir_env, mock_binary_index):
+                                      ci_base_environment, mock_binary_index):
     """Verify correct behavior when using temporary-storage-url-prefix"""
-    project_dir_env(tmpdir.strpath)
     filename = str(tmpdir.join('spack.yaml'))
     with open(filename, 'w') as f:
         f.write("""\
@@ -1650,9 +1622,8 @@ spack:
 def test_ci_generate_read_broken_specs_url(tmpdir, mutable_mock_env_path,
                                            install_mockery,
                                            mock_packages, monkeypatch,
-                                           project_dir_env):
+                                           ci_base_environment):
     """Verify that `broken-specs-url` works as intended"""
-    project_dir_env(tmpdir.strpath)
     spec_a = Spec('a')
     spec_a.concretize()
     a_full_hash = spec_a.full_hash()
@@ -1708,8 +1679,7 @@ spack:
 
 def test_ci_reproduce(tmpdir, mutable_mock_env_path,
                       install_mockery, mock_packages, monkeypatch,
-                      last_two_git_commits, project_dir_env, mock_binary_index):
-    project_dir_env(tmpdir.strpath)
+                      last_two_git_commits, ci_base_environment, mock_binary_index):
     working_dir = tmpdir.join('repro_dir')
     image_name = 'org/image:tag'