tests: improved `spack test` command line options

Previously, `spack test` automatically passed all of its arguments to
`pytest -k` if no options were provided, and to `pytest` if they were.
`spack test -l` also provided a list of test filenames, but they didn't
really let you completely narrow down which tests you wanted to run.

Instead of trying to do our own weird thing, this passes `spack test`
args directly to `pytest`, and omits the implicit `-k`.  This means we
can now run, e.g.:

```console
$ spack test spec_syntax.py::TestSpecSyntax::test_ambiguous
```

This wasn't possible before, because we'd pass the fully qualified name
to `pytest -k` and get an error.

Because `pytest` doesn't have the greatest ability to list tests, I've
tweaked the `-l`/`--list`, `-L`/`--list-long`, and `-N`/`--list-names`
options to `spack test` so that they help you understand the names
better.  you can combine these options with `-k` or other arguments to do
pretty powerful searches.

This one makes it easy to get a list of names so you can run tests in
different orders (something I find useful for debugging `pytest` issues):

```console
$ spack test --list-names -k "spec and concretize"
cmd/env.py::test_concretize_user_specs_together
concretize.py::TestConcretize::test_conflicts_in_spec
concretize.py::TestConcretize::test_find_spec_children
concretize.py::TestConcretize::test_find_spec_none
concretize.py::TestConcretize::test_find_spec_parents
concretize.py::TestConcretize::test_find_spec_self
concretize.py::TestConcretize::test_find_spec_sibling
concretize.py::TestConcretize::test_no_matching_compiler_specs
concretize.py::TestConcretize::test_simultaneous_concretization_of_specs
spec_dag.py::TestSpecDag::test_concretize_deptypes
spec_dag.py::TestSpecDag::test_copy_concretized
```

You can combine any list option with keywords:

```console
$ spack test --list -k microarchitecture
llnl/util/cpu.py  modules/lmod.py
```

```console
$ spack test --list-long -k microarchitecture
llnl/util/cpu.py::
    test_generic_microarchitecture

modules/lmod.py::TestLmod::
    test_only_generic_microarchitectures_in_root
```

Or just list specific files:

```console
$ spack test --list-long cmd/test.py
cmd/test.py::
    test_list                       test_list_names_with_pytest_arg
    test_list_long                  test_list_with_keywords
    test_list_long_with_pytest_arg  test_list_with_pytest_arg
    test_list_names
```

Hopefully this stuff will help with debugging test issues.

- [x] make `spack test` send args directly to `pytest` instead of trying
  to do fancy things.
- [x] rework `--list`, `--list-long`, and add `--list-names` to make
  searching for tests easier.
