From 702774edea6692a91a52c407dc7d615c2f0f2c60 Mon Sep 17 00:00:00 2001
From: Gregory Becker <becker33@llnl.gov>
Date: Fri, 4 Jun 2021 09:54:16 -0700
Subject: [PATCH] add test, minor refactor to make testing easier

---
 lib/spack/spack/environment.py  | 16 +++++++++-------
 lib/spack/spack/test/cmd/env.py | 30 ++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index 8df066d5713..be7a25fffd9 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -510,6 +510,11 @@ def from_dict(base_path, d):
                               d.get('link', default_view_link),
                               d.get('keep', _default_keep))
 
+    @property
+    def _impl_dir(self):
+        root_dir, root_name = os.path.split(self.root)
+        return os.path.join(root_dir, '._%s' % root_name)
+
     @property
     def _current_root(self):
         if not os.path.islink(self.root):
@@ -524,9 +529,7 @@ def _current_root(self):
 
     def _next_root(self, specs):
         content_hash = self.content_hash(specs)
-        root_dir = os.path.dirname(self.root)
-        root_name = os.path.basename(self.root)
-        return os.path.join(root_dir, '._%s' % root_name, content_hash)
+        return os.path.join(self._impl_dir, content_hash)
 
     def content_hash(self, specs):
         d = syaml.syaml_dict([
@@ -644,7 +647,7 @@ def regenerate(self, all_specs, roots):
                            with_dependencies=False)
 
             # create symlink from tmpname to new_root
-            root_dirname, root_basename = os.path.split(self.root)
+            root_dirname = os.path.dirname(self.root)
             tmp_symlink_name = os.path.join(root_dirname, '._view_link')
             if os.path.exists(tmp_symlink_name):
                 os.unlink(tmp_symlink_name)
@@ -660,9 +663,8 @@ def regenerate(self, all_specs, roots):
             # If we have more than the number of views to keep in the directory
             # delete the oldest until the correct number remain. Do not count
             # things that aren't named as a root would be.
-            root_impl_dir = os.path.join(root_dirname, '._%s' % root_basename)
-            roots = [os.path.join(root_impl_dir, entry)
-                     for entry in os.listdir(root_impl_dir)
+            roots = [os.path.join(self._impl_dir, entry)
+                     for entry in os.listdir(self._impl_dir)
                      if re.match(r'[a-z0-9]{32}', entry)]
 
             num_to_remove = len(roots) - self.keep
diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py
index 58094e1abb0..5d7cebeabe4 100644
--- a/lib/spack/spack/test/cmd/env.py
+++ b/lib/spack/spack/test/cmd/env.py
@@ -2581,6 +2581,36 @@ def test_multiple_modules_post_env_hook(tmpdir, install_mockery, mock_fetch):
     assert spec.prefix in full_contents
 
 
+def test_view_keep_number(tmpdir, install_mockery, mock_fetch):
+    spack_yaml = """
+spack:
+  specs:
+  - trivial-install-test-package
+  view:
+    default:
+      root: view
+      keep: 1
+"""
+    _env_create('test', StringIO(spack_yaml))
+
+    with ev.read('test') as e:
+        install()
+
+        # Add something that will affect the view hash
+        e.default_view.exclude = ['gcc']
+        e.regenerate_views()
+
+        view_path = e.default_view._impl_dir
+
+    assert len(os.listdir(view_path)) == 1
+
+    with ev.read('test') as e:
+        e.default_view.keep = 2
+        e.regenerate_views()
+
+    assert len(os.listdir(view_path)) == 2
+
+
 @pytest.mark.regression('24148')
 def test_virtual_spec_concretize_together(tmpdir):
     # An environment should permit to concretize "mpi"