unit test: refactored bindist.py (#21497)

Modifications:
- Make use of SpackCommand objects wherever possible
- Deduplicated code when possible
- Moved cleaning of mirrors to fixtures
- Ensure mock configuration has a clear initialization order

lib/spack/spack/test/bindist.py

@@ -2,95 +2,61 @@
 # Spack Project Developers. See the top-level COPYRIGHT file for details.
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
-
-"""
-This test checks creating and install buildcaches
-"""
 import os
 import sys
+import platform
+
 import py
 import pytest
-import argparse
+
-import platform
+import spack.binary_distribution as bindist
+import spack.config
+import spack.hooks.sbang as sbang
+import spack.main
+import spack.mirror
 import spack.repo
 import spack.store
-import spack.binary_distribution as bindist
-import spack.cmd.buildcache as buildcache
-import spack.cmd.install as install
-import spack.cmd.uninstall as uninstall
-import spack.cmd.mirror as mirror
-import spack.hooks.sbang as sbang
-from spack.main import SpackCommand
-import spack.mirror
 import spack.util.gpg
 import spack.util.web as web_util
+
 from spack.directory_layout import YamlDirectoryLayout
 from spack.spec import Spec
 
-
+mirror_cmd = spack.main.SpackCommand('mirror')
-def_install_path_scheme = '${ARCHITECTURE}/${COMPILERNAME}-${COMPILERVER}/${PACKAGE}-${VERSION}-${HASH}'  # noqa: E501
+install_cmd = spack.main.SpackCommand('install')
-ndef_install_path_scheme = '${PACKAGE}/${VERSION}/${ARCHITECTURE}-${COMPILERNAME}-${COMPILERVER}-${HASH}'  # noqa: E501
+uninstall_cmd = spack.main.SpackCommand('uninstall')
-
+buildcache_cmd = spack.main.SpackCommand('buildcache')
-mirror_path_def = None
-mirror_path_rel = None
-
-mirror_cmd = SpackCommand('mirror')
-install_cmd = SpackCommand('install')
-uninstall_cmd = SpackCommand('uninstall')
-buildcache_cmd = SpackCommand('buildcache')
 
 
 @pytest.fixture(scope='function')
 def cache_directory(tmpdir):
-    old_cache_path = spack.caches.fetch_cache
+    fetch_cache_dir = tmpdir.ensure('fetch_cache', dir=True)
-    tmpdir.ensure('fetch_cache', dir=True)
+    fsc = spack.fetch_strategy.FsCache(str(fetch_cache_dir))
-    fsc = spack.fetch_strategy.FsCache(str(tmpdir.join('fetch_cache')))
+    spack.config.caches, old_cache_path = fsc, spack.caches.fetch_cache
-    spack.config.caches = fsc
+
     yield spack.config.caches
-    tmpdir.join('fetch_cache').remove()
+
+    fetch_cache_dir.remove()
     spack.config.caches = old_cache_path
 
 
-@pytest.fixture(scope='session')
+@pytest.fixture(scope='module')
-def session_mirror_def(tmpdir_factory):
+def mirror_dir(tmpdir_factory):
     dir = tmpdir_factory.mktemp('mirror')
-    global mirror_path_rel
-    mirror_path_rel = dir
     dir.ensure('build_cache', dir=True)
-    yield dir
+    yield str(dir)
     dir.join('build_cache').remove()
 
 
 @pytest.fixture(scope='function')
-def mirror_directory_def(session_mirror_def):
+def test_mirror(mirror_dir):
-    yield str(session_mirror_def)
+    mirror_url = 'file://%s' % mirror_dir
-
+    mirror_cmd('add', '--scope', 'site', 'test-mirror-func', mirror_url)
-
-@pytest.fixture(scope='session')
-def session_mirror_rel(tmpdir_factory):
-    dir = tmpdir_factory.mktemp('mirror')
-    global mirror_path_rel
-    mirror_path_rel = dir
-    dir.ensure('build_cache', dir=True)
-    yield dir
-    dir.join('build_cache').remove()
-
-
-@pytest.fixture(scope='function')
-def mirror_directory_rel(session_mirror_rel):
-    yield(session_mirror_rel)
-
-
-@pytest.fixture(scope='function')
-def function_mirror(tmpdir):
-    mirror_dir = str(tmpdir.join('mirror'))
-    mirror_cmd('add', '--scope', 'site', 'test-mirror-func',
-               'file://%s' % mirror_dir)
     yield mirror_dir
     mirror_cmd('rm', '--scope=site', 'test-mirror-func')
 
 
-@pytest.fixture(scope='session')
+@pytest.fixture(scope='module')
 def config_directory(tmpdir_factory):
     tmpdir = tmpdir_factory.mktemp('test_configs')
     # restore some sane defaults for packages and config
@@ -116,8 +82,14 @@ def config_directory(tmpdir_factory):
 
 
 @pytest.fixture(scope='function')
-def default_config(tmpdir_factory, config_directory, monkeypatch):
+def default_config(
-
+        tmpdir_factory, config_directory, monkeypatch,
+        install_mockery_mutable_config
+):
+    # This fixture depends on install_mockery_mutable_config to ensure
+    # there is a clear order of initialization. The substitution of the
+    # config scopes here is done on top of the substitution that comes with
+    # install_mockery_mutable_config
     mutable_dir = tmpdir_factory.mktemp('mutable_config').join('tmp')
     config_directory.copy(mutable_dir)
 
@@ -126,7 +98,7 @@ def default_config(tmpdir_factory, config_directory, monkeypatch):
           for name in ['site/%s' % platform.system().lower(),
                        'site', 'user']])
 
