Merge pull request #108 from asottile/fix_helps

Fix --help for local-run and add a test

paasta_tools/paasta_cli/cmds/local_run.py

@@ -261,20 +261,24 @@ def add_subparser(subparsers):
     build_pull_group = list_parser.add_mutually_exclusive_group()
     build_pull_group.add_argument(
         '-b', '--build',
-        help=("Build the docker image to run from scratch using the local Makefile's ",
-              "'cook-image' target. Defaults to try to use the local Makefile if present. ",
-              "otherwise local-run will pull and run the Docker image that is marked for ",
-              "deployment in the Docker registry. Mutually exclusive with '--pull'."),
+        help=(
+            "Build the docker image to run from scratch using the local Makefile's "
+            "'cook-image' target. Defaults to try to use the local Makefile if present. "
+            "otherwise local-run will pull and run the Docker image that is marked for "
+            "deployment in the Docker registry. Mutually exclusive with '--pull'."
+        ),
         required=False,
         action='store_true',
         default=None,
     )
     build_pull_group.add_argument(
         '-p', '--pull',
-        help=("Pull the docker image marked for deployment from the Docker registry and ",
-              "use that for the local-run. This is the opposite of --build. Defaults to ",
-              "autodetect a Makefile, if present will not pull, and instead assume that ",
-              "a local build is desired. Mutally exclusive with '--build'"),
+        help=(
+            "Pull the docker image marked for deployment from the Docker registry and "
+            "use that for the local-run. This is the opposite of --build. Defaults to "
+            "autodetect a Makefile, if present will not pull, and instead assume that "
+            "a local build is desired. Mutally exclusive with '--build'"
+        ),
         required=False,
         action='store_true',
         default=None,

paasta_tools/paasta_cli/paasta_cli.py

@@ -20,7 +20,7 @@
 
 from paasta_tools.paasta_cli import cmds
 from paasta_tools.paasta_cli.utils \
-    import file_names_in_dir as paasta_commands_dir, load_method
+    import modules_in_pkg as paasta_commands_dir, load_method
 from paasta_tools.utils import configure_log
 
 
@@ -40,32 +40,37 @@ def add_subparser(command, subparsers):
     add_subparser_fn(subparsers)
 
 
-def parse_args():
-    """Initialize autocompletion and configure the argument parser.
-
-    :return: an argparse.Namespace object mapping parameter names to the inputs
-             from sys.argv
-    """
+def get_argparser():
     parser = argparse.ArgumentParser(description="Yelp PaaSTA client")
 
     subparsers = parser.add_subparsers(help="[-h, --help] for subcommand help")
 
     for command in sorted(paasta_commands_dir(cmds)):
         add_subparser(command, subparsers)
 
+    return parser
+
+
+def parse_args(argv):
+    """Initialize autocompletion and configure the argument parser.
+
+    :return: an argparse.Namespace object mapping parameter names to the inputs
+             from sys.argv
+    """
+    parser = get_argparser()
     argcomplete.autocomplete(parser)
 
-    return parser.parse_args()
+    return parser.parse_args(argv)
 
 
-def main():
+def main(argv=None):
     """Perform a paasta call. Read args from sys.argv and pass parsed args onto
     appropriate command in paata_cli/cmds directory.
 
     Ensure we kill any child pids before we quit
     """
     configure_log()
-    args = parse_args()
+    args = parse_args(argv)
     args.command(args)
 
 if __name__ == '__main__':

paasta_tools/paasta_cli/utils.py

@@ -13,8 +13,8 @@
 # limitations under the License.
 
 import fnmatch
-import glob
 import logging
+import pkgutil
 import os
 from socket import gaierror
 from socket import gethostbyname_ex
@@ -46,21 +46,15 @@ def load_method(module_name, method_name):
     return method
 
 
-def file_names_in_dir(directory):
-    """Read and return the files names in the directory.
+def modules_in_pkg(pkg):
+    """Return the list of modules in a python package (a module with a
+    __init__.py file.)
 
-    :return: a list of strings such as ['list','check'] that correspond to the
-             files in the directory without their extensions.
+    :return: a list of strings such as `['list', 'check']` that correspond to
+             the module names in the package.
     """
-    dir_path = os.path.dirname(os.path.abspath(directory.__file__))
-    path = os.path.join(dir_path, '*.py')
-
-    for file_name in glob.glob(path):
-        basename = os.path.basename(file_name)
-        root, _ = os.path.splitext(basename)
-        if root == '__init__':
-            continue
-        yield root
+    for _, module_name, _ in pkgutil.walk_packages(pkg.__path__):
+        yield module_name
 
 
 def is_file_in_dir(file_name, path):

tests/paasta_cli/test_cmds_help.py

@@ -11,11 +11,15 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
+import argparse
+from StringIO import StringIO
 
+import pytest
 from mock import patch
-from StringIO import StringIO
 
 from paasta_tools.paasta_cli.cmds.help import paasta_help
+from paasta_tools.paasta_cli.paasta_cli import get_argparser
+from paasta_tools.paasta_cli.paasta_cli import main
 
 
 @patch('sys.stdout', new_callable=StringIO)
@@ -24,3 +28,22 @@ def test_list_paasta_list(mock_stdout):
     paasta_help(args)
     output = mock_stdout.getvalue()
     assert 'http://y/paasta' in output
+
+
+def each_command():
+    parser = get_argparser()
+    # We're doing some wacky inspection here, let's make sure things are sane
+    assert isinstance(parser._actions[1], argparse._SubParsersAction)
+    choices = tuple(parser._actions[1].choices)
+    assert choices
+    assert 'local-run' in choices
+    return choices
+
+
+@pytest.mark.parametrize('cmd', each_command())
+def test_help(cmd, capsys):
+    # Should pass and produce something
+    with pytest.raises(SystemExit) as excinfo:
+        main((cmd, '--help'))
+    assert excinfo.value.code == 0
+    assert cmd in capsys.readouterr()[0]

tests/paasta_cli/test_utils.py

@@ -394,3 +394,11 @@ def test_list_teams():
 def test_lazy_choices_completer():
     completer = utils.lazy_choices_completer(lambda: ['1', '2', '3'])
     assert completer(prefix='') == ['1', '2', '3']
+
+
+def test_modules_in_pkg():
+    from paasta_tools.paasta_cli import cmds
+    ret = tuple(utils.modules_in_pkg(cmds))
+    assert '__init__' not in ret
+    assert 'version' in ret
+    assert 'list_clusters' in ret