You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Madan U Sreenivasan <ma...@collab.net> on 2006/05/24 09:08:09 UTC

[PATCH] raise Custom Exception on invalid test argument

[[[
Spew out a customized Exception when the test is provided with an
invalid argument.

* subversion/tests/cmdline/svntest/main.py
   (run_tests): Catch ValueError Exception and throw back
    a customised Exception that makes more sense to the user.
]]]

Re: [PATCH] raise Custom Exception on invalid test argument

Posted by "C. Michael Pilato" <cm...@collab.net>.
Madan U Sreenivasan wrote:
> 
> [[[
> Spew out a customized Exception when the test is provided with an
> invalid argument.
> 
> * subversion/tests/cmdline/svntest/main.py
>   (run_tests): Catch ValueError Exception and throw back
>    a customised Exception that makes more sense to the user.
> ]]]
> 
> 
> ------------------------------------------------------------------------
> 
> Index: svntest/main.py
> ===================================================================
> --- svntest/main.py	(revision 19800)
> +++ svntest/main.py	(working copy)
> @@ -808,7 +808,13 @@
>      elif arg.startswith('BASE_URL='):
>        test_area_url = arg[9:]
>      else:
> -      testnums.append(int(arg))
> +      try:
> +        testnums.append(int(arg))
> +      except ValueError:
> +        err_str  = "Invalid argument :%s.\n" % arg
> +        err_str += "Valid arguments are: "
> +        err_str += "`list' and/or a valid test number."
> +        raise Exception(err_str)

These last four lines can be just one statement, you know.  Suggest:

   raise Exception("""Invalid argument: %s
Valid arguments are 'list' and/or a valid test number.""" % (arg))

Or, if the indentation bothers you:

   raise Exception("Invalid argument: %s\n" % (arg) + \
                   "Valid arguments are 'list' and/or a " + \
                   "valid test number.")

But beyond that, the valid arguments *aren't* just 'list' and a number,
because the very conditional you're modifying also allows for arguments that
begin with 'BASE_URL='.  If the goal is to list all the arguments we accept,
you'll need to mention that one, too.  If, however, the goal is to list the
argument we *want* to accept, then don't mention 'list' (because we'd prefer
folks to use the '--list' option instead).

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand