* [PATCH i-g-t v2] tests/gem_flink_basic: Add documentation for subtests
@ 2017-08-31 21:33 Vinay Belgaumkar
2017-08-31 22:11 ` ✓ Fi.CI.BAT: success for tests/gem_flink_basic: Add documentation for subtests (rev2) Patchwork
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Vinay Belgaumkar @ 2017-08-31 21:33 UTC (permalink / raw)
To: intel-gfx
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@intel.com>
Cc: Eric Anholt <eric@anholt.net>
Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@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");
+
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:
+ * Test creation and use of flink.
+ */
igt_subtest("basic")
test_flink(fd);
+
+ /**
+ * 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.
+ */
igt_subtest("flink-lifetime")
test_flink_lifetime(fd);
}
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread
* ✓ Fi.CI.BAT: success for tests/gem_flink_basic: Add documentation for subtests (rev2)
2017-08-31 21:33 [PATCH i-g-t v2] tests/gem_flink_basic: Add documentation for subtests Vinay Belgaumkar
@ 2017-08-31 22:11 ` Patchwork
2017-09-01 2:04 ` ✓ Fi.CI.IGT: " Patchwork
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2017-08-31 22:11 UTC (permalink / raw)
To: Vinay Belgaumkar; +Cc: intel-gfx
== Series Details ==
Series: tests/gem_flink_basic: Add documentation for subtests (rev2)
URL : https://patchwork.freedesktop.org/series/29499/
State : success
== Summary ==
IGT patchset tested on top of latest successful build
5ce65a9a51f17e0183e3e4f8943981ee7b96cadd pm_rps: Changes in waitboost scenario
with latest DRM-Tip kernel build CI_DRM_3023
ccf4ca2d9338 drm-tip: 2017y-08m-31d-18h-37m-46s UTC integration manifest
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-legacy:
fail -> PASS (fi-snb-2600) fdo#100215
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fi-bdw-5557u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:458s
fi-bdw-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:444s
fi-blb-e6850 total:288 pass:224 dwarn:1 dfail:0 fail:0 skip:63 time:364s
fi-bsw-n3050 total:288 pass:243 dwarn:0 dfail:0 fail:0 skip:45 time:568s
fi-bwr-2160 total:288 pass:184 dwarn:0 dfail:0 fail:0 skip:104 time:255s
fi-bxt-j4205 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:528s
fi-byt-j1900 total:288 pass:254 dwarn:1 dfail:0 fail:0 skip:33 time:522s
fi-byt-n2820 total:288 pass:250 dwarn:1 dfail:0 fail:0 skip:37 time:521s
fi-elk-e7500 total:288 pass:230 dwarn:0 dfail:0 fail:0 skip:58 time:435s
fi-glk-2a total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:612s
fi-hsw-4770 total:288 pass:261 dwarn:0 dfail:0 fail:2 skip:25 time:464s
fi-hsw-4770r total:288 pass:263 dwarn:0 dfail:0 fail:0 skip:25 time:425s
fi-ilk-650 total:288 pass:229 dwarn:0 dfail:0 fail:0 skip:59 time:424s
fi-ivb-3520m total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:505s
fi-ivb-3770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:481s
fi-kbl-7500u total:288 pass:264 dwarn:1 dfail:0 fail:0 skip:23 time:518s
fi-kbl-7560u total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:600s
fi-kbl-r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:599s
fi-pnv-d510 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:528s
fi-skl-6260u total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:469s
fi-skl-6700k total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:538s
fi-skl-6770hq total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:496s
fi-skl-gvtdvm total:288 pass:266 dwarn:0 dfail:0 fail:0 skip:22 time:444s
fi-skl-x1585l total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:496s
fi-snb-2520m total:288 pass:251 dwarn:0 dfail:0 fail:0 skip:37 time:551s
fi-snb-2600 total:288 pass:250 dwarn:0 dfail:0 fail:0 skip:38 time:405s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_135/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* ✓ Fi.CI.IGT: success for tests/gem_flink_basic: Add documentation for subtests (rev2)
2017-08-31 21:33 [PATCH i-g-t v2] tests/gem_flink_basic: Add documentation for subtests Vinay Belgaumkar
2017-08-31 22:11 ` ✓ Fi.CI.BAT: success for tests/gem_flink_basic: Add documentation for subtests (rev2) Patchwork
@ 2017-09-01 2:04 ` Patchwork
2017-09-01 10:54 ` tests/gem_flink_basic: Add documentation for subtests Katarzyna Dec
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2017-09-01 2:04 UTC (permalink / raw)
To: Vinay Belgaumkar; +Cc: intel-gfx
== Series Details ==
Series: tests/gem_flink_basic: Add documentation for subtests (rev2)
URL : https://patchwork.freedesktop.org/series/29499/
State : success
== Summary ==
Test perf:
Subgroup polling:
fail -> PASS (shard-hsw) fdo#102252
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
shard-hsw total:2265 pass:1233 dwarn:0 dfail:0 fail:16 skip:1016 time:9611s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_135/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: tests/gem_flink_basic: Add documentation for subtests
2017-08-31 21:33 [PATCH i-g-t v2] tests/gem_flink_basic: Add documentation for subtests Vinay Belgaumkar
2017-08-31 22:11 ` ✓ Fi.CI.BAT: success for tests/gem_flink_basic: Add documentation for subtests (rev2) Patchwork
2017-09-01 2:04 ` ✓ Fi.CI.IGT: " Patchwork
@ 2017-09-01 10:54 ` Katarzyna Dec
2017-09-01 11:55 ` [PATCH i-g-t v2] " Arkadiusz Hiler
2017-09-04 8:30 ` Daniel Vetter
4 siblings, 0 replies; 11+ messages in thread
From: Katarzyna Dec @ 2017-09-01 10:54 UTC (permalink / raw)
To: intel-gfx
> Date: Thu, 31 Aug 2017 14:33:23 -0700
> From: Vinay Belgaumkar <vinay.belgaumkar@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.belgaumkar@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@intel.com>
> Cc: Eric Anholt <eric@anholt.net>
>
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH i-g-t v2] tests/gem_flink_basic: Add documentation for subtests
2017-08-31 21:33 [PATCH i-g-t v2] tests/gem_flink_basic: Add documentation for subtests Vinay Belgaumkar
` (2 preceding siblings ...)
2017-09-01 10:54 ` tests/gem_flink_basic: Add documentation for subtests Katarzyna Dec
@ 2017-09-01 11:55 ` Arkadiusz Hiler
2017-09-04 7:02 ` Katarzyna Dec
2017-09-06 0:04 ` Belgaumkar, Vinay
2017-09-04 8:30 ` Daniel Vetter
4 siblings, 2 replies; 11+ messages in thread
From: Arkadiusz Hiler @ 2017-09-01 11:55 UTC (permalink / raw)
To: Vinay Belgaumkar; +Cc: intel-gfx
On Thu, Aug 31, 2017 at 02:33:23PM -0700, Vinay Belgaumkar wrote:
> Added the missing IGT_TEST_DESCRIPTION and some subtest
> descriptions. Trying to establish a method to document
Hey Vinay,
Please add appropriate tag to the subject, as this is clearly an RFC.
> 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@intel.com>
> Cc: Eric Anholt <eric@anholt.net>
>
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@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");
> +
> 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:
Much better than the previous proposal.
But I do not like having the subtest name twice - in the comment and in
the igt_subtest().
IMO it's better to have a parser that can extract the name from the
following igt_subtest() than having a place where when we have
duplication and we can go out of sync.
I've been following your efforts and I have some question and thoughts
to share.
### Questions
1. What's the actual problem statement? What are you trying to solve
here?
2. Who, in your mind, is the supposed reader of the documentation?
How's that different form someone who is supposed to look at the
code directly?
3. Why are you trying to drive this? What's your motivation?
### My Two (Euro)cents
As Michal stressed, in a reply to the previous revision, we should not
be doing C to English translation. My reasoning:
1. Maintenance hell - if you describe inner workings of tests in too
much detail, you will have two places that you have to update when you
are making even the slightest adjustments.
And people will forget to update the comments, and will receive negative
reviews, and will have to respin the series making the changes again,
this time in the "English" implementation.
To me it's unnecessary rising of the bar for the contributors.
2. Code is enough - I think it's safe to assume that anyone who is
enough technically inclined to understand the English translation of the
code will be able to understand the code itself.
And the code is openly and freely available. So I do not see much use of
embedding it into the documentation.
3. The more manual tasks we have for the tests developers, the less
appealing the project is. If it will get unpleasant, the people will
think twice about contributing - and not to contribute better things,
but whether the chores are worth their time.
### My Expectations
Definitely we should improve IGT documentation and general readability.
But having too much documentation is even wore.
1. Subtest documentation should be as brief as possible and give you good
intuition on what it is exercising - for actual details people should
refer the source code.
2. It should not describe the "feature" it is testing, there are other
places to do that. It should just give enough of a context to be
understood by someone who has the general idea of the "feature".
3. It should feel like an added value to the developers, not as a
unnecessary, manual chore.
4. It should feel natural - it should just take a single glance at the
surrounding code and developer should know what to do - how the
commenting is done, what style should be assumed. Many people do not
read guidelines, and the longer the file is, the less likely is that
someone will read it through. The guidelines should be simple, to the
point, statements, that are supposed by actual good examples.
It's also easier to get a good idea what to do when you are are pointed to
good code in the actual code base instead of some artificial example.
5. Comments inside the tests should be "the last resort", if the code
could not be rewritten easily in more readable manner. Their main
purpose is to help people reading code understand non-obvious parts.
This should be enforced by reviewers - if it takes you too long to
understand something, comment on that, possibly with suggested
rewrite/proposition of a comment.
Looking forward to your reply!
--
Cheers,
Arek
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH i-g-t v2] tests/gem_flink_basic: Add documentation for subtests
2017-09-01 11:55 ` [PATCH i-g-t v2] " Arkadiusz Hiler
@ 2017-09-04 7:02 ` Katarzyna Dec
2017-09-06 0:04 ` Belgaumkar, Vinay
1 sibling, 0 replies; 11+ messages in thread
From: Katarzyna Dec @ 2017-09-04 7:02 UTC (permalink / raw)
To: Belgaumkar, Vinay; +Cc: intel-gfx
On Fri, Sep 01, 2017 at 12:55:37PM +0100, Arkadiusz Hiler wrote:
> On Thu, Aug 31, 2017 at 02:33:23PM -0700, Vinay Belgaumkar wrote:
> > Added the missing IGT_TEST_DESCRIPTION and some subtest
> > descriptions. Trying to establish a method to document
>
> Hey Vinay,
>
> Please add appropriate tag to the subject, as this is clearly an RFC.
>
> > 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@intel.com>
> > Cc: Eric Anholt <eric@anholt.net>
> >
> > Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@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");
> > +
> > 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:
I aggree with Arek that there is no need to tell explicitly testcase
name. It is seen from the code.
>
> Much better than the previous proposal.
>
> But I do not like having the subtest name twice - in the comment and in
> the igt_subtest().
>
> IMO it's better to have a parser that can extract the name from the
> following igt_subtest() than having a place where when we have
> duplication and we can go out of sync.
>
>
>
> I've been following your efforts and I have some question and thoughts
> to share.
>
> ### Questions
>
> 1. What's the actual problem statement? What are you trying to solve
> here?
>
> 2. Who, in your mind, is the supposed reader of the documentation?
> How's that different form someone who is supposed to look at the
> code directly?
>
> 3. Why are you trying to drive this? What's your motivation?
>
>
> ### My Two (Euro)cents
>
> As Michal stressed, in a reply to the previous revision, we should not
> be doing C to English translation. My reasoning:
>
> 1. Maintenance hell - if you describe inner workings of tests in too
> much detail, you will have two places that you have to update when you
> are making even the slightest adjustments.
>
> And people will forget to update the comments, and will receive negative
> reviews, and will have to respin the series making the changes again,
> this time in the "English" implementation.
>
> To me it's unnecessary rising of the bar for the contributors.
>
> 2. Code is enough - I think it's safe to assume that anyone who is
> enough technically inclined to understand the English translation of the
> code will be able to understand the code itself.
>
> And the code is openly and freely available. So I do not see much use of
> embedding it into the documentation.
>
> 3. The more manual tasks we have for the tests developers, the less
> appealing the project is. If it will get unpleasant, the people will
> think twice about contributing - and not to contribute better things,
> but whether the chores are worth their time.
>
>
> ### My Expectations
>
> Definitely we should improve IGT documentation and general readability.
> But having too much documentation is even wore.
>
> 1. Subtest documentation should be as brief as possible and give you good
> intuition on what it is exercising - for actual details people should
> refer the source code.
>
> 2. It should not describe the "feature" it is testing, there are other
> places to do that. It should just give enough of a context to be
> understood by someone who has the general idea of the "feature".
>
> 3. It should feel like an added value to the developers, not as a
> unnecessary, manual chore.
>
> 4. It should feel natural - it should just take a single glance at the
> surrounding code and developer should know what to do - how the
> commenting is done, what style should be assumed. Many people do not
> read guidelines, and the longer the file is, the less likely is that
> someone will read it through. The guidelines should be simple, to the
> point, statements, that are supposed by actual good examples.
> It's also easier to get a good idea what to do when you are are pointed to
> good code in the actual code base instead of some artificial example.
>
> 5. Comments inside the tests should be "the last resort", if the code
> could not be rewritten easily in more readable manner. Their main
> purpose is to help people reading code understand non-obvious parts.
> This should be enforced by reviewers - if it takes you too long to
> understand something, comment on that, possibly with suggested
> rewrite/proposition of a comment.
>
In my opinion test should be written in a way that person familiar with
IGT and kernel code is able to tell what is going on.
We could add some brief info what/how is tested. But only brief one.
Too much documentation will be confsing and will be a burden to
developers.
Do we really want that?
Regards,
Kasia
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH i-g-t v2] tests/gem_flink_basic: Add documentation for subtests
2017-08-31 21:33 [PATCH i-g-t v2] tests/gem_flink_basic: Add documentation for subtests Vinay Belgaumkar
` (3 preceding siblings ...)
2017-09-01 11:55 ` [PATCH i-g-t v2] " Arkadiusz Hiler
@ 2017-09-04 8:30 ` Daniel Vetter
2017-09-05 23:34 ` Belgaumkar, Vinay
4 siblings, 1 reply; 11+ messages in thread
From: Daniel Vetter @ 2017-09-04 8:30 UTC (permalink / raw)
To: Vinay Belgaumkar; +Cc: intel-gfx
On Thu, Aug 31, 2017 at 02:33:23PM -0700, Vinay Belgaumkar wrote:
> 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@intel.com>
> Cc: Eric Anholt <eric@anholt.net>
>
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@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");
> +
> 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");
Do we really want to remove runtime-dumped information (maybe we could
tune it down to debug level) with comments that in my experience, no one
ever reads?
I just looked again at the igt library docs, and like last year when I've
done that, we've accumulated sizeable chunks of missing docs and warnings.
So who's going to maintain this? We can barely keep docs in shape for the
core libs, how exactly are we going to keep docs in shape for the hundreds
of testcases we have? Do you have a team of 10+ people working on this?
Same about IGT_TEST_DESCRIPTION, not enough people seem to care even for
that simple one-liner to make it work.
My proposal would be that we first try to get the docs we have into decent
shape, which means establishing as something everyone takes care of.
Adding more lofty documentation goals that won't pan out imo just doesn't
make much sense.
And yes this is from the person who did push for docs almost everywhere.
It's hard work, and it's a lot of hard work.
-Daniel
> -
> 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:
> + * Test creation and use of flink.
> + */
> igt_subtest("basic")
> test_flink(fd);
> +
> + /**
> + * 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.
> + */
> igt_subtest("flink-lifetime")
> test_flink_lifetime(fd);
> }
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH i-g-t v2] tests/gem_flink_basic: Add documentation for subtests
2017-09-04 8:30 ` Daniel Vetter
@ 2017-09-05 23:34 ` Belgaumkar, Vinay
2017-09-08 6:45 ` Daniel Vetter
0 siblings, 1 reply; 11+ messages in thread
From: Belgaumkar, Vinay @ 2017-09-05 23:34 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On 9/4/2017 1:30 AM, Daniel Vetter wrote:
> On Thu, Aug 31, 2017 at 02:33:23PM -0700, Vinay Belgaumkar wrote:
>> 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@intel.com>
>> Cc: Eric Anholt <eric@anholt.net>
>>
>> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@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");
>> +
>> 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");
>
> Do we really want to remove runtime-dumped information (maybe we could
> tune it down to debug level) with comments that in my experience, no one
> ever reads?
I removed that to avoid duplication as per the previous review comments.
I do think it's useful to have both.
>
> I just looked again at the igt library docs, and like last year when I've
> done that, we've accumulated sizeable chunks of missing docs and warnings.
> So who's going to maintain this? We can barely keep docs in shape for the
> core libs, how exactly are we going to keep docs in shape for the hundreds
> of testcases we have? Do you have a team of 10+ people working on this?
I think this will be more effective if this becomes a collective
responsibility, not just an individual team's effort. We should enforce
regular updates to the documentation during code reviews. We do not need
to start updating all 300+ tests for now. We can add this form of
documentation whenever we encounter a test that is hard to follow or
needs clarification. Same goes for the libraries.
>
> Same about IGT_TEST_DESCRIPTION, not enough people seem to care even for
> that simple one-liner to make it work.
>
> My proposal would be that we first try to get the docs we have into decent
> shape, which means establishing as something everyone takes care of.
> Adding more lofty documentation goals that won't pan out imo just doesn't
> make much sense.
Agree, but if we really want to make this efficient, we can look up the
API documentation while describing a certain test and update both as
necessary.
>
> And yes this is from the person who did push for docs almost everywhere.
> It's hard work, and it's a lot of hard work.
> -Daniel
>
I agree. There still seem to be mixed views on even whether we need
documentation or not.
>> -
>> 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:
>> + * Test creation and use of flink.
>> + */
>> igt_subtest("basic")
>> test_flink(fd);
>> +
>> + /**
>> + * 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.
>> + */
>> igt_subtest("flink-lifetime")
>> test_flink_lifetime(fd);
>> }
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH i-g-t v2] tests/gem_flink_basic: Add documentation for subtests
2017-09-01 11:55 ` [PATCH i-g-t v2] " Arkadiusz Hiler
2017-09-04 7:02 ` Katarzyna Dec
@ 2017-09-06 0:04 ` Belgaumkar, Vinay
2017-09-07 12:37 ` Arkadiusz Hiler
1 sibling, 1 reply; 11+ messages in thread
From: Belgaumkar, Vinay @ 2017-09-06 0:04 UTC (permalink / raw)
To: Arkadiusz Hiler; +Cc: intel-gfx
On 9/1/2017 4:55 AM, Arkadiusz Hiler wrote:
> On Thu, Aug 31, 2017 at 02:33:23PM -0700, Vinay Belgaumkar wrote:
>> Added the missing IGT_TEST_DESCRIPTION and some subtest
>> descriptions. Trying to establish a method to document
>
> Hey Vinay,
>
> Please add appropriate tag to the subject, as this is clearly an RFC.
>
>> 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@intel.com>
>> Cc: Eric Anholt <eric@anholt.net>
>>
>> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@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");
>> +
>> 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:
>
> Much better than the previous proposal.
>
> But I do not like having the subtest name twice - in the comment and in
> the igt_subtest().
>
> IMO it's better to have a parser that can extract the name from the
> following igt_subtest() than having a place where when we have
> duplication and we can go out of sync.
Agreed. If the new documentation tool can support parsing of
igt_subtest, then we do not need this. Although, when someone tries to
rearrange code, it might prove useful to have the name of the subtest to
which the documentation belongs.
>
>
>
> I've been following your efforts and I have some question and thoughts
> to share.
>
> ### Questions
>
> 1. What's the actual problem statement? What are you trying to solve
> here?
Subtests do not have documentation, I am trying to add that.
>
> 2. Who, in your mind, is the supposed reader of the documentation?
> How's that different form someone who is supposed to look at the
> code directly?
Test/kernel developers and anyone who uses these tests.
>
> 3. Why are you trying to drive this? What's your motivation?
Our team is moving to use IGT instead of our internal test framework. We
think it will be easier for folks to ramp up on these tests if there was
sufficient documentation.
>
>
> ### My Two (Euro)cents
>
> As Michal stressed, in a reply to the previous revision, we should not
> be doing C to English translation. My reasoning:
>
> 1. Maintenance hell - if you describe inner workings of tests in too
> much detail, you will have two places that you have to update when you
> are making even the slightest adjustments.
>
> And people will forget to update the comments, and will receive negative
> reviews, and will have to respin the series making the changes again,
> this time in the "English" implementation.
>
> To me it's unnecessary rising of the bar for the contributors.
>
> 2. Code is enough - I think it's safe to assume that anyone who is
> enough technically inclined to understand the English translation of the
> code will be able to understand the code itself.
>
> And the code is openly and freely available. So I do not see much use of
> embedding it into the documentation.
>
> 3. The more manual tasks we have for the tests developers, the less
> appealing the project is. If it will get unpleasant, the people will
> think twice about contributing - and not to contribute better things,
> but whether the chores are worth their time.
>
Not sure how I can convince that we need documentation apart from the
usual advantages of improving code readability. As for maintenance, I
have tried to make the description as short as possible. We already
document APIs and code today, so it's not a big change that is being
suggested.
>
> ### My Expectations
>
> Definitely we should improve IGT documentation and general readability.
> But having too much documentation is even wore.
>
> 1. Subtest documentation should be as brief as possible and give you good
> intuition on what it is exercising - for actual details people should
> refer the source code.
Agreed. My second version attempts to meet those goals.
>
> 2. It should not describe the "feature" it is testing, there are other
> places to do that. It should just give enough of a context to be
> understood by someone who has the general idea of the "feature".
I tried that approach, but other opinions seem to suggest there is no
point in describing what the test is doing, since the test code will
elaborate on that anyways.
>
> 3. It should feel like an added value to the developers, not as a
> unnecessary, manual chore.
Agree.
>
> 4. It should feel natural - it should just take a single glance at the
> surrounding code and developer should know what to do - how the
> commenting is done, what style should be assumed. Many people do not
> read guidelines, and the longer the file is, the less likely is that
> someone will read it through. The guidelines should be simple, to the
> point, statements, that are supposed by actual good examples.
> It's also easier to get a good idea what to do when you are are pointed to
> good code in the actual code base instead of some artificial example.
The guidelines are the same as API documentation, so that should help.
>
> 5. Comments inside the tests should be "the last resort", if the code
> could not be rewritten easily in more readable manner. Their main
> purpose is to help people reading code understand non-obvious parts.
> This should be enforced by reviewers - if it takes you too long to
> understand something, comment on that, possibly with suggested
> rewrite/proposition of a comment.
I would argue that anybody looking at test code for the first time would
try and find some form of documentation about it. You can write code in
a very lucid manner, but given the feature complexities, some form of
feature description is definitely helpful.
>
>
> Looking forward to your reply!
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH i-g-t v2] tests/gem_flink_basic: Add documentation for subtests
2017-09-06 0:04 ` Belgaumkar, Vinay
@ 2017-09-07 12:37 ` Arkadiusz Hiler
0 siblings, 0 replies; 11+ messages in thread
From: Arkadiusz Hiler @ 2017-09-07 12:37 UTC (permalink / raw)
To: Belgaumkar, Vinay; +Cc: intel-gfx
On Tue, Sep 05, 2017 at 05:04:35PM -0700, Belgaumkar, Vinay wrote:
>
>
> On 9/1/2017 4:55 AM, Arkadiusz Hiler wrote:
> > On Thu, Aug 31, 2017 at 02:33:23PM -0700, Vinay Belgaumkar wrote:
> > > Added the missing IGT_TEST_DESCRIPTION and some subtest
> > > descriptions. Trying to establish a method to document
> >
> > Hey Vinay,
> >
> > Please add appropriate tag to the subject, as this is clearly an RFC.
> >
> > > 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@intel.com>
> > > Cc: Eric Anholt <eric@anholt.net>
> > >
> > > Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@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");
> > > +
> > > 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:
> >
> > Much better than the previous proposal.
> >
> > But I do not like having the subtest name twice - in the comment and in
> > the igt_subtest().
> >
> > IMO it's better to have a parser that can extract the name from the
> > following igt_subtest() than having a place where when we have
> > duplication and we can go out of sync.
>
> Agreed. If the new documentation tool can support parsing of igt_subtest,
> then we do not need this. Although, when someone tries to rearrange code, it
> might prove useful to have the name of the subtest to which the
> documentation belongs.
Nope. Redundancy works in very few cases. Changes like those you've
mentioned should be easy to catch in a review. I don't see that as
enough of a justification.
> > I've been following your efforts and I have some question and thoughts
> > to share.
> >
> > ### Questions
> >
> > 1. What's the actual problem statement? What are you trying to solve
> > here?
>
> Subtests do not have documentation, I am trying to add that.
That's not a problem statement with a proposed solution.
That's having the documentation for the sake of having documentation.
"I want to paint walls blue because they are not blue".
I was expecting (basing on your past replies) something like:
"A lot of tests are hard to understand. We want to make them easier to
digest by adding comments on the unclear parts."
Was I wrong? Is it actually about having a documentation just for the
sake of it?
Having clear goals for documentation should help to come up with
solution meeting them.
> > 2. Who, in your mind, is the supposed reader of the documentation?
> > How's that different form someone who is supposed to look at the
> > code directly?
>
> Test/kernel developers and anyone who uses these tests.
So basically "everyone".
What would they expect from the documentation and why?
How is "test runner" different form "kernel developer"?
IMHO it's very important to have understanding who is the intended
reader of the documentation, how tech-savvy they are, what they may
expect and how capable they are using complementary sources of
information.
> > 3. Why are you trying to drive this? What's your motivation?
>
> Our team is moving to use IGT instead of our internal test framework. We
> think it will be easier for folks to ramp up on these tests if there was
> sufficient documentation.
Looking forward to seeing more contributions from the team in the future
then :-)
> > ### My Two (Euro)cents
> >
> > As Michal stressed, in a reply to the previous revision, we should not
> > be doing C to English translation. My reasoning:
> >
> > 1. Maintenance hell - if you describe inner workings of tests in too
> > much detail, you will have two places that you have to update when you
> > are making even the slightest adjustments.
> >
> > And people will forget to update the comments, and will receive negative
> > reviews, and will have to respin the series making the changes again,
> > this time in the "English" implementation.
> >
> > To me it's unnecessary rising of the bar for the contributors.
> >
> > 2. Code is enough - I think it's safe to assume that anyone who is
> > enough technically inclined to understand the English translation of the
> > code will be able to understand the code itself.
> >
> > And the code is openly and freely available. So I do not see much use of
> > embedding it into the documentation.
> >
> > 3. The more manual tasks we have for the tests developers, the less
> > appealing the project is. If it will get unpleasant, the people will
> > think twice about contributing - and not to contribute better things,
> > but whether the chores are worth their time.
> >
>
> Not sure how I can convince that we need documentation apart from the usual
> advantages of improving code readability. As for maintenance, I have tried
> to make the description as short as possible. We already document APIs and
> code today, so it's not a big change that is being suggested.
But you do not have to convince me that IGTs can be hard to digest and
could use some good ol' documentation here and there.
I just want to ensure that we won't find ourselves knee deep in
unenforcible guidelines that impair development instead of accelerating
it and making it more friendly.
I just want to be realistic.
> > ### My Expectations
> >
> > Definitely we should improve IGT documentation and general readability.
> > But having too much documentation is even wore.
> >
> > 1. Subtest documentation should be as brief as possible and give you good
> > intuition on what it is exercising - for actual details people should
> > refer the source code.
>
> Agreed. My second version attempts to meet those goals.
> >
> > 2. It should not describe the "feature" it is testing, there are other
> > places to do that. It should just give enough of a context to be
> > understood by someone who has the general idea of the "feature".
>
> I tried that approach, but other opinions seem to suggest there is no point
> in describing what the test is doing, since the test code will elaborate on
> that anyways.
Not in depth. Just something that gives you a decent idea what is
happening, then you can dig into the code (or not).
> > 3. It should feel like an added value to the developers, not as a
> > unnecessary, manual chore.
>
> Agree.
>
> >
> > 4. It should feel natural - it should just take a single glance at the
> > surrounding code and developer should know what to do - how the
> > commenting is done, what style should be assumed. Many people do not
> > read guidelines, and the longer the file is, the less likely is that
> > someone will read it through. The guidelines should be simple, to the
> > point, statements, that are supposed by actual good examples.
> > It's also easier to get a good idea what to do when you are are pointed to
> > good code in the actual code base instead of some artificial example.
>
> The guidelines are the same as API documentation, so that should help.
>
> >
> > 5. Comments inside the tests should be "the last resort", if the code
> > could not be rewritten easily in more readable manner. Their main
> > purpose is to help people reading code understand non-obvious parts.
> > This should be enforced by reviewers - if it takes you too long to
> > understand something, comment on that, possibly with suggested
> > rewrite/proposition of a comment.
>
> I would argue that anybody looking at test code for the first time would try
> and find some form of documentation about it. You can write code in a very
> lucid manner, but given the feature complexities, some form of feature
> description is definitely helpful.
Yes, but only when giving additional context to the code. I am arguing
against full documentation of the features or facilities. That's the
kernel/PRMs/system_manual responsibility.
> > Looking forward to your reply!
--
Cheers,
Arek
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH i-g-t v2] tests/gem_flink_basic: Add documentation for subtests
2017-09-05 23:34 ` Belgaumkar, Vinay
@ 2017-09-08 6:45 ` Daniel Vetter
0 siblings, 0 replies; 11+ messages in thread
From: Daniel Vetter @ 2017-09-08 6:45 UTC (permalink / raw)
To: Belgaumkar, Vinay; +Cc: intel-gfx
On Tue, Sep 05, 2017 at 04:34:32PM -0700, Belgaumkar, Vinay wrote:
>
>
> On 9/4/2017 1:30 AM, Daniel Vetter wrote:
> > On Thu, Aug 31, 2017 at 02:33:23PM -0700, Vinay Belgaumkar wrote:
> > > 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@intel.com>
> > > Cc: Eric Anholt <eric@anholt.net>
> > >
> > > Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@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");
> > > +
> > > 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");
> >
> > Do we really want to remove runtime-dumped information (maybe we could
> > tune it down to debug level) with comments that in my experience, no one
> > ever reads?
>
> I removed that to avoid duplication as per the previous review comments. I
> do think it's useful to have both.
tbh I think the log output is more useful than 1-line comments that say
nothing more than what the testname already hints at.
> > I just looked again at the igt library docs, and like last year when I've
> > done that, we've accumulated sizeable chunks of missing docs and warnings.
> > So who's going to maintain this? We can barely keep docs in shape for the
> > core libs, how exactly are we going to keep docs in shape for the hundreds
> > of testcases we have? Do you have a team of 10+ people working on this?
>
> I think this will be more effective if this becomes a collective
> responsibility, not just an individual team's effort. We should enforce
> regular updates to the documentation during code reviews. We do not need to
> start updating all 300+ tests for now. We can add this form of documentation
> whenever we encounter a test that is hard to follow or needs clarification.
> Same goes for the libraries.
Yes. But just wishing that it becomes a collective effort doesn't make it
one. This requires hard work of convincing an overloaded team that the
added effort is worth it. And we can't even sustain that well for
libraries.
If you expect others to do more work, it won't happen if you don't put
some serious effort (and lots of good convincing in). And without that
effort this is just going to be another doc effort that fizzles out before
it pays off.
> > Same about IGT_TEST_DESCRIPTION, not enough people seem to care even for
> > that simple one-liner to make it work.
> >
> > My proposal would be that we first try to get the docs we have into decent
> > shape, which means establishing as something everyone takes care of.
> > Adding more lofty documentation goals that won't pan out imo just doesn't
> > make much sense.
>
> Agree, but if we really want to make this efficient, we can look up the API
> documentation while describing a certain test and update both as necessary.
>
> >
> > And yes this is from the person who did push for docs almost everywhere.
> > It's hard work, and it's a lot of hard work.
> > -Daniel
> >
>
> I agree. There still seem to be mixed views on even whether we need
> documentation or not.
Yes.
-Daniel
>
> > > -
> > > 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:
> > > + * Test creation and use of flink.
> > > + */
> > > igt_subtest("basic")
> > > test_flink(fd);
> > > +
> > > + /**
> > > + * 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.
> > > + */
> > > igt_subtest("flink-lifetime")
> > > test_flink_lifetime(fd);
> > > }
> > > --
> > > 1.9.1
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-09-08 6:45 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-31 21:33 [PATCH i-g-t v2] tests/gem_flink_basic: Add documentation for subtests Vinay Belgaumkar
2017-08-31 22:11 ` ✓ Fi.CI.BAT: success for tests/gem_flink_basic: Add documentation for subtests (rev2) Patchwork
2017-09-01 2:04 ` ✓ Fi.CI.IGT: " Patchwork
2017-09-01 10:54 ` tests/gem_flink_basic: Add documentation for subtests Katarzyna Dec
2017-09-01 11:55 ` [PATCH i-g-t v2] " Arkadiusz Hiler
2017-09-04 7:02 ` Katarzyna Dec
2017-09-06 0:04 ` Belgaumkar, Vinay
2017-09-07 12:37 ` Arkadiusz Hiler
2017-09-04 8:30 ` Daniel Vetter
2017-09-05 23:34 ` Belgaumkar, Vinay
2017-09-08 6:45 ` Daniel Vetter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.