Split 'spack module' into multiple commands

'spack module' has been split into multiple commands, each one tied to a
specific module type. This permits the specialization of the new
commands with features that are module type specific (e.g. set the
default module file in lmod when multiple versions of the same package
are installed at the same time).

10 files changed, 413 insertions, 194 deletions

diff --git a/lib/spack/docs/module_file_support.rst b/lib/spack/docs/module_file_support.rst
index 471ef3aca5..2351f935b5 100644
--- a/lib/spack/docs/module_file_support.rst
+++ b/lib/spack/docs/module_file_support.rst
@@ -636,7 +636,7 @@ and in the customization of both their layout and content, but also ships with
 a tool to ease the burden of their maintenance in production environments.
 This tool is the ``spack module`` command:
 
-.. command-output:: spack module --help
+.. command-output:: spack tcl --help
 
 .. _cmd-spack-module-refresh:
 
@@ -647,7 +647,7 @@ This tool is the ``spack module`` command:
 The command that regenerates module files to update their content or
 their layout is ``module refresh``:
 
-.. command-output:: spack module refresh --help
+.. command-output:: spack tcl refresh --help
 
 A set of packages can be selected using anonymous specs for the optional
 ``constraint`` positional argument. The argument ``--module-type`` identifies
@@ -663,7 +663,7 @@ before regeneration if the change in layout is radical.
 If instead what you need is just to delete a few module files, then the right
 command is ``module rm``:
 
-.. command-output:: spack module rm --help
+.. command-output:: spack tcl rm --help
 
 .. note::
   We care about your module files!
diff --git a/lib/spack/spack/cmd/common/arguments.py b/lib/spack/spack/cmd/common/arguments.py
index ea5bc1c25d..4a3ddc6326 100644
--- a/lib/spack/spack/cmd/common/arguments.py
+++ b/lib/spack/spack/cmd/common/arguments.py
@@ -87,12 +87,6 @@ _arguments['constraint'] = Args(
     'constraint', nargs=argparse.REMAINDER, action=ConstraintAction,
     help='constraint to select a subset of installed packages')
 
-_arguments['module_type'] = Args(
-    '-m', '--module-type',
-    choices=spack.modules.module_types.keys(),
-    action='append',
-    help='type of module file. More than one choice is allowed [default: tcl]')
-
 _arguments['yes_to_all'] = Args(
     '-y', '--yes-to-all', action='store_true', dest='yes_to_all',
     help='assume "yes" is the answer to every confirmation request')
diff --git a/lib/spack/spack/cmd/module.py b/lib/spack/spack/cmd/common/modules.py
index 11bbaf7759..901eb060c5 100644
--- a/lib/spack/spack/cmd/module.py
+++ b/lib/spack/spack/cmd/common/modules.py
@@ -22,10 +22,12 @@
 # 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 __future__ import print_function
+"""Contains all the functions that are common to the implementation of
+each module file command.
+"""
 
 import collections
-import os
+import os.path
 import shutil
 
 from llnl.util import filesystem, tty
@@ -55,12 +57,12 @@ def subcommand(subparser_name):
         callbacks[subparser_name] = callback
         return callback
     return decorator
+from . import arguments
 
 
 def setup_parser(subparser):
     sp = subparser.add_subparsers(metavar='SUBCOMMAND', dest='subparser_name')
 
-    # spack module refresh
     refresh_parser = sp.add_parser('refresh', help='regenerate module files')
     refresh_parser.add_argument(
         '--delete-tree',
@@ -68,25 +70,22 @@ def setup_parser(subparser):
         action='store_true'
     )
     arguments.add_common_arguments(
-        refresh_parser, ['constraint', 'module_type', 'yes_to_all']
+        refresh_parser, ['constraint', 'yes_to_all']
     )
 
-    # spack module find
     find_parser = sp.add_parser('find', help='find module files for packages')
     find_parser.add_argument(
         '--full-path',
         help='display full path to module file',
         action='store_true'
     )
-    arguments.add_common_arguments(find_parser, ['constraint', 'module_type'])
+    arguments.add_common_arguments(find_parser, ['constraint'])
 
-    # spack module rm
     rm_parser = sp.add_parser('rm', help='remove module files')
     arguments.add_common_arguments(
-        rm_parser, ['constraint', 'module_type', 'yes_to_all']
+        rm_parser, ['constraint', 'yes_to_all']
     )
 