-    monkeypatch.setattr(spack.config, 'config', cfg)
+    spack.config.config, old_config = cfg, spack.config.config
 
     # This is essential, otherwise the cache will create weird side effects
     # that will compromise subsequent tests if compilers.yaml is modified
@@ -148,18 +120,25 @@ def default_config(tmpdir_factory, config_directory, monkeypatch):
     timeout = spack.config.get('config:connect_timeout')
     if not timeout:
         spack.config.set('config:connect_timeout', 10, scope='user')
+
     yield spack.config.config
+
+    spack.config.config = old_config
     mutable_dir.remove()
 
 
 @pytest.fixture(scope='function')
 def install_dir_default_layout(tmpdir):
     """Hooks a fake install directory with a default layout"""
-    real_store = spack.store.store
+    scheme = os.path.join(
-    real_layout = spack.store.layout
+        '${architecture}',
-    spack.store.store = spack.store.Store(str(tmpdir.join('opt')))
+        '${compiler.name}-${compiler.version}',
-    spack.store.layout = YamlDirectoryLayout(str(tmpdir.join('opt')),
+        '${name}-${version}-${hash}'
-                                             path_scheme=def_install_path_scheme)  # noqa: E501
+    )
+    real_store, real_layout = spack.store.store, spack.store.layout
+    opt_dir = tmpdir.join('opt')
+    spack.store.store = spack.store.Store(str(opt_dir))
+    spack.store.layout = YamlDirectoryLayout(str(opt_dir), path_scheme=scheme)
     try:
         yield spack.store
     finally:
@@ -170,11 +149,14 @@ def install_dir_default_layout(tmpdir):
 @pytest.fixture(scope='function')
 def install_dir_non_default_layout(tmpdir):
     """Hooks a fake install directory with a non-default layout"""
-    real_store = spack.store.store
+    scheme = os.path.join(
-    real_layout = spack.store.layout
+        '${name}', '${version}',
-    spack.store.store = spack.store.Store(str(tmpdir.join('opt')))
+        '${architecture}-${compiler.name}-${compiler.version}-${hash}'
-    spack.store.layout = YamlDirectoryLayout(str(tmpdir.join('opt')),
+    )
-                                             path_scheme=ndef_install_path_scheme)  # noqa: E501
+    real_store, real_layout = spack.store.store, spack.store.layout
+    opt_dir = tmpdir.join('opt')
+    spack.store.store = spack.store.Store(str(opt_dir))
+    spack.store.layout = YamlDirectoryLayout(str(opt_dir), path_scheme=scheme)
     try:
         yield spack.store
     finally:
