From c8a10e4910f8222e34b22b2e37c26ace4bae968f Mon Sep 17 00:00:00 2001
From: Harmen Stoppels <harmenstoppels@gmail.com>
Date: Tue, 30 Mar 2021 13:23:39 +0200
Subject: Fix clearing cache of InternalConfigScope (#22609)

Co-authored-by: Massimiliano Culpo <massimiliano.culpo@gmail.com>
---
 lib/spack/spack/config.py                    |  4 ++
 lib/spack/spack/test/cmd/common/arguments.py | 64 +++++++++++-----------------
 lib/spack/spack/test/config.py               | 21 +++++++++
 3 files changed, 49 insertions(+), 40 deletions(-)

diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index e761c20f7f..8794072e96 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -361,6 +361,10 @@ class InternalConfigScope(ConfigScope):
     def __repr__(self):
         return '<InternalConfigScope: %s>' % self.name
 
+    def clear(self):
+        # no cache to clear here.
+        pass
+
     @staticmethod
     def _process_dict_keyname_overrides(data):
         """Turn a trailing `:' in a key name into an override attribute."""
diff --git a/lib/spack/spack/test/cmd/common/arguments.py b/lib/spack/spack/test/cmd/common/arguments.py
index 76e39bcad0..6646144a81 100644
--- a/lib/spack/spack/test/cmd/common/arguments.py
+++ b/lib/spack/spack/test/cmd/common/arguments.py
@@ -15,53 +15,37 @@ import spack.config
 
 
 @pytest.fixture()
-def parser():
+def job_parser():
+    # --jobs needs to write to a command_line config scope, so this is the only
+    # scope we create.
     p = argparse.ArgumentParser()
     arguments.add_common_arguments(p, ['jobs'])
-    yield p
-    # Cleanup the command line scope if it was set during tests
-    spack.config.config.clear_caches()
-    if 'command_line' in spack.config.config.scopes:
-        spack.config.config.scopes['command_line'].clear()
-
-
-@pytest.fixture(params=[1, 2, 4, 8, 16, 32])
-def ncores(monkeypatch, request):
-    """Mocks having a machine with n cores for the purpose of
-    computing config:build_jobs.
-    """
-    def _cpu_count():
-        return request.param
-
-    # Patch multiprocessing.cpu_count() to return the value we need
-    monkeypatch.setattr(multiprocessing, 'cpu_count', _cpu_count)
-    # Patch the configuration parts that have been cached already
-    monkeypatch.setitem(spack.config.config_defaults['config'],
-                        'build_jobs', min(16, request.param))
-    monkeypatch.setitem(
-        spack.config.config.scopes, '_builtin',
-        spack.config.InternalConfigScope(
-            '_builtin', spack.config.config_defaults
-        ))
-    return request.param
-
-
-@pytest.mark.parametrize('cli_args,requested', [
-    (['-j', '24'], 24),
-    # Here we report the default if we have enough cores, as the cap
-    # on the available number of cores will be taken care of in the test
-    ([], 16)
-])
-def test_setting_parallel_jobs(parser, cli_args, ncores, requested):
-    expected = min(requested, ncores)
-    namespace = parser.parse_args(cli_args)
+    scopes = [spack.config.InternalConfigScope('command_line', {'config': {}})]
+
+    with spack.config.use_configuration(*scopes):
+        yield p
+
+
+@pytest.mark.parametrize("ncores", [1, 2, 4, 8, 16, 32])
+def test_setting_jobs_flag(job_parser, ncores, monkeypatch):
+    monkeypatch.setattr(multiprocessing, 'cpu_count', lambda: ncores)
+    namespace = job_parser.parse_args(['-j', '24'])
+    expected = min(24, ncores)
     assert namespace.jobs == expected
     assert spack.config.get('config:build_jobs') == expected
 
 
-def test_negative_integers_not_allowed_for_parallel_jobs(parser):
+@pytest.mark.parametrize("ncores", [1, 2, 4, 8, 16, 32])
+def test_omitted_job_flag(job_parser, ncores, monkeypatch):
+    monkeypatch.setattr(multiprocessing, 'cpu_count', lambda: ncores)
+    namespace = job_parser.parse_args([])
+    assert namespace.jobs == min(ncores, 16)
+    assert spack.config.get('config:build_jobs') is None
+
+
+def test_negative_integers_not_allowed_for_parallel_jobs(job_parser):
     with pytest.raises(ValueError) as exc_info:
-        parser.parse_args(['-j', '-2'])
+        job_parser.parse_args(['-j', '-2'])
 
     assert 'expected a positive integer' in str(exc_info.value)
 
diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py
index 10561601be..9384eb5b47 100644
--- a/lib/spack/spack/test/config.py
+++ b/lib/spack/spack/test/config.py
@@ -1068,3 +1068,24 @@ def test_single_file_scope_cache_clearing(env_yaml):
     after = scope.get_section('config')
     assert after
     assert before == after
+
+
+@pytest.mark.regression('22611')
+def test_internal_config_scope_cache_clearing():
+    """
+    An InternalConfigScope object is constructed from data that is already
+    in memory, therefore it doesn't have any cache to clear. Here we ensure
+    that calling the clear method is consistent with that..
+    """
+    data = {
+        'config': {
+            'build_jobs': 10
+        }
+    }
+    internal_scope = spack.config.InternalConfigScope('internal', data)
+    # Ensure that the initial object is properly set
+    assert internal_scope.sections['config'] == data
+    # Call the clear method
+    internal_scope.clear()
+    # Check that this didn't affect the scope object
+    assert internal_scope.sections['config'] == data
-- 
cgit v1.2.3-70-g09d2