-    # spack module loads
     loads_parser = sp.add_parser(
         'loads',
         help='prompt the list of modules associated with a constraint'
@@ -104,7 +103,7 @@ def setup_parser(subparser):
         help="exclude package from output; may be specified multiple times"
     )
     arguments.add_common_arguments(
-        loads_parser, ['constraint', 'module_type', 'recurse_dependencies']
+        loads_parser, ['constraint', 'recurse_dependencies']
     )
 
 
@@ -120,22 +119,6 @@ class NoSpecMatches(Exception):
     """
 
 
-class MultipleModuleTypes(Exception):
-    """Raised when multiple module types match a cli request, in a context
-    where this is not allowed.
-    """
-
-
-def one_module_or_raise(module_types):
-    """Ensures exactly one module type has been selected, or raises the
-    appropriate exception.
-    """
-    # Ensure a single module type has been selected
-    if len(module_types) > 1:
-        raise MultipleModuleTypes()
-    return module_types[0]
-
-
 def one_spec_or_raise(specs):
     """Ensures exactly one spec has been selected, or raises the appropriate
     exception.
@@ -150,12 +133,9 @@ def one_spec_or_raise(specs):
     return specs[0]
 
 
-@subcommand('loads')
-def loads(module_types, specs, args):
+def loads(module_type, specs, args):
     """Prompt the list of modules associated with a list of specs"""
 
-    module_type = one_module_or_raise(module_types)
-
     # Get a comprehensive list of specs
     if args.recurse_dependencies:
         specs_from_user_constraint = specs[:]