@@ -190,308 +172,149 @@ def install_dir_non_default_layout(tmpdir):
 
 
 @pytest.mark.requires_executables(*args)
-@pytest.mark.disable_clean_stage_check
 @pytest.mark.maybeslow
-@pytest.mark.usefixtures('default_config', 'cache_directory',
+@pytest.mark.usefixtures(
-                         'install_dir_default_layout')
+    'default_config', 'cache_directory', 'install_dir_default_layout',
-def test_default_rpaths_create_install_default_layout(tmpdir,
+    'test_mirror'
-                                                      mirror_directory_def,
+)
-                                                      install_mockery):
+def test_default_rpaths_create_install_default_layout(mirror_dir):
     """
     Test the creation and installation of buildcaches with default rpaths
     into the default directory layout scheme.
     """
+    gspec, cspec = Spec('garply').concretized(), Spec('corge').concretized()
 
-    gspec = Spec('garply')
+    # Install 'corge' without using a cache
-    gspec.concretize()
+    install_cmd('--no-cache', cspec.name)
-    cspec = Spec('corge')
-    cspec.concretize()
-
-    iparser = argparse.ArgumentParser()
-    install.setup_parser(iparser)
-    # Install some packages with dependent packages
-    iargs = iparser.parse_args(['--no-cache', cspec.name])
-    install.install(iparser, iargs)
-
-    global mirror_path_def
-    mirror_path_def = mirror_directory_def
-    mparser = argparse.ArgumentParser()
-    mirror.setup_parser(mparser)
-    margs = mparser.parse_args(
-        ['add', '--scope', 'site', 'test-mirror-def', 'file://%s' % mirror_path_def])
-    mirror.mirror(mparser, margs)
-    margs = mparser.parse_args(['list'])
-    mirror.mirror(mparser, margs)
-
-    # setup argument parser
-    parser = argparse.ArgumentParser()
-    buildcache.setup_parser(parser)
-
-    # Set default buildcache args
-    create_args = ['create', '-a', '-u', '-d', str(mirror_path_def),
-                   cspec.name]
-    install_args = ['install', '-a', '-u', cspec.name]
 
     # Create a buildache
-    args = parser.parse_args(create_args)
+    buildcache_cmd('create', '-au', '-d', mirror_dir, cspec.name)
-    buildcache.buildcache(parser, args)
+    # Test force overwrite create buildcache (-f option)
-    # Test force overwrite create buildcache
+    buildcache_cmd('create', '-auf', '-d', mirror_dir, cspec.name)
-    create_args.insert(create_args.index('-a'), '-f')
+
-    args = parser.parse_args(create_args)
+    # Create mirror index
-    buildcache.buildcache(parser, args)
+    mirror_url = 'file://{0}'.format(mirror_dir)
-    # create mirror index
+    buildcache_cmd('update-index', '-d', mirror_url)
-    args = parser.parse_args(['update-index', '-d', 'file://%s' % str(mirror_path_def)])
+    # List the buildcaches in the mirror
-    buildcache.buildcache(parser, args)
+    buildcache_cmd('list', '-alv')
-    # list the buildcaches in the mirror
-    args = parser.parse_args(['list', '-a', '-l', '-v'])
-    buildcache.buildcache(parser, args)
 
     # Uninstall the package and deps
-    uparser = argparse.ArgumentParser()
+    uninstall_cmd('-y', '--dependents', gspec.name)
-    uninstall.setup_parser(uparser)
-    uargs = uparser.parse_args(['-y', '--dependents', gspec.name])
-    uninstall.uninstall(uparser, uargs)
 
-    # test install
+    # Test installing from build caches
-    args = parser.parse_args(install_args)
+    buildcache_cmd('install', '-au', cspec.name)
-    buildcache.buildcache(parser, args)
 
     # This gives warning that spec is already installed
-    buildcache.buildcache(parser, args)
+    buildcache_cmd('install', '-au', cspec.name)
 
