[Piglit] [PATCH] util: keep track of failures in subtests

[Martin Peres]

On 03/03/15 14:55, Jose Fonseca wrote:
> On 26/02/15 16:06, Martin Peres wrote:
>> ---
>>   tests/util/piglit-util.c | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/tests/util/piglit-util.c b/tests/util/piglit-util.c
>> index b094625..b5bd89f 100644
>> --- a/tests/util/piglit-util.c
>> +++ b/tests/util/piglit-util.c
>> @@ -273,6 +273,20 @@ piglit_set_timeout(double seconds, enum 
>> piglit_result timeout_result)
>>   #endif
>>   }
>>
>
>
>> +static bool _piglit_subtests_all_passed = true;
>> +
>> +bool piglit_subtests_all_passed()
>> +{
>> +    return _piglit_subtests_all_passed;
>> +}
>> +
>> +bool piglit_subtests_all_passed_reset()
>> +{
>> +    bool ret = _piglit_subtests_all_passed;
>> +    _piglit_subtests_all_passed = true;
>> +    return ret;
>> +}
>> +
>>   void
>>   piglit_report_subtest_result(enum piglit_result result, const char 
>> *format, ...)
>>   {
>> @@ -287,6 +301,9 @@ piglit_report_subtest_result(enum piglit_result 
>> result, const char *format, ...)
>>       fflush(stdout);
>>
>>       va_end(ap);
>> +
>> +    if (result == PIGLIT_FAIL)
>> +        _piglit_subtests_all_passed = false;
>>   }
>>
>>   #ifdef _WIN32
>>
>
> Sorry for the delay.  This looks good overall, though it might be 
> better to change the interface to use enum piglit_result instead of 
> bool, so that it eventually it handles WARP/SKIP better (eg., if all 
> subtests SKIP, then report SKIP).

No probs for the delay. That is indeed a good proposition.
>
> BTW, is it really true that if one subtest fails, the overall test 
> should be false?  For example, when I was looking at 
> iglit_report_subtest_result callers, I noticed that 
> tests/spec/amd_performance_monitor/api.c will not report false 
> overall, even though may report true.

That is a matter of conventions I guess but to me it does not make sense 
to say that a test passed if some parts of it failed. Has anyone a test 
in mind where it would make sense? Worst case, we could make a function 
that does not update the global state.

>
>
> Maybe a simpler approach would be to the piglit/framework to handle 
> this universally.

Indeed, that could be a possibility. Let piglit return PASS only if the 
the global result was pass and all the subtests passed.