[Piglit] [PATCH 2/2] framework: Provide more helpful error message when loading results fails

[Brian Paul]

On 11/04/2016 01:49 PM, Dylan Baker wrote:
> Currently if there is a typo in a filename (which piglit uses to
> determine if a particular result can be loaded) then a message about
> an unsupported backend will be generated. Obviously, the more common
> reason for this error is a typo or pointing at the wrong directory, not
> that you've tried to load a result format piglit doesn't understand.
>
> With that in mind this patch catches that exception in every case and
> raises a PiglitFatalError, appending a message that this is probably a
> typo or that you've pointed at the wrong directory.
>
> cc: Brian Paul <brianp at vmware.com>
> Signed-off-by: Dylan Baker <dylanx.c.baker at intel.com>
> ---
>   framework/programs/summary.py | 10 +++++++++-
>   framework/summary/console_.py | 10 +++++++++-
>   framework/summary/html_.py    | 19 ++++++++++++++++---
>   3 files changed, 34 insertions(+), 5 deletions(-)
>
> diff --git a/framework/programs/summary.py b/framework/programs/summary.py
> index e400d9a..784e084 100644
> --- a/framework/programs/summary.py
> +++ b/framework/programs/summary.py
> @@ -186,7 +186,15 @@ def csv(input_):
>                           help="JSON results file to be converted")
>       args = parser.parse_args(unparsed)
>
> -    testrun = backends.load(args.testResults)
> +    try:
> +        testrun = backends.load(args.testResults)
> +    except backends.BackendError as e:
> +        raise exceptions.PiglitFatalError(
> +            six.text_type(e) + \
> +            '\n'
> +            'This might mean that you pointed at the wrong directory or a '
> +            'typo in your file extension.')
> +
>
>       def write_results(output):
>           for name, result in six.iteritems(testrun.tests):
> diff --git a/framework/summary/console_.py b/framework/summary/console_.py
> index e17a1d8..b8cdafb 100644
> --- a/framework/summary/console_.py
> +++ b/framework/summary/console_.py
> @@ -30,6 +30,7 @@ import textwrap
>   import six
>
>   from framework import grouptools, backends
> +from framework import exceptions
>   from .common import Results
>
>   __all__ = [
> @@ -100,7 +101,14 @@ def _print_result(results, list_):
>   def console(results, mode):
>       """ Write summary information to the console """
>       assert mode in ['summary', 'diff', 'incomplete', 'all'], mode
> -    results = Results([backends.load(r) for r in results])
> +    try:
> +        results = Results([backends.load(r) for r in results])
> +    except backends.BackendError as e:
> +        raise exceptions.PiglitFatalError(
> +            six.text_type(e) + \
> +            '\n'
> +            'This might mean that you pointed at the wrong directory or a '
> +            'typo in your file extension.')
>
>       # Print the name of the test and the status from each test run
>       if mode == 'all':
> diff --git a/framework/summary/html_.py b/framework/summary/html_.py
> index f7fdc85..4983b62 100644
> --- a/framework/summary/html_.py
> +++ b/framework/summary/html_.py
> @@ -172,7 +172,14 @@ def html(results, destination, exclude):
>       heavy lifting, this method just passes it a bunch of dicts and lists
>       of dicts, which mako turns into pretty HTML.
>       """
> -    results = Results([backends.load(i) for i in results])
> +    try:
> +        results = Results([backends.load(r) for r in results])
> +    except backends.BackendError as e:
> +        raise exceptions.PiglitFatalError(
> +            six.text_type(e) + \
> +            '\n'
> +            'This might mean that you pointed at the wrong directory or a '
> +            'typo in your file extension.')
>
>       _copy_static_files(destination)
>       _make_testrun_info(results, destination, exclude)
> @@ -181,8 +188,14 @@ def html(results, destination, exclude):
>
>   def feat(results, destination, feat_desc):
>       """Produce HTML feature readiness summary."""
> -
> -    feat_res = FeatResults([backends.load(i) for i in results], feat_desc)
> +    try:
> +        feat_res = FeatResults([backends.load(i) for i in results], feat_desc)
> +    except backends.BackendError as e:
> +        raise exceptions.PiglitFatalError(
> +            six.text_type(e) + \
> +            '\n'
> +            'This might mean that you pointed at the wrong directory or a '
> +            'typo in your file extension.')
>
>       _copy_static_files(destination)
>       _make_testrun_info(feat_res, destination)
>

This works, but
1. seems like a lot of duplicated code.
2. How about a simpler error message like: "Unable to open '%s'"

Thanks.

-Brian