unit tests: replace nose with pytest

[alalazo]

TLDR

This PR changes the testing framework from nose to pytest for the reasons discussed in #2368.

Added benefits:

• fixtures fit well the combinatorial problem we will be facing when adding install tests for packages
• using fixtures scope the execution time for tests has been trimmed down to 3-4 minutes

Modifications

• removed nose from lib/spack/external/
• made spack test a wrapper around pytest
• rewrote all the mocking classes as a set of fixtures
• removed the mocking classes
• ported all the tests that required mocking to pytest syntax

I left the port of tests that are written as plain unittest.TestCase for future PRs: this one is already huge as it is, and tests that just depend on unittest.TestCase can be ported one module at a time.

Miscellaneous Notes

• pytest is better run from spack root directory
• spack test is just a wrapper around pytest that can be used everywhere
• if coverage is needed run pytest --cov, as spack test will give wrong results (for any other thing the two commands are equivalent)

Running spack test without pytest:

$ spack test
==> Error: Install 'pytest' to have the 'spack test' command available

Running the entire test suite

$ spack test
=============================================================================================== test session starts ===============================================================================================
platform linux2 -- Python 2.7.6, pytest-3.0.5, py-1.4.32, pluggy-0.4.0
rootdir: /home/mculpo/PycharmProjects/spack, inifile: pytest.ini
plugins: cov-2.4.0
collected 470 items 

lib/spack/spack/test/architecture.py .......
<more output skipped>
lib/spack/spack/test/cmd/uninstall.py .

============================================================================================ slowest 20 test durations ============================================================================================
4.35s setup    lib/spack/spack/test/database.py::test_005_db_exists
4.19s teardown lib/spack/spack/test/database.py::test_030_db_sanity_from_another_process
3.94s setup    lib/spack/spack/test/cmd/uninstall.py::test_uninstall
3.93s setup    lib/spack/spack/test/cmd/module.py::test_exit_with_failure[failure_args0]
2.45s call     lib/spack/spack/test/python_version.py::PythonVersionTest::test_core_module_compatibility
1.80s call     lib/spack/spack/test/python_version.py::PythonVersionTest::test_package_module_compatibility
1.03s call     lib/spack/spack/test/mirror.py::TestMirror::test_all_mirror
0.84s call     lib/spack/spack/test/spec_yaml.py::test_ordered_read_not_required_for_consistent_dag_hash
0.79s call     lib/spack/spack/test/directory_layout.py::test_read_and_write_spec
0.76s call     lib/spack/spack/test/lock.py::LockTest::test_complex_acquire_and_release_chain
0.71s call     lib/spack/spack/test/modules.py::TestTcl::test_autoload
0.61s call     lib/spack/spack/test/concretize.py::TestConcretize::test_concretize_with_restricted_virtual
0.54s call     lib/spack/spack/test/database.py::test_025_reindex
0.42s call     lib/spack/spack/test/architecture.py::test_user_input_combination
0.42s call     lib/spack/spack/test/modules.py::TestTcl::test_prerequisites
0.42s setup    lib/spack/spack/test/hg_fetch.py::test_fetch[default]
0.38s call     lib/spack/spack/test/install.py::test_store
0.37s setup    lib/spack/spack/test/mirror.py::TestMirror::test_svn_mirror
0.36s call     lib/spack/spack/test/modules.py::TestLmod::test_autoload
0.35s call     lib/spack/spack/test/modules.py::TestTcl::test_blacklist
=========================================================================================== 470 passed in 50.75 seconds ===========================================================================================

Run a few tests selectively

$ spack test -k 'spec_dag or concretize'
=============================================================================================== test session starts ===============================================================================================
platform linux2 -- Python 2.7.6, pytest-3.0.5, py-1.4.32, pluggy-0.4.0
rootdir: /home/mculpo/PycharmProjects/spack, inifile: pytest.ini
plugins: cov-2.4.0
collected 470 items 

