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

[Eric Anholt]

Dylan Baker <dylan at pnwbakers.com> writes:

> [ Unknown signature status ]
> Quoting Martin Peres (2017-10-20 00:38:18)
>> On 19/10/17 19:50, Dylan Baker wrote:
>> > 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.
>> 
>> The problem is that execution errors, or abort-on also exit with error 
>> code 1. So either we introduce a new one for this particular error, or 
>> we just return 0 which means all went well.
>> 
>> Thinking more about it though, we should still generate results, just 
>> with nothing inside (since we asked for nothing). This way, we don't 
>> need to special case the fact that the test list is empty.
>> 
>> What do you think?
>
> I don't like generating empty results, I think it violates least surprise, you
> shouldn't get results if there's nothing to put in them. Especially since the
> most common case for empty results is user error, such as using -x and -t
> options such that no tests are run. Which, actually, is something that
> non-automated users have complained about in the past.
>
> We do have a PiglitUserError that has returncode 3, or we could extend
> PiglitFatalError to also take an int argument that is the returncode on exit.

Please just generate empty results without blowing up.  I use "-t
notarealtest" to cut out the piglit test component of X server testing
when I'm trying to get quick runs of the rest of the tests, but then I
have to also filter out the piglit framework dying to find my errors.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/piglit/attachments/20171020/c0903d01/attachment-0001.sig>