Mark slow unit tests (#6994)

* Marking database tests as slow

* Marking url command tests as slow

* Marking every test that uses database as slow

* Marking tests that import files as slow

* Marking gpg tests as slow

* Marking all versions and one list tests as slow

* Added more markers to unit tests + cli option to skip slow tests

Following a discussion with Axel, the generic 'slowtest' marker has been
split into 'db', 'network' and 'maybeslow'. A brief description of the
meaning of each marker has been added to pytest.ini.

A command line option to run only fast tests has been added to
'spack test'

* Don't use classes to group tests together

Reverted grouping tests under a class, as required in the review

* Minor style changes

conftest.py

@@ -0,0 +1,52 @@
+##############################################################################
+# Copyright (c) 2013-2017, Lawrence Livermore National Security, LLC.
+# Produced at the Lawrence Livermore National Laboratory.
+#
+# This file is part of Spack.
+# Created by Todd Gamblin, tgamblin@llnl.gov, All rights reserved.
+# LLNL-CODE-647188
+#
+# For details, see https://github.com/spack/spack
+# Please also see the NOTICE and LICENSE files for our notice and the LGPL.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU Lesser General Public License (as
+# published by the Free Software Foundation) version 2.1, February 1999.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and
+# conditions of the GNU Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+##############################################################################
+
+import pytest
+
+
+# Hooks to add command line options or set other custom behaviors.
+# They must be placed here to be found by pytest. See:
+#
+# https://docs.pytest.org/en/latest/writing_plugins.html
+#
+def pytest_addoption(parser):
+    group = parser.getgroup("Spack specific command line options")
+    group.addoption(
+        '--fast', action='store_true', default=False,
+        help='runs only "fast" unit tests, instead of the whole suite')
+
+
+def pytest_collection_modifyitems(config, items):
+    if not config.getoption('--fast'):
+        # --fast not given, run all the tests
+        return
+
+    slow_tests = ['db', 'network', 'maybeslow']
+    skip_as_slow = pytest.mark.skip(
+        reason='skipped slow test [--fast command line option given]'
+    )
+    for item in items:
+        if any(x in item.keywords for x in slow_tests):
+            item.add_marker(skip_as_slow)

lib/spack/spack/test/cmd/dependencies.py

@@ -24,6 +24,8 @@
 ##############################################################################
 import re
 
+import pytest
+
 from llnl.util.tty.color import color_when
 
 import spack
@@ -50,6 +52,7 @@ def test_transitive_dependencies(builtin_mock):
     assert expected == actual
 
 
+@pytest.mark.db
 def test_immediate_installed_dependencies(builtin_mock, database):
     with color_when(False):
         out = dependencies('--installed', 'mpileaks^mpich')
@@ -63,6 +66,7 @@ def test_immediate_installed_dependencies(builtin_mock, database):
     assert expected == hashes
 
 
+@pytest.mark.db
 def test_transitive_installed_dependencies(builtin_mock, database):
     with color_when(False):
         out = dependencies('--installed', '--transitive', 'mpileaks^zmpi')

lib/spack/spack/test/cmd/dependents.py

@@ -24,6 +24,8 @@
 ##############################################################################
 import re
 
+import pytest
+
 from llnl.util.tty.color import color_when
 
 import spack
@@ -48,6 +50,7 @@ def test_transitive_dependents(builtin_mock):
          'patch-a-dependency', 'patch-several-dependencies'])
 
 
+@pytest.mark.db
 def test_immediate_installed_dependents(builtin_mock, database):
     with color_when(False):
         out = dependents('--installed', 'libelf')
@@ -64,6 +67,7 @@ def test_immediate_installed_dependents(builtin_mock, database):
     assert expected == hashes
 
 
+@pytest.mark.db
 def test_transitive_installed_dependents(builtin_mock, database):
     with color_when(False):
         out = dependents('--installed', '--transitive', 'fake')

lib/spack/spack/test/cmd/find.py

@@ -84,10 +84,9 @@ def test_query_arguments():
     assert q_args['explicit'] is False
 
 
+@pytest.mark.db
 @pytest.mark.usefixtures('database', 'mock_display')