lib/spack/spack/test/concretize.py ...........................................
lib/spack/spack/test/concretize_preferences.py .....
lib/spack/spack/test/spec_dag.py .............................

============================================================================================ slowest 20 test durations ============================================================================================
0.65s call     lib/spack/spack/test/concretize.py::TestConcretize::test_concretize_with_restricted_virtual
0.42s call     lib/spack/spack/test/concretize_preferences.py::TestConcretizePreferences::test_preferred_compilers
0.30s call     lib/spack/spack/test/concretize_preferences.py::TestConcretizePreferences::test_preferred_variants
0.28s call     lib/spack/spack/test/spec_dag.py::TestSpecDag::test_deptype_traversal
0.28s call     lib/spack/spack/test/spec_dag.py::TestSpecDag::test_deptype_traversal_run
0.25s call     lib/spack/spack/test/spec_dag.py::TestSpecDag::test_deptype_traversal_full
0.24s call     lib/spack/spack/test/concretize_preferences.py::TestConcretizePreferences::test_preferred_providers
0.24s call     lib/spack/spack/test/spec_dag.py::TestSpecDag::test_copy_concretized
0.21s call     lib/spack/spack/test/spec_dag.py::TestSpecDag::test_preorder_edge_traversal
0.21s call     lib/spack/spack/test/spec_dag.py::TestSpecDag::test_postorder_edge_traversal
0.21s call     lib/spack/spack/test/spec_dag.py::TestSpecDag::test_preorder_node_traversal
0.21s call     lib/spack/spack/test/spec_dag.py::TestSpecDag::test_deptype_traversal_with_builddeps
0.20s call     lib/spack/spack/test/spec_dag.py::TestSpecDag::test_normalize_twice
0.19s call     lib/spack/spack/test/spec_dag.py::TestSpecDag::test_preorder_path_traversal
0.18s call     lib/spack/spack/test/concretize.py::TestConcretize::test_concretize_link_dep_of_build_dep
0.18s call     lib/spack/spack/test/spec_dag.py::TestSpecDag::test_postorder_node_traversal
0.18s call     lib/spack/spack/test/spec_dag.py::TestSpecDag::test_postorder_path_traversal
0.17s call     lib/spack/spack/test/concretize.py::TestConcretize::test_concretize[callpath]
0.16s call     lib/spack/spack/test/concretize_preferences.py::TestConcretizePreferences::test_preferred_versions
0.15s call     lib/spack/spack/test/spec_dag.py::TestSpecDag::test_normalize_mpileaks
============================================================================================== 393 tests deselected ===============================================================================================
==================================================================================== 77 passed, 393 deselected in 8.46 seconds ====================================================================================

Show the setup plan for a set of tests

$ spack test -k test_05 --setup-plan
=============================================================================================== test session starts ===============================================================================================
platform linux2 -- Python 2.7.6, pytest-3.0.5, py-1.4.32, pluggy-0.4.0
rootdir: /home/mculpo/PycharmProjects/spack, inifile: pytest.ini
plugins: cov-2.4.0
collected 470 items 

lib/spack/spack/test/database.py 
      SETUP    F monkeypatch
      SETUP    F mock_fetch_cache (fixtures used: monkeypatch)
      SETUP    F no_stdin_duplication (fixtures used: monkeypatch)
SETUP    S tmpdir_factory
SETUP    S repo_path
  SETUP    M builtin_mock (fixtures used: repo_path)
SETUP    S linux_os
SETUP    S configuration_dir (fixtures used: linux_os, tmpdir_factory)
  SETUP    M config (fixtures used: configuration_dir)
  SETUP    M database (fixtures used: builtin_mock, config, tmpdir_factory)
        lib/spack/spack/test/database.py::test_050_basic_query (fixtures used: builtin_mock, config, configuration_dir, database, linux_os, mock_fetch_cache, monkeypatch, no_stdin_duplication, repo_path, tmpdir_factory)
      TEARDOWN F no_stdin_duplication
      TEARDOWN F mock_fetch_cache
      TEARDOWN F monkeypatch
  TEARDOWN M database
  TEARDOWN M config
  TEARDOWN M builtin_mock
