[lit] Leverage argparse features to remove some code

[rnk]

lgtm

[serge-sans-paille]

llvm/utils/lit/lit/cl_arguments.py

204

It's generally better to catch the conversion error explicitly (here `ValueError`)

211

require*s*

[yln]

llvm/utils/lit/lit/cl_arguments.py

204

Done. Thanks!

211

Updated.

[yln]

Address comments.

llvm/utils/lit/lit/cl_arguments.py

 import argparse
 import os
 import shlex
 import sys
 
 import lit.util
 
 def parse_args():
     parser = argparse.ArgumentParser()
     parser.add_argument('test_paths',
-            nargs='*',
+            nargs='+',
             help='Files or paths to include in the test suite')
 
     parser.add_argument("--version",
             dest="show_version",
             help="Show version and exit",
             action="store_true",
             default=False)
     parser.add_argument("-j", "--workers",
             dest="numWorkers",
             metavar="N",
             help="Number of workers used for testing",
-            type=int,
-            default=None)
+            type=_positive_int,
+            default=lit.util.detectCPUs())
     parser.add_argument("--config-prefix",
             dest="configPrefix",
             metavar="NAME",
             help="Prefix for 'lit' config files",
-            action="store",
             default=None)
     parser.add_argument("-D", "--param",
             dest="userParameters",
             metavar="NAME=VAL",
             help="Add 'NAME' = 'VAL' to the user defined parameters",
             type=str,
             action="append",
             default=[])
