Merge pull request #980 from python-cmd2/move_module_loading

Fixed AttributeError when loading CommandSet

14 files changed, 336 insertions, 52 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index fd8d29ba..292c9115 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,8 @@
+## 1.3.4 (TBD)
+* Bug Fixes
+    * Fixed `AttributeError` when `CommandSet` that uses `as_subcommand_to` decorator is loaded during
+    `cmd2.Cmd.__init__()`.
+
 ## 1.3.3 (August 13, 2020)
 * Breaking changes
     * CommandSet command functions (do_, complete_, help_) will no longer have the cmd2 app
diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py
index 610ce4a3..f6a5251c 100644
--- a/cmd2/cmd2.py
+++ b/cmd2/cmd2.py
@@ -259,22 +259,6 @@ class Cmd(cmd.Cmd):
                                                 multiline_commands=multiline_commands,
                                                 shortcuts=shortcuts)
 
-        # Load modular commands
-        self._installed_command_sets = []  # type: List[CommandSet]
-        self._cmd_to_command_sets = {}  # type: Dict[str, CommandSet]
-        if command_sets:
-            for command_set in command_sets:
-                self.register_command_set(command_set)
-
-        if auto_load_commands:
-            self._autoload_commands()
-
-        # Verify commands don't have invalid names (like starting with a shortcut)
-        for cur_cmd in self.get_all_commands():
-            valid, errmsg = self.statement_parser.is_valid_command(cur_cmd)
-            if not valid:
-                raise ValueError("Invalid command name {!r}: {}".format(cur_cmd, errmsg))
-
         # Stores results from the last command run to enable usage of results in a Python script or interactive console
         # Built-in commands don't make use of this.  It is purely there for user-defined commands and convenience.
         self.last_result = None
@@ -412,6 +396,28 @@ class Cmd(cmd.Cmd):
         # If False, then complete() will sort the matches using self.default_sort_key before they are displayed.
         self.matches_sorted = False
 
+        ############################################################################################################
+        # The following code block loads CommandSets, verifies command names, and registers subcommands.
+        # This block should appear after all attributes have been created since the registration code
+        # depends on them and it's possible a module's on_register() method may need to access some.
+        ############################################################################################################
+        # Load modular commands
+        self._installed_command_sets = []  # type: List[CommandSet]
+        self._cmd_to_command_sets = {}  # type: Dict[str, CommandSet]
+        if command_sets:
+            for command_set in command_sets:
+                self.register_command_set(command_set)
+
+        if auto_load_commands:
+            self._autoload_commands()
+
+        # Verify commands don't have invalid names (like starting with a shortcut)
+        for cur_cmd in self.get_all_commands():
+            valid, errmsg = self.statement_parser.is_valid_command(cur_cmd)
+            if not valid:
+                raise ValueError("Invalid command name {!r}: {}".format(cur_cmd, errmsg))
+
+        # Add functions decorated to be subcommands
         self._register_subcommands(self)
 
     def find_commandsets(self, commandset_type: Type[CommandSet], *, subclass_match: bool = False) -> List[CommandSet]:
@@ -631,10 +637,14 @@ class Cmd(cmd.Cmd):
 
         # iterate through all matching methods
         for method_name, method in methods:
-            subcommand_name = getattr(method, constants.SUBCMD_ATTR_NAME)
+            subcommand_name = getattr(method, constants.SUBCMD_ATTR_NAME)  # type: str
             full_command_name = getattr(method, constants.SUBCMD_ATTR_COMMAND)  # type: str
             subcmd_parser = getattr(method, constants.CMD_ATTR_ARGPARSER)
 
+            subcommand_valid, errmsg = self.statement_parser.is_valid_command(subcommand_name, is_subcommand=True)
+            if not subcommand_valid:
+                raise CommandSetRegistrationError('Subcommand {} is not valid: {}'.format(str(subcommand_name), errmsg))
+
             command_tokens = full_command_name.split()
             command_name = command_tokens[0]
             subcommand_names = command_tokens[1:]
diff --git a/cmd2/parsing.py b/cmd2/parsing.py
index a7ee74a1..657db32c 100755
--- a/cmd2/parsing.py
+++ b/cmd2/parsing.py
@@ -277,7 +277,7 @@ class StatementParser:
         expr = r'\A\s*(\S*?)({})'.format(second_group)
         self._command_pattern = re.compile(expr)
 
-    def is_valid_command(self, word: str) -> Tuple[bool, str]:
+    def is_valid_command(self, word: str, *, is_subcommand: bool = False) -> Tuple[bool, str]:
         """Determine whether a word is a valid name for a command.
 
         Commands can not include redirection characters, whitespace,
@@ -285,6 +285,7 @@ class StatementParser:
         shortcut.
 
         :param word: the word to check as a command
+        :param is_subcommand: Flag whether this command name is a subcommand name
         :return: a tuple of a boolean and an error string
 
         If word is not a valid command, return ``False`` and an error string
@@ -297,18 +298,22 @@ class StatementParser:
         """
         valid = False
 
+        if not isinstance(word, str):
+            return False, 'must be a string. Received {} instead'.format(str(type(word)))
+
         if not word:
             return False, 'cannot be an empty string'
 
         if word.startswith(constants.COMMENT_CHAR):
             return False, 'cannot start with the comment character'
 
-        for (shortcut, _) in self.shortcuts:
-            if word.startswith(shortcut):
-                # Build an error string with all shortcuts listed
-                errmsg = 'cannot start with a shortcut: '
-                errmsg += ', '.join(shortcut for (shortcut, _) in self.shortcuts)
-                return False, errmsg
+        if not is_subcommand:
+            for (shortcut, _) in self.shortcuts:
+                if word.startswith(shortcut):
+                    # Build an error string with all shortcuts listed
+                    errmsg = 'cannot start with a shortcut: '
+                    errmsg += ', '.join(shortcut for (shortcut, _) in self.shortcuts)
+                    return False, errmsg
 
         errmsg = 'cannot contain: whitespace, quotes, '
         errchars = []