-class TestFindWithTags(object):
-
-    def test_tag1(self, parser, specs):
+def test_tag1(parser, specs):
 
     args = parser.parse_args(['--tags', 'tag1'])
     spack.cmd.find.find(parser, args)
@@ -96,14 +95,20 @@ def test_tag1(self, parser, specs):
     assert 'mpich' in [x.name for x in specs]
     assert 'mpich2' in [x.name for x in specs]
 
-    def test_tag2(self, parser, specs):
+
+@pytest.mark.db
+@pytest.mark.usefixtures('database', 'mock_display')
+def test_tag2(parser, specs):
     args = parser.parse_args(['--tags', 'tag2'])
     spack.cmd.find.find(parser, args)
 
     assert len(specs) == 1
     assert 'mpich' in [x.name for x in specs]
 
-    def test_tag2_tag3(self, parser, specs):
+
+@pytest.mark.db
+@pytest.mark.usefixtures('database', 'mock_display')
+def test_tag2_tag3(parser, specs):
     args = parser.parse_args(['--tags', 'tag2', '--tags', 'tag3'])
     spack.cmd.find.find(parser, args)
 

lib/spack/spack/test/cmd/gpg.py

@@ -52,6 +52,7 @@ def has_gnupg2():
         return False
 
 
+@pytest.mark.maybeslow
 @pytest.mark.skipif(not has_gnupg2(),
                     reason='These tests require gnupg2')
 def test_gpg(gpg, tmpdir, testing_gpg_directory):

lib/spack/spack/test/cmd/graph.py

@@ -30,31 +30,37 @@
 graph = SpackCommand('graph')
 
 
-def test_graph_ascii(builtin_mock, database):
+@pytest.mark.db
+@pytest.mark.usefixtures('builtin_mock', 'database')
+def test_graph_ascii():
     """Tests spack graph --ascii"""
-
     graph('--ascii', 'dt-diamond')
 
 
-def test_graph_dot(builtin_mock, database):
+@pytest.mark.db
+@pytest.mark.usefixtures('builtin_mock', 'database')
+def test_graph_dot():
     """Tests spack graph --dot"""
-
     graph('--dot', 'dt-diamond')
 
 
-def test_graph_normalize(builtin_mock, database):
+@pytest.mark.db
+@pytest.mark.usefixtures('builtin_mock', 'database')
+def test_graph_normalize():
     """Tests spack graph --normalize"""
-
     graph('--normalize', 'dt-diamond')
 
 
-def test_graph_static(builtin_mock, database):
+@pytest.mark.db
+@pytest.mark.usefixtures('builtin_mock', 'database')
+def test_graph_static():
     """Tests spack graph --static"""
-
     graph('--static', 'dt-diamond')
 
 
-def test_graph_installed(builtin_mock, database):
+@pytest.mark.db
+@pytest.mark.usefixtures('builtin_mock', 'database')
+def test_graph_installed():
     """Tests spack graph --installed"""
 
     graph('--installed')
@@ -63,9 +69,10 @@ def test_graph_installed(builtin_mock, database):
         graph('--installed', 'dt-diamond')
 
 
-def test_graph_deptype(builtin_mock, database):
+@pytest.mark.db
+@pytest.mark.usefixtures('builtin_mock', 'database')
+def test_graph_deptype():
     """Tests spack graph --deptype"""
-
     graph('--deptype', 'all', 'dt-diamond')
 
 

lib/spack/spack/test/cmd/list.py

@@ -79,6 +79,7 @@ def test_list_filter(self, parser, pkg_names):
         assert 'py-numpy' in pkg_names
         assert 'perl-file-copy-recursive' in pkg_names
 
+    @pytest.mark.maybeslow
     def test_list_search_description(self, parser, pkg_names):
         args = parser.parse_args(['--search-description', 'xml'])
         spack.cmd.list.list(parser, args)
@@ -94,6 +95,7 @@ def test_list_tags(self, parser, pkg_names):
         assert 'cloverleaf3d' in pkg_names
         assert 'hdf5' not in pkg_names
 
