[igt-dev] [PATCH i-g-t] tests/kms_getfb: Use fixtures and subtest groups

[Antonio Argenziano]

On 30/03/18 08:49, Daniel Stone wrote:
> Make sure we gently skip when required features (CCS, getfb2) are not
> supported on the running kernel, and clean up properly after ourselves.
> 
> Signed-off-by: Daniel Stone <daniels at collabora.com>
> ---
>   tests/kms_getfb.c | 11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/kms_getfb.c b/tests/kms_getfb.c
> index 5f0bd2a9..81d796a4 100644
> --- a/tests/kms_getfb.c
> +++ b/tests/kms_getfb.c
> @@ -102,7 +102,7 @@ static void test_handle_input(int fd)
>   		add.pitches[0] = 1024*4;
>   		add.handles[0] = igt_create_bo_with_dimensions(fd, 1024, 1024,
>   			DRM_FORMAT_XRGB8888, 0, 0, NULL, NULL, NULL);
> -		igt_assert(add.handles[0]);
> +		igt_require(add.handles[0] != 0);
>   		do_ioctl(fd, DRM_IOCTL_MODE_ADDFB2, &add);
>   	}
>   
> @@ -134,6 +134,9 @@ static void test_handle_input(int fd)
>   		igt_require(get.fb_id > 0);
>   		do_ioctl_err(fd, DRM_IOCTL_MODE_GETFB, &get, ENOENT);
>   	}
> +
> +	igt_fixture
> +		gem_close(fd, add.handles[0]);
>   }
>   
>   static void test_duplicate_handles(int fd)
> @@ -198,9 +201,11 @@ igt_main
>   	igt_fixture
>   		fd = drm_open_driver_master(DRIVER_ANY);
>   
> -	test_handle_input(fd);
> +	igt_subtest_group
> +		test_handle_input(fd);
>   
> -	test_duplicate_handles(fd);
> +	igt_subtest_group
> +		test_duplicate_handles(fd);

Since both test_handle_input and test_duplicate_handles define some 
sub-tests and some fixtures, I think it would be better to unwrap those 
functions and do all fixtures and subtest definition in igt_main to make 
it more readable. It seems like you would also be able to reuse the same 
fixture across the two subtest_groups.

FWIW I also don't think the skips in CI are caused by this change.

Thanks,
Antonio

>   
>   	igt_fixture
>   		close(fd);
>