From 7368586f0dfc25a97d982636e1eda9e5ebb5d41b Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Mon, 29 Jan 2018 15:19:50 +0100 Subject: 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 | 52 +++++++++++++++++++++ lib/spack/spack/test/cmd/dependencies.py | 4 ++ lib/spack/spack/test/cmd/dependents.py | 4 ++ lib/spack/spack/test/cmd/find.py | 37 ++++++++------- lib/spack/spack/test/cmd/gpg.py | 1 + lib/spack/spack/test/cmd/graph.py | 29 +++++++----- lib/spack/spack/test/cmd/list.py | 2 + lib/spack/spack/test/cmd/module.py | 17 +++++-- lib/spack/spack/test/cmd/uninstall.py | 12 +++-- lib/spack/spack/test/cmd/url.py | 2 + lib/spack/spack/test/cmd/versions.py | 6 ++- lib/spack/spack/test/database.py | 79 ++++++++++++++++++-------------- lib/spack/spack/test/package_sanity.py | 3 ++ lib/spack/spack/test/python_version.py | 4 ++ lib/spack/spack/test/spec_syntax.py | 7 +++ pytest.ini | 6 ++- 16 files changed, 193 insertions(+), 72 deletions(-) create mode 100644 conftest.py diff --git a/conftest.py b/conftest.py new file mode 100644 index 0000000000..85adf54d1d --- /dev/null +++ b/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) diff --git a/lib/spack/spack/test/cmd/dependencies.py b/lib/spack/spack/test/cmd/dependencies.py index 737a74caae..6a8d845fea 100644 --- a/lib/spack/spack/test/cmd/dependencies.py +++ b/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') diff --git a/lib/spack/spack/test/cmd/dependents.py b/lib/spack/spack/test/cmd/dependents.py index acd64ec3d4..9a20f3639b 100644 --- a/lib/spack/spack/test/cmd/dependents.py +++ b/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') diff --git a/lib/spack/spack/test/cmd/find.py b/lib/spack/spack/test/cmd/find.py index 69973e715b..b946141a87 100644 --- a/lib/spack/spack/test/cmd/find.py +++ b/lib/spack/spack/test/cmd/find.py @@ -84,27 +84,32 @@ 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(parser, specs): - def test_tag1(self, parser, specs): + args = parser.parse_args(['--tags', 'tag1']) + spack.cmd.find.find(parser, args) - args = parser.parse_args(['--tags', 'tag1']) - spack.cmd.find.find(parser, args) + assert len(specs) == 2 + assert 'mpich' in [x.name for x in specs] + assert 'mpich2' in [x.name for x in specs] - assert len(specs) == 2 - assert 'mpich' in [x.name for x in specs] - assert 'mpich2' in [x.name for x in specs] - def test_tag2(self, parser, specs): - args = parser.parse_args(['--tags', 'tag2']) - spack.cmd.find.find(parser, args) +@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] - assert len(specs) == 1 - assert 'mpich' in [x.name for x in specs] - def test_tag2_tag3(self, parser, specs): - args = parser.parse_args(['--tags', 'tag2', '--tags', 'tag3']) - spack.cmd.find.find(parser, args) +@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) - assert len(specs) == 0 + assert len(specs) == 0 diff --git a/lib/spack/spack/test/cmd/gpg.py b/lib/spack/spack/test/cmd/gpg.py index ef2f64211a..dadaba5833 100644 --- a/lib/spack/spack/test/cmd/gpg.py +++ b/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): diff --git a/lib/spack/spack/test/cmd/graph.py b/lib/spack/spack/test/cmd/graph.py index 3278407d3f..8350a26d55 100644 --- a/lib/spack/spack/test/cmd/graph.py +++ b/lib/spack/spack/test/cmd/graph.py @@ -30,31 +30,37 @@ import pytest 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') diff --git a/lib/spack/spack/test/cmd/list.py b/lib/spack/spack/test/cmd/list.py index 976bacb9fb..33339026b4 100644 --- a/lib/spack/spack/test/cmd/list.py +++ b/lib/spack/spack/test/cmd/list.py @@ -79,6 +79,7 @@ class TestListCommand(object): 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 @@ class TestListCommand(object): 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']) diff --git a/lib/spack/spack/test/cmd/module.py b/lib/spack/spack/test/cmd/module.py index f56083df6e..3cdd5e81c2 100644 --- a/lib/spack/spack/test/cmd/module.py +++ b/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] diff --git a/lib/spack/spack/test/cmd/uninstall.py b/lib/spack/spack/test/cmd/uninstall.py index 096dfe74e4..280db6f126 100644 --- a/lib/spack/spack/test/cmd/uninstall.py +++ b/lib/spack/spack/test/cmd/uninstall.py @@ -39,19 +39,25 @@ class MockArgs(object): 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') diff --git a/lib/spack/spack/test/cmd/url.py b/lib/spack/spack/test/cmd/url.py index 84fcf90135..724dffaa4f 100644 --- a/lib/spack/spack/test/cmd/url.py +++ b/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 diff --git a/lib/spack/spack/test/cmd/versions.py b/lib/spack/spack/test/cmd/versions.py index 7da177eaef..76636ce689 100644 --- a/lib/spack/spack/test/cmd/versions.py +++ b/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.""" diff --git a/lib/spack/spack/test/database.py b/lib/spack/spack/test/database.py index 72ae6ad278..4c17fc8607 100644 --- a/lib/spack/spack/test/database.py +++ b/lib/spack/spack/test/database.py @@ -38,6 +38,9 @@ from spack.test.conftest import MockPackageMultiRepo 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 diff --git a/lib/spack/spack/test/package_sanity.py b/lib/spack/spack/test/package_sanity.py index adc6867b72..e1259eaf7e 100644 --- a/lib/spack/spack/test/package_sanity.py +++ b/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() diff --git a/lib/spack/spack/test/python_version.py b/lib/spack/spack/test/python_version.py index b9bb18bfd2..6d0c42fcde 100644 --- a/lib/spack/spack/test/python_version.py +++ b/lib/spack/spack/test/python_version.py @@ -37,6 +37,8 @@ import os 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])) diff --git a/lib/spack/spack/test/spec_syntax.py b/lib/spack/spack/test/spec_syntax.py index 009cb5c129..f134319532 100644 --- a/lib/spack/spack/test/spec_syntax.py +++ b/lib/spack/spack/test/spec_syntax.py @@ -253,6 +253,7 @@ class TestSpecSyntax(object): 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 @@ class TestSpecSyntax(object): 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 @@ class TestSpecSyntax(object): 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 @@ class TestSpecSyntax(object): ' / ' + 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 @@ class TestSpecSyntax(object): # 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 @@ class TestSpecSyntax(object): '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 @@ class TestSpecSyntax(object): '/' + no_such_hash, 'mpileaks /' + no_such_hash]) + @pytest.mark.db def test_redundant_spec(self, database): """Check that redundant spec constraints raise errors. diff --git a/pytest.ini b/pytest.ini index 0d8d2b271f..e8f5e33c71 100644 --- a/pytest.ini +++ b/pytest.ini @@ -2,4 +2,8 @@ [pytest] addopts = --durations=20 -ra testpaths = lib/spack/spack/test -python_files = *.py \ No newline at end of file +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.) -- cgit v1.2.3-60-g2f50