def parse_args():
     format_group.add_argument("-a", "--show-all",
             dest="showAllOutput",
             help="Display all commandlines and output",
             action="store_true",
             default=False)
     format_group.add_argument("-o", "--output",
             dest="output_path",
             help="Write test results to the provided path",
-            action="store",
             metavar="PATH")
     format_group.add_argument("--no-progress-bar",
             dest="useProgressBar",
             help="Do not use curses based progress bar",
             action="store_false",
             default=True)
     format_group.add_argument("--show-unsupported",
             help="Show unsupported tests",
execution_group.add_argument("--timeout",
             dest="maxIndividualTestTime",
             help="Maximum time to spend running a single test (in seconds). "
                  "0 means no time limit. [Default: 0]",
             type=int,
             default=None)
     execution_group.add_argument("--max-failures",
             dest="maxFailures",
             help="Stop execution after the given number of failures.",
-            action="store",
-            type=int,
+            type=_positive_int,
             default=None)
 
     selection_group = parser.add_argument_group("Test Selection")
     selection_group.add_argument("--max-tests",
             dest="maxTests",
             metavar="N",
             help="Maximum number of tests to run",
-            action="store",
             type=int,
             default=None)
     selection_group.add_argument("--max-time",
             dest="maxTime",
             metavar="N",
             help="Maximum time to spend testing (in seconds)",
-            action="store",
             type=float,
             default=None)
     selection_group.add_argument("--shuffle",
             help="Run tests in random order",
             action="store_true",
             default=False)
     selection_group.add_argument("-i", "--incremental",
             help="Run modified and failing tests first (updates mtimes)",
             action="store_true",
             default=False)
     selection_group.add_argument("--filter",
             metavar="REGEX",
             help="Only run tests with paths matching the given regular expression",
-            action="store",
             default=os.environ.get("LIT_FILTER"))
-    selection_group.add_argument("--num-shards", dest="numShards", metavar="M",
+    selection_group.add_argument("--num-shards",
+            dest="numShards",
+            metavar="M",
             help="Split testsuite into M pieces and only run one",
-            action="store",
-            type=int,
+            type=_positive_int,
             default=os.environ.get("LIT_NUM_SHARDS"))
     selection_group.add_argument("--run-shard",
             dest="runShard",
             metavar="N",
             help="Run shard #N of the testsuite",
-            action="store",
-            type=int,
+            type=_positive_int,
             default=os.environ.get("LIT_RUN_SHARD"))
 
     debug_group = parser.add_argument_group("Debug and Experimental Options")
     debug_group.add_argument("--debug",
             help="Enable debugging (for 'lit' development)",
             action="store_true",
             default=False)
     debug_group.add_argument("--show-suites",
             dest="showSuites",
             help="Show discovered test suites",
             action="store_true",
             default=False)
     debug_group.add_argument("--show-tests",
             dest="showTests",
             help="Show all discovered tests",
             action="store_true",
             default=False)
 
     opts = parser.parse_args(sys.argv[1:] +
                              shlex.split(os.environ.get("LIT_OPTS", "")))
 
-    # Validate options
-    if not opts.test_paths:
-        parser.error('No inputs specified')
-
-    if opts.numWorkers is None:
-        opts.numWorkers = lit.util.detectCPUs()
-    elif opts.numWorkers <= 0:
-        parser.error("Option '--workers' or '-j' requires positive integer")
-
-    if opts.maxFailures is not None and opts.maxFailures <= 0:
-        parser.error("Option '--max-failures' requires positive integer")
-
+    # Validate command line options
     if opts.echoAllCommands:
         opts.showOutput = True
 
-    if (opts.numShards is not None) or (opts.runShard is not None):
-        if (opts.numShards is None) or (opts.runShard is None):
+    if opts.numShards or opts.runShard:
+        if not opts.numShards or not opts.runShard:
             parser.error("--num-shards and --run-shard must be used together")
-        if opts.numShards <= 0:
-            parser.error("--num-shards must be positive")
-        if (opts.runShard < 1) or (opts.runShard > opts.numShards):
+        if opts.runShard > opts.numShards:
             parser.error("--run-shard must be between 1 and --num-shards (inclusive)")
 
     return opts
+
+def _positive_int(arg):
+    try:
+        n = int(arg)
+    except ValueError:
+        raise _arg_error('positive integer', arg)
+    if n <= 0:
+        raise _arg_error('positive integer', arg)
+    return n
+
+def _arg_error(desc, arg):
+    msg = "requires %s, but found '%s'" % (desc, arg)
+    return argparse.ArgumentTypeError(msg)

llvm/utils/lit/tests/max-failures.py

 # Check the behavior of --max-failures option.
 #
 # RUN: not %{lit} -j 1 -v %{inputs}/max-failures > %t.out
 # RUN: not %{lit} --max-failures=1 -j 1 -v %{inputs}/max-failures >> %t.out
 # RUN: not %{lit} --max-failures=2 -j 1 -v %{inputs}/max-failures >> %t.out
 # RUN: not %{lit} --max-failures=0 -j 1 -v %{inputs}/max-failures 2>> %t.out
 # RUN: FileCheck < %t.out %s
 #
 # END.
 
 # CHECK: Failing Tests (31)
 # CHECK: Failing Tests (1)
 # CHECK: Failing Tests (2)
-# CHECK: error: Option '--max-failures' requires positive integer
+# CHECK: error: argument --max-failures: requires positive integer, but found '0'

llvm/utils/lit/tests/selecting.py

 # CHECK-SHARD-BIG-ERR3: note: Selecting shard 50/100 = size 0/5 = tests #(100*k)+50 = []
 # CHECK-SHARD-BIG-OUT3: Testing: 0 of 5 tests
 
 
 # Check that range constraints are enforced
 #
 # RUN: not %{lit} --num-shards 0 --run-shard 2 %{inputs}/discovery >%t.out 2>%t.err
 # RUN: FileCheck --check-prefix=CHECK-SHARD-ERR < %t.err %s
-# CHECK-SHARD-ERR: error: --num-shards must be positive
+# CHECK-SHARD-ERR: error: argument --num-shards: requires positive integer, but found '0'
 #
 # RUN: not %{lit} --num-shards 3 --run-shard 4 %{inputs}/discovery >%t.out 2>%t.err
 # RUN: FileCheck --check-prefix=CHECK-SHARD-ERR2 < %t.err %s
 # CHECK-SHARD-ERR2: error: --run-shard must be between 1 and --num-shards (inclusive)