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

[Martin Peres]

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?