+    @pytest.mark.maybeslow
     def test_list_formatter(self, parser, pkg_names):
         # TODO: Test the output of the commands
         args = parser.parse_args(['--format', 'name_only'])

lib/spack/spack/test/cmd/module.py

@@ -67,14 +67,17 @@ def failure_args(request):
 # TODO : this requires having a separate directory for test modules
 # TODO : add tests for loads and find to check the prompt format
 
-
-def test_exit_with_failure(database, parser, failure_args):
+@pytest.mark.db
+@pytest.mark.usefixtures('database')
+def test_exit_with_failure(parser, failure_args):
     args = parser.parse_args(failure_args)
     with pytest.raises(SystemExit):
         module.module(parser, args)
 
 
-def test_remove_and_add_tcl(database, parser):
+@pytest.mark.db
+@pytest.mark.usefixtures('database')
+def test_remove_and_add_tcl(parser):
     """Tests adding and removing a tcl module file."""
 
     # Remove existing modules [tcl]
@@ -97,11 +100,13 @@ def test_remove_and_add_tcl(database, parser):
         assert os.path.exists(item)
 
 
+@pytest.mark.db
+@pytest.mark.usefixtures('database')
 @pytest.mark.parametrize('cli_args', [
     ['--module-type', 'tcl', 'libelf'],
     ['--module-type', 'tcl', '--full-path', 'libelf']
 ])
-def test_find(database, parser, cli_args):
+def test_find(parser, cli_args):
     """Tests the 'spack module find' under a few common scenarios."""
 
     # Try to find it for tcl module files
@@ -109,7 +114,9 @@ def test_find(database, parser, cli_args):
     module.module(parser, args)
 
 
-def test_remove_and_add_dotkit(database, parser):
+@pytest.mark.db
+@pytest.mark.usefixtures('database')
+def test_remove_and_add_dotkit(parser):
     """Tests adding and removing a dotkit module file."""
 
     # Remove existing modules [dotkit]

lib/spack/spack/test/cmd/uninstall.py

@@ -39,19 +39,25 @@ def __init__(self, packages, all=False, force=False, dependents=False):
         self.yes_to_all = True
 
 
-def test_multiple_matches(database):
+@pytest.mark.db
+@pytest.mark.usefixtures('database')
+def test_multiple_matches():
     """Test unable to uninstall when multiple matches."""
     with pytest.raises(SpackCommandError):
         uninstall('-y', 'mpileaks')
 
 
-def test_installed_dependents(database):
+@pytest.mark.db
+@pytest.mark.usefixtures('database')
+def test_installed_dependents():
     """Test can't uninstall when ther are installed dependents."""
     with pytest.raises(SpackCommandError):
         uninstall('-y', 'libelf')
 
 
-def test_recursive_uninstall(database):
+@pytest.mark.db
+@pytest.mark.usefixtures('database')
+def test_recursive_uninstall():
     """Test recursive uninstall."""
     uninstall('-y', '-a', '--dependents', 'callpath')
 

lib/spack/spack/test/cmd/url.py

@@ -83,6 +83,7 @@ def test_url_with_no_version_fails():
         url('parse', 'http://www.netlib.org/voronoi/triangle.zip')
 
 
+@pytest.mark.network
 def test_url_list():
     out = url('list')
     total_urls = len(out.split('\n'))
@@ -112,6 +113,7 @@ def test_url_list():
     assert 0 < correct_version_urls < total_urls
 
 
+@pytest.mark.network
 def test_url_summary():
     """Test the URL summary command."""
     # test url_summary, the internal function that does the work

lib/spack/spack/test/cmd/versions.py

@@ -22,24 +22,28 @@
 # License along with this program; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
-from spack.main import SpackCommand
+import pytest
 
+from spack.main import SpackCommand
 
 versions = SpackCommand('versions')
 
 
+@pytest.mark.network
 def test_remote_versions():
     """Test a package for which remote versions should be available."""
 
     versions('zlib')
 
 
+@pytest.mark.network
 def test_no_versions():
     """Test a package for which no remote versions are available."""
 
     versions('converge')
 
 
+@pytest.mark.network
 def test_no_unchecksummed_versions():
     """Test a package for which no unchecksummed versions are available."""
 

