[Intel-gfx] tests/gem_flink_basic: Add documentation for subtests

[Katarzyna Dec]

> Date: Thu, 31 Aug 2017 14:33:23 -0700
> From: Vinay Belgaumkar <vinay.belgaumkar at intel.com>
> To: intel-gfx at lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH i-g-t v2] tests/gem_flink_basic: Add
> 	documentation for subtests
> Message-ID:
> 	<1504215203-197533-1-git-send-email-vinay.belgaumkar at intel.com>
> Content-Type: text/plain; charset=UTF-8
> 
> Added the missing IGT_TEST_DESCRIPTION and some subtest
> descriptions. Trying to establish a method to document
> subtests, it should describe the feature being tested
> rather than how. The HOW part can, if needed, be
> described in the test code.
> 
> Documenting subtests will give us a good way to trace
> feature test coverage, and also help a faster ramp
> for understanding the test code.
> 
> v2: Removed duplication, addressed comments, cc'd test author
> 
> Cc: Michał Winiarski <michal.winiarski at intel.com>
> Cc: Eric Anholt <eric at anholt.net>
> 
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar at intel.com>
> ---
>  tests/gem_flink_basic.c | 36 ++++++++++++++++++++++++++----------
>  1 file changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/tests/gem_flink_basic.c b/tests/gem_flink_basic.c
> index 26ae7d6..9c8c4c3 100644
> --- a/tests/gem_flink_basic.c
> +++ b/tests/gem_flink_basic.c
> @@ -36,6 +36,8 @@
>  #include <sys/ioctl.h>
>  #include "drm.h"
>  
> +IGT_TEST_DESCRIPTION("Tests for flink - a way to export a gem object by name");
Why do we need this tag for?
> +
>  static void
>  test_flink(int fd)
>  {
> @@ -44,8 +46,6 @@ test_flink(int fd)
>  	struct drm_gem_open open_struct;
>  	int ret;
>  
> -	igt_info("Testing flink and open.\n");
> -
>  	memset(&create, 0, sizeof(create));
>  	create.size = 16 * 1024;
>  	ret = ioctl(fd, DRM_IOCTL_I915_GEM_CREATE, &create);
> @@ -69,8 +69,6 @@ test_double_flink(int fd)
>  	struct drm_gem_flink flink2;
>  	int ret;
>  
> -	igt_info("Testing repeated flink.\n");
> -
>  	memset(&create, 0, sizeof(create));
>  	create.size = 16 * 1024;
>  	ret = ioctl(fd, DRM_IOCTL_I915_GEM_CREATE, &create);
> @@ -92,8 +90,6 @@ test_bad_flink(int fd)
>  	struct drm_gem_flink flink;
>  	int ret;
>  
> -	igt_info("Testing error return on bad flink ioctl.\n");
> -
>  	flink.handle = 0x10101010;
>  	ret = ioctl(fd, DRM_IOCTL_GEM_FLINK, &flink);
>  	igt_assert(ret == -1 && errno == ENOENT);
> @@ -105,8 +101,6 @@ test_bad_open(int fd)
>  	struct drm_gem_open open_struct;
>  	int ret;
>  
> -	igt_info("Testing error return on bad open ioctl.\n");
> -
>  	open_struct.name = 0x10101010;
>  	ret = ioctl(fd, DRM_IOCTL_GEM_OPEN, &open_struct);
>  
> @@ -121,8 +115,6 @@ test_flink_lifetime(int fd)
>  	struct drm_gem_open open_struct;
>  	int ret, fd2;
>  
> -	igt_info("Testing flink lifetime.\n");
> -
>  	fd2 = drm_open_driver(DRIVER_INTEL);
>  
>  	memset(&create, 0, sizeof(create));
> @@ -134,11 +126,13 @@ test_flink_lifetime(int fd)
>  	ret = ioctl(fd2, DRM_IOCTL_GEM_FLINK, &flink);
>  	igt_assert_eq(ret, 0);
>  
> +	/* Open another reference to the gem object */
>  	open_struct.name = flink.name;
>  	ret = ioctl(fd, DRM_IOCTL_GEM_OPEN, &open_struct);
>  	igt_assert_eq(ret, 0);
>  	igt_assert(open_struct.handle != 0);
>  
> +	/* Before closing the previous one */
>  	close(fd2);
>  	fd2 = drm_open_driver(DRIVER_INTEL);
>  
> @@ -155,14 +149,36 @@ igt_main
>  	igt_fixture
>  		fd = drm_open_driver(DRIVER_INTEL);
>  
> +	/**
> +	 * basic:
Do we need explicitly tell here what test is is? Isn't it obvious from
subtest name?
> +	 * Test creation and use of flink.
> +	 */
>  	igt_subtest("basic")
>  		test_flink(fd);
> +
> +	/**
I think double star in comment is not proper kernel comment style
> +	 * double-flink:
> +	 * This test validates the ability to create multiple flinks
> +	 * for the same gem object. They should obtain the same name.
> +	 */
>  	igt_subtest("double-flink")
>  		test_double_flink(fd);
> +
> +	/**
> +	 * bad-flink:
> +	 * Negative test for invalid flink usage.
> +	 */
>  	igt_subtest("bad-flink")
>  		test_bad_flink(fd);
> +
>  	igt_subtest("bad-open")
>  		test_bad_open(fd);
> +
> +	/**
> +	 * flink-lifetime:
> +	 * Flink lifetime is limited to that of the gem object it
> +	 * points to.
> +	 */