Switch to using command line options to specify status. Expand help output, and refactor several test cases.

1 files changed, 63 insertions, 76 deletions

diff --git a/python/subunit/_output.py b/python/subunit/_output.py
index c65fbe0..ae405ef 100644
--- a/python/subunit/_output.py
+++ b/python/subunit/_output.py
@@ -12,7 +12,7 @@
 #  license you chose for the specific language governing permissions and
 #  limitations under that license.
 
-from argparse import ArgumentParser
+from argparse import ArgumentError, ArgumentParser, Action
 import datetime
 from functools import partial
 from sys import stdin, stdout
@@ -37,24 +37,74 @@ def parse_arguments(args=None, ParserClass=ArgumentParser):
 
     ParserClass can be specified to override the class we use to parse the
     command-line arguments. This is useful for testing.
+
     """
 
-    file_args = ParserClass(add_help=False)
-    file_args.add_argument(
+    class StatusAction(Action):
+        """A custom action that stores option name and argument separately.
+
+        This is part of a workaround for the fact that argparse does not
+        support optional subcommands (http://bugs.python.org/issue9253).
+        """
+
+        def __init__(self, status_name, *args, **kwargs):
+            super(StatusAction, self).__init__(*args, **kwargs)
+            self._status_name = status_name
+
+        def __call__(self, parser, namespace, values, option_string=None):
+            if getattr(namespace, self.dest, None) is not None:
+                raise ArgumentError(self, "Only one status may be specified at once.")
+            setattr(namespace, self.dest, self._status_name)
+            setattr(namespace, 'test_id', values[0])
+
+
+    parser = ParserClass(
+        prog='subunit-output',
+        description="A tool to generate a subunit result byte-stream",
+    )
+
+    status_commands = parser.add_argument_group(
+        "Status Commands",
+        "These options report the status of a test. TEST_ID must be a string "
+            "that uniquely identifies the test."
+    )
+    final_actions = 'success fail skip xfail uxsuccess'.split()
+    for action in "inprogress success fail skip exists xfail uxsuccess".split():
+        final_text =  "This is a final state: No more status reports may "\
+            "be generated for this test id after this one."
+
+        status_commands.add_argument(
+            "--%s" % action,
+            nargs=1,
+            action=partial(StatusAction, action),
+            dest="action",
+            metavar="TEST_ID",
+            help="Report a test status." + final_text if action in final_actions else ""
+        )
+
+    file_commands = parser.add_argument_group(
+        "File Options",
+        "These options control attaching data to a result stream. They can "
+            "either be specified with a status command, in which case the file "
+            "is attached to the test status, or by themselves, in which case "
+            "the file is attached to the stream (and not associated with any "
+            "test id)."
+    )
+    file_commands.add_argument(
         "--attach-file",
         help="Attach a file to the result stream for this test. If '-' is "
             "specified, stdin will be read instead. In this case, the file "
             "name will be set to 'stdin' (but can still be overridden with "
             "the --file-name option)."
     )
-    file_args.add_argument(
+    file_commands.add_argument(
         "--file-name",
         help="The name to give this file attachment. If not specified, the "
             "name of the file on disk will be used, or 'stdin' in the case "
             "where '-' was passed to the '--attach-file' argument. This option"
             " may only be specified when '--attach-file' is specified.",
         )
-    file_args.add_argument(
+    file_commands.add_argument(
         "--mimetype",
         help="The mime type to send with this file. This is only used if the "
             "--attach-file argument is used. This argument is optional. If it "
@@ -63,85 +113,19 @@ def parse_arguments(args=None, ParserClass=ArgumentParser):
         default=None
     )
 
-    common_args = ParserClass(add_help=False)
-    common_args.add_argument(
-        "test_id",
-        help="A string that uniquely identifies this test."
-    )
-    common_args.add_argument(
+    parser.add_argument(
         "--tags",
-        help="A comma-separated list of tags to associate with this test.",
+        help="A comma-separated list of tags to associate with a test. This "
+            "option may only be used with a status command.",
         type=lambda s: s.split(','),
         default=None
     )
 
-    parser = ParserClass(
-        prog='subunit-output',
-        description="A tool to generate a subunit result byte-stream",
-        usage="%(prog)s [-h] action [-h] test [--attach-file ATTACH_FILE]"
-            "[--mimetype MIMETYPE] [--tags TAGS]",
-        epilog="Additional help can be printed by passing -h to an action"
-            "(e.g.- '%(prog)s pass -h' will show help for the 'pass' action).",
-        parents=[file_args]
-    )
-    sub_parsers = parser.add_subparsers(
-        dest="action",
-        title="actions",
-        description="These actions are supported by this tool",
-    )
-
-    final_state = "This is a final action: No more actions may be generated "\
-        "for this test id after this one."
-
-    sub_parsers.add_parser(
-        "inprogress",
-        help="Report that a test is in progress.",
-        parents=[common_args, file_args]
-    )
-
-    sub_parsers.add_parser(
-        "success",
-        help="Report that a test has succeeded. " + final_state,
-        parents=[common_args, file_args],
-    )
-
-    sub_parsers.add_parser(
-        "fail",
-        help="Report that a test has failed. " + final_state,
-        parents=[common_args, file_args]
-    )
-
-    sub_parsers.add_parser(
-        "skip",
-        help="Report that a test was skipped. " + final_state,
-        parents=[common_args, file_args]
-    )
-
-    sub_parsers.add_parser(
-        "exists",
-        help="Report that a test exists. " + final_state,
-        parents=[common_args, file_args]
-    )
-
-    sub_parsers.add_parser(
-        "xfail",
-        help="Report that a test has failed expectedly (this is not counted as "
-            "a failure). " + final_state,
-        parents=[common_args, file_args],
-    )
-
-    sub_parsers.add_parser(
-        "uxsuccess",
-        help="Report that a test has succeeded unexpectedly (this is counted "
-            " as a failure). " + final_state,
-        parents=[common_args, file_args],
-    )
-
     args = parser.parse_args(args)
     if args.mimetype and not args.attach_file:
-        parser.error("Cannot specify --mimetype without --attach_file")
+        parser.error("Cannot specify --mimetype without --attach-file")
     if args.file_name and not args.attach_file:
-        parser.error("Cannot specify --file-name without --attach_file")
+        parser.error("Cannot specify --file-name without --attach-file")
     if args.attach_file:
         if args.attach_file == '-':
             if not args.file_name:
@@ -152,6 +136,9 @@ def parse_arguments(args=None, ParserClass=ArgumentParser):
                 args.attach_file = open(args.attach_file)
             except IOError as e:
                 parser.error("Cannot open %s (%s)" % (args.attach_file, e.strerror))
+    if args.tags and not args.action:
+        parser.error("Cannot specify --tags without a status command")
+
     return args