diff --git a/docs/api/index.rst b/docs/api/index.rst
index 17a25907..1a49adfa 100644
--- a/docs/api/index.rst
+++ b/docs/api/index.rst
@@ -32,6 +32,7 @@ This documentation is for ``cmd2`` version |version|.
    py_bridge
    table_creator
    utils
+   plugin_external_test
 
 **Modules**
 
@@ -56,3 +57,4 @@ This documentation is for ``cmd2`` version |version|.
   embedded python environment to the host app
 - :ref:`api/table_creator:cmd2.table_creator` - table creation module
 - :ref:`api/utils:cmd2.utils` - various utility classes and functions
+- :ref:`api/plugin_external_test:cmd2_ext_test` - External test plugin
diff --git a/docs/api/plugin_external_test.rst b/docs/api/plugin_external_test.rst
new file mode 100644
index 00000000..58450b11
--- /dev/null
+++ b/docs/api/plugin_external_test.rst
@@ -0,0 +1,9 @@
+cmd2_ext_test
+=============
+
+External Test Plugin
+
+
+.. autoclass:: cmd2_ext_test.ExternalTestMixin
+    :members:
+
diff --git a/docs/features/builtin_commands.rst b/docs/features/builtin_commands.rst
index e08b5c24..d5112458 100644
--- a/docs/features/builtin_commands.rst
+++ b/docs/features/builtin_commands.rst
@@ -70,6 +70,8 @@ quit
 
 This command exits the ``cmd2`` application.
 
+.. _feature-builtin-commands-run-pyscript:
+
 run_pyscript
 ~~~~~~~~~~~~
 
diff --git a/docs/features/scripting.rst b/docs/features/scripting.rst
index 141eaa62..f92942be 100644
--- a/docs/features/scripting.rst
+++ b/docs/features/scripting.rst
@@ -61,6 +61,8 @@ session.
   (Cmd) command # this is not a comment
 
 
+.. _scripting-python-scripts:
+
 Python Scripts
 --------------
 
diff --git a/docs/index.rst b/docs/index.rst
index 9153c47f..30584f42 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -76,6 +76,15 @@ Plugins
    plugins/index
 
 
+Testing
+=======
+
+.. toctree::
+   :maxdepth: 2
+
+   testing
+
+
 API Reference
 =============
 
diff --git a/docs/plugins/external_test.rst b/docs/plugins/external_test.rst
index 74407b97..ac0026c6 100644
--- a/docs/plugins/external_test.rst
+++ b/docs/plugins/external_test.rst
@@ -5,11 +5,13 @@ Overview
 ~~~~~~~~
 
 .. _cmd2_external_test_plugin:
-   https://github.com/python-cmd2/cmd2-ext-test/
+   https://github.com/python-cmd2/cmd2/tree/cmdset_settables/plugins/ext_test
 
-The cmd2_external_test_plugin_ supports testing of a cmd2 application by exposing access cmd2 commands with the same
-context as from within a cmd2 pyscript.  This allows for verification of an application's support for pyscripts and
-enables the cmd2 application to be tested as part of a larger system integration test.
+The `External Test Plugin <cmd2_external_test_plugin_>`_ supports testing of a cmd2 application by exposing access cmd2
+commands with the same context as from within a cmd2 :ref:`Python Scripts <scripting-python-scripts>`.  This interface
+captures ``stdout``, ``stderr``, as well as any application-specific data returned by the command.  This also allows
+for verification of an application's support for :ref:`Python Scripts <scripting-python-scripts>` and enables the cmd2
+application to be tested as part of a larger system integration test.
 
 
 Example cmd2 Application
@@ -59,11 +61,11 @@ In your test, define a fixture for your cmd2 application
 Writing Tests
 ~~~~~~~~~~~~~
 
-Now write your tests that validate your application using the `app_cmd` function to access
-the cmd2 application's commands. This allows invocation of the application's commands in the
+Now write your tests that validate your application using the :meth:`~cmd2_ext_test.ExternalTestMixin.app_cmd()`
+function to access the cmd2 application's commands. This allows invocation of the application's commands in the
 same format as a user would type. The results from calling a command matches what is returned
-from running an python script with cmd2's pyscript command, which provides stdout, stderr, and
-the command's result data.
+from running an python script with cmd2's :ref:`feature-builtin-commands-run-pyscript` command, which provides
+``stdout``, ``stderr``, and the command's result data.
 
 .. code-block:: python
 