TEARDOWN S configuration_dir
TEARDOWN S linux_os
TEARDOWN S repo_path
TEARDOWN S tmpdir_factory

============================================================================================ slowest 20 test durations ============================================================================================
0.00s setup    lib/spack/spack/test/database.py::test_050_basic_query
0.00s teardown lib/spack/spack/test/database.py::test_050_basic_query
============================================================================================== 469 tests deselected ===============================================================================================
========================================================================================= 469 deselected in 0.50 seconds ==========================================================================================

Getting help on the ton of options that are not above

$ spack test -h
...

[alalazo]

@tgamblin @scheibelp @becker33 The first commit is plain porting, and there are plenty of options to play with (I just activated a report for the 20 slowest tests, see Travis).

How do you feel about trying to convert some tests with native pytest syntax? Or start using the dependency injection mechanism for fixtures? By the way: I still have to fit the pieces together, but it seems to me that parametric fixtures could help to automate integration tests as in #1507

[citibeth]

Can you please give some motivational rationale behind this change? I'm sure there's a good reason, but it's not apparent in this PR so far.

[alalazo]

@citibeth #2368

[alalazo]

@tgamblin @becker33 @scheibelp @hegner

I think you may want to have a look at the test build_system_guess refactored to use parametric fixtures, as it is a short example of how the feature works. Now: I don't mean to bother you on how cool pytest is (and it is!). The idea is that we can re-use the feature for deployment tests.

What I roughly have in mind is:

1. collect in a common place a set of parametric fixtures based on detectable properties (like the architecture)
2. write these fixtures to map to virtual packages (mpi, lapack, etc.) or other peculiar properties of the system under test
3. write in the package folder of a few selected packages (the ones we really care about) a test_deploy.py python module that will try to install all the flavors we want to test, eventually parametrized on the fixtures above

The funcargs dependency injection mechanism will take take care of the Cartesian product and start a build for each relevant combination. That should be enough to test packages selectively when they get modified, or to trigger everything for testing a release.

Does it make sense to you, at least from an high level perspective?

[alalazo]

spack test is now a thin wrapper around pytest. To see which options are available:

spack test -h

To have a list of the tests:

spack test --collect-only

To run just a few selected tests see the -k option, e.g. :

spack test -k spec

will run any test that contains the word "spec" at module, class or function scope

[alalazo]

@tgamblin @becker33 @scheibelp

Can you have a brief look at:

