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

[Karthik B S <karthik.b.s@intel.com>]

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@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
>
> Regards,
> Kamil
>
>>   
>>   	igt_subtest_group {
>>   		igt_fixture
>> -- 
>> 2.43.0
>>