@@ -181,7 +161,7 @@ def loads(module_types, specs, args):
     module_commands = {
         'tcl': 'module load ',
         'lmod': 'module load ',
-        'dotkit': 'dotkit use '
+        'dotkit': 'use '
     }
 
     d = {
@@ -199,14 +179,12 @@ def loads(module_types, specs, args):
         print(prompt_template.format(**d))
 
 
-@subcommand('find')
-def find(module_types, specs, args):
+def find(module_type, specs, args):
     """Returns the module file "use" name if there's a single match. Raises
     error messages otherwise.
     """
 
     spec = one_spec_or_raise(specs)
-    module_type = one_module_or_raise(module_types)
 
     # Check if the module file is present
     writer = spack.modules.module_types[module_type](spec)
@@ -222,40 +200,37 @@ def find(module_types, specs, args):
         print(writer.layout.use_name)
 
 
-@subcommand('rm')
-def rm(module_types, specs, args):
+def rm(module_type, specs, args):
     """Deletes the module files associated with every spec in specs, for every
     module type in module types.
     """
-    for module_type in module_types:
 
-        module_cls = spack.modules.module_types[module_type]
-        module_exist = lambda x: os.path.exists(module_cls(x).layout.filename)
+    module_cls = spack.modules.module_types[module_type]
+    module_exist = lambda x: os.path.exists(module_cls(x).layout.filename)
 
-        specs_with_modules = [spec for spec in specs if module_exist(spec)]
+    specs_with_modules = [spec for spec in specs if module_exist(spec)]
 
-        modules = [module_cls(spec) for spec in specs_with_modules]
+    modules = [module_cls(spec) for spec in specs_with_modules]
 
-        if not modules:
-            tty.die('No module file matches your query')
+    if not modules:
+        tty.die('No module file matches your query')
 
-        # Ask for confirmation
-        if not args.yes_to_all:
-            msg = 'You are about to remove {0} module files for:\n'
-            tty.msg(msg.format(module_type))
-            spack.cmd.display_specs(specs_with_modules, long=True)
-            print('')
-            answer = tty.get_yes_or_no('Do you want to proceed?')
-            if not answer:
-                tty.die('Will not remove any module files')
+    # Ask for confirmation
+    if not args.yes_to_all:
+        msg = 'You are about to remove {0} module files for:\n'
+        tty.msg(msg.format(module_type))
+        spack.cmd.display_specs(specs_with_modules, long=True)
+        print('')
+        answer = tty.get_yes_or_no('Do you want to proceed?')
+        if not answer:
+            tty.die('Will not remove any module files')
 
-        # Remove the module files
-        for s in modules:
-            s.remove()
+    # Remove the module files
+    for s in modules:
+        s.remove()
 
 
-@subcommand('refresh')
-def refresh(module_types, specs, args):
+def refresh(module_type, specs, args):
     """Regenerates the module files for every spec in specs and every module
     type in module types.
     """
@@ -267,8 +242,7 @@ def refresh(module_types, specs, args):
 
     if not args.yes_to_all:
         msg = 'You are about to regenerate {types} module files for:\n'
-        types = ', '.join(module_types)
-        tty.msg(msg.format(types=types))
+        tty.msg(msg.format(types=module_type))
         spack.cmd.display_specs(specs, long=True)
         print('')
         answer = tty.get_yes_or_no('Do you want to proceed?')
@@ -276,56 +250,69 @@ def refresh(module_types, specs, args):
             tty.die('Module file regeneration aborted.')
 
     # Cycle over the module types and regenerate module files
-    for module_type in module_types:
-
-        cls = spack.modules.module_types[module_type]
-
-        # skip unknown packages.
-        writers = [
-            cls(spec) for spec in specs
-            if spack.repo.path.exists(spec.name)]
-
-        # Filter blacklisted packages early
-        writers = [x for x in writers if not x.conf.blacklisted]
-
-        # Detect name clashes in module files
-        file2writer = collections.defaultdict(list)
-        for item in writers:
-            file2writer[item.layout.filename].append(item)
-
-        if len(file2writer) != len(writers):
-            message = 'Name clashes detected in module files:\n'
-            for filename, writer_list in file2writer.items():
-                if len(writer_list) > 1:
-                    message += '\nfile: {0}\n'.format(filename)
-                    for x in writer_list:
-                        message += 'spec: {0}\n'.format(x.spec.format())
-            tty.error(message)
-            tty.error('Operation aborted')
-            raise SystemExit(1)
-
-        if len(writers) == 0:
-            msg = 'Nothing to be done for {0} module files.'
-            tty.msg(msg.format(module_type))
-            continue
-
-        # If we arrived here we have at least one writer
-        module_type_root = writers[0].layout.dirname()
-        # Proceed regenerating module files
-        tty.msg('Regenerating {name} module files'.format(name=module_type))
-        if os.path.isdir(module_type_root) and args.delete_tree:
-            shutil.rmtree(module_type_root, ignore_errors=False)
-        filesystem.mkdirp(module_type_root)
-        for x in writers:
-            try:
-                x.write(overwrite=True)
-            except Exception as e:
-                msg = 'Could not write module file [{0}]'
-                tty.warn(msg.format(x.layout.filename))
-                tty.warn('\t--> {0} <--'.format(str(e)))
-
-
-def module(parser, args):
+
+    cls = spack.modules.module_types[module_type]
+
+    # Skip unknown packages.
+    writers = [
+        cls(spec) for spec in specs
+        if spack.repo.path.exists(spec.name)]
+
+    # Filter blacklisted packages early
+    writers = [x for x in writers if not x.conf.blacklisted]
+
+    # Detect name clashes in module files
+    file2writer = collections.defaultdict(list)
+    for item in writers:
+        file2writer[item.layout.filename].append(item)
+
+    if len(file2writer) != len(writers):
+        message = 'Name clashes detected in module files:\n'
+        for filename, writer_list in file2writer.items():
+            if len(writer_list) > 1:
+                message += '\nfile: {0}\n'.format(filename)
+                for x in writer_list:
+                    message += 'spec: {0}\n'.format(x.spec.format())
+        tty.error(message)
+        tty.error('Operation aborted')
+        raise SystemExit(1)
+
+    if len(writers) == 0:
+        msg = 'Nothing to be done for {0} module files.'
+        tty.msg(msg.format(module_type))
+        return
+
+    # If we arrived here we have at least one writer
+    module_type_root = writers[0].layout.dirname()
+    # Proceed regenerating module files
+    tty.msg('Regenerating {name} module files'.format(name=module_type))
+    if os.path.isdir(module_type_root) and args.delete_tree:
+        shutil.rmtree(module_type_root, ignore_errors=False)
+    filesystem.mkdirp(module_type_root)
+    for x in writers:
+        try:
+            x.write(overwrite=True)
+        except Exception as e:
+            msg = 'Could not write module file [{0}]'
+            tty.warn(msg.format(x.layout.filename))
+            tty.warn('\t--> {0} <--'.format(str(e)))
+
+
+#: Dictionary populated with the list of sub-commands.
+#: Each sub-command must be callable and accept 3 arguments:
+#:
+#:   - module_type: the type of module it refers to
+#:   - specs : the list of specs to be processed
+#:   - args : namespace containing the parsed command line arguments
+callbacks = {
+    'refresh': refresh,
+    'rm': rm,
+    'find': find,
+    'loads': loads
+}
+
+
+def modules_cmd(parser, args, module_type, callbacks=callbacks):
 
     # Qualifiers to be used when querying the db for specs
     constraint_qualifiers = {
@@ -339,17 +326,9 @@ def module(parser, args):
     # Get the specs that match the query from the DB
     specs = args.specs(**query_args)
 
-    # Set the module types that have been selected
-    module_types = args.module_type
-    if module_types is None:
-        # If no selection has been made select all of them
-        module_types = ['tcl']
-
-    module_types = list(set(module_types))
-
     try:
 
-        callbacks[args.subparser_name](module_types, specs, args)
+        callbacks[args.subparser_name](module_type, specs, args)
 
     except MultipleSpecsMatch:
         msg = "the constraint '{query}' matches multiple packages:\n"
@@ -362,7 +341,3 @@ def module(parser, args):
         msg = "the constraint '{query}' matches no package."
         tty.error(msg.format(query=args.constraint))
         tty.die('In this context exactly **one** match is needed: please specify your constraints better.')  # NOQA: ignore=E501
-
-    except MultipleModuleTypes:
-        msg = "this command needs exactly **one** module type active."
-        tty.die(msg)
diff --git a/lib/spack/spack/cmd/dotkit.py b/lib/spack/spack/cmd/dotkit.py
new file mode 100644
index 0000000000..619389eb04
--- /dev/null
+++ b/lib/spack/spack/cmd/dotkit.py
@@ -0,0 +1,42 @@
+##############################################################################
+# 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 spack.cmd.common.modules
+
+description = "manipulate dotkit module files"
+section = "environment"
+level = "short"
+
+#: Type of the modules managed by this command
+_module_type = 'dotkit'
+
+
+def setup_parser(subparser):
+    spack.cmd.common.modules.setup_parser(subparser)
+
+
+def dotkit(parser, args):
+    spack.cmd.common.modules.modules_cmd(
+        parser, args, module_type=_module_type
+    )
diff --git a/lib/spack/spack/cmd/lmod.py b/lib/spack/spack/cmd/lmod.py
new file mode 100644
index 0000000000..620ce38209
--- /dev/null
+++ b/lib/spack/spack/cmd/lmod.py
@@ -0,0 +1,42 @@
+##############################################################################
+# 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 spack.cmd.common.modules
+
+description = "manipulate hierarchical module files"
+section = "environment"
+level = "short"
+
+#: Type of the modules managed by this command
+_module_type = 'lmod'
+
+
+def setup_parser(subparser):
+    spack.cmd.common.modules.setup_parser(subparser)
+
+
+def lmod(parser, args):
+    spack.cmd.common.modules.modules_cmd(
+        parser, args, module_type=_module_type
+    )
diff --git a/lib/spack/spack/cmd/tcl.py b/lib/spack/spack/cmd/tcl.py
new file mode 100644
index 0000000000..56e40e9df9
--- /dev/null
+++ b/lib/spack/spack/cmd/tcl.py
@@ -0,0 +1,42 @@
+##############################################################################
+# 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 spack.cmd.common.modules
+
+description = "manipulate non-hierarchical module files"
+section = "environment"
+level = "short"
+
+#: Type of the modules managed by this command
+_module_type = 'tcl'
+
+
+def setup_parser(subparser):
+    spack.cmd.common.modules.setup_parser(subparser)
+
+
+def tcl(parser, args):
+    spack.cmd.common.modules.modules_cmd(
+        parser, args, module_type=_module_type
+    )
diff --git a/lib/spack/spack/schema/config.py b/lib/spack/spack/schema/config.py
index ac94a12558..9773213b7d 100644
--- a/lib/spack/spack/schema/config.py
+++ b/lib/spack/spack/schema/config.py
@@ -31,7 +31,7 @@
 
 schema = {
     '$schema': 'http://json-schema.org/schema#',
-    'title': 'Spack module file configuration file schema',
+    'title': 'Spack core configuration file schema',
     'type': 'object',
     'additionalProperties': False,
     'patternProperties': {
diff --git a/lib/spack/spack/test/cmd/module.py b/lib/spack/spack/test/cmd/dotkit.py
index df55f0bd5e..a9a3ac5507 100644
--- a/lib/spack/spack/test/cmd/module.py
+++ b/lib/spack/spack/test/cmd/dotkit.py
@@ -26,27 +26,18 @@ import argparse
 import os.path
 
 import pytest
-import spack.cmd.module as module
-import spack.modules as modules
 
+import spack.cmd.dotkit
+import spack.main
+import spack.modules
 
-def _get_module_files(args):
-
-    files = []
-    specs = args.specs()
+dotkit = spack.main.SpackCommand('dotkit')
 
-    for module_type in args.module_type:
-        writer_cls = modules.module_types[module_type]
-        files.extend([writer_cls(spec).layout.filename for spec in specs])
-    return files
 
-
-@pytest.fixture(scope='module')
-def parser():
-    """Returns the parser for the module command"""
-    parser = argparse.ArgumentParser()
-    module.setup_parser(parser)
-    return parser
+def _get_module_files(args):
+    specs = args.specs()
+    writer_cls = spack.modules.module_types['dotkit']
+    return [writer_cls(spec).layout.filename for spec in specs]
 
 
 @pytest.fixture(
@@ -54,8 +45,6 @@ def parser():
         ['rm', 'doesnotexist'],  # Try to remove a non existing module
         ['find', 'mpileaks'],  # Try to find a module with multiple matches
         ['find', 'doesnotexist'],  # Try to find a module with no matches
-        # Try to find a module specifying more than one type
-        ['find', '-m', 'tcl', '-m', 'lmod', 'libelf'],
     ]
 )
 def failure_args(request):
@@ -63,55 +52,34 @@ def failure_args(request):
     return request.param
 
 
+@pytest.fixture(scope='module')
+def parser():
+    """Returns the parser for the module command"""
+    parser = argparse.ArgumentParser()
+    spack.cmd.dotkit.setup_parser(parser)
+    return parser
+
 # TODO : test the --delete-tree option
 # TODO : this requires having a separate directory for test modules
 # TODO : add tests for loads and find to check the prompt format
 
-@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)
-
 
 @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]
-    args = parser.parse_args(['rm', '-y', '-m', 'tcl', 'mpileaks'])
-    module_files = _get_module_files(args)
-
-    for item in module_files:
-        assert os.path.exists(item)
-
-    module.module(parser, args)
-
-    for item in module_files:
-        assert not os.path.exists(item)
-
-    # Add them back [tcl]
-    args = parser.parse_args(['refresh', '-y', '-m', 'tcl', 'mpileaks'])
-    module.module(parser, args)
-
-    for item in module_files:
-        assert os.path.exists(item)
+def test_exit_with_failure(database, failure_args):
+    with pytest.raises(spack.main.SpackCommandError):
+        dotkit(*failure_args)
 
 
 @pytest.mark.db
 @pytest.mark.usefixtures('database')
 @pytest.mark.parametrize('cli_args', [
-    ['--module-type', 'tcl', 'libelf'],
-    ['--module-type', 'tcl', '--full-path', 'libelf']
+    ['libelf'],
+    ['--full-path', 'libelf']
 ])
-def test_find(parser, cli_args):
-    """Tests the 'spack module find' under a few common scenarios."""
-
-    # Try to find it for tcl module files
-    args = parser.parse_args(['find'] + cli_args)
-    module.module(parser, args)
+def test_find(cli_args):
+    """Tests 'spack dotkit find' under a few common scenarios."""
+    dotkit(*(['find'] + cli_args))
 
 
 @pytest.mark.db
@@ -119,17 +87,15 @@ def test_find(parser, cli_args):
 def test_remove_and_add_dotkit(parser):
     """Tests adding and removing a dotkit module file."""
 
-    # Remove existing modules [dotkit]
-    args = parser.parse_args(['rm', '-y', '-m', 'dotkit', 'mpileaks'])
-    module_files = _get_module_files(args)
+    rm_cli_args = ['rm', '-y', 'mpileaks']
+    module_files = _get_module_files(parser.parse_args(rm_cli_args))
     for item in module_files:
         assert os.path.exists(item)
-    module.module(parser, args)
+
+    dotkit(*rm_cli_args)
     for item in module_files:
         assert not os.path.exists(item)
 
-    # Add them back [dotkit]
-    args = parser.parse_args(['refresh', '-y', '-m', 'dotkit', 'mpileaks'])
-    module.module(parser, args)
+    dotkit('refresh', '-y', 'mpileaks')
     for item in module_files:
         assert os.path.exists(item)
diff --git a/lib/spack/spack/test/cmd/lmod.py b/lib/spack/spack/test/cmd/lmod.py
new file mode 100644
index 0000000000..c7c0089be9
--- /dev/null
+++ b/lib/spack/spack/test/cmd/lmod.py
@@ -0,0 +1,63 @@
+##############################################################################
+# 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
+
+import spack.main
+import spack.modules as modules
+
+lmod = spack.main.SpackCommand('lmod')
+
+
+def _get_module_files(args):
+
+    files = []
+    specs = args.specs()
+
+    for module_type in args.module_type:
+        writer_cls = modules.module_types[module_type]
+        files.extend([writer_cls(spec).layout.filename for spec in specs])
+    return files
+
+
+@pytest.fixture(
+    params=[
+        ['rm', 'doesnotexist'],  # Try to remove a non existing module
+        ['find', 'mpileaks'],  # Try to find a module with multiple matches
+        ['find', 'doesnotexist'],  # Try to find a module with no matches
+    ]
+)
+def failure_args(request):
+    """A list of arguments that will cause a failure"""
+    return request.param
+
+
+# TODO : test the --delete-tree option
+# 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, failure_args):
+    with pytest.raises(spack.main.SpackCommandError):
+        lmod(*failure_args)
diff --git a/lib/spack/spack/test/cmd/tcl.py b/lib/spack/spack/test/cmd/tcl.py
new file mode 100644
index 0000000000..64182cbe2e
--- /dev/null
+++ b/lib/spack/spack/test/cmd/tcl.py
@@ -0,0 +1,95 @@
+##############################################################################
+# 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 argparse
+import os.path
+
+import pytest
+
+import spack.cmd.tcl
+import spack.main
+import spack.modules
+
+tcl = spack.main.SpackCommand('tcl')
+
+
+def _get_module_files(args):
+    specs = args.specs()
+    writer_cls = spack.modules.module_types['tcl']
+    return [writer_cls(spec).layout.filename for spec in specs]
+
+
+@pytest.fixture(
+    params=[
+        ['rm', 'doesnotexist'],  # Try to remove a non existing module
+        ['find', 'mpileaks'],  # Try to find a module with multiple matches
+        ['find', 'doesnotexist'],  # Try to find a module with no matches
+    ]
+)
+def failure_args(request):
+    """A list of arguments that will cause a failure"""
+    return request.param
+
+
+@pytest.fixture(scope='module')
+def parser():
+    """Returns the parser for the module command"""
+    parser = argparse.ArgumentParser()
+    spack.cmd.tcl.setup_parser(parser)
+    return parser
+
+# TODO : test the --delete-tree option
+# 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, failure_args):
+    with pytest.raises(spack.main.SpackCommandError):
+        tcl(*failure_args)
+
+
+def test_remove_and_add_tcl(database, parser):
+    """Tests adding and removing a dotkit module file."""
+
+    rm_cli_args = ['rm', '-y', 'mpileaks']
+    module_files = _get_module_files(parser.parse_args(rm_cli_args))
+    for item in module_files:
+        assert os.path.exists(item)
+
+    tcl(*rm_cli_args)
+    for item in module_files:
+        assert not os.path.exists(item)
+
+    tcl('refresh', '-y', 'mpileaks')
+    for item in module_files:
+        assert os.path.exists(item)
+
+
+@pytest.mark.parametrize('cli_args', [
+    ['libelf'],
+    ['--full-path', 'libelf']
+])
+def test_find(database, cli_args):
+    """Tests 'spack tcl find' under a few common scenarios."""
+    tcl(*(['find'] + cli_args))