Dylan Baker <dy...@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.hi...@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@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.
signature.asc
Description: PGP signature
_______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/piglit