• the porting of MockPackagesTest in conftest.py (basically everything below the # Test-specific fixtures comment)
• its use in concretize.py and architecture.py

? While I am at it I am trying to port the code to native pytest syntax, but would like to have a green light before undertaking the bulk of the work.

One of the benefit of doing this is that we can query or run basically everything we want with a very fine control other details. For instance:

$ pytest --setup-plan -k test_user_defaults
=============================================================================================== test session starts ===============================================================================================
platform linux2 -- Python 2.7.6, pytest-3.0.5, py-1.4.31, pluggy-0.4.0
rootdir: /home/mculpo/PycharmProjects/spack, inifile: pytest.ini
plugins: cov-2.4.0
collected 452 items 

lib/spack/spack/test/architecture.py 
      SETUP    F monkeypatch
      SETUP    F mock_fetch_cache (fixtures used: monkeypatch)
      SETUP    F no_stdin_duplication (fixtures used: monkeypatch)
SETUP    S tmpdir_factory
SETUP    S linux_os
  SETUP    M configuration_files (fixtures used: linux_os, tmpdir_factory)
        lib/spack/spack/test/architecture.py::test_user_defaults (fixtures used: configuration_files, linux_os, mock_fetch_cache, monkeypatch, no_stdin_duplication, tmpdir_factory)
      TEARDOWN F no_stdin_duplication
      TEARDOWN F mock_fetch_cache
      TEARDOWN F monkeypatch
  TEARDOWN M configuration_files
TEARDOWN S linux_os
TEARDOWN S tmpdir_factory

============================================================================================ slowest 20 test durations ============================================================================================
0.00s setup    lib/spack/spack/test/architecture.py::test_user_defaults
0.00s teardown lib/spack/spack/test/architecture.py::test_user_defaults
============================================================================================== 451 tests deselected ===============================================================================================
========================================================================================= 451 deselected in 1.03 seconds ==========================================================================================

shows what will be the plan for running a single test in architecture.py. UnitTest classes are supported, but not so well integrated in the tool.

[alalazo]

@tgamblin @becker33 I think this is ready to be reviewed. The PR is huge (and I couldn't do much to contain its size), but it's just porting of tests. From my side I would appreciate a thorough review of anything in:

• lib/conftest.py
• lib/spack/spack/test/conftest.py
• lib/spack/spack/test/stage.py

The first two files contain the code that before was in the Mock* classes. The last file instead contained tests that were quite contrived to port. Well, fire at will 😄

[alalazo]

To everybody: description above updated. You are welcome to try the PR and leave feedbacks.

[scheibelp]

Generally speaking LGTM. My only concern is the general implicit availability of some fixtures (e.g. 'config' from conftest) - I'd prefer any test that wanted to use it would have to do:

@pytest.mark.usefixtures('config'

[scheibelp]

I'd prefer url_and_build_system as "system" by itself is a bit generic

Also: as you said this seems to be a great example of the usefulness of fixtures - one can add new unit tests by specifying new parameter tuples.

[scheibelp]

and also sets up the resource to be pulled by the stage with the appropriate file name

[scheibelp]

Earlier these tests were referencing "config" defined in conftest in an implicit manner. I think this was updated in your most recent commit (i.e. this line is new). Is there anything preventing a user from referencing "config" in an implicit manner elsewhere? Brief reading suggests this can be prevented although I'm having trouble seeing how that is achieved.

[alalazo]

Is there anything preventing a user from referencing "config" in an implicit manner elsewhere?

I don't think so, and I wouldn't be worried about that. Arguments to tests are only fixtures: if an unknown argument is present pytest will quit with a very informative error message. Here I grouped the fixtures together because in the tests below they were used only for their side effects. In cases where you need the object returned by the fixture the only way possible is using the funcargs mechanism (i.e. referencing fixtures in an implicit manner).

[scheibelp]

if an unknown argument is present pytest will quit with a very informative error message

That is good, although I am more concerned with other potential issues with having a generically-named variable like "config" available in the global namespace. E.g. a user may create their own unit test, create their own instance of "config" then rename it and forget to change the reference. That's the sort of thing which may cause some frustration IMO. I suppose a more specific name would help like "global_config"

[alalazo]

That is good, although I am more concerned with other potential issues with having a generically-named variable like "config" available in the global namespace.

I agree if we want to change the name of fixtures for something better. For 'config' if there's general agreement over something else I'll make the change (didn't do it now as it touches a lot of files and would like to have everybody agreeing on that).

Just to be sure this is clear to everybody reading the thread: the fixture config is not available in the global namespace. pytest has a dependency injection mechanism (commonly referred to as funcargs) which works more or less like this:

1. the decorator @pytest.fixture registers a function as a fixture (this function will be used as a factory to create a fixture object when required)
2. since tests are automatically run by pytest they can only accept fixture arguments: if the name of an argument matches the name of a registered fixture, an object will be injected
3. fixtures can be overriden with these rules

[alalazo]

Expected failures work as expected (see fc36376): now we have a nice way to document bugs in core and go TDD.

[tgamblin]

P.S. Can someone remove nose from lib/spack/externals/init.py and replace it with pytest?
@adamjstewart: done.

[tgamblin]

We can consolidate testing commands in a future PR, but to prepare for that I made the spack test wrapper a bit less thin and added a few features. In particular:

1. spack test supports something approximating the prior, simpler interface now.
   1. You can run spack test -l to list tests much the same way as you used to.
   2. You can run spack test -L to get a full hierarchy of pytest tests.
   3. Help is a bit more readable by default and it tells you you can use -L to get the full list.
   4. Anything from the output of these can be used in spack test <test-to-run> and it will be forwarded on to pytest -k <pattern>.

[tgamblin]

@alalazo: I got rid of pytest-cov and switched back to the external coverage command for code coverage. I think this is better because:

1. This includes imports and other parts of Spack that happen before commands run (see here).
2. coverage supports options for handling multiprocessing, while pytest-cov doesn't seem to, though maybe it does them by default.
3. coverage doesn't seem to take much more time (2.5%) than pytest --cov. I suspect that if it does, it's because it covers more of the code than pyetst --cov.

See what you think.

[alalazo]

@tgamblin I just checked the PR, and everything seems fine to me. Thanks for taking the time of finalizing it! 👍

Given the size of this PR I think it's better to argue over minor points or cosmetic changes after this is merged and on smaller / more focused PRs. 😊

---

Miscellaneous replies:

This includes imports and other parts of Spack that happen before commands run (see here).

The same answer says:

As of 2014, pytest-cov seems to have changed hands. py.test --cov jedi test seems to be a useful command again (look at the comments). However, you don't need to use it. But in combination with xdist it can speed up your coverage reports.

That was one of the two reasons why I went for pytest + plugins: prepare everything for the use of pytest-xdist. The other reason was to go in the direction of being conformant to what most major packages in PyPI do: use pytest as an external tool.

I don't see this as a major issue though: people seem to like having a wrapper command for tests, and I suspect we won't be PyPI packageable for a long time...

coverage supports options for handling multiprocessing, while pytest-cov doesn't seem to, though maybe it does them by default

As far as I understood handling multiprocessing libraries within your application should be a pytest capability.

On top of that pytest-cov can be used with pytest-xdist to run tests in parallel (but I am pretty sure that also coverage can do that). I think we need to re-code the management of some shared mock resources (e.g. database) to use concurrency in unit tests though.

[alalazo]

I forgot to advertize this to people following the PR: passing to pytest we also have the possibility to insert expected failures in unit tests. We may use this feature to embrace TDD where possible:

• code a test that triggers the failure (and mark it as an expected failure) in a bug report PR
• fix the code so that the test passes in a subsequent PR

This will ensure that Travis won't go mad over a known bug, and will give developers a more focused way to work on an issue.

[pramodk]

Trying latest develop on new system gives:

$ source spack/share/spack/setup-env.sh
Traceback (most recent call last):
  File "/gpfs/homeb/pcp0/pcp0043/spack/bin/spack", line 55, in <module>
    import nose
ImportError: No module named nose
Traceback (most recent call last):
  File "/gpfs/homeb/pcp0/pcp0043/spack/bin/spack", line 55, in <module>
    import nose
ImportError: No module named nose
.................

I see that nose module is not installed on the system. Is this mandatory? If not, could you take a look?

[citibeth]

Nose was removed with #2502. Maybe somebody didn't get rid of all the `import nose` statements?

…

On Thu, Dec 29, 2016 at 12:31 PM, Pramod Kumbhar ***@***.***> wrote: Trying latest develop on new system gives: $ source spack/share/spack/setup-env.sh Traceback (most recent call last): File "/gpfs/homeb/pcp0/pcp0043/spack/bin/spack", line 55, in <module> import nose ImportError: No module named nose Traceback (most recent call last): File "/gpfs/homeb/pcp0/pcp0043/spack/bin/spack", line 55, in <module> import nose ImportError: No module named nose ................. I see that nose module is not installed on the system. Is this mandatory? If not, could you take a look? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2502 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB1cd2j973rHLh1M_jfsUi-RXLK6a-K0ks5rM-5WgaJpZM4LGinM> .

[tgamblin]

@citibeth This is already fixed by #2685.