diff --git a/docs/testing.rst b/docs/testing.rst
new file mode 100644
index 00000000..811e1137
--- /dev/null
+++ b/docs/testing.rst
@@ -0,0 +1,46 @@
+Testing
+=======
+
+.. toctree::
+   :maxdepth: 1
+
+Overview
+~~~~~~~~
+
+This covers special considerations when writing unit tests for a cmd2 application.
+
+
+Testing Commands
+~~~~~~~~~~~~~~~~
+
+The :doc:`External Test Plugin <plugins/external_test>` provides a mixin class with an :meth:`` function that
+allows external calls to application commands.  The :meth:`~cmd2_ext_test.ExternalTestMixin.app_cmd()` function captures
+and returns stdout, stderr, and the command-specific result data.
+
+
+Mocking
+~~~~~~~
+
+.. _python_mock_autospeccing:
+   https://docs.python.org/3/library/unittest.mock.html#autospeccing
+.. _python_mock_patch:
+   https://docs.python.org/3/library/unittest.mock.html#patch
+
+If you need to mock anything in your cmd2 application, and most specifically in sub-classes of :class:`~cmd2.Cmd` or
+:class:`~cmd2.command_definition.CommandSet`, you must use `Autospeccing <python_mock_autospeccing_>`_,
+`spec=True <python_mock_patch_>`_, or whatever equivalant is provided in the mocking library you're using.
+
+In order to automatically load functions as commands cmd2 performs a number of reflection calls to look up attributes
+of classes defined in your cmd2 application. Many mocking libraries will automatically create mock objects to match any
+attribute being requested, regardless of whether they're present in the object being mocked. This behavior can
+incorrectly instruct cmd2 to treat a function or attribute as something it needs to recognize and process. To prevent
+this, you should always mock with `Autospeccing <python_mock_autospeccing_>`_ or `spec=True <python_mock_patch_>`_
+enabled.
+
+Example of spec=True
+====================
+.. code-block:: python
+
+    def test_mocked_methods():
+        with mock.patch.object(MockMethodApp, 'foo', spec=True):
+            cli = MockMethodApp()
diff --git a/noxfile.py b/noxfile.py
index 7d51a948..a46c008e 100644
--- a/noxfile.py
+++ b/noxfile.py
@@ -6,6 +6,7 @@ def docs(session):
     session.install('sphinx',
                     'sphinx-rtd-theme',
                     '.',
+                    'plugins/ext_test',
                     )
     session.chdir('docs')
     tmpdir = session.create_tmp()
diff --git a/tests/test_argparse.py b/tests/test_argparse.py
index 1334f9e3..0d46b15a 100644
--- a/tests/test_argparse.py
+++ b/tests/test_argparse.py
@@ -92,6 +92,7 @@ class ArgparseApp(cmd2.Cmd):
     known_parser.add_argument('-p', '--piglatin', action='store_true', help='atinLay')
     known_parser.add_argument('-s', '--shout', action='store_true', help='N00B EMULATION MODE')
     known_parser.add_argument('-r', '--repeat', type=int, help='output [n] times')
+
     @cmd2.with_argparser(known_parser, with_unknown_args=True)
     def do_speak(self, args, extra, *, keyword_arg: Optional[str] = None):
         """Repeat what you tell me to."""
@@ -131,89 +132,108 @@ def test_invalid_syntax(argparse_app):
     out, err = run_cmd(argparse_app, 'speak "')
     assert err[0] == "Invalid syntax: No closing quotation"
 
+
 def test_argparse_basic_command(argparse_app):
     out, err = run_cmd(argparse_app, 'say hello')
     assert out == ['hello']
 
+
 def test_argparse_remove_quotes(argparse_app):
     out, err = run_cmd(argparse_app, 'say "hello there"')
     assert out == ['hello there']
 
+
 def test_argparser_kwargs(argparse_app, capsys):
     """Test with_argparser wrapper passes through kwargs to command function"""
     argparse_app.do_say('word', keyword_arg="foo")
     out, err = capsys.readouterr()
     assert out == "foo\n"
 
+
 def test_argparse_preserve_quotes(argparse_app):
     out, err = run_cmd(argparse_app, 'tag mytag "hello"')
     assert out[0] == '<mytag>"hello"</mytag>'
 
+
 def test_argparse_custom_namespace(argparse_app):
     out, err = run_cmd(argparse_app, 'test_argparse_ns')
     assert out[0] == 'custom'
 
+
 def test_argparse_with_list(argparse_app):
     out, err = run_cmd(argparse_app, 'speak -s hello world!')
     assert out == ['HELLO WORLD!']
 
+
 def test_argparse_with_list_remove_quotes(argparse_app):
     out, err = run_cmd(argparse_app, 'speak -s hello "world!"')
     assert out == ['HELLO WORLD!']
 
+
 def test_argparse_with_list_preserve_quotes(argparse_app):
     out, err = run_cmd(argparse_app, 'test_argparse_with_list_quotes "hello" person')
     assert out[0] == '"hello" person'
 
+
 def test_argparse_with_list_custom_namespace(argparse_app):
     out, err = run_cmd(argparse_app, 'test_argparse_with_list_ns')
     assert out[0] == 'custom'
 
+
 def test_argparse_with_list_and_empty_doc(argparse_app):
     out, err = run_cmd(argparse_app, 'speak -s hello world!')
     assert out == ['HELLO WORLD!']
 
+
 def test_argparser_correct_args_with_quotes_and_midline_options(argparse_app):
     out, err = run_cmd(argparse_app, "speak 'This  is a' -s test of the emergency broadcast system!")
     assert out == ['THIS  IS A TEST OF THE EMERGENCY BROADCAST SYSTEM!']
 
+
 def test_argparser_and_unknown_args_kwargs(argparse_app, capsys):
     """Test with_argparser_and_unknown_args wrapper passes through kwargs to command function"""
     argparse_app.do_speak('', keyword_arg="foo")
     out, err = capsys.readouterr()
     assert out == "foo\n"
 
+
 def test_argparse_quoted_arguments_multiple(argparse_app):
     out, err = run_cmd(argparse_app, 'say "hello  there" "rick & morty"')
     assert out == ['hello  there rick & morty']
 
+
 def test_argparse_help_docstring(argparse_app):
     out, err = run_cmd(argparse_app, 'help say')
     assert out[0].startswith('usage: say')
     assert out[1] == ''
     assert out[2] == 'Repeat what you tell me to.'
 
+
 def test_argparse_help_description(argparse_app):
     out, err = run_cmd(argparse_app, 'help tag')
     assert out[0].startswith('usage: tag')
     assert out[1] == ''
     assert out[2] == 'create a html tag'
 
+
 def test_argparse_prog(argparse_app):
     out, err = run_cmd(argparse_app, 'help tag')
     progname = out[0].split(' ')[1]
     assert progname == 'tag'
 
+
 def test_arglist(argparse_app):
     out, err = run_cmd(argparse_app, 'arglist "we  should" get these')
     assert out[0] == 'True'
 