- [x] make it possible to mix Spack's list args with `pytest` args
  (they're just fancy parsing around `pytest --collect-only`)
- [x] add docs
- [x] add tests
- [x] update spack completion

5 files changed, 266 insertions, 62 deletions

diff --git a/lib/spack/docs/contribution_guide.rst b/lib/spack/docs/contribution_guide.rst
index 5aa63ab776..0b79141ee3 100644
--- a/lib/spack/docs/contribution_guide.rst
+++ b/lib/spack/docs/contribution_guide.rst
@@ -64,6 +64,8 @@ If you take a look in ``$SPACK_ROOT/.travis.yml``, you'll notice that we test
 against Python 2.6, 2.7, and 3.4-3.7 on both macOS and Linux. We currently
 perform 3 types of tests:
 
+.. _cmd-spack-test:
+
 ^^^^^^^^^^
 Unit Tests
 ^^^^^^^^^^
@@ -86,40 +88,83 @@ To run *all* of the unit tests, use:
 
    $ spack test
 
-These tests may take several minutes to complete. If you know you are only
-modifying a single Spack feature, you can run a single unit test at a time:
+These tests may take several minutes to complete. If you know you are
+only modifying a single Spack feature, you can run subsets of tests at a
+time.  For example, this would run all the tests in
+``lib/spack/spack/test/architecture.py``:
 
 .. code-block:: console
 
-   $ spack test architecture
+   $ spack test architecture.py
+
+And this would run the ``test_platform`` test from that file:
+
+.. code-block:: console
 
-This allows you to develop iteratively: make a change, test that change, make
-another change, test that change, etc. To get a list of all available unit
-tests, run:
+   $ spack test architecture.py::test_platform
+
+This allows you to develop iteratively: make a change, test that change,
+make another change, test that change, etc.  We use `pytest
+<http://pytest.org/>`_ as our tests fromework, and these types of
+arguments are just passed to the ``pytest`` command underneath. See `the
+pytest docs
+<http://doc.pytest.org/en/latest/usage.html#specifying-tests-selecting-tests>`_
+for more details on test selection syntax.
+
+``spack test`` has a few special options that can help you understand
+what tests are available.  To get a list of all available unit test
+files, run:
 
 .. command-output:: spack test --list
+   :ellipsis: 5
+
+To see a more detailed list of available unit tests, use ``spack test
+--list-long``:
+
+.. command-output:: spack test --list-long
+   :ellipsis: 10
+
+And to see the fully qualified names of all tests, use ``--list-names``:
+
+.. command-output:: spack test --list-names
+   :ellipsis: 5
+
+You can combine these with ``pytest`` arguments to restrict which tests
+you want to know about.  For example, to see just the tests in
+``architecture.py``:
+
+.. command-output:: spack test --list-long architecture.py
+
+You can also combine any of these options with a ``pytest`` keyword
+search.  For example, to see the names of all tests that have "spec"
+or "concretize" somewhere in their names:
 
-A more detailed list of available unit tests can be found by running
-``spack test --long-list``.
+.. command-output:: spack test --list-names -k "spec and concretize"
 
-By default, ``pytest`` captures the output of all unit tests. If you add print
-statements to a unit test and want to see the output, simply run:
+By default, ``pytest`` captures the output of all unit tests, and it will
+print any captured output for failed tests. Sometimes it's helpful to see
+your output interactively, while the tests run (e.g., if you add print
+statements to a unit tests).  To see the output *live*, use the ``-s``
+argument to ``pytest``:
 
 .. code-block:: console
 
-   $ spack test -s -k architecture
+   $ spack test -s architecture.py::test_platform
 
-Unit tests are crucial to making sure bugs aren't introduced into Spack. If you
-are modifying core Spack libraries or adding new functionality, please consider
-adding new unit tests or strengthening existing tests.
+Unit tests are crucial to making sure bugs aren't introduced into
+Spack. If you are modifying core Spack libraries or adding new
+functionality, please add new unit tests for your feature, and consider
+strengthening existing tests.  You will likely be asked to do this if you
+submit a pull request to the Spack project on GitHub.  Check out the
+`pytest docs <http://pytest.org/>`_ and feel free to ask for guidance on
+how to write tests!
 
 .. note::
 
-   There is also a ``run-unit-tests`` script in ``share/spack/qa`` that
-   runs the unit tests. Afterwards, it reports back to Codecov with the
-   percentage of Spack that is covered by unit tests. This script is
-   designed for Travis CI. If you want to run the unit tests yourself, we
-   suggest you use ``spack test``.
+   You may notice the ``share/spack/qa/run-unit-tests`` script in the
+   repository.  This script is designed for Travis CI.  It runs the unit
+   tests and reports coverage statistics back to Codecov. If you want to
+   run the unit tests yourself, we suggest you use ``spack test``.
 
 ^^^^^^^^^^^^
 Flake8 Tests
diff --git a/lib/spack/docs/developer_guide.rst b/lib/spack/docs/developer_guide.rst
index 738e326b12..de2fe80f85 100644
--- a/lib/spack/docs/developer_guide.rst
+++ b/lib/spack/docs/developer_guide.rst
@@ -363,12 +363,12 @@ Developer commands
 ``spack doc``
 ^^^^^^^^^^^^^
 
-.. _cmd-spack-test:
-
 ^^^^^^^^^^^^^^
 ``spack test``
 ^^^^^^^^^^^^^^
 
+See the :ref:`contributor guide section <cmd-spack-test>` on ``spack test``.
+
 .. _cmd-spack-python:
 
 ^^^^^^^^^^^^^^^^
diff --git a/lib/spack/spack/cmd/test.py b/lib/spack/spack/cmd/test.py
index 3a45b30446..f2ca8fc93b 100644
--- a/lib/spack/spack/cmd/test.py
+++ b/lib/spack/spack/cmd/test.py
@@ -4,20 +4,22 @@
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
 from __future__ import print_function
+from __future__ import division
 
+import collections
 import sys
-import os
 import re
 import argparse
 import pytest
 from six import StringIO
 
+import llnl.util.tty.color as color
 from llnl.util.filesystem import working_dir
 from llnl.util.tty.colify import colify
 
 import spack.paths
 
-description = "run spack's unit tests"
+description = "run spack's unit tests (wrapper around pytest)"
 section = "developer"
 level = "long"
 
@@ -25,61 +27,130 @@ level = "long"
 def setup_parser(subparser):
     subparser.add_argument(
         '-H', '--pytest-help', action='store_true', default=False,
-        help="print full pytest help message, showing advanced options")
-
-    list_group = subparser.add_mutually_exclusive_group()
-    list_group.add_argument(
-        '-l', '--list', action='store_true', default=False,
-        help="list basic test names")
-    list_group.add_argument(
-        '-L', '--long-list', action='store_true', default=False,
-        help="list the entire hierarchy of tests")
+        help="show full pytest help, with advanced options")
+
+    # extra spack arguments to list tests
+    list_group = subparser.add_argument_group("listing tests")
+    list_mutex = list_group.add_mutually_exclusive_group()
+    list_mutex.add_argument(
+        '-l', '--list', action='store_const', default=None,
+        dest='list', const='list', help="list test filenames")
+    list_mutex.add_argument(
+        '-L', '--list-long', action='store_const', default=None,
+        dest='list', const='long', help="list all test functions")
+    list_mutex.add_argument(
+        '-N', '--list-names', action='store_const', default=None,
+        dest='list', const='names', help="list full names of all tests")
+
+    # use tests for extension
     subparser.add_argument(
         '--extension', default=None,
-        help="run test for a given Spack extension"
-    )
+        help="run test for a given spack extension")
+
+    # spell out some common pytest arguments, so they'll show up in help
+    pytest_group = subparser.add_argument_group(
+        "common pytest arguments (spack test --pytest-help for more details)")
+    pytest_group.add_argument(
+        "-s", action='append_const', dest='parsed_args', const='-s',
+        help="print output while tests run (disable capture)")
+    pytest_group.add_argument(
+        "-k", action='store', metavar="EXPRESSION", dest='expression',
+        help="filter tests by keyword (can also use w/list options)")
+    pytest_group.add_argument(
+        "--showlocals", action='append_const', dest='parsed_args',
+        const='--showlocals', help="show local variable values in tracebacks")
+
+    # remainder is just passed to pytest
     subparser.add_argument(
-        'tests', nargs=argparse.REMAINDER,
-        help="list of tests to run (will be passed to pytest -k)")
+        'pytest_args', nargs=argparse.REMAINDER, help="arguments for pytest")
 
 
-def do_list(args, unknown_args):
+def do_list(args, extra_args):
     """Print a lists of tests than what pytest offers."""
     # Run test collection and get the tree out.
     old_output = sys.stdout
     try:
         sys.stdout = output = StringIO()
