[Piglit] [PATCH v2 6/7] Replaces getopt with argparse
Dylan Baker
baker.dylan.c at gmail.com
Fri Mar 8 11:41:19 PST 2013
Oops, I looked at that and I broke something in V2. I have V3 patches that
address that issue I'm sending now.
On Fri, Mar 8, 2013 at 9:06 AM, Jordan Justen <jljusten at gmail.com> wrote:
> On Tue, Mar 5, 2013 at 4:47 PM, Dylan Baker <baker.dylan.c at gmail.com>
> wrote:
> > This change gets us cleaner, more readable argument parsing code.
> > Another advantage of this approach is that it automatically generates
> > help menus, which means options will not be implemented without a help
> > entry (like --resume)
> >
> > V2: - Does not remove deprecated options, only marks them as deprecated
> >
> > Signed-off-by: Dylan Baker <baker.dylan.c at gmail.com>
> > ---
> > piglit-print-commands.py | 98 +++++++------------
> > piglit-run.py | 249
> +++++++++++++++++++++++------------------------
> > 2 files changed, 159 insertions(+), 188 deletions(-)
> >
> > diff --git a/piglit-print-commands.py b/piglit-print-commands.py
> > index 3479ef1..9dd2290 100755
> > --- a/piglit-print-commands.py
> > +++ b/piglit-print-commands.py
> > @@ -22,7 +22,7 @@
> > # DEALINGS IN THE SOFTWARE.
> >
> >
> > -from getopt import getopt, GetoptError
> > +import argparse
> > import os.path as path
> > import re
> > import sys, os
> > @@ -38,77 +38,51 @@ from framework.gleantest import GleanTest
> >
> #############################################################################
> > ##### Main program
> >
> #############################################################################
> > -def usage():
> > - USAGE = """\
> > -Usage: %(progName)s [options] [profile.tests]
> > -
> > -Prints a list of all the tests and how to run them. Ex:
> > - piglit test name ::: /path/to/piglit/bin/program <args>
> > - glean test name ::: PIGLIT_TEST='...' /path/to/piglit/bin/glean -v
> -v -v ...
> > -
> > -Options:
> > - -h, --help Show this message
> > - -t regexp, --include-tests=regexp Run only matching tests (can be
> used more
> > - than once)
> > - --tests=regexp Run only matching tests (can be used more
> > - than once) DEPRICATED use --include-tests
> instead
> > - -x regexp, --exclude-tests=regexp Exclude matching tests (can be used
> > - more than once)
> > -Example:
> > - %(progName)s tests/all.tests
> > - %(progName)s -t basic tests/all.tests
> > - Print tests whose path contains the word 'basic'
> > -
> > - %(progName)s -t ^glean/ -t tex tests/all.tests
> > - Print tests that are in the 'glean' group or whose path
> contains
> > - the substring 'tex'
> > -"""
> > - print USAGE % {'progName': sys.argv[0]}
> > - sys.exit(1)
> >
> > def main():
> > - env = core.Environment()
> > + parser = argparse.ArgumentParser(sys.argv)
> > +
> > + parser.add_argument("-t", "--include-tests",
> > + default = [],
> > + action = "append",
> > + metavar = "<regex>",
> > + help = "Run only matching tests (can be used
> more than once)")
> > + parser.add_argument("--tests",
> > + default = [],
> > + action = "append",
> > + metavar = "<regex>",
> > + help = "Run only matching tests (can be used
> more than once)" \
> > + "Deprecated")
> > + parser.add_argument("-x", "--exclude-tests",
> > + default = [],
> > + action = "append",
> > + metavar = "<regex>",
> > + help = "Exclude matching tests (can be used
> more than once)")
> > + parser.add_argument("testProfile",
> > + metavar = "<Path to testfile>",
> > + help = "Path to results folder")
> > +
> > + args = parser.parse_args()
> >
> > - try:
> > - option_list = [
> > - "help",
> > - "tests=",
> > - "include-tests=",
> > - "exclude-tests=",
> > - ]
> > - options, args = getopt(sys.argv[1:], "ht:x:",
> option_list)
> > - except GetoptError:
> > - usage()
> > -
> > - OptionName = ''
> > - test_filter = []
> > - exclude_filter = []
> > -
> > - for name, value in options:
> > - if name in ('-h', '--help'):
> > - usage()
> > - elif name in ('-t', '--include-tests'):
> > - test_filter.append(value)
> > - env.filter.append(re.compile(value))
> > - elif name in ('--tests'):
> > - test_filter.append(value)
> > - env.filter.append(re.compile(value))
> > - print "Warning: Option --tets is deprecated, " \
> > - "use --include-tests instead"
> > - elif name in ('-x', '--exclude-tests'):
> > - exclude_filter.append(value)
> > - env.exclude_filter.append(re.compile(value))
> > + env = core.Environment()
> >
> > - if len(args) != 1:
> > - usage()
> > + # If --tests is called warn that it is deprecated
> > + if args.tests is no []:
> > + print "Warning: Option --tests is deprecated. Use
> --include-tests"
> >
> > - profileFilename = args[0]
> > + # Append includes and excludes to env
> > + for each in args.include_tests:
> > + env.filter.append(re.compile(each))
> > + for each in args.tests:
> > + env.filter.append(re.compile(each))
> > + for each in args.exclude_tests:
> > + env.exclude_filter.append(re.compile(each))
> >
> > # Change to the piglit's path
> > piglit_dir = path.dirname(path.realpath(sys.argv[0]))
> > os.chdir(piglit_dir)
> >
> > - profile = core.loadTestProfile(profileFilename)
> > + profile = core.loadTestProfile(args.testFile)
> >
> > def getCommand(test):
> > command = ''
> > diff --git a/piglit-run.py b/piglit-run.py
> > index 599981d..1a50651 100755
> > --- a/piglit-run.py
> > +++ b/piglit-run.py
> > @@ -22,7 +22,7 @@
> > # DEALINGS IN THE SOFTWARE.
> >
> >
> > -from getopt import getopt, GetoptError
> > +import argparse
> > import os.path as path
> > import re
> > import sys, os
> > @@ -37,123 +37,112 @@ from framework.threads import synchronized_self
> >
> #############################################################################
> > ##### Main program
> >
> #############################################################################
> > -def usage():
> > - USAGE = """\
> > -Usage: %(progName)s [options] [profile.tests] [results]
> > - %(progName)s [options] -r [results]
> > -
> > -Options:
> > - -h, --help Show this message
> > - -d, --dry-run Do not execute the tests
> > - -t regexp, --include-tests=regexp Run only matching tests (can be
> used more
> > - than once)
> > - --tests=regexp Run only matching tests (can be used more
> > - than once)
> DEPRICATED: use --include-tests instead
> > - -x regexp, --exclude-tests=regexp Excludey matching tests (can be used
> > - more than once)
> > - -n name, --name=name Name of the testrun
> > - -c bool, --concurrent= Enable/disable concurrent test runs. Valid
> > - option values are: 0, 1, on, off.
> (default: on)
> > - DEPRICATED: use
> --no-concurrency to turn
> > - concurrent test
> runs off
> > - --no-concurrency Disables concurrent test runs
> > - --valgrind Run tests in valgrind's memcheck.
> > - -p platform, --platform=platform Name of the piglit platform to use.
> > - --resume Resume and interupted test run
> > -
> > -Example:
> > - %(progName)s tests/all.tests results/all
> > - Run all tests, store the results in the directory results/all
> > -
> > - %(progName)s -t basic tests/all.tests results/all
> > - Run all tests whose path contains the word 'basic'
> > -
> > - %(progName)s -t ^glean/ -t tex tests/all.tests results/all
> > - Run all tests that are in the 'glean' group or whose path
> contains
> > - the substring 'tex'
> > -
> > - %(progName)s -r -x bad-test results/all
> > - Resume an interrupted test run whose results are stored in the
> > - directory results/all, skipping bad-test.
> > -"""
> > - print USAGE % {'progName': sys.argv[0]}
> > - sys.exit(1)
> >
> > def main():
> > env = core.Environment()
> >
> > - try:
> > - option_list = [
> > - "help",
> > - "dry-run",
> > - "resume",
> > - "valgrind",
> > - "include-tests=",
> > - "tests=",
> > - "name=",
> > - "exclude-tests=",
> > - "no-concurrency",
> > - "concurrent=",
> > - "platform=",
> > - ]
> > - options, args = getopt(sys.argv[1:], "hdrt:n:x:c:p:",
> option_list)
> > - except GetoptError:
> > - usage()
> > -
> > - OptionName = ''
> > - OptionResume = False
> > - test_filter = []
> > - exclude_filter = []
> > - platform = None
> > -
> > - for name, value in options:
> > - if name in ('-h', '--help'):
> > - usage()
> > - elif name in ('-d', '--dry-run'):
> > - env.execute = False
> > - elif name in ('-r', '--resume'):
> > - OptionResume = True
> > - elif name in ('--valgrind'):
> > - env.valgrind = True
> > - elif name in ('-t', '--include-tests'):
> > - test_filter.append(value)
> > - env.filter.append(re.compile(value))
> > - elif name in ('--tests'):
> > - test_filter.append(value)
> > - env.filter.append(re.compile(value))
> > - print "Warning: Option --tests is deprecated, " \
> > - "use --include-tests instead"
> > - elif name in ('-x', '--exclude-tests'):
> > - exclude_filter.append(value)
> > - env.exclude_filter.append(re.compile(value))
> > - elif name in ('-n', '--name'):
> > - OptionName = value
> > - elif name in ('--no-concurrency'):
> > + parser = argparse.ArgumentParser(sys.argv)
> > +
> > +
> > + # Either require that a name for the test is passed or that
> > + # resume is requested
> > + excGroup1 = parser.add_mutually_exclusive_group()
> > + excGroup1.add_argument("-n", "--name",
> > + metavar = "<test name>",
> > + default = None,
> > + help = "Name of this test run")
> > + excGroup1.add_argument("-r", "--resume",
> > + action = "store_true",
> > + help = "Resume an interupted test run")
> > +
> > + parser.add_argument("-d", "--dry-run",
> > + action = "store_true",
> > + help = "Do not execute the tests")
> > + parser.add_argument("-t", "--include-tests",
> > + default = [],
> > + action = "append",
> > + metavar = "<regex>",
> > + help = "Run only matching tests (can be used
> more than once)")
> > + parser.add_argument("--tests",
> > + default = [],
> > + action = "append",
> > + metavar = "<regex>",
> > + help = "Run only matching tests (can be used
> more than once) " \
> > + "DEPRECATED: use --include-tests
> instead")
> > + parser.add_argument("-x", "--exclude-tests",
> > + default = [],
> > + action = "append",
> > + metavar = "<regex>",
> > + help = "Exclude matching tests (can be used
> more than once)")
> > +
> > + # The new option going forward should be --no-concurrency, but to
> > + # maintain backwards compatability the --c, --concurrent option
> should
> > + # also be maintained. This code allows only one of the two
> options to be
> > + # supplied, or it throws an error
> > + excGroup2 = parser.add_mutually_exclusive_group()
> > + excGroup2.add_argument("--no-concurrency",
> > + action = "store_true",
> > + help = "Disable concurrent test runs")
> > + excGroup2.add_argument("-c", "--concurrent",
> > + action = "store",
> > + metavar = "<boolean>",
> > + choices = ["1", "0", "on", "off"],
> > + help = "Deprecated: Turn concrrent runs on or
> off")
> > +
> > + parser.add_argument("-p", "--platform",
> > + choices = ["glx", "x11_egl", "wayland", "gbm"],
> > + help = "Name of windows system passed to
> waffle")
> > + parser.add_argument("--valgrind",
> > + action = "store_true",
> > + help = "Run tests in valgrind's memcheck")
> > + parser.add_argument("testProfile",
> > + metavar = "<Path to test profile>",
> > + help = "Path to testfile to run")
> > + parser.add_argument("resultsPath",
> > + metavar = "<Results Path>",
> > + help = "Path to results folder")
> > +
> > + args = parser.parse_args()
> > +
> > +
> > + # Set the platform to pass to waffle
> > + if args.platform is not None:
> > + os.environ['PIGLIT_PLATFORM'] = args.platform
> > +
> > + # Set dry-run
> > + if args.dry_run is True:
> > + env.execute = False
> > +
> > + # Set valgrind
> > + if args.valgrind is True:
> > + env.valgrind = True
> > +
> > + # Turn concurency off if requested
> > + # Deprecated
> > + if args.concurrent is not None:
> > + if (args.concurrent == '1' or args.concurrent == 'on'):
> > + env.concurrent = True
> > + print "Warning: Option -c, --concurrent is
> deprecated, " \
> > + "concurrent test runs are on by
> default"
> > + elif (args.concurrent == '0' or args.concurrent ==
> 'off'):
> > env.concurrent = False
> > - elif name in ('-c', '--concurrent'):
> > - if value in ('1', 'on'):
> > - env.concurrent = True
> > - print "Warning: Option -c, --concurrent
> is deprecated, " \
> > - "concurrent test runs
> are on by default"
> > - elif value in ('0', 'off'):
> > - env.concurrent = False
> > - print "Warning: Option -c, --concurrent
> is deprecated, " \
> > - "use --no-concurrency
> for non-concurrent test runs"
> > - else:
> > - usage()
> > - elif name in ('-p, --platform'):
> > - platform = value
> > -
> > - if platform is not None:
> > - os.environ['PIGLIT_PLATFORM'] = platform
> > -
> > - if OptionResume:
> > - if test_filter or OptionName:
> > - print "-r is not compatible with -t or -n."
> > - usage()
> > - if len(args) != 1:
> > - usage()
> > - resultsDir = path.realpath(args[0])
> > + print "Warning: Option -c, --concurrent is
> deprecated, " \
> > + "use --no-concurrent for
> non-concurrent test runs"
> > + # Ne need for else, since argparse restricts the
> arguments allowed
> > +
> > + # Not deprecated
> > + elif args.no_concurrency is True:
> > + env.concurrent = False
> > +
> > + # If the deprecated tests option was passed print a warning
> > + if args.tests is not []:
> > + print "Warning: Option --tests is deprecated, use " \
> > + "--include-tests instead"
>
> I got the warning message when using -t. I think we may want 'if
> len(args.tests) > 0' instead.
>
> Series Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>
>
> I'll commit the series with this change later today if no further
> comments come in.
>
> Thanks,
>
> -Jordan
>
> > +
> > + # If resume is requested attempt to load the results file
> > + # in the specified path
> > + if args.resume is True:
> > + resultsDir = path.realpath(args.resultsPath)
> >
> > # Load settings from the old results JSON
> > old_results = core.loadTestResults(resultsDir)
> > @@ -164,14 +153,21 @@ def main():
> > for value in old_results.options['exclude_filter']:
> > exclude_filter.append(value)
> > env.exclude_filter.append(re.compile(value))
> > - else:
> > - if len(args) != 2:
> > - usage()
> >
> > - profileFilename = args[0]
> > - resultsDir = path.realpath(args[1])
> > -
> > - # Change to the piglit's path
> > + # Otherwise parse additional settings from the command line
> > + else:
> > + profileFilename = args.testProfile
> > + resultsDir = args.resultsPath
> > +
> > + # Set the excluded and included tests regex
> > + for each in args.include_tests:
> > + env.filter.append(re.compile(each))
> > + for each in args.tests:
> > + env.filter.append(re.compile(each))
> > + for each in args.exclude_tests:
> > + env.exclude_filter.append(re.compile(each))
> > +
> > + # Change working directory to the root of the piglit directory
> > piglit_dir = path.dirname(path.realpath(sys.argv[0]))
> > os.chdir(piglit_dir)
> >
> > @@ -180,10 +176,10 @@ def main():
> > results = core.TestrunResult()
> >
> > # Set results.name
> > - if OptionName is '':
> > - results.name = path.basename(resultsDir)
> > + if args.name is not None:
> > + results.name = args.name
> > else:
> > - results.name = OptionName
> > + results.name = path.basename(resultsDir)
> >
> > # Begin json.
> > result_filepath = os.path.join(resultsDir, 'main')
> > @@ -196,9 +192,9 @@ def main():
> > json_writer.open_dict()
> > json_writer.write_dict_item('profile', profileFilename)
> > json_writer.write_dict_key('filter')
> > - result_file.write(json.dumps(test_filter))
> > + result_file.write(json.dumps(args.include_tests))
> > json_writer.write_dict_key('exclude_filter')
> > - result_file.write(json.dumps(exclude_filter))
> > + result_file.write(json.dumps(args.exclude_tests))
> > json_writer.close_dict()
> >
> > json_writer.write_dict_item('name', results.name)
> > @@ -212,7 +208,7 @@ def main():
> > # If resuming an interrupted test run, re-write all of the
> existing
> > # results since we clobbered the results file. Also, exclude
> them
> > # from being run again.
> > - if OptionResume:
> > + if args.resume is True:
> > for (key, value) in old_results.tests.items():
> > if os.path.sep != '/':
> > key = key.replace(os.path.sep, '/', -1)
> > @@ -236,5 +232,6 @@ def main():
> > print 'Thank you for running Piglit!'
> > print 'Results have been written to ' + result_filepath
> >
> > +
> > if __name__ == "__main__":
> > main()
> > --
> > 1.8.1.4
> >
> > _______________________________________________
> > Piglit mailing list
> > Piglit at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/piglit
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20130308/adeb0333/attachment-0001.html>
More information about the Piglit
mailing list