+
 def test_arglist_kwargs(argparse_app, capsys):
     """Test with_argument_list wrapper passes through kwargs to command function"""
     argparse_app.do_arglist('arg', keyword_arg="foo")
     out, err = capsys.readouterr()
     assert out == "foo\n"
 
+
 def test_preservelist(argparse_app):
     out, err = run_cmd(argparse_app, 'preservelist foo "bar baz"')
     assert out[0] == "['foo', '\"bar baz\"']"
@@ -269,6 +289,7 @@ class SubcommandApp(cmd2.Cmd):
         func = getattr(args, 'func')
         func(self, args)
 
+
 @pytest.fixture
 def subcommand_app():
     app = SubcommandApp()
@@ -284,17 +305,20 @@ def test_subcommand_bar(subcommand_app):
     out, err = run_cmd(subcommand_app, 'base bar baz')
     assert out == ['((baz))']
 
+
 def test_subcommand_invalid(subcommand_app):
     out, err = run_cmd(subcommand_app, 'base baz')
     assert err[0].startswith('usage: base')
     assert err[1].startswith("base: error: argument SUBCOMMAND: invalid choice: 'baz'")
 
+
 def test_subcommand_base_help(subcommand_app):
     out, err = run_cmd(subcommand_app, 'help base')
     assert out[0].startswith('usage: base')
     assert out[1] == ''
     assert out[2] == 'Base command help'
 
+
 def test_subcommand_help(subcommand_app):
     # foo has no aliases
     out, err = run_cmd(subcommand_app, 'help base foo')
@@ -334,10 +358,12 @@ def test_subcommand_help(subcommand_app):
     assert out[1] == ''
     assert out[2] == 'positional arguments:'
 
+
 def test_subcommand_invalid_help(subcommand_app):
     out, err = run_cmd(subcommand_app, 'help base baz')
     assert out[0].startswith('usage: base')
 
+
 def test_add_another_subcommand(subcommand_app):
     """
     This tests makes sure _set_parser_prog() sets _prog_prefix on every _SubParsersAction so that all future calls
@@ -345,3 +371,40 @@ def test_add_another_subcommand(subcommand_app):
     """
     new_parser = subcommand_app.base_subparsers.add_parser('new_sub', help="stuff")
     assert new_parser.prog == "base new_sub"
+
+
+def test_unittest_mock():
+    from unittest import mock
+    from cmd2 import CommandSetRegistrationError
+
+    with mock.patch.object(ArgparseApp, 'namespace_provider'):
+        with pytest.raises(CommandSetRegistrationError):
+            app = ArgparseApp()
+
+    with mock.patch.object(ArgparseApp, 'namespace_provider', spec=True):
+        app = ArgparseApp()
+
+    with mock.patch.object(ArgparseApp, 'namespace_provider', spec_set=True):
+        app = ArgparseApp()
+
+    with mock.patch.object(ArgparseApp, 'namespace_provider', autospec=True):
+        app = ArgparseApp()
+
+
+def test_pytest_mock_invalid(mocker):
+    from cmd2 import CommandSetRegistrationError
+
+    mocker.patch.object(ArgparseApp, 'namespace_provider')
+    with pytest.raises(CommandSetRegistrationError):
+        app = ArgparseApp()
+
+
+@pytest.mark.parametrize('spec_param', [
+    {'spec': True},
+    {'spec_set': True},
+    {'autospec': True},
+])
+def test_pytest_mock_valid(mocker, spec_param):
+    mocker.patch.object(ArgparseApp, 'namespace_provider', **spec_param)
+    app = ArgparseApp()
+
diff --git a/tests/test_parsing.py b/tests/test_parsing.py
index c2c242fe..2eccec7c 100755
--- a/tests/test_parsing.py
+++ b/tests/test_parsing.py
@@ -25,6 +25,7 @@ def parser():
     )
     return parser
 
+
 @pytest.fixture
 def default_parser():
     parser = StatementParser()
@@ -48,6 +49,7 @@ def test_parse_empty_string(parser):
     assert statement.command_and_args == line
     assert statement.argv == statement.arg_list
 
+
 def test_parse_empty_string_default(default_parser):
     line = ''
     statement = default_parser.parse(line)
@@ -65,6 +67,7 @@ def test_parse_empty_string_default(default_parser):
     assert statement.command_and_args == line
     assert statement.argv == statement.arg_list
 