lib/spack/spack/test/database.py

@@ -38,6 +38,9 @@
 from spack.util.executable import Executable
 
 
+pytestmark = pytest.mark.db
+
+
 def _print_ref_counts():
     """Print out all ref counts for the graph used here, for debugging"""
     recs = []
@@ -105,6 +108,36 @@ def _check_db_sanity(install_db):
     _check_merkleiness()
 
 
+def _check_remove_and_add_package(install_db, spec):
+    """Remove a spec from the DB, then add it and make sure everything's
+    still ok once it is added.  This checks that it was
+    removed, that it's back when added again, and that ref
+    counts are consistent.
+    """
+    original = install_db.query()
+    install_db._check_ref_counts()
+
+    # Remove spec
+    concrete_spec = install_db.remove(spec)
+    install_db._check_ref_counts()
+    remaining = install_db.query()
+
+    # ensure spec we removed is gone
+    assert len(original) - 1 == len(remaining)
+    assert all(s in original for s in remaining)
+    assert concrete_spec not in remaining
+
+    # add it back and make sure everything is ok.
+    install_db.add(concrete_spec, spack.store.layout)
+    installed = install_db.query()
+    assert concrete_spec in installed
+    assert installed == original
+
+    # sanity check against direcory layout and check ref counts.
+    _check_db_sanity(install_db)
+    install_db._check_ref_counts()
+
+
 def _mock_install(spec):
     s = spack.spec.Spec(spec)
     s.concretize()
@@ -170,9 +203,15 @@ def test_010_all_install_sanity(database):
     assert len(libelf_specs) == 1
 
     # Query by dependency
-    assert len([s for s in all_specs if s.satisfies('mpileaks ^mpich')]) == 1
-    assert len([s for s in all_specs if s.satisfies('mpileaks ^mpich2')]) == 1
-    assert len([s for s in all_specs if s.satisfies('mpileaks ^zmpi')]) == 1
+    assert len(
+        [s for s in all_specs if s.satisfies('mpileaks ^mpich')]
+    ) == 1
+    assert len(
+        [s for s in all_specs if s.satisfies('mpileaks ^mpich2')]
+    ) == 1
+    assert len(
+        [s for s in all_specs if s.satisfies('mpileaks ^zmpi')]
+    ) == 1
 
 
 def test_015_write_and_read(database):
@@ -255,36 +294,6 @@ def test_050_basic_query(database):
     assert len(install_db.query('mpileaks ^zmpi')) == 1
 
 
-def _check_remove_and_add_package(install_db, spec):
-    """Remove a spec from the DB, then add it and make sure everything's
-    still ok once it is added.  This checks that it was
-    removed, that it's back when added again, and that ref
-    counts are consistent.
-    """
-    original = install_db.query()
-    install_db._check_ref_counts()
-
-    # Remove spec
-    concrete_spec = install_db.remove(spec)
-    install_db._check_ref_counts()
-    remaining = install_db.query()
-
-    # ensure spec we removed is gone
-    assert len(original) - 1 == len(remaining)
-    assert all(s in original for s in remaining)
-    assert concrete_spec not in remaining
-
-    # add it back and make sure everything is ok.
-    install_db.add(concrete_spec, spack.store.layout)
-    installed = install_db.query()
-    assert concrete_spec in installed
-    assert installed == original
-
-    # sanity check against direcory layout and check ref counts.
-    _check_db_sanity(install_db)
-    install_db._check_ref_counts()
-
-
 def test_060_remove_and_add_root_package(database):
     install_db = database.mock.db
     _check_remove_and_add_package(install_db, 'mpileaks ^mpich')
@@ -394,8 +403,8 @@ def test_115_reindex_with_packages_not_in_repo(database, refresh_db_on_exit):
 
     saved_repo = spack.repo
     # Dont add any package definitions to this repository, the idea is that
-    # packages should not have to be defined in the repository once they are
-    # installed
+    # packages should not have to be defined in the repository once they
+    # are installed
     mock_repo = MockPackageMultiRepo([])
     try:
         spack.repo = mock_repo

lib/spack/spack/test/package_sanity.py