-        pytest.main(['--collect-only'])
+        pytest.main(['--collect-only'] + extra_args)
     finally:
         sys.stdout = old_output
 
-    # put the output in a more readable tree format.
     lines = output.getvalue().split('\n')
-    output_lines = []
+    tests = collections.defaultdict(lambda: set())
+    prefix = []
+
+    # collect tests into sections
     for line in lines:
         match = re.match(r"(\s*)<([^ ]*) '([^']*)'", line)
         if not match:
             continue
         indent, nodetype, name = match.groups()
 
-        # only print top-level for short list
-        if args.list:
-            if not indent:
-                output_lines.append(
-                    os.path.basename(name).replace('.py', ''))
-        else:
-            print(indent + name)
+        # strip parametrized tests
+        if "[" in name:
+            name = name[:name.index("[")]
+
+        depth = len(indent) // 2
 
-    if args.list:
-        colify(output_lines)
+        if nodetype.endswith("Function"):
+            key = tuple(prefix)
+            tests[key].add(name)
+        else:
+            prefix = prefix[:depth]
+            prefix.append(name)
+
+    def colorize(c, prefix):
+        if isinstance(prefix, tuple):
+            return "::".join(
+                color.colorize("@%s{%s}" % (c, p))
+                for p in prefix if p != "()"
+            )
+        return color.colorize("@%s{%s}" % (c, prefix))
+
+    if args.list == "list":
+        files = set(prefix[0] for prefix in tests)
+        color_files = [colorize("B", file) for file in sorted(files)]
+        colify(color_files)
+
+    elif args.list == "long":
+        for prefix, functions in sorted(tests.items()):
+            path = colorize("*B", prefix) + "::"
+            functions = [colorize("c", f) for f in sorted(functions)]
+            color.cprint(path)
+            colify(functions, indent=4)
+            print()
+
+    else:  # args.list == "names"
+        all_functions = [
+            colorize("*B", prefix) + "::" + colorize("c", f)
+            for prefix, functions in sorted(tests.items())
+            for f in sorted(functions)
+        ]
+        colify(all_functions)
+
+
+def add_back_pytest_args(args, unknown_args):
+    """Add parsed pytest args, unknown args, and remainder together.
+
+    We add some basic pytest arguments to the Spack parser to ensure that
+    they show up in the short help, so we have to reassemble things here.
+    """
+    result = args.parsed_args or []
+    result += unknown_args or []
+    result += args.pytest_args or []
+    if args.expression:
+        result += ["-k", args.expression]
+    return result
 
 
 def test(parser, args, unknown_args):
     if args.pytest_help:
         # make the pytest.main help output more accurate
         sys.argv[0] = 'spack test'