-    # test overwrite install
+    # Test overwrite install
-    install_args.insert(install_args.index('-a'), '-f')
+    buildcache_cmd('install', '-afu', cspec.name)
-    args = parser.parse_args(install_args)
-    buildcache.buildcache(parser, args)
 
-    args = parser.parse_args(['keys', '-f'])
+    buildcache_cmd('keys', '-f')
-    buildcache.buildcache(parser, args)
+    buildcache_cmd('list')
 
-    args = parser.parse_args(['list'])
+    buildcache_cmd('list', '-a')
-    buildcache.buildcache(parser, args)
+    buildcache_cmd('list', '-l', '-v')
-
-    args = parser.parse_args(['list', '-a'])
-    buildcache.buildcache(parser, args)
-
-    args = parser.parse_args(['list', '-l', '-v'])
-    buildcache.buildcache(parser, args)
-    bindist.clear_spec_cache()
-    spack.stage.purge()
-    margs = mparser.parse_args(
-        ['rm', '--scope', 'site', 'test-mirror-def'])
-    mirror.mirror(mparser, margs)
 
 
 @pytest.mark.requires_executables(*args)
-@pytest.mark.disable_clean_stage_check
 @pytest.mark.maybeslow
 @pytest.mark.nomockstage
-@pytest.mark.usefixtures('default_config', 'cache_directory',
+@pytest.mark.usefixtures(
-                         'install_dir_non_default_layout')
+    'default_config', 'cache_directory', 'install_dir_non_default_layout',
-def test_default_rpaths_install_nondefault_layout(tmpdir,
+    'test_mirror'
-                                                  install_mockery):
+)
+def test_default_rpaths_install_nondefault_layout(mirror_dir):
     """
     Test the creation and installation of buildcaches with default rpaths
     into the non-default directory layout scheme.
     """
-
+    cspec = Spec('corge').concretized()
-    gspec = Spec('garply')
-    gspec.concretize()
-    cspec = Spec('corge')
-    cspec.concretize()
-
-    global mirror_path_def
-    mparser = argparse.ArgumentParser()
-    mirror.setup_parser(mparser)
-    margs = mparser.parse_args(
-        ['add', '--scope', 'site', 'test-mirror-def', 'file://%s' % mirror_path_def])
-    mirror.mirror(mparser, margs)
-
-    # setup argument parser
-    parser = argparse.ArgumentParser()
-    buildcache.setup_parser(parser)
-
-    # Set default buildcache args
-    install_args = ['install', '-a', '-u', '%s' % cspec.name]
 
     # Install some packages with dependent packages
     # test install in non-default install path scheme
-    args = parser.parse_args(install_args)
+    buildcache_cmd('install', '-au', cspec.name)
-    buildcache.buildcache(parser, args)
-    # test force install in non-default install path scheme
-    install_args.insert(install_args.index('-a'), '-f')
-    args = parser.parse_args(install_args)
-    buildcache.buildcache(parser, args)
 
-    bindist.clear_spec_cache()
+    # Test force install in non-default install path scheme
-    spack.stage.purge()
+    buildcache_cmd('install', '-auf', cspec.name)
-    margs = mparser.parse_args(
-        ['rm', '--scope', 'site', 'test-mirror-def'])
-    mirror.mirror(mparser, margs)
 
 
 @pytest.mark.requires_executables(*args)
-@pytest.mark.disable_clean_stage_check
 @pytest.mark.maybeslow
 @pytest.mark.nomockstage
-@pytest.mark.usefixtures('default_config', 'cache_directory',
+@pytest.mark.usefixtures(
-                         'install_dir_default_layout')
+    'default_config', 'cache_directory', 'install_dir_default_layout'
-def test_relative_rpaths_create_default_layout(tmpdir,
+)
-                                               mirror_directory_rel,
+def test_relative_rpaths_create_default_layout(mirror_dir):
-                                               install_mockery):
     """
     Test the creation and installation of buildcaches with relative
     rpaths into the default directory layout scheme.
     """
 
-    gspec = Spec('garply')
+    gspec, cspec = Spec('garply').concretized(), Spec('corge').concretized()
-    gspec.concretize()
-    cspec = Spec('corge')
-    cspec.concretize()
 