@@ -26,6 +26,8 @@
 
 import re
 
+import pytest
+
 import spack
 from spack.repository import RepoPath
 
@@ -36,6 +38,7 @@ def check_db():
         spack.repo.get(name)
 
 
+@pytest.mark.maybeslow
 def test_get_all_packages():
     """Get all packages once and make sure that works."""
     check_db()

lib/spack/spack/test/python_version.py

@@ -37,6 +37,8 @@
 import sys
 import re
 
+import pytest
+
 import llnl.util.tty as tty
 import spack
 
@@ -154,11 +156,13 @@ def check_python_versions(files):
     assert not all_issues
 
 
+@pytest.mark.maybeslow
 def test_core_module_compatibility():
     """Test that all core spack modules work with supported Python versions."""
     check_python_versions(pyfiles([spack.lib_path], exclude=exclude_paths))
 
 
+@pytest.mark.maybeslow
 def test_package_module_compatibility():
     """Test that all spack packages work with supported Python versions."""
     check_python_versions(pyfiles([spack.packages_path]))

lib/spack/spack/test/spec_syntax.py

@@ -253,6 +253,7 @@ def _check_hash_parse(self, spec):
             str(spec), spec.name + '@' + str(spec.version) +
             ' /' + spec.dag_hash()[:6])
 
+    @pytest.mark.db
     def test_spec_by_hash(self, database):
         specs = database.mock.db.query()
         assert len(specs)  # make sure something's in the DB
@@ -260,6 +261,7 @@ def test_spec_by_hash(self, database):
         for spec in specs:
             self._check_hash_parse(spec)
 
+    @pytest.mark.db
     def test_dep_spec_by_hash(self, database):
         mpileaks_zmpi = database.mock.db.query_one('mpileaks ^zmpi')
         zmpi = database.mock.db.query_one('zmpi')
@@ -287,6 +289,7 @@ def test_dep_spec_by_hash(self, database):
         assert 'fake' in mpileaks_hash_fake_and_zmpi
         assert mpileaks_hash_fake_and_zmpi['fake'] == fake
 
+    @pytest.mark.db
     def test_multiple_specs_with_hash(self, database):
         mpileaks_zmpi = database.mock.db.query_one('mpileaks ^zmpi')
         callpath_mpich2 = database.mock.db.query_one('callpath ^mpich2')
@@ -319,6 +322,7 @@ def test_multiple_specs_with_hash(self, database):
                          ' / ' + callpath_mpich2.dag_hash())
         assert len(specs) == 2
 
+    @pytest.mark.db
     def test_ambiguous_hash(self, database):
         x1 = Spec('a')
         x1._hash = 'xy'
@@ -335,6 +339,7 @@ def test_ambiguous_hash(self, database):
         # ambiguity in first hash character AND spec name
         self._check_raises(AmbiguousHashError, ['a/x'])
 
+    @pytest.mark.db
     def test_invalid_hash(self, database):
         mpileaks_zmpi = database.mock.db.query_one('mpileaks ^zmpi')
         zmpi = database.mock.db.query_one('zmpi')
@@ -352,6 +357,7 @@ def test_invalid_hash(self, database):
             'mpileaks ^mpich /' + mpileaks_zmpi.dag_hash(),
             'mpileaks ^zmpi /' + mpileaks_mpich.dag_hash()])
 
+    @pytest.mark.db
     def test_nonexistent_hash(self, database):
         """Ensure we get errors for nonexistant hashes."""
         specs = database.mock.db.query()
@@ -365,6 +371,7 @@ def test_nonexistent_hash(self, database):
             '/' + no_such_hash,
             'mpileaks /' + no_such_hash])
 
+    @pytest.mark.db
     def test_redundant_spec(self, database):
         """Check that redundant spec constraints raise errors.
 

pytest.ini

@@ -3,3 +3,7 @@
 addopts = --durations=20 -ra
 testpaths = lib/spack/spack/test
 python_files = *.py
+markers =
+  db: tests that require creating a DB
+  network: tests that require access to the network
+  maybeslow: tests that may be slow (e.g. access a lot the filesystem, etc.)