-        pytest.main(['-h'])
-        return
+        return pytest.main(['-h'])
+
+    # add back any parsed pytest args we need to pass to pytest
+    pytest_args = add_back_pytest_args(args, unknown_args)
 
     # The default is to test the core of Spack. If the option `--extension`
     # has been used, then test that extension.
@@ -91,15 +162,8 @@ def test(parser, args, unknown_args):
 
     # pytest.ini lives in the root of the spack repository.
     with working_dir(pytest_root):
-        # --list and --long-list print the test output better.
-        if args.list or args.long_list:
-            do_list(args, unknown_args)
+        if args.list:
+            do_list(args, pytest_args)
             return
 
-        # Allow keyword search without -k if no options are specified
-        if (args.tests and not unknown_args and
-            not any(arg.startswith('-') for arg in args.tests)):
-            return pytest.main(['-k'] + args.tests)
-
-        # Just run the pytest command
-        return pytest.main(unknown_args + args.tests)
+        return pytest.main(pytest_args)
diff --git a/lib/spack/spack/test/cmd/test.py b/lib/spack/spack/test/cmd/test.py
new file mode 100644
index 0000000000..3595f91953
--- /dev/null
+++ b/lib/spack/spack/test/cmd/test.py
@@ -0,0 +1,94 @@
+# Copyright 2013-2019 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+from spack.main import SpackCommand
+
+spack_test = SpackCommand('test')
+
+
+def test_list():
+    output = spack_test('--list')
+    assert "test.py" in output
+    assert "spec_semantics.py" in output
+    assert "test_list" not in output
+
+
+def test_list_with_pytest_arg():
+    output = spack_test('--list', 'cmd/test.py')
+    assert output.strip() == "cmd/test.py"
+
+
+def test_list_with_keywords():
+    output = spack_test('--list', '-k', 'cmd/test.py')
+    assert output.strip() == "cmd/test.py"
+
+
+def test_list_long(capsys):
+    with capsys.disabled():
+        output = spack_test('--list-long')
+    assert "test.py::\n" in output
+    assert "test_list" in output
+    assert "test_list_with_pytest_arg" in output
+    assert "test_list_with_keywords" in output
+    assert "test_list_long" in output
+    assert "test_list_long_with_pytest_arg" in output
+    assert "test_list_names" in output
+    assert "test_list_names_with_pytest_arg" in output
+
+    assert "spec_dag.py::\n" in output
+    assert 'test_installed_deps' in output
+    assert 'test_test_deptype' in output
+
+
+def test_list_long_with_pytest_arg(capsys):
+    with capsys.disabled():
+        output = spack_test('--list-long', 'cmd/test.py')
+    assert "test.py::\n" in output
+    assert "test_list" in output
+    assert "test_list_with_pytest_arg" in output
+    assert "test_list_with_keywords" in output
+    assert "test_list_long" in output
+    assert "test_list_long_with_pytest_arg" in output
+    assert "test_list_names" in output
+    assert "test_list_names_with_pytest_arg" in output
+
+    assert "spec_dag.py::\n" not in output
+    assert 'test_installed_deps' not in output
+    assert 'test_test_deptype' not in output
+
+
+def test_list_names():
+    output = spack_test('--list-names')
+    assert "test.py::test_list\n" in output
+    assert "test.py::test_list_with_pytest_arg\n" in output
+    assert "test.py::test_list_with_keywords\n" in output
+    assert "test.py::test_list_long\n" in output
+    assert "test.py::test_list_long_with_pytest_arg\n" in output
+    assert "test.py::test_list_names\n" in output
+    assert "test.py::test_list_names_with_pytest_arg\n" in output
+
+    assert "spec_dag.py::test_installed_deps\n" in output
+    assert 'spec_dag.py::test_test_deptype\n' in output
+
+
+def test_list_names_with_pytest_arg():
+    output = spack_test('--list-names', 'cmd/test.py')
+    assert "test.py::test_list\n" in output
+    assert "test.py::test_list_with_pytest_arg\n" in output
+    assert "test.py::test_list_with_keywords\n" in output
+    assert "test.py::test_list_long\n" in output
+    assert "test.py::test_list_long_with_pytest_arg\n" in output
+    assert "test.py::test_list_names\n" in output
+    assert "test.py::test_list_names_with_pytest_arg\n" in output
+
+    assert "spec_dag.py::test_installed_deps\n" not in output
+    assert 'spec_dag.py::test_test_deptype\n' not in output
+
+
+def test_pytest_help():
+    output = spack_test('--pytest-help')
+    assert "-k EXPRESSION" in output
+    assert "pytest-warnings:" in output
+    assert "--collect-only" in output
diff --git a/share/spack/spack-completion.bash b/share/spack/spack-completion.bash
index ee2eaa8286..2836db1896 100755
--- a/share/spack/spack-completion.bash
+++ b/share/spack/spack-completion.bash
@@ -1072,7 +1072,8 @@ function _spack_test {
     if $list_options
     then
         compgen -W "-h --help -H --pytest-help -l --list
-                    -L --long-list" -- "$cur"
+                    -L --list-long -N --list-names -s -k
+                    --showlocals" -- "$cur"
     else
         compgen -W "$(_tests)" -- "$cur"
     fi