[Piglit] [PATCH 5/6] Replaces getopt with argparse

Jordan Justen jljusten at gmail.com
Mon Mar 4 15:51:13 PST 2013


On Mon, Mar 4, 2013 at 10:59 AM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
> The biggest gains I see is that argparse takes less code, its gives a simple
> way to restrict the arguments passed in, and it generates help and usage
> with minimal user input.
>
> 2.7 shipped in 2010, and is the default on the latest OSX, most *nix
> distributions and has been available on windows for about a year. So bumping
> from 2.6 to 2.7 shouldn't be an issue.I have seen nothing that makes me
> think optparse will be removed from python.

I don't depend on python 2.6, but I know we held off from a change
last summer that would've bumped the required version to 2.7.

Personally, I don't see that argparse brings much over optparse. But,
I guess 2 modules for parsing parameters just wasn't enough. :)

-Jordan

> On Mon, Mar 4, 2013 at 10:16 AM, Jordan Justen <jljusten at gmail.com> wrote:
>>
>> On Sat, Mar 2, 2013 at 4:00 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)
>> >
>> > The drawback is that it makes use of argparse, which is only available
>> > in python 2.7+ and 3.2+. However, since python 2.7 was released in
>> > July 2010, that doesn't seem unreasonable.
>>
>> optparse has been around since 2.3 and provides a lot of the same
>> functionality. It is deprecated by argparse, but I don't think actual
>> removal of optparse is planned.
>>
>> Does argparse provide something you need that optparse doesn't?
>>
>> Then again, I don't know if python 2.6 support is important.
>>
>> -Jordan
>>
>> > Signed-off-by: Dylan Baker <baker.dylan.c at gmail.com>
>> > ---
>> >  README                   |   2 +-
>> >  piglit-print-commands.py |  80 +++++++--------------
>> >  piglit-run.py            | 184
>> > +++++++++++++++++++++--------------------------
>> >  3 files changed, 109 insertions(+), 157 deletions(-)
>> >
>> > diff --git a/README b/README
>> > index 68108c9..69f9e9b 100644
>> > --- a/README
>> > +++ b/README
>> > @@ -29,7 +29,7 @@ The original tests have been taken from
>> >
>> >  First of all, you need to make sure that the following are installed:
>> >
>> > -  - Python 2.6 or greater
>> > +  - Python 2.7 or greater
>> >    - numpy (http://www.numpy.org)
>> >    - cmake (http://www.cmake.org)
>> >    - GL, glu and glut libraries and development packages (i.e. headers)
>> > diff --git a/piglit-print-commands.py b/piglit-print-commands.py
>> > index 26177c5..d4a2a52 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,69 +38,39 @@ 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)
>> > -  -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("-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",
>> > -                        "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 ('-x', '--exclude-tests'):
>> > -                       exclude_filter.append(value)
>> > -                       env.exclude_filter.append(re.compile(value))
>> > -
>> > -       if len(args) != 1:
>> > -               usage()
>> > +       env = core.Environment()
>> >
>> > -       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.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 a5f6ed9..57997c5 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,99 +37,75 @@ 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)
>> > -  -x regexp, --exclude-tests=regexp Excludey matching tests (can be
>> > used
>> > -                            more than once)
>> > -  -n name, --name=name      Name of the testrun
>> > -  --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=",
>> > -                        "name=",
>> > -                        "exclude-tests=",
>> > -                        "no-concurrency",
>> > -                        "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 ('-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'):
>> > -                       env.concurrent = False
>> > -               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])
>> > +       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("-x", "--exclude-tests",
>> > +                       default = [],
>> > +                       action  = "append",
>> > +                       metavar = "<regex>",
>> > +                       help    = "Exclude matching tests (can be used
>> > more than once)")
>> > +       parser.add_argument("--no-concurrency",
>> > +                       action  = "store_true",
>> > +                       help    = "Disable concurrent test runs")
>> > +       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 platoform 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
>> > +       if args.no_concurrency is True:
>> > +               env.concurrent = False
>> > +
>> > +       # 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)
>> > @@ -140,14 +116,19 @@ def main():
>> >                 for value in old_results.options['exclude_filter']:
>> >                         exclude_filter.append(value)
>> >                         env.exclude_filter.append(re.compile(value))
>> > +
>> > +       # Otherwise parse additional settings from the command line
>> >         else:
>> > -               if len(args) != 2:
>> > -                       usage()
>> > +               profileFilename = args.testFile
>> > +               resultsDir = args.resultsPath
>> >
>> > -               profileFilename = args[0]
>> > -               resultsDir = path.realpath(args[1])
>> > +               # Set the excluded and included tests regex
>> > +               for each in args.include_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
>> > +       # Change working directory to the root of the piglit directory
>> >         piglit_dir = path.dirname(path.realpath(sys.argv[0]))
>> >         os.chdir(piglit_dir)
>> >
>> > @@ -156,10 +137,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')
>> > @@ -172,9 +153,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)
>> > @@ -188,7 +169,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 not None:
>> >                 for (key, value) in old_results.tests.items():
>> >                         if os.path.sep != '/':
>> >                                 key = key.replace(os.path.sep, '/', -1)
>> > @@ -212,5 +193,6 @@ def main():
>> >         print 'Thank you for running Piglit!'
>> >         print 'Results have been written to ' + result_filepath
>> >
>> > +
>> >  if __name__ == "__main__":
>> >         main()
>> > --
>> > 1.8.1.2
>> >
>> > _______________________________________________
>> > Piglit mailing list
>> > Piglit at lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/piglit
>
>


More information about the Piglit mailing list