Testing Bugfix: refactor clean --python-cache to support all (#31449)

There were two choices: 1) remove '-p' from '-a' or 2) allow monkeypatching
the cleaning of the python cache since clean's test_function_calls isn't
supposed to be actually cleaning anything.

This commit supports the latter and adds a test case for `-p`.

2 files changed, 63 insertions, 13 deletions

diff --git a/lib/spack/spack/cmd/clean.py b/lib/spack/spack/cmd/clean.py
index 83a1a81f79..6948aa3177 100644
--- a/lib/spack/spack/cmd/clean.py
+++ b/lib/spack/spack/cmd/clean.py
@@ -58,6 +58,21 @@ def setup_parser(subparser):
     arguments.add_common_arguments(subparser, ['specs'])
 
 
+def remove_python_cache():
+    for directory in [lib_path, var_path]:
+        for root, dirs, files in os.walk(directory):
+            for f in files:
+                if f.endswith('.pyc') or f.endswith('.pyo'):
+                    fname = os.path.join(root, f)
+                    tty.debug('Removing {0}'.format(fname))
+                    os.remove(fname)
+            for d in dirs:
+                if d == '__pycache__':
+                    dname = os.path.join(root, d)
+                    tty.debug('Removing {0}'.format(dname))
+                    shutil.rmtree(dname)
+
+
 def clean(parser, args):
     # If nothing was set, activate the default
     if not any([args.specs, args.stage, args.downloads, args.failures,
@@ -95,18 +110,7 @@ def clean(parser, args):
 
     if args.python_cache:
         tty.msg('Removing python cache files')
-        for directory in [lib_path, var_path]:
-            for root, dirs, files in os.walk(directory):
-                for f in files:
-                    if f.endswith('.pyc') or f.endswith('.pyo'):
-                        fname = os.path.join(root, f)
-                        tty.debug('Removing {0}'.format(fname))
-                        os.remove(fname)
-                for d in dirs:
-                    if d == '__pycache__':
-                        dname = os.path.join(root, d)
-                        tty.debug('Removing {0}'.format(dname))
-                        shutil.rmtree(dname)
+        remove_python_cache()
 
     if args.bootstrap:
         bootstrap_prefix = spack.util.path.canonicalize_path(
diff --git a/lib/spack/spack/test/cmd/clean.py b/lib/spack/spack/test/cmd/clean.py
index 82c8833a0c..39b4dc3cd9 100644
--- a/lib/spack/spack/test/cmd/clean.py
+++ b/lib/spack/spack/test/cmd/clean.py
@@ -3,10 +3,13 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
+import os
 import sys
 
 import pytest
 
+import llnl.util.filesystem as fs
+
 import spack.caches
 import spack.main
 import spack.package_base
@@ -41,11 +44,13 @@ def mock_calls_for_clean(monkeypatch):
         spack.caches.misc_cache, 'destroy', Counter('caches'))
     monkeypatch.setattr(
         spack.installer, 'clear_failures', Counter('failures'))
+    monkeypatch.setattr(spack.cmd.clean, 'remove_python_cache',
+                        Counter('python_cache'))
 
     yield counts
 
 
-all_effects = ['stages', 'downloads', 'caches', 'failures']
+all_effects = ['stages', 'downloads', 'caches', 'failures', 'python_cache']
 
 
 @pytest.mark.usefixtures(
@@ -57,6 +62,7 @@ all_effects = ['stages', 'downloads', 'caches', 'failures']
     ('-sd',      ['stages', 'downloads']),
     ('-m',       ['caches']),
     ('-f',       ['failures']),
+    ('-p',       ['python_cache']),
     ('-a',       all_effects),
     ('',         []),
 ])
@@ -69,3 +75,43 @@ def test_function_calls(command_line, effects, mock_calls_for_clean):
     # number of times
     for name in ['package'] + all_effects:
         assert mock_calls_for_clean[name] == (1 if name in effects else 0)
+
+
+def test_remove_python_cache(tmpdir, monkeypatch):
+    cache_files = ['file1.pyo', 'file2.pyc']
+    source_file = 'file1.py'
+
+    def _setup_files(directory):
+        # Create a python cache and source file.
+        cache_dir = fs.join_path(directory, '__pycache__')
+        fs.mkdirp(cache_dir)
+        fs.touch(fs.join_path(directory, source_file))
+        fs.touch(fs.join_path(directory, cache_files[0]))
+        for filename in cache_files:
+            # Ensure byte code files in python cache directory
+            fs.touch(fs.join_path(cache_dir, filename))
+
+    def _check_files(directory):
+        # Ensure the python cache created by _setup_files is removed
+        # and the source file is not.
+        assert os.path.exists(fs.join_path(directory, source_file))
+        assert not os.path.exists(fs.join_path(directory, cache_files[0]))
+        assert not os.path.exists(fs.join_path(directory, '__pycache__'))
+
+    source_dir = fs.join_path(tmpdir, 'lib', 'spack', 'spack')
+    var_dir = fs.join_path(tmpdir, 'var', 'spack', 'stuff')
+
+    for d in [source_dir, var_dir]:
+        _setup_files(d)
+
+    # Patching the path variables from-import'd by spack.cmd.clean is needed
+    # to ensure the paths used by the command for this test reflect the
+    # temporary directory locations and not those from spack.paths when
+    # the clean command's module was imported.
+    monkeypatch.setattr(spack.cmd.clean, "lib_path", source_dir)
+    monkeypatch.setattr(spack.cmd.clean, "var_path", var_dir)
+
+    spack.cmd.clean.remove_python_cache()
+
+    for d in [source_dir, var_dir]:
+        _check_files(d)