[PATCH i-g-t] tests/kms_addfb_basic: Split tests into subtest groups

[Kamil Konieczny]

Hi Karthik,
On 2025-03-14 at 15:13:57 +0530, Karthik B S wrote:
> Hi Kamil,
> 
> On 3/13/2025 2:13 AM, Kamil Konieczny wrote:
> > Hi Karthik,
> > On 2025-03-12 at 16:00:08 +0530, Karthik B S wrote:
> > > Multiple subtest groups exist in kms_addfb_basic with individual
> > > igt_fixutre calls. Split these using igt_subtest_group so that any
> > > failure in the fixture of one of the individual subtest groups don't
> > > affect the results of subtests in other groups.
> > > 
> > > Signed-off-by: Karthik B S <karthik.b.s at intel.com>
> > > ---
> > >   tests/kms_addfb_basic.c | 15 ++++++++++-----
> > >   1 file changed, 10 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/tests/kms_addfb_basic.c b/tests/kms_addfb_basic.c
> > > index e5ff9ea32..268e4e0e4 100644
> > > --- a/tests/kms_addfb_basic.c
> > > +++ b/tests/kms_addfb_basic.c
> > > @@ -989,15 +989,20 @@ igt_main
> > >   		igt_require(has_addfb2_iface(fd));
> > >   	}
> > > -	invalid_tests(fd);
> > > +	igt_subtest_group
> > > +		invalid_tests(fd);
> > > -	pitch_tests(fd);
> > > +	igt_subtest_group
> > > +		pitch_tests(fd);
> > > -	prop_tests(fd);
> > > +	igt_subtest_group
> > > +		prop_tests(fd);
> > > -	master_tests(fd);
> > > +	igt_subtest_group
> > > +		master_tests(fd);
> > > -	tiling_tests(fd);
> > > +	igt_subtest_group
> > > +		tiling_tests(fd);
> > I looked into those tests and imho this needs a major refactor,
> > for example there should not be any igt_assert() in a fixture.
> > Generally igt tests works ok for one fixture in begin, last one
> > at igt_main() end. When used in-between it becomes more
> > complicated to handle cleanup.
> 
> Thank you for the review.
> 
> I tried out by inducing an error using an igt_assert(0) within the
> igt_fixture of a subtest group and verified that the this change handles the
> issue by not failing the subtests in other subtest groups. As agreed
> offline, will use this as a quick fix to handle failures in the current
> design and for the long run will check if there is a way these fb
> tests(kms_addfb_basic, kms_getfb) can be redesigned to be more robust.
> 
> Thanks,
> Karthik.B.S

Yes, after a second reading this could be a first step
followed by refactoring later, so as this could improve
testing a little this is

Reviewed-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>

> > 
> > Regards,
> > Kamil
> > 
> > >   	igt_subtest_group {
> > >   		igt_fixture
> > > -- 
> > > 2.43.0
> > >