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

[Dylan Baker]

On Wed, Mar 04, 2015 at 10:00:03AM +0200, Martin Peres wrote:
> On 03/03/15 20:32, Dylan Baker wrote:
> > On Tue, Mar 03, 2015 at 03:12:51PM +0200, Martin Peres wrote:
> >> 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.
> >>
> > I agree with this, and it is what piglit currently does. Right now the
> > framework disregards the overall status of the test and return the
> > 'worst' subtest status as the overall test status.
> 
> Really? That sounds great! How can I use this behaviour?

Should be done automatically by the summary generator. I'm not sure if
that's being done in the raw result files, but it certainly could be
changed easily enough

> >
> >>>
> >>> 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.
> >> _______________________________________________
> >> Piglit mailing list
> >> Piglit at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/piglit
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20150304/e426c771/attachment.sig>