718106 - run_tests.py should display help if no arguments are given

Status: RESOLVED FIXED

(Testing :: Talos, defect)

Description

[Jeff Hammel]

currently running `python run_tests.py` displays no output and exits
0.  Instead, it should run `parser.print_help()` and also probably
exit 0.

Comment 1

[Burak Yiğit Kaya [:BYK]]

I would love to take on this with argparse module though I think we need to wait for it a little longer.

Comment 2

[Jeff Hammel]

argparse would be great to have, though i wouldn't want to block anything related to talos command line handling on it.  Even using optparse, we have a ton of things that should be made better (example: the handling of --activeTests via colon separators)

Comment 3

[Burak Yiğit Kaya [:BYK]]

Attachment: Patch for printing help string if no arguments are given at all

I put the check *before* parsing the arguments since the help text only mentions [options] and printing the help text when some options provided but no "actual" arguments are given does not make much sense in IMO.

Comment 4

[Jeff Hammel]

Comment on attachment 589129 [details] [diff] [review]
Patch for printing help string if no arguments are given at all

We should put this after the `parser.parse_args` call since, as is, you can still get in the situation of `python run_tests.py -d` which is just as confusing (and which a user evaluating program usage from the CLI would be lead to do after `python run_tests.py` with no arguments.  Instead, we should just make the usage more helpful; e.g. http://hg.mozilla.org/build/talos/file/d93f2b21985c/talos/run_tests.py#l581 passing in usage="%prog [options] manifest.yml <manifest.yml> <...>"

Comment 5

[Burak Yiğit Kaya [:BYK]]

Attachment: Print help if no actual arguments are provided

I made the changes according to the review. Changed the proposed usage string a little bit to be compliant with the argparse standard generated output(see examples in http://docs.python.org/library/argparse.html)

Comment 6

[Jeff Hammel]

Comment on attachment 589287 [details] [diff] [review]
Print help if no actual arguments are provided

nice :) thanks!

Comment 7

[Jeff Hammel]

Was going to push to try, but the patch seems to have bitrotted

Comment 8

[Jeff Hammel]

Attachment: unbitrot

Comment 9

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 595513 [details] [diff] [review]
unbitrot

Review of attachment 595513 [details] [diff] [review]:
-----------------------------------------------------------------

::: talos/run_tests.py
@@ +574,5 @@
>  
>  def main(args=sys.argv[1:]):
>  
>    # parse command line options
> +  usage = "%prog [options] manifest.yml [manifest.yml ...]"

hmm, do we use this string?  I assume the parser.print_help() does it automatically, just thought we had to define it in a class or assign it some how.

Comment 10

[Joel Maher ( :jmaher ) (UTC -8)]

this is a check outside of the config generation and runtime, landed:
http://hg.mozilla.org/build/talos/rev/2c84941f35ed

Comment 11

[Jeff Hammel]

(In reply to Joel Maher (:jmaher) from comment #9)
> Comment on attachment 595513 [details] [diff] [review]
> unbitrot
> 
> Review of attachment 595513 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: talos/run_tests.py
> @@ +574,5 @@
> >  
> >  def main(args=sys.argv[1:]):
> >  
> >    # parse command line options
> > +  usage = "%prog [options] manifest.yml [manifest.yml ...]"
> 
> hmm, do we use this string?  I assume the parser.print_help() does it
> automatically, just thought we had to define it in a class or assign it some
> how.

Beh, was supposed to pass in as

parser = PerfConfigurator.TalosOptions(usage=usage)

It would be a good follow-up, though even as is its infinitely better than it was before.  Thanks, BYK, and sorry for the long turnaround.