-    global mirror_path_rel
+    # Install 'corge' without using a cache
-    mirror_path_rel = mirror_directory_rel
+    install_cmd('--no-cache', cspec.name)
-    # Install patchelf needed for relocate in linux test environment
-    iparser = argparse.ArgumentParser()
-    install.setup_parser(iparser)
-    # Install some packages with dependent packages
-    iargs = iparser.parse_args(['--no-cache', cspec.name])
-    install.install(iparser, iargs)
 
-    # setup argument parser
+    # Create build cache with relative rpaths
-    parser = argparse.ArgumentParser()
+    buildcache_cmd(
-    buildcache.setup_parser(parser)
+        'create', '-aur', '-d', mirror_dir, cspec.name
+    )
 
-    # set default buildcache args
+    # Create mirror index
-    create_args = ['create', '-a', '-u', '-r', '-d',
+    mirror_url = 'file://%s' % mirror_dir
-                   str(mirror_path_rel),
+    buildcache_cmd('update-index', '-d', mirror_url)
-                   cspec.name]
 
-    # create build cache with relatived rpaths
-    args = parser.parse_args(create_args)
-    buildcache.buildcache(parser, args)
-    # create mirror index
-    args = parser.parse_args(['update-index', '-d', 'file://%s' % str(mirror_path_rel)])
-    buildcache.buildcache(parser, args)
     # Uninstall the package and deps
-    uparser = argparse.ArgumentParser()
+    uninstall_cmd('-y', '--dependents', gspec.name)
-    uninstall.setup_parser(uparser)
-    uargs = uparser.parse_args(['-y', '--dependents', gspec.name])
-    uninstall.uninstall(uparser, uargs)
-
-    bindist.clear_spec_cache()
-    spack.stage.purge()
 
 
 @pytest.mark.requires_executables(*args)
-@pytest.mark.disable_clean_stage_check
 @pytest.mark.maybeslow
 @pytest.mark.nomockstage
-@pytest.mark.usefixtures('default_config', 'cache_directory',
+@pytest.mark.usefixtures(
-                         'install_dir_default_layout')
+    'default_config', 'cache_directory', 'install_dir_default_layout',
-def test_relative_rpaths_install_default_layout(tmpdir,
+    'test_mirror'
-                                                install_mockery):
+)
+def test_relative_rpaths_install_default_layout(mirror_dir):
     """
     Test the creation and installation of buildcaches with relative
     rpaths into the default directory layout scheme.
     """
+    gspec, cspec = Spec('garply').concretized(), Spec('corge').concretized()
 
-    gspec = Spec('garply')
+    # Install buildcache created with relativized rpaths
-    gspec.concretize()
+    buildcache_cmd('install', '-auf', cspec.name)
-    cspec = Spec('corge')
-    cspec.concretize()
-
-    global mirror_path_rel
-    mparser = argparse.ArgumentParser()
-    mirror.setup_parser(mparser)
-    margs = mparser.parse_args(
-        ['add', '--scope', 'site', 'test-mirror-rel', 'file://%s' % mirror_path_rel])
-    mirror.mirror(mparser, margs)
-
-    iparser = argparse.ArgumentParser()
-    install.setup_parser(iparser)
-
-    # setup argument parser
-    parser = argparse.ArgumentParser()
-    buildcache.setup_parser(parser)
-
-    # set default buildcache args
-    install_args = ['install', '-a', '-u', '-f',
-                    cspec.name]
-
-    # install buildcache created with relativized rpaths
-    args = parser.parse_args(install_args)
-    buildcache.buildcache(parser, args)
 
     # This gives warning that spec is already installed
-    buildcache.buildcache(parser, args)
+    buildcache_cmd('install', '-auf', cspec.name)
 
     # Uninstall the package and deps
-    uparser = argparse.ArgumentParser()
+    uninstall_cmd('-y', '--dependents', gspec.name)
-    uninstall.setup_parser(uparser)
-    uargs = uparser.parse_args(['-y', '--dependents', gspec.name])
-    uninstall.uninstall(uparser, uargs)
 
