On 11/04/2011 07:01 PM, Don Zickus wrote: > I am new to autotest and ever since I started using it, I found myself > fumbling around trying to find or run tests. It seemed more complicated > than it should be to be productive. > > Now I understand autotest is mainly used for automation, but it can > also be useful to the individual developer who wants to run some tests > local in a controlled environment. > > As a person who enjoys the subcommands found in git and perf, I decided > to create a framework to support subcommands to help me navigate > around autotest. > > This patch is the basic piece that hooks into autotest. Later patches > add commands that I find useful (and I hope others do too :-) ). > > The approach I took was to try and keep the original autotest workflow > and just quickly jump into a function to test for certains commands and > process them. Otherwise, just return and do the normal autotest workflow.
+1 for this, and we'll certainly get there. I have some comments about the code, and as I was thinking about your patchset during the week, I remember that our CLI does have a pretty sophisticated parsing system to do command parsing to make it work just like git, such as cli/atest job list cli/atest job abort cli/atest job help And things like that. Also, cli is kinda of our 'crown jewel' in autotest because it's almost 90% unit tested (my compliments Jean Marc and crew). I think I have delayed looking into your patch because I wanted to look at how we would implement your proposal with the API in there. But what the hell, let's just review your stuff now and think about next steps later... > Signed-off-by: Don Zickus<[email protected]> > --- > client/bin/autotest | 5 +++- > client/bin/cmdparser.py | 54 > +++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 58 insertions(+), 1 deletions(-) > create mode 100644 client/bin/cmdparser.py > > diff --git a/client/bin/autotest b/client/bin/autotest > index 7d36a05..1d19cba 100755 > --- a/client/bin/autotest > +++ b/client/bin/autotest > @@ -7,7 +7,7 @@ import common > from optparse import OptionParser > from autotest_lib.client.bin import job > from autotest_lib.client.common_lib import global_config > - > +from autotest_lib.client.bin import cmdparser > > # Use the name of the binary to find the real installation directory > # aka $AUTODIR. Update our path to include the $AUTODIR/bin/tests > @@ -74,6 +74,9 @@ def usage(): > > options, args = parser.parse_args() > > +cmd_parser = cmdparser.CommandParser() > +args = cmd_parser.parse_args(args) > + > # Check for a control file if not in prebuild mode. > if len(args) != 1 and options.client_test_setup is None: > print "Missing control file!" > diff --git a/client/bin/cmdparser.py b/client/bin/cmdparser.py > new file mode 100644 > index 0000000..f2683f0 > --- /dev/null > +++ b/client/bin/cmdparser.py > @@ -0,0 +1,54 @@ > +"""A command parser utility > + > + Just some wrapper commands around autotest client > + > + Copyright Don Zickus<[email protected]> 2011 > +""" > + > +import copy, os, platform, re, shutil, sys, time, traceback, types, glob > +import logging, getpass, errno, weakref > + > +class CommandParser(): > + """A client-side command wrapper for newbies > + > + """ > + > + cmdlist = ['help'] > + > + def __init__(self): > + return ^ Here you could just omit the constructor. Maybe a better idea is to initialize the class with args, assign args as a class attribute and then omitting the argument from parse_args. > + def parse_args(self, args): > + """ > + Process a client side command > + > + @param args: command line args > + """ > + > + if len(args) and args[0] in self.cmdlist: ^ Here you just need to test for args, since an empty list evaluates to false. > + cmd = args.pop(0) > + else: > + #do things the traditional way > + return args > + > + try: > + args = getattr(self, cmd)(args) > + except SystemExit as e: > + sys.exit(e) > + except: > + sys.stderr.write("Failed: command %s\n" % cmd) > + sys.exit(1) ^ Here I'm not sure if getattr can ever raise SystemExit, I can only see AttributeError and possibly what returns from getattr not being a callable. About logging messages, we use the python logging module, so rather than writing to stderr we call logging.error logging.error('Something went bad, but we don't want to throw an exception for it') I am OK with just calling sys.exit with a rc != 0, it's more unix-y. But I'd consider creating an exception class for it and raising, when a py program ends with exception, the rc will be != 0 anyway. > + # args are cleaned up, return to process the traditional way > + return args > + > + def help(self, args): > + """ > + List the commands and their usage strings > + > + @param args is not used here > + """ > + > + print "help\t\tOutput a list of supported commands" > + raise SystemExit(0) ^ Here we could print the help msg with logging.info _______________________________________________ Autotest mailing list [email protected] http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
