[Piglit] [PATCH] framework: Do not run with an empty test list

[Dylan Baker]

Quoting Martin Peres (2017-10-19 07:17:25)
> On 30/09/17 23:42, Dylan Baker wrote:
> > Actually CC'ing him this time....
> > 
> > Quoting Dylan Baker (2017-09-29 20:29:34)
> >> Quoting Arkadiusz Hiler (2017-09-26 03:27:50)
> >>> Because in Python we have `bool([]}) == False`, providing empty test
> >>> list resulted in hitting the same code path as not providing it at all,
> >>> meaning that we run everything.
> >>>
> >>> Let's just exit early with an appropriate message instead.
> >>>
> >>> This will get rid of the rather surprising behavior and will help making
> >>> the execution less prone to automated list generation errors (which has
> >>> already bitten us) as well as human errors.
> >>>
> >>> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler at intel.com>
> >>> ---
> >>>   framework/programs/run.py | 4 ++++
> >>>   1 file changed, 4 insertions(+)
> >>>
> >>> diff --git a/framework/programs/run.py b/framework/programs/run.py
> >>> index 4524f171b..0fec264ec 100644
> >>> --- a/framework/programs/run.py
> >>> +++ b/framework/programs/run.py
> >>> @@ -327,6 +327,10 @@ def run(input_):
> >>>               stripped = (t.split('#')[0].strip() for t in test_list)
> >>>               forced_test_list = [t for t in stripped if t]
> >>>   
> >>> +            # to avoid running everything
> >>> +            if not forced_test_list:
> >>> +                raise exceptions.PiglitFatalError("Empty test list provided")
> >>> +
> >>>       backend = backends.get_backend(args.backend)(
> >>>           args.results_path,
> >>>           junit_suffix=args.junit_suffix,
> >>> -- 
> >>> 2.13.5
> >>>
> >>> _______________________________________________
> >>> Piglit mailing list
> >>> Piglit at lists.freedesktop.org
> >>> https://lists.freedesktop.org/mailman/listinfo/piglit
> >>
> >> Hmmm, there is a case that we do want to continue, and that's for resume, CC'ing
> >> Martin to see if this breaks their use case.
> 
> Sorry for the delay! Thanks for caring about CI :)
> 
> So, I like the patch, but disagree on the "raise" here. Why not simply 
> print("Empty test list provided") and sys.exit(0)? There is nothing to 
> do after all, so why report that an error happened?
> 
> If you still want to return an error, then please make sure it is one I 
> can test against.
> 
> Thanks,
> Martin

PiglitFatalError is actually caught by the main script, the error is printed
with 'Fatal Error: ' prepended and sys.exit(1) is called. So if that's what you
want than this is find as-is I think.

Dylan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: signature
URL: <https://lists.freedesktop.org/archives/piglit/attachments/20171019/e672f435/attachment.sig>