-    # install build cache
+    # Install build cache
-    buildcache.buildcache(parser, args)
+    buildcache_cmd('install', '-auf', cspec.name)
 
-    # test overwrite install
+    # Test overwrite install
-    install_args.insert(install_args.index('-a'), '-f')
+    buildcache_cmd('install', '-auf', cspec.name)
-    args = parser.parse_args(install_args)
-    buildcache.buildcache(parser, args)
-
-    bindist.clear_spec_cache()
-    spack.stage.purge()
-    margs = mparser.parse_args(
-        ['rm', '--scope', 'site', 'test-mirror-rel'])
-    mirror.mirror(mparser, margs)
 
 
 @pytest.mark.requires_executables(*args)
-@pytest.mark.disable_clean_stage_check
 @pytest.mark.maybeslow
 @pytest.mark.nomockstage
-@pytest.mark.usefixtures('default_config', 'cache_directory',
+@pytest.mark.usefixtures(
-                         'install_dir_non_default_layout')
+    'default_config', 'cache_directory', 'install_dir_non_default_layout',
-def test_relative_rpaths_install_nondefault(tmpdir,
+    'test_mirror'
-                                            install_mockery):
+)
+def test_relative_rpaths_install_nondefault(mirror_dir):
     """
     Test the installation of buildcaches with relativized rpaths
     into the non-default directory layout scheme.
     """
+    cspec = Spec('corge').concretized()
 
