[Piglit] Regression with eeec306bc2197a2fc0eeb064462a1244d39c16fa "framework: don't report failed cases as PASS"

[Xing, Homer]

Hi Tom,

I have just sent out this patch, fixed the second problem you mentioned.

http://lists.freedesktop.org/archives/piglit/2013-August/007085.html 

Best regards,
Homer Hsing

-----Original Message-----
From: Xing, Homer 
Sent: Wednesday, August 28, 2013 9:21 AM
To: 'Tom Stellard'; Dylan Baker
Cc: piglit at lists.freedesktop.org
Subject: RE: [Piglit] Regression with eeec306bc2197a2fc0eeb064462a1244d39c16fa "framework: don't report failed cases as PASS"

Hi Tom,

Last subtest result should not overwrite the first.
When multiple subtests had the same name, PIGLIT:subtest {'+neg' : 'fail'} PIGLIT:subtest {'+neg' : 'pass'} The '+neg' subtest would be displayed as 'fail' in the summary.
Overwriting relationship between test cases should not exist.

For the second problem you mentioned, I admit it was not the intention of the patch. I will send out another patch to fix that problem.

Best regards,
Homer

-----Original Message-----
From: Tom Stellard [mailto:tom at stellard.net]
Sent: Tuesday, August 27, 2013 5:50 AM
To: Dylan Baker
Cc: piglit at lists.freedesktop.org; Xing, Homer
Subject: Re: [Piglit] Regression with eeec306bc2197a2fc0eeb064462a1244d39c16fa "framework: don't report failed cases as PASS"

On Mon, Aug 26, 2013 at 11:59:40AM -0700, Dylan Baker wrote:
> Tom,
> I'm a little confused, this was the exact intention of the patch series.
> 

My understanding was that the intention was to fix incorrect reporting when multiple subtests had the same name.  For example, with the old behavior, this:

PIGLIT:subtest {'+neg' : 'fail'}
PIGLIT:subtest {'+neg' : 'pass'}


The '+neg' subtest would be displayed as 'pass' in the summary, because the last subtest result was overwriting the first.  I thought the patch would fix this, so that '+neg' would display as 'fail' in the summary.

I think the patches fixes this, but it also introduces another problem with results like this:

PIGLIT:subtest {'-pos' : 'pass'}
PIGLIT:subtest {'+inf' : 'fail'}

In this case the summary shows both '-pos' and '+inf' as failed, which it shouldn't.  If this really was the intention of the patch, then I think it should be reverted, because it completed defeats the purpose of having subtests.

-Tom

> Dylan
> 
> On Monday 26 August 2013 10:09:10 Tom Stellard wrote:
> > Hi Homer,
> > 
> > This commit introduces a regression in piglit for reporting subtest 
> > results.  Now if one subtests fails, all subtests report failure, 
> > which is incorrect.  I've looked through the changes in this commit, 
> > and I don't understand which change introduced the regression, could 
> > you take a look?
> > 
> > Thanks,
> > Tom
> > _______________________________________________
> > Piglit mailing list
> > Piglit at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/piglit
>