+
 @pytest.mark.parametrize('line,tokens', [
     ('command', ['command']),
     (constants.COMMENT_CHAR + 'comment', []),
@@ -79,6 +82,7 @@ def test_tokenize_default(default_parser, line, tokens):
     tokens_to_test = default_parser.tokenize(line)
     assert tokens_to_test == tokens
 
+
 @pytest.mark.parametrize('line,tokens', [
     ('command', ['command']),
     ('# comment', []),
@@ -96,10 +100,12 @@ def test_tokenize(parser, line, tokens):
     tokens_to_test = parser.tokenize(line)
     assert tokens_to_test == tokens
 
+
 def test_tokenize_unclosed_quotes(parser):
     with pytest.raises(exceptions.Cmd2ShlexError):
         _ = parser.tokenize('command with "unclosed quotes')
 
+
 @pytest.mark.parametrize('tokens,command,args', [
     ([], '', ''),
     (['command'], 'command', ''),
@@ -110,6 +116,7 @@ def test_command_and_args(parser, tokens, command, args):
     assert command == parsed_command
     assert args == parsed_args
 
+
 @pytest.mark.parametrize('line', [
     'plainword',
     '"one word"',
@@ -131,6 +138,7 @@ def test_parse_single_word(parser, line):
     assert statement.output_to == ''
     assert statement.command_and_args == line
 
+
 @pytest.mark.parametrize('line,terminator', [
     ('termbare;', ';'),
     ('termbare ;', ';'),
@@ -146,6 +154,7 @@ def test_parse_word_plus_terminator(parser, line, terminator):
     assert statement.terminator == terminator
     assert statement.expanded_command_line == statement.command + statement.terminator
 
+
 @pytest.mark.parametrize('line,terminator', [
     ('termbare;  suffx', ';'),
     ('termbare ;suffx', ';'),
@@ -163,6 +172,7 @@ def test_parse_suffix_after_terminator(parser, line, terminator):
     assert statement.suffix == 'suffx'
     assert statement.expanded_command_line == statement.command + statement.terminator + ' ' + statement.suffix
 
+
 def test_parse_command_with_args(parser):
     line = 'command with args'
     statement = parser.parse(line)
@@ -172,6 +182,7 @@ def test_parse_command_with_args(parser):
     assert statement.argv == ['command', 'with', 'args']
     assert statement.arg_list == statement.argv[1:]
 
+
 def test_parse_command_with_quoted_args(parser):
     line = 'command with "quoted args" and "some not"'
     statement = parser.parse(line)
@@ -181,6 +192,7 @@ def test_parse_command_with_quoted_args(parser):
     assert statement.argv == ['command', 'with', 'quoted args', 'and', 'some not']
     assert statement.arg_list == ['with', '"quoted args"', 'and', '"some not"']
 
+
 def test_parse_command_with_args_terminator_and_suffix(parser):
     line = 'command with args and terminator; and suffix'
     statement = parser.parse(line)
@@ -192,6 +204,7 @@ def test_parse_command_with_args_terminator_and_suffix(parser):
     assert statement.terminator == ';'
     assert statement.suffix == 'and suffix'
 
+
 def test_parse_comment(parser):
     statement = parser.parse(constants.COMMENT_CHAR + ' this is all a comment')
     assert statement.command == ''
@@ -200,6 +213,7 @@ def test_parse_comment(parser):
     assert not statement.argv
     assert not statement.arg_list
 
+
 def test_parse_embedded_comment_char(parser):
     command_str = 'hi ' + constants.COMMENT_CHAR + ' not a comment'
     statement = parser.parse(command_str)
@@ -209,7 +223,8 @@ def test_parse_embedded_comment_char(parser):
     assert statement.argv == shlex_split(command_str)
     assert statement.arg_list == statement.argv[1:]
 
-@pytest.mark.parametrize('line',[
+
+@pytest.mark.parametrize('line', [
     'simple | piped',
     'simple|piped',
 ])
@@ -223,6 +238,7 @@ def test_parse_simple_pipe(parser, line):
     assert statement.pipe_to == 'piped'
     assert statement.expanded_command_line == statement.command + ' | ' + statement.pipe_to
 
+
 def test_parse_double_pipe_is_not_a_pipe(parser):
     line = 'double-pipe || is not a pipe'
     statement = parser.parse(line)
@@ -233,6 +249,7 @@ def test_parse_double_pipe_is_not_a_pipe(parser):
     assert statement.arg_list == statement.argv[1:]
     assert not statement.pipe_to
 
+
 def test_parse_complex_pipe(parser):
     line = 'command with args, terminator&sufx | piped'
     statement = parser.parse(line)
@@ -245,13 +262,14 @@ def test_parse_complex_pipe(parser):
     assert statement.suffix == 'sufx'
     assert statement.pipe_to == 'piped'
 
+
 @pytest.mark.parametrize('line,output', [
     ('help > out.txt', '>'),
     ('help>out.txt', '>'),
     ('help >> out.txt', '>>'),
     ('help>>out.txt', '>>'),
 ])
-def test_parse_redirect(parser,line, output):
+def test_parse_redirect(parser, line, output):
     statement = parser.parse(line)
     assert statement.command == 'help'
     assert statement == ''
@@ -260,19 +278,13 @@ def test_parse_redirect(parser,line, output):
     assert statement.output_to == 'out.txt'
     assert statement.expanded_command_line == statement.command + ' ' + statement.output + ' ' + statement.output_to
 
-def test_parse_redirect_with_args(parser):
-    line = 'output into > afile.txt'
-    statement = parser.parse(line)
-    assert statement.command == 'output'
-    assert statement == 'into'
-    assert statement.args == statement
-    assert statement.argv == ['output', 'into']
-    assert statement.arg_list == statement.argv[1:]
-    assert statement.output == '>'
-    assert statement.output_to == 'afile.txt'
 
-def test_parse_redirect_with_dash_in_path(parser):
-    line = 'output into > python-cmd2/afile.txt'
+@pytest.mark.parametrize('dest', [
+    'afile.txt',                # without dashes
+    'python-cmd2/afile.txt',    # with dashes in path
+])
+def test_parse_redirect_with_args(parser, dest):
+    line = 'output into > {}'.format(dest)
     statement = parser.parse(line)
     assert statement.command == 'output'
     assert statement == 'into'
@@ -280,7 +292,8 @@ def test_parse_redirect_with_dash_in_path(parser):
     assert statement.argv == ['output', 'into']
     assert statement.arg_list == statement.argv[1:]
     assert statement.output == '>'
-    assert statement.output_to == 'python-cmd2/afile.txt'
+    assert statement.output_to == dest
+
 
 def test_parse_redirect_append(parser):
     line = 'output appended to >> /tmp/afile.txt'
@@ -293,6 +306,7 @@ def test_parse_redirect_append(parser):
     assert statement.output == '>>'
     assert statement.output_to == '/tmp/afile.txt'
 
+
 def test_parse_pipe_then_redirect(parser):
     line = 'output into;sufx | pipethrume plz > afile.txt'
     statement = parser.parse(line)
@@ -307,6 +321,7 @@ def test_parse_pipe_then_redirect(parser):
     assert statement.output == ''
     assert statement.output_to == ''
 
+
 def test_parse_multiple_pipes(parser):
     line = 'output into;sufx | pipethrume plz | grep blah'
     statement = parser.parse(line)
@@ -321,6 +336,7 @@ def test_parse_multiple_pipes(parser):
     assert statement.output == ''
     assert statement.output_to == ''
 
+
 def test_redirect_then_pipe(parser):
     line = 'help alias > file.txt | grep blah'
     statement = parser.parse(line)
@@ -335,6 +351,7 @@ def test_redirect_then_pipe(parser):
     assert statement.output == '>'
     assert statement.output_to == 'file.txt'
 
+
 def test_append_then_pipe(parser):
     line = 'help alias >> file.txt | grep blah'
     statement = parser.parse(line)
@@ -349,6 +366,7 @@ def test_append_then_pipe(parser):
     assert statement.output == '>>'
     assert statement.output_to == 'file.txt'
 
+
 def test_append_then_redirect(parser):
     line = 'help alias >> file.txt > file2.txt'
     statement = parser.parse(line)
@@ -363,6 +381,7 @@ def test_append_then_redirect(parser):
     assert statement.output == '>>'
     assert statement.output_to == 'file.txt'
 
+
 def test_redirect_then_append(parser):
     line = 'help alias > file.txt >> file2.txt'
     statement = parser.parse(line)
@@ -377,6 +396,7 @@ def test_redirect_then_append(parser):
     assert statement.output == '>'
     assert statement.output_to == 'file.txt'
 
+
 def test_redirect_to_quoted_string(parser):
     line = 'help alias > "file.txt"'
     statement = parser.parse(line)
@@ -391,6 +411,7 @@ def test_redirect_to_quoted_string(parser):
     assert statement.output == '>'
     assert statement.output_to == '"file.txt"'
 
+
 def test_redirect_to_single_quoted_string(parser):
     line = "help alias > 'file.txt'"
     statement = parser.parse(line)
@@ -405,6 +426,7 @@ def test_redirect_to_single_quoted_string(parser):
     assert statement.output == '>'
     assert statement.output_to == "'file.txt'"
 
+
 def test_redirect_to_empty_quoted_string(parser):
     line = 'help alias > ""'
     statement = parser.parse(line)
@@ -419,6 +441,7 @@ def test_redirect_to_empty_quoted_string(parser):
     assert statement.output == '>'
     assert statement.output_to == ''
 
+
 def test_redirect_to_empty_single_quoted_string(parser):
     line = "help alias > ''"
     statement = parser.parse(line)
@@ -433,6 +456,7 @@ def test_redirect_to_empty_single_quoted_string(parser):
     assert statement.output == '>'
     assert statement.output_to == ''
 
+
 def test_parse_output_to_paste_buffer(parser):
     line = 'output to paste buffer >> '
     statement = parser.parse(line)
@@ -443,6 +467,7 @@ def test_parse_output_to_paste_buffer(parser):
     assert statement.arg_list == statement.argv[1:]
     assert statement.output == '>>'
 
+
 def test_parse_redirect_inside_terminator(parser):
     """The terminator designates the end of the commmand/arguments portion.
     If a redirector occurs before a terminator, then it will be treated as
@@ -456,7 +481,8 @@ def test_parse_redirect_inside_terminator(parser):
     assert statement.arg_list == statement.argv[1:]
     assert statement.terminator == ';'
 
-@pytest.mark.parametrize('line,terminator',[
+
+@pytest.mark.parametrize('line,terminator', [
     ('multiline with | inside;', ';'),
     ('multiline with | inside ;', ';'),
     ('multiline with | inside;;;', ';'),
@@ -475,6 +501,7 @@ def test_parse_multiple_terminators(parser, line, terminator):
     assert statement.arg_list == statement.argv[1:]
     assert statement.terminator == terminator
 
+
 def test_parse_unfinished_multiliine_command(parser):
     line = 'multiline has > inside an unfinished command'
     statement = parser.parse(line)
@@ -486,6 +513,7 @@ def test_parse_unfinished_multiliine_command(parser):
     assert statement.arg_list == statement.argv[1:]
     assert statement.terminator == ''
 
+
 def test_parse_basic_multiline_command(parser):
     line = 'multiline foo\nbar\n\n'
     statement = parser.parse(line)
@@ -498,7 +526,8 @@ def test_parse_basic_multiline_command(parser):
     assert statement.raw == line
     assert statement.terminator == '\n'
 
-@pytest.mark.parametrize('line,terminator',[
+
+@pytest.mark.parametrize('line,terminator', [
     ('multiline has > inside;', ';'),
     ('multiline has > inside;;;', ';'),
     ('multiline has > inside;; ;;', ';'),
@@ -514,6 +543,7 @@ def test_parse_multiline_command_ignores_redirectors_within_it(parser, line, ter
     assert statement.arg_list == statement.argv[1:]
     assert statement.terminator == terminator
 
+
 def test_parse_multiline_terminated_by_empty_line(parser):
     line = 'multiline command ends\n\n'
     statement = parser.parse(line)
@@ -525,7 +555,8 @@ def test_parse_multiline_terminated_by_empty_line(parser):
     assert statement.arg_list == statement.argv[1:]
     assert statement.terminator == '\n'
 
-@pytest.mark.parametrize('line,terminator',[
+
+@pytest.mark.parametrize('line,terminator', [
     ('multiline command "with\nembedded newline";', ';'),
     ('multiline command "with\nembedded newline";;;', ';'),
     ('multiline command "with\nembedded newline";; ;;', ';'),
@@ -543,6 +574,7 @@ def test_parse_multiline_with_embedded_newline(parser, line, terminator):
     assert statement.arg_list == ['command', '"with\nembedded newline"']
     assert statement.terminator == terminator
 
+
 def test_parse_multiline_ignores_terminators_in_quotes(parser):
     line = 'multiline command "with term; ends" now\n\n'
     statement = parser.parse(line)
@@ -554,6 +586,7 @@ def test_parse_multiline_ignores_terminators_in_quotes(parser):
     assert statement.arg_list == ['command', '"with term; ends"', 'now']
     assert statement.terminator == '\n'
 
+
 def test_parse_command_with_unicode_args(parser):
     line = 'drink café'
     statement = parser.parse(line)
@@ -563,6 +596,7 @@ def test_parse_command_with_unicode_args(parser):
     assert statement.argv == ['drink', 'café']
     assert statement.arg_list == statement.argv[1:]
 
+
 def test_parse_unicode_command(parser):
     line = 'café au lait'
     statement = parser.parse(line)
@@ -572,6 +606,7 @@ def test_parse_unicode_command(parser):
     assert statement.argv == ['café', 'au', 'lait']
     assert statement.arg_list == statement.argv[1:]
 
+
 def test_parse_redirect_to_unicode_filename(parser):
     line = 'dir home > café'
     statement = parser.parse(line)
@@ -583,10 +618,12 @@ def test_parse_redirect_to_unicode_filename(parser):
     assert statement.output == '>'
     assert statement.output_to == 'café'
 
+
 def test_parse_unclosed_quotes(parser):
     with pytest.raises(exceptions.Cmd2ShlexError):
         _ = parser.tokenize("command with 'unclosed quotes")
 
+
 def test_empty_statement_raises_exception():
     app = cmd2.Cmd()
     with pytest.raises(exceptions.EmptyStatement):
@@ -595,6 +632,7 @@ def test_empty_statement_raises_exception():
     with pytest.raises(exceptions.EmptyStatement):
         app._complete_statement(' ')
 
+
 @pytest.mark.parametrize('line,command,args', [
     ('helpalias', 'help', ''),
     ('helpalias mycommand', 'help', 'mycommand'),
@@ -610,6 +648,7 @@ def test_parse_alias_and_shortcut_expansion(parser, line, command, args):
     assert statement == args
     assert statement.args == statement
 
+
 def test_parse_alias_on_multiline_command(parser):
     line = 'anothermultiline has > inside an unfinished command'
     statement = parser.parse(line)
@@ -619,6 +658,7 @@ def test_parse_alias_on_multiline_command(parser):
     assert statement == 'has > inside an unfinished command'
     assert statement.terminator == ''
 
+
 @pytest.mark.parametrize('line,output', [
     ('helpalias > out.txt', '>'),
     ('helpalias>out.txt', '>'),
@@ -633,6 +673,7 @@ def test_parse_alias_redirection(parser, line, output):
     assert statement.output == output
     assert statement.output_to == 'out.txt'
 
+
 @pytest.mark.parametrize('line', [
     'helpalias | less',
     'helpalias|less',
@@ -644,6 +685,7 @@ def test_parse_alias_pipe(parser, line):
     assert statement.args == statement
     assert statement.pipe_to == 'less'
 
+
 @pytest.mark.parametrize('line', [
     'helpalias;',
     'helpalias;;',
@@ -659,6 +701,7 @@ def test_parse_alias_terminator_no_whitespace(parser, line):
     assert statement.args == statement
     assert statement.terminator == ';'
 
+
 def test_parse_command_only_command_and_args(parser):
     line = 'help history'
     statement = parser.parse_command_only(line)
@@ -675,6 +718,7 @@ def test_parse_command_only_command_and_args(parser):
     assert statement.output == ''
     assert statement.output_to == ''
 
+
 def test_parse_command_only_strips_line(parser):
     line = '  help history  '
     statement = parser.parse_command_only(line)
@@ -691,6 +735,7 @@ def test_parse_command_only_strips_line(parser):
     assert statement.output == ''
     assert statement.output_to == ''
 
+
 def test_parse_command_only_expands_alias(parser):
     line = 'fake foobar.py "somebody.py'
     statement = parser.parse_command_only(line)
@@ -707,6 +752,7 @@ def test_parse_command_only_expands_alias(parser):
     assert statement.output == ''
     assert statement.output_to == ''
 
+
 def test_parse_command_only_expands_shortcuts(parser):
     line = '!cat foobar.txt'
     statement = parser.parse_command_only(line)
@@ -724,6 +770,7 @@ def test_parse_command_only_expands_shortcuts(parser):
     assert statement.output == ''
     assert statement.output_to == ''
 
+
 def test_parse_command_only_quoted_args(parser):
     line = 'l "/tmp/directory with spaces/doit.sh"'
     statement = parser.parse_command_only(line)
@@ -741,6 +788,7 @@ def test_parse_command_only_quoted_args(parser):
     assert statement.output == ''
     assert statement.output_to == ''
 
+
 @pytest.mark.parametrize('line,args', [
     ('helpalias > out.txt', '> out.txt'),
     ('helpalias>out.txt', '>out.txt'),
@@ -765,6 +813,7 @@ def test_parse_command_only_specialchars(parser, line, args):
     assert statement.output == ''
     assert statement.output_to == ''
 
+
 @pytest.mark.parametrize('line', [
     '',
     ';',
@@ -794,6 +843,7 @@ def test_parse_command_only_empty(parser, line):
     assert statement.output == ''
     assert statement.output_to == ''
 
+
 def test_parse_command_only_multiline(parser):
     line = 'multiline with partially "open quotes and no terminator'
     statement = parser.parse_command_only(line)
@@ -836,7 +886,16 @@ def test_statement_is_immutable():
         statement.raw = 'baz'
 
 
-def test_is_valid_command_invalid(parser):
+def test_is_valid_command_invalid(mocker, parser):
+    # Non-string command
+    # noinspection PyTypeChecker
+    valid, errmsg = parser.is_valid_command(5)
+    assert not valid and 'must be a string' in errmsg
+
+    mock = mocker.MagicMock()
+    valid, errmsg = parser.is_valid_command(mock)
+    assert not valid and 'must be a string' in errmsg
+
     # Empty command
     valid, errmsg = parser.is_valid_command('')
     assert not valid and 'cannot be an empty string' in errmsg
@@ -865,12 +924,18 @@ def test_is_valid_command_invalid(parser):
     valid, errmsg = parser.is_valid_command(';shell')
     assert not valid and 'cannot contain: whitespace, quotes,' in errmsg
 
+
 def test_is_valid_command_valid(parser):
-    # Empty command
+    # Valid command
     valid, errmsg = parser.is_valid_command('shell')
     assert valid
     assert not errmsg
 
+    # Subcommands can start with shortcut
+    valid, errmsg = parser.is_valid_command('!subcmd', is_subcommand=True)
+    assert valid
+    assert not errmsg
+
 
 def test_macro_normal_arg_pattern():
     # This pattern matches digits surrounded by exactly 1 brace on a side and 1 or more braces on the opposite side
@@ -922,6 +987,7 @@ def test_macro_normal_arg_pattern():
     matches = pattern.findall('{5text}')
     assert not matches
 
+
 def test_macro_escaped_arg_pattern():
     # This pattern matches digits surrounded by 2 or more braces on both sides
     from cmd2.parsing import MacroArg
diff --git a/tests_isolated/test_commandset/test_commandset.py b/tests_isolated/test_commandset/test_commandset.py
index bab5d536..6856881a 100644
--- a/tests_isolated/test_commandset/test_commandset.py
+++ b/tests_isolated/test_commandset/test_commandset.py
@@ -62,6 +62,26 @@ class CommandSetA(CommandSetBase):
         self._cmd.poutput('Elderberry {}!!'.format(ns.arg1))
         self._cmd.last_result = {'arg1': ns.arg1}
 
+    # Test that CommandSet with as_subcommand_to decorator successfully loads
+    # during `cmd2.Cmd.__init__()`.
+    main_parser = cmd2.Cmd2ArgumentParser(description="Main Command")
+    main_subparsers = main_parser.add_subparsers(dest='subcommand', metavar='SUBCOMMAND')
+    main_subparsers.required = True
+
+    @cmd2.with_category('Alone')
+    @cmd2.with_argparser(main_parser)
+    def do_main(self, args: argparse.Namespace) -> None:
+        # Call handler for whatever subcommand was selected
+        handler = args.get_handler()
+        handler(args)
+
+    # main -> sub
+    subcmd_parser = cmd2.Cmd2ArgumentParser(add_help=False, description="Sub Command")
+
+    @cmd2.as_subcommand_to('main', 'sub', subcmd_parser, help="sub command")
+    def subcmd_func(self, args: argparse.Namespace) -> None:
+        self._cmd.poutput("Subcommand Ran")
+
 
 @cmd2.with_default_category('Command Set B')
 class CommandSetB(CommandSetBase):
@@ -87,6 +107,11 @@ def test_autoload_commands(command_sets_app):
 
     assert 'Alone' in cmds_cats
     assert 'elderberry' in cmds_cats['Alone']
+    assert 'main' in cmds_cats['Alone']
+
+    # Test subcommand was autoloaded
+    result = command_sets_app.app_cmd('main sub')
+    assert 'Subcommand Ran' in result.stdout
 
     assert 'Also Alone' in cmds_cats
     assert 'durian' in cmds_cats['Also Alone']
@@ -150,6 +175,11 @@ def test_load_commands(command_sets_manual):
 
     assert 'Alone' in cmds_cats
     assert 'elderberry' in cmds_cats['Alone']
+    assert 'main' in cmds_cats['Alone']
+
+    # Test subcommand was loaded
+    result = command_sets_manual.app_cmd('main sub')
+    assert 'Subcommand Ran' in result.stdout
 
     assert 'Fruits' in cmds_cats
     assert 'cranberry' in cmds_cats['Fruits']
@@ -172,6 +202,11 @@ def test_load_commands(command_sets_manual):
 
     assert 'Alone' in cmds_cats
     assert 'elderberry' in cmds_cats['Alone']
+    assert 'main' in cmds_cats['Alone']
+
+    # Test subcommand was loaded
+    result = command_sets_manual.app_cmd('main sub')
+    assert 'Subcommand Ran' in result.stdout
 
     assert 'Fruits' in cmds_cats
     assert 'cranberry' in cmds_cats['Fruits']
@@ -809,3 +844,30 @@ def test_path_complete(command_sets_manual):
     first_match = complete_tester(text, line, begidx, endidx, command_sets_manual)
 
     assert first_match is not None
+
+
+def test_bad_subcommand():
+    class BadSubcommandApp(cmd2.Cmd):
+        """Class for testing usage of `as_subcommand_to` decorator directly in a Cmd2 subclass."""
+
+        def __init__(self, *args, **kwargs):
+            super(BadSubcommandApp, self).__init__(*args, **kwargs)
+
+        cut_parser = cmd2.Cmd2ArgumentParser('cut')
+        cut_subparsers = cut_parser.add_subparsers(title='item', help='item to cut')
+
+        @cmd2.with_argparser(cut_parser)
+        def do_cut(self, ns: argparse.Namespace):
+            """Cut something"""
+            pass
+
+        banana_parser = cmd2.Cmd2ArgumentParser(add_help=False)
+        banana_parser.add_argument('direction', choices=['discs', 'lengthwise'])
+
+        @cmd2.as_subcommand_to('cut', 'bad name', banana_parser, help='This should fail')
+        def cut_banana(self, ns: argparse.Namespace):
+            """Cut banana"""
+            self.poutput('cutting banana: ' + ns.direction)
+
+    with pytest.raises(CommandSetRegistrationError):
+        app = BadSubcommandApp()