-    gspec = Spec('garply')
+    # Test install in non-default install path scheme and relative path
-    gspec.concretize()
+    buildcache_cmd('install', '-auf', cspec.name)
-    cspec = Spec('corge')
-    cspec.concretize()
-
-    global mirror_path_rel
-
-    mparser = argparse.ArgumentParser()
-    mirror.setup_parser(mparser)
-    margs = mparser.parse_args(
-        ['add', '--scope', 'site', 'test-mirror-rel', 'file://%s' % mirror_path_rel])
-    mirror.mirror(mparser, margs)
-
-    iparser = argparse.ArgumentParser()
-    install.setup_parser(iparser)
-
-    # setup argument parser
-    parser = argparse.ArgumentParser()
-    buildcache.setup_parser(parser)
-
-    # Set default buildcache args
-    install_args = ['install', '-a', '-u', '-f', '%s' % cspec.name]
-
-    # test install in non-default install path scheme and relative path
-    args = parser.parse_args(install_args)
-    buildcache.buildcache(parser, args)
-
-    bindist.clear_spec_cache()
-    spack.stage.purge()
-    margs = mparser.parse_args(
-        ['rm', '--scope', 'site', 'test-mirror-rel'])
-    mirror.mirror(mparser, margs)
 
 
 @pytest.mark.skipif(not spack.util.gpg.has_gpg(),
@@ -534,38 +357,20 @@ def test_push_and_fetch_keys(mock_gnupghome):
 
 
 @pytest.mark.requires_executables(*args)
-@pytest.mark.disable_clean_stage_check
 @pytest.mark.maybeslow
 @pytest.mark.nomockstage
-@pytest.mark.usefixtures('default_config', 'cache_directory',
+@pytest.mark.usefixtures(
-                         'install_dir_non_default_layout')
+    'default_config', 'cache_directory', 'install_dir_non_default_layout',
-def test_built_spec_cache(tmpdir,
+    'test_mirror'
-                          install_mockery):
+)
+def test_built_spec_cache(mirror_dir):
     """ Because the buildcache list command fetches the buildcache index
     and uses it to populate the binary_distribution built spec cache, when
     this test calls get_mirrors_for_spec, it is testing the popluation of
     that cache from a buildcache index. """
-    global mirror_path_rel
+    buildcache_cmd('list', '-a', '-l')
 
-    mparser = argparse.ArgumentParser()
+    gspec, cspec = Spec('garply').concretized(), Spec('corge').concretized()
-    mirror.setup_parser(mparser)
-    margs = mparser.parse_args(
-        ['add', '--scope', 'site', 'test-mirror-rel', 'file://%s' % mirror_path_rel])
-    mirror.mirror(mparser, margs)
-
-    # setup argument parser
-    parser = argparse.ArgumentParser()
-    buildcache.setup_parser(parser)
-
-    list_args = ['list', '-a', '-l']
-    args = parser.parse_args(list_args)
-    buildcache.buildcache(parser, args)
-
-    gspec = Spec('garply')
-    gspec.concretize()
-
-    cspec = Spec('corge')
-    cspec.concretize()
 
     full_hash_map = {
         'garply': gspec.full_hash(),
@@ -590,12 +395,6 @@ def test_built_spec_cache(tmpdir,
         assert(result['mirror_url'] not in cspec_mirrors)
         cspec_mirrors[result['mirror_url']] = True
 
-    bindist.clear_spec_cache()
-
-    margs = mparser.parse_args(
-        ['rm', '--scope', 'site', 'test-mirror-rel'])
-    mirror.mirror(mparser, margs)
-
 
 def fake_full_hash(spec):
     # Generate an arbitrary hash that is intended to be different than
@@ -604,8 +403,11 @@ def fake_full_hash(spec):
     return 'tal4c7h4z0gqmixb1eqa92mjoybxn5l6'
 
 
-def test_spec_needs_rebuild(install_mockery_mutable_config, mock_packages,
+@pytest.mark.usefixtures(
-                            mock_fetch, monkeypatch, tmpdir):
+    'install_mockery_mutable_config', 'mock_packages', 'mock_fetch',
+    'test_mirror'
+)
+def test_spec_needs_rebuild(monkeypatch, tmpdir):
     """Make sure needs_rebuild properly compares remote full_hash
     against locally computed one, avoiding unnecessary rebuilds"""
 
@@ -613,8 +415,6 @@ def test_spec_needs_rebuild(install_mockery_mutable_config, mock_packages,
     mirror_dir = tmpdir.join('mirror_dir')
     mirror_url = 'file://{0}'.format(mirror_dir.strpath)
 
-    mirror_cmd('add', 'test-mirror', mirror_url)
-
     s = Spec('libdwarf').concretized()
 
     # Install a package
@@ -683,24 +483,25 @@ def mock_list_url(url, recursive=False):
     assert expect in err
 
 
-@pytest.mark.usefixtures('mock_fetch')
+@pytest.mark.usefixtures('mock_fetch', 'install_mockery')
-def test_update_sbang(tmpdir, install_mockery, function_mirror):
+def test_update_sbang(tmpdir, test_mirror):
-    """
+    """Test the creation and installation of buildcaches with default rpaths
-    Test the creation and installation of buildcaches with default rpaths
     into the non-default directory layout scheme, triggering an update of the
     sbang.
     """
-
+    scheme = os.path.join(
+        '${name}', '${version}',
+        '${architecture}-${compiler.name}-${compiler.version}-${hash}'
+    )
     # Save the original store and layout before we touch ANYTHING.
-    real_store = spack.store.store
+    real_store, real_layout = spack.store.store, spack.store.layout
-    real_layout = spack.store.layout
 
     # Concretize a package with some old-fashioned sbang lines.
     sspec = Spec('old-sbang')
     sspec.concretize()
 
     # Need a fake mirror with *function* scope.
-    mirror_dir = function_mirror
+    mirror_dir = test_mirror
 
     # Assumes all commands will concretize sspec the same way.
     install_cmd('--no-cache', sspec.name)
@@ -718,9 +519,11 @@ def test_update_sbang(tmpdir, install_mockery, function_mirror):
     try:
         # New install tree locations...
         # Too fine-grained to do be done in a fixture
-        spack.store.store = spack.store.Store(str(tmpdir.join('newtree')))
+        newtree_dir = tmpdir.join('newtree')
-        spack.store.layout = YamlDirectoryLayout(str(tmpdir.join('newtree')),
+        spack.store.store = spack.store.Store(str(newtree_dir))
-                                                 path_scheme=ndef_install_path_scheme)  # noqa: E501
+        spack.store.layout = YamlDirectoryLayout(
+            str(newtree_dir), path_scheme=scheme
+        )
 
         # Install package from buildcache
         buildcache_cmd('install', '-a', '-u', '-f', sspec.name)