> Date: Thu, 31 Aug 2017 14:33:23 -0700
> From: Vinay Belgaumkar <vinay.belgaum...@intel.com>
> To: intel-gfx@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.belgaum...@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.winiar...@intel.com>
> Cc: Eric Anholt <e...@anholt.net>
> 
> Signed-off-by: Vinay Belgaumkar <vinay.belgaum...@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.
> +      */
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to