Updated unit test.

Author: kmvanbrunt

cmd2/cmd2.py

@@ -875,7 +875,8 @@ def unregister_command_set(self, cmdset: CommandSet) -> None:
                 if command in self._cmd_to_command_sets:
                     del self._cmd_to_command_sets[command]
 
-                # A command synonym does not own the parser.
+                # Only remove the parser if this is the actual
+                # command since command synonyms don't own it.
                 if cmd_func_name == command_method.__name__:
                     self._command_parsers.remove(command_method)
 
@@ -890,6 +891,18 @@ def unregister_command_set(self, cmdset: CommandSet) -> None:
             self._installed_command_sets.remove(cmdset)
 
     def _check_uninstallable(self, cmdset: CommandSet) -> None:
+        def check_parser_uninstallable(parser: argparse.ArgumentParser) -> None:
+            for action in parser._actions:
+                if isinstance(action, argparse._SubParsersAction):
+                    for subparser in action.choices.values():
+                        attached_cmdset = getattr(subparser, constants.PARSER_ATTR_COMMANDSET, None)
+                        if attached_cmdset is not None and attached_cmdset is not cmdset:
+                            raise CommandSetRegistrationError(
+                                'Cannot uninstall CommandSet when another CommandSet depends on it'
+                            )
+                        check_parser_uninstallable(subparser)
+                    break
+
         methods: List[Tuple[str, Callable[..., Any]]] = inspect.getmembers(
             cmdset,
             predicate=lambda meth: isinstance(meth, Callable)  # type: ignore[arg-type]
@@ -898,26 +911,12 @@ def _check_uninstallable(self, cmdset: CommandSet) -> None:
         )
 
         for cmd_func_name, command_method in methods:
-            # Do nothing if this is a command synonym since it does not own the parser.
-            if cmd_func_name != command_method.__name__:
-                continue
-
-            command_parser = self._command_parsers.get(command_method)
-
-            def check_parser_uninstallable(parser: argparse.ArgumentParser) -> None:
-                for action in parser._actions:
-                    if isinstance(action, argparse._SubParsersAction):
-                        for subparser in action.choices.values():
-                            attached_cmdset = getattr(subparser, constants.PARSER_ATTR_COMMANDSET, None)
-                            if attached_cmdset is not None and attached_cmdset is not cmdset:
-                                raise CommandSetRegistrationError(
-                                    'Cannot uninstall CommandSet when another CommandSet depends on it'
-                                )
-                            check_parser_uninstallable(subparser)
-                        break
-
-            if command_parser is not None:
-                check_parser_uninstallable(command_parser)
+            # We only need to check if it's safe to remove the parser if this
+            # is the actual command since command synonyms don't own it.
+            if cmd_func_name == command_method.__name__:
+                command_parser = self._command_parsers.get(command_method)
+                if command_parser is not None:
+                    check_parser_uninstallable(command_parser)
 
     def _register_subcommands(self, cmdset: Union[CommandSet, 'Cmd']) -> None:
         """

tests_isolated/test_commandset/test_commandset.py

@@ -164,8 +164,10 @@ def do_builtin(self, _):
         # Create a synonym to a command inside of this CommandSet
         do_builtin_synonym = do_builtin
 
-        # Create a synonym to a command outside of this CommandSet
-        do_help_synonym = cmd2.Cmd.do_help
+        # Create a synonym to a command outside of this CommandSet with subcommands.
+        # This will best test the synonym check in cmd2.Cmd._check_uninstallable() when
+        # we unresgister this CommandSet.
+        do_alias_synonym = cmd2.Cmd.do_alias
 
     cs = SynonymCommandSet("foo")
     app = WithCommandSets(command_sets=[cs])
@@ -176,21 +178,21 @@ def do_builtin(self, _):
     assert builtin_parser is not None
     assert builtin_parser is builtin_synonym_parser
 
-    help_parser = app._command_parsers.get(cmd2.Cmd.do_help)
-    help_synonym_parser = app._command_parsers.get(app.do_help_synonym)
-    assert help_parser is not None
-    assert help_parser is help_synonym_parser
+    alias_parser = app._command_parsers.get(cmd2.Cmd.do_alias)
+    alias_synonym_parser = app._command_parsers.get(app.do_alias_synonym)
+    assert alias_parser is not None
+    assert alias_parser is alias_synonym_parser
 
     # Unregister the CommandSet and make sure built-in command and synonyms are gone
     app.unregister_command_set(cs)
     assert not hasattr(app, "do_builtin")
     assert not hasattr(app, "do_builtin_synonym")
-    assert not hasattr(app, "do_help_synonym")
+    assert not hasattr(app, "do_alias_synonym")
 
-    # Make sure the help command still exists, has the same parser, and works.
-    assert help_parser is app._command_parsers.get(cmd2.Cmd.do_help)
-    out, err = run_cmd(app, 'help')
-    assert app.doc_header in out
+    # Make sure the alias command still exists, has the same parser, and works.
+    assert alias_parser is app._command_parsers.get(cmd2.Cmd.do_alias)
+    out, err = run_cmd(app, 'alias --help')
+    assert normalize(alias_parser.format_help())[0] in out
 
 
 def test_custom_construct_commandsets():