Merge pull request #855 from spotify/erikbern/cmdline-2

more test for command line integration
spotify · Mar 16, 2015 · be728d8 · be728d8
2 parents 777b0cd + a36cf7e
commit be728d8
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 17 deletions.
diff --git a/luigi/interface.py b/luigi/interface.py
@@ -254,6 +254,9 @@ class ArgParseInterface(Interface):
     """
 
     def parse_task(self, cmdline_args=None, main_task_cls=None):
+        if cmdline_args is None:
+            cmdline_args = sys.argv[1:]
+
         parser = argparse.ArgumentParser()
 
         add_global_parameters(parser)
@@ -269,9 +272,12 @@ def parse_task(self, cmdline_args=None, main_task_cls=None):
             # Parse global arguments and pull out the task name.
             # We used to do this using subparsers+command, but some issues with
             # argparse across different versions of Python (2.7.9) made it hard.
-            args, unknown = parser.parse_known_args(args=cmdline_args)
+            args, unknown = parser.parse_known_args(args=[a for a in cmdline_args if a != '--help'])
             if len(unknown) == 0:
+                # In case it included a --help argument, run again
+                parser.parse_known_args(args=cmdline_args)
                 raise SystemExit('No task specified')
+
             task_name = unknown[0]
             if task_name not in task_names:
                 error_task_names(task_name, task_names)
@@ -319,11 +325,14 @@ class DynamicArgParseInterface(ArgParseInterface):
     """
 
     def parse(self, cmdline_args=None, main_task_cls=None):
+        if cmdline_args is None:
+            cmdline_args = sys.argv[1:]
+
         parser = argparse.ArgumentParser()
 
         add_global_parameters(parser)
 
-        args, unknown = parser.parse_known_args(args=cmdline_args)
+        args, unknown = parser.parse_known_args(args=[a for a in cmdline_args if a != '--help'])
         module = args.module
 
         __import__(module)

diff --git a/luigi/parameter.py b/luigi/parameter.py
@@ -323,7 +323,9 @@ def add_to_cmdline_parser(self, parser, param_name, task_name, optparse=False, g
 
         description = []
         description.append('%s.%s' % (task_name, param_name))
-        if self.description:
+        if glob:
+            description.append('for all instances of class %s' % task_name)
+        elif self.description:
             description.append(self.description)
         if self.has_value:
             description.append(" [default: %s]" % (self.value,))

diff --git a/test/cmdline_test.py b/test/cmdline_test.py
@@ -21,6 +21,7 @@
 except ImportError:
     import configparser as ConfigParser
 import logging
+import mock
 import os
 import subprocess
 from helpers import unittest
@@ -29,7 +30,6 @@
 from luigi import six
 
 import luigi
-import mock
 from luigi.mock import MockTarget
 
 
@@ -168,35 +168,39 @@ def _run_cmdline(self, args):
         env['PYTHONPATH'] = env.get('PYTHONPATH', '') + ':.:test'
         p = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=env)
         stdout, stderr = p.communicate()  # Unfortunately subprocess.check_output is 2.7+
-        self.assertEquals(p.returncode, 0)
-        return stdout, stderr
+        return p.returncode, stdout, stderr
 
     def test_bin_luigi(self):
         t = luigi.LocalTarget(is_tmp=True)
         args = ['./bin/luigi', '--module', 'cmdline_test', 'WriteToFile', '--filename', t.path, '--local-scheduler', '--no-lock']
         self._run_cmdline(args)
         self.assertTrue(t.exists())
 
-    def test_console_script_luigi(self):
-        t = luigi.LocalTarget(is_tmp=True)
-        args = ['luigi', '--module', 'cmdline_test', 'WriteToFile', '--filename', t.path, '--local-scheduler', '--no-lock']
-        self._run_cmdline(args)
-        self.assertTrue(t.exists())
-
     def test_direct_python(self):
         t = luigi.LocalTarget(is_tmp=True)
         args = ['python', 'test/cmdline_test.py', 'WriteToFile', '--filename', t.path, '--local-scheduler', '--no-lock']
         self._run_cmdline(args)
         self.assertTrue(t.exists())
 
+    def test_direct_python_help(self):
+        returncode, stdout, stderr = self._run_cmdline(['python', 'test/cmdline_test.py', '--help'])
+        self.assertTrue(stdout.find(b'--FooBaseClass-x') != -1)
+        self.assertFalse(stdout.find(b'--x') != -1)
+
+    def test_direct_python_help_class(self):
+        returncode, stdout, stderr = self._run_cmdline(['python', 'test/cmdline_test.py', 'FooBaseClass', '--help'])
+        self.assertTrue(stdout.find(b'--FooBaseClass-x') != -1)
+        self.assertTrue(stdout.find(b'--x') != -1)
+
     def test_bin_luigi_help(self):
-        help_str = self._run_cmdline(['python', 'test/cmdline_test.py', 'FooBaseClass', '--help'])
-        # TODO(erikbern): we need to resolve this and fix this test!
-        # self.assertTrue(help_str.index('--x') != -1)
+        returncode, stdout, stderr = self._run_cmdline(['./bin/luigi', '--module', 'cmdline_test', '--help'])
+        self.assertTrue(stdout.find(b'--FooBaseClass-x') != -1)
+        self.assertFalse(stdout.find(b'--x') != -1)
 
-    def test_bin_luigi_new_style_param_help(self):
-        stdout, stderr = self._run_cmdline(['python', 'test/cmdline_test.py', '--help'])
+    def test_bin_luigi_help_class(self):
+        returncode, stdout, stderr = self._run_cmdline(['./bin/luigi', '--module', 'cmdline_test', 'FooBaseClass', '--help'])
         self.assertTrue(stdout.find(b'--FooBaseClass-x') != -1)
+        self.assertTrue(stdout.find(b'--x') != -1)
 
 
 class NewStyleParameters822Test(unittest.TestCase):
@@ -220,4 +224,5 @@ def test_subclasses_2(self):
 
 
 if __name__ == '__main__':
+    # Needed for one of the tests
     luigi.run()